1. 06 Jun, 2017 4 commits
    • Jacob Keller's avatar
      igb: fix race condition with PTP_TX_IN_PROGRESS bits · 4ccdc013
      Jacob Keller authored
      Hardware related to the igb driver has a limitation of only handling one
      Tx timestamp at a time. Thus, the driver uses a state bit lock to
      enforce that only one timestamp request is honored at a time.
      
      Unfortunately this suffers from a simple race condition. The bit lock is
      not cleared until after skb_tstamp_tx() is called notifying the stack of
      a new Tx timestamp. Even a well behaved application which sends only one
      timestamp request at once and waits for a response might wake up and
      send a new packet before the bit lock is cleared. This results in
      needlessly dropping some Tx timestamp requests.
      
      We can fix this by unlocking the state bit as soon as we read the
      Timestamp register, as this is the first point at which it is safe to
      unlock.
      
      To avoid issues with the skb pointer, we'll use a copy of the pointer
      and set the global variable in the driver structure to NULL first. This
      ensures that the next timestamp request does not modify our local copy
      of the skb pointer.
      
      This ensures that well behaved applications do not accidentally race
      with the unlock bit. Obviously an application which sends multiple Tx
      timestamp requests at once will still only timestamp one packet at
      a time. Unfortunately there is nothing we can do about this.
      Reported-by: default avatarDavid Mirabito <davidm@metamako.com>
      Signed-off-by: default avatarJacob Keller <jacob.e.keller@intel.com>
      Tested-by: default avatarAaron Brown <aaron.f.brown@intel.com>
      Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
      4ccdc013
    • Jacob Keller's avatar
      e1000e: fix race condition around skb_tstamp_tx() · 5012863b
      Jacob Keller authored
      The e1000e driver and related hardware has a limitation on Tx PTP
      packets which requires we limit to timestamping a single packet at once.
      We do this by verifying that we never request a new Tx timestamp while
      we still have a tx_hwtstamp_skb pointer.
      
      Unfortunately the driver suffers from a race condition around this. The
      tx_hwtstamp_skb pointer is not set to NULL until after skb_tstamp_tx()
      is called. This function notifies the stack and applications of a new
      timestamp. Even a well behaved application that only sends a new request
      when the first one is finished might be woken up and possibly send
      a packet before we can free the timestamp in the driver again. The
      result is that we needlessly ignore some Tx timestamp requests in this
      corner case.
      
      Fix this by assigning the tx_hwtstamp_skb pointer prior to calling
      skb_tstamp_tx() and use a temporary pointer to hold the timestamped skb
      until that function finishes. This ensures that the application is not
      woken up until the driver is ready to begin timestamping a new packet.
      
      This ensures that well behaved applications do not accidentally race
      with condition to skip Tx timestamps. Obviously an application which
      sends multiple Tx timestamp requests at once will still only timestamp
      one packet at a time. Unfortunately there is nothing we can do about
      this.
      Reported-by: default avatarDavid Mirabito <davidm@metamako.com>
      Signed-off-by: default avatarJacob Keller <jacob.e.keller@intel.com>
      Tested-by: default avatarAaron Brown <aaron.f.brown@intel.com>
      Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
      5012863b
    • Arnd Bergmann's avatar
      igb: mark PM functions as __maybe_unused · 000ba1f2
      Arnd Bergmann authored
      The new wake function is only used by the suspend/resume handlers that
      are defined in inside of an #ifdef, which can cause this harmless
      warning:
      
      drivers/net/ethernet/intel/igb/igb_main.c:7988:13: warning: 'igb_deliver_wake_packet' defined but not used [-Wunused-function]
      
      Removing the #ifdef, instead using a __maybe_unused annotation
      simplifies the code and avoids the warning.
      
      Fixes: b90fa876 ("igb: Enable reading of wake up packet")
      Signed-off-by: default avatarArnd Bergmann <arnd@arndb.de>
      Tested-by: default avatarAaron Brown <aaron.f.brown@intel.com>
      Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
      000ba1f2
    • Matwey V Kornilov's avatar
      igb: Explicitly select page 0 at initialization · 440aeca4
      Matwey V Kornilov authored
      The functions igb_read_phy_reg_gs40g/igb_write_phy_reg_gs40g (which were
      removed in 2a3cdead) explicitly selected the required page at every phy_reg
      access. Currently, igb_get_phy_id_82575 relays on the fact that page 0 is
      already selected. The assumption is not fulfilled for my Lex 3I380CW
      motherboard with integrated dual i211 based gigabit ethernet. This leads to igb
      initialization failure and network interfaces are not working:
      
          igb: Intel(R) Gigabit Ethernet Network Driver - version 5.4.0-k
          igb: Copyright (c) 2007-2014 Intel Corporation.
          igb: probe of 0000:01:00.0 failed with error -2
          igb: probe of 0000:02:00.0 failed with error -2
      
      In order to fix it, we explicitly select page 0 before first access to phy
      registers.
      
      See also: https://bugzilla.suse.com/show_bug.cgi?id=1009911
      See also: http://www.lex.com.tw/products/pdf/3I380A&3I380CW.pdf
      
      Fixes: 2a3cdead ("igb: Remove GS40G specific defines/functions")
      Cc: <stable@vger.kernel.org> # 4.5+
      Signed-off-by: default avatarMatwey V Kornilov <matwey@sai.msu.ru>
      Tested-by: default avatarAaron Brown <aaron.f.brown@intel.com>
      Signed-off-by: default avatarJeff Kirsher <jeffrey.t.kirsher@intel.com>
      440aeca4
  2. 05 Jun, 2017 36 commits