summaryrefslogtreecommitdiff
path: root/net/switchdev
diff options
context:
space:
mode:
authorVladimir Oltean <vladimir.oltean@nxp.com>2021-07-22 02:05:55 +0300
committerDavid S. Miller <davem@davemloft.net>2021-07-22 10:45:40 +0300
commit2b0a5688493ab0f54774a2f89d3d91ab238f7ab4 (patch)
treed17dce1e811a075eeb8e14a6c51573458fb2aee2 /net/switchdev
parentf796fcd613a97f39cea2629ac39cc040158812d0 (diff)
downloadlinux-2b0a5688493ab0f54774a2f89d3d91ab238f7ab4.tar.xz
net: switchdev: fix FDB entries towards foreign ports not getting propagated to us
The newly introduced switchdev_handle_fdb_{add,del}_to_device helpers solved a problem but introduced another one. They have a severe design bug: they do not propagate FDB events on foreign interfaces to us, i.e. this use case: br0 / \ / \ / \ / \ swp0 eno0 (switchdev) (foreign) when an address is learned on eno0, what is supposed to happen is that this event should also be propagated towards swp0. Somehow I managed to convince myself that this did work correctly, but obviously it does not. The trouble with foreign interfaces is that we must reach a switchdev net_device pointer through a foreign net_device that has no direct upper/lower relationship with it. So we need to do exploratory searching through the lower interfaces of the foreign net_device's bridge upper (to reach swp0 from eno0, we must check its upper, br0, for lower interfaces that pass the check_cb and foreign_dev_check_cb). This is something that the previous code did not do, it just assumed that "dev" will become a switchdev interface at some point, somehow, probably by magic. With this patch, assisted address learning on the CPU port works again in DSA: ip link add br0 type bridge ip link set swp0 master br0 ip link set eno0 master br0 ip link set br0 up [ 46.708929] mscc_felix 0000:00:00.5 swp0: Adding FDB entry towards eno0, addr 00:04:9f:05:f4:ab vid 0 as host address Fixes: 8ca07176ab00 ("net: switchdev: introduce a fanout helper for SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE") Reported-by: Eric Woudstra <ericwouds@gmail.com> Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net/switchdev')
-rw-r--r--net/switchdev/switchdev.c214
1 files changed, 142 insertions, 72 deletions
diff --git a/net/switchdev/switchdev.c b/net/switchdev/switchdev.c
index 42e88d3d66a7..0ae3478561f4 100644
--- a/net/switchdev/switchdev.c
+++ b/net/switchdev/switchdev.c
@@ -378,6 +378,56 @@ int call_switchdev_blocking_notifiers(unsigned long val, struct net_device *dev,
}
EXPORT_SYMBOL_GPL(call_switchdev_blocking_notifiers);
+struct switchdev_nested_priv {
+ bool (*check_cb)(const struct net_device *dev);
+ bool (*foreign_dev_check_cb)(const struct net_device *dev,
+ const struct net_device *foreign_dev);
+ const struct net_device *dev;
+ struct net_device *lower_dev;
+};
+
+static int switchdev_lower_dev_walk(struct net_device *lower_dev,
+ struct netdev_nested_priv *priv)
+{
+ struct switchdev_nested_priv *switchdev_priv = priv->data;
+ bool (*foreign_dev_check_cb)(const struct net_device *dev,
+ const struct net_device *foreign_dev);
+ bool (*check_cb)(const struct net_device *dev);
+ const struct net_device *dev;
+
+ check_cb = switchdev_priv->check_cb;
+ foreign_dev_check_cb = switchdev_priv->foreign_dev_check_cb;
+ dev = switchdev_priv->dev;
+
+ if (check_cb(lower_dev) && !foreign_dev_check_cb(lower_dev, dev)) {
+ switchdev_priv->lower_dev = lower_dev;
+ return 1;
+ }
+
+ return 0;
+}
+
+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_rcu(dev, switchdev_lower_dev_walk, &priv);
+
+ return switchdev_priv.lower_dev;
+}
+
static int __switchdev_handle_fdb_add_to_device(struct net_device *dev,
const struct net_device *orig_dev,
const struct switchdev_notifier_fdb_info *fdb_info,
@@ -392,37 +442,18 @@ static int __switchdev_handle_fdb_add_to_device(struct net_device *dev,
const struct switchdev_notifier_fdb_info *fdb_info))
{
const struct switchdev_notifier_info *info = &fdb_info->info;
- struct net_device *lower_dev;
+ struct net_device *br, *lower_dev;
struct list_head *iter;
int err = -EOPNOTSUPP;
- if (check_cb(dev)) {
- /* Handle FDB entries on foreign interfaces as FDB entries
- * towards the software bridge.
- */
- if (foreign_dev_check_cb && foreign_dev_check_cb(dev, orig_dev)) {
- struct net_device *br = netdev_master_upper_dev_get_rcu(dev);
-
- if (!br || !netif_is_bridge_master(br))
- return 0;
-
- /* No point in handling FDB entries on a foreign bridge */
- if (foreign_dev_check_cb(dev, br))
- return 0;
-
- return __switchdev_handle_fdb_add_to_device(br, orig_dev,
- fdb_info, check_cb,
- foreign_dev_check_cb,
- add_cb, lag_add_cb);
- }
-
+ if (check_cb(dev))
return add_cb(dev, orig_dev, info->ctx, fdb_info);
- }
- /* If we passed over the foreign check, it means that the LAG interface
- * is offloaded.
- */
if (netif_is_lag_master(dev)) {
+ if (!switchdev_lower_dev_find(dev, check_cb, foreign_dev_check_cb))
+ goto maybe_bridged_with_us;
+
+ /* This is a LAG interface that we offload */
if (!lag_add_cb)
return -EOPNOTSUPP;
@@ -432,20 +463,49 @@ static int __switchdev_handle_fdb_add_to_device(struct net_device *dev,
/* Recurse through lower interfaces in case the FDB entry is pointing
* towards a bridge device.
*/
- netdev_for_each_lower_dev(dev, lower_dev, iter) {
- /* Do not propagate FDB entries across bridges */
- if (netif_is_bridge_master(lower_dev))
- continue;
+ if (netif_is_bridge_master(dev)) {
+ if (!switchdev_lower_dev_find(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(lower_dev, check_cb,
+ foreign_dev_check_cb))
+ continue;
+
+ err = __switchdev_handle_fdb_add_to_device(lower_dev, orig_dev,
+ fdb_info, check_cb,
+ foreign_dev_check_cb,
+ add_cb, lag_add_cb);
+ if (err && err != -EOPNOTSUPP)
+ return err;
+ }
- err = __switchdev_handle_fdb_add_to_device(lower_dev, orig_dev,
- fdb_info, check_cb,
- foreign_dev_check_cb,
- add_cb, lag_add_cb);
- if (err && err != -EOPNOTSUPP)
- return err;
+ return 0;
}
- 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.
+ */
+ br = netdev_master_upper_dev_get_rcu(dev);
+ if (!br || !netif_is_bridge_master(br))
+ return 0;
+
+ if (!switchdev_lower_dev_find(br, check_cb, foreign_dev_check_cb))
+ return 0;
+
+ return __switchdev_handle_fdb_add_to_device(br, orig_dev, fdb_info,
+ check_cb, foreign_dev_check_cb,
+ add_cb, lag_add_cb);
}
int switchdev_handle_fdb_add_to_device(struct net_device *dev,
@@ -487,37 +547,18 @@ static int __switchdev_handle_fdb_del_to_device(struct net_device *dev,
const struct switchdev_notifier_fdb_info *fdb_info))
{
const struct switchdev_notifier_info *info = &fdb_info->info;
- struct net_device *lower_dev;
+ struct net_device *br, *lower_dev;
struct list_head *iter;
int err = -EOPNOTSUPP;
- if (check_cb(dev)) {
- /* Handle FDB entries on foreign interfaces as FDB entries
- * towards the software bridge.
- */
- if (foreign_dev_check_cb && foreign_dev_check_cb(dev, orig_dev)) {
- struct net_device *br = netdev_master_upper_dev_get_rcu(dev);
-
- if (!br || !netif_is_bridge_master(br))
- return 0;
-
- /* No point in handling FDB entries on a foreign bridge */
- if (foreign_dev_check_cb(dev, br))
- return 0;
-
- return __switchdev_handle_fdb_del_to_device(br, orig_dev,
- fdb_info, check_cb,
- foreign_dev_check_cb,
- del_cb, lag_del_cb);
- }
-
+ if (check_cb(dev))
return del_cb(dev, orig_dev, info->ctx, fdb_info);
- }
- /* If we passed over the foreign check, it means that the LAG interface
- * is offloaded.
- */
if (netif_is_lag_master(dev)) {
+ if (!switchdev_lower_dev_find(dev, check_cb, foreign_dev_check_cb))
+ goto maybe_bridged_with_us;
+
+ /* This is a LAG interface that we offload */
if (!lag_del_cb)
return -EOPNOTSUPP;
@@ -527,20 +568,49 @@ static int __switchdev_handle_fdb_del_to_device(struct net_device *dev,
/* Recurse through lower interfaces in case the FDB entry is pointing
* towards a bridge device.
*/
- netdev_for_each_lower_dev(dev, lower_dev, iter) {
- /* Do not propagate FDB entries across bridges */
- if (netif_is_bridge_master(lower_dev))
- continue;
+ if (netif_is_bridge_master(dev)) {
+ if (!switchdev_lower_dev_find(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(lower_dev, check_cb,
+ foreign_dev_check_cb))
+ continue;
+
+ err = __switchdev_handle_fdb_del_to_device(lower_dev, orig_dev,
+ fdb_info, check_cb,
+ foreign_dev_check_cb,
+ del_cb, lag_del_cb);
+ if (err && err != -EOPNOTSUPP)
+ return err;
+ }
- err = __switchdev_handle_fdb_del_to_device(lower_dev, orig_dev,
- fdb_info, check_cb,
- foreign_dev_check_cb,
- del_cb, lag_del_cb);
- if (err && err != -EOPNOTSUPP)
- return err;
+ return 0;
}
- 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.
+ */
+ br = netdev_master_upper_dev_get_rcu(dev);
+ if (!br || !netif_is_bridge_master(br))
+ return 0;
+
+ if (!switchdev_lower_dev_find(br, check_cb, foreign_dev_check_cb))
+ return 0;
+
+ return __switchdev_handle_fdb_del_to_device(br, orig_dev, fdb_info,
+ check_cb, foreign_dev_check_cb,
+ del_cb, lag_del_cb);
}
int switchdev_handle_fdb_del_to_device(struct net_device *dev,