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 <[email protected]> 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/[email protected]?part=1
