[PATCH v3 net-next 4/7] net: bridge: switchdev: make br_fdb_replay offer sleepable context to consumers

From: Vladimir Oltean <vladimir.oltean@nxp.com>
To: netdev@vger.kernel.org, Jakub Kicinski <kuba@kernel.org>,
	"David S. Miller" <davem@davemloft.net>
Cc: Roopa Prabhu <roopa@nvidia.com>,
	Nikolay Aleksandrov <nikolay@nvidia.com>,
	Andrew Lunn <andrew@lunn.ch>,
	Florian Fainelli <f.fainelli@gmail.com>,
	Vivien Didelot <vivien.didelot@gmail.com>,
	Vladimir Oltean <olteanv@gmail.com>,
	Vadym Kochan <vkochan@marvell.com>,
	Taras Chornyi <tchornyi@marvell.com>,
	Jiri Pirko <jiri@nvidia.com>, Ido Schimmel <idosch@nvidia.com>,
	UNGLinuxDriver@microchip.com,
	Grygorii Strashko <grygorii.strashko@ti.com>,
	Marek Behun <kabel@blackhole.sk>,
	DENG Qingfang <dqfext@gmail.com>,
	Kurt Kanzenbach <kurt@linutronix.de>,
	Hauke Mehrtens <hauke@hauke-m.de>,
	Woojung Huh <woojung.huh@microchip.com>,
	Sean Wang <sean.wang@mediatek.com>,
	Landen Chao <Landen.Chao@mediatek.com>,
	Claudiu Manoil <claudiu.manoil@nxp.com>,
	Alexandre Belloni <alexandre.belloni@bootlin.com>,
	George McCollister <george.mccollister@gmail.com>,
	Ioana Ciornei <ioana.ciornei@nxp.com>,
	Saeed Mahameed <saeedm@nvidia.com>,
	Leon Romanovsky <leon@kernel.org>,
	Lars Povlsen <lars.povlsen@microchip.com>,
	Steen Hegelund <Steen.Hegelund@microchip.com>,
	Julian Wiedmann <jwi@linux.ibm.com>,
	Alexandra Winter <wintera@linux.ibm.com>,
	Karsten Graul <kgraul@linux.ibm.com>,
	Heiko Carstens <hca@linux.ibm.com>,
	Vasily Gorbik <gor@linux.ibm.com>,
	Christian Borntraeger <borntraeger@de.ibm.com>,
	Ivan Vecera <ivecera@redhat.com>, Vlad Buslov <vladbu@nvidia.com>,
	Jianbo Liu <jianbol@nvidia.com>, Mark Bloch <mbloch@nvidia.com>,
	Roi Dayan <roid@nvidia.com>,
	Tobias Waldekranz <tobias@waldekranz.com>,
	Vignesh Raghavendra <vigneshr@ti.com>,
	Jesse Brandeburg <jesse.brandeburg@intel.com>,
	linux-s390@vger.kernel.org
Subject: [PATCH v3 net-next 4/7] net: bridge: switchdev: make br_fdb_replay offer sleepable context to consumers
Date: Fri, 20 Aug 2021 14:57:43 +0300	[thread overview]
Message-ID: <20210820115746.3701811-5-vladimir.oltean@nxp.com> (raw)
In-Reply-To: <20210820115746.3701811-1-vladimir.oltean@nxp.com>

Now that the SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE events are notified on
the blocking chain, it would be nice if we could also drop the
rcu_read_lock() atomic context from br_fdb_replay() so that drivers can
actually benefit from the blocking context and simplify their logic.

Do something similar to what is done in br_mdb_queue_one/br_mdb_replay_one.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
v2->v3:
since we want to drop the RCU protection early, we need to copy the
fdb->addr from the bridge FDB entries early too. So we need to queue the
FDB entries as switchdev objects and not bridge objects (those still
have an "addr" pointer and not array) and keep them in a list. So add a
list element to the switchdev FDB structure, like we have in struct
switchdev_obj too.

 include/net/switchdev.h   |  1 +
 net/bridge/br_switchdev.c | 88 +++++++++++++++++++++++++++------------
 2 files changed, 62 insertions(+), 27 deletions(-)

