Re: [RFC PATCH net-next] net: linkwatch: only report IF_OPER_LOWERLAYERDOWN if iflink is actually down

[Jakub Kicinski <kuba@kernel.org>]

On Thu, 22 Sep 2022 01:05:06 +0300 Vladimir Oltean wrote:
> RFC 2863 says:
> 
>    The lowerLayerDown state is also a refinement on the down state.
>    This new state indicates that this interface runs "on top of" one or
>    more other interfaces (see ifStackTable) and that this interface is
>    down specifically because one or more of these lower-layer interfaces
>    are down.
> 
> DSA interfaces are virtual network devices, stacked on top of the DSA
> master, but they have a physical MAC, with a PHY that reports a real
> link status.
> 
> But since DSA (perhaps improperly) uses an iflink to describe the
> relationship to its master since commit c084080151e1 ("dsa: set ->iflink
> on slave interfaces to the ifindex of the parent"), default_operstate()
> will misinterpret this to mean that every time the carrier of a DSA
> interface is not ok, it is because of the master being not ok.
> 
> In fact, since commit c0a8a9c27493 ("net: dsa: automatically bring user
> ports down when master goes down"), DSA cannot even in theory be in the
> lowerLayerDown state, because it just calls dev_close_many(), thereby
> going down, when the master goes down.
> 
> We could revert the commit that creates an iflink between a DSA user
> port and its master, especially since now we have an alternative
> IFLA_DSA_MASTER which has less side effects. But there may be tooling in
> use which relies on the iflink, which has existed since 2009.
> 
> We could also probably do something local within DSA to overwrite what
> rfc2863_policy() did, in a way similar to hsr_set_operstate(), but this
> seems like a hack.
> 
> What seems appropriate is to follow the iflink, and check the carrier
> status of that interface as well. If that's down too, yes, keep
> reporting lowerLayerDown, otherwise just down.

Well explained. Seems like a judgment call. IMHO the RFC is acceptable.
I'd be tempted to extend it with a comment explaining that some special
uppers (i.e. DSA) have additional sources for being down so we should
double check the lower is indeed the source of the state.