Re: [RFC PATCH 2/4] net: dsa: link aggregation support

["Tobias Waldekranz" <tobias@waldekranz.com>]

On Wed Oct 28, 2020 at 3:58 AM CET, Vladimir Oltean wrote:
> When you use dsa_broadcast, it is reachable from _all_ switch trees, not
> from "the" switch tree. This was added to support "islands" of
> inter-compatible DSA switches separated by other DSA switches with
> incompatible taggers. Not sure if it was a voluntary decision to use
> that as opposed to plain dsa_port_notify. Not a problem either way.

You're right, I want dsa_port_notify. I will change it and also remove
the tree_index from the notifier info struct.

> > +	/* For multichip systems, we must ensure that each hash bucket
> > +	 * is only enabled on a single egress port throughout the
> > +	 * whole tree.
>
> Or else?
> I don't really understand this statement.

Or else we will send the same packet through multiple ports. I.e. if
we have swp0..2 in a LAG with bucket config like this:

Bucket#  swp0  swp1  swp2
      0     Y     n     n
      1     Y     n     n
      2     Y     n     n
      3     Y     Y     n
      4     n     Y     n
      5     n     Y     n
      6     n     n     Y
      7     n     n     Y

Packets that hash to bucket 3 would be sent out through both swp0 and
swp1, which the receiver would interpret as two distinct packets with
the same contents.

I will reword it to make it more clear.

> > +	struct dsa_lag *lag;
> > +	unsigned long busy = 0;
>
> Reverse Christmas notation please?

I have no excuses. :)

> > -	if (obj->orig_dev != dev)
> > +	if (!(obj->orig_dev == dev ||
> > +	      (dp->lag && obj->orig_dev == dp->lag->dev)))
>
> A small comment here maybe?

Yep, will do.

Thanks,
Tobias