Re: [RFC PATCH net-next 0/7] DSA master state tracking

[Ansuel Smith <ansuelsmth@gmail.com>]

On Thu, Dec 09, 2021 at 05:56:17PM +0000, Vladimir Oltean wrote:
> On Thu, Dec 09, 2021 at 06:44:38PM +0100, Ansuel Smith wrote:
> > > I think the problem is that we also need to track the operstate of the
> > > master (netif_oper_up via NETDEV_CHANGE) before declaring it as good to go.
> > > You can see that this is exactly the line after which the timeouts disappear:
> > > 
> > > [    7.146901] IPv6: ADDRCONF(NETDEV_CHANGE): eth0: link becomes ready
> > > 
> > > I didn't really want to go there, because now I'm not sure how to
> > > synthesize the information for the switch drivers to consume it.
> > > Anyway I've prepared a v2 patchset and I'll send it out very soon.
> > 
> > Wonder if we should leave the driver decide when it's ready by parsing
> > the different state? (And change
> > the up ops to something like a generic change?)
> 
> There isn't just one state to track, which is precisely the problem that
> I had to deal with for v2. The master is operational during the time
> frame between NETDEV_UP and NETDEV_GOING_DOWN, intersected with the
> interval during which netif_oper_up(master) is true. So in the simple
> state propagation approach, DSA would need to provide at least two ops
> to switches, one for admin state and the other for oper state. And the
> switch driver would need to AND the two and keep state by itself.
> Letting the driver make the decision would have been acceptable to me if
> we could have 3 ops and a common implementation, something like this:
> 
> static void qca8k_master_state_change(struct dsa_switch *ds,
> 				      const struct dsa_master *master)
> {
> 	bool operational = (master->flags & IFF_UP) && netif_oper_up(master);
> }
> 
> 	.master_admin_state_change	= qca8k_master_state_change,
> 	.master_oper_state_change	= qca8k_master_state_change,
> 
> but the problem is that during NETDEV_GOING_DOWN, master->flags & IFF_UP
> is still true, so this wouldn't work. And replacing the NETDEV_GOING_DOWN
> notifier with the NETDEV_DOWN one would solve that problem, but it would
> no longer guarantee that the switch can disable this feature without
> timeouts before the master is down - because now it _is_ down.

Ok will have to test v2 and check if this is also fixed.

-- 
	Ansuel