diff --git a/include/net/switchdev.h b/include/net/switchdev.h
index e27da5bd665f..5570df4d9b76 100644
--- a/include/net/switchdev.h
+++ b/include/net/switchdev.h
@@ -218,6 +218,7 @@ struct switchdev_notifier_info {
 
 struct switchdev_notifier_fdb_info {
 	struct switchdev_notifier_info info; /* must be first */
+	struct list_head list;
 	unsigned char addr[ETH_ALEN];
 	u16 vid;
 	u8 added_by_user:1,
diff --git a/net/bridge/br_switchdev.c b/net/bridge/br_switchdev.c
index c7c8e23c2147..f2cb066e3ebb 100644
--- a/net/bridge/br_switchdev.c
+++ b/net/bridge/br_switchdev.c
@@ -122,21 +122,30 @@ int br_switchdev_set_port_flag(struct net_bridge_port *p,
 	return 0;
 }
 
+static void br_switchdev_fdb_populate(struct net_bridge *br,
+				      struct switchdev_notifier_fdb_info *info,
+				      struct net_device **dev,
+				      const struct net_bridge_fdb_entry *fdb)
+{
+	const struct net_bridge_port *dst = READ_ONCE(fdb->dst);
+
+	ether_addr_copy(info->addr, fdb->key.addr.addr);
+	info->vid = fdb->key.vlan_id;
+	info->added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
+	info->is_local = test_bit(BR_FDB_LOCAL, &fdb->flags);
+	info->offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags);
+
+	*dev = (!dst || info->is_local) ? br->dev : dst->dev;
+}
+
 void
 br_switchdev_fdb_notify(struct net_bridge *br,
 			const struct net_bridge_fdb_entry *fdb, int type)
 {
-	const struct net_bridge_port *dst = READ_ONCE(fdb->dst);
 	struct switchdev_notifier_fdb_info info = {};
 	struct net_device *dev;
 
-	ether_addr_copy(info.addr, fdb->key.addr.addr);
-	info.vid = fdb->key.vlan_id;
-	info.added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
-	info.is_local = test_bit(BR_FDB_LOCAL, &fdb->flags);
-	info.offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags);
-
-	dev = (!dst || info.is_local) ? br->dev : dst->dev;
+	br_switchdev_fdb_populate(br, &info, &dev, fdb);
 
 	switch (type) {
 	case RTM_DELNEIGH:
@@ -270,32 +279,43 @@ static void nbp_switchdev_del(struct net_bridge_port *p)
 	}
 }
 
-static int br_fdb_replay_one(struct net_bridge *br, struct notifier_block *nb,
-			     const struct net_bridge_fdb_entry *fdb,
-			     unsigned long action, const void *ctx)
+static int br_fdb_replay_one(struct notifier_block *nb,
+			     struct switchdev_notifier_fdb_info *info,
+			     unsigned long action)
 {
-	const struct net_bridge_port *p = READ_ONCE(fdb->dst);
-	struct switchdev_notifier_fdb_info item;
 	int err;
 
-	ether_addr_copy(item.addr, fdb->key.addr.addr);
-	item.vid = fdb->key.vlan_id;
-	item.added_by_user = test_bit(BR_FDB_ADDED_BY_USER, &fdb->flags);
-	item.offloaded = test_bit(BR_FDB_OFFLOADED, &fdb->flags);
-	item.is_local = test_bit(BR_FDB_LOCAL, &fdb->flags);
-	item.info.dev = (!p || item.is_local) ? br->dev : p->dev;
-	item.info.ctx = ctx;
-
-	err = nb->notifier_call(nb, action, &item);
+	err = nb->notifier_call(nb, action, info);
 	return notifier_to_errno(err);
 }
 
+static int br_fdb_queue_one(struct net_bridge *br, struct list_head *fdb_list,
+			    const struct net_bridge_fdb_entry *fdb,
+			    const void *ctx)
+{
+	struct switchdev_notifier_fdb_info *info;
+	struct net_device *dev;
+
+	info = kzalloc(sizeof(*info), GFP_ATOMIC);
+	if (!info)
+		return -ENOMEM;
+
+	br_switchdev_fdb_populate(br, info, &dev, fdb);
+	info->info.dev = dev;
+	info->info.ctx = ctx;
+	list_add_tail(&info->list, fdb_list);
+
+	return 0;
+}
+
 static int br_fdb_replay(const struct net_device *br_dev, const void *ctx,
 			 bool adding, struct notifier_block *nb)
 {
+	struct switchdev_notifier_fdb_info *info, *tmp;
 	struct net_bridge_fdb_entry *fdb;
 	struct net_bridge *br;
 	unsigned long action;
+	LIST_HEAD(fdb_list);
 	int err = 0;
 
 	if (!nb)
@@ -308,20 +328,34 @@ static int br_fdb_replay(const struct net_device *br_dev, const void *ctx,
 
 	br = netdev_priv(br_dev);
 
+	rcu_read_lock();
+
+	hlist_for_each_entry_rcu(fdb, &br->fdb_list, fdb_node) {
+		err = br_fdb_queue_one(br, &fdb_list, fdb, ctx);
+		if (err) {
+			rcu_read_unlock();
+			goto out_free_fdb;
+		}
+	}
+
+	rcu_read_unlock();
+
 	if (adding)
 		action = SWITCHDEV_FDB_ADD_TO_DEVICE;
 	else
 		action = SWITCHDEV_FDB_DEL_TO_DEVICE;
 
-	rcu_read_lock();
-
-	hlist_for_each_entry_rcu(fdb, &br->fdb_list, fdb_node) {
-		err = br_fdb_replay_one(br, nb, fdb, action, ctx);
+	list_for_each_entry(info, &fdb_list, list) {
+		err = br_fdb_replay_one(nb, info, action);
 		if (err)
 			break;
 	}
 
-	rcu_read_unlock();
+out_free_fdb:
+	list_for_each_entry_safe(info, tmp, &fdb_list, list) {
+		list_del(&info->list);
+		kfree(info);
+	}
 
 	return err;
 }
-- 
2.25.1

