* RE: [PATCH net 2/5] e1000e: do not cache the hwtstamp config before it is applied
2026-10-28 9:04 [PATCH net 2/5] e1000e: do not cache the hwtstamp config before it is applied Linkui Xiao
@ 2026-10-28 18:32 ` Loktionov, Aleksandr
2026-10-28 22:12 ` Vadim Fedorenko
2026-11-02 22:15 ` Jacob Keller
2 siblings, 0 replies; 5+ messages in thread
From: Loktionov, Aleksandr @ 2026-10-28 18:32 UTC (permalink / raw)
To: Linkui Xiao, Nguyen, Anthony L, Kitszel, Przemyslaw,
andrew+netdev, davem, edumazet, kuba, pabeni
Cc: intel-wired-lan, netdev, linux-kernel, Linkui Xiao, stable
> -----Original Message-----
> From: Linkui Xiao <xiaolinkui@126.com>
> Sent: Wednesday, October 28, 2026 10:05 AM
> To: Nguyen, Anthony L <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; andrew+netdev@lunn.ch;
> davem@davemloft.net; edumazet@kernel.org; kuba@kernel.org;
> pabeni@redhat.com
> Cc: intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-
> kernel@vger.kernel.org; Linkui Xiao <xiaolinkui@kylinos.cn>;
> stable@vger.kernel.org
> Subject: [PATCH net 2/5] e1000e: do not cache the hwtstamp config before
> it is applied
>
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>
> e1000e_config_hwtstamp() stores the requested configuration in
> adapter->hwtstamp_config right after the rx filter has been
> translated, but the two register writes that follow are verified by
> reading the registers back and each of them can fail with -EAGAIN.
>
> When one of them does, the driver is left holding a configuration the
> hardware never accepted:
>
> - e1000e_hwtstamp_get() hands it back to user space, so an
> application that reads the configuration back is told that
> hardware time stamping is in effect while it is not;
>
> - e1000e_systim_reset() re-applies it after every reset, including
> the ones the watchdog triggers, and keeps failing;
>
> - e1000_watchdog() starts polling TSYNCRXCTL_VALID for the cached rx
> filter, and e1000_xmit_frame() still parks skbs that ask for a Tx
> time stamp because it only looks at FLAG_HAS_HW_TIMESTAMP, so they
> are eventually dropped by e1000e_tx_hwtstamp_work() with a
> "clearing Tx timestamp hang" warning.
>
> Cache the configuration only once both registers have been verified, so
> that adapter->hwtstamp_config always describes what the hardware is
> programmed with.
>
> Fixes: 62d7e3a2d006 ("e1000e: Validate hwtstamp_config completely before
> applying it")
> Cc: stable@vger.kernel.org
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> ---
> drivers/net/ethernet/intel/e1000e/netdev.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c
> b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 35f5590228eb..7ae33273734f 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -3715,8 +3715,6 @@ static int e1000e_config_hwtstamp(struct
> e1000_adapter *adapter,
> return -ERANGE;
> }
>
> - adapter->hwtstamp_config = *config;
> -
> /* enable/disable Tx h/w time stamping */
> regval = er32(TSYNCTXCTL);
> regval &= ~E1000_TSYNCTXCTL_ENABLED;
> @@ -3763,6 +3761,8 @@ static int e1000e_config_hwtstamp(struct
> e1000_adapter *adapter,
> er32(RXSTMPH);
> er32(TXSTMPH);
>
> + adapter->hwtstamp_config = *config;
> +
> return 0;
> }
>
> --
> 2.25.1
Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net 2/5] e1000e: do not cache the hwtstamp config before it is applied
2026-10-28 9:04 [PATCH net 2/5] e1000e: do not cache the hwtstamp config before it is applied Linkui Xiao
2026-10-28 18:32 ` Loktionov, Aleksandr
@ 2026-10-28 22:12 ` Vadim Fedorenko
2026-11-02 22:15 ` Jacob Keller
2 siblings, 0 replies; 5+ messages in thread
From: Vadim Fedorenko @ 2026-10-28 22:12 UTC (permalink / raw)
To: Linkui Xiao, anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev,
davem, edumazet, kuba, pabeni
Cc: intel-wired-lan, netdev, linux-kernel, Linkui Xiao, stable
On 28/10/2026 09:04, Linkui Xiao wrote:
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>
> e1000e_config_hwtstamp() stores the requested configuration in
> adapter->hwtstamp_config right after the rx filter has been
> translated, but the two register writes that follow are verified by
> reading the registers back and each of them can fail with -EAGAIN.
>
> When one of them does, the driver is left holding a configuration the
> hardware never accepted:
>
> - e1000e_hwtstamp_get() hands it back to user space, so an
> application that reads the configuration back is told that
> hardware time stamping is in effect while it is not;
>
> - e1000e_systim_reset() re-applies it after every reset, including
> the ones the watchdog triggers, and keeps failing;
>
> - e1000_watchdog() starts polling TSYNCRXCTL_VALID for the cached rx
> filter, and e1000_xmit_frame() still parks skbs that ask for a Tx
> time stamp because it only looks at FLAG_HAS_HW_TIMESTAMP, so they
> are eventually dropped by e1000e_tx_hwtstamp_work() with a
> "clearing Tx timestamp hang" warning.
>
> Cache the configuration only once both registers have been verified,
> so that adapter->hwtstamp_config always describes what the hardware
> is programmed with.
>
> Fixes: 62d7e3a2d006 ("e1000e: Validate hwtstamp_config completely before applying it")
> Cc: stable@vger.kernel.org
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> ---
> drivers/net/ethernet/intel/e1000e/netdev.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 35f5590228eb..7ae33273734f 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -3715,8 +3715,6 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
> return -ERANGE;
> }
>
> - adapter->hwtstamp_config = *config;
> -
> /* enable/disable Tx h/w time stamping */
> regval = er32(TSYNCTXCTL);
> regval &= ~E1000_TSYNCTXCTL_ENABLED;
> @@ -3763,6 +3761,8 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
> er32(RXSTMPH);
> er32(TXSTMPH);
>
> + adapter->hwtstamp_config = *config;
> +
> return 0;
> }
>
Reviewed-by: Vadim Fedorenko <vadim.fedorenko@linux.dev>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net 2/5] e1000e: do not cache the hwtstamp config before it is applied
2026-10-28 9:04 [PATCH net 2/5] e1000e: do not cache the hwtstamp config before it is applied Linkui Xiao
2026-10-28 18:32 ` Loktionov, Aleksandr
2026-10-28 22:12 ` Vadim Fedorenko
@ 2026-11-02 22:15 ` Jacob Keller
2026-11-03 1:04 ` Linkui Xiao
2 siblings, 1 reply; 5+ messages in thread
From: Jacob Keller @ 2026-11-02 22:15 UTC (permalink / raw)
To: Linkui Xiao, anthony.l.nguyen, przemyslaw.kitszel, andrew+netdev,
davem, edumazet, kuba, pabeni
Cc: intel-wired-lan, netdev, linux-kernel, Linkui Xiao, stable
On 10/28/2026 2:04 AM, Linkui Xiao wrote:
> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>
> e1000e_config_hwtstamp() stores the requested configuration in
> adapter->hwtstamp_config right after the rx filter has been
> translated, but the two register writes that follow are verified by
> reading the registers back and each of them can fail with -EAGAIN.
>
> When one of them does, the driver is left holding a configuration the
> hardware never accepted:
>
> - e1000e_hwtstamp_get() hands it back to user space, so an
> application that reads the configuration back is told that
> hardware time stamping is in effect while it is not;
>
> - e1000e_systim_reset() re-applies it after every reset, including
> the ones the watchdog triggers, and keeps failing;
>
> - e1000_watchdog() starts polling TSYNCRXCTL_VALID for the cached rx
> filter, and e1000_xmit_frame() still parks skbs that ask for a Tx
> time stamp because it only looks at FLAG_HAS_HW_TIMESTAMP, so they
> are eventually dropped by e1000e_tx_hwtstamp_work() with a
> "clearing Tx timestamp hang" warning.
>
> Cache the configuration only once both registers have been verified,
> so that adapter->hwtstamp_config always describes what the hardware
> is programmed with.
>
Any idea why this got marked as patch 2 out of 5? This doesn't appear on
either netdev or iwl with any other patches.
I'm going ahead and applying it to Intel Wired LAN regardless, since the
fix looks good.
Thanks,
Jake
> Fixes: 62d7e3a2d006 ("e1000e: Validate hwtstamp_config completely before applying it")
> Cc: stable@vger.kernel.org
> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
> ---
> drivers/net/ethernet/intel/e1000e/netdev.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
> index 35f5590228eb..7ae33273734f 100644
> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
> @@ -3715,8 +3715,6 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
> return -ERANGE;
> }
>
> - adapter->hwtstamp_config = *config;
> -
> /* enable/disable Tx h/w time stamping */
> regval = er32(TSYNCTXCTL);
> regval &= ~E1000_TSYNCTXCTL_ENABLED;
> @@ -3763,6 +3761,8 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
> er32(RXSTMPH);
> er32(TXSTMPH);
>
> + adapter->hwtstamp_config = *config;
> +
> return 0;
> }
>
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH net 2/5] e1000e: do not cache the hwtstamp config before it is applied
2026-11-02 22:15 ` Jacob Keller
@ 2026-11-03 1:04 ` Linkui Xiao
0 siblings, 0 replies; 5+ messages in thread
From: Linkui Xiao @ 2026-11-03 1:04 UTC (permalink / raw)
To: Jacob Keller, anthony.l.nguyen, przemyslaw.kitszel,
andrew+netdev, davem, edumazet, kuba, pabeni
Cc: intel-wired-lan, netdev, linux-kernel, Linkui Xiao, stable
Hi Jake,
Thank you for applying the patch and for the review.
On 2026/11/3 06:15, Jacob Keller wrote:
> On 10/28/2026 2:04 AM, Linkui Xiao wrote:
>> From: Linkui Xiao <xiaolinkui@kylinos.cn>
>>
>> e1000e_config_hwtstamp() stores the requested configuration in
>> adapter->hwtstamp_config right after the rx filter has been
>> translated, but the two register writes that follow are verified by
>> reading the registers back and each of them can fail with -EAGAIN.
>>
>> When one of them does, the driver is left holding a configuration the
>> hardware never accepted:
>>
>> - e1000e_hwtstamp_get() hands it back to user space, so an
>> application that reads the configuration back is told that
>> hardware time stamping is in effect while it is not;
>>
>> - e1000e_systim_reset() re-applies it after every reset, including
>> the ones the watchdog triggers, and keeps failing;
>>
>> - e1000_watchdog() starts polling TSYNCRXCTL_VALID for the cached rx
>> filter, and e1000_xmit_frame() still parks skbs that ask for a Tx
>> time stamp because it only looks at FLAG_HAS_HW_TIMESTAMP, so they
>> are eventually dropped by e1000e_tx_hwtstamp_work() with a
>> "clearing Tx timestamp hang" warning.
>>
>> Cache the configuration only once both registers have been verified,
>> so that adapter->hwtstamp_config always describes what the hardware
>> is programmed with.
>>
>
> Any idea why this got marked as patch 2 out of 5? This doesn't appear on
> either netdev or iwl with any other patches.
Regarding your question about the "2/5" marking: it was an oversight on
my end. I generated this fix from a local branch that contained other
commits, and `git format-patch` automatically added the series
numbering. The patch is indeed a standalone fix and was not intended to
be part of a series posted elsewhere.
I'll make sure to use a proper invocation for single patches going forward.
Thanks again for your time and for taking this fix into the Intel Wired
LAN tree.
Best regards,
Linkui
>
> I'm going ahead and applying it to Intel Wired LAN regardless, since the
> fix looks good.
>
> Thanks,
> Jake
>
>> Fixes: 62d7e3a2d006 ("e1000e: Validate hwtstamp_config completely before applying it")
>> Cc: stable@vger.kernel.org
>> Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
>> ---
>> drivers/net/ethernet/intel/e1000e/netdev.c | 4 ++--
>> 1 file changed, 2 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/e1000e/netdev.c b/drivers/net/ethernet/intel/e1000e/netdev.c
>> index 35f5590228eb..7ae33273734f 100644
>> --- a/drivers/net/ethernet/intel/e1000e/netdev.c
>> +++ b/drivers/net/ethernet/intel/e1000e/netdev.c
>> @@ -3715,8 +3715,6 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
>> return -ERANGE;
>> }
>>
>> - adapter->hwtstamp_config = *config;
>> -
>> /* enable/disable Tx h/w time stamping */
>> regval = er32(TSYNCTXCTL);
>> regval &= ~E1000_TSYNCTXCTL_ENABLED;
>> @@ -3763,6 +3761,8 @@ static int e1000e_config_hwtstamp(struct e1000_adapter *adapter,
>> er32(RXSTMPH);
>> er32(TXSTMPH);
>>
>> + adapter->hwtstamp_config = *config;
>> +
>> return 0;
>> }
>>
^ permalink raw reply [flat|nested] 5+ messages in thread