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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <C6OKWDSV75FQ.1YBMNPWQ63810@wkz-x280>
Date:   Wed, 28 Oct 2020 15:03:15 +0100
From:   "Tobias Waldekranz" <tobias@...dekranz.com>
To:     "Vladimir Oltean" <olteanv@...il.com>
Cc:     <andrew@...n.ch>, <vivien.didelot@...il.com>,
        <f.fainelli@...il.com>, <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH 2/4] net: dsa: link aggregation support

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