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.

quoted

+	/* 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.

quoted

+	struct dsa_lag *lag;
+	unsigned long busy = 0;

Reverse Christmas notation please?

I have no excuses. :)

quoted

-	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