Re: [PATCH] Revert "net: linkwatch: add check for netdevice being present to linkwatch_do_dev"
From: Nikolay Aleksandrov
Date: Fri Sep 18 2020 - 08:40:08 EST
On Mon, 2020-09-14 at 09:40 +0200, Geert Uytterhoeven wrote:
> Hi David,
> CC bridge
> On Sun, Sep 13, 2020 at 3:34 AM David Miller <davem@xxxxxxxxxxxxx> wrote:
> > From: Geert Uytterhoeven <geert@xxxxxxxxxxxxxx>
> > Date: Sat, 12 Sep 2020 14:33:59 +0200
> > > "dev" is not the bridge device, but the physical Ethernet interface, which
> > > may already be suspended during s2ram.
> > Hmmm, ok.
> > Looking more deeply NETDEV_CHANGE causes br_port_carrier_check() to run which
> > exits early if netif_running() is false, which is going to be true if
> > netif_device_present() is false:
> > *notified = false;
> > if (!netif_running(br->dev))
> > return;
> > The only other work the bridge notifier does is:
> > if (event != NETDEV_UNREGISTER)
> > br_vlan_port_event(p, event);
> > and:
> > /* Events that may cause spanning tree to refresh */
> > if (!notified && (event == NETDEV_CHANGEADDR || event == NETDEV_UP ||
> > event == NETDEV_CHANGE || event == NETDEV_DOWN))
> > br_ifinfo_notify(RTM_NEWLINK, NULL, p);
> > So some vlan stuff, and emitting a netlink message to any available
> > listeners.
> > Should we really do all of this for a device which is not even
> > present?
> > This whole situation seems completely illogical. The device is
> > useless, it logically has no link or other state that can be managed
> > or used, while it is not present.
> > So all of these bridge operations should only happen when the device
> > transitions back to present again.
> Thanks for your analysis!
> I'd like to defer this to the bridge people (CC).
Sorry for the delay. Interesting problem. :)
Thanks for the analysis, I don't see any issues with checking if the device
isn't present. It will have to go through some testing, but no obvious
objections/issues. Have you tried if it fixes your case?
I have briefly gone over drivers' use of net_device_detach(), mostly it's used
for suspends, but there are a few cases which use it on IO error or when a
device is actually detaching (VF detach). The vlan port event is for vlan
devices on top of the bridge when BROPT_VLAN_BRIDGE_BINDING is enabled and their
carrier is changed based on vlan participating ports' state.