RE: [PATCH v14 net-next 03/13] dpll: fix stale iteration in dpll_pin_on_pin_unregister()

[Nitka, Grzegorz]

> -----Original Message-----
> From: Jakub Kicinski <kuba@xxxxxxxxxx>
> Sent: Thursday, June 11, 2026 7:41 PM
> To: Paolo Abeni <pabeni@xxxxxxxxxx>
> Cc: Nitka, Grzegorz <grzegorz.nitka@xxxxxxxxx>; netdev@xxxxxxxxxxxxxxx;
> linux-kernel@xxxxxxxxxxxxxxx; intel-wired-lan@xxxxxxxxxxxxxxxx; Oros, Petr
> <poros@xxxxxxxxxx>; richardcochran@xxxxxxxxx;
> andrew+netdev@xxxxxxx; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; Nguyen, Anthony L
> <anthony.l.nguyen@xxxxxxxxx>; Prathosh.Satish@xxxxxxxxxxxxx; Vecera,
> Ivan <ivecera@xxxxxxxxxx>; jiri@xxxxxxxxxxx; Kubalewski, Arkadiusz
> <arkadiusz.kubalewski@xxxxxxxxx>; vadim.fedorenko@xxxxxxxxx;
> donald.hunter@xxxxxxxxx; horms@xxxxxxxxxx; davem@xxxxxxxxxxxxx;
> edumazet@xxxxxxxxxx
> Subject: Re: [PATCH v14 net-next 03/13] dpll: fix stale iteration in
> dpll_pin_on_pin_unregister()
> 
> On Thu, 11 Jun 2026 16:41:07 +0200 Paolo Abeni wrote:
> > Fixes tag could be stripped when applying the patches as needed, and I
> > think that could be preferable to a repost just for such thing, given
> > the current PW load...
> 
> To be clear tho, this series has already been marked as changes
> requested. Presumably based on the other comments?

Hi Kuba, Paolo

To sum up ...

For v14 patchset, I got only comments from Arek and Paolo.
In my opinion, after rethinking, Arek's concerns are not valid (explained in
the responses). Maybe I could squash some changes, but the final code would
remain the same as for v14.

Paolo raised 'Fixes' tags which I added and critical divide-by-zero panic
Regarding 'Fixes' tag, It might be my fault or misunderstanding.
I can remove them if we want and re-send the series.
Regarding div-by-zero - see my comments about AI concern list below.

Also, what was raised by AI, I unintentionally changed WARN_ON to WARN_ON_ONCE
in patch 2. I'd restore it to WARN_ON.

Below is my summary on other AI concerns (take a look at "potential issues" section).
I'm aware we're about window closure. I'm polishing this series for a long time.
A lot was fixed already. I'm afraid adding another fixes will trigger a new ones 😊
Please, let me know if we still have time for another version. I can do that if you think
the remaining issues are 'must have' fix. In my subjective opinion - it's not.

Potential issues:
---------------------
Patch 11:
- hw->lane_num < 0
  It's a fatal error scenario. PTP module already returning an error in such case,
  so it's visible to the user anyway.
  I could add something similar to dpll module initialization. But from my perspective
  it's more a kind of very defensive approach.

Patch 13
- potential software state desynchronized from the hardware:
  Worth to fix but in the next release - this is in my opinion a kind of
  a corner/sophisticated scenario.

- bypass the new ice_txclk_update_and_notify when pf->ptp.state != ICE_PTP_READY
  Again, worth to fix in the next release (fatal PTP error case)

- worker thread is preempted right before clearing txclk_switch_requested
  Race is real but practically unreachable. Good to have it fixed in the next release
  
Not an issue (false-positive):
-------------------------------------
Patch 11:
- potential divide by zero panic (CRITICAL)
  That was fixed in v14. In case PTP hardware initilization is failed, DPLL
  re-initializes module will set hw->ptp.ports_per_phy on its own.

- error path in ice_dpll_init_e825 and the risk of double-destroy the mutex
  when ice_dpll_deinit()
  still guarded by ICE_FLAG_DPLL flag

Patch 13
- attempt to lock ctrl_pf->dplls.lock when controlling PF's DPLL subsytem is not
  initilized
  This condition is checked when triggering tx-clk change and appropriate error
  is returned.

These is pre-existing issue, out of the scope of this series in my opinion
- patch 12: silently ignoring non-EMODE firmware errors.

Regards

Grzegorz