Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     =?Windows-1252?Q?Marek_Beh=FAn?= <kabel@kernel.org>
CC:     "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Eric Dumazet <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Florian Fainelli <f.fainelli@gmail.com>,
        Vivien Didelot <vivien.didelot@gmail.com>,
        Andrew Lunn <andrew@lunn.ch>,
        Vladimir Oltean <olteanv@gmail.com>,
        Claudiu Manoil <claudiu.manoil@nxp.com>,
        Alexandre Belloni <alexandre.belloni@bootlin.com>,
        "UNGLinuxDriver@microchip.com" <UNGLinuxDriver@microchip.com>,
        Colin Foster <colin.foster@in-advantage.com>,
        Roopa Prabhu <roopa@nvidia.com>,
        Nikolay Aleksandrov <razor@blackwall.org>,
        Tobias Waldekranz <tobias@waldekranz.com>,
        Ansuel Smith <ansuelsmth@gmail.com>,
        DENG Qingfang <dqfext@gmail.com>,
        =?Windows-1252?Q?Alvin_=8Aipraga?= <alsi@bang-olufsen.dk>,
        Linus Walleij <linus.walleij@linaro.org>,
        Luiz Angelo Daros de Luca <luizluca@gmail.com>,
        Felix Fietkau <nbd@nbd.name>, John Crispin <john@phrozen.org>,
        Sean Wang <sean.wang@mediatek.com>
Subject: Re: [PATCH net-next 0/9] DSA changes for multiple CPU ports (part 4)
Thread-Topic: [PATCH net-next 0/9] DSA changes for multiple CPU ports (part 4)
Thread-Index: AQHYvKsLmg9+Fvc+3E2TksPGMZq+1a3L9Q+AgAEQ6QCAAmo6gA==
Date:   Sun, 4 Sep 2022 15:41:15 +0000
Message-ID: <20220904154115.oybr524635lfrgqp@skbuf>
References: <20220830195932.683432-1-vladimir.oltean@nxp.com>
 <20220902103145.faccoawnaqh6cn3r@skbuf> <20220903044832.125984e2@thinkpad>
