On Mon, 2023-12-04 at 08:23 -0800, Jakub Kicinski wrote:
> On Sun, 03 Dec 2023 19:51:28 +0100 Johannes Berg wrote:
> > I think I wouldn't mind now, and perhaps if we want to sync in netlink
> > we should also do this here so that it's consistent, but I'm not sure
> > I'd want this to be the only way to do it, I might imagine that someone
> > might want this in some kind of container that doesn't necessarily have
> > (full) access there? Dunno.
>
> Also dunno :) We can add a "sync" version of netif_carrier_ok()
> and then call if from whatever places we need.
[note: netif_carrier_ok(), not netif_carrier_on(), almost confused that]
Yeah I guess we can have a netif_carrier_ok_sync(), though it feels kind
of dubious to hide such an important detail in the middle of a netlink
message building:
if (nla_put_string(skb, IFLA_IFNAME, dev->name) ||
...
#ifdef CONFIG_RPS
nla_put_u32(skb, IFLA_NUM_RX_QUEUES, dev->num_rx_queues) ||
#endif
put_master_ifindex(skb, dev) ||
nla_put_u8(skb, IFLA_CARRIER, netif_carrier_ok(dev)) ||
...
Also, if we ever _do_ want to make it optional, then it's problematic,
do we do netif_carrier_ok_maybe_sync(dev, sync)? ;-)
> > We _could_ also use an input attribute on the rtnl_getlink() call to
> > have userspace explicitly opt in to doing the sync before returning
> > information?
>
> Yeah, maybe.. IMHO a more "Rusty Russell API levels" thing to do would
> be to allow opting out, as those who set the magic flag "know what they
> are doing" and returning unsync'ed carrier may be surprising.
> Also a "don't sync flag" we can add later, once someone who actually
> cares appears, avoiding uAPI growth 😁️
>
> Anyway, up to you :)
Heh. But do I want to get blamed for the (perhaps inevitable?)
performance regression? I guess I'll try ...
Actually I could even still combine this with the netif carrier up count
in the wireless events, so we only have to do the rtnl_getlink if we
haven't seen an event yet, and - in the likely common case - save the
extra roundtrip? Though I guess it's not a huge problem, it's once per
connection basically.
johannes
