From d8c2858181ccf0ca506b75ffe1ffab25a090d0e4 Mon Sep 17 00:00:00 2001 From: "Minghao Chi (CGEL ZTE)" Date: Thu, 10 Feb 2022 06:10:08 +0000 Subject: net/switchdev: use struct_size over open coded arithmetic Replace zero-length array with flexible-array member and make use of the struct_size() helper in kmalloc(). For example: struct switchdev_deferred_item { ... unsigned long data[]; }; Make use of the struct_size() helper instead of an open-coded version in order to avoid any potential type mistakes. Reported-by: Zeal Robot Signed-off-by: Minghao Chi (CGEL ZTE) Signed-off-by: David S. Miller --- net/switchdev/switchdev.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'net/switchdev/switchdev.c') diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c index b62565278fac..12e6b4146bfb 100644 --- a/net/switchdev/switchdev.c +++ b/net/switchdev/switchdev.c @@ -85,7 +85,7 @@ static int switchdev_deferred_enqueue(struct net_device *dev, { struct switchdev_deferred_item *dfitem; - dfitem = kmalloc(sizeof(*dfitem) + data_len, GFP_ATOMIC); + dfitem = kmalloc(struct_size(dfitem, data, data_len), GFP_ATOMIC); if (!dfitem) return -ENOMEM; dfitem->dev = dev; -- cgit From 7b465f4cf39ea1f5df7f425b843578b60f673155 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Tue, 15 Feb 2022 19:02:15 +0200 Subject: net: switchdev: rename switchdev_lower_dev_find to switchdev_lower_dev_find_rcu switchdev_lower_dev_find() assumes RCU read-side critical section calling context, since it uses netdev_walk_all_lower_dev_rcu(). Rename it appropriately, in preparation of adding a similar iterator that assumes writer-side rtnl_mutex protection. Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- net/switchdev/switchdev.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'net/switchdev/switchdev.c') diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c index 12e6b4146bfb..d53f364870a5 100644 --- a/net/switchdev/switchdev.c +++ b/net/switchdev/switchdev.c @@ -409,10 +409,10 @@ static int switchdev_lower_dev_walk(struct net_device *lower_dev, } static struct net_device * -switchdev_lower_dev_find(struct net_device *dev, - bool (*check_cb)(const struct net_device *dev), - bool (*foreign_dev_check_cb)(const struct net_device *dev, - const struct net_device *foreign_dev)) +switchdev_lower_dev_find_rcu(struct net_device *dev, + bool (*check_cb)(const struct net_device *dev), + bool (*foreign_dev_check_cb)(const struct net_device *dev, + const struct net_device *foreign_dev)) { struct switchdev_nested_priv switchdev_priv = { .check_cb = check_cb, @@ -451,7 +451,7 @@ static int __switchdev_handle_fdb_event_to_device(struct net_device *dev, return mod_cb(dev, orig_dev, event, info->ctx, fdb_info); if (netif_is_lag_master(dev)) { - if (!switchdev_lower_dev_find(dev, check_cb, foreign_dev_check_cb)) + if (!switchdev_lower_dev_find_rcu(dev, check_cb, foreign_dev_check_cb)) goto maybe_bridged_with_us; /* This is a LAG interface that we offload */ @@ -465,7 +465,7 @@ static int __switchdev_handle_fdb_event_to_device(struct net_device *dev, * towards a bridge device. */ if (netif_is_bridge_master(dev)) { - if (!switchdev_lower_dev_find(dev, check_cb, foreign_dev_check_cb)) + if (!switchdev_lower_dev_find_rcu(dev, check_cb, foreign_dev_check_cb)) return 0; /* This is a bridge interface that we offload */ @@ -478,8 +478,8 @@ static int __switchdev_handle_fdb_event_to_device(struct net_device *dev, * that we offload. */ if (!check_cb(lower_dev) && - !switchdev_lower_dev_find(lower_dev, check_cb, - foreign_dev_check_cb)) + !switchdev_lower_dev_find_rcu(lower_dev, check_cb, + foreign_dev_check_cb)) continue; err = __switchdev_handle_fdb_event_to_device(lower_dev, orig_dev, @@ -501,7 +501,7 @@ maybe_bridged_with_us: if (!br || !netif_is_bridge_master(br)) return 0; - if (!switchdev_lower_dev_find(br, check_cb, foreign_dev_check_cb)) + if (!switchdev_lower_dev_find_rcu(br, check_cb, foreign_dev_check_cb)) return 0; return __switchdev_handle_fdb_event_to_device(br, orig_dev, event, fdb_info, -- cgit From c4076cdd21f8d68a96f1e7124bd8915c7e31a474 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Tue, 15 Feb 2022 19:02:16 +0200 Subject: net: switchdev: introduce switchdev_handle_port_obj_{add,del} for foreign interfaces The switchdev_handle_port_obj_add() helper is good for replicating a port object on the lower interfaces of @dev, if that object was emitted on a bridge, or on a bridge port that is a LAG. However, drivers that use this helper limit themselves to a box from which they can no longer intercept port objects notified on neighbor ports ("foreign interfaces"). One such driver is DSA, where software bridging with foreign interfaces such as standalone NICs or Wi-Fi APs is an important use case. There, a VLAN installed on a neighbor bridge port roughly corresponds to a forwarding VLAN installed on the DSA switch's CPU port. To support this use case while also making use of the benefits of the switchdev_handle_* replication helper for port objects, introduce a new variant of these functions that crawls through the neighbor ports of @dev, in search of potentially compatible switchdev ports that are interested in the event. The strategy is identical to switchdev_handle_fdb_event_to_device(): if @dev wasn't a switchdev interface, then go one step upper, and recursively call this function on the bridge that this port belongs to. At the next recursion step, __switchdev_handle_port_obj_add() will iterate through the bridge's lower interfaces. Among those, some will be switchdev interfaces, and one will be the original @dev that we came from. To prevent infinite recursion, we must suppress reentry into the original @dev, and just call the @add_cb for the switchdev_interfaces. It looks like this: br0 / | \ / | \ / | \ swp0 swp1 eth0 1. __switchdev_handle_port_obj_add(eth0) -> check_cb(eth0) returns false -> eth0 has no lower interfaces -> eth0's bridge is br0 -> switchdev_lower_dev_find(br0, check_cb, foreign_dev_check_cb)) finds br0 2. __switchdev_handle_port_obj_add(br0) -> check_cb(br0) returns false -> netdev_for_each_lower_dev -> check_cb(swp0) returns true, so we don't skip this interface 3. __switchdev_handle_port_obj_add(swp0) -> check_cb(swp0) returns true, so we call add_cb(swp0) (back to netdev_for_each_lower_dev from 2) -> check_cb(swp1) returns true, so we don't skip this interface 4. __switchdev_handle_port_obj_add(swp1) -> check_cb(swp1) returns true, so we call add_cb(swp1) (back to netdev_for_each_lower_dev from 2) -> check_cb(eth0) returns false, so we skip this interface to avoid infinite recursion Note: eth0 could have been a LAG, and we don't want to suppress the recursion through its lowers if those exist, so when check_cb() returns false, we still call switchdev_lower_dev_find() to estimate whether there's anything worth a recursion beneath that LAG. Using check_cb() and foreign_dev_check_cb(), switchdev_lower_dev_find() not only figures out whether the lowers of the LAG are switchdev, but also whether they actively offload the LAG or not (whether the LAG is "foreign" to the switchdev interface or not). The port_obj_info->orig_dev is preserved across recursive calls, so switchdev drivers still know on which device was this notification originally emitted. Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- include/net/switchdev.h | 39 +++++++++++++ net/switchdev/switchdev.c | 140 +++++++++++++++++++++++++++++++++++++++++++--- 2 files changed, 171 insertions(+), 8 deletions(-) (limited to 'net/switchdev/switchdev.c') diff --git a/include/net/switchdev.h b/include/net/switchdev.h index 92cc763991e9..c32e1c8f79ec 100644 --- a/include/net/switchdev.h +++ b/include/net/switchdev.h @@ -324,11 +324,26 @@ int switchdev_handle_port_obj_add(struct net_device *dev, int (*add_cb)(struct net_device *dev, const void *ctx, const struct switchdev_obj *obj, struct netlink_ext_ack *extack)); +int switchdev_handle_port_obj_add_foreign(struct net_device *dev, + struct switchdev_notifier_port_obj_info *port_obj_info, + bool (*check_cb)(const struct net_device *dev), + bool (*foreign_dev_check_cb)(const struct net_device *dev, + const struct net_device *foreign_dev), + int (*add_cb)(struct net_device *dev, const void *ctx, + const struct switchdev_obj *obj, + struct netlink_ext_ack *extack)); int switchdev_handle_port_obj_del(struct net_device *dev, struct switchdev_notifier_port_obj_info *port_obj_info, bool (*check_cb)(const struct net_device *dev), int (*del_cb)(struct net_device *dev, const void *ctx, const struct switchdev_obj *obj)); +int switchdev_handle_port_obj_del_foreign(struct net_device *dev, + struct switchdev_notifier_port_obj_info *port_obj_info, + bool (*check_cb)(const struct net_device *dev), + bool (*foreign_dev_check_cb)(const struct net_device *dev, + const struct net_device *foreign_dev), + int (*del_cb)(struct net_device *dev, const void *ctx, + const struct switchdev_obj *obj)); int switchdev_handle_port_attr_set(struct net_device *dev, struct switchdev_notifier_port_attr_info *port_attr_info, @@ -447,6 +462,18 @@ switchdev_handle_port_obj_add(struct net_device *dev, return 0; } +static inline int switchdev_handle_port_obj_add_foreign(struct net_device *dev, + struct switchdev_notifier_port_obj_info *port_obj_info, + bool (*check_cb)(const struct net_device *dev), + bool (*foreign_dev_check_cb)(const struct net_device *dev, + const struct net_device *foreign_dev), + int (*add_cb)(struct net_device *dev, const void *ctx, + const struct switchdev_obj *obj, + struct netlink_ext_ack *extack)) +{ + return 0; +} + static inline int switchdev_handle_port_obj_del(struct net_device *dev, struct switchdev_notifier_port_obj_info *port_obj_info, @@ -457,6 +484,18 @@ switchdev_handle_port_obj_del(struct net_device *dev, return 0; } +static inline int +switchdev_handle_port_obj_del_foreign(struct net_device *dev, + struct switchdev_notifier_port_obj_info *port_obj_info, + bool (*check_cb)(const struct net_device *dev), + bool (*foreign_dev_check_cb)(const struct net_device *dev, + const struct net_device *foreign_dev), + int (*del_cb)(struct net_device *dev, const void *ctx, + const struct switchdev_obj *obj)) +{ + return 0; +} + static inline int switchdev_handle_port_attr_set(struct net_device *dev, struct switchdev_notifier_port_attr_info *port_attr_info, diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c index d53f364870a5..6a00c390547b 100644 --- a/net/switchdev/switchdev.c +++ b/net/switchdev/switchdev.c @@ -429,6 +429,27 @@ switchdev_lower_dev_find_rcu(struct net_device *dev, return switchdev_priv.lower_dev; } +static struct net_device * +switchdev_lower_dev_find(struct net_device *dev, + bool (*check_cb)(const struct net_device *dev), + bool (*foreign_dev_check_cb)(const struct net_device *dev, + const struct net_device *foreign_dev)) +{ + struct switchdev_nested_priv switchdev_priv = { + .check_cb = check_cb, + .foreign_dev_check_cb = foreign_dev_check_cb, + .dev = dev, + .lower_dev = NULL, + }; + struct netdev_nested_priv priv = { + .data = &switchdev_priv, + }; + + netdev_walk_all_lower_dev(dev, switchdev_lower_dev_walk, &priv); + + return switchdev_priv.lower_dev; +} + static int __switchdev_handle_fdb_event_to_device(struct net_device *dev, struct net_device *orig_dev, unsigned long event, const struct switchdev_notifier_fdb_info *fdb_info, @@ -536,13 +557,15 @@ EXPORT_SYMBOL_GPL(switchdev_handle_fdb_event_to_device); static int __switchdev_handle_port_obj_add(struct net_device *dev, struct switchdev_notifier_port_obj_info *port_obj_info, bool (*check_cb)(const struct net_device *dev), + bool (*foreign_dev_check_cb)(const struct net_device *dev, + const struct net_device *foreign_dev), int (*add_cb)(struct net_device *dev, const void *ctx, const struct switchdev_obj *obj, struct netlink_ext_ack *extack)) { struct switchdev_notifier_info *info = &port_obj_info->info; + struct net_device *br, *lower_dev; struct netlink_ext_ack *extack; - struct net_device *lower_dev; struct list_head *iter; int err = -EOPNOTSUPP; @@ -566,15 +589,42 @@ static int __switchdev_handle_port_obj_add(struct net_device *dev, if (netif_is_bridge_master(lower_dev)) continue; + /* When searching for switchdev interfaces that are neighbors + * of foreign ones, and @dev is a bridge, do not recurse on the + * foreign interface again, it was already visited. + */ + if (foreign_dev_check_cb && !check_cb(lower_dev) && + !switchdev_lower_dev_find(lower_dev, check_cb, foreign_dev_check_cb)) + continue; + err = __switchdev_handle_port_obj_add(lower_dev, port_obj_info, - check_cb, add_cb); + check_cb, foreign_dev_check_cb, + add_cb); if (err && err != -EOPNOTSUPP) return err; } - return err; + /* Event is neither on a bridge nor a LAG. Check whether it is on an + * interface that is in a bridge with us. + */ + if (!foreign_dev_check_cb) + return err; + + br = netdev_master_upper_dev_get(dev); + if (!br || !netif_is_bridge_master(br)) + return err; + + if (!switchdev_lower_dev_find(br, check_cb, foreign_dev_check_cb)) + return err; + + return __switchdev_handle_port_obj_add(br, port_obj_info, check_cb, + foreign_dev_check_cb, add_cb); } +/* Pass through a port object addition, if @dev passes @check_cb, or replicate + * it towards all lower interfaces of @dev that pass @check_cb, if @dev is a + * bridge or a LAG. + */ int switchdev_handle_port_obj_add(struct net_device *dev, struct switchdev_notifier_port_obj_info *port_obj_info, bool (*check_cb)(const struct net_device *dev), @@ -585,21 +635,46 @@ int switchdev_handle_port_obj_add(struct net_device *dev, int err; err = __switchdev_handle_port_obj_add(dev, port_obj_info, check_cb, - add_cb); + NULL, add_cb); if (err == -EOPNOTSUPP) err = 0; return err; } EXPORT_SYMBOL_GPL(switchdev_handle_port_obj_add); +/* Same as switchdev_handle_port_obj_add(), except if object is notified on a + * @dev that passes @foreign_dev_check_cb, it is replicated towards all devices + * that pass @check_cb and are in the same bridge as @dev. + */ +int switchdev_handle_port_obj_add_foreign(struct net_device *dev, + struct switchdev_notifier_port_obj_info *port_obj_info, + bool (*check_cb)(const struct net_device *dev), + bool (*foreign_dev_check_cb)(const struct net_device *dev, + const struct net_device *foreign_dev), + int (*add_cb)(struct net_device *dev, const void *ctx, + const struct switchdev_obj *obj, + struct netlink_ext_ack *extack)) +{ + int err; + + err = __switchdev_handle_port_obj_add(dev, port_obj_info, check_cb, + foreign_dev_check_cb, add_cb); + if (err == -EOPNOTSUPP) + err = 0; + return err; +} +EXPORT_SYMBOL_GPL(switchdev_handle_port_obj_add_foreign); + static int __switchdev_handle_port_obj_del(struct net_device *dev, struct switchdev_notifier_port_obj_info *port_obj_info, bool (*check_cb)(const struct net_device *dev), + bool (*foreign_dev_check_cb)(const struct net_device *dev, + const struct net_device *foreign_dev), int (*del_cb)(struct net_device *dev, const void *ctx, const struct switchdev_obj *obj)) { struct switchdev_notifier_info *info = &port_obj_info->info; - struct net_device *lower_dev; + struct net_device *br, *lower_dev; struct list_head *iter; int err = -EOPNOTSUPP; @@ -621,15 +696,42 @@ static int __switchdev_handle_port_obj_del(struct net_device *dev, if (netif_is_bridge_master(lower_dev)) continue; + /* When searching for switchdev interfaces that are neighbors + * of foreign ones, and @dev is a bridge, do not recurse on the + * foreign interface again, it was already visited. + */ + if (foreign_dev_check_cb && !check_cb(lower_dev) && + !switchdev_lower_dev_find(lower_dev, check_cb, foreign_dev_check_cb)) + continue; + err = __switchdev_handle_port_obj_del(lower_dev, port_obj_info, - check_cb, del_cb); + check_cb, foreign_dev_check_cb, + del_cb); if (err && err != -EOPNOTSUPP) return err; } - return err; + /* Event is neither on a bridge nor a LAG. Check whether it is on an + * interface that is in a bridge with us. + */ + if (!foreign_dev_check_cb) + return err; + + br = netdev_master_upper_dev_get(dev); + if (!br || !netif_is_bridge_master(br)) + return err; + + if (!switchdev_lower_dev_find(br, check_cb, foreign_dev_check_cb)) + return err; + + return __switchdev_handle_port_obj_del(br, port_obj_info, check_cb, + foreign_dev_check_cb, del_cb); } +/* Pass through a port object deletion, if @dev passes @check_cb, or replicate + * it towards all lower interfaces of @dev that pass @check_cb, if @dev is a + * bridge or a LAG. + */ int switchdev_handle_port_obj_del(struct net_device *dev, struct switchdev_notifier_port_obj_info *port_obj_info, bool (*check_cb)(const struct net_device *dev), @@ -639,13 +741,35 @@ int switchdev_handle_port_obj_del(struct net_device *dev, int err; err = __switchdev_handle_port_obj_del(dev, port_obj_info, check_cb, - del_cb); + NULL, del_cb); if (err == -EOPNOTSUPP) err = 0; return err; } EXPORT_SYMBOL_GPL(switchdev_handle_port_obj_del); +/* Same as switchdev_handle_port_obj_del(), except if object is notified on a + * @dev that passes @foreign_dev_check_cb, it is replicated towards all devices + * that pass @check_cb and are in the same bridge as @dev. + */ +int switchdev_handle_port_obj_del_foreign(struct net_device *dev, + struct switchdev_notifier_port_obj_info *port_obj_info, + bool (*check_cb)(const struct net_device *dev), + bool (*foreign_dev_check_cb)(const struct net_device *dev, + const struct net_device *foreign_dev), + int (*del_cb)(struct net_device *dev, const void *ctx, + const struct switchdev_obj *obj)) +{ + int err; + + err = __switchdev_handle_port_obj_del(dev, port_obj_info, check_cb, + foreign_dev_check_cb, del_cb); + if (err == -EOPNOTSUPP) + err = 0; + return err; +} +EXPORT_SYMBOL_GPL(switchdev_handle_port_obj_del_foreign); + static int __switchdev_handle_port_attr_set(struct net_device *dev, struct switchdev_notifier_port_attr_info *port_attr_info, bool (*check_cb)(const struct net_device *dev), -- cgit From acd8df5880d7c80b0317dce8df3e65b6a6825c88 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Mon, 21 Feb 2022 14:01:30 +0200 Subject: net: switchdev: avoid infinite recursion from LAG to bridge with port object handler The logic from switchdev_handle_port_obj_add_foreign() is directly adapted from switchdev_handle_fdb_event_to_device(), which already detects events on foreign interfaces and reoffloads them towards the switchdev neighbors. However, when we have a simple br0 <-> bond0 <-> swp0 topology and the switchdev_handle_port_obj_add_foreign() gets called on bond0, we get stuck into an infinite recursion: 1. bond0 does not pass check_cb(), so we attempt to find switchdev neighbor interfaces. For that, we recursively call __switchdev_handle_port_obj_add() for bond0's bridge, br0. 2. __switchdev_handle_port_obj_add() recurses through br0's lowers, essentially calling __switchdev_handle_port_obj_add() for bond0 3. Go to step 1. This happens because switchdev_handle_fdb_event_to_device() and switchdev_handle_port_obj_add_foreign() are not exactly the same. The FDB event helper special-cases LAG interfaces with its lag_mod_cb(), so this is why we don't end up in an infinite loop - because it doesn't attempt to treat LAG interfaces as potentially foreign bridge ports. The problem is solved by looking ahead through the bridge's lowers to see whether there is any switchdev interface that is foreign to the @dev we are currently processing. This stops the recursion described above at step 1: __switchdev_handle_port_obj_add(bond0) will not create another call to __switchdev_handle_port_obj_add(br0). Going one step upper should only happen when we're starting from a bridge port that has been determined to be "foreign" to the switchdev driver that passes the foreign_dev_check_cb(). Fixes: c4076cdd21f8 ("net: switchdev: introduce switchdev_handle_port_obj_{add,del} for foreign interfaces") Signed-off-by: Vladimir Oltean Signed-off-by: David S. Miller --- net/switchdev/switchdev.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) (limited to 'net/switchdev/switchdev.c') diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c index 6a00c390547b..28d2ccfe109c 100644 --- a/net/switchdev/switchdev.c +++ b/net/switchdev/switchdev.c @@ -564,7 +564,7 @@ static int __switchdev_handle_port_obj_add(struct net_device *dev, struct netlink_ext_ack *extack)) { struct switchdev_notifier_info *info = &port_obj_info->info; - struct net_device *br, *lower_dev; + struct net_device *br, *lower_dev, *switchdev; struct netlink_ext_ack *extack; struct list_head *iter; int err = -EOPNOTSUPP; @@ -614,7 +614,11 @@ static int __switchdev_handle_port_obj_add(struct net_device *dev, if (!br || !netif_is_bridge_master(br)) return err; - if (!switchdev_lower_dev_find(br, check_cb, foreign_dev_check_cb)) + switchdev = switchdev_lower_dev_find(br, check_cb, foreign_dev_check_cb); + if (!switchdev) + return err; + + if (!foreign_dev_check_cb(switchdev, dev)) return err; return __switchdev_handle_port_obj_add(br, port_obj_info, check_cb, @@ -674,7 +678,7 @@ static int __switchdev_handle_port_obj_del(struct net_device *dev, const struct switchdev_obj *obj)) { struct switchdev_notifier_info *info = &port_obj_info->info; - struct net_device *br, *lower_dev; + struct net_device *br, *lower_dev, *switchdev; struct list_head *iter; int err = -EOPNOTSUPP; @@ -721,7 +725,11 @@ static int __switchdev_handle_port_obj_del(struct net_device *dev, if (!br || !netif_is_bridge_master(br)) return err; - if (!switchdev_lower_dev_find(br, check_cb, foreign_dev_check_cb)) + switchdev = switchdev_lower_dev_find(br, check_cb, foreign_dev_check_cb); + if (!switchdev) + return err; + + if (!foreign_dev_check_cb(switchdev, dev)) return err; return __switchdev_handle_port_obj_del(br, port_obj_info, check_cb, -- cgit From ec638740fce990ad2b9af43ead8088d6d6eb2145 Mon Sep 17 00:00:00 2001 From: Vladimir Oltean Date: Wed, 23 Feb 2022 16:00:50 +0200 Subject: net: switchdev: remove lag_mod_cb from switchdev_handle_fdb_event_to_device When the switchdev_handle_fdb_event_to_device() event replication helper was created, my original thought was that FDB events on LAG interfaces should most likely be special-cased, not just replicated towards all switchdev ports beneath that LAG. So this replication helper currently does not recurse through switchdev lower interfaces of LAG bridge ports, but rather calls the lag_mod_cb() if that was provided. No switchdev driver uses this helper for FDB events on LAG interfaces yet, so that was an assumption which was yet to be tested. It is certainly usable for that purpose, as my RFC series shows: https://patchwork.kernel.org/project/netdevbpf/cover/20220210125201.2859463-1-vladimir.oltean@nxp.com/ however this approach is slightly convoluted because: - the switchdev driver gets a "dev" that isn't its own net device, but rather the LAG net device. It must call switchdev_lower_dev_find(dev) in order to get a handle of any of its own net devices (the ones that pass check_cb). - in order for FDB entries on LAG ports to be correctly refcounted per the number of switchdev ports beneath that LAG, we haven't escaped the need to iterate through the LAG's lower interfaces. Except that is now the responsibility of the switchdev driver, because the replication helper just stopped half-way. So, even though yes, FDB events on LAG bridge ports must be special-cased, in the end it's simpler to let switchdev_handle_fdb_* just iterate through the LAG port's switchdev lowers, and let the switchdev driver figure out that those physical ports are under a LAG. The switchdev_handle_fdb_event_to_device() helper takes a "foreign_dev_check" callback so it can figure out whether @dev can autonomously forward to @foreign_dev. DSA fills this method properly: if the LAG is offloaded by another port in the same tree as @dev, then it isn't foreign. If it is a software LAG, it is foreign - forwarding happens in software. Whether an interface is foreign or not decides whether the replication helper will go through the LAG's switchdev lowers or not. Since the lan966x doesn't properly fill this out, FDB events on software LAG uppers will get called. By changing lan966x_foreign_dev_check(), we can suppress them. Whereas DSA will now start receiving FDB events for its offloaded LAG uppers, so we need to return -EOPNOTSUPP, since we currently don't do the right thing for them. Cc: Horatiu Vultur Signed-off-by: Vladimir Oltean Reviewed-by: Florian Fainelli Signed-off-by: Jakub Kicinski --- .../ethernet/microchip/lan966x/lan966x_switchdev.c | 12 ++-- include/net/switchdev.h | 10 +-- net/dsa/slave.c | 6 +- net/switchdev/switchdev.c | 80 ++++++++-------------- 4 files changed, 42 insertions(+), 66 deletions(-) (limited to 'net/switchdev/switchdev.c') diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c index 85099a51d4c7..e3555c94294d 100644 --- a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c @@ -419,6 +419,9 @@ static int lan966x_netdevice_event(struct notifier_block *nb, return notifier_from_errno(ret); } +/* We don't offload uppers such as LAG as bridge ports, so every device except + * the bridge itself is foreign. + */ static bool lan966x_foreign_dev_check(const struct net_device *dev, const struct net_device *foreign_dev) { @@ -426,10 +429,10 @@ static bool lan966x_foreign_dev_check(const struct net_device *dev, struct lan966x *lan966x = port->lan966x; if (netif_is_bridge_master(foreign_dev)) - if (lan966x->bridge != foreign_dev) - return true; + if (lan966x->bridge == foreign_dev) + return false; - return false; + return true; } static int lan966x_switchdev_event(struct notifier_block *nb, @@ -449,8 +452,7 @@ static int lan966x_switchdev_event(struct notifier_block *nb, err = switchdev_handle_fdb_event_to_device(dev, event, ptr, lan966x_netdevice_check, lan966x_foreign_dev_check, - lan966x_handle_fdb, - NULL); + lan966x_handle_fdb); return notifier_from_errno(err); } diff --git a/include/net/switchdev.h b/include/net/switchdev.h index c32e1c8f79ec..3e424d40fae3 100644 --- a/include/net/switchdev.h +++ b/include/net/switchdev.h @@ -313,10 +313,7 @@ int switchdev_handle_fdb_event_to_device(struct net_device *dev, unsigned long e const struct net_device *foreign_dev), int (*mod_cb)(struct net_device *dev, struct net_device *orig_dev, unsigned long event, const void *ctx, - const struct switchdev_notifier_fdb_info *fdb_info), - int (*lag_mod_cb)(struct net_device *dev, struct net_device *orig_dev, - unsigned long event, const void *ctx, - const struct switchdev_notifier_fdb_info *fdb_info)); + const struct switchdev_notifier_fdb_info *fdb_info)); int switchdev_handle_port_obj_add(struct net_device *dev, struct switchdev_notifier_port_obj_info *port_obj_info, @@ -443,10 +440,7 @@ switchdev_handle_fdb_event_to_device(struct net_device *dev, unsigned long event const struct net_device *foreign_dev), int (*mod_cb)(struct net_device *dev, struct net_device *orig_dev, unsigned long event, const void *ctx, - const struct switchdev_notifier_fdb_info *fdb_info), - int (*lag_mod_cb)(struct net_device *dev, struct net_device *orig_dev, - unsigned long event, const void *ctx, - const struct switchdev_notifier_fdb_info *fdb_info)) + const struct switchdev_notifier_fdb_info *fdb_info)) { return 0; } diff --git a/net/dsa/slave.c b/net/dsa/slave.c index e31c7710fee9..4ea6e0fd4b99 100644 --- a/net/dsa/slave.c +++ b/net/dsa/slave.c @@ -2461,6 +2461,9 @@ static int dsa_slave_fdb_event(struct net_device *dev, bool host_addr = fdb_info->is_local; struct dsa_switch *ds = dp->ds; + if (dp->lag) + return -EOPNOTSUPP; + if (ctx && ctx != dp) return 0; @@ -2526,8 +2529,7 @@ static int dsa_slave_switchdev_event(struct notifier_block *unused, err = switchdev_handle_fdb_event_to_device(dev, event, ptr, dsa_slave_dev_check, dsa_foreign_dev_check, - dsa_slave_fdb_event, - NULL); + dsa_slave_fdb_event); return notifier_from_errno(err); default: return NOTIFY_DONE; diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c index 28d2ccfe109c..474f76383033 100644 --- a/net/switchdev/switchdev.c +++ b/net/switchdev/switchdev.c @@ -458,63 +458,40 @@ static int __switchdev_handle_fdb_event_to_device(struct net_device *dev, const struct net_device *foreign_dev), int (*mod_cb)(struct net_device *dev, struct net_device *orig_dev, unsigned long event, const void *ctx, - const struct switchdev_notifier_fdb_info *fdb_info), - int (*lag_mod_cb)(struct net_device *dev, struct net_device *orig_dev, - unsigned long event, const void *ctx, - const struct switchdev_notifier_fdb_info *fdb_info)) + const struct switchdev_notifier_fdb_info *fdb_info)) { const struct switchdev_notifier_info *info = &fdb_info->info; - struct net_device *br, *lower_dev; + struct net_device *br, *lower_dev, *switchdev; struct list_head *iter; int err = -EOPNOTSUPP; if (check_cb(dev)) return mod_cb(dev, orig_dev, event, info->ctx, fdb_info); - if (netif_is_lag_master(dev)) { - if (!switchdev_lower_dev_find_rcu(dev, check_cb, foreign_dev_check_cb)) - goto maybe_bridged_with_us; - - /* This is a LAG interface that we offload */ - if (!lag_mod_cb) - return -EOPNOTSUPP; - - return lag_mod_cb(dev, orig_dev, event, info->ctx, fdb_info); - } - /* Recurse through lower interfaces in case the FDB entry is pointing - * towards a bridge device. + * towards a bridge or a LAG device. */ - if (netif_is_bridge_master(dev)) { - if (!switchdev_lower_dev_find_rcu(dev, check_cb, foreign_dev_check_cb)) - return 0; - - /* This is a bridge interface that we offload */ - netdev_for_each_lower_dev(dev, lower_dev, iter) { - /* Do not propagate FDB entries across bridges */ - if (netif_is_bridge_master(lower_dev)) - continue; - - /* Bridge ports might be either us, or LAG interfaces - * that we offload. - */ - if (!check_cb(lower_dev) && - !switchdev_lower_dev_find_rcu(lower_dev, check_cb, - foreign_dev_check_cb)) - continue; - - err = __switchdev_handle_fdb_event_to_device(lower_dev, orig_dev, - event, fdb_info, check_cb, - foreign_dev_check_cb, - mod_cb, lag_mod_cb); - if (err && err != -EOPNOTSUPP) - return err; - } + netdev_for_each_lower_dev(dev, lower_dev, iter) { + /* Do not propagate FDB entries across bridges */ + if (netif_is_bridge_master(lower_dev)) + continue; - return 0; + /* Bridge ports might be either us, or LAG interfaces + * that we offload. + */ + if (!check_cb(lower_dev) && + !switchdev_lower_dev_find_rcu(lower_dev, check_cb, + foreign_dev_check_cb)) + continue; + + err = __switchdev_handle_fdb_event_to_device(lower_dev, orig_dev, + event, fdb_info, check_cb, + foreign_dev_check_cb, + mod_cb); + if (err && err != -EOPNOTSUPP) + return err; } -maybe_bridged_with_us: /* Event is neither on a bridge nor a LAG. Check whether it is on an * interface that is in a bridge with us. */ @@ -522,12 +499,16 @@ maybe_bridged_with_us: if (!br || !netif_is_bridge_master(br)) return 0; - if (!switchdev_lower_dev_find_rcu(br, check_cb, foreign_dev_check_cb)) + switchdev = switchdev_lower_dev_find_rcu(br, check_cb, foreign_dev_check_cb); + if (!switchdev) return 0; + if (!foreign_dev_check_cb(switchdev, dev)) + return err; + return __switchdev_handle_fdb_event_to_device(br, orig_dev, event, fdb_info, check_cb, foreign_dev_check_cb, - mod_cb, lag_mod_cb); + mod_cb); } int switchdev_handle_fdb_event_to_device(struct net_device *dev, unsigned long event, @@ -537,16 +518,13 @@ int switchdev_handle_fdb_event_to_device(struct net_device *dev, unsigned long e const struct net_device *foreign_dev), int (*mod_cb)(struct net_device *dev, struct net_device *orig_dev, unsigned long event, const void *ctx, - const struct switchdev_notifier_fdb_info *fdb_info), - int (*lag_mod_cb)(struct net_device *dev, struct net_device *orig_dev, - unsigned long event, const void *ctx, - const struct switchdev_notifier_fdb_info *fdb_info)) + const struct switchdev_notifier_fdb_info *fdb_info)) { int err; err = __switchdev_handle_fdb_event_to_device(dev, dev, event, fdb_info, check_cb, foreign_dev_check_cb, - mod_cb, lag_mod_cb); + mod_cb); if (err == -EOPNOTSUPP) err = 0; -- cgit