1. 06 Jun, 2017 8 commits
    • Jacob Keller's avatar
      igb: check for Tx timestamp timeouts during watchdog · e5f36ad1
      Jacob Keller authored
      The igb driver has logic to handle only one Tx timestamp at a time,
      using a state bit lock to avoid multiple requests at once.
      
      It may be possible, if incredibly unlikely, that a Tx timestamp event is
      requested but never completes. Since we use an interrupt scheme to
      determine when the Tx timestamp occurred we would never clear the state
      bit in this case.
      
      Add an igb_ptp_tx_hang() function similar to the already existing
      igb_ptp_rx_hang() function. This function runs in the watchdog routine
      and makes sure we eventually recover from this case instead of
      permanently disabling Tx timestamps.
      
      Note: there is no currently known way to cause this without hacking the
      driver code to force it.
      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>
      e5f36ad1
    • Jacob Keller's avatar
      igb: add statistic indicating number of skipped Tx timestamps · c3b8f85e
      Jacob Keller authored
      The igb driver can only handle one Tx timestamp request at a time.
      This means it is possible for an application timestamp request to be
      ignored.
      
      There is no easy way for an administrator to determine if this occurred.
      Add a new statistic which tracks this, tx_hwtstamp_skipped.
      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>
      c3b8f85e
    • Jacob Keller's avatar
      e1000e: add statistic indicating number of skipped Tx timestamps · cff57141
      Jacob Keller authored
      The e1000e driver can only handle one Tx timestamp request at a time.
      This means it is possible for an application timestamp request to be
      ignored.
      
      There is no easy way for an administrator to determine if this occurred.
      Add a new statistic which tracks this, tx_hwtstamp_skipped.
      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>
      cff57141
    • Jacob Keller's avatar
      igb: avoid permanent lock of *_PTP_TX_IN_PROGRESS · 74344e32
      Jacob Keller authored
      The igb driver uses a state bit lock to avoid handling more than one Tx
      timestamp request at once. This is required because hardware is limited
      to a single set of registers for Tx timestamps.
      
      The state bit lock is not properly cleaned up during
      igb_xmit_frame_ring() if the transmit fails such as due to DMA or TSO
      failure. In some hardware this results in blocking timestamps until the
      service task times out. In other hardware this results in a permanent
      lock of the timestamp bit because we never receive an interrupt
      indicating the timestamp occurred, since indeed the packet was never
      transmitted.
      
      Fix this by checking for DMA and TSO errors in igb_xmit_frame_ring() and
      properly cleaning up after ourselves when these occur.
      Reported-by: default avatarReported-by: David 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>
      74344e32
    • 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 32 commits