netdev - Re: [PATCH RFC net-next 1/5] net: mvpp2: add support for hardware timestamps

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <Z_0fCjkiry0AKS7j@shell.armlinux.org.uk>
Date: Mon, 14 Apr 2025 15:43:22 +0100
From: "Russell King (Oracle)" <linux@...linux.org.uk>
To: Kory Maincent <kory.maincent@...tlin.com>
Cc: Andrew Lunn <andrew@...n.ch>, Heiner Kallweit <hkallweit1@...il.com>,
	Andrew Lunn <andrew+netdev@...n.ch>,
	"David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>,
	Marcin Wojtas <marcin.s.wojtas@...il.com>, netdev@...r.kernel.org,
	Paolo Abeni <pabeni@...hat.com>
Subject: Re: [PATCH RFC net-next 1/5] net: mvpp2: add support for hardware
 timestamps

On Mon, Apr 14, 2025 at 02:51:50PM +0200, Kory Maincent wrote:
> On Fri, 11 Apr 2025 22:26:31 +0100
> Russell King <rmk+kernel@...linux.org.uk> wrote:
> 
> > Add support for hardware timestamps in (e.g.) the PHY by calling
> > skb_tx_timestamp() as close as reasonably possible to the point that
> > the hardware is instructed to send the queued packets.
> > 
> > Signed-off-by: Russell King <rmk+kernel@...linux.org.uk>
> 
> Reviewed-by: Kory Maincent <kory.maincent@...tlin.com>
> 
> > ---
> >  drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c
> > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c index
> > 416a926a8281..e3f8aa139d1e 100644 ---
> > a/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c +++
> > b/drivers/net/ethernet/marvell/mvpp2/mvpp2_main.c @@ -4439,6 +4439,8 @@
> > static netdev_tx_t mvpp2_tx(struct sk_buff *skb, struct net_device *dev)
> > txq_pcpu->count += frags; aggr_txq->count += frags;
> >  
> > +		skb_tx_timestamp(skb);
> > +
> >  		/* Enable transmit */
> >  		wmb();
> >  		mvpp2_aggr_txq_pend_desc_add(port, frags);
> 
> Small question for my curiosity here. Shouldn't we move the skb_tx_timestamp()
> call after the memory barrier for a better precision or is it negligible?

Depends what the wmb() is there for, which is entirely undocumented.

mvpp2_aggr_txq_pend_desc_add() uses writel(), which is itself required
to ensure that writes to memory before the writel() occurs are visible
to DMA agents. So, the wmb() there shouldn't be necessary if all
that's going on here is to ensure that the packet is visible to the
buffer manager hardware.

On arm64, that's __io_wmb(), which becomes __dma_wmb() and ultimately
"dmb oshst". wmb() on the other hand is a heavier barrier, "dsb st".
This driver ends up doing both, inexplicably.

It would help if people would document what purpose a barrier exists
for.

Without knowing what the wmb() is there for, I wouldn't like to move
it the other side.

-- 
RMK's Patch system: https://www.armlinux.org.uk/developer/patches/
FTTP is here! 80Mbps down 10Mbps up. Decent connectivity at last!