diff mbox series

[RFC,v2,net-next,05/17] net: bridge: implement unicast filtering for the bridge device

Message ID 20210224114350.2791260-6-olteanv@gmail.com (mailing list archive)
State RFC
Delegated to: Netdev Maintainers
Headers show
Series RX filtering in DSA | expand

Checks

Context Check Description
netdev/cover_letter success Link
netdev/fixes_present success Link
netdev/patch_count fail Series longer than 15 patches
netdev/tree_selection success Clearly marked for net-next
netdev/subject_prefix success Link
netdev/cc_maintainers warning 3 maintainers not CCed: davem@davemloft.net kuba@kernel.org bridge@lists.linux-foundation.org
netdev/source_inline success Was 0 now: 0
netdev/verify_signedoff success Link
netdev/module_param success Was 0 now: 0
netdev/build_32bit success Errors and warnings before: 0 this patch: 0
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/verify_fixes success Link
netdev/checkpatch success total: 0 errors, 0 warnings, 0 checks, 42 lines checked
netdev/build_allmodconfig_warn success Errors and warnings before: 0 this patch: 0
netdev/header_inline success Link
netdev/stable success Stable not CCed

Commit Message

Vladimir Oltean Feb. 24, 2021, 11:43 a.m. UTC
From: Vladimir Oltean <vladimir.oltean@nxp.com>

The bridge device currently goes into promiscuous mode when it has an
upper with a different MAC address than itself. But it could do better:
it could sync the MAC addresses of its uppers to the software FDB, as
local entries pointing to the bridge itself. This is compatible with
switchdev, since drivers are now instructed to trap these MAC addresses
to the CPU.

Note that the dev_uc_add API does not propagate VLAN ID, so this only
works for VLAN-unaware bridges.

Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
 net/bridge/br_device.c | 23 ++++++++++++++++++++---
 1 file changed, 20 insertions(+), 3 deletions(-)

Comments

Ido Schimmel March 1, 2021, 3:22 p.m. UTC | #1
On Wed, Feb 24, 2021 at 01:43:38PM +0200, Vladimir Oltean wrote:
> From: Vladimir Oltean <vladimir.oltean@nxp.com>
> 
> The bridge device currently goes into promiscuous mode when it has an
> upper with a different MAC address than itself. But it could do better:
> it could sync the MAC addresses of its uppers to the software FDB, as
> local entries pointing to the bridge itself. This is compatible with
> switchdev, since drivers are now instructed to trap these MAC addresses
> to the CPU.
> 
> Note that the dev_uc_add API does not propagate VLAN ID, so this only
> works for VLAN-unaware bridges.

IOW, it breaks VLAN-aware bridges...

I understand that you do not want to track bridge uppers, but once you
look beyond L2 you will need to do it anyway.

Currently, you only care about getting packets with specific DMACs to
the CPU. With L3 offload you will need to send these packets to your
router block instead and track other attributes of these uppers such as
their MTU so that the hardware will know to generate MTU exceptions. In
addition, the hardware needs to know the MAC addresses of these uppers
so that it will rewrite the SMAC of forwarded packets.
diff mbox series

Patch

diff --git a/net/bridge/br_device.c b/net/bridge/br_device.c
index 3f2f06b4dd27..a7d9d35e70d0 100644
--- a/net/bridge/br_device.c
+++ b/net/bridge/br_device.c
@@ -179,8 +179,25 @@  static int br_dev_open(struct net_device *dev)
 	return 0;
 }
 
-static void br_dev_set_multicast_list(struct net_device *dev)
+static int br_dev_sync_uc(struct net_device *dev, const unsigned char *addr)
 {
+	struct net_bridge *br = netdev_priv(dev);
+
+	return br_fdb_insert(br, NULL, addr, 0);
+}
+
+static int br_dev_unsync_uc(struct net_device *dev, const unsigned char *addr)
+{
+	struct net_bridge *br = netdev_priv(dev);
+
+	br_fdb_find_delete_local(br, NULL, addr, 0);
+
+	return 0;
+}
+
+static void br_dev_set_rx_mode(struct net_device *dev)
+{
+	__dev_uc_sync(dev, br_dev_sync_uc, br_dev_unsync_uc);
 }
 
 static void br_dev_change_rx_flags(struct net_device *dev, int change)
@@ -399,7 +416,7 @@  static const struct net_device_ops br_netdev_ops = {
 	.ndo_start_xmit		 = br_dev_xmit,
 	.ndo_get_stats64	 = dev_get_tstats64,
 	.ndo_set_mac_address	 = br_set_mac_address,
-	.ndo_set_rx_mode	 = br_dev_set_multicast_list,
+	.ndo_set_rx_mode	 = br_dev_set_rx_mode,
 	.ndo_change_rx_flags	 = br_dev_change_rx_flags,
 	.ndo_change_mtu		 = br_change_mtu,
 	.ndo_do_ioctl		 = br_dev_ioctl,
@@ -436,7 +453,7 @@  void br_dev_setup(struct net_device *dev)
 	dev->needs_free_netdev = true;
 	dev->ethtool_ops = &br_ethtool_ops;
 	SET_NETDEV_DEVTYPE(dev, &br_type);
-	dev->priv_flags = IFF_EBRIDGE | IFF_NO_QUEUE;
+	dev->priv_flags = IFF_EBRIDGE | IFF_NO_QUEUE | IFF_UNICAST_FLT;
 
 	dev->features = COMMON_FEATURES | NETIF_F_LLTX | NETIF_F_NETNS_LOCAL |
 			NETIF_F_HW_VLAN_CTAG_TX | NETIF_F_HW_VLAN_STAG_TX;