[PATCH net-next v8 0/2] net: dsa: mv88e6xxx: Add partial support for TCAM entries

[PATCH net-next v8 0/2] net: dsa: mv88e6xxx: Add partial support for TCAM entries

[Cedric Jehasse via B4 Relay]

This series adds partial Ternary Content Addressable Memory (TCAM) for
the mv88e6390 and mv88e6393 family of switches. TCAM entries allow the
switch to match the first 48 or 96 bytes of a frame and take actions on
matched frames.

This patch introduces a subset of the available TCAM functionality.
Matching on ip addresses/protocol and trapping to the cpu.

Eg. to trap traffic with destination ip 224.0.1.129 to the cpu:

tc qdisc add dev p1 clsact
tc filter add dev p1 ingress protocol ip flower skip_sw \
    dst_ip 224.0.1.129 action trap

Review of the mv88e6xxx changes have brought to light something in
cls_flower:
When adding a classifier with an ipv4 address both
FLOW_DISSECTOR_KEY_IPV4_ADDRS and FLOW_DISSECTOR_KEY_IPV6_ADDRS bits are
set in dissector->used_keys.
A change was made to address this.

Signed-off-by: Cedric Jehasse <cedric.jehasse@luminex.be>
---
Changes in v8:
- add teardown function for tcflower to free entries in the tcam list
- Link to v7: https://lore.kernel.org/r/20260310-net-next-mv88e6xxx-tcam-v7-0-7d9db9012b28@luminex.be

Changes in v7:
- clear register field before setting it
- fix name inconsistency of mv88e6xx_flower_parse_key
- use defines for spv and spv_mask register fields
- Link to v6: https://lore.kernel.org/r/20260305-net-next-mv88e6xxx-tcam-v6-0-32c7fbac129d@luminex.be

Changes in v6:
- add num_tcam_entries in chip info and check this is not exceeded when
  adding an entry
- fix use after free in error path of tcam entry deletion
- fix copy/paste mistake in comment
- Link to v5: https://lore.kernel.org/r/20260303-net-next-mv88e6xxx-tcam-v5-0-6f9129687276@luminex.be

Changes in v5:
- changes to cls_flower to only set the correct
  FLOW_DISSECTOR_KEY_IPV*_ADDRS bits
- Link to v4: https://lore.kernel.org/r/20260225-net-next-mv88e6xxx-tcam-v4-1-117f600e41d7@luminex.be

Changes in v4:
- return -ENOMEM on allocation failure
- fix double mv88e6xxx_reg_unlock
- return -EOPNOTSUPP for unsupported addr_type
- improve error handling in mv88e6xxx_tcam_entry_del
- Link to v3: https://lore.kernel.org/r/20260224-net-next-mv88e6xxx-tcam-v3-1-dc18887d71f2@luminex.be

Changes in v3:
- move locking to top level functions
- use defines for match field offsets
- use correct comment style
- remove warning used for debugging
- rename rc variables to err
- limit line length to 80 characters
- Link to v2: https://lore.kernel.org/r/20260217-net-next-mv88e6xxx-tcam-v2-1-d84cf420da46@luminex.be

Changes in RFC-v2:
- moved tcam_addr to chip info struct and remove tcam_support field
- added 6393-specific tcam ops that write the TCAM extension register
- rename the defines for TCAM registers to be consistent
- reverse christmas tree variable declarations
- Link to v1: https://lore.kernel.org/r/20260213-net-next-mv88e6xxx-tcam-v1-1-051e552e2afc@luminex.be

---
Cedric Jehasse (2):
      net/sched: cls_flower: remove unions from fl_flow_key
      net: dsa: mv88e6xxx: Add partial support for TCAM entries

 drivers/net/dsa/mv88e6xxx/Makefile   |   2 +
 drivers/net/dsa/mv88e6xxx/chip.c     |  35 ++++
 drivers/net/dsa/mv88e6xxx/chip.h     |  52 ++++++
 drivers/net/dsa/mv88e6xxx/port.c     |  28 ++-
 drivers/net/dsa/mv88e6xxx/port.h     |   7 +-
 drivers/net/dsa/mv88e6xxx/tcam.c     | 338 +++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/tcam.h     |  41 +++++
 drivers/net/dsa/mv88e6xxx/tcflower.c | 167 +++++++++++++++++
 drivers/net/dsa/mv88e6xxx/tcflower.h |  14 ++
 net/sched/cls_flower.c               |  12 +-
 10 files changed, 686 insertions(+), 10 deletions(-)
---
base-commit: cd51b40495c09b92beea6893663b3d0ed7605e81
change-id: 20260213-net-next-mv88e6xxx-tcam-f65be16008fb

Best regards,
-- 
Cedric Jehasse <cedric.jehasse@luminex.be>

[PATCH net-next v8 1/2] net/sched: cls_flower: remove unions from fl_flow_key

[Cedric Jehasse via B4 Relay]

From: Cedric Jehasse <cedric.jehasse@luminex.be>

When creating a flower classifier with an ipv4 address the
flow_dissector has both FLOW_DISSECTOR_KEY_IPV4_ADDRS and
FLOW_DISSECTOR_KEY_IPV6_ADDRS bits set in used_keys.
This happens because ipv4/ipv6 fields are a union and
FL_KEY_SET_IF_MASKED() will interpret either being set as both.

Removing the unions fixes this behavior without needing special handling
for union fields.

Example of a command that caused FLOW_DISSECTOR_KEY_IPV4_ADDRS and
FLOW_DISSECTOR_KEY_IPV6_ADDRS to be set:
tc filter add dev p1 ingress protocol ip flower skip_sw \
    dst_ip 224.0.1.129 action trap

Signed-off-by: Cedric Jehasse <cedric.jehasse@luminex.be>
---
 net/sched/cls_flower.c | 12 ++++--------
 1 file changed, 4 insertions(+), 8 deletions(-)

diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
index 7669371c1354..b95dbe847dde 100644
--- a/net/sched/cls_flower.c
+++ b/net/sched/cls_flower.c
@@ -59,18 +59,14 @@ struct fl_flow_key {
 	struct flow_dissector_key_eth_addrs eth;
 	struct flow_dissector_key_vlan vlan;
 	struct flow_dissector_key_vlan cvlan;
-	union {
-		struct flow_dissector_key_ipv4_addrs ipv4;
-		struct flow_dissector_key_ipv6_addrs ipv6;
-	};
+	struct flow_dissector_key_ipv4_addrs ipv4;
+	struct flow_dissector_key_ipv6_addrs ipv6;
 	struct flow_dissector_key_ports tp;
 	struct flow_dissector_key_icmp icmp;
 	struct flow_dissector_key_arp arp;
 	struct flow_dissector_key_keyid enc_key_id;
-	union {
-		struct flow_dissector_key_ipv4_addrs enc_ipv4;
-		struct flow_dissector_key_ipv6_addrs enc_ipv6;
-	};
+	struct flow_dissector_key_ipv4_addrs enc_ipv4;
+	struct flow_dissector_key_ipv6_addrs enc_ipv6;
 	struct flow_dissector_key_ports enc_tp;
 	struct flow_dissector_key_mpls mpls;
 	struct flow_dissector_key_tcp tcp;

-- 
2.43.0

[PATCH net-next v8 2/2] net: dsa: mv88e6xxx: Add partial support for TCAM entries

[Cedric Jehasse via B4 Relay]

From: Cedric Jehasse <cedric.jehasse@luminex.be>

This patch adds partial Ternary Content Addressable Memory (TCAM) for
the mv88e6390 and mv88e6393 family of switches. TCAM entries allow the
switch to match the first 48 or 96 bytes of a frame and take actions on
matched frames.

This patch introduces a subset of the available TCAM functionality.
Matching on ip addresses/protocol and trapping to the cpu.

Eg. to trap traffic with destination ip 224.0.1.129 to the cpu:

tc qdisc add dev p1 clsact
tc filter add dev p1 ingress protocol ip flower skip_sw \
    dst_ip 224.0.1.129 action trap

Signed-off-by: Cedric Jehasse <cedric.jehasse@luminex.be>
---
 drivers/net/dsa/mv88e6xxx/Makefile   |   2 +
 drivers/net/dsa/mv88e6xxx/chip.c     |  35 ++++
 drivers/net/dsa/mv88e6xxx/chip.h     |  52 ++++++
 drivers/net/dsa/mv88e6xxx/port.c     |  28 ++-
 drivers/net/dsa/mv88e6xxx/port.h     |   7 +-
 drivers/net/dsa/mv88e6xxx/tcam.c     | 338 +++++++++++++++++++++++++++++++++++
 drivers/net/dsa/mv88e6xxx/tcam.h     |  41 +++++
 drivers/net/dsa/mv88e6xxx/tcflower.c | 167 +++++++++++++++++
 drivers/net/dsa/mv88e6xxx/tcflower.h |  14 ++
 9 files changed, 682 insertions(+), 2 deletions(-)

diff --git a/drivers/net/dsa/mv88e6xxx/Makefile b/drivers/net/dsa/mv88e6xxx/Makefile
index dd961081d631..b0b08c6f159c 100644
--- a/drivers/net/dsa/mv88e6xxx/Makefile
+++ b/drivers/net/dsa/mv88e6xxx/Makefile
@@ -21,6 +21,8 @@ mv88e6xxx-objs += serdes.o
 mv88e6xxx-objs += smi.o
 mv88e6xxx-objs += switchdev.o
 mv88e6xxx-objs += trace.o
+mv88e6xxx-objs += tcflower.o
+mv88e6xxx-objs += tcam.o
 
 # for tracing framework to find trace.h
 CFLAGS_trace.o := -I$(src)
diff --git a/drivers/net/dsa/mv88e6xxx/chip.c b/drivers/net/dsa/mv88e6xxx/chip.c
index 09002c853b78..f1bfa3228b0a 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.c
+++ b/drivers/net/dsa/mv88e6xxx/chip.c
@@ -43,6 +43,8 @@
 #include "ptp.h"
 #include "serdes.h"
 #include "smi.h"
+#include "tcam.h"
+#include "tcflower.h"
 
 static void assert_reg_lock(struct mv88e6xxx_chip *chip)
 {
@@ -3560,6 +3562,11 @@ static int mv88e6xxx_setup_port(struct mv88e6xxx_chip *chip, int port)
 		if (err)
 			return err;
 	}
+	if (chip->info->ops->port_enable_tcam) {
+		err = chip->info->ops->port_enable_tcam(chip, port);
+		if (err)
+			return err;
+	}
 
 	if (chip->info->ops->port_tag_remap) {
 		err = chip->info->ops->port_tag_remap(chip, port);
@@ -3938,6 +3945,14 @@ static int mv88e6xxx_mdios_register(struct mv88e6xxx_chip *chip)
 	return 0;
 }
 
+static int mv88e6xxx_tcam_setup(struct mv88e6xxx_chip *chip)
+{
+	if (!mv88e6xxx_has_tcam(chip))
+		return 0;
+
+	return chip->info->ops->tcam_ops->flush_tcam(chip);
+}
+
 static void mv88e6xxx_teardown(struct dsa_switch *ds)
 {
 	struct mv88e6xxx_chip *chip = ds->priv;
@@ -3947,6 +3962,7 @@ static void mv88e6xxx_teardown(struct dsa_switch *ds)
 	mv88e6xxx_teardown_devlink_regions_global(ds);
 	mv88e6xxx_hwtstamp_free(chip);
 	mv88e6xxx_ptp_free(chip);
+	mv88e6xxx_flower_teardown(chip);
 	mv88e6xxx_mdios_unregister(chip);
 }
 
@@ -4083,6 +4099,10 @@ static int mv88e6xxx_setup(struct dsa_switch *ds)
 	if (err)
 		goto unlock;
 
+	err = mv88e6xxx_tcam_setup(chip);
+	if (err)
+		goto unlock;
+
 unlock:
 	mv88e6xxx_reg_unlock(chip);
 
@@ -5134,6 +5154,7 @@ static const struct mv88e6xxx_ops mv88e6290_ops = {
 	.ptp_ops = &mv88e6390_ptp_ops,
 	.phylink_get_caps = mv88e6390_phylink_get_caps,
 	.pcs_ops = &mv88e6390_pcs_ops,
+	.tcam_ops = &mv88e6390_tcam_ops,
 };
 
 static const struct mv88e6xxx_ops mv88e6320_ops = {
@@ -5525,6 +5546,7 @@ static const struct mv88e6xxx_ops mv88e6390_ops = {
 	.serdes_get_regs = mv88e6390_serdes_get_regs,
 	.phylink_get_caps = mv88e6390_phylink_get_caps,
 	.pcs_ops = &mv88e6390_pcs_ops,
+	.tcam_ops = &mv88e6390_tcam_ops,
 };
 
 static const struct mv88e6xxx_ops mv88e6390x_ops = {
@@ -5621,6 +5643,7 @@ static const struct mv88e6xxx_ops mv88e6393x_ops = {
 	.port_set_cmode = mv88e6393x_port_set_cmode,
 	.port_setup_message_port = mv88e6xxx_setup_message_port,
 	.port_set_upstream_port = mv88e6393x_port_set_upstream_port,
+	.port_enable_tcam = mv88e6xxx_port_enable_tcam,
 	.stats_snapshot = mv88e6390_g1_stats_snapshot,
 	.stats_set_histogram = mv88e6390_g1_stats_set_histogram,
 	.stats_get_sset_count = mv88e6320_stats_get_sset_count,
@@ -5652,6 +5675,7 @@ static const struct mv88e6xxx_ops mv88e6393x_ops = {
 	.ptp_ops = &mv88e6352_ptp_ops,
 	.phylink_get_caps = mv88e6393x_phylink_get_caps,
 	.pcs_ops = &mv88e6393x_pcs_ops,
+	.tcam_ops = &mv88e6393_tcam_ops,
 };
 
 static const struct mv88e6xxx_info mv88e6xxx_table[] = {
@@ -6125,6 +6149,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_databases = 4096,
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 8,
+		.num_tcam_entries = 256,
 		.internal_phys_offset = 1,
 		.max_vid = 8191,
 		.max_sid = 63,
@@ -6132,6 +6157,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
 		.global2_addr = 0x1c,
+		.tcam_addr = 0x1f,
 		.age_time_coeff = 3750,
 		.g1_irqs = 10,
 		.g2_irqs = 14,
@@ -6227,12 +6253,14 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.num_gpio = 16,
+		.num_tcam_entries = 256,
 		.max_vid = 8191,
 		.max_sid = 63,
 		.port_base_addr = 0x0,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
 		.global2_addr = 0x1c,
+		.tcam_addr = 0x1f,
 		.age_time_coeff = 3750,
 		.g1_irqs = 9,
 		.g2_irqs = 14,
@@ -6439,12 +6467,14 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 9,
 		.num_gpio = 16,
+		.num_tcam_entries = 256,
 		.max_vid = 8191,
 		.max_sid = 63,
 		.port_base_addr = 0x0,
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
 		.global2_addr = 0x1c,
+		.tcam_addr = 0x1f,
 		.age_time_coeff = 3750,
 		.g1_irqs = 9,
 		.g2_irqs = 14,
@@ -6490,6 +6520,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.num_databases = 4096,
 		.num_ports = 11,	/* 10 + Z80 */
 		.num_internal_phys = 8,
+		.num_tcam_entries = 256,
 		.internal_phys_offset = 1,
 		.max_vid = 8191,
 		.max_sid = 63,
@@ -6497,6 +6528,7 @@ static const struct mv88e6xxx_info mv88e6xxx_table[] = {
 		.phy_base_addr = 0x0,
 		.global1_addr = 0x1b,
 		.global2_addr = 0x1c,
+		.tcam_addr = 0x1f,
 		.age_time_coeff = 3750,
 		.g1_irqs = 10,
 		.g2_irqs = 14,
@@ -6589,6 +6621,7 @@ static struct mv88e6xxx_chip *mv88e6xxx_alloc_chip(struct device *dev)
 	INIT_LIST_HEAD(&chip->mdios);
 	idr_init(&chip->policies);
 	INIT_LIST_HEAD(&chip->msts);
+	INIT_LIST_HEAD(&chip->tcam.entries);
 
 	return chip;
 }
@@ -7184,6 +7217,8 @@ static const struct dsa_switch_ops mv88e6xxx_switch_ops = {
 	.port_hwtstamp_get	= mv88e6xxx_port_hwtstamp_get,
 	.port_txtstamp		= mv88e6xxx_port_txtstamp,
 	.port_rxtstamp		= mv88e6xxx_port_rxtstamp,
+	.cls_flower_add		= mv88e6xxx_cls_flower_add,
+	.cls_flower_del         = mv88e6xxx_cls_flower_del,
 	.get_ts_info		= mv88e6xxx_get_ts_info,
 	.devlink_param_get	= mv88e6xxx_devlink_param_get,
 	.devlink_param_set	= mv88e6xxx_devlink_param_set,
diff --git a/drivers/net/dsa/mv88e6xxx/chip.h b/drivers/net/dsa/mv88e6xxx/chip.h
index e073446ee7d0..2b235ac2c5df 100644
--- a/drivers/net/dsa/mv88e6xxx/chip.h
+++ b/drivers/net/dsa/mv88e6xxx/chip.h
@@ -135,12 +135,14 @@ struct mv88e6xxx_info {
 	unsigned int num_ports;
 	unsigned int num_internal_phys;
 	unsigned int num_gpio;
+	unsigned int num_tcam_entries;
 	unsigned int max_vid;
 	unsigned int max_sid;
 	unsigned int port_base_addr;
 	unsigned int phy_base_addr;
 	unsigned int global1_addr;
 	unsigned int global2_addr;
+	unsigned int tcam_addr;
 	unsigned int age_time_coeff;
 	unsigned int g1_irqs;
 	unsigned int g2_irqs;
@@ -210,6 +212,7 @@ struct mv88e6xxx_avb_ops;
 struct mv88e6xxx_ptp_ops;
 struct mv88e6xxx_pcs_ops;
 struct mv88e6xxx_cc_coeffs;
+struct mv88e6xxx_tcam_ops;
 
 struct mv88e6xxx_irq {
 	u16 masked;
@@ -339,6 +342,10 @@ struct mv88e6xxx_hw_stat {
 	int type;
 };
 
+struct mv88e6xxx_tcam {
+	struct list_head entries;
+};
+
 struct mv88e6xxx_chip {
 	const struct mv88e6xxx_info *info;
 
@@ -444,6 +451,35 @@ struct mv88e6xxx_chip {
 
 	/* FID map */
 	DECLARE_BITMAP(fid_bitmap, MV88E6XXX_N_FID);
+
+	/* TCAM entries */
+	struct mv88e6xxx_tcam tcam;
+};
+
+#define TCAM_MATCH_SIZE 96
+
+struct mv88e6xxx_tcam_key {
+	u16 spv;
+	u16 spv_mask;
+
+	u8 frame_data[TCAM_MATCH_SIZE];
+	u8 frame_mask[TCAM_MATCH_SIZE];
+};
+
+struct mv88e6xxx_tcam_action {
+	u8 dpv_mode;
+	u16 dpv;
+};
+
+struct mv88e6xxx_tcam_entry {
+	struct list_head list;
+	unsigned long cookie;
+	u32 prio;
+	u8 hw_idx;
+
+	struct mv88e6xxx_tcam_key key;
+	struct mv88e6xxx_tcam_action action;
+
 };
 
 struct mv88e6xxx_bus_ops {
@@ -678,6 +714,11 @@ struct mv88e6xxx_ops {
 
 	/* Max Frame Size */
 	int (*set_max_frame_size)(struct mv88e6xxx_chip *chip, int mtu);
+
+	int (*port_enable_tcam)(struct mv88e6xxx_chip *chip, int port);
+
+	/* Ternary Content Addressable Memory operations */
+	const struct mv88e6xxx_tcam_ops *tcam_ops;
 };
 
 struct mv88e6xxx_irq_ops {
@@ -752,6 +793,12 @@ struct mv88e6xxx_pcs_ops {
 
 };
 
+struct mv88e6xxx_tcam_ops {
+	int (*entry_add)(struct mv88e6xxx_chip *chip,
+			 struct mv88e6xxx_tcam_entry *entry, u8 idx);
+	int (*flush_tcam)(struct mv88e6xxx_chip *chip);
+};
+
 static inline bool mv88e6xxx_has_stu(struct mv88e6xxx_chip *chip)
 {
 	return chip->info->max_sid > 0 &&
@@ -769,6 +816,11 @@ static inline bool mv88e6xxx_has_lag(struct mv88e6xxx_chip *chip)
 	return !!chip->info->global2_addr;
 }
 
+static inline bool mv88e6xxx_has_tcam(struct mv88e6xxx_chip *chip)
+{
+	return !!chip->info->tcam_addr;
+}
+
 static inline unsigned int mv88e6xxx_num_databases(struct mv88e6xxx_chip *chip)
 {
 	return chip->info->num_databases;
diff --git a/drivers/net/dsa/mv88e6xxx/port.c b/drivers/net/dsa/mv88e6xxx/port.c
index 66b1b7277281..49cd82930b7a 100644
--- a/drivers/net/dsa/mv88e6xxx/port.c
+++ b/drivers/net/dsa/mv88e6xxx/port.c
@@ -1380,7 +1380,33 @@ int mv88e6xxx_port_disable_learn_limit(struct mv88e6xxx_chip *chip, int port)
 
 int mv88e6xxx_port_disable_pri_override(struct mv88e6xxx_chip *chip, int port)
 {
-	return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_PRI_OVERRIDE, 0);
+	u16 reg;
+	int err;
+
+	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_PRI_OVERRIDE,
+				  &reg);
+	if (err)
+		return err;
+
+	reg &= MV88E6XXX_PORT_PRI_OVERRIDE_TCAM_MODE_MASK;
+	return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_PRI_OVERRIDE,
+				    reg);
+}
+
+int mv88e6xxx_port_enable_tcam(struct mv88e6xxx_chip *chip, int port)
+{
+	u16 reg;
+	int err;
+
+	err = mv88e6xxx_port_read(chip, port, MV88E6XXX_PORT_PRI_OVERRIDE,
+				  &reg);
+	if (err)
+		return err;
+
+	reg &= ~MV88E6XXX_PORT_PRI_OVERRIDE_TCAM_MODE_MASK;
+	reg |= MV88E6XXX_PORT_PRI_OVERRIDE_TCAM_MODE_96_BYTE;
+	return mv88e6xxx_port_write(chip, port, MV88E6XXX_PORT_PRI_OVERRIDE,
+				    reg);
 }
 
 /* Offset 0x0E: Policy & MGMT Control Register for FAMILY 6191X 6193X 6393X */
diff --git a/drivers/net/dsa/mv88e6xxx/port.h b/drivers/net/dsa/mv88e6xxx/port.h
index c1d2f99efb1c..a2492cf4d920 100644
--- a/drivers/net/dsa/mv88e6xxx/port.h
+++ b/drivers/net/dsa/mv88e6xxx/port.h
@@ -254,7 +254,10 @@
 #define MV88E6XXX_PORT_ATU_CTL		0x0c
 
 /* Offset 0x0D: Priority Override Register */
-#define MV88E6XXX_PORT_PRI_OVERRIDE	0x0d
+#define MV88E6XXX_PORT_PRI_OVERRIDE			0x0d
+#define MV88E6XXX_PORT_PRI_OVERRIDE_TCAM_MODE_MASK	0x0003
+#define MV88E6XXX_PORT_PRI_OVERRIDE_TCAM_MODE_48_BYTE	0x0001
+#define MV88E6XXX_PORT_PRI_OVERRIDE_TCAM_MODE_96_BYTE	0x0002
 
 /* Offset 0x0E: Policy Control Register */
 #define MV88E6XXX_PORT_POLICY_CTL		0x0e
@@ -608,4 +611,6 @@ int mv88e6xxx_port_hidden_wait(struct mv88e6xxx_chip *chip);
 int mv88e6xxx_port_hidden_read(struct mv88e6xxx_chip *chip, int block, int port,
 			       int reg, u16 *val);
 
+int mv88e6xxx_port_enable_tcam(struct mv88e6xxx_chip *chip, int port);
+
 #endif /* _MV88E6XXX_PORT_H */
diff --git a/drivers/net/dsa/mv88e6xxx/tcam.c b/drivers/net/dsa/mv88e6xxx/tcam.c
new file mode 100644
index 000000000000..fc16bab8e772
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/tcam.c
@@ -0,0 +1,338 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Marvell 88E6xxx Switch TCAM support
+ *
+ * Copyright (c) 2026 Luminex Network Intelligence
+ */
+
+#include "linux/list.h"
+
+#include "chip.h"
+#include "tcam.h"
+
+/* TCAM operatation register */
+#define MV88E6XXX_TCAM_OP			0x00
+#define MV88E6XXX_TCAM_OP_BUSY			0x8000
+#define MV88E6XXX_TCAM_OP_OP_MASK		0x7000
+#define MV88E6XXX_TCAM_OP_OP_FLUSH_ALL		0x1000
+#define MV88E6XXX_TCAM_OP_OP_FLUSH		0x2000
+#define MV88E6XXX_TCAM_OP_OP_LOAD		0x3000
+#define MV88E6XXX_TCAM_OP_OP_GET_NEXT		0x4000
+#define MV88E6XXX_TCAM_OP_OP_READ		0x5000
+
+/* TCAM extension register */
+#define MV88E6XXX_TCAM_EXTENSION		0x01
+
+/* TCAM keys register 1 */
+#define MV88E6XXX_TCAM_KEYS1			0x02
+#define MV88E6XXX_TCAM_KEYS1_FT_MASK		0xC000
+#define MV88E6XXX_TCAM_KEYS1_SPV_MASK		0x0007
+#define MV88E6XXX_TCAM_KEYS1_SPV_MASK_MASK	0x0700
+
+/* TCAM keys register 2 */
+#define MV88E6XXX_TCAM_KEYS2			0x03
+#define MV88E6XXX_TCAM_KEYS2_SPV_MASK		0x00ff
+#define MV88E6XXX_TCAM_KEYS2_SPV_MASK_MASK	0xff00
+
+#define MV88E6XXX_TCAM_ING_ACT3			0x04
+#define MV88E6XXX_TCAM_ING_ACT3_SF		0x0800
+#define MV88E6XXX_TCAM_ING_ACT3_DPV_MASK	0x07ff
+
+#define MV88E6XXX_TCAM_ING_ACT5			0x06
+#define MV88E6XXX_TCAM_ING_ACT5_DPV_MODE_MASK	0xc000
+
+static int mv88e6xxx_tcam_write(struct mv88e6xxx_chip *chip, int reg, u16 val)
+{
+	return mv88e6xxx_write(chip, chip->info->tcam_addr, reg, val);
+}
+
+static int mv88e6xxx_tcam_wait(struct mv88e6xxx_chip *chip)
+{
+	int bit = __bf_shf(MV88E6XXX_TCAM_OP_BUSY);
+
+	return mv88e6xxx_wait_bit(chip, chip->info->tcam_addr,
+				  MV88E6XXX_TCAM_OP, bit, 0);
+}
+
+static int mv88e6xxx_tcam_read_page(struct mv88e6xxx_chip *chip, u8 page,
+				    u8 entry)
+
+{
+	u16 val = MV88E6XXX_TCAM_OP_BUSY | MV88E6XXX_TCAM_OP_OP_READ |
+		  (page & 0x3) << 10 | entry;
+	int err;
+
+	err = mv88e6xxx_tcam_write(chip, MV88E6XXX_TCAM_OP, val);
+	if (err)
+		return err;
+
+	return mv88e6xxx_tcam_wait(chip);
+}
+
+static int mv88e6xxx_tcam_load_page(struct mv88e6xxx_chip *chip, u8 page,
+				    u8 entry)
+{
+	u16 val = MV88E6XXX_TCAM_OP_BUSY | MV88E6XXX_TCAM_OP_OP_LOAD |
+		  (page & 0x3) << 10 | entry;
+	int err;
+
+	err = mv88e6xxx_tcam_write(chip, MV88E6XXX_TCAM_OP, val);
+	if (err)
+		return err;
+
+	return mv88e6xxx_tcam_wait(chip);
+}
+
+static int mv88e6xxx_tcam_flush_entry(struct mv88e6xxx_chip *chip, u8 entry)
+{
+	u16 val = MV88E6XXX_TCAM_OP_BUSY | MV88E6XXX_TCAM_OP_OP_FLUSH | entry;
+	int err;
+
+	err = mv88e6xxx_tcam_write(chip, MV88E6XXX_TCAM_OP, val);
+	if (err)
+		return err;
+
+	return mv88e6xxx_tcam_wait(chip);
+}
+
+static int mv88e6xxx_tcam_flush_all(struct mv88e6xxx_chip *chip)
+{
+	u16 val = MV88E6XXX_TCAM_OP_BUSY | MV88E6XXX_TCAM_OP_OP_FLUSH_ALL;
+	int err;
+
+	err = mv88e6xxx_tcam_write(chip, MV88E6XXX_TCAM_OP, val);
+	if (err)
+		return err;
+
+	return mv88e6xxx_tcam_wait(chip);
+}
+
+struct mv88e6xxx_tcam_entry *
+mv88e6xxx_tcam_entry_find(struct mv88e6xxx_chip *chip, unsigned long cookie)
+{
+	struct mv88e6xxx_tcam_entry *entry;
+
+	list_for_each_entry(entry, &chip->tcam.entries, list)
+		if (entry->cookie == cookie)
+			return entry;
+
+	return NULL;
+}
+
+/* insert tcam entry in ordered list and move existing entries if necessary */
+static int mv88e6xxx_tcam_insert_entry(struct mv88e6xxx_chip *chip,
+				       struct mv88e6xxx_tcam_entry *entry)
+{
+	struct mv88e6xxx_tcam_entry *elem;
+	struct list_head *hpos;
+	int err;
+
+	list_for_each_prev(hpos, &chip->tcam.entries) {
+		u8 move_idx;
+
+		elem = list_entry(hpos, struct mv88e6xxx_tcam_entry, list);
+		if (entry->prio >= elem->prio)
+			break;
+
+		move_idx = elem->hw_idx + 1;
+
+		err = mv88e6xxx_tcam_flush_entry(chip, move_idx);
+		if (err)
+			return err;
+
+		err = chip->info->ops->tcam_ops->entry_add(chip, elem,
+							   move_idx);
+		if (err)
+			return err;
+
+		elem->hw_idx = move_idx;
+	}
+
+	if (list_is_head(hpos, &chip->tcam.entries)) {
+		entry->hw_idx = 0;
+	} else {
+		elem = list_entry(hpos, struct mv88e6xxx_tcam_entry, list);
+		entry->hw_idx = elem->hw_idx + 1;
+	}
+	list_add(&entry->list, hpos);
+	return 0;
+}
+
+int mv88e6xxx_tcam_entry_add(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_tcam_entry *entry)
+{
+	int err;
+	struct mv88e6xxx_tcam_entry *last;
+
+	last = list_last_entry_or_null(&chip->tcam.entries,
+				       struct mv88e6xxx_tcam_entry, list);
+	if (last && last->hw_idx == chip->info->num_tcam_entries - 1)
+		return -ENOSPC;
+
+	err = mv88e6xxx_tcam_insert_entry(chip, entry);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_tcam_flush_entry(chip, entry->hw_idx);
+	if (err)
+		goto unlink_out;
+
+	err = chip->info->ops->tcam_ops->entry_add(chip, entry, entry->hw_idx);
+	if (err)
+		goto unlink_out;
+
+	return 0;
+
+unlink_out:
+	list_del(&entry->list);
+	return err;
+}
+
+int mv88e6xxx_tcam_entry_del(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_tcam_entry *entry)
+{
+	struct mv88e6xxx_tcam_entry *elem = entry;
+	u8 move_idx = entry->hw_idx;
+	int err;
+
+	err = mv88e6xxx_tcam_flush_entry(chip, entry->hw_idx);
+	if (err)
+		return err;
+
+	/* move entries that come after the deleted entry forward */
+	list_for_each_entry_continue(elem, &chip->tcam.entries, list) {
+		u8 tmp_idx = elem->hw_idx;
+
+		err = mv88e6xxx_tcam_flush_entry(chip, move_idx);
+		if (err)
+			break;
+
+		err = chip->info->ops->tcam_ops->entry_add(chip, elem,
+							   move_idx);
+		if (err)
+			break;
+
+		elem->hw_idx =  move_idx;
+		move_idx = tmp_idx;
+
+		/* flush the last entry after moving entries */
+		if (list_is_last(&elem->list, &chip->tcam.entries))
+			err = mv88e6xxx_tcam_flush_entry(chip, tmp_idx);
+	}
+
+	list_del(&entry->list);
+	kfree(entry);
+	return err;
+}
+
+static int mv88e6390_tcam_entry_add(struct mv88e6xxx_chip *chip,
+				    struct mv88e6xxx_tcam_entry *entry, u8 idx)
+{
+	int err = 0;
+	int i;
+	u16 val, spv_mask, spv;
+
+	err = mv88e6xxx_tcam_read_page(chip, 2, idx);
+	if (err)
+		return err;
+	if (entry->action.dpv_mode != 0) {
+		val = MV88E6XXX_TCAM_ING_ACT3_SF |
+		      (entry->action.dpv & MV88E6XXX_TCAM_ING_ACT3_DPV_MASK);
+
+		err = mv88e6xxx_tcam_write(chip, MV88E6XXX_TCAM_ING_ACT3, val);
+		if (err)
+			return err;
+
+		val = entry->action.dpv_mode << 14;
+		err = mv88e6xxx_tcam_write(chip, MV88E6XXX_TCAM_ING_ACT5, val);
+		if (err)
+			return err;
+	}
+	err = mv88e6xxx_tcam_load_page(chip, 2, idx);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_tcam_read_page(chip, 1, idx);
+	if (err)
+		return err;
+
+	for (i = PAGE0_MATCH_SIZE;
+	     i < PAGE0_MATCH_SIZE + PAGE1_MATCH_SIZE; i++) {
+		if (entry->key.frame_mask[i]) {
+			val = entry->key.frame_mask[i] << 8 |
+			      entry->key.frame_data[i];
+
+			err = mv88e6xxx_tcam_write(chip,
+						   i - PAGE0_MATCH_SIZE + 2,
+						   val);
+			if (err)
+				return err;
+		}
+	}
+	err = mv88e6xxx_tcam_load_page(chip, 1, idx);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_tcam_read_page(chip, 0, idx);
+	if (err)
+		return err;
+
+	for (i = 0; i < PAGE0_MATCH_SIZE; i++) {
+		if (entry->key.frame_mask[i]) {
+			val = entry->key.frame_mask[i] << 8 |
+			      entry->key.frame_data[i];
+
+			err = mv88e6xxx_tcam_write(chip, i + 6, val);
+			if (err)
+				return err;
+		}
+	}
+
+	spv_mask = entry->key.spv_mask & mv88e6xxx_port_mask(chip);
+	spv = entry->key.spv & mv88e6xxx_port_mask(chip);
+	/* frame type mask bits must be set for a valid entry */
+	val = MV88E6XXX_TCAM_KEYS1_FT_MASK |
+	      (spv_mask & MV88E6XXX_TCAM_KEYS1_SPV_MASK_MASK) |
+	      ((spv >> 8) & MV88E6XXX_TCAM_KEYS1_SPV_MASK);
+	err = mv88e6xxx_tcam_write(chip, MV88E6XXX_TCAM_KEYS1, val);
+	if (err)
+		return err;
+
+	val = ((spv_mask << 8) & MV88E6XXX_TCAM_KEYS2_SPV_MASK_MASK) |
+	      (spv & MV88E6XXX_TCAM_KEYS2_SPV_MASK);
+	err = mv88e6xxx_tcam_write(chip, MV88E6XXX_TCAM_KEYS2, val);
+	if (err)
+		return err;
+
+	err = mv88e6xxx_tcam_load_page(chip, 0, idx);
+	if (err)
+		return err;
+
+	entry->hw_idx = idx;
+	return 0;
+}
+
+static int mv88e6393_tcam_entry_add(struct mv88e6xxx_chip *chip,
+				    struct mv88e6xxx_tcam_entry *entry, u8 idx)
+{
+	int err;
+
+	/* select block 0 port 0, then adding an entry is the same as 6390 as
+	 * other blocks aren't used at the moment
+	 */
+	err = mv88e6xxx_tcam_write(chip, MV88E6XXX_TCAM_EXTENSION, 0x00);
+	if (err)
+		return err;
+
+	return mv88e6390_tcam_entry_add(chip, entry, idx);
+}
+
+const struct mv88e6xxx_tcam_ops mv88e6390_tcam_ops = {
+	.entry_add = mv88e6390_tcam_entry_add,
+	.flush_tcam = mv88e6xxx_tcam_flush_all,
+};
+
+const struct mv88e6xxx_tcam_ops mv88e6393_tcam_ops = {
+	.entry_add = mv88e6393_tcam_entry_add,
+	.flush_tcam = mv88e6xxx_tcam_flush_all,
+};
diff --git a/drivers/net/dsa/mv88e6xxx/tcam.h b/drivers/net/dsa/mv88e6xxx/tcam.h
new file mode 100644
index 000000000000..d82bc9bc0768
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/tcam.h
@@ -0,0 +1,41 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/*
+ * Copyright (c) 2026 Luminex Network Intelligence
+ */
+#ifndef _MV88E6XXX_TCAM_H_
+#define _MV88E6XXX_TCAM_H_
+
+#define PAGE0_MATCH_SIZE 22
+#define PAGE1_MATCH_SIZE 26
+
+#define DPV_MODE_NOP		0x0
+#define DPV_MODE_AND		0x1
+#define DPV_MODE_OR		0x2
+#define DPV_MODE_REPLACE	0x3
+
+int mv88e6xxx_tcam_entry_add(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_tcam_entry *entry);
+int mv88e6xxx_tcam_entry_del(struct mv88e6xxx_chip *chip,
+			     struct mv88e6xxx_tcam_entry *entry);
+struct mv88e6xxx_tcam_entry *
+mv88e6xxx_tcam_entry_find(struct mv88e6xxx_chip *chip, unsigned long cookie);
+#define mv88e6xxx_tcam_match_set(key, _offset, data, mask) \
+	do { \
+		typeof(_offset) (offset) = (_offset); \
+		BUILD_BUG_ON((offset) + sizeof((data)) > TCAM_MATCH_SIZE); \
+		__mv88e6xxx_tcam_match_set(key, offset, sizeof(data), \
+					   (u8 *)&(data), (u8 *)&(mask)); \
+	} while (0)
+
+static inline void __mv88e6xxx_tcam_match_set(struct mv88e6xxx_tcam_key *key,
+					      unsigned int offset, size_t size,
+					      u8 *data, u8 *mask)
+{
+	memcpy(&key->frame_data[offset], data, size);
+	memcpy(&key->frame_mask[offset], mask, size);
+}
+
+extern const struct mv88e6xxx_tcam_ops mv88e6390_tcam_ops;
+extern const struct mv88e6xxx_tcam_ops mv88e6393_tcam_ops;
+#endif
diff --git a/drivers/net/dsa/mv88e6xxx/tcflower.c b/drivers/net/dsa/mv88e6xxx/tcflower.c
new file mode 100644
index 000000000000..d67604a55b9f
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/tcflower.c
@@ -0,0 +1,167 @@
+// SPDX-License-Identifier: GPL-2.0-or-later
+/*
+ * Marvell 88E6xxx Switch flower support
+ *
+ * Copyright (c) 2026 Luminex Network Intelligence
+ */
+
+#include "chip.h"
+#include "tcflower.h"
+#include "tcam.h"
+
+#define MV88E6XXX_ETHTYPE_OFFSET 16
+#define MV88E6XXX_IP_PROTO_OFFSET 27
+#define MV88E6XXX_IPV4_SRC_OFFSET 30
+#define MV88E6XXX_IPV4_DST_OFFSET 34
+
+static int mv88e6xxx_flower_parse_key(struct mv88e6xxx_chip *chip,
+				      struct netlink_ext_ack *extack,
+				      struct flow_cls_offload *cls,
+				      struct mv88e6xxx_tcam_key *key)
+{
+	struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
+	struct flow_dissector *dissector = rule->match.dissector;
+	u16 addr_type = 0;
+
+	if (dissector->used_keys &
+	    ~(BIT_ULL(FLOW_DISSECTOR_KEY_BASIC) |
+	      BIT_ULL(FLOW_DISSECTOR_KEY_CONTROL) |
+	      BIT_ULL(FLOW_DISSECTOR_KEY_IPV4_ADDRS))) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Unsupported keys used");
+		return -EOPNOTSUPP;
+	}
+
+	if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_CONTROL)) {
+		struct flow_match_control match;
+
+		flow_rule_match_control(rule, &match);
+		addr_type = match.key->addr_type;
+
+		if (flow_rule_has_control_flags(match.mask->flags,
+						cls->common.extack))
+			return -EOPNOTSUPP;
+	}
+	if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_BASIC)) {
+		struct flow_match_basic match;
+
+		flow_rule_match_basic(rule, &match);
+		mv88e6xxx_tcam_match_set(key, MV88E6XXX_ETHTYPE_OFFSET,
+					 match.key->n_proto,
+					 match.mask->n_proto);
+		mv88e6xxx_tcam_match_set(key, MV88E6XXX_IP_PROTO_OFFSET,
+					 match.key->ip_proto,
+					 match.mask->ip_proto);
+	}
+
+	if (addr_type == FLOW_DISSECTOR_KEY_IPV4_ADDRS) {
+		struct flow_match_ipv4_addrs match;
+
+		flow_rule_match_ipv4_addrs(cls->rule, &match);
+		mv88e6xxx_tcam_match_set(key, MV88E6XXX_IPV4_SRC_OFFSET,
+					 match.key->src,
+					 match.mask->src);
+		mv88e6xxx_tcam_match_set(key, MV88E6XXX_IPV4_DST_OFFSET,
+					 match.key->dst,
+					 match.mask->dst);
+	}
+
+	return 0;
+}
+
+int mv88e6xxx_cls_flower_add(struct dsa_switch *ds, int port,
+			     struct flow_cls_offload *cls, bool ingress)
+{
+	struct flow_rule *rule = flow_cls_offload_flow_rule(cls);
+	struct netlink_ext_ack *extack = cls->common.extack;
+	struct mv88e6xxx_chip *chip = ds->priv;
+	struct mv88e6xxx_tcam_key key = { 0 };
+	const struct flow_action_entry *act;
+	unsigned long cookie = cls->cookie;
+	struct mv88e6xxx_tcam_entry *entry;
+	int err, i;
+
+	if (!mv88e6xxx_has_tcam(chip)) {
+		NL_SET_ERR_MSG_MOD(extack, "hardware offload not supported");
+		return -EOPNOTSUPP;
+	}
+
+	err = mv88e6xxx_flower_parse_key(chip, extack, cls, &key);
+	if (err)
+		return err;
+
+	mv88e6xxx_reg_lock(chip);
+	entry = mv88e6xxx_tcam_entry_find(chip, cookie);
+	if (entry) {
+		err = -EEXIST;
+		goto err_unlock;
+	}
+
+	entry = kzalloc(sizeof(*entry), GFP_KERNEL);
+	if (!entry) {
+		err = -ENOMEM;
+		goto err_unlock;
+	}
+
+	entry->cookie = cookie;
+	entry->prio = cls->common.prio;
+	entry->key = key;
+
+	flow_action_for_each(i, act, &rule->action) {
+		switch (act->id) {
+		case FLOW_ACTION_TRAP: {
+			int cpu = dsa_upstream_port(ds, port);
+
+			entry->action.dpv_mode = DPV_MODE_REPLACE;
+			entry->action.dpv = BIT(cpu);
+			break;
+		}
+		default:
+			NL_SET_ERR_MSG_MOD(extack, "action not supported");
+			err = -EOPNOTSUPP;
+			goto err_free_entry;
+		}
+	}
+
+	entry->key.spv = BIT(port);
+	entry->key.spv_mask = mv88e6xxx_port_mask(chip);
+
+	err = mv88e6xxx_tcam_entry_add(chip, entry);
+	if (err)
+		goto err_free_entry;
+
+	mv88e6xxx_reg_unlock(chip);
+	return  0;
+
+err_free_entry:
+	kfree(entry);
+err_unlock:
+	mv88e6xxx_reg_unlock(chip);
+	return err;
+}
+
+int mv88e6xxx_cls_flower_del(struct dsa_switch *ds, int port,
+			     struct flow_cls_offload *cls, bool ingress)
+{
+	struct mv88e6xxx_chip *chip = ds->priv;
+	struct mv88e6xxx_tcam_entry *entry;
+	int err = 0;
+
+	mv88e6xxx_reg_lock(chip);
+	entry = mv88e6xxx_tcam_entry_find(chip, cls->cookie);
+
+	if (entry)
+		err = mv88e6xxx_tcam_entry_del(chip, entry);
+	mv88e6xxx_reg_unlock(chip);
+	return  err;
+}
+
+void mv88e6xxx_flower_teardown(struct mv88e6xxx_chip *chip)
+{
+	struct mv88e6xxx_tcam_entry *pos, *n;
+
+	list_for_each_entry_safe(pos, n, &chip->tcam.entries, list) {
+		list_del(&pos->list);
+		kfree(pos);
+	}
+}
diff --git a/drivers/net/dsa/mv88e6xxx/tcflower.h b/drivers/net/dsa/mv88e6xxx/tcflower.h
new file mode 100644
index 000000000000..fb79f12fe18c
--- /dev/null
+++ b/drivers/net/dsa/mv88e6xxx/tcflower.h
@@ -0,0 +1,14 @@
+/* SPDX-License-Identifier: GPL-2.0-or-later */
+
+/*
+ * Copyright (c) 2026 Luminex Network Intelligence
+ */
+#ifndef _MV88E6XXX_TCFLOWER_H_
+#define _MV88E6XXX_TCFLOWER_H_
+
+int mv88e6xxx_cls_flower_add(struct dsa_switch *ds, int port,
+			     struct flow_cls_offload *cls, bool ingress);
+int mv88e6xxx_cls_flower_del(struct dsa_switch *ds, int port,
+			     struct flow_cls_offload *cls, bool ingress);
+void mv88e6xxx_flower_teardown(struct mv88e6xxx_chip *chip);
+#endif

-- 
2.43.0

Re: [PATCH net-next v8 1/2] net/sched: cls_flower: remove unions from fl_flow_key

[Simon Horman]

On Wed, Mar 11, 2026 at 11:46:18AM +0100, Cedric Jehasse via B4 Relay wrote:
> From: Cedric Jehasse <cedric.jehasse@luminex.be>
> 
> When creating a flower classifier with an ipv4 address the
> flow_dissector has both FLOW_DISSECTOR_KEY_IPV4_ADDRS and
> FLOW_DISSECTOR_KEY_IPV6_ADDRS bits set in used_keys.
> This happens because ipv4/ipv6 fields are a union and
> FL_KEY_SET_IF_MASKED() will interpret either being set as both.
> 
> Removing the unions fixes this behavior without needing special handling
> for union fields.
> 
> Example of a command that caused FLOW_DISSECTOR_KEY_IPV4_ADDRS and
> FLOW_DISSECTOR_KEY_IPV6_ADDRS to be set:
> tc filter add dev p1 ingress protocol ip flower skip_sw \
>     dst_ip 224.0.1.129 action trap
> 
> Signed-off-by: Cedric Jehasse <cedric.jehasse@luminex.be>

Hi Cedric,

This doesn't seem to be the right approach to me.

It seems to me that the use of a union is intentional here, as either IPv4
or IPv6 addresses can be present in each case - never both.  And that
control.addr_type and enc_control.addr_type are intended to allow
differentiation of the address type in use for each of these unions.

> ---
>  net/sched/cls_flower.c | 12 ++++--------
>  1 file changed, 4 insertions(+), 8 deletions(-)
> 
> diff --git a/net/sched/cls_flower.c b/net/sched/cls_flower.c
> index 7669371c1354..b95dbe847dde 100644
> --- a/net/sched/cls_flower.c
> +++ b/net/sched/cls_flower.c
> @@ -59,18 +59,14 @@ struct fl_flow_key {
>  	struct flow_dissector_key_eth_addrs eth;
>  	struct flow_dissector_key_vlan vlan;
>  	struct flow_dissector_key_vlan cvlan;
> -	union {
> -		struct flow_dissector_key_ipv4_addrs ipv4;
> -		struct flow_dissector_key_ipv6_addrs ipv6;
> -	};
> +	struct flow_dissector_key_ipv4_addrs ipv4;
> +	struct flow_dissector_key_ipv6_addrs ipv6;
>  	struct flow_dissector_key_ports tp;
>  	struct flow_dissector_key_icmp icmp;
>  	struct flow_dissector_key_arp arp;
>  	struct flow_dissector_key_keyid enc_key_id;
> -	union {
> -		struct flow_dissector_key_ipv4_addrs enc_ipv4;
> -		struct flow_dissector_key_ipv6_addrs enc_ipv6;
> -	};
> +	struct flow_dissector_key_ipv4_addrs enc_ipv4;
> +	struct flow_dissector_key_ipv6_addrs enc_ipv6;
>  	struct flow_dissector_key_ports enc_tp;
>  	struct flow_dissector_key_mpls mpls;
>  	struct flow_dissector_key_tcp tcp;
> 
> -- 
> 2.43.0
> 
> 

Re: [PATCH net-next v8 1/2] net/sched: cls_flower: remove unions from fl_flow_key

[Cedric Jehasse]

>On Wed, Mar 11, 2026 at 11:46:18AM +0100, Cedric Jehasse via B4 Relay wrote:
>> From: Cedric Jehasse <cedric.jehasse@luminex.be>
>>
>> When creating a flower classifier with an ipv4 address the
>> flow_dissector has both FLOW_DISSECTOR_KEY_IPV4_ADDRS and
>> FLOW_DISSECTOR_KEY_IPV6_ADDRS bits set in used_keys.
>> This happens because ipv4/ipv6 fields are a union and
>> FL_KEY_SET_IF_MASKED() will interpret either being set as both.
>>
>> Removing the unions fixes this behavior without needing special handling
>> for union fields.
>>
>> Example of a command that caused FLOW_DISSECTOR_KEY_IPV4_ADDRS and
>> FLOW_DISSECTOR_KEY_IPV6_ADDRS to be set:
>> tc filter add dev p1 ingress protocol ip flower skip_sw \
>>     dst_ip 224.0.1.129 action trap
>>
>> Signed-off-by: Cedric Jehasse <cedric.jehasse@luminex.be>
>
>Hi Cedric,
>
>This doesn't seem to be the right approach to me.
>
>It seems to me that the use of a union is intentional here, as either IPv4
>or IPv6 addresses can be present in each case - never both.  And that
>control.addr_type and enc_control.addr_type are intended to allow
>differentiation of the address type in use for each of these unions.

I chose this approach because keeping the unions required special handling for
these union keys. Removing the union it keeps the code simpler.
I understand the union exists because IPv4 and IPv6 addresses are mutually
exclusive, but some other keys are also mutually exclusive (e.g. arp and icmp
key wil never be present at the same time)

Re: [PATCH net-next v8 1/2] net/sched: cls_flower: remove unions from fl_flow_key

[Jakub Kicinski]

On Fri, 13 Mar 2026 13:26:51 +0000 Simon Horman wrote:
> It seems to me that the use of a union is intentional here, as either IPv4
> or IPv6 addresses can be present in each case - never both.  And that
> control.addr_type and enc_control.addr_type are intended to allow
> differentiation of the address type in use for each of these unions.

My reading was that the initial author simply wanted to save space in
the struct.

As the commit message explains this leads to complications in the logic
which sets the keys. The alternative is to complicate
FL_KEY_SET_IF_MASKED - doable, but given that the union feels like a
micro-optimization in the first place the simpler approach of separating
fields seems okay too? (TBH my mind also initially went down the
FL_KEY_SET_IF_MASKED rabbit hole but once I saw the simplicity of
Cedric's patch I changed my mind)

Re: [PATCH net-next v8 1/2] net/sched: cls_flower: remove unions from fl_flow_key

[Simon Horman]

On Sat, Mar 14, 2026 at 10:00:02AM -0700, Jakub Kicinski wrote:
> On Fri, 13 Mar 2026 13:26:51 +0000 Simon Horman wrote:
> > It seems to me that the use of a union is intentional here, as either IPv4
> > or IPv6 addresses can be present in each case - never both.  And that
> > control.addr_type and enc_control.addr_type are intended to allow
> > differentiation of the address type in use for each of these unions.
> 
> My reading was that the initial author simply wanted to save space in
> the struct.
> 
> As the commit message explains this leads to complications in the logic
> which sets the keys. The alternative is to complicate
> FL_KEY_SET_IF_MASKED - doable, but given that the union feels like a
> micro-optimization in the first place the simpler approach of separating
> fields seems okay too? (TBH my mind also initially went down the
> FL_KEY_SET_IF_MASKED rabbit hole but once I saw the simplicity of
> Cedric's patch I changed my mind)

Sure, now this has been put to me more than once I agree.

But if we go this way, then can we also simplify some of the existing logic?
As a follow-up?

Re: [PATCH net-next v8 1/2] net/sched: cls_flower: remove unions from fl_flow_key

[Jakub Kicinski]

On Mon, 16 Mar 2026 08:34:47 +0000 Simon Horman wrote:
> On Sat, Mar 14, 2026 at 10:00:02AM -0700, Jakub Kicinski wrote:
> > On Fri, 13 Mar 2026 13:26:51 +0000 Simon Horman wrote:  
> > > It seems to me that the use of a union is intentional here, as either IPv4
> > > or IPv6 addresses can be present in each case - never both.  And that
> > > control.addr_type and enc_control.addr_type are intended to allow
> > > differentiation of the address type in use for each of these unions.  
> > 
> > My reading was that the initial author simply wanted to save space in
> > the struct.
> > 
> > As the commit message explains this leads to complications in the logic
> > which sets the keys. The alternative is to complicate
> > FL_KEY_SET_IF_MASKED - doable, but given that the union feels like a
> > micro-optimization in the first place the simpler approach of separating
> > fields seems okay too? (TBH my mind also initially went down the
> > FL_KEY_SET_IF_MASKED rabbit hole but once I saw the simplicity of
> > Cedric's patch I changed my mind)  
> 
> Sure, now this has been put to me more than once I agree.
> 
> But if we go this way, then can we also simplify some of the existing logic?
> As a follow-up?

Which logic do you have in mind? Sorry if I'm being slow.

Re: [PATCH net-next v8 0/2] net: dsa: mv88e6xxx: Add partial support for TCAM entries

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Wed, 11 Mar 2026 11:46:17 +0100 you wrote:
> This series adds partial Ternary Content Addressable Memory (TCAM) for
> the mv88e6390 and mv88e6393 family of switches. TCAM entries allow the
> switch to match the first 48 or 96 bytes of a frame and take actions on
> matched frames.
> 
> This patch introduces a subset of the available TCAM functionality.
> Matching on ip addresses/protocol and trapping to the cpu.
> 
> [...]

Here is the summary with links:
  - [net-next,v8,1/2] net/sched: cls_flower: remove unions from fl_flow_key
    https://git.kernel.org/netdev/net-next/c/b513dde96c0a
  - [net-next,v8,2/2] net: dsa: mv88e6xxx: Add partial support for TCAM entries
    https://git.kernel.org/netdev/net-next/c/639f1dcfde55

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH net-next v8 1/2] net/sched: cls_flower: remove unions from fl_flow_key

[Simon Horman]

On Mon, Mar 16, 2026 at 04:19:17PM -0700, Jakub Kicinski wrote:
> On Mon, 16 Mar 2026 08:34:47 +0000 Simon Horman wrote:
> > On Sat, Mar 14, 2026 at 10:00:02AM -0700, Jakub Kicinski wrote:
> > > On Fri, 13 Mar 2026 13:26:51 +0000 Simon Horman wrote:  
> > > > It seems to me that the use of a union is intentional here, as either IPv4
> > > > or IPv6 addresses can be present in each case - never both.  And that
> > > > control.addr_type and enc_control.addr_type are intended to allow
> > > > differentiation of the address type in use for each of these unions.  
> > > 
> > > My reading was that the initial author simply wanted to save space in
> > > the struct.
> > > 
> > > As the commit message explains this leads to complications in the logic
> > > which sets the keys. The alternative is to complicate
> > > FL_KEY_SET_IF_MASKED - doable, but given that the union feels like a
> > > micro-optimization in the first place the simpler approach of separating
> > > fields seems okay too? (TBH my mind also initially went down the
> > > FL_KEY_SET_IF_MASKED rabbit hole but once I saw the simplicity of
> > > Cedric's patch I changed my mind)  
> > 
> > Sure, now this has been put to me more than once I agree.
> > 
> > But if we go this way, then can we also simplify some of the existing logic?
> > As a follow-up?
> 
> Which logic do you have in mind? Sorry if I'm being slow.

Sorry for being vague.
Let me do my homework and come back if I have a concrete suggestion.