> -----Original Message----- > From: Jacob Keller <[email protected]> > Sent: Saturday, September 26, 2026 1:57 AM > To: Intel Wired LAN <[email protected]>; Machnikowski, > Maciej <[email protected]>; Keller, Jacob E > <[email protected]>; Korba, Przemyslaw > <[email protected]>; Nguyen, Anthony L > <[email protected]>; Nitka, Grzegorz > <[email protected]>; Kubalewski, Arkadiusz > <[email protected]> > Cc: Keller, Jacob E <[email protected]> > Subject: [PATCH iwl-net v3 01/15] ice: fix removal of PTP timestamp > tracker during reset > > Commit 7a25fe5cd5fb ("ice: stop destroying and reinitalizing Tx > tracker during reset") intended to modify the PTP reset flow of the > driver so that it stopped calling ice_ptp_reset_tx_tracker() during > teardown and stopped calling ice_ptp_init_tx_*() during rebuild. > > Unfortunately, the commit only removed the calls to > ice_ptp_init_tx_*(). > This fixed a memory leak in PF reset. However, now a CORE, GLOBAL, or > EMP reset will leave the device unable to initiate Tx timestamp > requests indefinitely. > > In practice the CORE and GLOBAL resets rarely happen in production > environments, while EMP resets happen after a firmware update that is > often followed by a platform reboot. This explains why this has not > been caught until now. However, it is trivial to verify by triggering > the reset from userspace via ethtool. For ice the following command > will trigger a GLOBAL > reset: > > $ ethtool --reset eno8303np0 irq-shared dma-shared filter-shared \ > offload-shared ram-shared mac-shared > phy-shared > > Remove the call of ice_ptp_release_tx_tracker() from > ice_ptp_prepare_for_reset(), to keep the tracker memory in place so > that timestamping can resume after a reset. > > During review of a previous version of this change, sashiko pointed > out that the teardown flows for ice_ptp_init() and ice_ptp_release() > could potentially leak the PTP timestamp tracker. > > Fix ice_ptp_init() so that it allocates the tracker before adding the > port to the port list, and properly calls ice_ptp_release_tx_tracker() > as part of its cleanup on error. Ensure the ps_lock mutex isn't > destroyed until the PF has been cleared from the port list. > > Fix ice_ptp_release() so that it handles the cleanup if PTP is in the > error state by cancelling the kworker items and releasing the Tx > tracker as appropriate. > > This was found by Sashiko review during feedback for an unrelated > change, and iterated based on further feedback from Sashiko after the > initial fix to remove the call to ice_ptp_release_tx_tracker(); > > Closes: https://sashiko.dev/#/patchset/20260821-jk-e825c-minimized- > fixes-v1-0-9d0731eb4858%40intel.com?part=8 > Closes: https://lore.kernel.org/netdev/20260916011213.1632286-1- > [email protected]/ > Fixes: 7a25fe5cd5fb ("ice: stop destroying and reinitalizing Tx > tracker during reset") > Signed-off-by: Jacob Keller <[email protected]> > --- > drivers/net/ethernet/intel/ice/ice_ptp.c | 31 ++++++++++++++++++++--- > -------- > 1 file changed, 20 insertions(+), 11 deletions(-) > > diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c > b/drivers/net/ethernet/intel/ice/ice_ptp.c > index eaec36ab6ae3..fe21cee4f9de 100644 > --- a/drivers/net/ethernet/intel/ice/ice_ptp.c > +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c > @@ -2939,8 +2939,6 @@ void ice_ptp_prepare_for_reset(struct ice_pf > *pf, enum ice_reset_req reset_type) > if (ice_pf_src_tmr_owned(pf) && hw->mac_type == > ICE_MAC_GENERIC_3K_E825) > ice_ptp_prepare_rebuild_sec(pf, false, reset_type); > > - ice_ptp_release_tx_tracker(pf, &pf->ptp.port.tx); > - > /* Disable periodic outputs */ > ice_ptp_disable_all_perout(pf); > > @@ -3347,13 +3345,13 @@ void ice_ptp_init(struct ice_pf *pf) > } > } > > - err = ice_ptp_setup_pf(pf); > - if (err) > - goto err_exit; > - > err = ice_ptp_init_port(pf, &ptp->port); > if (err) > - goto err_clean_pf; > + goto err_destroy_ps_lock; > + > + err = ice_ptp_setup_pf(pf); > + if (err) > + goto err_release_tx_tracker; > > /* Start the PHY timestamping block */ > ice_ptp_reset_phy_timestamping(pf); > @@ -3365,14 +3363,17 @@ void ice_ptp_init(struct ice_pf *pf) > > err = ice_ptp_init_work(pf, ptp); > if (err) > - goto err_exit; > + goto err_clean_pf; > > dev_info(ice_pf_to_dev(pf), "PTP init successful\n"); > return; > > err_clean_pf: > - mutex_destroy(&ptp->port.ps_lock); > ice_ptp_cleanup_pf(pf); > +err_release_tx_tracker: > + ice_ptp_release_tx_tracker(pf, &pf->ptp.port.tx); > +err_destroy_ps_lock: > + mutex_destroy(&ptp->port.ps_lock); > err_exit: > /* If we registered a PTP clock, release it */ > if (pf->ptp.clock) { > @@ -3399,12 +3400,20 @@ void ice_ptp_release(struct ice_pf *pf) > return; > > if (pf->ptp.state != ICE_PTP_READY) { > - mutex_destroy(&pf->ptp.port.ps_lock); > - ice_ptp_cleanup_pf(pf); > + if (pf->ptp.kworker) { > + kthread_cancel_delayed_work_sync(&pf->ptp.work); > + if (pf->hw.mac_type == ICE_MAC_GENERIC) > + kthread_cancel_delayed_work_sync(&pf- > >ptp.port.ov_work); > + kthread_destroy_worker(pf->ptp.kworker); > + pf->ptp.kworker = NULL; > + } > if (pf->ptp.clock) { > ptp_clock_unregister(pf->ptp.clock); > pf->ptp.clock = NULL; > } > + ice_ptp_cleanup_pf(pf); > + ice_ptp_release_tx_tracker(pf, &pf->ptp.port.tx); > + mutex_destroy(&pf->ptp.port.ps_lock); > return; > } > > > -- > 2.56.0.rc0.395.gd1f3524e15dc
Reviewed-by: Aleksandr Loktionov <[email protected]>
