From:   Vladimir Oltean <vladimir.oltean@nxp.com>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>
CC:     "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "stable@vger.kernel.org" <stable@vger.kernel.org>,
        "David S. Miller" <davem@davemloft.net>,
        Sasha Levin <sashal@kernel.org>
Subject: Re: [PATCH 5.15 301/917] net: dsa: flush switchdev workqueue when
 leaving the bridge
Thread-Topic: [PATCH 5.15 301/917] net: dsa: flush switchdev workqueue when
 leaving the bridge
Thread-Index: AQHX2lCGilYJje69nES2QBLPNshWzKwFPeQA
Date:   Mon, 15 Nov 2021 23:33:59 +0000
Message-ID: <20211115233358.u6n44mop73le7rkw@skbuf>
References: <20211115165428.722074685@linuxfoundation.org>
 <20211115165438.966642608@linuxfoundation.org>
In-Reply-To: <20211115165438.966642608@linuxfoundation.org>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-antispam-messagedata-chunkcount: 1
x-ms-exchange-antispam-messagedata-0: =?us-ascii?Q?w76LzN+9w6OgKiLspqA3ipW9/EwbaRrQ1ZVSkVa1fwFMDvt9cw8lT0LEFvEr?=
 =?us-ascii?Q?twh80bbXoBcLVW22ZcivAx2K2zI5VU5T3pJWgjkwjeErvEzZJHST1smHrpKk?=
 =?us-ascii?Q?FL9K0YLL6UPwJBq07zCjN8OWRH5HTeh48FkOQvOe/3iYJ4jD6mKNqpBDtMwG?=
 =?us-ascii?Q?hhGNIGbfd3FZt/Akq9otdJL2byDg6yku920cUNmtqY+riYxU+aHJjGowAVwV?=
 =?us-ascii?Q?sj85XFoMdlK+cldASE72kGGmgmKCI0NDKeClj6qrfcdzROhhiN5Wttn18JyK?=
 =?us-ascii?Q?kMKrktfESEM3Nmh3hvfqCzBrxCOB9z706pyx+W6TnhwmOGZXiDrdKVCDmpvA?=
 =?us-ascii?Q?9bSXhJ3+owbPHW2MOm9eBTBu/YkWKNPPHqn9JuzvIprGNZK2+4U+NgTaJ8ob?=
 =?us-ascii?Q?Z70eDU0cdukTRGSorpi9AaqS1gqqBmC9GPyfSXgRyNhvDuK6Aol82XP7n5sB?=
 =?us-ascii?Q?ZmLtfpCeuT7QpfR4OHMvj4Wp4+hrc9T0rHT7rUqYiPYiuokDyoKdZ2e7tD65?=
 =?us-ascii?Q?AFlSt6BYL0layk59DrZo9VSBuZI8HbSYsaoKn5oqEUmhYiKPgySDRhhVcUxH?=
 =?us-ascii?Q?6LV3W+ta1tMgD7T1m6oHHPaW2mJpcEOaLJzUzbmOeGHDa726HARbYYYYwKm1?=
 =?us-ascii?Q?NCYK06Cz/dzS2Rkf2boGnkYoP7P18LFunpeIO1l5qf8LzsUt2H72cjNs1/8P?=
 =?us-ascii?Q?OBtWkxreNTwgWxhxSV9jpytdYuhnk1jJP/QFRdbejk65venUxD1sbK/wXRG2?=
 =?us-ascii?Q?glEyptDk++WWfYX8EVtOCI980bswPJSrc4zJdf3TXtzErdIPjRQE2tkkn+Aw?=
 =?us-ascii?Q?lvR3iAdA4V8BetpoHJrI/JNi8Y4jtvEU7QKOAhfsPQz90lNw5eMFWttsFjBV?=
 =?us-ascii?Q?fNhjepn3FV14C7Kf+xvFtcly3v/R3CA/01ElKRksjMucxzmABJ+MeSmU+ugh?=
 =?us-ascii?Q?oc7gCKqtW93ip/2J+B7VemYc6X0q7hyNcnb35Xc2VPtMUija4/eSCAUZoftH?=
 =?us-ascii?Q?zkzOQe8aXyrn/AxzKru3QY+DdJJUYmE0DnWQGg8Orf2YAYyT/60HyEz5rtxh?=
 =?us-ascii?Q?KhFnTflsA8qjKv63Sct+7z6fv/Q8Z2yHvq0PkeB4Jk3HstwQKFMUY4109/eX?=
 =?us-ascii?Q?XwuGlqFOadnVn43svDTd3bAa5JSQ9/J52IMmfVOc42QtoAIt6PLF77ZNY5uq?=
 =?us-ascii?Q?wFv7s6gdmABPg+ZgG48uQIn9WE3RCuZbsCm9OIv3eF1Id31njrdXEDzCGVJo?=
 =?us-ascii?Q?9s8DM/+0XeX7Y9br6nxlQVS2+jBSrvWxzgaX4/H33JQT09vmvCT5RWIgyr/F?=
 =?us-ascii?Q?Bo8R91uuvRJ1hfvemKJ9ERTCpUcjNA20AnqQPRvfQIen9S34eJnal+po8/HT?=
 =?us-ascii?Q?9A7D6lRj2AX5vOULRi9ASZxfmxCTj+x7K22P4xhCSXWJE0+OigEnoRZUA48k?=
 =?us-ascii?Q?WQh+kotvEOimRnJ581S4qNU++CMjrsINU7J5becHgS6WGdEv8eGmYjjMKTDB?=
 =?us-ascii?Q?wQglr84hDp0ia+pSg34+f4nQ+Mtr2pJxRYgO2Nrj/ctfuWx0B9CyLFBo7oE/?=
 =?us-ascii?Q?BjwkoKdI5JVRsQMJW8HuYs1ewyyghj6TbsXjNCtvt79/N/qEayUAURRKW6QN?=
 =?us-ascii?Q?WCOh9JPWiiT6YJPSmqDUHDE=3D?=
