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:     Horatiu Vultur <horatiu.vultur@microchip.com>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "netdev@vger.kernel.org" <netdev@vger.kernel.org>,
        "davem@davemloft.net" <davem@davemloft.net>,
        "edumazet@google.com" <edumazet@google.com>,
        "kuba@kernel.org" <kuba@kernel.org>,
        "pabeni@redhat.com" <pabeni@redhat.com>
Subject: Re: [PATCH net-next v2 0/7] net: lan966x: Add lag support
Thread-Topic: [PATCH net-next v2 0/7] net: lan966x: Add lag support
Thread-Index: AQHYimHdDTDh5jwai02cndkVFmS37q1mUicAgAICugCAAWcnAA==
Date:   Fri, 1 Jul 2022 16:34:15 +0000
Message-ID: <20220701163414.q3dpd32a2hwi6dtg@skbuf>
References: <20220627201330.45219-1-horatiu.vultur@microchip.com>
 <20220629122630.qd7nsqmkxoshovhc@skbuf>
 <20220630190846.aqagifacrleejrjc@soft-dev3-1.localhost>
In-Reply-To: <20220630190846.aqagifacrleejrjc@soft-dev3-1.localhost>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?x/sevI0LJ8VB3Dq32uq+jRzMmYp4rnZ7BSMYNZBt7aA/MckJFSfNI//YfKi1?=
 =?us-ascii?Q?S4xyn3Uv01nLOeqIDqyR9ym6J/34VLOGE+odiDhktmP5M8B6Af8Shvp64bC/?=
 =?us-ascii?Q?Up84HvjXUqTVvyPRiEycwNKs+Rm0RUqAQhXWIFHUSu8POyDX2buLUe3FOJ9J?=
 =?us-ascii?Q?biFYK9M8S0c8q6b75Q4iFpDVdmGEwmFHRxexHGsXBnF04TXwYenb2DvreAod?=
 =?us-ascii?Q?t75wJunpHpDFgotMihH8ysKuIwFeyShqAaVvQNl2LHyEI1LkhiCsbKRgDSJb?=
 =?us-ascii?Q?ykL5/+bZw9fsUuW5BqggqSO69Y1pwT9U+nG+uve9eQcbLeOG8RrEP5MSYeZf?=
 =?us-ascii?Q?QV1a2mf+Dy6LJQTnd0tcX4LIFLkd1g9tHRMdIb19b2JIQKPnS15p2Jlcbu3A?=
 =?us-ascii?Q?6W2AycNEjdWYwgO9wft/8ksJz2BO6r1ChjquleNcDjzYYkBj7xFW+T/BxNWX?=
 =?us-ascii?Q?SY3XEYpeUXV69nOrFSDh3GhJKJlB3Eoj5FjkWCiU4tAwaK95Xee/hDsuwXLd?=
 =?us-ascii?Q?bn8AUR2ib0xmKQdymkHPdSLzSy/bpYl3dlL0HkYWgvvWodXCPpscN5iyCtH/?=
 =?us-ascii?Q?Z/9RAvZJ7sAPpetN3gMXSPFOvG01w7Mdgohq72qtH8RnLjwnLjRISWAkHwDH?=
 =?us-ascii?Q?xtN1EfyC7GI4tHdFiMiEhF3drIKqOw6NeCPpq2AYpeTFmhOFA/sSxXU3MCGm?=
 =?us-ascii?Q?ozDxKgAeAtVfnk1DTSC60CTRyC0NXxlEeGMWLJLg/EXp6/VffxiRXmH5LOhd?=
 =?us-ascii?Q?m96LK0HH12YqotGQbyY/SBzWEWCjpDQSea5KjEbrC0oGeC7xda75aCWGfcWu?=
 =?us-ascii?Q?9pHhzed8iHMIm3Lus8TiLM79VnknKimEkHFe4/B5+wwI0XNp5znE3gc7xtTp?=
 =?us-ascii?Q?vLJOkx6UmnGIklhDTWXI9JtpXkHJhnqpQJkytFo4KNjhzSBQCp3vKVp9iMr7?=
 =?us-ascii?Q?hmK1zY29zMpdncKISi24yb5UNW/qqj6CL6d7WYUq+RV+ZXUKvAyjbZi8LOUu?=
 =?us-ascii?Q?/CUzKkZnvgjcEpvHhTaaHwAnyPjx7siWl0h2ws/VgL822RgTC0K2b41a7bpF?=
 =?us-ascii?Q?Sypy0xYt/+6ixZo9rJja66USg1NrGWyOYYqkzXwUukGq5AzymDVe+cXejl8u?=
 =?us-ascii?Q?BF58VX142sTX1KDwkVb+gOWqrGlR3Wz837QdZ3KPqDs4FHU3sJvSbGoM2nkA?=
 =?us-ascii?Q?GiJZJdyE8bIBRzelncQQ+aaWrGY06W0Wzkk2usdYmdHlQ4wj+14PeVrlB41G?=
 =?us-ascii?Q?ud68Gnp1udjp0FhnCyhLFnktg+xIorJMW5mrxcMygKAtnxdWLlvbsBjPJHZX?=
 =?us-ascii?Q?3S2kNN0WT4jjJk/PILgdoFYtQE7FlMsCKQ/mvey3+HI7xjKs+cXmZWqD9AVf?=
 =?us-ascii?Q?4JHxrZT+NVY+2SOnvxkVkMGHfb9wUc6VYrHfh9YbeQwtN37/6PMKJtnz62h8?=
 =?us-ascii?Q?Z+SMLHupHp7jZH9vVAOZCdcs7ErVwAI7evO7xkubqza8fnYCFSoAGu1TJ9Tr?=
 =?us-ascii?Q?tOvwvGNnuP7MYufcrykSpY5Sw4hyiR7eBPizTsOM85BOv78IpD32JkFCnZoO?=
 =?us-ascii?Q?G1i+H1ScdwuYkDt/L0N20tb77BhyljS0NvhN1tM2IceX0XsLRz+7i8XuQ5Ao?=
 =?us-ascii?Q?lg=3D=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <E0A90D906843704FB8F5D1A2730815BA@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: 698d57aa-0b65-4f4e-c228-08da5b7f89df
