[PATCH net-next 0/3] net: lan966x: Extend switchdev with mdb support

[PATCH net-next 0/3] net: lan966x: Extend switchdev with mdb support

[Horatiu Vultur]

This patch series extends lan966x with mdb support by implementing
the switchdev callbacks: SWITCHDEV_OBJ_ID_PORT_MDB and
SWITCHDEV_OBJ_ID_HOST_MDB.
It adds support for both ipv4/ipv6 entries and l2 entries.

Horatiu Vultur (3):
  net: lan966x: Add function lan966x_mac_cpu_copy()
  net: lan966x: Add PGID_FIRST and PGID_LAST
  net: lan966x: Extend switchdev with mdb support

 .../net/ethernet/microchip/lan966x/Makefile   |   2 +-
 .../ethernet/microchip/lan966x/lan966x_mac.c  |  27 +-
 .../ethernet/microchip/lan966x/lan966x_main.c |   2 +
 .../ethernet/microchip/lan966x/lan966x_main.h |  24 +-
 .../ethernet/microchip/lan966x/lan966x_mdb.c  | 500 ++++++++++++++++++
 .../ethernet/microchip/lan966x/lan966x_regs.h |   6 +
 .../microchip/lan966x/lan966x_switchdev.c     |   8 +
 .../ethernet/microchip/lan966x/lan966x_vlan.c |   7 +-
 8 files changed, 568 insertions(+), 8 deletions(-)
 create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c

-- 
2.33.0

[PATCH net-next 1/3] net: lan966x: Add function lan966x_mac_cpu_copy()

[Horatiu Vultur]

Extend mac functionality with the function lan966x_mac_cpu_copy. This
function adds an entry in the MAC table where a frame is copy to the CPU
but also can be forward on the front ports.
This functionality is needed for mdb support. In case the CPU and some
of the front ports subscribe to an IP multicast address.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 .../ethernet/microchip/lan966x/lan966x_mac.c  | 27 ++++++++++++++++---
 .../ethernet/microchip/lan966x/lan966x_main.h |  5 ++++
 .../ethernet/microchip/lan966x/lan966x_regs.h |  6 +++++
 3 files changed, 34 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
index efadb8d326cc..ae3a7a10e0d6 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
@@ -68,16 +68,18 @@ static void lan966x_mac_select(struct lan966x *lan966x,
 	lan_wr(mach, lan966x, ANA_MACHDATA);
 }
 