Content-Type: text/plain; charset="us-ascii"
Content-ID: <14BBAAF5FCB06B43B40B1ABC7EB6C0D7@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: b6be95eb-65d7-4ec4-611b-08d9a890661a
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Nov 2021 23:33:59.0568
 (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: pKxiAaGCM1Nu/z/XeeNCDMeQ/hjYvjJnmtnxf3LwecrgabnRnND7gIQd8UjtsoTGa8XVKdFKwAsLNY4cbI1V+w==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: VI1PR04MB6015
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Nov 15, 2021 at 05:56:36PM +0100, Greg Kroah-Hartman wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
>=20
> [ Upstream commit d7d0d423dbaa73fd0506e25971dfdab6bf185d00 ]
>=20
> DSA is preparing to offer switch drivers an API through which they can
> associate each FDB entry with a struct net_device *bridge_dev. This can
> be used to perform FDB isolation (the FDB lookup performed on the
> ingress of a standalone, or bridged port, should not find an FDB entry
> that is present in the FDB of another bridge).
>=20
> In preparation of that work, DSA needs to ensure that by the time we
> call the switch .port_fdb_add and .port_fdb_del methods, the
> dp->bridge_dev pointer is still valid, i.e. the port is still a bridge
> port.
>=20
> This is not guaranteed because the SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE API
> requires drivers that must have sleepable context to handle those events
> to schedule the deferred work themselves. DSA does this through the
> dsa_owq.
>=20
> It can happen that a port leaves a bridge, del_nbp() flushes the FDB on
> that port, SWITCHDEV_FDB_DEL_TO_DEVICE is notified in atomic context,
> DSA schedules its deferred work, but del_nbp() finishes unlinking the
> bridge as a master from the port before DSA's deferred work is run.
>=20
> Fundamentally, the port must not be unlinked from the bridge until all
> FDB deletion deferred work items have been flushed. The bridge must wait
> for the completion of these hardware accesses.
>=20
> An attempt has been made to address this issue centrally in switchdev by
> making SWITCHDEV_FDB_DEL_TO_DEVICE deferred (=3D> blocking) at the switch=
dev
> level, which would offer implicit synchronization with del_nbp:
>=20
> https://patchwork.kernel.org/project/netdevbpf/cover/20210820115746.37018=
11-1-vladimir.oltean@nxp.com/
>=20
> but it seems that any attempt to modify switchdev's behavior and make
> the events blocking there would introduce undesirable side effects in
> other switchdev consumers.
>=20
> The most undesirable behavior seems to be that
> switchdev_deferred_process_work() takes the rtnl_mutex itself, which
> would be worse off than having the rtnl_mutex taken individually from
> drivers which is what we have now (except DSA which has removed that
> lock since commit 0faf890fc519 ("net: dsa: drop rtnl_lock from
> dsa_slave_switchdev_event_work")).
>=20
> So to offer the needed guarantee to DSA switch drivers, I have come up
> with a compromise solution that does not require switchdev rework:
> we already have a hook at the last moment in time when the bridge is
> still an upper of ours: the NETDEV_PRECHANGEUPPER handler. We can flush
> the dsa_owq manually from there, which makes all FDB deletions
> synchronous.
>=20
> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
> Signed-off-by: David S. Miller <davem@davemloft.net>
> Signed-off-by: Sasha Levin <sashal@kernel.org>
> ---
>  net/dsa/port.c | 2 ++
>  1 file changed, 2 insertions(+)
>=20
> diff --git a/net/dsa/port.c b/net/dsa/port.c
> index 616330a16d319..3947537ed46ba 100644
> --- a/net/dsa/port.c
> +++ b/net/dsa/port.c
> @@ -380,6 +380,8 @@ void dsa_port_pre_bridge_leave(struct dsa_port *dp, s=
truct net_device *br)
>  	switchdev_bridge_port_unoffload(brport_dev, dp,
>  					&dsa_slave_switchdev_notifier,
>  					&dsa_slave_switchdev_blocking_notifier);
> +
> +	dsa_flush_workqueue();
>  }
> =20
>  void dsa_port_bridge_leave(struct dsa_port *dp, struct net_device *br)
> --=20
> 2.33.0
>=20
>=20
>

This patch represents preparation work for a new feature. Unless it
constitutes a dependency for some other bugfix patches (which I doubt),
my suggestion is to not backport it. Thanks.=