X-MS-Exchange-CrossTenant-originalarrivaltime: 01 Jul 2022 16:34:15.8307
 (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: JVLSjzsLiz8bYznaO0tLgw1f1F9tG3W5eeOWnYzyk0Ip4wIY86xymhscaV6NH9yd4Mwfbb31mw9Pn0k6n35aVA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: AM8PR04MB7347
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 Thu, Jun 30, 2022 at 09:08:46PM +0200, Horatiu Vultur wrote:
> > I've downloaded and applied your patches and I have some general feedba=
ck.
> > Some of it relates to changes which were not made and hence I couldn't
> > have commented on the patches themselves, so I'm posting it here.
> >=20
> > 1. switchdev_bridge_port_offload() returns an error code if object
> > replay failed, or if it couldn't get the port parent id, or if the user
> > tries to join a lan966x port and a port belonging to another switchdev
> > driver to the same LAG. It would be good to propagate this error and no=
t
> > ignore it.
>=20
> Yes, I will do that.
>=20
> What about the case when the other port is not a switchdev port. For
> example:
> ip link set dev eth0 master bond0
> ip link set dev dummy master bond0
> ip link set dev bond0 master br0
>=20
> At the last line, I was expecting to get an error.

switchdev_bridge_port_offload() currently only detects mismatched port
parent IDs, so it will not detect this condition where one bond slave is
switchdev and the other isn't. This is because the non-switchdev bond
slave does not even call switchdev_bridge_port_offload(), it's completely
silent from the perspective of the bridge.
Fact is that we don't have a common layer that enforces all common sense
netdev adjacency restrictions with switchdev, and that is one of the big
problems of the system.

> > 6. You are missing LAG FDB migration logic in lan966x_lag_port_join().
> > Specifically, you assume that the lan966x_lag_first_port() will never
> > change, probably because you just make the switch ports join the LAG in
> > the order 1, 2, 3. But they can also join in the order 3, 2, 1.
>=20
> It would work, but there will be problems when the ports start to leave
> the LAG.
> It would work because all the ports under the LAG will have the same
> value in PGID_ID for DST_IDX. So if the MAC entry points to any of
> this entries will be OK.

OK, I forgot DEST_IDX selects PGID and not logical port ID directly.

> The problem is when the port leaves the LAG, if the MAC entry points
> to the port that left the LAG then is not working anymore.
> I will fix this in the next series.=