Re: [PATCH net] net: tsnep: fix timestamping with a stacked DSA driver

[Gerhard Engleder <gerhard@engleder-embedded.com>]

On 12.05.25 15:24, Vladimir Oltean wrote:
> This driver seems susceptible to a form of the bug explained in commit
> c26a2c2ddc01 ("gianfar: Fix TX timestamping with a stacked DSA driver")
> and in Documentation/networking/timestamping.rst section "Other caveats
> for MAC drivers", specifically it timestamps any skb which has
> SKBTX_HW_TSTAMP, and does not consider adapter->hwtstamp_config.tx_type.

Is it necessary in general to check adapter->hwtstamp_config.tx_type for
HWTSTAMP_TX_ON or only to fix this bug? In 
Documentation/networking/timestamping.rst
section "Hardware Timestamping Implementation: Device Drivers" only the
check of (skb_shinfo(skb)->tx_flags & SKBTX_HW_TSTAMP) is mentioned.

> Evaluate the proper TX timestamping condition only once on the TX
> path (in tsnep_netdev_xmit_frame()) and pass it down to
> tsnep_xmit_frame_ring() and tsnep_tx_activate() through a bool variable.

What about also storing the TX timestamping condition in TX entry and
evaluating it in tsnep_tx_poll() instead of checking
adapter->hwtstamp_config.tx_type again? This way the timestamping
decision is only done in tsnep_netdev_xmit_frame() and tsnep_tx_poll()
cannot decide differently e.g. if hardware timestamping was deactivated
in between. Also this means that SKBTX_IN_PROGRESS is only set but
never evaluated by tsnep, which should fix this bug AFAIU.

> Also evaluate it again in the TX confirmation path, in tsnep_tx_poll(),
> since I don't know whether TSNEP_DESC_EXTENDED_WRITEBACK_FLAG is a
> confounding condition and may be set for other reasons than hardware
> timestamping too.

Yes, there is also some DMA statistic included besides timestamping so
it can be set for other reasons too in the future.

I can take over this patch and test it when I understand more clearly
what needs to be done.

Gerhard