Re: [PATCH net] veth: Add updating of trans_start

From: Jay Vosburgh <jay.vosburgh@canonical.com>
To: Jakub Kicinski <kuba@kernel.org>
Cc: "David S. Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Paolo Abeni <pabeni@redhat.com>,
	netdev@vger.kernel.org, Jonathan Toppins <jtoppins@redhat.com>,
	Veaceslav Falico <vfalico@gmail.com>,
	Andy Gospodarek <andy@greyhouse.net>
Subject: Re: [PATCH net] veth: Add updating of trans_start
Date: Fri, 17 Jun 2022 17:27:43 -0700	[thread overview]
Message-ID: <28607.1655512063@famine> (raw)
In-Reply-To: <20220617124413.6848c826@kernel.org>

Jakub Kicinski <kuba@kernel.org> wrote:

>On Fri, 17 Jun 2022 09:42:55 -0700 Jay Vosburgh wrote:
>> 	In this case, it's to permit the bonding ARP / ND monitor to
>> function if that software device (veth in this case) is added to a bond
>> using the ARP / ND monitor (which relies on trans_start, and has done so
>> since at least 2.6.0).  I'll agree it's a niche case; this was broken
>> for veth for quite some time, but veth + netns is handy for software
>> only test cases, so it seems worth doing.
>
>I presume it needs it to check if the device has transmitted anything
>in the last unit of time, can we look at the device stats for LLTX for
>example?

	Yes, that's the use case.  

	Hmm.  Polling the device stats would likely work for software
devices, although the unit of time varies (some checks are fixed at one
unit, but others can be N units depending on the missed_max option
setting).

	Polling hardware devices might not work; as I recall, some
devices only update the statistics on timespans on the order of seconds,
e.g., bnx2 and tg3 appear to update once per second.  But those do
update trans_start.

	The question then becomes how to distinguish a software LLTX
device from a hardware LLTX device.

>> 	I didn't exhaustively check all LLTX drivers, but, e.g., tun
>> does update trans_start:
>> 
>> drivers/net/tun.c:
>> 
>>        /* NETIF_F_LLTX requires to do our own update of trans_start */
>>         queue = netdev_get_tx_queue(dev, txq);
>>         txq_trans_cond_update(queue);
>
>Well, it is _an_ example, but the only one I can find. And the
>justification is the same as yours now -- make bonding work a31d27fb.
>Because of that I don't think we can use tun as a proof that trans 
>start should be updated on LLTX devices as a general, stack-wide rule.
>There's a lot more LLTX devices than veth and tun.

	I'm not suggesting that all (software) LLTX software devices be
updated.

	-J

---
	-Jay Vosburgh, jay.vosburgh@canonical.com