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

Tobias Waldekranz
2020-10-28T23:31:10Z

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