diff options
Diffstat (limited to 'target/linux/generic/backport-5.4')
6 files changed, 794 insertions, 0 deletions
diff --git a/target/linux/generic/backport-5.4/770-v5.12-net-bridge-notify-switchdev-of-disappearance-of-old-.patch b/target/linux/generic/backport-5.4/770-v5.12-net-bridge-notify-switchdev-of-disappearance-of-old-.patch new file mode 100644 index 0000000000..df4e74cd96 --- /dev/null +++ b/target/linux/generic/backport-5.4/770-v5.12-net-bridge-notify-switchdev-of-disappearance-of-old-.patch @@ -0,0 +1,126 @@ +From 90dc8fd36078a536671adae884d0b929cce6480a Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean <vladimir.oltean@nxp.com> +Date: Wed, 6 Jan 2021 11:51:30 +0200 +Subject: [PATCH] net: bridge: notify switchdev of disappearance of old FDB + entry upon migration + +Currently the bridge emits atomic switchdev notifications for +dynamically learnt FDB entries. Monitoring these notifications works +wonders for switchdev drivers that want to keep their hardware FDB in +sync with the bridge's FDB. + +For example station A wants to talk to station B in the diagram below, +and we are concerned with the behavior of the bridge on the DUT device: + + DUT + +-------------------------------------+ + | br0 | + | +------+ +------+ +------+ +------+ | + | | | | | | | | | | + | | swp0 | | swp1 | | swp2 | | eth0 | | + +-------------------------------------+ + | | | + Station A | | + | | + +--+------+--+ +--+------+--+ + | | | | | | | | + | | swp0 | | | | swp0 | | + Another | +------+ | | +------+ | Another + switch | br0 | | br0 | switch + | +------+ | | +------+ | + | | | | | | | | + | | swp1 | | | | swp1 | | + +--+------+--+ +--+------+--+ + | + Station B + +Interfaces swp0, swp1, swp2 are handled by a switchdev driver that has +the following property: frames injected from its control interface bypass +the internal address analyzer logic, and therefore, this hardware does +not learn from the source address of packets transmitted by the network +stack through it. So, since bridging between eth0 (where Station B is +attached) and swp0 (where Station A is attached) is done in software, +the switchdev hardware will never learn the source address of Station B. +So the traffic towards that destination will be treated as unknown, i.e. +flooded. + +This is where the bridge notifications come in handy. When br0 on the +DUT sees frames with Station B's MAC address on eth0, the switchdev +driver gets these notifications and can install a rule to send frames +towards Station B's address that are incoming from swp0, swp1, swp2, +only towards the control interface. This is all switchdev driver private +business, which the notification makes possible. + +All is fine until someone unplugs Station B's cable and moves it to the +other switch: + + DUT + +-------------------------------------+ + | br0 | + | +------+ +------+ +------+ +------+ | + | | | | | | | | | | + | | swp0 | | swp1 | | swp2 | | eth0 | | + +-------------------------------------+ + | | | + Station A | | + | | + +--+------+--+ +--+------+--+ + | | | | | | | | + | | swp0 | | | | swp0 | | + Another | +------+ | | +------+ | Another + switch | br0 | | br0 | switch + | +------+ | | +------+ | + | | | | | | | | + | | swp1 | | | | swp1 | | + +--+------+--+ +--+------+--+ + | + Station B + +Luckily for the use cases we care about, Station B is noisy enough that +the DUT hears it (on swp1 this time). swp1 receives the frames and +delivers them to the bridge, who enters the unlikely path in br_fdb_update +of updating an existing entry. It moves the entry in the software bridge +to swp1 and emits an addition notification towards that. + +As far as the switchdev driver is concerned, all that it needs to ensure +is that traffic between Station A and Station B is not forever broken. +If it does nothing, then the stale rule to send frames for Station B +towards the control interface remains in place. But Station B is no +longer reachable via the control interface, but via a port that can +offload the bridge port learning attribute. It's just that the port is +prevented from learning this address, since the rule overrides FDB +updates. So the rule needs to go. The question is via what mechanism. + +It sure would be possible for this switchdev driver to keep track of all +addresses which are sent to the control interface, and then also listen +for bridge notifier events on its own ports, searching for the ones that +have a MAC address which was previously sent to the control interface. +But this is cumbersome and inefficient. Instead, with one small change, +the bridge could notify of the address deletion from the old port, in a +symmetrical manner with how it did for the insertion. Then the switchdev +driver would not be required to monitor learn/forget events for its own +ports. It could just delete the rule towards the control interface upon +bridge entry migration. This would make hardware address learning be +possible again. Then it would take a few more packets until the hardware +and software FDB would be in sync again. + +Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> +Acked-by: Nikolay Aleksandrov <nikolay@nvidia.com> +Reviewed-by: Ido Schimmel <idosch@nvidia.com> +Reviewed-by: Andrew Lunn <andrew@lunn.ch> +Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> +Signed-off-by: Jakub Kicinski <kuba@kernel.org> +--- + net/bridge/br_fdb.c | 1 + + 1 file changed, 1 insertion(+) + +--- a/net/bridge/br_fdb.c ++++ b/net/bridge/br_fdb.c +@@ -581,6 +581,7 @@ void br_fdb_update(struct net_bridge *br + + /* fastpath: update of existing entry */ + if (unlikely(source != fdb->dst && !fdb->is_sticky)) { ++ br_switchdev_fdb_notify(fdb, RTM_DELNEIGH); + fdb->dst = source; + fdb_modified = true; + /* Take over HW learned entry */ diff --git a/target/linux/generic/backport-5.4/771-v5.12-net-dsa-be-louder-when-a-non-legacy-FDB-operation-fa.patch b/target/linux/generic/backport-5.4/771-v5.12-net-dsa-be-louder-when-a-non-legacy-FDB-operation-fa.patch new file mode 100644 index 0000000000..967502f022 --- /dev/null +++ b/target/linux/generic/backport-5.4/771-v5.12-net-dsa-be-louder-when-a-non-legacy-FDB-operation-fa.patch @@ -0,0 +1,52 @@ +From 2fd186501b1cff155cc4a755c210793cfc0dffb5 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean <vladimir.oltean@nxp.com> +Date: Wed, 6 Jan 2021 11:51:31 +0200 +Subject: [PATCH] net: dsa: be louder when a non-legacy FDB operation fails + +The dev_close() call was added in commit c9eb3e0f8701 ("net: dsa: Add +support for learning FDB through notification") "to indicate inconsistent +situation" when we could not delete an FDB entry from the port. + +bridge fdb del d8:58:d7:00:ca:6d dev swp0 self master + +It is a bit drastic and at the same time not helpful if the above fails +to only print with netdev_dbg log level, but on the other hand to bring +the interface down. + +So increase the verbosity of the error message, and drop dev_close(). + +Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> +Reviewed-by: Andrew Lunn <andrew@lunn.ch> +Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> +Signed-off-by: Jakub Kicinski <kuba@kernel.org> +--- + net/dsa/slave.c | 10 +++++++--- + 1 file changed, 7 insertions(+), 3 deletions(-) + +--- a/net/dsa/slave.c ++++ b/net/dsa/slave.c +@@ -1590,7 +1590,9 @@ static void dsa_slave_switchdev_event_wo + + err = dsa_port_fdb_add(dp, fdb_info->addr, fdb_info->vid); + if (err) { +- netdev_dbg(dev, "fdb add failed err=%d\n", err); ++ netdev_err(dev, ++ "failed to add %pM vid %d to fdb: %d\n", ++ fdb_info->addr, fdb_info->vid, err); + break; + } + fdb_info->offloaded = true; +@@ -1605,9 +1607,11 @@ static void dsa_slave_switchdev_event_wo + + err = dsa_port_fdb_del(dp, fdb_info->addr, fdb_info->vid); + if (err) { +- netdev_dbg(dev, "fdb del failed err=%d\n", err); +- dev_close(dev); ++ netdev_err(dev, ++ "failed to delete %pM vid %d from fdb: %d\n", ++ fdb_info->addr, fdb_info->vid, err); + } ++ + break; + } + rtnl_unlock(); diff --git a/target/linux/generic/backport-5.4/772-v5.12-net-dsa-don-t-use-switchdev_notifier_fdb_info-in-dsa.patch b/target/linux/generic/backport-5.4/772-v5.12-net-dsa-don-t-use-switchdev_notifier_fdb_info-in-dsa.patch new file mode 100644 index 0000000000..ad7c242280 --- /dev/null +++ b/target/linux/generic/backport-5.4/772-v5.12-net-dsa-don-t-use-switchdev_notifier_fdb_info-in-dsa.patch @@ -0,0 +1,226 @@ +From c4bb76a9a0ef87c4cc1f636defed5f12deb9f5a7 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean <vladimir.oltean@nxp.com> +Date: Wed, 6 Jan 2021 11:51:32 +0200 +Subject: [PATCH] net: dsa: don't use switchdev_notifier_fdb_info in + dsa_switchdev_event_work + +Currently DSA doesn't add FDB entries on the CPU port, because it only +does so through switchdev, which is associated with a net_device, and +there are none of those for the CPU port. + +But actually FDB addresses on the CPU port have some use cases of their +own, if the switchdev operations are initiated from within the DSA +layer. There is just one problem with the existing code: it passes a +structure in dsa_switchdev_event_work which was retrieved directly from +switchdev, so it contains a net_device. We need to generalize the +contents to something that covers the CPU port as well: the "ds, port" +tuple is fine for that. + +Note that the new procedure for notifying the successful FDB offload is +inspired from the rocker model. + +Also, nothing was being done if added_by_user was false. Let's check for +that a lot earlier, and don't actually bother to schedule the worker +for nothing. + +Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> +Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> +Signed-off-by: Jakub Kicinski <kuba@kernel.org> +--- + net/dsa/dsa_priv.h | 12 +++++ + net/dsa/slave.c | 106 ++++++++++++++++++++++----------------------- + 2 files changed, 65 insertions(+), 53 deletions(-) + +--- a/net/dsa/dsa_priv.h ++++ b/net/dsa/dsa_priv.h +@@ -62,6 +62,18 @@ struct dsa_notifier_vlan_info { + int port; + }; + ++struct dsa_switchdev_event_work { ++ struct dsa_switch *ds; ++ int port; ++ struct work_struct work; ++ unsigned long event; ++ /* Specific for SWITCHDEV_FDB_ADD_TO_DEVICE and ++ * SWITCHDEV_FDB_DEL_TO_DEVICE ++ */ ++ unsigned char addr[ETH_ALEN]; ++ u16 vid; ++}; ++ + struct dsa_slave_priv { + /* Copy of CPU port xmit for faster access in slave transmit hot path */ + struct sk_buff * (*xmit)(struct sk_buff *skb, +--- a/net/dsa/slave.c ++++ b/net/dsa/slave.c +@@ -1565,76 +1565,66 @@ static int dsa_slave_netdevice_event(str + return NOTIFY_DONE; + } + +-struct dsa_switchdev_event_work { +- struct work_struct work; +- struct switchdev_notifier_fdb_info fdb_info; +- struct net_device *dev; +- unsigned long event; +-}; ++static void ++dsa_fdb_offload_notify(struct dsa_switchdev_event_work *switchdev_work) ++{ ++ struct dsa_switch *ds = switchdev_work->ds; ++ struct switchdev_notifier_fdb_info info; ++ struct dsa_port *dp; ++ ++ if (!dsa_is_user_port(ds, switchdev_work->port)) ++ return; ++ ++ info.addr = switchdev_work->addr; ++ info.vid = switchdev_work->vid; ++ info.offloaded = true; ++ dp = dsa_to_port(ds, switchdev_work->port); ++ call_switchdev_notifiers(SWITCHDEV_FDB_OFFLOADED, ++ dp->slave, &info.info, NULL); ++} + + static void dsa_slave_switchdev_event_work(struct work_struct *work) + { + struct dsa_switchdev_event_work *switchdev_work = + container_of(work, struct dsa_switchdev_event_work, work); +- struct net_device *dev = switchdev_work->dev; +- struct switchdev_notifier_fdb_info *fdb_info; +- struct dsa_port *dp = dsa_slave_to_port(dev); ++ struct dsa_switch *ds = switchdev_work->ds; ++ struct dsa_port *dp; + int err; + ++ dp = dsa_to_port(ds, switchdev_work->port); ++ + rtnl_lock(); + switch (switchdev_work->event) { + case SWITCHDEV_FDB_ADD_TO_DEVICE: +- fdb_info = &switchdev_work->fdb_info; +- if (!fdb_info->added_by_user) +- break; +- +- err = dsa_port_fdb_add(dp, fdb_info->addr, fdb_info->vid); ++ err = dsa_port_fdb_add(dp, switchdev_work->addr, ++ switchdev_work->vid); + if (err) { +- netdev_err(dev, +- "failed to add %pM vid %d to fdb: %d\n", +- fdb_info->addr, fdb_info->vid, err); ++ dev_err(ds->dev, ++ "port %d failed to add %pM vid %d to fdb: %d\n", ++ dp->index, switchdev_work->addr, ++ switchdev_work->vid, err); + break; + } +- fdb_info->offloaded = true; +- call_switchdev_notifiers(SWITCHDEV_FDB_OFFLOADED, dev, +- &fdb_info->info, NULL); ++ dsa_fdb_offload_notify(switchdev_work); + break; + + case SWITCHDEV_FDB_DEL_TO_DEVICE: +- fdb_info = &switchdev_work->fdb_info; +- if (!fdb_info->added_by_user) +- break; +- +- err = dsa_port_fdb_del(dp, fdb_info->addr, fdb_info->vid); ++ err = dsa_port_fdb_del(dp, switchdev_work->addr, ++ switchdev_work->vid); + if (err) { +- netdev_err(dev, +- "failed to delete %pM vid %d from fdb: %d\n", +- fdb_info->addr, fdb_info->vid, err); ++ dev_err(ds->dev, ++ "port %d failed to delete %pM vid %d from fdb: %d\n", ++ dp->index, switchdev_work->addr, ++ switchdev_work->vid, err); + } + + break; + } + rtnl_unlock(); + +- kfree(switchdev_work->fdb_info.addr); + kfree(switchdev_work); +- dev_put(dev); +-} +- +-static int +-dsa_slave_switchdev_fdb_work_init(struct dsa_switchdev_event_work * +- switchdev_work, +- const struct switchdev_notifier_fdb_info * +- fdb_info) +-{ +- memcpy(&switchdev_work->fdb_info, fdb_info, +- sizeof(switchdev_work->fdb_info)); +- switchdev_work->fdb_info.addr = kzalloc(ETH_ALEN, GFP_ATOMIC); +- if (!switchdev_work->fdb_info.addr) +- return -ENOMEM; +- ether_addr_copy((u8 *)switchdev_work->fdb_info.addr, +- fdb_info->addr); +- return 0; ++ if (dsa_is_user_port(ds, dp->index)) ++ dev_put(dp->slave); + } + + /* Called under rcu_read_lock() */ +@@ -1642,7 +1632,9 @@ static int dsa_slave_switchdev_event(str + unsigned long event, void *ptr) + { + struct net_device *dev = switchdev_notifier_info_to_dev(ptr); ++ const struct switchdev_notifier_fdb_info *fdb_info; + struct dsa_switchdev_event_work *switchdev_work; ++ struct dsa_port *dp; + int err; + + if (event == SWITCHDEV_PORT_ATTR_SET) { +@@ -1655,20 +1647,32 @@ static int dsa_slave_switchdev_event(str + if (!dsa_slave_dev_check(dev)) + return NOTIFY_DONE; + ++ dp = dsa_slave_to_port(dev); ++ + switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC); + if (!switchdev_work) + return NOTIFY_BAD; + + INIT_WORK(&switchdev_work->work, + dsa_slave_switchdev_event_work); +- switchdev_work->dev = dev; ++ switchdev_work->ds = dp->ds; ++ switchdev_work->port = dp->index; + switchdev_work->event = event; + + switch (event) { + case SWITCHDEV_FDB_ADD_TO_DEVICE: /* fall through */ + case SWITCHDEV_FDB_DEL_TO_DEVICE: +- if (dsa_slave_switchdev_fdb_work_init(switchdev_work, ptr)) +- goto err_fdb_work_init; ++ fdb_info = ptr; ++ ++ if (!fdb_info->added_by_user) { ++ kfree(switchdev_work); ++ return NOTIFY_OK; ++ } ++ ++ ether_addr_copy(switchdev_work->addr, ++ fdb_info->addr); ++ switchdev_work->vid = fdb_info->vid; ++ + dev_hold(dev); + break; + default: +@@ -1678,10 +1682,6 @@ static int dsa_slave_switchdev_event(str + + dsa_schedule_work(&switchdev_work->work); + return NOTIFY_OK; +- +-err_fdb_work_init: +- kfree(switchdev_work); +- return NOTIFY_BAD; + } + + static int dsa_slave_switchdev_blocking_event(struct notifier_block *unused, diff --git a/target/linux/generic/backport-5.4/773-v5.12-net-dsa-move-switchdev-event-implementation-under-th.patch b/target/linux/generic/backport-5.4/773-v5.12-net-dsa-move-switchdev-event-implementation-under-th.patch new file mode 100644 index 0000000000..619c71c7bb --- /dev/null +++ b/target/linux/generic/backport-5.4/773-v5.12-net-dsa-move-switchdev-event-implementation-under-th.patch @@ -0,0 +1,85 @@ +From 447d290a58bd335d68f665713842365d3d6447df Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean <vladimir.oltean@nxp.com> +Date: Wed, 6 Jan 2021 11:51:33 +0200 +Subject: [PATCH] net: dsa: move switchdev event implementation under the same + switch/case statement + +We'll need to start listening to SWITCHDEV_FDB_{ADD,DEL}_TO_DEVICE +events even for interfaces where dsa_slave_dev_check returns false, so +we need that check inside the switch-case statement for SWITCHDEV_FDB_*. + +This movement also avoids a useless allocation / free of switchdev_work +on the untreated "default event" case. + +Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> +Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> +Signed-off-by: Jakub Kicinski <kuba@kernel.org> +--- + net/dsa/slave.c | 35 ++++++++++++++++------------------- + 1 file changed, 16 insertions(+), 19 deletions(-) + +--- a/net/dsa/slave.c ++++ b/net/dsa/slave.c +@@ -1637,31 +1637,29 @@ static int dsa_slave_switchdev_event(str + struct dsa_port *dp; + int err; + +- if (event == SWITCHDEV_PORT_ATTR_SET) { ++ switch (event) { ++ case SWITCHDEV_PORT_ATTR_SET: + err = switchdev_handle_port_attr_set(dev, ptr, + dsa_slave_dev_check, + dsa_slave_port_attr_set); + return notifier_from_errno(err); +- } +- +- if (!dsa_slave_dev_check(dev)) +- return NOTIFY_DONE; ++ case SWITCHDEV_FDB_ADD_TO_DEVICE: ++ case SWITCHDEV_FDB_DEL_TO_DEVICE: ++ if (!dsa_slave_dev_check(dev)) ++ return NOTIFY_DONE; + +- dp = dsa_slave_to_port(dev); ++ dp = dsa_slave_to_port(dev); + +- switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC); +- if (!switchdev_work) +- return NOTIFY_BAD; +- +- INIT_WORK(&switchdev_work->work, +- dsa_slave_switchdev_event_work); +- switchdev_work->ds = dp->ds; +- switchdev_work->port = dp->index; +- switchdev_work->event = event; ++ switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC); ++ if (!switchdev_work) ++ return NOTIFY_BAD; ++ ++ INIT_WORK(&switchdev_work->work, ++ dsa_slave_switchdev_event_work); ++ switchdev_work->ds = dp->ds; ++ switchdev_work->port = dp->index; ++ switchdev_work->event = event; + +- switch (event) { +- case SWITCHDEV_FDB_ADD_TO_DEVICE: /* fall through */ +- case SWITCHDEV_FDB_DEL_TO_DEVICE: + fdb_info = ptr; + + if (!fdb_info->added_by_user) { +@@ -1674,13 +1672,12 @@ static int dsa_slave_switchdev_event(str + switchdev_work->vid = fdb_info->vid; + + dev_hold(dev); ++ dsa_schedule_work(&switchdev_work->work); + break; + default: +- kfree(switchdev_work); + return NOTIFY_DONE; + } + +- dsa_schedule_work(&switchdev_work->work); + return NOTIFY_OK; + } + diff --git a/target/linux/generic/backport-5.4/774-v5.12-net-dsa-exit-early-in-dsa_slave_switchdev_event-if-w.patch b/target/linux/generic/backport-5.4/774-v5.12-net-dsa-exit-early-in-dsa_slave_switchdev_event-if-w.patch new file mode 100644 index 0000000000..25e0d30e1a --- /dev/null +++ b/target/linux/generic/backport-5.4/774-v5.12-net-dsa-exit-early-in-dsa_slave_switchdev_event-if-w.patch @@ -0,0 +1,42 @@ +From 5fb4a451a87d8ed3363d28b63a3295399373d6c4 Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean <vladimir.oltean@nxp.com> +Date: Wed, 6 Jan 2021 11:51:34 +0200 +Subject: [PATCH] net: dsa: exit early in dsa_slave_switchdev_event if we can't + program the FDB + +Right now, the following would happen for a switch driver that does not +implement .port_fdb_add or .port_fdb_del. + +dsa_slave_switchdev_event returns NOTIFY_OK and schedules: +-> dsa_slave_switchdev_event_work + -> dsa_port_fdb_add + -> dsa_port_notify(DSA_NOTIFIER_FDB_ADD) + -> dsa_switch_fdb_add + -> if (!ds->ops->port_fdb_add) return -EOPNOTSUPP; + -> an error is printed with dev_dbg, and + dsa_fdb_offload_notify(switchdev_work) is not called. + +We can avoid scheduling the worker for nothing and say NOTIFY_DONE. +Because we don't call dsa_fdb_offload_notify, the static FDB entry will +remain just in the software bridge. + +Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> +Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> +Reviewed-by: Andrew Lunn <andrew@lunn.ch> +Signed-off-by: Jakub Kicinski <kuba@kernel.org> +--- + net/dsa/slave.c | 3 +++ + 1 file changed, 3 insertions(+) + +--- a/net/dsa/slave.c ++++ b/net/dsa/slave.c +@@ -1650,6 +1650,9 @@ static int dsa_slave_switchdev_event(str + + dp = dsa_slave_to_port(dev); + ++ if (!dp->ds->ops->port_fdb_add || !dp->ds->ops->port_fdb_del) ++ return NOTIFY_DONE; ++ + switchdev_work = kzalloc(sizeof(*switchdev_work), GFP_ATOMIC); + if (!switchdev_work) + return NOTIFY_BAD; diff --git a/target/linux/generic/backport-5.4/775-v5.12-net-dsa-listen-for-SWITCHDEV_-FDB-DEL-_ADD_TO_DEVICE.patch b/target/linux/generic/backport-5.4/775-v5.12-net-dsa-listen-for-SWITCHDEV_-FDB-DEL-_ADD_TO_DEVICE.patch new file mode 100644 index 0000000000..ecc74ea050 --- /dev/null +++ b/target/linux/generic/backport-5.4/775-v5.12-net-dsa-listen-for-SWITCHDEV_-FDB-DEL-_ADD_TO_DEVICE.patch @@ -0,0 +1,263 @@ +From d5f19486cee79d04c054427577ac96ed123706db Mon Sep 17 00:00:00 2001 +From: Vladimir Oltean <vladimir.oltean@nxp.com> +Date: Wed, 6 Jan 2021 11:51:35 +0200 +Subject: [PATCH] net: dsa: listen for SWITCHDEV_{FDB,DEL}_ADD_TO_DEVICE on + foreign bridge neighbors + +Some DSA switches (and not only) cannot learn source MAC addresses from +packets injected from the CPU. They only perform hardware address +learning from inbound traffic. + +This can be problematic when we have a bridge spanning some DSA switch +ports and some non-DSA ports (which we'll call "foreign interfaces" from +DSA's perspective). + +There are 2 classes of problems created by the lack of learning on +CPU-injected traffic: +- excessive flooding, due to the fact that DSA treats those addresses as + unknown +- the risk of stale routes, which can lead to temporary packet loss + +To illustrate the second class, consider the following situation, which +is common in production equipment (wireless access points, where there +is a WLAN interface and an Ethernet switch, and these form a single +bridging domain). + + AP 1: + +------------------------------------------------------------------------+ + | br0 | + +------------------------------------------------------------------------+ + +------------+ +------------+ +------------+ +------------+ +------------+ + | swp0 | | swp1 | | swp2 | | swp3 | | wlan0 | + +------------+ +------------+ +------------+ +------------+ +------------+ + | ^ ^ + | | | + | | | + | Client A Client B + | + | + | + +------------+ +------------+ +------------+ +------------+ +------------+ + | swp0 | | swp1 | | swp2 | | swp3 | | wlan0 | + +------------+ +------------+ +------------+ +------------+ +------------+ + +------------------------------------------------------------------------+ + | br0 | + +------------------------------------------------------------------------+ + AP 2 + +- br0 of AP 1 will know that Clients A and B are reachable via wlan0 +- the hardware fdb of a DSA switch driver today is not kept in sync with + the software entries on other bridge ports, so it will not know that + clients A and B are reachable via the CPU port UNLESS the hardware + switch itself performs SA learning from traffic injected from the CPU. + Nonetheless, a substantial number of switches don't. +- the hardware fdb of the DSA switch on AP 2 may autonomously learn that + Client A and B are reachable through swp0. Therefore, the software br0 + of AP 2 also may or may not learn this. In the example we're + illustrating, some Ethernet traffic has been going on, and br0 from AP + 2 has indeed learnt that it can reach Client B through swp0. + +One of the wireless clients, say Client B, disconnects from AP 1 and +roams to AP 2. The topology now looks like this: + + AP 1: + +------------------------------------------------------------------------+ + | br0 | + +------------------------------------------------------------------------+ + +------------+ +------------+ +------------+ +------------+ +------------+ + | swp0 | | swp1 | | swp2 | | swp3 | | wlan0 | + +------------+ +------------+ +------------+ +------------+ +------------+ + | ^ + | | + | Client A + | + | + | Client B + | | + | v + +------------+ +------------+ +------------+ +------------+ +------------+ + | swp0 | | swp1 | | swp2 | | swp3 | | wlan0 | + +------------+ +------------+ +------------+ +------------+ +------------+ + +------------------------------------------------------------------------+ + | br0 | + +------------------------------------------------------------------------+ + AP 2 + +- br0 of AP 1 still knows that Client A is reachable via wlan0 (no change) +- br0 of AP 1 will (possibly) know that Client B has left wlan0. There + are cases where it might never find out though. Either way, DSA today + does not process that notification in any way. +- the hardware FDB of the DSA switch on AP 1 may learn autonomously that + Client B can be reached via swp0, if it receives any packet with + Client 1's source MAC address over Ethernet. +- the hardware FDB of the DSA switch on AP 2 still thinks that Client B + can be reached via swp0. It does not know that it has roamed to wlan0, + because it doesn't perform SA learning from the CPU port. + +Now Client A contacts Client B. +AP 1 routes the packet fine towards swp0 and delivers it on the Ethernet +segment. +AP 2 sees a frame on swp0 and its fdb says that the destination is swp0. +Hairpinning is disabled => drop. + +This problem comes from the fact that these switches have a 'blind spot' +for addresses coming from software bridging. The generic solution is not +to assume that hardware learning can be enabled somehow, but to listen +to more bridge learning events. It turns out that the bridge driver does +learn in software from all inbound frames, in __br_handle_local_finish. +A proper SWITCHDEV_FDB_ADD_TO_DEVICE notification is emitted for the +addresses serviced by the bridge on 'foreign' interfaces. The software +bridge also does the right thing on migration, by notifying that the old +entry is deleted, so that does not need to be special-cased in DSA. When +it is deleted, we just need to delete our static FDB entry towards the +CPU too, and wait. + +The problem is that DSA currently only cares about SWITCHDEV_FDB_ADD_TO_DEVICE +events received on its own interfaces, such as static FDB entries. + +Luckily we can change that, and DSA can listen to all switchdev FDB +add/del events in the system and figure out if those events were emitted +by a bridge that spans at least one of DSA's own ports. In case that is +true, DSA will also offload that address towards its own CPU port, in +the eventuality that there might be bridge clients attached to the DSA +switch who want to talk to the station connected to the foreign +interface. + +In terms of implementation, we need to keep the fdb_info->added_by_user +check for the case where the switchdev event was targeted directly at a +DSA switch port. But we don't need to look at that flag for snooped +events. So the check is currently too late, we need to move it earlier. +This also simplifies the code a bit, since we avoid uselessly allocating +and freeing switchdev_work. + +We could probably do some improvements in the future. For example, +multi-bridge support is rudimentary at the moment. If there are two +bridges spanning a DSA switch's ports, and both of them need to service +the same MAC address, then what will happen is that the migration of one +of those stations will trigger the deletion of the FDB entry from the +CPU port while it is still used by other bridge. That could be improved +with reference counting but is left for another time. + +This behavior needs to be enabled at driver level by setting +ds->assisted_learning_on_cpu_port = true. This is because we don't want +to inflict a potential performance penalty (accesses through +MDIO/I2C/SPI are expensive) to hardware that really doesn't need it +because address learning on the CPU port works there. + +Reported-by: DENG Qingfang <dqfext@gmail.com> +Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com> +Reviewed-by: Florian Fainelli <f.fainelli@gmail.com> +Reviewed-by: Andrew Lunn <andrew@lunn.ch> +Signed-off-by: Jakub Kicinski <kuba@kernel.org> +[Backported to linux-5.4.y] +Signed-off-by: DENG Qingfang <dqfext@gmail.com> +--- + include/net/dsa.h | 5 ++++ + net/dsa/slave.c | 63 ++++++++++++++++++++++++++++++++++++++--------- + 2 files changed, 57 insertions(+), 11 deletions(-) + +--- a/include/net/dsa.h ++++ b/include/net/dsa.h +@@ -279,6 +279,11 @@ struct dsa_switch { + */ + bool configure_vlan_while_not_filtering; + ++ /* Let DSA manage the FDB entries towards the CPU, based on the ++ * software bridge database. ++ */ ++ bool assisted_learning_on_cpu_port; ++ + /* In case vlan_filtering_is_global is set, the VLAN awareness state + * should be retrieved from here and not from the per-port settings. + */ +--- a/net/dsa/slave.c ++++ b/net/dsa/slave.c +@@ -1627,6 +1627,25 @@ static void dsa_slave_switchdev_event_wo + dev_put(dp->slave); + } + ++static int dsa_lower_dev_walk(struct net_device *lower_dev, void *data) ++{ ++ if (dsa_slave_dev_check(lower_dev)) { ++ *((void **)data) = (void *)netdev_priv(lower_dev); ++ return 1; ++ } ++ ++ return 0; ++} ++ ++static struct dsa_slave_priv *dsa_slave_dev_lower_find(struct net_device *dev) ++{ ++ struct dsa_slave_priv *data = NULL; ++ ++ netdev_walk_all_lower_dev_rcu(dev, dsa_lower_dev_walk, (void **) &data); ++ ++ return data; ++} ++ + /* Called under rcu_read_lock() */ + static int dsa_slave_switchdev_event(struct notifier_block *unused, + unsigned long event, void *ptr) +@@ -1645,10 +1664,37 @@ static int dsa_slave_switchdev_event(str + return notifier_from_errno(err); + case SWITCHDEV_FDB_ADD_TO_DEVICE: + case SWITCHDEV_FDB_DEL_TO_DEVICE: +- if (!dsa_slave_dev_check(dev)) +- return NOTIFY_DONE; ++ fdb_info = ptr; ++ ++ if (dsa_slave_dev_check(dev)) { ++ if (!fdb_info->added_by_user) ++ return NOTIFY_OK; ++ ++ dp = dsa_slave_to_port(dev); ++ } else { ++ /* Snoop addresses learnt on foreign interfaces ++ * bridged with us, for switches that don't ++ * automatically learn SA from CPU-injected traffic ++ */ ++ struct net_device *br_dev; ++ struct dsa_slave_priv *p; ++ ++ br_dev = netdev_master_upper_dev_get_rcu(dev); ++ if (!br_dev) ++ return NOTIFY_DONE; ++ ++ if (!netif_is_bridge_master(br_dev)) ++ return NOTIFY_DONE; ++ ++ p = dsa_slave_dev_lower_find(br_dev); ++ if (!p) ++ return NOTIFY_DONE; + +- dp = dsa_slave_to_port(dev); ++ dp = p->dp->cpu_dp; ++ ++ if (!dp->ds->assisted_learning_on_cpu_port) ++ return NOTIFY_DONE; ++ } + + if (!dp->ds->ops->port_fdb_add || !dp->ds->ops->port_fdb_del) + return NOTIFY_DONE; +@@ -1663,18 +1709,13 @@ static int dsa_slave_switchdev_event(str + switchdev_work->port = dp->index; + switchdev_work->event = event; + +- fdb_info = ptr; +- +- if (!fdb_info->added_by_user) { +- kfree(switchdev_work); +- return NOTIFY_OK; +- } +- + ether_addr_copy(switchdev_work->addr, + fdb_info->addr); + switchdev_work->vid = fdb_info->vid; + +- dev_hold(dev); ++ /* Hold a reference on the slave for dsa_fdb_offload_notify */ ++ if (dsa_is_user_port(dp->ds, dp->index)) ++ dev_hold(dev); + dsa_schedule_work(&switchdev_work->work); + break; + default: |