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]
Date:   Sun, 4 Sep 2022 15:41:15 +0000
From:   Vladimir Oltean <vladimir.oltean@....com>
To:     Marek Behún <kabel@...nel.org>
CC:     "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "David S. Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Florian Fainelli <f.fainelli@...il.com>,
        Vivien Didelot <vivien.didelot@...il.com>,
        Andrew Lunn <andrew@...n.ch>,
        Vladimir Oltean <olteanv@...il.com>,
        Claudiu Manoil <claudiu.manoil@....com>,
        Alexandre Belloni <alexandre.belloni@...tlin.com>,
        "UNGLinuxDriver@...rochip.com" <UNGLinuxDriver@...rochip.com>,
        Colin Foster <colin.foster@...advantage.com>,
        Roopa Prabhu <roopa@...dia.com>,
        Nikolay Aleksandrov <razor@...ckwall.org>,
        Tobias Waldekranz <tobias@...dekranz.com>,
        Ansuel Smith <ansuelsmth@...il.com>,
        DENG Qingfang <dqfext@...il.com>,
        Alvin Šipraga <alsi@...g-olufsen.dk>,
        Linus Walleij <linus.walleij@...aro.org>,
        Luiz Angelo Daros de Luca <luizluca@...il.com>,
        Felix Fietkau <nbd@....name>, John Crispin <john@...ozen.org>,
        Sean Wang <sean.wang@...iatek.com>
Subject: Re: [PATCH net-next 0/9] DSA changes for multiple CPU ports (part 4)

On Sat, Sep 03, 2022 at 04:48:32AM +0200, Marek Behún wrote:
> My opinion is that it would be better to add new DSA specific netlink
> operations instead of using the existing iflink as I did in the that
> patch.

Ok, I'll send out the iproute2 patch today.

> I think that DSA should have it's own IP subcommands. Using the
> standard, already existing API, is not sufficient for more complex
> configurations/DSA routing settings. Consider DSA where there are
> multiple switches and the switches are connected via multiple ports:
> 
> +----------+   +---------------+   +---------+
> |     eth0 <---> sw0p0   sw0p2 <---> sw1p0
> | cpu      |   |               |   |        ....
> |     eth1 <---> sw0p1   s20p3 <---> sw1p1
> +----------+   +---------------+   +---------+
> 
> The routing is more complicated in this scenario.

This is so problematic that I don't even know where to start.

Does anyone do this? What do they want to achieve? How do they want the
system to behave?

Let's push your example even one step further:

 +----------+   +---------------+   +---------------+   +---------+
 |     eth0 <---> sw0p0   sw0p2 <---> sw1p0   sw1p2 <---> sw2p0
 | cpu      |   |               |   |               |   |        ....
 |     eth1 <---> sw0p1   sw0p3 <---> sw1p1   sw1p3 <---> sw2p1
 +----------+   +---------------+   +---------------+   +---------+

With our current DT bindings, DSA (cascade) ports would need to contain
links to all DSA ports of indirectly connected switches, because this is
what the dst->rtable expects.

	switch@0 {
		reg = <0>;
		dsa,member = <0 0>;

		ethernet-ports {
			port@0 {
				reg = <0>;
				ethernet = <&eth0>;
			};

			port@1 {
				reg = <1>;
				ethernet = <&eth1>;
			};

			sw0p2: port@2 {
				reg = <2>;
				link = <&sw1p0>, <&sw2p0>, <&sw2p1>;
			};

			sw0p3: port@3 {
				reg = <3>;
				link = <&sw1p1>, <&sw2p0>, <&sw2p1>;
			};
		};
	};

	switch@1 {
		reg = <1>;
		dsa,member = <0 1>;

		ethernet-ports {
			sw1p0: port@0 {
				reg = <0>;
				link = <&sw0p2>;
			};

			sw1p1: port@1 {
				reg = <1>;
				link = <&sw0p3>;
			};

			sw1p2: port@2 {
				reg = <2>;
				link = <&sw2p0>;
			};

			sw1p3: port@3 {
				reg = <3>;
				link = <&sw2p1>;
			};
		};
	};

	switch@2 {
		reg = <2>;
		dsa,member = <0 2>;

		ethernet-ports {
			port@0 {
				reg = <0>;
				link = <&sw1p2>, <&sw0p2>, <&sw0p3>;
			};

			port@1 {
				reg = <1>;
				link = <&sw1p3>, <&sw0p2>, <&sw0p3>;
			};
		};
	};

> The old API is not sufficient.

There is no "old API" to speak of, because none of this is exposed.
So, 'insufficient' is an understatement. Furthermore, the rtnl_link_ops
are exposed per *user* port, we still gain zero control of the
dst->rtable.

The main consumer of the dst->rtable is this:

/* Return the local port used to reach an arbitrary switch device */
static inline unsigned int dsa_routing_port(struct dsa_switch *ds, int device)
{
	struct dsa_switch_tree *dst = ds->dst;
	struct dsa_link *dl;

	list_for_each_entry(dl, &dst->rtable, list)
		if (dl->dp->ds == ds && dl->link_dp->ds->index == device)
			return dl->dp->index;

	return ds->num_ports;
}

Notice the singular *the* local port. So it would currently return the
local cascade port from the first dsa_link added to the rtable (in turn,
the first port which has a 'link' OF node description to a cascade port
of @device). If there are any further dsa_links between a cascade port
of this switch and a cascade port of that switch, they are ignored
(which is a good thing in terms of compatibility between old kernels and
new device trees, but still raises questions).

For each of the functions in the call graph below, we need to determine
exactly what we need to make behave differently (consider a potential
second, third, fourth dsa_link, and how to expose this structure):

                             dsa_port_host_address_match
                              |
                              |  dsa_port_host_vlan_match
                              |   |
                              v   v
                  dsa_switch_is_upstream_of
                  |                        |
                  |                        |
                  | mv88e6xxx_lag_sync_map |
                  v            |           |
          dsa_is_upstream_port |           |
                          |    |           |
   mv88e6xxx_port_vlan    |    |           |
              |           |    |           |
              v           |    |           |
dsa_switch_upstream_port  |    |           |
                     |    |    |           |
                     |    |    |           |
 plenty of drivers   |    |    |           |
               |     |    |    |           |
               v     v    v    |           |
           dsa_upstream_port   |           |
                       |       |           |
                       v       v           |
                     dsa_towards_port      |
                                   |       |
                                   |       |
                                   v       v
                              dsa_routing_port

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