From: Tony Nguyen <anthony.l.nguyen@intel.com> Date: 2021-08-27 20:40:41
This series contains updates to ice driver only.
Jake corrects the iterator used for looping Tx timestamp and removes
dead code related to pin configuration. He also adds locking around
flushing of the Tx tracker and restarts the periodic clock following
time changes.
Brett corrects the locking around updating netdev dev_addr.
The following are changes since commit 5fe2a6b4344cbb2120d6d81e371b7ec8e75f03e2:
Merge tag 'mlx5-fixes-2021-08-26' of git://git.kernel.org/pub/scm/linux/kernel/git/saeed/linux
and are available in the git repository at:
git://git.kernel.org/pub/scm/linux/kernel/git/tnguy/net-queue 100GbE
Brett Creeley (1):
ice: Only lock to update netdev dev_addr
Jacob Keller (4):
ice: fix Tx queue iteration for Tx timestamp enablement
ice: remove dead code for allocating pin_config
ice: add lock around Tx timestamp tracker flush
ice: restart periodic outputs around time changes
drivers/net/ethernet/intel/ice/ice_main.c | 13 +++--
drivers/net/ethernet/intel/ice/ice_ptp.c | 66 ++++++++++++++++++-----
2 files changed, 63 insertions(+), 16 deletions(-)
--
2.26.2
From: Tony Nguyen <anthony.l.nguyen@intel.com> Date: 2021-08-27 20:40:30
From: Jacob Keller <jacob.e.keller@intel.com>
The driver accidentally copied the ice_for_each_rxq iterator when
implementing enablement of the ptp_tx bit for the Tx rings. We still
load the Tx rings and set the ptp_tx field, but we iterate over the
count of the num_rxq.
If the number of Tx and Rx queues differ, this could either cause
a buffer overrun when accessing the tx_rings list if num_txq is greater
than num_rxq, or it could cause us to fail to enable Tx timestamps for
some rings.
This was not noticed originally as we generally have the same number of
Tx and Rx queues.
Fixes: ea9b847cda64 ("ice: enable transmit timestamps for E810 devices")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Gurucharan G <redacted>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
@@ -22,7 +22,7 @@ static void ice_set_tx_tstamp(struct ice_pf *pf, bool on)return;/* Set the timestamp enable flag for all the Tx rings */-ice_for_each_rxq(vsi,i){+ice_for_each_txq(vsi,i){if(!vsi->tx_rings[i])continue;vsi->tx_rings[i]->ptp_tx=on;
From: Tony Nguyen <anthony.l.nguyen@intel.com> Date: 2021-08-27 20:40:35
From: Jacob Keller <jacob.e.keller@intel.com>
We have code in the ice driver which allocates the pin_config structure
if n_pins is > 0, but we never set n_pins to be greater than zero.
There's no reason to keep this code until we actually have pin_config
support. Remove this. We can re-add it properly when we implement
support for pin_config for E810-T devices.
Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Gurucharan G <redacted>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 11 -----------
1 file changed, 11 deletions(-)
@@ -1064,17 +1064,6 @@ static long ice_ptp_create_clock(struct ice_pf *pf)info=&pf->ptp.info;dev=ice_pf_to_dev(pf);-/* Allocate memory for kernel pins interface */-if(info->n_pins){-info->pin_config=devm_kcalloc(dev,info->n_pins,-sizeof(*info->pin_config),-GFP_KERNEL);-if(!info->pin_config){-info->n_pins=0;-return-ENOMEM;-}-}-/* Attempt to register the clock before enabling the hardware. */clock=ptp_clock_register(info,dev);if(IS_ERR(clock))
From: Tony Nguyen <anthony.l.nguyen@intel.com> Date: 2021-08-27 20:40:37
From: Jacob Keller <jacob.e.keller@intel.com>
When we enabled auxiliary input/output support for the E810 device, we
forgot to add logic to restart the output when we change time. This is
important as the periodic output will be incorrect after a time change
otherwise.
This unfortunately includes the adjust time function, even though it
uses an atomic hardware interface. The atomic adjustment can still cause
the pin output to stall permanently, so we need to stop and restart it.
Introduce wrapper functions to temporarily disable and then re-enable
the clock outputs.
Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Sunitha D Mekala <redacted>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 49 ++++++++++++++++++++++++
1 file changed, 49 insertions(+)
From: Tony Nguyen <anthony.l.nguyen@intel.com> Date: 2021-08-27 20:40:38
From: Jacob Keller <jacob.e.keller@intel.com>
The driver didn't take the lock while flushing the Tx tracker, which
could cause a race where one thread is trying to read timestamps out
while another thread is trying to read the tracker to check the
timestamps.
Avoid this by ensuring that flushing is locked against read accesses.
Fixes: ea9b847cda64 ("ice: enable transmit timestamps for E810 devices")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Gurucharan G <redacted>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
---
drivers/net/ethernet/intel/ice/ice_ptp.c | 4 ++++
1 file changed, 4 insertions(+)
@@ -5144,8 +5145,13 @@ static int ice_set_mac_address(struct net_device *netdev, void *pi)}netif_addr_lock_bh(netdev);+ether_addr_copy(old_mac,netdev->dev_addr);+/* change the netdev's MAC address */+memcpy(netdev->dev_addr,mac,netdev->addr_len);+netif_addr_unlock_bh(netdev);+/* Clean up old MAC filter. Not an error if old filter doesn't exist */-status=ice_fltr_remove_mac(vsi,netdev->dev_addr,ICE_FWD_TO_VSI);+status=ice_fltr_remove_mac(vsi,old_mac,ICE_FWD_TO_VSI);if(status&&status!=ICE_ERR_DOES_NOT_EXIST){err=-EADDRNOTAVAIL;gotoerr_update_filters;
@@ -5168,13 +5174,12 @@ static int ice_set_mac_address(struct net_device *netdev, void *pi)if(err){netdev_err(netdev,"can't set MAC %pM. filter update failed\n",mac);+netif_addr_lock_bh(netdev);+ether_addr_copy(netdev->dev_addr,old_mac);netif_addr_unlock_bh(netdev);returnerr;}-/* change the netdev's MAC address */-memcpy(netdev->dev_addr,mac,netdev->addr_len);-netif_addr_unlock_bh(netdev);netdev_dbg(vsi->netdev,"updated MAC address to %pM\n",netdev->dev_addr);
From: Jakub Kicinski <kuba@kernel.org> Date: 2021-08-28 00:43:42
On Fri, 27 Aug 2021 13:43:55 -0700 Tony Nguyen wrote:
From: Jacob Keller <jacob.e.keller@intel.com>
We have code in the ice driver which allocates the pin_config structure
if n_pins is > 0, but we never set n_pins to be greater than zero.
There's no reason to keep this code until we actually have pin_config
support. Remove this. We can re-add it properly when we implement
support for pin_config for E810-T devices.
Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Gurucharan G <redacted>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
Removing dead code is not really a fix. Let's see if Linus cuts 5.14
this weekend, in which case it won't matter.
Hello:
This series was applied to netdev/net.git (refs/heads/master):
On Fri, 27 Aug 2021 13:43:53 -0700 you wrote:
This series contains updates to ice driver only.
Jake corrects the iterator used for looping Tx timestamp and removes
dead code related to pin configuration. He also adds locking around
flushing of the Tx tracker and restarts the periodic clock following
time changes.
[...]
From: "Keller, Jacob E" <jacob.e.keller@intel.com> Date: 2021-08-30 19:21:10
-----Original Message-----
From: Jakub Kicinski <kuba@kernel.org>
Sent: Friday, August 27, 2021 5:44 PM
To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
Cc: davem@davemloft.net; Keller, Jacob E <jacob.e.keller@intel.com>;
netdev@vger.kernel.org; richardcochran@gmail.com; Machnikowski, Maciej
[off-list ref]; G, GurucharanX [off-list ref]
Subject: Re: [PATCH net 2/5] ice: remove dead code for allocating pin_config
On Fri, 27 Aug 2021 13:43:55 -0700 Tony Nguyen wrote:
quoted
From: Jacob Keller <jacob.e.keller@intel.com>
We have code in the ice driver which allocates the pin_config structure
if n_pins is > 0, but we never set n_pins to be greater than zero.
There's no reason to keep this code until we actually have pin_config
support. Remove this. We can re-add it properly when we implement
support for pin_config for E810-T devices.
Fixes: 172db5f91d5f ("ice: add support for auxiliary input/output pins")
Signed-off-by: Jacob Keller <jacob.e.keller@intel.com>
Tested-by: Gurucharan G <redacted>
Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com>
Removing dead code is not really a fix. Let's see if Linus cuts 5.14
this weekend, in which case it won't matter.
It's a fix in my mind because the code was included in the original due to a mishandled rebase when working on series of patches. But yea, from outside that context its not really a fix since it doesn't change things from an external perspective.
Thanks,
Jake