--- v3
+++ v4
@@ -1,24 +1,105 @@
From: Vladimir Oltean <vladimir.oltean@nxp.com>
+
+We want to add reference counting for FDB entries in cross-chip
+topologies, and in order for that to have any chance of working and not
+be unbalanced (leading to entries which are never deleted), we need to
+ensure that higher layers are sane, because if they aren't, it's garbage
+in, garbage out.
+
+For example, if we add a bridge FDB entry twice, the bridge properly
+errors out:
+
+$ bridge fdb add dev swp0 00:01:02:03:04:07 master static
+$ bridge fdb add dev swp0 00:01:02:03:04:07 master static
+RTNETLINK answers: File exists
+
+However, the same thing cannot be said about the bridge bypass
+operations:
+
+$ bridge fdb add dev swp0 00:01:02:03:04:07
+$ bridge fdb add dev swp0 00:01:02:03:04:07
+$ bridge fdb add dev swp0 00:01:02:03:04:07
+$ bridge fdb add dev swp0 00:01:02:03:04:07
+$ echo $?
+0
+
+But one 'bridge fdb del' is enough to remove the entry, no matter how
+many times it was added.
+
+The bridge bypass operations are impossible to maintain in these
+circumstances and lack of support for reference counting the cross-chip
+notifiers is holding us back from making further progress, so just drop
+support for them. The only way left for users to install static bridge
+FDB entries is the proper one, using the "master static" flags.
+
+With this change, rtnl_fdb_add() falls back to calling
+ndo_dflt_fdb_add() which uses the duplicate-exclusive variant of
+dev_uc_add(): dev_uc_add_excl(). Because DSA does not (yet) declare
+IFF_UNICAST_FLT, this results in us going to promiscuous mode:
+
+$ bridge fdb add dev swp0 00:01:02:03:04:05
+[ 28.206743] device swp0 entered promiscuous mode
+$ bridge fdb add dev swp0 00:01:02:03:04:05
+RTNETLINK answers: File exists
+
+So even if it does not completely fail, there is at least some indication
+that it is behaving differently from before, and closer to user space
+expectations, I would argue (the lack of a "local|static" specifier
+defaults to "local", or "host-only", so dev_uc_add() is a reasonable
+default implementation). If the generic implementation of .ndo_fdb_add
+provided by Vlad Yasevich is a proof of anything, it only proves that
+the implementation provided by DSA was always wrong, by not looking at
+"ndm->ndm_state & NUD_NOARP" (the "static" flag which means that the FDB
+entry points outwards) and "ndm->ndm_state & NUD_PERMANENT" (the "local"
+flag which means that the FDB entry points towards the host). It all
+used to mean the same thing to DSA.
Signed-off-by: Vladimir Oltean <vladimir.oltean@nxp.com>
---
- net/bridge/br_mdb.c | 3 ++-
- 1 file changed, 2 insertions(+), 1 deletion(-)
+ net/dsa/slave.c | 23 -----------------------
+ 1 file changed, 23 deletions(-)
-diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
-index 17a720b4473f..fda61a90cfe5 100644
---- a/net/bridge/br_mdb.c
-+++ b/net/bridge/br_mdb.c
-@@ -617,7 +617,8 @@ int br_mdb_replay(struct net_device *br_dev, struct net_device *dev,
+diff --git a/net/dsa/slave.c b/net/dsa/slave.c
+index 898ed9cf756f..64acb1e11cd7 100644
+--- a/net/dsa/slave.c
++++ b/net/dsa/slave.c
+@@ -1651,27 +1651,6 @@ static const struct ethtool_ops dsa_slave_ethtool_ops = {
+ .self_test = dsa_slave_net_selftest,
+ };
- ASSERT_RTNL();
-
-- if (!netif_is_bridge_master(br_dev) || !netif_is_bridge_port(dev))
-+ if (!netif_is_bridge_master(br_dev) ||
-+ (!netif_is_bridge_port(dev) && !netif_is_bridge_master(dev)))
- return -EINVAL;
-
- br = netdev_priv(br_dev);
+-/* legacy way, bypassing the bridge *****************************************/
+-static int dsa_legacy_fdb_add(struct ndmsg *ndm, struct nlattr *tb[],
+- struct net_device *dev,
+- const unsigned char *addr, u16 vid,
+- u16 flags,
+- struct netlink_ext_ack *extack)
+-{
+- struct dsa_port *dp = dsa_slave_to_port(dev);
+-
+- return dsa_port_fdb_add(dp, addr, vid);
+-}
+-
+-static int dsa_legacy_fdb_del(struct ndmsg *ndm, struct nlattr *tb[],
+- struct net_device *dev,
+- const unsigned char *addr, u16 vid)
+-{
+- struct dsa_port *dp = dsa_slave_to_port(dev);
+-
+- return dsa_port_fdb_del(dp, addr, vid);
+-}
+-
+ static struct devlink_port *dsa_slave_get_devlink_port(struct net_device *dev)
+ {
+ struct dsa_port *dp = dsa_slave_to_port(dev);
+@@ -1713,8 +1692,6 @@ static const struct net_device_ops dsa_slave_netdev_ops = {
+ .ndo_change_rx_flags = dsa_slave_change_rx_flags,
+ .ndo_set_rx_mode = dsa_slave_set_rx_mode,
+ .ndo_set_mac_address = dsa_slave_set_mac_address,
+- .ndo_fdb_add = dsa_legacy_fdb_add,
+- .ndo_fdb_del = dsa_legacy_fdb_del,
+ .ndo_fdb_dump = dsa_slave_fdb_dump,
+ .ndo_do_ioctl = dsa_slave_ioctl,
+ .ndo_get_iflink = dsa_slave_get_iflink,
--
2.25.1