Re: [PATCH iwl-net v1] i40e: fix races in PTP external timestamp work handling

Vadim Fedorenko <[email protected]>
Newsgroups org.kernel.vger.netdev,org.osuosl.intel-wired-lan
Message-ID <[email protected]>
On 05/08/2026 08:13, luoxuanqiang wrote:
> 
> 在 2026/8/5 05:33, Vadim Fedorenko 写道:
>> On 24/07/2026 10:34, [email protected] wrote:
>>>
>>> diff --git a/drivers/net/ethernet/intel/i40e/i40e_ptp.c b/drivers/ 
>>> net/ethernet/intel/i40e/i40e_ptp.c
>>> index ff62b5f2c8150..fd51ab8c10c20 100644
>>> --- a/drivers/net/ethernet/intel/i40e/i40e_ptp.c
>>> +++ b/drivers/net/ethernet/intel/i40e/i40e_ptp.c
>>> @@ -169,6 +169,17 @@ static void i40e_ptp_extts0_work(struct 
>>> work_struct *work)
>>>       ptp_clock_event(pf->ptp_clock, &event);
>>>   }
>>>   +/**
>>> + * i40e_ptp_init_work - Initialize PTP work for a PF
>>> + * @pf: Board private structure
>>> + *
>>> + * Initialize work which must remain valid for the lifetime of the PF.
>>> + */
>>> +void i40e_ptp_init_work(struct i40e_pf *pf)
>>> +{
>>> +    INIT_WORK(&pf->ptp_extts0_work, i40e_ptp_extts0_work);
>>> +}
>>> +
>>
>> why do you need extra function for a single line of code? why cannot 
>> you put it into i40e_ptp_init?
>>
>>
> Thanks for pointing this out.
> 
> i40e_ptp_init() is also called from the reset/rebuild path:
> 
>    i40e_service_task()
>      -> i40e_reset_subtask()
>        -> i40e_reset_and_rebuild()
>          -> i40e_rebuild()
>            -> i40e_setup_pf_switch()
>              -> i40e_ptp_init()
> 
> Putting INIT_WORK() there would reinitialize the work item during reset or
> recovery. If the work is pending or running, this races with the workqueue,
> resets its bookkeeping, and breaks the workqueue's non-reentrance
> guarantee.

ok, but then this driver has bigger problem. the same i40e_ptp_init()
re-inits tmreg_lock mutex and ptp_rx_lock spinlock. While latter might
be ok during reset (hopefully no traffic processed while the nic is in
reset), tmreg_lock mutex can be used for adjfine/adjtime/gettimex64/
settime64 callbacks as ptp device is still visible to user space.

I believe ptp part of the driver has to be redesigned properly.

> 
> The work item is therefore initialized once per PF from i40e_sw_init().
> The helper also keeps the work callback private to i40e_ptp.c.
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.