-int lan966x_mac_learn(struct lan966x *lan966x, int port,
-		      const unsigned char mac[ETH_ALEN],
-		      unsigned int vid,
-		      enum macaccess_entry_type type)
+static int lan966x_mac_learn_impl(struct lan966x *lan966x, int port,
+				  bool cpu_copy,
+				  const unsigned char mac[ETH_ALEN],
+				  unsigned int vid,
+				  enum macaccess_entry_type type)
 {
 	lan966x_mac_select(lan966x, mac, vid);
 
 	/* Issue a write command */
 	lan_wr(ANA_MACACCESS_VALID_SET(1) |
 	       ANA_MACACCESS_CHANGE2SW_SET(0) |
+	       ANA_MACACCESS_MAC_CPU_COPY_SET(cpu_copy) |
 	       ANA_MACACCESS_DEST_IDX_SET(port) |
 	       ANA_MACACCESS_ENTRYTYPE_SET(type) |
 	       ANA_MACACCESS_MAC_TABLE_CMD_SET(MACACCESS_CMD_LEARN),
@@ -86,6 +88,23 @@ int lan966x_mac_learn(struct lan966x *lan966x, int port,
 	return lan966x_mac_wait_for_completion(lan966x);
 }
 
+int lan966x_mac_cpu_copy(struct lan966x *lan966x, int port,
+			 bool cpu_copy,
+			 const unsigned char mac[ETH_ALEN],
+			 unsigned int vid,
+			 enum macaccess_entry_type type)
+{
+	return lan966x_mac_learn_impl(lan966x, port, cpu_copy, mac, vid, type);
+}
+
+int lan966x_mac_learn(struct lan966x *lan966x, int port,
+		      const unsigned char mac[ETH_ALEN],
+		      unsigned int vid,
+		      enum macaccess_entry_type type)
+{
+	return lan966x_mac_learn_impl(lan966x, port, false, mac, vid, type);
+}
+
 int lan966x_mac_forget(struct lan966x *lan966x,
 		       const unsigned char mac[ETH_ALEN],
 		       unsigned int vid,
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
index c399b1256edc..a7a2a3537036 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
@@ -157,6 +157,11 @@ int lan966x_port_pcs_set(struct lan966x_port *port,
 			 struct lan966x_port_config *config);
 void lan966x_port_init(struct lan966x_port *port);
 
+int lan966x_mac_cpu_copy(struct lan966x *lan966x, int port,
+			 bool cpu_copy,
+			 const unsigned char mac[ETH_ALEN],
+			 unsigned int vid,
+			 enum macaccess_entry_type type);
 int lan966x_mac_learn(struct lan966x *lan966x, int port,
 		      const unsigned char mac[ETH_ALEN],
 		      unsigned int vid,
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h b/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h
index a13c469e139a..797560172aca 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h
@@ -169,6 +169,12 @@ enum lan966x_target {
 #define ANA_MACACCESS_CHANGE2SW_GET(x)\
 	FIELD_GET(ANA_MACACCESS_CHANGE2SW, x)
 
+#define ANA_MACACCESS_MAC_CPU_COPY               BIT(16)
+#define ANA_MACACCESS_MAC_CPU_COPY_SET(x)\
+	FIELD_PREP(ANA_MACACCESS_MAC_CPU_COPY, x)
+#define ANA_MACACCESS_MAC_CPU_COPY_GET(x)\
+	FIELD_GET(ANA_MACACCESS_MAC_CPU_COPY, x)
+
 #define ANA_MACACCESS_VALID                      BIT(12)
 #define ANA_MACACCESS_VALID_SET(x)\
 	FIELD_PREP(ANA_MACACCESS_VALID, x)
-- 
2.33.0

[PATCH net-next 2/3] net: lan966x: Add PGID_FIRST and PGID_LAST

[Horatiu Vultur]

The first entries in the PGID table are used by the front ports while
the last entries are used for different purposes like flooding mask,
copy to CPU, etc. So add these macros to define which entries can be
used for general purpose.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 drivers/net/ethernet/microchip/lan966x/lan966x_main.h | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
index a7a2a3537036..49ce6a04ca40 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
@@ -30,7 +30,11 @@
 /* Reserved amount for (SRC, PRIO) at index 8*SRC + PRIO */
 #define QSYS_Q_RSRV			95
 
+#define CPU_PORT			8
+
 /* Reserved PGIDs */
+#define PGID_FIRST			(CPU_PORT + 1)
+#define PGID_LAST			PGID_CPU
 #define PGID_CPU			(PGID_AGGR - 6)
 #define PGID_UC				(PGID_AGGR - 5)
 #define PGID_BC				(PGID_AGGR - 4)
@@ -44,8 +48,6 @@
 #define LAN966X_SPEED_100		2
 #define LAN966X_SPEED_10		3
 
-#define CPU_PORT			8
-
 /* MAC table entry types.
  * ENTRYTYPE_NORMAL is subject to aging.
  * ENTRYTYPE_LOCKED is not subject to aging.
-- 
2.33.0

[PATCH net-next 3/3] net: lan966x: Extend switchdev with mdb support

[Horatiu Vultur]

Extend lan966x driver with mdb support by implementing the switchdev
calls: SWITCHDEV_OBJ_ID_PORT_MDB and SWITCHDEV_OBJ_ID_HOST_MDB.
It is allowed to add both ipv4/ipv6 entries and l2 entries. To add
ipv4/ipv6 entries is not required to use the PGID table while for l2
entries it is required. The PGID table is much smaller than MAC table
so only fewer l2 entries can be added.

Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
---
 .../net/ethernet/microchip/lan966x/Makefile   |   2 +-
 .../ethernet/microchip/lan966x/lan966x_main.c |   2 +
 .../ethernet/microchip/lan966x/lan966x_main.h |  13 +
 .../ethernet/microchip/lan966x/lan966x_mdb.c  | 500 ++++++++++++++++++
 .../microchip/lan966x/lan966x_switchdev.c     |   8 +
 .../ethernet/microchip/lan966x/lan966x_vlan.c |   7 +-
 6 files changed, 530 insertions(+), 2 deletions(-)
 create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c

diff --git a/drivers/net/ethernet/microchip/lan966x/Makefile b/drivers/net/ethernet/microchip/lan966x/Makefile
index ec1a1fa8b0d5..040cfff9f577 100644
--- a/drivers/net/ethernet/microchip/lan966x/Makefile
+++ b/drivers/net/ethernet/microchip/lan966x/Makefile
@@ -7,4 +7,4 @@ obj-$(CONFIG_LAN966X_SWITCH) += lan966x-switch.o
 
 lan966x-switch-objs  := lan966x_main.o lan966x_phylink.o lan966x_port.o \
 			lan966x_mac.o lan966x_ethtool.o lan966x_switchdev.o \
-			lan966x_vlan.o lan966x_fdb.o
+			lan966x_vlan.o lan966x_fdb.o lan966x_mdb.o
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
index 2c6bf7b0afdf..2cb70da63db3 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
@@ -926,6 +926,7 @@ static int lan966x_probe(struct platform_device *pdev)
 		lan966x_port_init(lan966x->ports[p]);
 	}
 
+	lan966x_mdb_init(lan966x);
 	err = lan966x_fdb_init(lan966x);
 	if (err)
 		goto cleanup_ports;
@@ -955,6 +956,7 @@ static int lan966x_remove(struct platform_device *pdev)
 	mutex_destroy(&lan966x->stats_lock);
 
 	lan966x_mac_purge_entries(lan966x);
+	lan966x_mdb_deinit(lan966x);
 	lan966x_fdb_deinit(lan966x);
 
 	return 0;
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
index 49ce6a04ca40..76f0b5446b2e 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
@@ -107,6 +107,10 @@ struct lan966x {
 	/* worqueue for fdb */
 	struct workqueue_struct *fdb_work;
 	struct list_head fdb_entries;
+
+	/* mdb */
+	struct list_head mdb_entries;
+	struct list_head pgid_entries;
 };
 
 struct lan966x_port_config {
@@ -213,6 +217,15 @@ int lan966x_handle_fdb(struct net_device *dev,
 		       unsigned long event, const void *ctx,
 		       const struct switchdev_notifier_fdb_info *fdb_info);
 
+void lan966x_mdb_init(struct lan966x *lan966x);
+void lan966x_mdb_deinit(struct lan966x *lan966x);
+int lan966x_handle_port_mdb_add(struct lan966x_port *port,
+				const struct switchdev_obj *obj);
+int lan966x_handle_port_mdb_del(struct lan966x_port *port,
+				const struct switchdev_obj *obj);
+void lan966x_mdb_erase_entries(struct lan966x *lan966x, u16 vid);
+void lan966x_mdb_write_entries(struct lan966x *lan966x, u16 vid);
+
 static inline void __iomem *lan_addr(void __iomem *base[],
 				     int id, int tinst, int tcnt,
 				     int gbase, int ginst,
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c b/drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c
new file mode 100644
index 000000000000..4fd8b06a56c1
--- /dev/null
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c
@@ -0,0 +1,500 @@
+// SPDX-License-Identifier: GPL-2.0+
+
+#include <net/switchdev.h>
+
+#include "lan966x_main.h"
+
+struct lan966x_pgid_entry {
+	struct list_head list;
+	int index;
+	refcount_t refcount;
+	u16 ports;
+};
+
+struct lan966x_mdb_entry {
+	struct list_head list;
+	unsigned char mac[ETH_ALEN];
+	u16 vid;
+	u16 ports;
+	struct lan966x_pgid_entry *pgid;
+	bool cpu_copy;
+};
+
+void lan966x_mdb_init(struct lan966x *lan966x)
+{
+	INIT_LIST_HEAD(&lan966x->mdb_entries);
+	INIT_LIST_HEAD(&lan966x->pgid_entries);
+}
+
+static void lan966x_mdb_purge_mdb_entries(struct lan966x *lan966x)
+{
+	struct lan966x_mdb_entry *mdb_entry, *tmp;
+
+	list_for_each_entry_safe(mdb_entry, tmp, &lan966x->mdb_entries, list) {
+		list_del(&mdb_entry->list);
+		kfree(mdb_entry);
+	}
+}
+
+static void lan966x_mdb_purge_pgid_entries(struct lan966x *lan966x)
+{
+	struct lan966x_pgid_entry *pgid_entry, *tmp;
+
+	list_for_each_entry_safe(pgid_entry, tmp, &lan966x->pgid_entries, list) {
+		list_del(&pgid_entry->list);
+		kfree(pgid_entry);
+	}
+}
+
+void lan966x_mdb_deinit(struct lan966x *lan966x)
+{
+	lan966x_mdb_purge_mdb_entries(lan966x);
+	lan966x_mdb_purge_pgid_entries(lan966x);
+}
+
+static struct lan966x_mdb_entry *
+lan966x_mdb_entry_get(struct lan966x *lan966x,
+		      const unsigned char *mac,
+		      u16 vid)
+{
+	struct lan966x_mdb_entry *mdb_entry;
+
+	list_for_each_entry(mdb_entry, &lan966x->mdb_entries, list) {
+		if (ether_addr_equal(mdb_entry->mac, mac) &&
+		    mdb_entry->vid == vid)
+			return mdb_entry;
+	}
+
+	return NULL;
+}
+
+static struct lan966x_mdb_entry *
+lan966x_mdb_entry_add(struct lan966x *lan966x,
+		      const struct switchdev_obj_port_mdb *mdb)
+{
+	struct lan966x_mdb_entry *mdb_entry;
+
+	mdb_entry = kzalloc(sizeof(*mdb_entry), GFP_KERNEL);
+	if (!mdb_entry)
+		return ERR_PTR(-ENOMEM);
+
+	ether_addr_copy(mdb_entry->mac, mdb->addr);
+	mdb_entry->vid = mdb->vid;
+
+	list_add_tail(&mdb_entry->list, &lan966x->mdb_entries);
+
+	return mdb_entry;
+}
+
+static void lan966x_mdb_encode_mac(unsigned char *mac,
+				   struct lan966x_mdb_entry *mdb_entry,
+				   enum macaccess_entry_type type)
+{
+	ether_addr_copy(mac, mdb_entry->mac);
+
+	if (type == ENTRYTYPE_MACV4) {
+		mac[0] = 0;
+		mac[1] = mdb_entry->ports >> 8;
+		mac[2] = mdb_entry->ports & 0xff;
+	} else if (type == ENTRYTYPE_MACV6) {
+		mac[0] = mdb_entry->ports >> 8;
+		mac[1] = mdb_entry->ports & 0xff;
+	}
+}
+
+static int lan966x_mdb_ip_add(struct lan966x_port *port,
+			      const struct switchdev_obj_port_mdb *mdb,
+			      enum macaccess_entry_type type)
+{
+	bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
+	struct lan966x *lan966x = port->lan966x;
+	struct lan966x_mdb_entry *mdb_entry;
+	unsigned char mac[ETH_ALEN];
+	bool cpu_copy = false;
+
+	mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
+	if (!mdb_entry) {
+		mdb_entry = lan966x_mdb_entry_add(lan966x, mdb);
+		if (IS_ERR(mdb_entry))
+			return PTR_ERR(mdb_entry);
+	} else {
+		lan966x_mdb_encode_mac(mac, mdb_entry, type);
+		lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
+	}
+
+	if (cpu_port)
+		mdb_entry->cpu_copy = true;
+	else
+		mdb_entry->ports |= BIT(port->chip_port);
+
+	/* Copy the frame to CPU only if the CPU is the vlan */
+	if (mdb_entry->cpu_copy) {
+		if (lan966x_vlan_cpu_member_cpu_vlan_mask(lan966x,
+							  mdb_entry->vid))
+			cpu_copy = true;
+	}
+
+	lan966x_mdb_encode_mac(mac, mdb_entry, type);
+	return lan966x_mac_cpu_copy(lan966x, 0, cpu_copy,
+				    mac, mdb_entry->vid, type);
+}
+
+static int lan966x_mdb_ip_del(struct lan966x_port *port,
+			      const struct switchdev_obj_port_mdb *mdb,
+			      enum macaccess_entry_type type)
+{
+	bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
+	struct lan966x *lan966x = port->lan966x;
+	struct lan966x_mdb_entry *mdb_entry;
+	unsigned char mac[ETH_ALEN];
+
+	mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
+	if (!mdb_entry) {
+		/* If the CPU originted this and the entry was not found, it is
+		 * because already another port has removed the entry
+		 */
+		if (cpu_port)
+			return 0;
+		return -ENOENT;
+	}
+
+	lan966x_mdb_encode_mac(mac, mdb_entry, type);
+	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
+
+	if (cpu_port)
+		mdb_entry->cpu_copy = false;
+	else
+		mdb_entry->ports &= ~BIT(port->chip_port);
+	if (!mdb_entry->ports && !mdb_entry->cpu_copy) {
+		list_del(&mdb_entry->list);
+		kfree(mdb_entry);
+		return 0;
+	}
+
+	lan966x_mdb_encode_mac(mac, mdb_entry, type);
+	return lan966x_mac_cpu_copy(lan966x, 0, mdb_entry->cpu_copy,
+				    mac, mdb_entry->vid, type);
+}
+
+static struct lan966x_pgid_entry *
+lan966x_pgid_entry_add(struct lan966x *lan966x, int index, u16 ports)
+{
+	struct lan966x_pgid_entry *pgid_entry;
+
+	pgid_entry = kzalloc(sizeof(*pgid_entry), GFP_KERNEL);
+	if (!pgid_entry)
+		return ERR_PTR(-ENOMEM);
+
+	pgid_entry->ports = ports;
+	pgid_entry->index = index;
+	refcount_set(&pgid_entry->refcount, 1);
+
+	list_add_tail(&pgid_entry->list, &lan966x->pgid_entries);
+
+	return pgid_entry;
+}
+
+static struct lan966x_pgid_entry *
+lan966x_pgid_entry_get(struct lan966x *lan966x,
+		       struct lan966x_mdb_entry *mdb_entry)
+{
+	struct lan966x_pgid_entry *pgid_entry;
+	int index;
+
+	/* Try to find an existing pgid that uses the same ports as the
+	 * mdb_entry
+	 */
+	list_for_each_entry(pgid_entry, &lan966x->pgid_entries, list) {
+		if (pgid_entry->ports == mdb_entry->ports) {
+			refcount_inc(&pgid_entry->refcount);
+			return pgid_entry;
+		}
+	}
+
+	/* Try to find an empty pgid entry and allocate one in case it finds it,
+	 * otherwise it means that there are no more resources
+	 */
+	for (index = PGID_FIRST; index < PGID_LAST; index++) {
+		bool used = false;
+
+		list_for_each_entry(pgid_entry, &lan966x->pgid_entries, list) {
+			if (pgid_entry->index == index) {
+				used = true;
+				break;
+			}
+		}
+
+		if (!used)
+			return lan966x_pgid_entry_add(lan966x, index,
+						      mdb_entry->ports);
+	}
+
+	return ERR_PTR(-ENOSPC);
+}
+
+static void lan966x_pgid_entry_del(struct lan966x *lan966x,
+				   struct lan966x_pgid_entry *pgid_entry)
+{
+	if (!refcount_dec_and_test(&pgid_entry->refcount))
+		return;
+
+	list_del(&pgid_entry->list);
+	kfree(pgid_entry);
+}
+
+static int lan966x_mdb_l2_add(struct lan966x_port *port,
+			      const struct switchdev_obj_port_mdb *mdb,
+			      enum macaccess_entry_type type)
+{
+	bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
+	struct lan966x *lan966x = port->lan966x;
+	struct lan966x_pgid_entry *pgid_entry;
+	struct lan966x_mdb_entry *mdb_entry;
+	unsigned char mac[ETH_ALEN];
+
+	mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
+	if (!mdb_entry) {
+		mdb_entry = lan966x_mdb_entry_add(lan966x, mdb);
+		if (IS_ERR(mdb_entry))
+			return PTR_ERR(mdb_entry);
+	} else {
+		lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
+		lan966x_mdb_encode_mac(mac, mdb_entry, type);
+		lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
+	}
+
+	if (cpu_port) {
+		mdb_entry->ports |= BIT(CPU_PORT);
+		mdb_entry->cpu_copy = true;
+	} else {
+		mdb_entry->ports |= BIT(port->chip_port);
+	}
+
+	pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
+	if (IS_ERR(pgid_entry)) {
+		list_del(&mdb_entry->list);
+		kfree(mdb_entry);
+		return PTR_ERR(pgid_entry);
+	}
+	mdb_entry->pgid = pgid_entry;
+
+	/* Copy the frame to CPU only if the CPU is the vlan */
+	if (mdb_entry->cpu_copy) {
+		if (!lan966x_vlan_cpu_member_cpu_vlan_mask(lan966x,
+							   mdb_entry->vid))
+			mdb_entry->ports &= BIT(CPU_PORT);
+	}
+
+	lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
+		ANA_PGID_PGID,
+		lan966x, ANA_PGID(pgid_entry->index));
+
+	return lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
+				 mdb_entry->vid, type);
+}
+
+static int lan966x_mdb_l2_del(struct lan966x_port *port,
+			      const struct switchdev_obj_port_mdb *mdb,
+			      enum macaccess_entry_type type)
+{
+	bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
+	struct lan966x *lan966x = port->lan966x;
+	struct lan966x_pgid_entry *pgid_entry;
+	struct lan966x_mdb_entry *mdb_entry;
+	unsigned char mac[ETH_ALEN];
+
+	mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
+	if (!mdb_entry)
+		return -ENOENT;
+
+	lan966x_mdb_encode_mac(mac, mdb_entry, type);
+	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
+	lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
+
+	if (cpu_port)
+		mdb_entry->ports &= ~BIT(CPU_PORT);
+	else
+		mdb_entry->ports &= ~BIT(port->chip_port);
+	if (!mdb_entry->ports) {
+		list_del(&mdb_entry->list);
+		kfree(mdb_entry);
+		return 0;
+	}
+
+	pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
+	if (IS_ERR(pgid_entry)) {
+		list_del(&mdb_entry->list);
+		kfree(mdb_entry);
+		return PTR_ERR(pgid_entry);
+	}
+	mdb_entry->pgid = pgid_entry;
+
+	lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
+		ANA_PGID_PGID,
+		lan966x, ANA_PGID(pgid_entry->index));
+
+	return lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
+				 mdb_entry->vid, type);
+}
+
+static enum macaccess_entry_type
+lan966x_mdb_classify(const unsigned char *mac)
+{
+	if (mac[0] == 0x01 && mac[1] == 0x00 && mac[2] == 0x5e)
+		return ENTRYTYPE_MACV4;
+	if (mac[0] == 0x33 && mac[1] == 0x33)
+		return ENTRYTYPE_MACV6;
+	return ENTRYTYPE_LOCKED;
+}
+
+int lan966x_handle_port_mdb_add(struct lan966x_port *port,
+				const struct switchdev_obj *obj)
+{
+	const struct switchdev_obj_port_mdb *mdb = SWITCHDEV_OBJ_PORT_MDB(obj);
+	enum macaccess_entry_type type;
+
+	/* Split the way the entries are added for ipv4/ipv6 and for l2. The
+	 * reason is that for ipv4/ipv6 it doesn't require to use any pgid
+	 * entry, while for l2 is required to use pgid entries
+	 */
+	type = lan966x_mdb_classify(mdb->addr);
+	if (type == ENTRYTYPE_MACV4 ||
+	    type == ENTRYTYPE_MACV6)
+		return lan966x_mdb_ip_add(port, mdb, type);
+	else
+		return lan966x_mdb_l2_add(port, mdb, type);
+
+	return 0;
+}
+
+int lan966x_handle_port_mdb_del(struct lan966x_port *port,
+				const struct switchdev_obj *obj)
+{
+	const struct switchdev_obj_port_mdb *mdb = SWITCHDEV_OBJ_PORT_MDB(obj);
+	enum macaccess_entry_type type;
+
+	/* Split the way the entries are removed for ipv4/ipv6 and for l2. The
+	 * reason is that for ipv4/ipv6 it doesn't require to use any pgid
+	 * entry, while for l2 is required to use pgid entries
+	 */
+	type = lan966x_mdb_classify(mdb->addr);
+	if (type == ENTRYTYPE_MACV4 ||
+	    type == ENTRYTYPE_MACV6)
+		return lan966x_mdb_ip_del(port, mdb, type);
+	else
+		return lan966x_mdb_l2_del(port, mdb, type);
+
+	return 0;
+}
+
+static void lan966x_mdb_ip_cpu_copy(struct lan966x *lan966x,
+				    struct lan966x_mdb_entry *mdb_entry,
+				    enum macaccess_entry_type type)
+{
+	unsigned char mac[ETH_ALEN];
+
+	lan966x_mdb_encode_mac(mac, mdb_entry, type);
+	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
+	lan966x_mac_cpu_copy(lan966x, 0, true, mac, mdb_entry->vid, type);
+}
+
+static void lan966x_mdb_l2_cpu_copy(struct lan966x *lan966x,
+				    struct lan966x_mdb_entry *mdb_entry,
+				    enum macaccess_entry_type type)
+{
+	struct lan966x_pgid_entry *pgid_entry;
+	unsigned char mac[ETH_ALEN];
+
+	lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
+	lan966x_mdb_encode_mac(mac, mdb_entry, type);
+	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
+
+	mdb_entry->ports |= BIT(CPU_PORT);
+
+	pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
+	if (IS_ERR(pgid_entry))
+		return;
+
+	mdb_entry->pgid = pgid_entry;
+
+	lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
+		ANA_PGID_PGID,
+		lan966x, ANA_PGID(pgid_entry->index));
+
+	lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
+			  mdb_entry->vid, type);
+}
+
+void lan966x_mdb_write_entries(struct lan966x *lan966x, u16 vid)
+{
+	struct lan966x_mdb_entry *mdb_entry;
+	enum macaccess_entry_type type;
+
+	list_for_each_entry(mdb_entry, &lan966x->mdb_entries, list) {
+		if (mdb_entry->vid != vid || !mdb_entry->cpu_copy)
+			continue;
+
+		type = lan966x_mdb_classify(mdb_entry->mac);
+		if (type == ENTRYTYPE_MACV4 ||
+		    type == ENTRYTYPE_MACV6)
+			lan966x_mdb_ip_cpu_copy(lan966x, mdb_entry, type);
+		else
+			lan966x_mdb_l2_cpu_copy(lan966x, mdb_entry, type);
+	}
+}
+
+static void lan966x_mdb_ip_cpu_remove(struct lan966x *lan966x,
+				      struct lan966x_mdb_entry *mdb_entry,
+				      enum macaccess_entry_type type)
+{
+	unsigned char mac[ETH_ALEN];
+
+	lan966x_mdb_encode_mac(mac, mdb_entry, type);
+	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
+	lan966x_mac_cpu_copy(lan966x, 0, false, mac, mdb_entry->vid, type);
+}
+
+static void lan966x_mdb_l2_cpu_remove(struct lan966x *lan966x,
+				      struct lan966x_mdb_entry *mdb_entry,
+				      enum macaccess_entry_type type)
+{
+	struct lan966x_pgid_entry *pgid_entry;
+	unsigned char mac[ETH_ALEN];
+
+	lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
+	lan966x_mdb_encode_mac(mac, mdb_entry, type);
+	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
+
+	mdb_entry->ports &= ~BIT(CPU_PORT);
+
+	pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
+	if (IS_ERR(pgid_entry))
+		return;
+
+	mdb_entry->pgid = pgid_entry;
+
+	lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
+		ANA_PGID_PGID,
+		lan966x, ANA_PGID(pgid_entry->index));
+
+	lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
+			  mdb_entry->vid, type);
+}
+
+void lan966x_mdb_erase_entries(struct lan966x *lan966x, u16 vid)
+{
+	struct lan966x_mdb_entry *mdb_entry;
+	enum macaccess_entry_type type;
+
+	list_for_each_entry(mdb_entry, &lan966x->mdb_entries, list) {
+		if (mdb_entry->vid != vid || !mdb_entry->cpu_copy)
+			continue;
+
+		type = lan966x_mdb_classify(mdb_entry->mac);
+		if (type == ENTRYTYPE_MACV4 ||
+		    type == ENTRYTYPE_MACV6)
+			lan966x_mdb_ip_cpu_remove(lan966x, mdb_entry, type);
+		else
+			lan966x_mdb_l2_cpu_remove(lan966x, mdb_entry, type);
+	}
+}
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
index deb3dd5be67a..7de55f6a4da8 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
@@ -438,6 +438,10 @@ static int lan966x_handle_port_obj_add(struct net_device *dev, const void *ctx,
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		err = lan966x_handle_port_vlan_add(port, obj);
 		break;
+	case SWITCHDEV_OBJ_ID_PORT_MDB:
+	case SWITCHDEV_OBJ_ID_HOST_MDB:
+		err = lan966x_handle_port_mdb_add(port, obj);
+		break;
 	default:
 		err = -EOPNOTSUPP;
 		break;
@@ -473,6 +477,10 @@ static int lan966x_handle_port_obj_del(struct net_device *dev, const void *ctx,
 	case SWITCHDEV_OBJ_ID_PORT_VLAN:
 		err = lan966x_handle_port_vlan_del(port, obj);
 		break;
+	case SWITCHDEV_OBJ_ID_PORT_MDB:
+	case SWITCHDEV_OBJ_ID_HOST_MDB:
+		err = lan966x_handle_port_mdb_del(port, obj);
+		break;
 	default:
 		err = -EOPNOTSUPP;
 		break;
diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c b/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
index 057f48ddf22c..8d7260cd7da9 100644
--- a/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
+++ b/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
@@ -219,6 +219,7 @@ void lan966x_vlan_port_add_vlan(struct lan966x_port *port,
 	if (lan966x_vlan_cpu_member_cpu_vlan_mask(lan966x, vid)) {
 		lan966x_vlan_cpu_add_vlan_mask(lan966x, vid);
 		lan966x_fdb_write_entries(lan966x, vid);
+		lan966x_mdb_write_entries(lan966x, vid);
 	}
 
 	lan966x_vlan_port_set_vid(port, vid, pvid, untagged);
@@ -241,6 +242,7 @@ void lan966x_vlan_port_del_vlan(struct lan966x_port *port, u16 vid)
 	if (!lan966x_vlan_port_any_vlan_mask(lan966x, vid)) {
 		lan966x_vlan_cpu_del_vlan_mask(lan966x, vid);
 		lan966x_fdb_erase_entries(lan966x, vid);
+		lan966x_mdb_erase_entries(lan966x, vid);
 	}
 }
 
@@ -254,8 +256,10 @@ void lan966x_vlan_cpu_add_vlan(struct lan966x *lan966x, u16 vid)
 	 * information so when a front port is added then it would add also the
 	 * CPU port.
 	 */
-	if (lan966x_vlan_port_any_vlan_mask(lan966x, vid))
+	if (lan966x_vlan_port_any_vlan_mask(lan966x, vid)) {
 		lan966x_vlan_cpu_add_vlan_mask(lan966x, vid);
+		lan966x_mdb_write_entries(lan966x, vid);
+	}
 
 	lan966x_vlan_cpu_add_cpu_vlan_mask(lan966x, vid);
 	lan966x_fdb_write_entries(lan966x, vid);
@@ -267,6 +271,7 @@ void lan966x_vlan_cpu_del_vlan(struct lan966x *lan966x, u16 vid)
 	lan966x_vlan_cpu_del_cpu_vlan_mask(lan966x, vid);
 	lan966x_vlan_cpu_del_vlan_mask(lan966x, vid);
 	lan966x_fdb_erase_entries(lan966x, vid);
+	lan966x_mdb_erase_entries(lan966x, vid);
 }
 
 void lan966x_vlan_init(struct lan966x *lan966x)
-- 
2.33.0

Re: [PATCH net-next 1/3] net: lan966x: Add function lan966x_mac_cpu_copy()

[Vladimir Oltean]

On Mon, Jan 03, 2022 at 02:10:37PM +0100, Horatiu Vultur wrote:
> Extend mac functionality with the function lan966x_mac_cpu_copy. This
> function adds an entry in the MAC table where a frame is copy to the CPU

s/copy/copied/

> but also can be forward on the front ports.

s/forward/forwarded/

> This functionality is needed for mdb support. In case the CPU and some
> of the front ports subscribe to an IP multicast address.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  .../ethernet/microchip/lan966x/lan966x_mac.c  | 27 ++++++++++++++++---
>  .../ethernet/microchip/lan966x/lan966x_main.h |  5 ++++
>  .../ethernet/microchip/lan966x/lan966x_regs.h |  6 +++++
>  3 files changed, 34 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
> index efadb8d326cc..ae3a7a10e0d6 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
> @@ -68,16 +68,18 @@ static void lan966x_mac_select(struct lan966x *lan966x,
>  	lan_wr(mach, lan966x, ANA_MACHDATA);
>  }
>  
> -int lan966x_mac_learn(struct lan966x *lan966x, int port,
> -		      const unsigned char mac[ETH_ALEN],
> -		      unsigned int vid,
> -		      enum macaccess_entry_type type)
> +static int lan966x_mac_learn_impl(struct lan966x *lan966x, int port,
> +				  bool cpu_copy,
> +				  const unsigned char mac[ETH_ALEN],
> +				  unsigned int vid,
> +				  enum macaccess_entry_type type)

In the kernel, the __lan966x_mac_learn naming scheme seems to be more
popular than _impl.

Also, may I suggest that the "int port" argument may be better named
"int pgid"?

>  {
>  	lan966x_mac_select(lan966x, mac, vid);
>  
>  	/* Issue a write command */
>  	lan_wr(ANA_MACACCESS_VALID_SET(1) |
>  	       ANA_MACACCESS_CHANGE2SW_SET(0) |
> +	       ANA_MACACCESS_MAC_CPU_COPY_SET(cpu_copy) |
>  	       ANA_MACACCESS_DEST_IDX_SET(port) |
>  	       ANA_MACACCESS_ENTRYTYPE_SET(type) |
>  	       ANA_MACACCESS_MAC_TABLE_CMD_SET(MACACCESS_CMD_LEARN),
> @@ -86,6 +88,23 @@ int lan966x_mac_learn(struct lan966x *lan966x, int port,
>  	return lan966x_mac_wait_for_completion(lan966x);
>  }
>  
> +int lan966x_mac_cpu_copy(struct lan966x *lan966x, int port,
> +			 bool cpu_copy,
> +			 const unsigned char mac[ETH_ALEN],
> +			 unsigned int vid,
> +			 enum macaccess_entry_type type)

This doesn't seem to use the "port" argument from any of its call sites.
It is always 0. What would it even mean?

> +{
> +	return lan966x_mac_learn_impl(lan966x, port, cpu_copy, mac, vid, type);
> +}
> +
> +int lan966x_mac_learn(struct lan966x *lan966x, int port,
> +		      const unsigned char mac[ETH_ALEN],
> +		      unsigned int vid,
> +		      enum macaccess_entry_type type)
> +{
> +	return lan966x_mac_learn_impl(lan966x, port, false, mac, vid, type);

If you call lan966x_mac_cpu_copy() on an address and then
lan966x_mac_learn() on the same address but on an external port, how
does that work (why doesn't the "false" here overwrite the cpu_copy in
the previous command, breaking the copy-to-CPU feature)?

> +}
> +
>  int lan966x_mac_forget(struct lan966x *lan966x,
>  		       const unsigned char mac[ETH_ALEN],
>  		       unsigned int vid,
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> index c399b1256edc..a7a2a3537036 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> @@ -157,6 +157,11 @@ int lan966x_port_pcs_set(struct lan966x_port *port,
>  			 struct lan966x_port_config *config);
>  void lan966x_port_init(struct lan966x_port *port);
>  
> +int lan966x_mac_cpu_copy(struct lan966x *lan966x, int port,
> +			 bool cpu_copy,
> +			 const unsigned char mac[ETH_ALEN],
> +			 unsigned int vid,
> +			 enum macaccess_entry_type type);
>  int lan966x_mac_learn(struct lan966x *lan966x, int port,
>  		      const unsigned char mac[ETH_ALEN],
>  		      unsigned int vid,
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h b/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h
> index a13c469e139a..797560172aca 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h
> @@ -169,6 +169,12 @@ enum lan966x_target {
>  #define ANA_MACACCESS_CHANGE2SW_GET(x)\
>  	FIELD_GET(ANA_MACACCESS_CHANGE2SW, x)
>  
> +#define ANA_MACACCESS_MAC_CPU_COPY               BIT(16)
> +#define ANA_MACACCESS_MAC_CPU_COPY_SET(x)\
> +	FIELD_PREP(ANA_MACACCESS_MAC_CPU_COPY, x)
> +#define ANA_MACACCESS_MAC_CPU_COPY_GET(x)\
> +	FIELD_GET(ANA_MACACCESS_MAC_CPU_COPY, x)
> +
>  #define ANA_MACACCESS_VALID                      BIT(12)
>  #define ANA_MACACCESS_VALID_SET(x)\
>  	FIELD_PREP(ANA_MACACCESS_VALID, x)
> -- 
> 2.33.0
>

Re: [PATCH net-next 3/3] net: lan966x: Extend switchdev with mdb support

[Vladimir Oltean]

On Mon, Jan 03, 2022 at 02:10:39PM +0100, Horatiu Vultur wrote:
> Extend lan966x driver with mdb support by implementing the switchdev
> calls: SWITCHDEV_OBJ_ID_PORT_MDB and SWITCHDEV_OBJ_ID_HOST_MDB.
> It is allowed to add both ipv4/ipv6 entries and l2 entries. To add
> ipv4/ipv6 entries is not required to use the PGID table while for l2
> entries it is required. The PGID table is much smaller than MAC table
> so only fewer l2 entries can be added.
> 
> Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> ---
>  .../net/ethernet/microchip/lan966x/Makefile   |   2 +-
>  .../ethernet/microchip/lan966x/lan966x_main.c |   2 +
>  .../ethernet/microchip/lan966x/lan966x_main.h |  13 +
>  .../ethernet/microchip/lan966x/lan966x_mdb.c  | 500 ++++++++++++++++++
>  .../microchip/lan966x/lan966x_switchdev.c     |   8 +
>  .../ethernet/microchip/lan966x/lan966x_vlan.c |   7 +-
>  6 files changed, 530 insertions(+), 2 deletions(-)
>  create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c
> 
> diff --git a/drivers/net/ethernet/microchip/lan966x/Makefile b/drivers/net/ethernet/microchip/lan966x/Makefile
> index ec1a1fa8b0d5..040cfff9f577 100644
> --- a/drivers/net/ethernet/microchip/lan966x/Makefile
> +++ b/drivers/net/ethernet/microchip/lan966x/Makefile
> @@ -7,4 +7,4 @@ obj-$(CONFIG_LAN966X_SWITCH) += lan966x-switch.o
>  
>  lan966x-switch-objs  := lan966x_main.o lan966x_phylink.o lan966x_port.o \
>  			lan966x_mac.o lan966x_ethtool.o lan966x_switchdev.o \
> -			lan966x_vlan.o lan966x_fdb.o
> +			lan966x_vlan.o lan966x_fdb.o lan966x_mdb.o
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> index 2c6bf7b0afdf..2cb70da63db3 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> @@ -926,6 +926,7 @@ static int lan966x_probe(struct platform_device *pdev)
>  		lan966x_port_init(lan966x->ports[p]);
>  	}
>  
> +	lan966x_mdb_init(lan966x);
>  	err = lan966x_fdb_init(lan966x);
>  	if (err)
>  		goto cleanup_ports;
> @@ -955,6 +956,7 @@ static int lan966x_remove(struct platform_device *pdev)
>  	mutex_destroy(&lan966x->stats_lock);
>  
>  	lan966x_mac_purge_entries(lan966x);
> +	lan966x_mdb_deinit(lan966x);
>  	lan966x_fdb_deinit(lan966x);
>  
>  	return 0;
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> index 49ce6a04ca40..76f0b5446b2e 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> @@ -107,6 +107,10 @@ struct lan966x {
>  	/* worqueue for fdb */
>  	struct workqueue_struct *fdb_work;
>  	struct list_head fdb_entries;
> +
> +	/* mdb */
> +	struct list_head mdb_entries;
> +	struct list_head pgid_entries;
>  };
>  
>  struct lan966x_port_config {
> @@ -213,6 +217,15 @@ int lan966x_handle_fdb(struct net_device *dev,
>  		       unsigned long event, const void *ctx,
>  		       const struct switchdev_notifier_fdb_info *fdb_info);
>  
> +void lan966x_mdb_init(struct lan966x *lan966x);
> +void lan966x_mdb_deinit(struct lan966x *lan966x);
> +int lan966x_handle_port_mdb_add(struct lan966x_port *port,
> +				const struct switchdev_obj *obj);
> +int lan966x_handle_port_mdb_del(struct lan966x_port *port,
> +				const struct switchdev_obj *obj);
> +void lan966x_mdb_erase_entries(struct lan966x *lan966x, u16 vid);
> +void lan966x_mdb_write_entries(struct lan966x *lan966x, u16 vid);
> +
>  static inline void __iomem *lan_addr(void __iomem *base[],
>  				     int id, int tinst, int tcnt,
>  				     int gbase, int ginst,
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c b/drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c
> new file mode 100644
> index 000000000000..4fd8b06a56c1
> --- /dev/null
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c
> @@ -0,0 +1,500 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +
> +#include <net/switchdev.h>
> +
> +#include "lan966x_main.h"
> +
> +struct lan966x_pgid_entry {
> +	struct list_head list;
> +	int index;
> +	refcount_t refcount;
> +	u16 ports;
> +};
> +
> +struct lan966x_mdb_entry {
> +	struct list_head list;
> +	unsigned char mac[ETH_ALEN];
> +	u16 vid;
> +	u16 ports;
> +	struct lan966x_pgid_entry *pgid;
> +	bool cpu_copy;
> +};
> +
> +void lan966x_mdb_init(struct lan966x *lan966x)
> +{
> +	INIT_LIST_HEAD(&lan966x->mdb_entries);
> +	INIT_LIST_HEAD(&lan966x->pgid_entries);
> +}
> +
> +static void lan966x_mdb_purge_mdb_entries(struct lan966x *lan966x)
> +{
> +	struct lan966x_mdb_entry *mdb_entry, *tmp;
> +
> +	list_for_each_entry_safe(mdb_entry, tmp, &lan966x->mdb_entries, list) {
> +		list_del(&mdb_entry->list);
> +		kfree(mdb_entry);
> +	}
> +}
> +
> +static void lan966x_mdb_purge_pgid_entries(struct lan966x *lan966x)
> +{
> +	struct lan966x_pgid_entry *pgid_entry, *tmp;
> +
> +	list_for_each_entry_safe(pgid_entry, tmp, &lan966x->pgid_entries, list) {
> +		list_del(&pgid_entry->list);
> +		kfree(pgid_entry);
> +	}
> +}
> +
> +void lan966x_mdb_deinit(struct lan966x *lan966x)
> +{
> +	lan966x_mdb_purge_mdb_entries(lan966x);
> +	lan966x_mdb_purge_pgid_entries(lan966x);
> +}
> +
> +static struct lan966x_mdb_entry *
> +lan966x_mdb_entry_get(struct lan966x *lan966x,
> +		      const unsigned char *mac,
> +		      u16 vid)
> +{
> +	struct lan966x_mdb_entry *mdb_entry;
> +
> +	list_for_each_entry(mdb_entry, &lan966x->mdb_entries, list) {
> +		if (ether_addr_equal(mdb_entry->mac, mac) &&
> +		    mdb_entry->vid == vid)
> +			return mdb_entry;
> +	}
> +
> +	return NULL;
> +}
> +
> +static struct lan966x_mdb_entry *
> +lan966x_mdb_entry_add(struct lan966x *lan966x,
> +		      const struct switchdev_obj_port_mdb *mdb)
> +{
> +	struct lan966x_mdb_entry *mdb_entry;
> +
> +	mdb_entry = kzalloc(sizeof(*mdb_entry), GFP_KERNEL);
> +	if (!mdb_entry)
> +		return ERR_PTR(-ENOMEM);
> +
> +	ether_addr_copy(mdb_entry->mac, mdb->addr);
> +	mdb_entry->vid = mdb->vid;
> +
> +	list_add_tail(&mdb_entry->list, &lan966x->mdb_entries);
> +
> +	return mdb_entry;
> +}
> +
> +static void lan966x_mdb_encode_mac(unsigned char *mac,
> +				   struct lan966x_mdb_entry *mdb_entry,
> +				   enum macaccess_entry_type type)
> +{
> +	ether_addr_copy(mac, mdb_entry->mac);
> +
> +	if (type == ENTRYTYPE_MACV4) {
> +		mac[0] = 0;
> +		mac[1] = mdb_entry->ports >> 8;
> +		mac[2] = mdb_entry->ports & 0xff;
> +	} else if (type == ENTRYTYPE_MACV6) {
> +		mac[0] = mdb_entry->ports >> 8;
> +		mac[1] = mdb_entry->ports & 0xff;
> +	}
> +}
> +
> +static int lan966x_mdb_ip_add(struct lan966x_port *port,
> +			      const struct switchdev_obj_port_mdb *mdb,
> +			      enum macaccess_entry_type type)
> +{
> +	bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
> +	struct lan966x *lan966x = port->lan966x;
> +	struct lan966x_mdb_entry *mdb_entry;
> +	unsigned char mac[ETH_ALEN];
> +	bool cpu_copy = false;
> +
> +	mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
> +	if (!mdb_entry) {
> +		mdb_entry = lan966x_mdb_entry_add(lan966x, mdb);
> +		if (IS_ERR(mdb_entry))
> +			return PTR_ERR(mdb_entry);
> +	} else {
> +		lan966x_mdb_encode_mac(mac, mdb_entry, type);
> +		lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> +	}
> +
> +	if (cpu_port)
> +		mdb_entry->cpu_copy = true;
> +	else
> +		mdb_entry->ports |= BIT(port->chip_port);
> +
> +	/* Copy the frame to CPU only if the CPU is the vlan */

s/is the vlan/is in the VLAN/

> +	if (mdb_entry->cpu_copy) {
> +		if (lan966x_vlan_cpu_member_cpu_vlan_mask(lan966x,
> +							  mdb_entry->vid))
> +			cpu_copy = true;
> +	}

I find it slightly simpler to squash the two if blocks into a single one.

	if (lan966x_vlan_cpu_member_cpu_vlan_mask(lan966x, mdb_entry->vid) &&
	    mdb_entry->cpu_copy)
		cpu_copy = true;

> +
> +	lan966x_mdb_encode_mac(mac, mdb_entry, type);
> +	return lan966x_mac_cpu_copy(lan966x, 0, cpu_copy,
> +				    mac, mdb_entry->vid, type);
> +}
> +
> +static int lan966x_mdb_ip_del(struct lan966x_port *port,
> +			      const struct switchdev_obj_port_mdb *mdb,
> +			      enum macaccess_entry_type type)
> +{
> +	bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
> +	struct lan966x *lan966x = port->lan966x;
> +	struct lan966x_mdb_entry *mdb_entry;
> +	unsigned char mac[ETH_ALEN];
> +
> +	mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
> +	if (!mdb_entry) {
> +		/* If the CPU originted this and the entry was not found, it is

s/originted/originated/

> +		 * because already another port has removed the entry
> +		 */
> +		if (cpu_port)
> +			return 0;

Could you explain again why this is normal? If another port has removed
the entry, how is the address copied to the CPU any longer?

> +		return -ENOENT;
> +	}
> +
> +	lan966x_mdb_encode_mac(mac, mdb_entry, type);
> +	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> +
> +	if (cpu_port)
> +		mdb_entry->cpu_copy = false;
> +	else
> +		mdb_entry->ports &= ~BIT(port->chip_port);
> +	if (!mdb_entry->ports && !mdb_entry->cpu_copy) {
> +		list_del(&mdb_entry->list);
> +		kfree(mdb_entry);
> +		return 0;
> +	}
> +
> +	lan966x_mdb_encode_mac(mac, mdb_entry, type);
> +	return lan966x_mac_cpu_copy(lan966x, 0, mdb_entry->cpu_copy,
> +				    mac, mdb_entry->vid, type);
> +}
> +
> +static struct lan966x_pgid_entry *
> +lan966x_pgid_entry_add(struct lan966x *lan966x, int index, u16 ports)
> +{
> +	struct lan966x_pgid_entry *pgid_entry;
> +
> +	pgid_entry = kzalloc(sizeof(*pgid_entry), GFP_KERNEL);
> +	if (!pgid_entry)
> +		return ERR_PTR(-ENOMEM);
> +
> +	pgid_entry->ports = ports;
> +	pgid_entry->index = index;
> +	refcount_set(&pgid_entry->refcount, 1);
> +
> +	list_add_tail(&pgid_entry->list, &lan966x->pgid_entries);
> +
> +	return pgid_entry;
> +}
> +
> +static struct lan966x_pgid_entry *
> +lan966x_pgid_entry_get(struct lan966x *lan966x,
> +		       struct lan966x_mdb_entry *mdb_entry)
> +{
> +	struct lan966x_pgid_entry *pgid_entry;
> +	int index;
> +
> +	/* Try to find an existing pgid that uses the same ports as the
> +	 * mdb_entry
> +	 */
> +	list_for_each_entry(pgid_entry, &lan966x->pgid_entries, list) {
> +		if (pgid_entry->ports == mdb_entry->ports) {
> +			refcount_inc(&pgid_entry->refcount);
> +			return pgid_entry;
> +		}
> +	}
> +
> +	/* Try to find an empty pgid entry and allocate one in case it finds it,
> +	 * otherwise it means that there are no more resources
> +	 */
> +	for (index = PGID_FIRST; index < PGID_LAST; index++) {
> +		bool used = false;
> +
> +		list_for_each_entry(pgid_entry, &lan966x->pgid_entries, list) {
> +			if (pgid_entry->index == index) {
> +				used = true;
> +				break;
> +			}
> +		}
> +
> +		if (!used)
> +			return lan966x_pgid_entry_add(lan966x, index,
> +						      mdb_entry->ports);
> +	}
> +
> +	return ERR_PTR(-ENOSPC);
> +}
> +
> +static void lan966x_pgid_entry_del(struct lan966x *lan966x,
> +				   struct lan966x_pgid_entry *pgid_entry)
> +{
> +	if (!refcount_dec_and_test(&pgid_entry->refcount))
> +		return;
> +
> +	list_del(&pgid_entry->list);
> +	kfree(pgid_entry);
> +}
> +
> +static int lan966x_mdb_l2_add(struct lan966x_port *port,
> +			      const struct switchdev_obj_port_mdb *mdb,
> +			      enum macaccess_entry_type type)
> +{
> +	bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
> +	struct lan966x *lan966x = port->lan966x;
> +	struct lan966x_pgid_entry *pgid_entry;
> +	struct lan966x_mdb_entry *mdb_entry;
> +	unsigned char mac[ETH_ALEN];
> +
> +	mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
> +	if (!mdb_entry) {
> +		mdb_entry = lan966x_mdb_entry_add(lan966x, mdb);
> +		if (IS_ERR(mdb_entry))
> +			return PTR_ERR(mdb_entry);
> +	} else {
> +		lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
> +		lan966x_mdb_encode_mac(mac, mdb_entry, type);
> +		lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> +	}
> +
> +	if (cpu_port) {
> +		mdb_entry->ports |= BIT(CPU_PORT);
> +		mdb_entry->cpu_copy = true;
> +	} else {
> +		mdb_entry->ports |= BIT(port->chip_port);
> +	}
> +
> +	pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
> +	if (IS_ERR(pgid_entry)) {
> +		list_del(&mdb_entry->list);
> +		kfree(mdb_entry);
> +		return PTR_ERR(pgid_entry);
> +	}
> +	mdb_entry->pgid = pgid_entry;
> +
> +	/* Copy the frame to CPU only if the CPU is the vlan */
> +	if (mdb_entry->cpu_copy) {
> +		if (!lan966x_vlan_cpu_member_cpu_vlan_mask(lan966x,
> +							   mdb_entry->vid))
> +			mdb_entry->ports &= BIT(CPU_PORT);
> +	}
> +
> +	lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
> +		ANA_PGID_PGID,
> +		lan966x, ANA_PGID(pgid_entry->index));
> +
> +	return lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
> +				 mdb_entry->vid, type);
> +}
> +
> +static int lan966x_mdb_l2_del(struct lan966x_port *port,
> +			      const struct switchdev_obj_port_mdb *mdb,
> +			      enum macaccess_entry_type type)
> +{
> +	bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
> +	struct lan966x *lan966x = port->lan966x;
> +	struct lan966x_pgid_entry *pgid_entry;
> +	struct lan966x_mdb_entry *mdb_entry;
> +	unsigned char mac[ETH_ALEN];
> +
> +	mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
> +	if (!mdb_entry)
> +		return -ENOENT;
> +
> +	lan966x_mdb_encode_mac(mac, mdb_entry, type);
> +	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> +	lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
> +
> +	if (cpu_port)
> +		mdb_entry->ports &= ~BIT(CPU_PORT);
> +	else
> +		mdb_entry->ports &= ~BIT(port->chip_port);
> +	if (!mdb_entry->ports) {
> +		list_del(&mdb_entry->list);
> +		kfree(mdb_entry);
> +		return 0;
> +	}
> +
> +	pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
> +	if (IS_ERR(pgid_entry)) {
> +		list_del(&mdb_entry->list);
> +		kfree(mdb_entry);
> +		return PTR_ERR(pgid_entry);
> +	}
> +	mdb_entry->pgid = pgid_entry;
> +
> +	lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
> +		ANA_PGID_PGID,
> +		lan966x, ANA_PGID(pgid_entry->index));
> +
> +	return lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
> +				 mdb_entry->vid, type);
> +}
> +
> +static enum macaccess_entry_type
> +lan966x_mdb_classify(const unsigned char *mac)
> +{
> +	if (mac[0] == 0x01 && mac[1] == 0x00 && mac[2] == 0x5e)
> +		return ENTRYTYPE_MACV4;
> +	if (mac[0] == 0x33 && mac[1] == 0x33)
> +		return ENTRYTYPE_MACV6;
> +	return ENTRYTYPE_LOCKED;
> +}
> +
> +int lan966x_handle_port_mdb_add(struct lan966x_port *port,
> +				const struct switchdev_obj *obj)
> +{
> +	const struct switchdev_obj_port_mdb *mdb = SWITCHDEV_OBJ_PORT_MDB(obj);
> +	enum macaccess_entry_type type;
> +
> +	/* Split the way the entries are added for ipv4/ipv6 and for l2. The
> +	 * reason is that for ipv4/ipv6 it doesn't require to use any pgid
> +	 * entry, while for l2 is required to use pgid entries
> +	 */
> +	type = lan966x_mdb_classify(mdb->addr);
> +	if (type == ENTRYTYPE_MACV4 ||
> +	    type == ENTRYTYPE_MACV6)
> +		return lan966x_mdb_ip_add(port, mdb, type);
> +	else
> +		return lan966x_mdb_l2_add(port, mdb, type);
> +
> +	return 0;
> +}
> +
> +int lan966x_handle_port_mdb_del(struct lan966x_port *port,
> +				const struct switchdev_obj *obj)
> +{
> +	const struct switchdev_obj_port_mdb *mdb = SWITCHDEV_OBJ_PORT_MDB(obj);
> +	enum macaccess_entry_type type;
> +
> +	/* Split the way the entries are removed for ipv4/ipv6 and for l2. The
> +	 * reason is that for ipv4/ipv6 it doesn't require to use any pgid
> +	 * entry, while for l2 is required to use pgid entries
> +	 */
> +	type = lan966x_mdb_classify(mdb->addr);
> +	if (type == ENTRYTYPE_MACV4 ||
> +	    type == ENTRYTYPE_MACV6)
> +		return lan966x_mdb_ip_del(port, mdb, type);
> +	else
> +		return lan966x_mdb_l2_del(port, mdb, type);
> +
> +	return 0;

The "return 0" is dead code. I would expect:

	if (type == ENTRYTYPE_MACV4 || type == ENTRYTYPE_MACV6)
		return lan966x_mdb_ip_del(port, mdb, type);

	return lan966x_mdb_l2_del(port, mdb, type);

> +}
> +
> +static void lan966x_mdb_ip_cpu_copy(struct lan966x *lan966x,
> +				    struct lan966x_mdb_entry *mdb_entry,
> +				    enum macaccess_entry_type type)
> +{
> +	unsigned char mac[ETH_ALEN];
> +
> +	lan966x_mdb_encode_mac(mac, mdb_entry, type);
> +	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> +	lan966x_mac_cpu_copy(lan966x, 0, true, mac, mdb_entry->vid, type);
> +}
> +
> +static void lan966x_mdb_l2_cpu_copy(struct lan966x *lan966x,
> +				    struct lan966x_mdb_entry *mdb_entry,
> +				    enum macaccess_entry_type type)
> +{
> +	struct lan966x_pgid_entry *pgid_entry;
> +	unsigned char mac[ETH_ALEN];
> +
> +	lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
> +	lan966x_mdb_encode_mac(mac, mdb_entry, type);
> +	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> +
> +	mdb_entry->ports |= BIT(CPU_PORT);
> +
> +	pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
> +	if (IS_ERR(pgid_entry))
> +		return;
> +
> +	mdb_entry->pgid = pgid_entry;
> +
> +	lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
> +		ANA_PGID_PGID,
> +		lan966x, ANA_PGID(pgid_entry->index));
> +
> +	lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
> +			  mdb_entry->vid, type);
> +}
> +
> +void lan966x_mdb_write_entries(struct lan966x *lan966x, u16 vid)
> +{
> +	struct lan966x_mdb_entry *mdb_entry;
> +	enum macaccess_entry_type type;
> +
> +	list_for_each_entry(mdb_entry, &lan966x->mdb_entries, list) {
> +		if (mdb_entry->vid != vid || !mdb_entry->cpu_copy)
> +			continue;
> +
> +		type = lan966x_mdb_classify(mdb_entry->mac);
> +		if (type == ENTRYTYPE_MACV4 ||
> +		    type == ENTRYTYPE_MACV6)
> +			lan966x_mdb_ip_cpu_copy(lan966x, mdb_entry, type);
> +		else
> +			lan966x_mdb_l2_cpu_copy(lan966x, mdb_entry, type);
> +	}
> +}
> +
> +static void lan966x_mdb_ip_cpu_remove(struct lan966x *lan966x,
> +				      struct lan966x_mdb_entry *mdb_entry,
> +				      enum macaccess_entry_type type)
> +{
> +	unsigned char mac[ETH_ALEN];
> +
> +	lan966x_mdb_encode_mac(mac, mdb_entry, type);
> +	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> +	lan966x_mac_cpu_copy(lan966x, 0, false, mac, mdb_entry->vid, type);
> +}
> +
> +static void lan966x_mdb_l2_cpu_remove(struct lan966x *lan966x,
> +				      struct lan966x_mdb_entry *mdb_entry,
> +				      enum macaccess_entry_type type)
> +{
> +	struct lan966x_pgid_entry *pgid_entry;
> +	unsigned char mac[ETH_ALEN];
> +
> +	lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
> +	lan966x_mdb_encode_mac(mac, mdb_entry, type);
> +	lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> +
> +	mdb_entry->ports &= ~BIT(CPU_PORT);
> +
> +	pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
> +	if (IS_ERR(pgid_entry))
> +		return;
> +
> +	mdb_entry->pgid = pgid_entry;
> +
> +	lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
> +		ANA_PGID_PGID,
> +		lan966x, ANA_PGID(pgid_entry->index));
> +
> +	lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
> +			  mdb_entry->vid, type);
> +}
> +
> +void lan966x_mdb_erase_entries(struct lan966x *lan966x, u16 vid)
> +{
> +	struct lan966x_mdb_entry *mdb_entry;
> +	enum macaccess_entry_type type;
> +
> +	list_for_each_entry(mdb_entry, &lan966x->mdb_entries, list) {
> +		if (mdb_entry->vid != vid || !mdb_entry->cpu_copy)
> +			continue;
> +
> +		type = lan966x_mdb_classify(mdb_entry->mac);
> +		if (type == ENTRYTYPE_MACV4 ||
> +		    type == ENTRYTYPE_MACV6)
> +			lan966x_mdb_ip_cpu_remove(lan966x, mdb_entry, type);
> +		else
> +			lan966x_mdb_l2_cpu_remove(lan966x, mdb_entry, type);
> +	}
> +}
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
> index deb3dd5be67a..7de55f6a4da8 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
> @@ -438,6 +438,10 @@ static int lan966x_handle_port_obj_add(struct net_device *dev, const void *ctx,
>  	case SWITCHDEV_OBJ_ID_PORT_VLAN:
>  		err = lan966x_handle_port_vlan_add(port, obj);
>  		break;
> +	case SWITCHDEV_OBJ_ID_PORT_MDB:
> +	case SWITCHDEV_OBJ_ID_HOST_MDB:
> +		err = lan966x_handle_port_mdb_add(port, obj);
> +		break;
>  	default:
>  		err = -EOPNOTSUPP;
>  		break;
> @@ -473,6 +477,10 @@ static int lan966x_handle_port_obj_del(struct net_device *dev, const void *ctx,
>  	case SWITCHDEV_OBJ_ID_PORT_VLAN:
>  		err = lan966x_handle_port_vlan_del(port, obj);
>  		break;
> +	case SWITCHDEV_OBJ_ID_PORT_MDB:
> +	case SWITCHDEV_OBJ_ID_HOST_MDB:

The HOST_MDB switchdev events are replicated per the number of ports in
the bridge. So I would expect that you keep refcounts on them, otherwise
the first deletion of such an element would trigger the removal of the
entry from hardware even though it's still in use.

> +		err = lan966x_handle_port_mdb_del(port, obj);
> +		break;
>  	default:
>  		err = -EOPNOTSUPP;
>  		break;
> diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c b/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
> index 057f48ddf22c..8d7260cd7da9 100644
> --- a/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
> +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
> @@ -219,6 +219,7 @@ void lan966x_vlan_port_add_vlan(struct lan966x_port *port,
>  	if (lan966x_vlan_cpu_member_cpu_vlan_mask(lan966x, vid)) {
>  		lan966x_vlan_cpu_add_vlan_mask(lan966x, vid);
>  		lan966x_fdb_write_entries(lan966x, vid);
> +		lan966x_mdb_write_entries(lan966x, vid);
>  	}
>  
>  	lan966x_vlan_port_set_vid(port, vid, pvid, untagged);
> @@ -241,6 +242,7 @@ void lan966x_vlan_port_del_vlan(struct lan966x_port *port, u16 vid)
>  	if (!lan966x_vlan_port_any_vlan_mask(lan966x, vid)) {
>  		lan966x_vlan_cpu_del_vlan_mask(lan966x, vid);
>  		lan966x_fdb_erase_entries(lan966x, vid);
> +		lan966x_mdb_erase_entries(lan966x, vid);
>  	}
>  }
>  
> @@ -254,8 +256,10 @@ void lan966x_vlan_cpu_add_vlan(struct lan966x *lan966x, u16 vid)
>  	 * information so when a front port is added then it would add also the
>  	 * CPU port.
>  	 */
> -	if (lan966x_vlan_port_any_vlan_mask(lan966x, vid))
> +	if (lan966x_vlan_port_any_vlan_mask(lan966x, vid)) {
>  		lan966x_vlan_cpu_add_vlan_mask(lan966x, vid);
> +		lan966x_mdb_write_entries(lan966x, vid);
> +	}
>  
>  	lan966x_vlan_cpu_add_cpu_vlan_mask(lan966x, vid);
>  	lan966x_fdb_write_entries(lan966x, vid);
> @@ -267,6 +271,7 @@ void lan966x_vlan_cpu_del_vlan(struct lan966x *lan966x, u16 vid)
>  	lan966x_vlan_cpu_del_cpu_vlan_mask(lan966x, vid);
>  	lan966x_vlan_cpu_del_vlan_mask(lan966x, vid);
>  	lan966x_fdb_erase_entries(lan966x, vid);
> +	lan966x_mdb_erase_entries(lan966x, vid);
>  }
>  
>  void lan966x_vlan_init(struct lan966x *lan966x)
> -- 
> 2.33.0
>

Re: [PATCH net-next 1/3] net: lan966x: Add function lan966x_mac_cpu_copy()

[Horatiu Vultur]

The 01/03/2022 14:27, Vladimir Oltean wrote:
> 
> On Mon, Jan 03, 2022 at 02:10:37PM +0100, Horatiu Vultur wrote:
> > Extend mac functionality with the function lan966x_mac_cpu_copy. This
> > function adds an entry in the MAC table where a frame is copy to the CPU
> 
> s/copy/copied/
> 
> > but also can be forward on the front ports.
> 
> s/forward/forwarded/
> 
> > This functionality is needed for mdb support. In case the CPU and some
> > of the front ports subscribe to an IP multicast address.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> >  .../ethernet/microchip/lan966x/lan966x_mac.c  | 27 ++++++++++++++++---
> >  .../ethernet/microchip/lan966x/lan966x_main.h |  5 ++++
> >  .../ethernet/microchip/lan966x/lan966x_regs.h |  6 +++++
> >  3 files changed, 34 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
> > index efadb8d326cc..ae3a7a10e0d6 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_mac.c
> > @@ -68,16 +68,18 @@ static void lan966x_mac_select(struct lan966x *lan966x,
> >       lan_wr(mach, lan966x, ANA_MACHDATA);
> >  }
> >
> > -int lan966x_mac_learn(struct lan966x *lan966x, int port,
> > -                   const unsigned char mac[ETH_ALEN],
> > -                   unsigned int vid,
> > -                   enum macaccess_entry_type type)
> > +static int lan966x_mac_learn_impl(struct lan966x *lan966x, int port,
> > +                               bool cpu_copy,
> > +                               const unsigned char mac[ETH_ALEN],
> > +                               unsigned int vid,
> > +                               enum macaccess_entry_type type)
> 
> In the kernel, the __lan966x_mac_learn naming scheme seems to be more
> popular than _impl.
> 
> Also, may I suggest that the "int port" argument may be better named
> "int pgid"?

Yes, I will rename it and change the argument name.

> 
> >  {
> >       lan966x_mac_select(lan966x, mac, vid);
> >
> >       /* Issue a write command */
> >       lan_wr(ANA_MACACCESS_VALID_SET(1) |
> >              ANA_MACACCESS_CHANGE2SW_SET(0) |
> > +            ANA_MACACCESS_MAC_CPU_COPY_SET(cpu_copy) |
> >              ANA_MACACCESS_DEST_IDX_SET(port) |
> >              ANA_MACACCESS_ENTRYTYPE_SET(type) |
> >              ANA_MACACCESS_MAC_TABLE_CMD_SET(MACACCESS_CMD_LEARN),
> > @@ -86,6 +88,23 @@ int lan966x_mac_learn(struct lan966x *lan966x, int port,
> >       return lan966x_mac_wait_for_completion(lan966x);
> >  }
> >
> > +int lan966x_mac_cpu_copy(struct lan966x *lan966x, int port,
> > +                      bool cpu_copy,
> > +                      const unsigned char mac[ETH_ALEN],
> > +                      unsigned int vid,
> > +                      enum macaccess_entry_type type)
> 
> This doesn't seem to use the "port" argument from any of its call sites.
> It is always 0. What would it even mean?

When adding MAC table entries for IPv4/IPv6 then the port which is the
DEST_IDX needs to be 0. It should not point to a PGID entry because the
destination ports are encoded in the MAC address.

> 
> > +{
> > +     return lan966x_mac_learn_impl(lan966x, port, cpu_copy, mac, vid, type);
> > +}
> > +
> > +int lan966x_mac_learn(struct lan966x *lan966x, int port,
> > +                   const unsigned char mac[ETH_ALEN],
> > +                   unsigned int vid,
> > +                   enum macaccess_entry_type type)
> > +{
> > +     return lan966x_mac_learn_impl(lan966x, port, false, mac, vid, type);
> 
> If you call lan966x_mac_cpu_copy() on an address and then
> lan966x_mac_learn() on the same address but on an external port, how
> does that work (why doesn't the "false" here overwrite the cpu_copy in
> the previous command, breaking the copy-to-CPU feature)?

Then you will overwrite the cpu_copy so the frames will not reach the
CPU anymore.
But you should not do that. The function lan966x_mac_cpu_copy() should be
used for IPv4/IPv6 and lan966x_mac_learn() for the other types.

Maybe the function lan966x_mac_cpu_copy() is too generic. It should be
something like lan966x_mac_ipv4(), lan966x_mac_ipv6() and these functions
will call __lan966x_mac_learn with the correct parameters. Also I can
add a WARN_ON(...) inside lan966x_mac_learn not to be used with the
IPv4/IPv6 types.

> 
> > +}
> > +
> >  int lan966x_mac_forget(struct lan966x *lan966x,
> >                      const unsigned char mac[ETH_ALEN],
> >                      unsigned int vid,
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > index c399b1256edc..a7a2a3537036 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > @@ -157,6 +157,11 @@ int lan966x_port_pcs_set(struct lan966x_port *port,
> >                        struct lan966x_port_config *config);
> >  void lan966x_port_init(struct lan966x_port *port);
> >
> > +int lan966x_mac_cpu_copy(struct lan966x *lan966x, int port,
> > +                      bool cpu_copy,
> > +                      const unsigned char mac[ETH_ALEN],
> > +                      unsigned int vid,
> > +                      enum macaccess_entry_type type);
> >  int lan966x_mac_learn(struct lan966x *lan966x, int port,
> >                     const unsigned char mac[ETH_ALEN],
> >                     unsigned int vid,
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h b/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h
> > index a13c469e139a..797560172aca 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_regs.h
> > @@ -169,6 +169,12 @@ enum lan966x_target {
> >  #define ANA_MACACCESS_CHANGE2SW_GET(x)\
> >       FIELD_GET(ANA_MACACCESS_CHANGE2SW, x)
> >
> > +#define ANA_MACACCESS_MAC_CPU_COPY               BIT(16)
> > +#define ANA_MACACCESS_MAC_CPU_COPY_SET(x)\
> > +     FIELD_PREP(ANA_MACACCESS_MAC_CPU_COPY, x)
> > +#define ANA_MACACCESS_MAC_CPU_COPY_GET(x)\
> > +     FIELD_GET(ANA_MACACCESS_MAC_CPU_COPY, x)
> > +
> >  #define ANA_MACACCESS_VALID                      BIT(12)
> >  #define ANA_MACACCESS_VALID_SET(x)\
> >       FIELD_PREP(ANA_MACACCESS_VALID, x)
> > --
> > 2.33.0
> >

-- 
/Horatiu

Re: [PATCH net-next 1/3] net: lan966x: Add function lan966x_mac_cpu_copy()

[Vladimir Oltean]

On Mon, Jan 03, 2022 at 04:39:10PM +0100, Horatiu Vultur wrote:
> > > +int lan966x_mac_learn(struct lan966x *lan966x, int port,
> > > +                   const unsigned char mac[ETH_ALEN],
> > > +                   unsigned int vid,
> > > +                   enum macaccess_entry_type type)
> > > +{
> > > +     return lan966x_mac_learn_impl(lan966x, port, false, mac, vid, type);
> > 
> > If you call lan966x_mac_cpu_copy() on an address and then
> > lan966x_mac_learn() on the same address but on an external port, how
> > does that work (why doesn't the "false" here overwrite the cpu_copy in
> > the previous command, breaking the copy-to-CPU feature)?
> 
> Then you will overwrite the cpu_copy so the frames will not reach the
> CPU anymore.
> But you should not do that. The function lan966x_mac_cpu_copy() should be
> used for IPv4/IPv6 and lan966x_mac_learn() for the other types.
> 
> Maybe the function lan966x_mac_cpu_copy() is too generic. It should be
> something like lan966x_mac_ipv4(), lan966x_mac_ipv6() and these functions
> will call __lan966x_mac_learn with the correct parameters. Also I can
> add a WARN_ON(...) inside lan966x_mac_learn not to be used with the
> IPv4/IPv6 types.

The intended usage pattern isn't clear at all in the current series.

Re: [PATCH net-next 3/3] net: lan966x: Extend switchdev with mdb support

[Horatiu Vultur]

The 01/03/2022 14:43, Vladimir Oltean wrote:
> 
> On Mon, Jan 03, 2022 at 02:10:39PM +0100, Horatiu Vultur wrote:
> > Extend lan966x driver with mdb support by implementing the switchdev
> > calls: SWITCHDEV_OBJ_ID_PORT_MDB and SWITCHDEV_OBJ_ID_HOST_MDB.
> > It is allowed to add both ipv4/ipv6 entries and l2 entries. To add
> > ipv4/ipv6 entries is not required to use the PGID table while for l2
> > entries it is required. The PGID table is much smaller than MAC table
> > so only fewer l2 entries can be added.
> >
> > Signed-off-by: Horatiu Vultur <horatiu.vultur@microchip.com>
> > ---
> >  .../net/ethernet/microchip/lan966x/Makefile   |   2 +-
> >  .../ethernet/microchip/lan966x/lan966x_main.c |   2 +
> >  .../ethernet/microchip/lan966x/lan966x_main.h |  13 +
> >  .../ethernet/microchip/lan966x/lan966x_mdb.c  | 500 ++++++++++++++++++
> >  .../microchip/lan966x/lan966x_switchdev.c     |   8 +
> >  .../ethernet/microchip/lan966x/lan966x_vlan.c |   7 +-
> >  6 files changed, 530 insertions(+), 2 deletions(-)
> >  create mode 100644 drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c
> >
> > diff --git a/drivers/net/ethernet/microchip/lan966x/Makefile b/drivers/net/ethernet/microchip/lan966x/Makefile
> > index ec1a1fa8b0d5..040cfff9f577 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/Makefile
> > +++ b/drivers/net/ethernet/microchip/lan966x/Makefile
> > @@ -7,4 +7,4 @@ obj-$(CONFIG_LAN966X_SWITCH) += lan966x-switch.o
> >
> >  lan966x-switch-objs  := lan966x_main.o lan966x_phylink.o lan966x_port.o \
> >                       lan966x_mac.o lan966x_ethtool.o lan966x_switchdev.o \
> > -                     lan966x_vlan.o lan966x_fdb.o
> > +                     lan966x_vlan.o lan966x_fdb.o lan966x_mdb.o
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > index 2c6bf7b0afdf..2cb70da63db3 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.c
> > @@ -926,6 +926,7 @@ static int lan966x_probe(struct platform_device *pdev)
> >               lan966x_port_init(lan966x->ports[p]);
> >       }
> >
> > +     lan966x_mdb_init(lan966x);
> >       err = lan966x_fdb_init(lan966x);
> >       if (err)
> >               goto cleanup_ports;
> > @@ -955,6 +956,7 @@ static int lan966x_remove(struct platform_device *pdev)
> >       mutex_destroy(&lan966x->stats_lock);
> >
> >       lan966x_mac_purge_entries(lan966x);
> > +     lan966x_mdb_deinit(lan966x);
> >       lan966x_fdb_deinit(lan966x);
> >
> >       return 0;
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > index 49ce6a04ca40..76f0b5446b2e 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_main.h
> > @@ -107,6 +107,10 @@ struct lan966x {
> >       /* worqueue for fdb */
> >       struct workqueue_struct *fdb_work;
> >       struct list_head fdb_entries;
> > +
> > +     /* mdb */
> > +     struct list_head mdb_entries;
> > +     struct list_head pgid_entries;
> >  };
> >
> >  struct lan966x_port_config {
> > @@ -213,6 +217,15 @@ int lan966x_handle_fdb(struct net_device *dev,
> >                      unsigned long event, const void *ctx,
> >                      const struct switchdev_notifier_fdb_info *fdb_info);
> >
> > +void lan966x_mdb_init(struct lan966x *lan966x);
> > +void lan966x_mdb_deinit(struct lan966x *lan966x);
> > +int lan966x_handle_port_mdb_add(struct lan966x_port *port,
> > +                             const struct switchdev_obj *obj);
> > +int lan966x_handle_port_mdb_del(struct lan966x_port *port,
> > +                             const struct switchdev_obj *obj);
> > +void lan966x_mdb_erase_entries(struct lan966x *lan966x, u16 vid);
> > +void lan966x_mdb_write_entries(struct lan966x *lan966x, u16 vid);
> > +
> >  static inline void __iomem *lan_addr(void __iomem *base[],
> >                                    int id, int tinst, int tcnt,
> >                                    int gbase, int ginst,
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c b/drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c
> > new file mode 100644
> > index 000000000000..4fd8b06a56c1
> > --- /dev/null
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_mdb.c
> > @@ -0,0 +1,500 @@
> > +// SPDX-License-Identifier: GPL-2.0+
> > +
> > +#include <net/switchdev.h>
> > +
> > +#include "lan966x_main.h"
> > +
> > +struct lan966x_pgid_entry {
> > +     struct list_head list;
> > +     int index;
> > +     refcount_t refcount;
> > +     u16 ports;
> > +};
> > +
> > +struct lan966x_mdb_entry {
> > +     struct list_head list;
> > +     unsigned char mac[ETH_ALEN];
> > +     u16 vid;
> > +     u16 ports;
> > +     struct lan966x_pgid_entry *pgid;
> > +     bool cpu_copy;
> > +};
> > +
> > +void lan966x_mdb_init(struct lan966x *lan966x)
> > +{
> > +     INIT_LIST_HEAD(&lan966x->mdb_entries);
> > +     INIT_LIST_HEAD(&lan966x->pgid_entries);
> > +}
> > +
> > +static void lan966x_mdb_purge_mdb_entries(struct lan966x *lan966x)
> > +{
> > +     struct lan966x_mdb_entry *mdb_entry, *tmp;
> > +
> > +     list_for_each_entry_safe(mdb_entry, tmp, &lan966x->mdb_entries, list) {
> > +             list_del(&mdb_entry->list);
> > +             kfree(mdb_entry);
> > +     }
> > +}
> > +
> > +static void lan966x_mdb_purge_pgid_entries(struct lan966x *lan966x)
> > +{
> > +     struct lan966x_pgid_entry *pgid_entry, *tmp;
> > +
> > +     list_for_each_entry_safe(pgid_entry, tmp, &lan966x->pgid_entries, list) {
> > +             list_del(&pgid_entry->list);
> > +             kfree(pgid_entry);
> > +     }
> > +}
> > +
> > +void lan966x_mdb_deinit(struct lan966x *lan966x)
> > +{
> > +     lan966x_mdb_purge_mdb_entries(lan966x);
> > +     lan966x_mdb_purge_pgid_entries(lan966x);
> > +}
> > +
> > +static struct lan966x_mdb_entry *
> > +lan966x_mdb_entry_get(struct lan966x *lan966x,
> > +                   const unsigned char *mac,
> > +                   u16 vid)
> > +{
> > +     struct lan966x_mdb_entry *mdb_entry;
> > +
> > +     list_for_each_entry(mdb_entry, &lan966x->mdb_entries, list) {
> > +             if (ether_addr_equal(mdb_entry->mac, mac) &&
> > +                 mdb_entry->vid == vid)
> > +                     return mdb_entry;
> > +     }
> > +
> > +     return NULL;
> > +}
> > +
> > +static struct lan966x_mdb_entry *
> > +lan966x_mdb_entry_add(struct lan966x *lan966x,
> > +                   const struct switchdev_obj_port_mdb *mdb)
> > +{
> > +     struct lan966x_mdb_entry *mdb_entry;
> > +
> > +     mdb_entry = kzalloc(sizeof(*mdb_entry), GFP_KERNEL);
> > +     if (!mdb_entry)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     ether_addr_copy(mdb_entry->mac, mdb->addr);
> > +     mdb_entry->vid = mdb->vid;
> > +
> > +     list_add_tail(&mdb_entry->list, &lan966x->mdb_entries);
> > +
> > +     return mdb_entry;
> > +}
> > +
> > +static void lan966x_mdb_encode_mac(unsigned char *mac,
> > +                                struct lan966x_mdb_entry *mdb_entry,
> > +                                enum macaccess_entry_type type)
> > +{
> > +     ether_addr_copy(mac, mdb_entry->mac);
> > +
> > +     if (type == ENTRYTYPE_MACV4) {
> > +             mac[0] = 0;
> > +             mac[1] = mdb_entry->ports >> 8;
> > +             mac[2] = mdb_entry->ports & 0xff;
> > +     } else if (type == ENTRYTYPE_MACV6) {
> > +             mac[0] = mdb_entry->ports >> 8;
> > +             mac[1] = mdb_entry->ports & 0xff;
> > +     }
> > +}
> > +
> > +static int lan966x_mdb_ip_add(struct lan966x_port *port,
> > +                           const struct switchdev_obj_port_mdb *mdb,
> > +                           enum macaccess_entry_type type)
> > +{
> > +     bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
> > +     struct lan966x *lan966x = port->lan966x;
> > +     struct lan966x_mdb_entry *mdb_entry;
> > +     unsigned char mac[ETH_ALEN];
> > +     bool cpu_copy = false;
> > +
> > +     mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
> > +     if (!mdb_entry) {
> > +             mdb_entry = lan966x_mdb_entry_add(lan966x, mdb);
> > +             if (IS_ERR(mdb_entry))
> > +                     return PTR_ERR(mdb_entry);
> > +     } else {
> > +             lan966x_mdb_encode_mac(mac, mdb_entry, type);
> > +             lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> > +     }
> > +
> > +     if (cpu_port)
> > +             mdb_entry->cpu_copy = true;
> > +     else
> > +             mdb_entry->ports |= BIT(port->chip_port);
> > +
> > +     /* Copy the frame to CPU only if the CPU is the vlan */
> 
> s/is the vlan/is in the VLAN/
> 
> > +     if (mdb_entry->cpu_copy) {
> > +             if (lan966x_vlan_cpu_member_cpu_vlan_mask(lan966x,
> > +                                                       mdb_entry->vid))
> > +                     cpu_copy = true;
> > +     }
> 
> I find it slightly simpler to squash the two if blocks into a single one.
> 
>         if (lan966x_vlan_cpu_member_cpu_vlan_mask(lan966x, mdb_entry->vid) &&
>             mdb_entry->cpu_copy)
>                 cpu_copy = true;
> 
> > +
> > +     lan966x_mdb_encode_mac(mac, mdb_entry, type);
> > +     return lan966x_mac_cpu_copy(lan966x, 0, cpu_copy,
> > +                                 mac, mdb_entry->vid, type);
> > +}
> > +
> > +static int lan966x_mdb_ip_del(struct lan966x_port *port,
> > +                           const struct switchdev_obj_port_mdb *mdb,
> > +                           enum macaccess_entry_type type)
> > +{
> > +     bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
> > +     struct lan966x *lan966x = port->lan966x;
> > +     struct lan966x_mdb_entry *mdb_entry;
> > +     unsigned char mac[ETH_ALEN];
> > +
> > +     mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
> > +     if (!mdb_entry) {
> > +             /* If the CPU originted this and the entry was not found, it is
> 
> s/originted/originated/
> 
> > +              * because already another port has removed the entry
> > +              */
> > +             if (cpu_port)
> > +                     return 0;
> 
> Could you explain again why this is normal? If another port has removed
> the entry, how is the address copied to the CPU any longer?

The cpu_port is set only when the entry is deleted by the bridge. So then
all the ports under the bridge will be notified about this. So when the first
port is notified it would delete the entry and then when the second port is
notified, it would try to delete the entry which is already deleted by
the first port. That is the reason why it returns 0 and not an error.

> 
> > +             return -ENOENT;
> > +     }
> > +
> > +     lan966x_mdb_encode_mac(mac, mdb_entry, type);
> > +     lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> > +
> > +     if (cpu_port)
> > +             mdb_entry->cpu_copy = false;
> > +     else
> > +             mdb_entry->ports &= ~BIT(port->chip_port);
> > +     if (!mdb_entry->ports && !mdb_entry->cpu_copy) {
> > +             list_del(&mdb_entry->list);
> > +             kfree(mdb_entry);
> > +             return 0;
> > +     }
> > +
> > +     lan966x_mdb_encode_mac(mac, mdb_entry, type);
> > +     return lan966x_mac_cpu_copy(lan966x, 0, mdb_entry->cpu_copy,
> > +                                 mac, mdb_entry->vid, type);
> > +}
> > +
> > +static struct lan966x_pgid_entry *
> > +lan966x_pgid_entry_add(struct lan966x *lan966x, int index, u16 ports)
> > +{
> > +     struct lan966x_pgid_entry *pgid_entry;
> > +
> > +     pgid_entry = kzalloc(sizeof(*pgid_entry), GFP_KERNEL);
> > +     if (!pgid_entry)
> > +             return ERR_PTR(-ENOMEM);
> > +
> > +     pgid_entry->ports = ports;
> > +     pgid_entry->index = index;
> > +     refcount_set(&pgid_entry->refcount, 1);
> > +
> > +     list_add_tail(&pgid_entry->list, &lan966x->pgid_entries);
> > +
> > +     return pgid_entry;
> > +}
> > +
> > +static struct lan966x_pgid_entry *
> > +lan966x_pgid_entry_get(struct lan966x *lan966x,
> > +                    struct lan966x_mdb_entry *mdb_entry)
> > +{
> > +     struct lan966x_pgid_entry *pgid_entry;
> > +     int index;
> > +
> > +     /* Try to find an existing pgid that uses the same ports as the
> > +      * mdb_entry
> > +      */
> > +     list_for_each_entry(pgid_entry, &lan966x->pgid_entries, list) {
> > +             if (pgid_entry->ports == mdb_entry->ports) {
> > +                     refcount_inc(&pgid_entry->refcount);
> > +                     return pgid_entry;
> > +             }
> > +     }
> > +
> > +     /* Try to find an empty pgid entry and allocate one in case it finds it,
> > +      * otherwise it means that there are no more resources
> > +      */
> > +     for (index = PGID_FIRST; index < PGID_LAST; index++) {
> > +             bool used = false;
> > +
> > +             list_for_each_entry(pgid_entry, &lan966x->pgid_entries, list) {
> > +                     if (pgid_entry->index == index) {
> > +                             used = true;
> > +                             break;
> > +                     }
> > +             }
> > +
> > +             if (!used)
> > +                     return lan966x_pgid_entry_add(lan966x, index,
> > +                                                   mdb_entry->ports);
> > +     }
> > +
> > +     return ERR_PTR(-ENOSPC);
> > +}
> > +
> > +static void lan966x_pgid_entry_del(struct lan966x *lan966x,
> > +                                struct lan966x_pgid_entry *pgid_entry)
> > +{
> > +     if (!refcount_dec_and_test(&pgid_entry->refcount))
> > +             return;
> > +
> > +     list_del(&pgid_entry->list);
> > +     kfree(pgid_entry);
> > +}
> > +
> > +static int lan966x_mdb_l2_add(struct lan966x_port *port,
> > +                           const struct switchdev_obj_port_mdb *mdb,
> > +                           enum macaccess_entry_type type)
> > +{
> > +     bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
> > +     struct lan966x *lan966x = port->lan966x;
> > +     struct lan966x_pgid_entry *pgid_entry;
> > +     struct lan966x_mdb_entry *mdb_entry;
> > +     unsigned char mac[ETH_ALEN];
> > +
> > +     mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
> > +     if (!mdb_entry) {
> > +             mdb_entry = lan966x_mdb_entry_add(lan966x, mdb);
> > +             if (IS_ERR(mdb_entry))
> > +                     return PTR_ERR(mdb_entry);
> > +     } else {
> > +             lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
> > +             lan966x_mdb_encode_mac(mac, mdb_entry, type);
> > +             lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> > +     }
> > +
> > +     if (cpu_port) {
> > +             mdb_entry->ports |= BIT(CPU_PORT);
> > +             mdb_entry->cpu_copy = true;
> > +     } else {
> > +             mdb_entry->ports |= BIT(port->chip_port);
> > +     }
> > +
> > +     pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
> > +     if (IS_ERR(pgid_entry)) {
> > +             list_del(&mdb_entry->list);
> > +             kfree(mdb_entry);
> > +             return PTR_ERR(pgid_entry);
> > +     }
> > +     mdb_entry->pgid = pgid_entry;
> > +
> > +     /* Copy the frame to CPU only if the CPU is the vlan */
> > +     if (mdb_entry->cpu_copy) {
> > +             if (!lan966x_vlan_cpu_member_cpu_vlan_mask(lan966x,
> > +                                                        mdb_entry->vid))
> > +                     mdb_entry->ports &= BIT(CPU_PORT);
> > +     }
> > +
> > +     lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
> > +             ANA_PGID_PGID,
> > +             lan966x, ANA_PGID(pgid_entry->index));
> > +
> > +     return lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
> > +                              mdb_entry->vid, type);
> > +}
> > +
> > +static int lan966x_mdb_l2_del(struct lan966x_port *port,
> > +                           const struct switchdev_obj_port_mdb *mdb,
> > +                           enum macaccess_entry_type type)
> > +{
> > +     bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
> > +     struct lan966x *lan966x = port->lan966x;
> > +     struct lan966x_pgid_entry *pgid_entry;
> > +     struct lan966x_mdb_entry *mdb_entry;
> > +     unsigned char mac[ETH_ALEN];
> > +
> > +     mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
> > +     if (!mdb_entry)
> > +             return -ENOENT;
> > +
> > +     lan966x_mdb_encode_mac(mac, mdb_entry, type);
> > +     lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> > +     lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
> > +
> > +     if (cpu_port)
> > +             mdb_entry->ports &= ~BIT(CPU_PORT);
> > +     else
> > +             mdb_entry->ports &= ~BIT(port->chip_port);
> > +     if (!mdb_entry->ports) {
> > +             list_del(&mdb_entry->list);
> > +             kfree(mdb_entry);
> > +             return 0;
> > +     }
> > +
> > +     pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
> > +     if (IS_ERR(pgid_entry)) {
> > +             list_del(&mdb_entry->list);
> > +             kfree(mdb_entry);
> > +             return PTR_ERR(pgid_entry);
> > +     }
> > +     mdb_entry->pgid = pgid_entry;
> > +
> > +     lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
> > +             ANA_PGID_PGID,
> > +             lan966x, ANA_PGID(pgid_entry->index));
> > +
> > +     return lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
> > +                              mdb_entry->vid, type);
> > +}
> > +
> > +static enum macaccess_entry_type
> > +lan966x_mdb_classify(const unsigned char *mac)
> > +{
> > +     if (mac[0] == 0x01 && mac[1] == 0x00 && mac[2] == 0x5e)
> > +             return ENTRYTYPE_MACV4;
> > +     if (mac[0] == 0x33 && mac[1] == 0x33)
> > +             return ENTRYTYPE_MACV6;
> > +     return ENTRYTYPE_LOCKED;
> > +}
> > +
> > +int lan966x_handle_port_mdb_add(struct lan966x_port *port,
> > +                             const struct switchdev_obj *obj)
> > +{
> > +     const struct switchdev_obj_port_mdb *mdb = SWITCHDEV_OBJ_PORT_MDB(obj);
> > +     enum macaccess_entry_type type;
> > +
> > +     /* Split the way the entries are added for ipv4/ipv6 and for l2. The
> > +      * reason is that for ipv4/ipv6 it doesn't require to use any pgid
> > +      * entry, while for l2 is required to use pgid entries
> > +      */
> > +     type = lan966x_mdb_classify(mdb->addr);
> > +     if (type == ENTRYTYPE_MACV4 ||
> > +         type == ENTRYTYPE_MACV6)
> > +             return lan966x_mdb_ip_add(port, mdb, type);
> > +     else
> > +             return lan966x_mdb_l2_add(port, mdb, type);
> > +
> > +     return 0;
> > +}
> > +
> > +int lan966x_handle_port_mdb_del(struct lan966x_port *port,
> > +                             const struct switchdev_obj *obj)
> > +{
> > +     const struct switchdev_obj_port_mdb *mdb = SWITCHDEV_OBJ_PORT_MDB(obj);
> > +     enum macaccess_entry_type type;
> > +
> > +     /* Split the way the entries are removed for ipv4/ipv6 and for l2. The
> > +      * reason is that for ipv4/ipv6 it doesn't require to use any pgid
> > +      * entry, while for l2 is required to use pgid entries
> > +      */
> > +     type = lan966x_mdb_classify(mdb->addr);
> > +     if (type == ENTRYTYPE_MACV4 ||
> > +         type == ENTRYTYPE_MACV6)
> > +             return lan966x_mdb_ip_del(port, mdb, type);
> > +     else
> > +             return lan966x_mdb_l2_del(port, mdb, type);
> > +
> > +     return 0;
> 
> The "return 0" is dead code. I would expect:
> 
>         if (type == ENTRYTYPE_MACV4 || type == ENTRYTYPE_MACV6)
>                 return lan966x_mdb_ip_del(port, mdb, type);
> 
>         return lan966x_mdb_l2_del(port, mdb, type);
> 
> > +}
> > +
> > +static void lan966x_mdb_ip_cpu_copy(struct lan966x *lan966x,
> > +                                 struct lan966x_mdb_entry *mdb_entry,
> > +                                 enum macaccess_entry_type type)
> > +{
> > +     unsigned char mac[ETH_ALEN];
> > +
> > +     lan966x_mdb_encode_mac(mac, mdb_entry, type);
> > +     lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> > +     lan966x_mac_cpu_copy(lan966x, 0, true, mac, mdb_entry->vid, type);
> > +}
> > +
> > +static void lan966x_mdb_l2_cpu_copy(struct lan966x *lan966x,
> > +                                 struct lan966x_mdb_entry *mdb_entry,
> > +                                 enum macaccess_entry_type type)
> > +{
> > +     struct lan966x_pgid_entry *pgid_entry;
> > +     unsigned char mac[ETH_ALEN];
> > +
> > +     lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
> > +     lan966x_mdb_encode_mac(mac, mdb_entry, type);
> > +     lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> > +
> > +     mdb_entry->ports |= BIT(CPU_PORT);
> > +
> > +     pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
> > +     if (IS_ERR(pgid_entry))
> > +             return;
> > +
> > +     mdb_entry->pgid = pgid_entry;
> > +
> > +     lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
> > +             ANA_PGID_PGID,
> > +             lan966x, ANA_PGID(pgid_entry->index));
> > +
> > +     lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
> > +                       mdb_entry->vid, type);
> > +}
> > +
> > +void lan966x_mdb_write_entries(struct lan966x *lan966x, u16 vid)
> > +{
> > +     struct lan966x_mdb_entry *mdb_entry;
> > +     enum macaccess_entry_type type;
> > +
> > +     list_for_each_entry(mdb_entry, &lan966x->mdb_entries, list) {
> > +             if (mdb_entry->vid != vid || !mdb_entry->cpu_copy)
> > +                     continue;
> > +
> > +             type = lan966x_mdb_classify(mdb_entry->mac);
> > +             if (type == ENTRYTYPE_MACV4 ||
> > +                 type == ENTRYTYPE_MACV6)
> > +                     lan966x_mdb_ip_cpu_copy(lan966x, mdb_entry, type);
> > +             else
> > +                     lan966x_mdb_l2_cpu_copy(lan966x, mdb_entry, type);
> > +     }
> > +}
> > +
> > +static void lan966x_mdb_ip_cpu_remove(struct lan966x *lan966x,
> > +                                   struct lan966x_mdb_entry *mdb_entry,
> > +                                   enum macaccess_entry_type type)
> > +{
> > +     unsigned char mac[ETH_ALEN];
> > +
> > +     lan966x_mdb_encode_mac(mac, mdb_entry, type);
> > +     lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> > +     lan966x_mac_cpu_copy(lan966x, 0, false, mac, mdb_entry->vid, type);
> > +}
> > +
> > +static void lan966x_mdb_l2_cpu_remove(struct lan966x *lan966x,
> > +                                   struct lan966x_mdb_entry *mdb_entry,
> > +                                   enum macaccess_entry_type type)
> > +{
> > +     struct lan966x_pgid_entry *pgid_entry;
> > +     unsigned char mac[ETH_ALEN];
> > +
> > +     lan966x_pgid_entry_del(lan966x, mdb_entry->pgid);
> > +     lan966x_mdb_encode_mac(mac, mdb_entry, type);
> > +     lan966x_mac_forget(lan966x, mac, mdb_entry->vid, type);
> > +
> > +     mdb_entry->ports &= ~BIT(CPU_PORT);
> > +
> > +     pgid_entry = lan966x_pgid_entry_get(lan966x, mdb_entry);
> > +     if (IS_ERR(pgid_entry))
> > +             return;
> > +
> > +     mdb_entry->pgid = pgid_entry;
> > +
> > +     lan_rmw(ANA_PGID_PGID_SET(mdb_entry->ports),
> > +             ANA_PGID_PGID,
> > +             lan966x, ANA_PGID(pgid_entry->index));
> > +
> > +     lan966x_mac_learn(lan966x, pgid_entry->index, mdb_entry->mac,
> > +                       mdb_entry->vid, type);
> > +}
> > +
> > +void lan966x_mdb_erase_entries(struct lan966x *lan966x, u16 vid)
> > +{
> > +     struct lan966x_mdb_entry *mdb_entry;
> > +     enum macaccess_entry_type type;
> > +
> > +     list_for_each_entry(mdb_entry, &lan966x->mdb_entries, list) {
> > +             if (mdb_entry->vid != vid || !mdb_entry->cpu_copy)
> > +                     continue;
> > +
> > +             type = lan966x_mdb_classify(mdb_entry->mac);
> > +             if (type == ENTRYTYPE_MACV4 ||
> > +                 type == ENTRYTYPE_MACV6)
> > +                     lan966x_mdb_ip_cpu_remove(lan966x, mdb_entry, type);
> > +             else
> > +                     lan966x_mdb_l2_cpu_remove(lan966x, mdb_entry, type);
> > +     }
> > +}
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
> > index deb3dd5be67a..7de55f6a4da8 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_switchdev.c
> > @@ -438,6 +438,10 @@ static int lan966x_handle_port_obj_add(struct net_device *dev, const void *ctx,
> >       case SWITCHDEV_OBJ_ID_PORT_VLAN:
> >               err = lan966x_handle_port_vlan_add(port, obj);
> >               break;
> > +     case SWITCHDEV_OBJ_ID_PORT_MDB:
> > +     case SWITCHDEV_OBJ_ID_HOST_MDB:
> > +             err = lan966x_handle_port_mdb_add(port, obj);
> > +             break;
> >       default:
> >               err = -EOPNOTSUPP;
> >               break;
> > @@ -473,6 +477,10 @@ static int lan966x_handle_port_obj_del(struct net_device *dev, const void *ctx,
> >       case SWITCHDEV_OBJ_ID_PORT_VLAN:
> >               err = lan966x_handle_port_vlan_del(port, obj);
> >               break;
> > +     case SWITCHDEV_OBJ_ID_PORT_MDB:
> > +     case SWITCHDEV_OBJ_ID_HOST_MDB:
> 
> The HOST_MDB switchdev events are replicated per the number of ports in
> the bridge.

Correct.

> So I would expect that you keep refcounts on them, otherwise
> the first deletion of such an element would trigger the removal of the
> entry from hardware even though it's still in use.

Sorry, I am not sure I am following, by whom is it still in use?

I was trying the following commands:

ip link set dev eth0 master br0
ip link set dev eth1 master br0
bridge mdb add dev br0 port br0 grp 225.1.2.3

Then both ports eth0 and eth1 will get a notification about this. And
they will add an entry in the MAC table for this.
Then when the following command is run:

bridge mdb del dev br0 port br0 grp 225.1.2.3

Then again both ports will get a notification about this and eth0 will
delete the entry from the MAC table. So why is this not correct? Then
will be eth1 who will get the notification and try to delete the entry
which is already deleted.

> 
> > +             err = lan966x_handle_port_mdb_del(port, obj);
> > +             break;
> >       default:
> >               err = -EOPNOTSUPP;
> >               break;
> > diff --git a/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c b/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
> > index 057f48ddf22c..8d7260cd7da9 100644
> > --- a/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
> > +++ b/drivers/net/ethernet/microchip/lan966x/lan966x_vlan.c
> > @@ -219,6 +219,7 @@ void lan966x_vlan_port_add_vlan(struct lan966x_port *port,
> >       if (lan966x_vlan_cpu_member_cpu_vlan_mask(lan966x, vid)) {
> >               lan966x_vlan_cpu_add_vlan_mask(lan966x, vid);
> >               lan966x_fdb_write_entries(lan966x, vid);
> > +             lan966x_mdb_write_entries(lan966x, vid);
> >       }
> >
> >       lan966x_vlan_port_set_vid(port, vid, pvid, untagged);
> > @@ -241,6 +242,7 @@ void lan966x_vlan_port_del_vlan(struct lan966x_port *port, u16 vid)
> >       if (!lan966x_vlan_port_any_vlan_mask(lan966x, vid)) {
> >               lan966x_vlan_cpu_del_vlan_mask(lan966x, vid);
> >               lan966x_fdb_erase_entries(lan966x, vid);
> > +             lan966x_mdb_erase_entries(lan966x, vid);
> >       }
> >  }
> >
> > @@ -254,8 +256,10 @@ void lan966x_vlan_cpu_add_vlan(struct lan966x *lan966x, u16 vid)
> >        * information so when a front port is added then it would add also the
> >        * CPU port.
> >        */
> > -     if (lan966x_vlan_port_any_vlan_mask(lan966x, vid))
> > +     if (lan966x_vlan_port_any_vlan_mask(lan966x, vid)) {
> >               lan966x_vlan_cpu_add_vlan_mask(lan966x, vid);
> > +             lan966x_mdb_write_entries(lan966x, vid);
> > +     }
> >
> >       lan966x_vlan_cpu_add_cpu_vlan_mask(lan966x, vid);
> >       lan966x_fdb_write_entries(lan966x, vid);
> > @@ -267,6 +271,7 @@ void lan966x_vlan_cpu_del_vlan(struct lan966x *lan966x, u16 vid)
> >       lan966x_vlan_cpu_del_cpu_vlan_mask(lan966x, vid);
> >       lan966x_vlan_cpu_del_vlan_mask(lan966x, vid);
> >       lan966x_fdb_erase_entries(lan966x, vid);
> > +     lan966x_mdb_erase_entries(lan966x, vid);
> >  }
> >
> >  void lan966x_vlan_init(struct lan966x *lan966x)
> > --
> > 2.33.0
> >

-- 
/Horatiu

Re: [PATCH net-next 3/3] net: lan966x: Extend switchdev with mdb support

[Vladimir Oltean]

On Mon, Jan 03, 2022 at 05:22:44PM +0100, Horatiu Vultur wrote:
> > > +static int lan966x_mdb_ip_del(struct lan966x_port *port,
> > > +                           const struct switchdev_obj_port_mdb *mdb,
> > > +                           enum macaccess_entry_type type)
> > > +{
> > > +     bool cpu_port = netif_is_bridge_master(mdb->obj.orig_dev);
> > > +     struct lan966x *lan966x = port->lan966x;
> > > +     struct lan966x_mdb_entry *mdb_entry;
> > > +     unsigned char mac[ETH_ALEN];
> > > +
> > > +     mdb_entry = lan966x_mdb_entry_get(lan966x, mdb->addr, mdb->vid);
> > > +     if (!mdb_entry) {
> > > +             /* If the CPU originted this and the entry was not found, it is
> >
> > s/originted/originated/
> >
> > > +              * because already another port has removed the entry
> > > +              */
> > > +             if (cpu_port)
> > > +                     return 0;
> >
> > Could you explain again why this is normal? If another port has removed
> > the entry, how is the address copied to the CPU any longer?
>
> The cpu_port is set only when the entry is deleted by the bridge. So then
> all the ports under the bridge will be notified about this. So when the first
> port is notified it would delete the entry and then when the second port is
> notified, it would try to delete the entry which is already deleted by
> the first port. That is the reason why it returns 0 and not an error.


> > The HOST_MDB switchdev events are replicated per the number of ports in
> > the bridge.
>
> Correct.
>
> > So I would expect that you keep refcounts on them, otherwise
> > the first deletion of such an element would trigger the removal of the
> > entry from hardware even though it's still in use.
>
> Sorry, I am not sure I am following, by whom is it still in use?
>
> I was trying the following commands:
>
> ip link set dev eth0 master br0
> ip link set dev eth1 master br0
> bridge mdb add dev br0 port br0 grp 225.1.2.3
>
> Then both ports eth0 and eth1 will get a notification about this. And
> they will add an entry in the MAC table for this.
> Then when the following command is run:
>
> bridge mdb del dev br0 port br0 grp 225.1.2.3
>
> Then again both ports will get a notification about this and eth0 will
> delete the entry from the MAC table. So why is this not correct? Then
> will be eth1 who will get the notification and try to delete the entry
> which is already deleted.

root@debian:~# ip link set swp0 up
[   42.938394] fsl_enetc 0000:00:00.2 eno2: configuring for fixed/internal link mode
[   42.949675] fsl_enetc 0000:00:00.2 eno2: Link is Up - 2.5Gbps/Full - flow control rx/tx
[   42.959044] mscc_felix 0000:00:00.5 swp0: configuring for inband/qsgmii link mode
root@debian:~# ip link set swp1 up
[   44.440675] mscc_felix 0000:00:00.5 swp1: configuring for inband/qsgmii link mode
root@debian:~# ip link set swp2 up
[   45.788811] mscc_felix 0000:00:00.5 swp2: configuring for inband/qsgmii link mode
root@debian:~# ip link set swp3 up
[   47.784439] mscc_felix 0000:00:00.5 swp3: configuring for inband/qsgmii link mode
[   51.887374] mscc_felix 0000:00:00.5 swp3: Link is Up - 1Gbps/Full - flow control rx/tx
[   51.895491] IPv6: ADDRCONF(NETDEV_CHANGE): swp3: link becomes ready
[  100.303070] mscc_felix 0000:00:00.5 swp0: Link is Up - 1Gbps/Full - flow control off
[  100.311016] IPv6: ADDRCONF(NETDEV_CHANGE): swp0: link becomes ready
root@debian:~# ip link add br0 type bridge
root@debian:~# ip link set swp0 master br0
[  115.436652] br0: port 1(swp0) entered blocking state
[  115.441736] br0: port 1(swp0) entered disabled state
[  115.447618] device swp0 entered promiscuous mode
root@debian:~# ip link set swp1 master br0
[  117.760700] br0: port 2(swp1) entered blocking state
[  117.765797] br0: port 2(swp1) entered disabled state
[  117.771335] device swp1 entered promiscuous mode
root@debian:~# ip link set swp2 master br0
[  119.612665] br0: port 3(swp2) entered blocking state
[  119.617750] br0: port 3(swp2) entered disabled state
[  119.623304] device swp2 entered promiscuous mode
root@debian:~# ip link set swp3 master br0
[  121.388388] br0: port 4(swp3) entered blocking state
[  121.393416] br0: port 4(swp3) entered disabled state
[  121.399292] device swp3 entered promiscuous mode
root@debian:~# [  148.905819] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:ff:9e:9e:96 vid 0
[  148.914819] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:ff:9e:9e:96 vid 0
[  148.923723] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:ff:9e:9e:96 vid 0
[  148.932606] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:ff:9e:9e:96 vid 0
[  148.941426] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:00:00:00:6a vid 0
[  148.950351] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:00:00:00:6a vid 0
[  148.959164] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:00:00:00:6a vid 0
[  148.967967] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:00:00:00:6a vid 0
[  150.649744] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:00:00:00:fb vid 0
[  150.658674] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:00:00:00:fb vid 0
[  150.667538] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:00:00:00:fb vid 0
[  150.676389] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:00:00:00:fb vid 0
root@debian:~# ip link set br0 up
[  148.868268] br0: port 4(swp3) entered blocking state
[  148.873283] br0: port 4(swp3) entered forwarding state
[  148.878530] br0: port 1(swp0) entered blocking state
[  148.883546] br0: port 1(swp0) entered forwarding state
[  148.889443] IPv6: ADDRCONF(NETDEV_CHANGE): br0: link becomes ready
[  148.905819] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:ff:9e:9e:96 vid 0
[  148.914819] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:ff:9e:9e:96 vid 0
[  148.923723] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:ff:9e:9e:96 vid 0
[  148.932606] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:ff:9e:9e:96 vid 0
[  148.941426] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:00:00:00:6a vid 0
[  148.950351] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:00:00:00:6a vid 0
[  148.959164] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:00:00:00:6a vid 0
[  148.967967] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:00:00:00:6a vid 0
[  150.649744] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 0 addr 33:33:00:00:00:fb vid 0
[  150.658674] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 1 addr 33:33:00:00:00:fb vid 0
[  150.667538] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 2 addr 33:33:00:00:00:fb vid 0
[  150.676389] mscc_felix 0000:00:00.5: dsa_port_host_mdb_add: port 3 addr 33:33:00:00:00:fb vid 0
root@debian:~# ip link set swp3 nomaster
[  160.416054] device swp3 left promiscuous mode
[  160.420745] br0: port 4(swp3) entered disabled state
[  160.445427] mscc_felix 0000:00:00.5: dsa_port_host_mdb_del: port 3 addr 33:33:00:00:00:fb vid 0
[  160.454201] mscc_felix 0000:00:00.5: dsa_port_host_mdb_del: port 3 addr 33:33:00:00:00:6a vid 0
[  160.464843] mscc_felix 0000:00:00.5: dsa_port_host_mdb_del: port 3 addr 33:33:ff:9e:9e:96 vid 0

Hope it's a bit clearer now. swp0, swp1 and swp2 are still under br0.