2020-12-13 18:49:00

by Vladimir Oltean

[permalink] [raw]
Subject: [PATCH v3 net-next 1/7] 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 <[email protected]>
Acked-by: Nikolay Aleksandrov <[email protected]>
---
Changes in v3:
None.

Changes in v2:
Patch is new.

net/bridge/br_fdb.c | 1 +
1 file changed, 1 insertion(+)

diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
index 32ac8343b0ba..b7490237f3fc 100644
--- a/net/bridge/br_fdb.c
+++ b/net/bridge/br_fdb.c
@@ -602,6 +602,7 @@ void br_fdb_update(struct net_bridge *br, struct net_bridge_port *source,
/* fastpath: update of existing entry */
if (unlikely(source != fdb->dst &&
!test_bit(BR_FDB_STICKY, &fdb->flags))) {
+ br_switchdev_fdb_notify(fdb, RTM_DELNEIGH);
fdb->dst = source;
fdb_modified = true;
/* Take over HW learned entry */
--
2.25.1


2020-12-13 18:53:38

by Ido Schimmel

[permalink] [raw]
Subject: Re: [PATCH v3 net-next 1/7] net: bridge: notify switchdev of disappearance of old FDB entry upon migration

On Sun, Dec 13, 2020 at 04:07:04PM +0200, Vladimir Oltean wrote:
> 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.

Can you please clarify why the FDB replacement notification is not
enough? Is it because the hardware you are working with manages MACs to
CPU in a separate table from its FDB table? I assume that's why you
refer to it as a "rule" instead of FDB entry? How common is this with
DSA switches?

Asking because it is not clear to me from the commit message. The patch
looks fine.

>
> 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 <[email protected]>
> Acked-by: Nikolay Aleksandrov <[email protected]>

Reviewed-by: Ido Schimmel <[email protected]>

2020-12-13 18:53:57

by Vladimir Oltean

[permalink] [raw]
Subject: Re: [PATCH v3 net-next 1/7] net: bridge: notify switchdev of disappearance of old FDB entry upon migration

Hi Ido,

On Sun, Dec 13, 2020 at 04:55:43PM +0200, Ido Schimmel wrote:
> > 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.
>
> Can you please clarify why the FDB replacement notification is not
> enough?

I didn't say it is not enough. I said it is a whole lot harder to track
from the listener side.

> Is it because the hardware you are working with manages MACs to
> CPU in a separate table from its FDB table? I assume that's why you
> refer to it as a "rule" instead of FDB entry? How common is this with
> DSA switches?

With DSA switches it's just more generic to use a static FDB entry as
the address trapping rule. But since FDB entries are global across the
switch and not really per source port, understandably other mechanisms
such as an ACL entry could be used just as well. And an ACL is what
other drivers (like drivers/staging/fsl-dpaa2/ethsw/) would use for this
purpose (of course, the code is not there yet; it's still in staging,
there are other issues to resolve first).

The mechanism does really not matter though, as long as it's "strong"
and not "weak" (i.e. the entry cannot be overridden by hardware address
learning on the front panel ports). So when the bridge gets any clue
that the L2 routing information is no longer up to date, the very least
we must do is we must delete this trapping rule to give the hardware a
chance to learn again. Where the address is migrated to is really not as
important as the fact that it migrated in the first place. [ ok, then
there's the case where it migrates from a foreign interface to another
foreign interface. For that scenario, we would delete the trapping rule
and then reinstall it, which is not ideal but also not incorrect. ]

> Reviewed-by: Ido Schimmel <[email protected]>

Thanks.