Re: [PATCH v3 net 1/4] net: bonding: replace dev_trans_start() with the jiffies of the last ARP/NS

[Vladimir Oltean <vladimir.oltean@nxp.com>]

On Tue, Aug 02, 2022 at 09:11:10AM -0700, Jakub Kicinski wrote:
> On Tue, 02 Aug 2022 11:05:19 +0200 Paolo Abeni wrote:
> > In any case, this looks like a significative rework, do you mind
> > consider it for the net-next, when it re-open?
> 
> It does seem like it could be a lot for stable.
> 
> Perhaps we could take:
> 
> https://lore.kernel.org/all/20220727152000.3616086-1-vladimir.oltean@nxp.com/
> 
> as is, without the extra work Stephen asked for (since it's gonna be
> reverted in net-next, anyway)? How do you feel about that option?

The patch you've linked to doesn't really do something sane. Deferring the
dev_trans_start() call to the lower device means, in the context of DSA,
retrieving the trans_start of the master's TX queues. But the same DSA
master (host port) services more than 1 DSA interface, so if you have
swp0 and swp1 in a bond0 with ARP monitoring, and swp2 is running an
iperf3 session, bond0 will happily interpret that as meaning that ARP
packets are continuously being sent.

Does it work, in the sense that the link comes up when it should, and
doesn't when it shouldn't? Yeah, but this proves to me that most of the
handling around the ARP monitor is just random gibberish/snake oil that
could have simply not been written in the first place, or I'm missing
some of the finer points. (happy to be proven wrong and see Cunningham's
law work its magic)

How about applying this to the "net" tree when it starts tracking the
5.20 release candidates (effectively a continuation of today's net-next),
and I can send backport patches after a month or so of some more testing?
I can prepare a backported version of this for 5.10 that Brian could
keep in his system during this time, and we could watch that for further
strange behavior.