Re: [PATCH net 0/6] bridge: Fix snooping in multi-bridge config with switchdev
From: Nikolay Aleksandrov <hidden>
Date: 2021-05-04 20:05:36
Also in:
bridge, lkml
On 04/05/2021 21:22, Joseph Huang wrote:
This series of patches contains the following fixes:
1. In a distributed system with multiple hardware-offloading bridges,
if a multicast source is attached to a Non-Querier bridge, the bridge
will not forward any multicast packets from that source to the Querier.
+--------------------+
| |
| Snooping | +------------+
| Bridge 1 |----| Listener 1 |
| (Querier) | +------------+
| |
+--------------------+
|
|
+----+---------+-----+
| | mrouter | |
+-----------+ | +---------+ | +------------+
| MC Source |----| Snooping |----| Listener 2 |
+-----------| | Bridge 2 | +------------+
| (Non-Querier) |
+--------------------+
In this scenario, Listener 1 will never receive multicast traffic
from MC Source since Snooping Bridge 2 does not forward multicast
packets to the mrouter port. Patches 0001, 0002, and 0003 address
this issue.
2. If mcast_flood is disabled on a bridge port, some of the snooping
functions stop working properly.
a. Consider the following scenario:
+--------------------+
| |
| Snooping | +------------+
| Bridge 1 |----| Listener 1 |
| (Querier) | +------------+
| |
+--------------------+
|
|
+--------------------+
| | mrouter | |
+-----------+ | +---------+ |
| MC Source |----| Snooping |
+-----------| | Bridge 2 |
| (Non-Querier) |
+--------------------+
In this scenario, Listener 1 will never receive multicast traffic
from MC Source if mcast_flood is disabled on the mrouter port on
Snooping Bridge 2. Patch 0004 addresses this issue.
b. For a Non-Querier bridge, if mcast_flood is disabled on a bridge
port, Queries received from other Querier will not be forwarded
out of that bridge port. Patch 0005 addresses this issue.
3. After a system boots up, the first couple Reports are not handled
properly:
1) the Report from the Host is being flooded (via br_flood) to all
bridge ports, and
2) if the mrouter port's mcast_flood is disabled, the Reports received
from other hosts will not be forwarded to the Querier.
Patch 0006 addresses this issue.
These patches were developed and verified initially against 5.4 kernel
(due to hardware platform limitation) and forward-patched to 5.12.
Snooping code introduced between 5.4 and 5.12 are not extensively tested
(only IGMPv2/MLDv1 were tested). The hardware platform used were two
bridges utilizing a single Marvell 88E6352 Ethernet switch chip (i.e.,
no cross-chip bridging involved).
Joseph Huang (6):
bridge: Refactor br_mdb_notify
bridge: Offload mrouter port forwarding to switchdev
bridge: Avoid traffic disruption when Querier state changes
bridge: Force mcast_flooding for mrouter ports
bridge: Flood Queries even when mcast_flood is disabled
bridge: Always multicast_flood Reports
net/bridge/br_device.c | 5 +-
net/bridge/br_forward.c | 3 +-
net/bridge/br_input.c | 5 +-
net/bridge/br_mdb.c | 70 +++++++++++++---------
net/bridge/br_multicast.c | 121 ++++++++++++++++++++++++++++++++++----
net/bridge/br_private.h | 11 +++-
6 files changed, 169 insertions(+), 46 deletions(-)
base-commit: 5e321ded302da4d8c5d5dd953423d9b748ab3775Hi, This patch-set is inappropriate for -net, if at all. It's quite late over here and I'll review the rest later, but I can say from a quick peek that patch 02 is unacceptable for it increases the complexity with 1 order of magnitude of all add/del call paths and some of them can be invoked on user packets. A lot of this functionality should be "hidden" in the driver or done by a user-space daemon/helper. Most of the flooding behaviour changes must be hidden behind some new option otherwise they'll break user setups that rely on the current. I'll review the patches in detail over the following few days, net-next is closed anyway. Cheers, Nik