Re: [PATCH net-next v2 0/4] add HSR offloading support for DSA switches

Vladimir Oltean
2021-02-09T17:15:32Z

On Tue, Feb 09, 2021 at 11:04:08AM -0600, George McCollister wrote:
> > >> In the DSA layer (dsa_slave_changeupper), could we merge the two HSR
> > >> join/leave calls somehow? My guess is all drivers are going to end up
> > >> having to do the same dance of deferring configuration until both ports
> > >> are known.
> > >
> > > Describe what you mean a bit more. Do you mean join and leave should
> > > each only be called once with both hsr ports being passed in?
> >
> > Exactly. Maybe we could use `netdev_for_each_lower_dev` to figure out if
> > the other port has already been switched over to the new upper or
> > something. I find it hard to believe that there is any hardware out
> > there that can do something useful with a single HSR/PRP port anyway.
>
> If one port failed maybe it would still be useful to join one port if
> the switch supported it? Maybe this couldn't ever happen anyway due
> the way hsr is designed.
>
> How were you thinking this would work? Would it just not use
> dsa_port_notify() and call a switch op directly after the second
> port's dsa_slave_changeupper() call? Or would we instead keep port
> notifiers and calls to dsa_switch_hsr_join for each port and just make
> dsa_switch_hsr_join() not call the switch op to create the HSR until
> the second port called it? I'm not all that familiar with how these
> dsa notifiers work and would prefer to stick with using a similar
> mechanism to the bridge and lag support. It would be nice to get some
> feedback from the DSA maintainers on how they would prefer it to work
> if they indeed had a preference at all.

Even though I understand where this is coming from, I have grown to
dislike overengineered solutions. DSA is there to standardize how an
Ethernet-connected switch interacts with the network stack, not to be
the middle man that tries to offer a lending hand everywhere.
If there is a 50/50 chance that this extra logic in the DSA mid layer
will not be needed for the second switch that offloads HSR/PRP, then I'd
go with "don't do it". Just emit a hsr_join for both ports and let the
driver deal with it.