Re: [PATCH iwl-net] ice: Skip PTP timestamp processing during release

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] ice: early return in ice_ptp_process_ts causes infinite interrupt loop
--

commit bedb82d9a31e734c0c507d3503f8d563a9326bcf
Author: Sergey Temerkhanov <sergey.temerkhanov@intel.com>

ice: Skip PTP timestamp processing during release

This commit skips PTP timestamp processing when the hardware is not in
the ICE_PTP_READY state. The intent is to avoid executing the timestamp
callbacks on unready hardware and hitting uninitialized structures.

> diff --git a/drivers/net/ethernet/intel/ice/ice_ptp.c b/drivers/net/ethernet/intel/ice/ice_ptp.c
> index b2de76065f46d..095dfe23140d5 100644
> --- a/drivers/net/ethernet/intel/ice/ice_ptp.c
> +++ b/drivers/net/ethernet/intel/ice/ice_ptp.c
> @@ -2815,6 +2815,9 @@ s8 ice_ptp_request_ts(struct ice_ptp_tx *tx, struct sk_buff *skb)
>  
>  void ice_ptp_process_ts(struct ice_pf *pf)
>  {
> +	if (pf->ptp.state != ICE_PTP_READY)
> +		return;
> +

[Severity: High]
Could this early return cause an infinite interrupt loop or kthread storm?

If the PTP state changes to ICE_PTP_UNINIT (during driver unload) or
ICE_PTP_RESETTING (during reset) while there are pending timestamps, it
appears this early return bypasses ice_ptp_process_tx_tstamp(). This leaves
the tx->in_use bits permanently set since the garbage collection of pending
timestamps is skipped.

If tx->in_use is not cleared, ice_ptp_tx_tstamps_pending() will continue to
evaluate to true.

On E830 devices, this can cause an infinite loop in hard IRQ context:

drivers/net/ethernet/intel/ice/ice_ptp.c:ice_ptp_ts_irq() {
    ...
		if (ice_ptp_tx_tstamps_pending(pf, true)) {
			/* Process outstanding Tx timestamps. If there
			 * is more work, re-arm the interrupt to trigger again.
			 */
			wr32(hw, PFINT_OICR, PFINT_OICR_TSYN_TX_M);
			ice_flush(hw);
		}
    ...
}

On non-E830 devices, it can continuously re-arm the OICR interrupt from the
worker thread, causing a kthread storm:

drivers/net/ethernet/intel/ice/ice_main.c:ice_misc_intr_thread_fn() {
    ...
	if (ice_ptp_tx_tstamps_pending(pf, true)) {
		/* If any new Tx timestamps happened while in interrupt,
		 * re-arm the interrupt to trigger it again.
		 */
		wr32(hw, PFINT_OICR, PFINT_OICR_TSYN_TX_M);
		ice_flush(hw);
	}
    ...
}

Does there need to be a way to clear tx->in_use and free the associated
SKBs when bypassing the timestamp processing here?

>  	switch (pf->ptp.tx_interrupt_mode) {
>  	case ICE_PTP_TX_INTERRUPT_NONE:
>  		/* This device has the clock owner handle timestamps for it */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001115232.13681-1-sergey.temerkhanov@intel.com?part=1