Re: [PATCH] net br_netlink.c:y allow non "disabled" state for !netif_oper_up() links

[Andrew Lunn <andrew@lunn.ch>]

On Wed, Nov 09, 2022 at 07:19:22PM +0100, Rodolfo Giometti wrote:
> On 09/11/22 18:34, Andrew Lunn wrote:
> > On Wed, Nov 09, 2022 at 04:24:10PM +0100, Rodolfo Giometti wrote:
> > > A generic loop-free network protocol (such as STP or MRP and others) may
> > > require that a link not in an operational state be into a non "disabled"
> > > state (such as listening).
> > > 
> > > For example MRP states that a MRM should set into a "BLOCKED" state (which is
> > > equivalent to the LISTENING state for Linux bridges) one of its ring
> > > connection if it detects that this connection is "DOWN" (that is the
> > > NO-CARRIER status).
> > 
> > Does MRP explain Why?
> > 
> > This change seems odd, and "Because the standard says so" is not the
> > best of explanations.
> 
> A MRM instance has two ports: primary port (PRM_RPort) and secondary port
> (SEC_RPort).
> 
> When both ports are UP (that is the CARRIER is on) the MRM is into the
> Ring_closed state and the PRM_RPort is in forwarding state while the
> SEC_RPort is in blocking state (remember that MRP blocking is equal to Linux
> bridge listening).
> 
> If the PRM_RPort losts its carrier and the link goes down the normative states that:
> 
> - ports role swap (PRM_RPort becomes SEC_RPort and vice versa).
> 
> - SEC_RPort must be set into blocking state.
> 
> - PRM_RPort must be set into forwarding state.
> 
> Then the MRM moves into a new state called Primary-UP. In this state, when
> the SEC_RPort returns to UP state (that is the CARRIER is up) it's returns
> into the Ring_closed state where both ports have the right status, that is
> the PRM_RPort is in forwarding state while the SEC_RPort is in blocking
> state.
> 
> This is just an example of one single case, but consider that, in general,
> when the carrier is lost the port state is moved into blocking so that when
> the carrier returns the port it's already into the right state.
> 
> Hope it's clearer now.

Yes, please add this to the commit message. The commit message is
supposed to explain Why, and this is a good example.
 
> However, despite this special case, I think that kernel code should
> implement mechanisms and not policies, shouldn't it? If user space needs a
> non operational port (that is with no carrier) into the listening state, why
> we should prevent it?

Did you dig deeper? Does the bridge make use of switchdev to tell the
hardware about this state change while the carrier is down? I also
wonder what the hardware drivers do? Since this is a change in
behaviour, they might not actually do anything. So then you have to
consider does it make sense for the bridge to set the state again
after the carrier comes up?

       Andrew