In-Reply-To: <20220903044832.125984e2@thinkpad>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?Windows-1252?Q?TvQqzMXEsAPAMh6pzpa5t/YhMxKkxexRIKprQDwHxvZiMqe0a/A8iym3?=
 =?Windows-1252?Q?mOCKQR5c+olZQcFzNaLwZoYmFhsNDQ/RNA4RASbKarnoGjuqWhETE7dB?=
 =?Windows-1252?Q?B20bYPwCwQWDd1VRuANcaGkfMbPXMmVItO8A5REnViP3/woguBR9Lgwv?=
 =?Windows-1252?Q?YkUwMnsPQpXS1P0C6kVtH0VDJVmrPoRE+QjRmcZ6bhwLDtynuAtMA957?=
 =?Windows-1252?Q?Zl9hhoIm2XUE/55y/3orXKf1g0izpuRvvpYgySroPwbIa9YZZ1PXt/cf?=
 =?Windows-1252?Q?+NRyEtjawu6DBke+R/Tq+I+uBKCNamEl4kZ6T8a2T4Ia2wjYQxy5388s?=
 =?Windows-1252?Q?sLR6ZyJYEdtWP4t9vqyEFEk9XkLr+Diyv57W8Yu8CjXWItzL8lQ3nm4t?=
 =?Windows-1252?Q?GxmfFdPAJummgI0weGgQaxIpxEj3ISMQ9uJXxVfiXyjZo5tlJlOCYcrc?=
 =?Windows-1252?Q?3UEmCO1PikBfoYge3SjpKnjs6oTIfUR3hIUhI73AWjKiiCqmK4nTZxLK?=
 =?Windows-1252?Q?W6GJiLZ42Vd11mhrZBydcktAGRVWDfXBWZvWzY5vpBgeEwu5Wsc4/Pbn?=
 =?Windows-1252?Q?VRGnn+lnSvlxtaKsvkVXFJ5yyKLBD87bSzbPsnVEXqkpwd+U/PpX3S9+?=
 =?Windows-1252?Q?Nfl0ntmb1+rbeq77hNE3Gk4K6ubCC71xC5Rx1PfuFJEb5CaG4NPPiwOG?=
 =?Windows-1252?Q?aplKXvdJ2Gb/Q4tXFVlhFvc6SO1sqXIXtuzZvbxRxDpxsL0LyM5Ea42f?=
 =?Windows-1252?Q?si9rFp6qvLCznYYWZ8bGBZOB44ko8Dnjjl2tEL6JPaudnfWx7ElOhn7m?=
 =?Windows-1252?Q?QIMxsf1PGLL87bKG1H7tthEVOuRVN9NBjE/DCzKQTs7zyDIprU6TSGxf?=
 =?Windows-1252?Q?J6bCVYd6k8dU5VSYuAAQQ5V6YKoaVcSH+H/HbHyCSl93kpWV/5L0g/N4?=
 =?Windows-1252?Q?Vrzb84N13Hg7QgKhE+voongwRkb8mjGD0YUNCUjnv5rRu7F/JexhsY5K?=
 =?Windows-1252?Q?WhiXUsSq4L3328PaMB+MwsJEiWExrOEYd96sjrpvy9wrQalK20R0cMPk?=
 =?Windows-1252?Q?ElPofcR1z1L674Xk15VGmNF0kYd80GcGWN8cKYWc9l+n2q92K0zJXbtS?=
 =?Windows-1252?Q?MwHjWbav7vm4L4+n28aSAAI0FmG2S029ABLaVv0+a7inz2tYATa9RL/J?=
 =?Windows-1252?Q?TVsmd/gYdcIcF59bL9cSKgofpw30KEkKtzkSC5NBVLDwpiA2LEDfV3Em?=
 =?Windows-1252?Q?3TzN44amYLftTWz8hOtjoVNUw485ZA0jbbf/XF0HVd8X07LIOuQS5VmJ?=
 =?Windows-1252?Q?BVsq372A/FJuxEnfcvJZgkunYIlzB4ZjsuiEVRIFLPKbJR2dsF+QtlVO?=
 =?Windows-1252?Q?fKL/zUx5A35P1gbBMXDqQrnKNZNbCR5t9LXv2MdaFfMII/5HXaQ7VSaR?=
 =?Windows-1252?Q?mwKfvWcrDgrCC/FVTsPSWtUopIZu0yik7lS33UwtFDvuugoYsLGSDW8s?=
 =?Windows-1252?Q?Xal61N1O4uBo7hQjJ+TLCBvojnPLTMrVwamsaLKE7kcZmoTXYazlHrgR?=
 =?Windows-1252?Q?I/zxrtiqLMzGLT/NPISGQ9J75RljxTnZ53QB7WNWUCXEz1Z8im1R3Aap?=
 =?Windows-1252?Q?lYUwzbN8mePvRqbYQ9A2OsxVRqvmHKi0z8iXZZ50lAsz40IsfvvQo4tT?=
 =?Windows-1252?Q?FtfWXh/c8NvyII7Nt4yeCen+KRG55bxwOw8+VLVS+Gzo9+EnTfGe2Q?=
 =?Windows-1252?Q?=3D=3D?=
Content-Type: text/plain; charset="Windows-1252"
Content-ID: <666F378CBF03D34E9CCBA5726861B393@eurprd04.prod.outlook.com>
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-OriginatorOrg: nxp.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-AuthSource: VI1PR04MB5136.eurprd04.prod.outlook.com
X-MS-Exchange-CrossTenant-Network-Message-Id: f1119a6e-dcd9-4c23-7a09-08da8e8be770
X-MS-Exchange-CrossTenant-originalarrivaltime: 04 Sep 2022 15:41:16.0540
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 686ea1d3-bc2b-4c6f-a92c-d99c5c301635
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: 31e7BqD/YtspTMA//pu0xnzIJi7wLhudn0f2YrTLD+MB1Ay2DyaX/3KIEdek6L7UqhpIMQB6Ssya9YRE/f/W6g==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM0PR04MB5667
X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED,
        DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE,
        RCVD_IN_MSPIKE_H2,SPF_HELO_PASS,SPF_PASS,T_SCC_BODY_TEXT_LINE
        autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Sat, Sep 03, 2022 at 04:48:32AM +0200, Marek Beh=FAn 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:
>=20
> +----------+   +---------------+   +---------+
> |     eth0 <---> sw0p0   sw0p2 <---> sw1p0
> | cpu      |   |               |   |        ....
> |     eth1 <---> sw0p1   s20p3 <---> sw1p1
> +----------+   +---------------+   +---------+
>=20
> 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 =3D <0>;
		dsa,member =3D <0 0>;

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

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

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

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

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

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

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

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

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

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

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

			port@1 {
				reg =3D <1>;
				link =3D <&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 devi=
ce)
{
	struct dsa_switch_tree *dst =3D ds->dst;
	struct dsa_link *dl;

	list_for_each_entry(dl, &dst->rtable, list)
		if (dl->dp->ds =3D=3D ds && dl->link_dp->ds->index =3D=3D 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=