[PATCH net-next 0/3] Add L2 hw acceleration for airoha_eth driver

[PATCH net-next 0/3] Add L2 hw acceleration for airoha_eth driver

[Lorenzo Bianconi]

Introduce the capability to offload L2 traffic defining flower rules in
the PSE/PPE engine available on EN7581 SoC.
Since the hw always reports L2/L3/L4 flower rules, link all L2 rules
sharing the same L2 info (with different L3/L4 info) in the L2 subflows
list of a given L2 PPE entry.

---
Lorenzo Bianconi (3):
      net: airoha: Add l2_flows rhashtable
      net: airoha: Add airoha_ppe_foe_flow_remove_entry_locked()
      net: airoha: Add L2 hw acceleration support

 drivers/net/ethernet/airoha/airoha_eth.c |   2 +-
 drivers/net/ethernet/airoha/airoha_eth.h |  22 ++-
 drivers/net/ethernet/airoha/airoha_ppe.c | 224 ++++++++++++++++++++++++++-----
 3 files changed, 215 insertions(+), 33 deletions(-)
---
base-commit: 61f96e684edd28ca40555ec49ea1555df31ba619
change-id: 20250313-airoha-flowtable-l2b-e0b50d4a3215

Best regards,
-- 
Lorenzo Bianconi <lorenzo@kernel.org>

[PATCH net-next 1/3] net: airoha: Add l2_flows rhashtable

[Lorenzo Bianconi]

Introduce l2_flows rhashtable in airoha_ppe struct in order to
store L2 flows committed by upper layers of the kernel. This is a
preliminary patch in order to offload L2 traffic rules.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.h | 15 ++++++-
 drivers/net/ethernet/airoha/airoha_ppe.c | 67 +++++++++++++++++++++++++++-----
 2 files changed, 72 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
index ec8908f904c61988c3dc973e187596c49af139fb..57925648155b104021c10821096ba267c9c7cef6 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.h
+++ b/drivers/net/ethernet/airoha/airoha_eth.h
@@ -422,12 +422,23 @@ struct airoha_flow_data {
 	} pppoe;
 };
 
+enum airoha_flow_entry_type {
+	FLOW_TYPE_L4,
+	FLOW_TYPE_L2,
+	FLOW_TYPE_L2_SUBFLOW,
+};
+
 struct airoha_flow_table_entry {
-	struct hlist_node list;
+	union {
+		struct hlist_node list;
+		struct rhash_head l2_node;
+	};
 
 	struct airoha_foe_entry data;
 	u32 hash;
 
+	enum airoha_flow_entry_type type;
+
 	struct rhash_head node;
 	unsigned long cookie;
 };
@@ -480,6 +491,8 @@ struct airoha_ppe {
 	void *foe;
 	dma_addr_t foe_dma;
 
+	struct rhashtable l2_flows;
+
 	struct hlist_head *foe_flow;
 	u16 foe_check_time[PPE_NUM_ENTRIES];
 
diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
index f10dab935cab6fad747fdfaa70b67903904c1703..aed4a22f3a8b8737f18509b48fc47eae594b9d5f 100644
--- a/drivers/net/ethernet/airoha/airoha_ppe.c
+++ b/drivers/net/ethernet/airoha/airoha_ppe.c
@@ -24,6 +24,13 @@ static const struct rhashtable_params airoha_flow_table_params = {
 	.automatic_shrinking = true,
 };
 
+static const struct rhashtable_params airoha_l2_flow_table_params = {
+	.head_offset = offsetof(struct airoha_flow_table_entry, l2_node),
+	.key_offset = offsetof(struct airoha_flow_table_entry, data.bridge),
+	.key_len = 2 * ETH_ALEN,
+	.automatic_shrinking = true,
+};
+
 static bool airoha_ppe2_is_enabled(struct airoha_eth *eth)
 {
 	return airoha_fe_rr(eth, REG_PPE_GLO_CFG(1)) & PPE_GLO_CFG_EN_MASK;
@@ -505,11 +512,36 @@ static void airoha_ppe_foe_insert_entry(struct airoha_ppe *ppe, u32 hash)
 	spin_unlock_bh(&ppe_lock);
 }
 
+static int
+airoha_ppe_foe_l2_flow_commit_entry(struct airoha_ppe *ppe,
+				    struct airoha_flow_table_entry *e)
+{
+	struct airoha_flow_table_entry *prev;
+
+	e->type = FLOW_TYPE_L2;
+	prev = rhashtable_lookup_get_insert_fast(&ppe->l2_flows, &e->l2_node,
+						 airoha_l2_flow_table_params);
+	if (!prev)
+		return 0;
+
+	if (IS_ERR(prev))
+		return PTR_ERR(prev);
+
+	return rhashtable_replace_fast(&ppe->l2_flows, &prev->l2_node,
+				       &e->l2_node,
+				       airoha_l2_flow_table_params);
+}
+
 static int airoha_ppe_foe_flow_commit_entry(struct airoha_ppe *ppe,
 					    struct airoha_flow_table_entry *e)
 {
-	u32 hash = airoha_ppe_foe_get_entry_hash(&e->data);
+	int type = FIELD_GET(AIROHA_FOE_IB1_BIND_PACKET_TYPE, e->data.ib1);
+	u32 hash;
 
+	if (type == PPE_PKT_TYPE_BRIDGE)
+		return airoha_ppe_foe_l2_flow_commit_entry(ppe, e);
+
+	hash = airoha_ppe_foe_get_entry_hash(&e->data);
 	e->hash = 0xffff;
 
 	spin_lock_bh(&ppe_lock);
@@ -524,13 +556,18 @@ static void airoha_ppe_foe_flow_remove_entry(struct airoha_ppe *ppe,
 {
 	spin_lock_bh(&ppe_lock);
 
-	hlist_del_init(&e->list);
-	if (e->hash != 0xffff) {
-		e->data.ib1 &= ~AIROHA_FOE_IB1_BIND_STATE;
-		e->data.ib1 |= FIELD_PREP(AIROHA_FOE_IB1_BIND_STATE,
-					  AIROHA_FOE_STATE_INVALID);
-		airoha_ppe_foe_commit_entry(ppe, &e->data, e->hash);
-		e->hash = 0xffff;
+	if (e->type == FLOW_TYPE_L2) {
+		rhashtable_remove_fast(&ppe->l2_flows, &e->l2_node,
+				       airoha_l2_flow_table_params);
+	} else {
+		hlist_del_init(&e->list);
+		if (e->hash != 0xffff) {
+			e->data.ib1 &= ~AIROHA_FOE_IB1_BIND_STATE;
+			e->data.ib1 |= FIELD_PREP(AIROHA_FOE_IB1_BIND_STATE,
+						  AIROHA_FOE_STATE_INVALID);
+			airoha_ppe_foe_commit_entry(ppe, &e->data, e->hash);
+			e->hash = 0xffff;
+		}
 	}
 
 	spin_unlock_bh(&ppe_lock);
@@ -890,9 +927,20 @@ int airoha_ppe_init(struct airoha_eth *eth)
 	if (err)
 		return err;
 
+	err = rhashtable_init(&ppe->l2_flows, &airoha_l2_flow_table_params);
+	if (err)
+		goto error_flow_table_destroy;
+
 	err = airoha_ppe_debugfs_init(ppe);
 	if (err)
-		rhashtable_destroy(&eth->flow_table);
+		goto error_l2_flow_table_destroy;
+
+	return 0;
+
+error_l2_flow_table_destroy:
+	rhashtable_destroy(&ppe->l2_flows);
+error_flow_table_destroy:
+	rhashtable_destroy(&eth->flow_table);
 
 	return err;
 }
@@ -909,6 +957,7 @@ void airoha_ppe_deinit(struct airoha_eth *eth)
 	}
 	rcu_read_unlock();
 
+	rhashtable_destroy(&eth->ppe->l2_flows);
 	rhashtable_destroy(&eth->flow_table);
 	debugfs_remove(eth->ppe->debugfs_dir);
 }

-- 
2.49.0

[PATCH net-next 2/3] net: airoha: Add airoha_ppe_foe_flow_remove_entry_locked()

[Lorenzo Bianconi]

Introduce airoha_ppe_foe_flow_remove_entry_locked utility routine
in order to run airoha_ppe_foe_flow_remove_entry holding ppe_lock.
This is a preliminary patch to L2 offloading support to airoha_eth
driver.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_ppe.c | 45 ++++++++++++++++++--------------
 1 file changed, 26 insertions(+), 19 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
index aed4a22f3a8b8737f18509b48fc47eae594b9d5f..8f75752c6714cc211a8efd0b6fdf5565ffa23c14 100644
--- a/drivers/net/ethernet/airoha/airoha_ppe.c
+++ b/drivers/net/ethernet/airoha/airoha_ppe.c
@@ -483,6 +483,26 @@ static int airoha_ppe_foe_commit_entry(struct airoha_ppe *ppe,
 	return 0;
 }
 
+static void airoha_ppe_foe_flow_remove_entry(struct airoha_ppe *ppe,
+					     struct airoha_flow_table_entry *e)
+{
+	lockdep_assert_held(&ppe_lock);
+
+	if (e->type == FLOW_TYPE_L2) {
+		rhashtable_remove_fast(&ppe->l2_flows, &e->l2_node,
+				       airoha_l2_flow_table_params);
+	} else {
+		hlist_del_init(&e->list);
+		if (e->hash != 0xffff) {
+			e->data.ib1 &= ~AIROHA_FOE_IB1_BIND_STATE;
+			e->data.ib1 |= FIELD_PREP(AIROHA_FOE_IB1_BIND_STATE,
+						  AIROHA_FOE_STATE_INVALID);
+			airoha_ppe_foe_commit_entry(ppe, &e->data, e->hash);
+			e->hash = 0xffff;
+		}
+	}
+}
+
 static void airoha_ppe_foe_insert_entry(struct airoha_ppe *ppe, u32 hash)
 {
 	struct airoha_flow_table_entry *e;
@@ -551,25 +571,12 @@ static int airoha_ppe_foe_flow_commit_entry(struct airoha_ppe *ppe,
 	return 0;
 }
 
-static void airoha_ppe_foe_flow_remove_entry(struct airoha_ppe *ppe,
-					     struct airoha_flow_table_entry *e)
+static void
+airoha_ppe_foe_flow_remove_entry_locked(struct airoha_ppe *ppe,
+					struct airoha_flow_table_entry *e)
 {
 	spin_lock_bh(&ppe_lock);
-
-	if (e->type == FLOW_TYPE_L2) {
-		rhashtable_remove_fast(&ppe->l2_flows, &e->l2_node,
-				       airoha_l2_flow_table_params);
-	} else {
-		hlist_del_init(&e->list);
-		if (e->hash != 0xffff) {
-			e->data.ib1 &= ~AIROHA_FOE_IB1_BIND_STATE;
-			e->data.ib1 |= FIELD_PREP(AIROHA_FOE_IB1_BIND_STATE,
-						  AIROHA_FOE_STATE_INVALID);
-			airoha_ppe_foe_commit_entry(ppe, &e->data, e->hash);
-			e->hash = 0xffff;
-		}
-	}
-
+	airoha_ppe_foe_flow_remove_entry(ppe, e);
 	spin_unlock_bh(&ppe_lock);
 }
 
@@ -762,7 +769,7 @@ static int airoha_ppe_flow_offload_replace(struct airoha_gdm_port *port,
 	return 0;
 
 remove_foe_entry:
-	airoha_ppe_foe_flow_remove_entry(eth->ppe, e);
+	airoha_ppe_foe_flow_remove_entry_locked(eth->ppe, e);
 free_entry:
 	kfree(e);
 
@@ -780,7 +787,7 @@ static int airoha_ppe_flow_offload_destroy(struct airoha_gdm_port *port,
 	if (!e)
 		return -ENOENT;
 
-	airoha_ppe_foe_flow_remove_entry(eth->ppe, e);
+	airoha_ppe_foe_flow_remove_entry_locked(eth->ppe, e);
 	rhashtable_remove_fast(&eth->flow_table, &e->node,
 			       airoha_flow_table_params);
 	kfree(e);

-- 
2.49.0

[PATCH net-next 3/3] net: airoha: Add L2 hw acceleration support

[Lorenzo Bianconi]

Similar to mtk driver, introduce the capability to offload L2 traffic
defining flower rules in the PSE/PPE engine available on EN7581 SoC.
Since the hw always reports L2/L3/L4 flower rules, link all L2 rules
sharing the same L2 info (with different L3/L4 info) in the L2 subflows
list of a given L2 PPE entry.

Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
---
 drivers/net/ethernet/airoha/airoha_eth.c |   2 +-
 drivers/net/ethernet/airoha/airoha_eth.h |  11 ++-
 drivers/net/ethernet/airoha/airoha_ppe.c | 162 +++++++++++++++++++++++++------
 3 files changed, 144 insertions(+), 31 deletions(-)

diff --git a/drivers/net/ethernet/airoha/airoha_eth.c b/drivers/net/ethernet/airoha/airoha_eth.c
index d748dc6de92367365db9f9548f9af52a7fdac187..723eba7cfa0cf405dc2ef9248b34bf6af897ceed 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.c
+++ b/drivers/net/ethernet/airoha/airoha_eth.c
@@ -694,7 +694,7 @@ static int airoha_qdma_rx_process(struct airoha_queue *q, int budget)
 
 		reason = FIELD_GET(AIROHA_RXD4_PPE_CPU_REASON, msg1);
 		if (reason == PPE_CPU_REASON_HIT_UNBIND_RATE_REACHED)
-			airoha_ppe_check_skb(eth->ppe, hash);
+			airoha_ppe_check_skb(eth->ppe, q->skb, hash);
 
 		done++;
 		napi_gro_receive(&q->napi, q->skb);
diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
index 57925648155b104021c10821096ba267c9c7cef6..e82abfc1a67bda7f675342327ae07601d81f2b8f 100644
--- a/drivers/net/ethernet/airoha/airoha_eth.h
+++ b/drivers/net/ethernet/airoha/airoha_eth.h
@@ -430,11 +430,15 @@ enum airoha_flow_entry_type {
 
 struct airoha_flow_table_entry {
 	union {
-		struct hlist_node list;
-		struct rhash_head l2_node;
+		struct hlist_node list; /* PPE L3 flow entry */
+		struct {
+			struct rhash_head l2_node;  /* L2 flow entry */
+			struct hlist_head l2_flows; /* PPE L2 subflows list */
+		};
 	};
 
 	struct airoha_foe_entry data;
+	struct hlist_node l2_subflow_node; /* PPE L2 subflow entry */
 	u32 hash;
 
 	enum airoha_flow_entry_type type;
@@ -548,7 +552,8 @@ u32 airoha_rmw(void __iomem *base, u32 offset, u32 mask, u32 val);
 bool airoha_is_valid_gdm_port(struct airoha_eth *eth,
 			      struct airoha_gdm_port *port);
 
-void airoha_ppe_check_skb(struct airoha_ppe *ppe, u16 hash);
+void airoha_ppe_check_skb(struct airoha_ppe *ppe, struct sk_buff *skb,
+			  u16 hash);
 int airoha_ppe_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
 				 void *cb_priv);
 int airoha_ppe_init(struct airoha_eth *eth);
diff --git a/drivers/net/ethernet/airoha/airoha_ppe.c b/drivers/net/ethernet/airoha/airoha_ppe.c
index 8f75752c6714cc211a8efd0b6fdf5565ffa23c14..e95a7b727dda2e0182e0ddb567cb13fb746cd455 100644
--- a/drivers/net/ethernet/airoha/airoha_ppe.c
+++ b/drivers/net/ethernet/airoha/airoha_ppe.c
@@ -204,6 +204,15 @@ static int airoha_get_dsa_port(struct net_device **dev)
 #endif
 }
 
+static void airoha_ppe_foe_set_bridge_addrs(struct airoha_foe_bridge *br,
+					    struct ethhdr *eh)
+{
+	br->dest_mac_hi = get_unaligned_be32(eh->h_dest);
+	br->dest_mac_lo = get_unaligned_be16(eh->h_dest + 4);
+	br->src_mac_hi = get_unaligned_be16(eh->h_source);
+	br->src_mac_lo = get_unaligned_be32(eh->h_source + 2);
+}
+
 static int airoha_ppe_foe_entry_prepare(struct airoha_eth *eth,
 					struct airoha_foe_entry *hwe,
 					struct net_device *dev, int type,
@@ -254,13 +263,7 @@ static int airoha_ppe_foe_entry_prepare(struct airoha_eth *eth,
 
 	qdata = FIELD_PREP(AIROHA_FOE_SHAPER_ID, 0x7f);
 	if (type == PPE_PKT_TYPE_BRIDGE) {
-		hwe->bridge.dest_mac_hi = get_unaligned_be32(data->eth.h_dest);
-		hwe->bridge.dest_mac_lo =
-			get_unaligned_be16(data->eth.h_dest + 4);
-		hwe->bridge.src_mac_hi =
-			get_unaligned_be16(data->eth.h_source);
-		hwe->bridge.src_mac_lo =
-			get_unaligned_be32(data->eth.h_source + 2);
+		airoha_ppe_foe_set_bridge_addrs(&hwe->bridge, &data->eth);
 		hwe->bridge.data = qdata;
 		hwe->bridge.ib2 = val;
 		l2 = &hwe->bridge.l2.common;
@@ -385,6 +388,19 @@ static u32 airoha_ppe_foe_get_entry_hash(struct airoha_foe_entry *hwe)
 		hv3 = hwe->ipv6.src_ip[1] ^ hwe->ipv6.dest_ip[1];
 		hv3 ^= hwe->ipv6.src_ip[0];
 		break;
+	case PPE_PKT_TYPE_BRIDGE: {
+		struct airoha_foe_mac_info *l2 = &hwe->bridge.l2;
+
+		hv1 = l2->common.src_mac_hi & 0xffff;
+		hv1 = hv1 << 16 | l2->src_mac_lo;
+
+		hv2 = l2->common.dest_mac_lo;
+		hv2 = hv2 << 16;
+		hv2 = hv2 | ((l2->common.src_mac_hi & 0xffff0000) >> 16);
+
+		hv3 = l2->common.dest_mac_hi;
+		break;
+	}
 	case PPE_PKT_TYPE_IPV4_DSLITE:
 	case PPE_PKT_TYPE_IPV6_6RD:
 	default:
@@ -483,30 +499,100 @@ static int airoha_ppe_foe_commit_entry(struct airoha_ppe *ppe,
 	return 0;
 }
 
+static void airoha_ppe_foe_remove_flow(struct airoha_ppe *ppe,
+				       struct airoha_flow_table_entry *e)
+{
+	lockdep_assert_held(&ppe_lock);
+
+	hlist_del_init(&e->list);
+	if (e->hash != 0xffff) {
+		e->data.ib1 &= ~AIROHA_FOE_IB1_BIND_STATE;
+		e->data.ib1 |= FIELD_PREP(AIROHA_FOE_IB1_BIND_STATE,
+					  AIROHA_FOE_STATE_INVALID);
+		airoha_ppe_foe_commit_entry(ppe, &e->data, e->hash);
+		e->hash = 0xffff;
+	}
+	if (e->type == FLOW_TYPE_L2_SUBFLOW) {
+		hlist_del_init(&e->l2_subflow_node);
+		kfree(e);
+	}
+}
+
+static void airoha_ppe_foe_remove_l2_flow(struct airoha_ppe *ppe,
+					  struct airoha_flow_table_entry *e)
+{
+	struct hlist_head *head = &e->l2_flows;
+	struct hlist_node *n;
+
+	lockdep_assert_held(&ppe_lock);
+
+	rhashtable_remove_fast(&ppe->l2_flows, &e->l2_node,
+			       airoha_l2_flow_table_params);
+	hlist_for_each_entry_safe(e, n, head, l2_subflow_node)
+		airoha_ppe_foe_remove_flow(ppe, e);
+}
+
 static void airoha_ppe_foe_flow_remove_entry(struct airoha_ppe *ppe,
 					     struct airoha_flow_table_entry *e)
 {
 	lockdep_assert_held(&ppe_lock);
 
-	if (e->type == FLOW_TYPE_L2) {
-		rhashtable_remove_fast(&ppe->l2_flows, &e->l2_node,
-				       airoha_l2_flow_table_params);
-	} else {
-		hlist_del_init(&e->list);
-		if (e->hash != 0xffff) {
-			e->data.ib1 &= ~AIROHA_FOE_IB1_BIND_STATE;
-			e->data.ib1 |= FIELD_PREP(AIROHA_FOE_IB1_BIND_STATE,
-						  AIROHA_FOE_STATE_INVALID);
-			airoha_ppe_foe_commit_entry(ppe, &e->data, e->hash);
-			e->hash = 0xffff;
-		}
-	}
+	if (e->type == FLOW_TYPE_L2)
+		airoha_ppe_foe_remove_l2_flow(ppe, e);
+	else
+		airoha_ppe_foe_remove_flow(ppe, e);
 }
 
-static void airoha_ppe_foe_insert_entry(struct airoha_ppe *ppe, u32 hash)
+static int
+airoha_ppe_foe_commit_subflow_entry(struct airoha_ppe *ppe,
+				    struct airoha_flow_table_entry *e,
+				    u32 hash)
+{
+	u32 mask = AIROHA_FOE_IB1_BIND_PACKET_TYPE | AIROHA_FOE_IB1_BIND_UDP;
+	struct airoha_foe_entry *hwe_p, hwe;
+	struct airoha_flow_table_entry *f;
+	struct airoha_foe_mac_info *l2;
+	int type;
+
+	hwe_p = airoha_ppe_foe_get_entry(ppe, hash);
+	if (!hwe_p)
+		return -EINVAL;
+
+	f = kzalloc(sizeof(*f), GFP_ATOMIC);
+	if (!f)
+		return -ENOMEM;
+
+	hlist_add_head(&f->l2_subflow_node, &e->l2_flows);
+	f->type = FLOW_TYPE_L2_SUBFLOW;
+	f->hash = hash;
+
+	memcpy(&hwe, hwe_p, sizeof(*hwe_p));
+	hwe.ib1 = (hwe.ib1 & mask) | (e->data.ib1 & ~mask);
+	l2 = &hwe.bridge.l2;
+	memcpy(l2, &e->data.bridge.l2, sizeof(*l2));
+
+	type = FIELD_GET(AIROHA_FOE_IB1_BIND_PACKET_TYPE, hwe.ib1);
+	if (type == PPE_PKT_TYPE_IPV4_HNAPT)
+		memcpy(&hwe.ipv4.new_tuple, &hwe.ipv4.orig_tuple,
+		       sizeof(hwe.ipv4.new_tuple));
+	else if (type >= PPE_PKT_TYPE_IPV6_ROUTE_3T &&
+		 l2->common.etype == ETH_P_IP)
+		l2->common.etype = ETH_P_IPV6;
+
+	hwe.bridge.ib2 = e->data.bridge.ib2;
+	airoha_ppe_foe_commit_entry(ppe, &hwe, hash);
+
+	return 0;
+}
+
+static void airoha_ppe_foe_insert_entry(struct airoha_ppe *ppe,
+					struct sk_buff *skb,
+					u32 hash)
 {
 	struct airoha_flow_table_entry *e;
+	struct airoha_foe_bridge br = {};
 	struct airoha_foe_entry *hwe;
+	bool commit_done = false;
 	struct hlist_node *n;
 	u32 index, state;
 
@@ -522,12 +608,33 @@ static void airoha_ppe_foe_insert_entry(struct airoha_ppe *ppe, u32 hash)
 
 	index = airoha_ppe_foe_get_entry_hash(hwe);
 	hlist_for_each_entry_safe(e, n, &ppe->foe_flow[index], list) {
-		if (airoha_ppe_foe_compare_entry(e, hwe)) {
-			airoha_ppe_foe_commit_entry(ppe, &e->data, hash);
-			e->hash = hash;
-			break;
+		if (e->type == FLOW_TYPE_L2_SUBFLOW) {
+			state = FIELD_GET(AIROHA_FOE_IB1_BIND_STATE, hwe->ib1);
+			if (state != AIROHA_FOE_STATE_BIND) {
+				e->hash = 0xffff;
+				airoha_ppe_foe_remove_flow(ppe, e);
+			}
+			continue;
+		}
+
+		if (commit_done || !airoha_ppe_foe_compare_entry(e, hwe)) {
+			e->hash = 0xffff;
+			continue;
 		}
+
+		airoha_ppe_foe_commit_entry(ppe, &e->data, hash);
+		commit_done = true;
+		e->hash = hash;
 	}
+
+	if (commit_done)
+		goto unlock;
+
+	airoha_ppe_foe_set_bridge_addrs(&br, eth_hdr(skb));
+	e = rhashtable_lookup_fast(&ppe->l2_flows, &br,
+				   airoha_l2_flow_table_params);
+	if (e)
+		airoha_ppe_foe_commit_subflow_entry(ppe, e, hash);
 unlock:
 	spin_unlock_bh(&ppe_lock);
 }
@@ -890,7 +997,8 @@ int airoha_ppe_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
 	return err;
 }
 
-void airoha_ppe_check_skb(struct airoha_ppe *ppe, u16 hash)
+void airoha_ppe_check_skb(struct airoha_ppe *ppe, struct sk_buff *skb,
+			  u16 hash)
 {
 	u16 now, diff;
 
@@ -903,7 +1011,7 @@ void airoha_ppe_check_skb(struct airoha_ppe *ppe, u16 hash)
 		return;
 
 	ppe->foe_check_time[hash] = now;
-	airoha_ppe_foe_insert_entry(ppe, hash);
+	airoha_ppe_foe_insert_entry(ppe, skb, hash);
 }
 
 int airoha_ppe_init(struct airoha_eth *eth)

-- 
2.49.0

Re: [PATCH net-next 1/3] net: airoha: Add l2_flows rhashtable

[Michal Kubiak]

On Mon, Apr 07, 2025 at 04:18:30PM +0200, Lorenzo Bianconi wrote:
> Introduce l2_flows rhashtable in airoha_ppe struct in order to
> store L2 flows committed by upper layers of the kernel. This is a
> preliminary patch in order to offload L2 traffic rules.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

The patch logic and coding style looks OK to me.
Just one question inline.

Thanks,
Michal

> ---
>  drivers/net/ethernet/airoha/airoha_eth.h | 15 ++++++-
>  drivers/net/ethernet/airoha/airoha_ppe.c | 67 +++++++++++++++++++++++++++-----
>  2 files changed, 72 insertions(+), 10 deletions(-)
> 
> diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
> index ec8908f904c61988c3dc973e187596c49af139fb..57925648155b104021c10821096ba267c9c7cef6 100644
> --- a/drivers/net/ethernet/airoha/airoha_eth.h
> +++ b/drivers/net/ethernet/airoha/airoha_eth.h
> @@ -422,12 +422,23 @@ struct airoha_flow_data {
>  	} pppoe;
>  };
>  
> +enum airoha_flow_entry_type {
> +	FLOW_TYPE_L4,

I didn't find any usage of L4 flow type in the series.
Is that reserved for future series? Shouldn't it be added together with
its usage then?

> +	FLOW_TYPE_L2,
> +	FLOW_TYPE_L2_SUBFLOW,
> +};
> +

Re: [PATCH net-next 1/3] net: airoha: Add l2_flows rhashtable

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1537 bytes --]

> On Mon, Apr 07, 2025 at 04:18:30PM +0200, Lorenzo Bianconi wrote:
> > Introduce l2_flows rhashtable in airoha_ppe struct in order to
> > store L2 flows committed by upper layers of the kernel. This is a
> > preliminary patch in order to offload L2 traffic rules.
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> The patch logic and coding style looks OK to me.
> Just one question inline.
> 
> Thanks,
> Michal
> 
> > ---
> >  drivers/net/ethernet/airoha/airoha_eth.h | 15 ++++++-
> >  drivers/net/ethernet/airoha/airoha_ppe.c | 67 +++++++++++++++++++++++++++-----
> >  2 files changed, 72 insertions(+), 10 deletions(-)
> > 
> > diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
> > index ec8908f904c61988c3dc973e187596c49af139fb..57925648155b104021c10821096ba267c9c7cef6 100644
> > --- a/drivers/net/ethernet/airoha/airoha_eth.h
> > +++ b/drivers/net/ethernet/airoha/airoha_eth.h
> > @@ -422,12 +422,23 @@ struct airoha_flow_data {
> >  	} pppoe;
> >  };
> >  
> > +enum airoha_flow_entry_type {
> > +	FLOW_TYPE_L4,
> 
> I didn't find any usage of L4 flow type in the series.
> Is that reserved for future series? Shouldn't it be added together with
> its usage then?

Hi Michal,

FLOW_TYPE_L4 is equal to 0 so it is the default value for
airoha_flow_table_entry type when not set explicitly.
It is done this way to reduce code changes.

Regards,
Lorenzo

> 
> > +	FLOW_TYPE_L2,
> > +	FLOW_TYPE_L2_SUBFLOW,
> > +};
> > +

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next 1/3] net: airoha: Add l2_flows rhashtable

[Michal Kubiak]

On Tue, Apr 08, 2025 at 06:49:40PM +0200, Lorenzo Bianconi wrote:
> > On Mon, Apr 07, 2025 at 04:18:30PM +0200, Lorenzo Bianconi wrote:
> > > Introduce l2_flows rhashtable in airoha_ppe struct in order to
> > > store L2 flows committed by upper layers of the kernel. This is a
> > > preliminary patch in order to offload L2 traffic rules.
> > > 
> > > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > 
> > The patch logic and coding style looks OK to me.
> > Just one question inline.
> > 
> > Thanks,
> > Michal
> > 
> > > ---
> > >  drivers/net/ethernet/airoha/airoha_eth.h | 15 ++++++-
> > >  drivers/net/ethernet/airoha/airoha_ppe.c | 67 +++++++++++++++++++++++++++-----
> > >  2 files changed, 72 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/drivers/net/ethernet/airoha/airoha_eth.h b/drivers/net/ethernet/airoha/airoha_eth.h
> > > index ec8908f904c61988c3dc973e187596c49af139fb..57925648155b104021c10821096ba267c9c7cef6 100644
> > > --- a/drivers/net/ethernet/airoha/airoha_eth.h
> > > +++ b/drivers/net/ethernet/airoha/airoha_eth.h
> > > @@ -422,12 +422,23 @@ struct airoha_flow_data {
> > >  	} pppoe;
> > >  };
> > >  
> > > +enum airoha_flow_entry_type {
> > > +	FLOW_TYPE_L4,
> > 
> > I didn't find any usage of L4 flow type in the series.
> > Is that reserved for future series? Shouldn't it be added together with
> > its usage then?
> 
> Hi Michal,
> 
> FLOW_TYPE_L4 is equal to 0 so it is the default value for
> airoha_flow_table_entry type when not set explicitly.
> It is done this way to reduce code changes.
> 
> Regards,
> Lorenzo
>

Thanks, Lorenzo! I'm OK with that.

Regards,
Reviewed-by: Michal Kubiak <michal.kubiak@intel.com>

Re: [PATCH net-next 0/3] Add L2 hw acceleration for airoha_eth driver

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 1137 bytes --]

> Introduce the capability to offload L2 traffic defining flower rules in
> the PSE/PPE engine available on EN7581 SoC.
> Since the hw always reports L2/L3/L4 flower rules, link all L2 rules
> sharing the same L2 info (with different L3/L4 info) in the L2 subflows
> list of a given L2 PPE entry.
> 
> ---
> Lorenzo Bianconi (3):
>       net: airoha: Add l2_flows rhashtable
>       net: airoha: Add airoha_ppe_foe_flow_remove_entry_locked()
>       net: airoha: Add L2 hw acceleration support
> 
>  drivers/net/ethernet/airoha/airoha_eth.c |   2 +-
>  drivers/net/ethernet/airoha/airoha_eth.h |  22 ++-
>  drivers/net/ethernet/airoha/airoha_ppe.c | 224 ++++++++++++++++++++++++++-----
>  3 files changed, 215 insertions(+), 33 deletions(-)
> ---
> base-commit: 61f96e684edd28ca40555ec49ea1555df31ba619
> change-id: 20250313-airoha-flowtable-l2b-e0b50d4a3215
> 
> Best regards,
> -- 
> Lorenzo Bianconi <lorenzo@kernel.org>
> 

Hi Andrew, David, Eric, Jakub and Paolo,

the series has been marked as 'changed requested' but Michal is fine with
current approach. Am I supposed to repost?

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next 1/3] net: airoha: Add l2_flows rhashtable

[Jakub Kicinski]

On Tue, 8 Apr 2025 18:49:40 +0200 Lorenzo Bianconi wrote:
> > I didn't find any usage of L4 flow type in the series.
> > Is that reserved for future series? Shouldn't it be added together with
> > its usage then?  
> 
> FLOW_TYPE_L4 is equal to 0 so it is the default value for
> airoha_flow_table_entry type when not set explicitly.
> It is done this way to reduce code changes.

That seems quite unintuitive. Could you init explicitly for the
benefit of people reading this code?

Re: [PATCH net-next 3/3] net: airoha: Add L2 hw acceleration support

[Michal Kubiak]

On Mon, Apr 07, 2025 at 04:18:32PM +0200, Lorenzo Bianconi wrote:
> Similar to mtk driver, introduce the capability to offload L2 traffic
> defining flower rules in the PSE/PPE engine available on EN7581 SoC.
> Since the hw always reports L2/L3/L4 flower rules, link all L2 rules
> sharing the same L2 info (with different L3/L4 info) in the L2 subflows
> list of a given L2 PPE entry.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> ---
>  drivers/net/ethernet/airoha/airoha_eth.c |   2 +-
>  drivers/net/ethernet/airoha/airoha_eth.h |  11 ++-
>  drivers/net/ethernet/airoha/airoha_ppe.c | 162 +++++++++++++++++++++++++------
>  3 files changed, 144 insertions(+), 31 deletions(-)
> 

[...]

> +static void airoha_ppe_foe_remove_flow(struct airoha_ppe *ppe,
> +				       struct airoha_flow_table_entry *e)
> +{
> +	lockdep_assert_held(&ppe_lock);
> +
> +	hlist_del_init(&e->list);
> +	if (e->hash != 0xffff) {
> +		e->data.ib1 &= ~AIROHA_FOE_IB1_BIND_STATE;
> +		e->data.ib1 |= FIELD_PREP(AIROHA_FOE_IB1_BIND_STATE,
> +					  AIROHA_FOE_STATE_INVALID);
> +		airoha_ppe_foe_commit_entry(ppe, &e->data, e->hash);
> +		e->hash = 0xffff;
> +	}
> +	if (e->type == FLOW_TYPE_L2_SUBFLOW) {
> +		hlist_del_init(&e->l2_subflow_node);
> +		kfree(e);
> +	}
> +}
> +
> +static void airoha_ppe_foe_remove_l2_flow(struct airoha_ppe *ppe,
> +					  struct airoha_flow_table_entry *e)
> +{
> +	struct hlist_head *head = &e->l2_flows;
> +	struct hlist_node *n;
> +
> +	lockdep_assert_held(&ppe_lock);
> +
> +	rhashtable_remove_fast(&ppe->l2_flows, &e->l2_node,
> +			       airoha_l2_flow_table_params);
> +	hlist_for_each_entry_safe(e, n, head, l2_subflow_node)
> +		airoha_ppe_foe_remove_flow(ppe, e);
> +}
> +
>  static void airoha_ppe_foe_flow_remove_entry(struct airoha_ppe *ppe,
>  					     struct airoha_flow_table_entry *e)
>  {
>  	lockdep_assert_held(&ppe_lock);
>  
> -	if (e->type == FLOW_TYPE_L2) {
> -		rhashtable_remove_fast(&ppe->l2_flows, &e->l2_node,
> -				       airoha_l2_flow_table_params);
> -	} else {
> -		hlist_del_init(&e->list);
> -		if (e->hash != 0xffff) {
> -			e->data.ib1 &= ~AIROHA_FOE_IB1_BIND_STATE;
> -			e->data.ib1 |= FIELD_PREP(AIROHA_FOE_IB1_BIND_STATE,
> -						  AIROHA_FOE_STATE_INVALID);
> -			airoha_ppe_foe_commit_entry(ppe, &e->data, e->hash);
> -			e->hash = 0xffff;
> -		}
> -	}
> +	if (e->type == FLOW_TYPE_L2)
> +		airoha_ppe_foe_remove_l2_flow(ppe, e);
> +	else
> +		airoha_ppe_foe_remove_flow(ppe, e);

It's not a hard request, more of a question: wouldn't it be better to
introduce "airoha_ppe_foe_remove_l2_flow()" and
"airoha_ppe_foe_remove_flow()" in the patch #2?
It looks like reorganizing the code can be part of the preliminary
patch and the current patch can just add the feature, e.g. L2_SUBFLOW.

Thanks,
Michal

Re: [PATCH net-next 2/3] net: airoha: Add airoha_ppe_foe_flow_remove_entry_locked()

[Michal Kubiak]

On Mon, Apr 07, 2025 at 04:18:31PM +0200, Lorenzo Bianconi wrote:
> Introduce airoha_ppe_foe_flow_remove_entry_locked utility routine
> in order to run airoha_ppe_foe_flow_remove_entry holding ppe_lock.
> This is a preliminary patch to L2 offloading support to airoha_eth
> driver.
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>

Could you please explain the reason of introducing the *_remove_entry_locked
function if "airoha_ppe_foe_flow_remove_entry()" is still never called out of
"airoha_ppe_foe_flow_remove_entry_locked()" context (at least in this
series)?
I would expect that it can be useful if you have an use case when you want
to call "airoha_ppe_foe_flow_remove_entry()" from another function that
has already taken the lock, but I haven't found such a context.

Thanks,
Michal

Re: [PATCH net-next 1/3] net: airoha: Add l2_flows rhashtable

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 561 bytes --]

> On Tue, 8 Apr 2025 18:49:40 +0200 Lorenzo Bianconi wrote:
> > > I didn't find any usage of L4 flow type in the series.
> > > Is that reserved for future series? Shouldn't it be added together with
> > > its usage then?  
> > 
> > FLOW_TYPE_L4 is equal to 0 so it is the default value for
> > airoha_flow_table_entry type when not set explicitly.
> > It is done this way to reduce code changes.
> 
> That seems quite unintuitive. Could you init explicitly for the
> benefit of people reading this code?

ack, I will do in v2.

Regards,
Lorenzo

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next 3/3] net: airoha: Add L2 hw acceleration support

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 3087 bytes --]

> On Mon, Apr 07, 2025 at 04:18:32PM +0200, Lorenzo Bianconi wrote:
> > Similar to mtk driver, introduce the capability to offload L2 traffic
> > defining flower rules in the PSE/PPE engine available on EN7581 SoC.
> > Since the hw always reports L2/L3/L4 flower rules, link all L2 rules
> > sharing the same L2 info (with different L3/L4 info) in the L2 subflows
> > list of a given L2 PPE entry.
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> > ---
> >  drivers/net/ethernet/airoha/airoha_eth.c |   2 +-
> >  drivers/net/ethernet/airoha/airoha_eth.h |  11 ++-
> >  drivers/net/ethernet/airoha/airoha_ppe.c | 162 +++++++++++++++++++++++++------
> >  3 files changed, 144 insertions(+), 31 deletions(-)
> > 
> 
> [...]
> 
> > +static void airoha_ppe_foe_remove_flow(struct airoha_ppe *ppe,
> > +				       struct airoha_flow_table_entry *e)
> > +{
> > +	lockdep_assert_held(&ppe_lock);
> > +
> > +	hlist_del_init(&e->list);
> > +	if (e->hash != 0xffff) {
> > +		e->data.ib1 &= ~AIROHA_FOE_IB1_BIND_STATE;
> > +		e->data.ib1 |= FIELD_PREP(AIROHA_FOE_IB1_BIND_STATE,
> > +					  AIROHA_FOE_STATE_INVALID);
> > +		airoha_ppe_foe_commit_entry(ppe, &e->data, e->hash);
> > +		e->hash = 0xffff;
> > +	}
> > +	if (e->type == FLOW_TYPE_L2_SUBFLOW) {
> > +		hlist_del_init(&e->l2_subflow_node);
> > +		kfree(e);
> > +	}
> > +}
> > +
> > +static void airoha_ppe_foe_remove_l2_flow(struct airoha_ppe *ppe,
> > +					  struct airoha_flow_table_entry *e)
> > +{
> > +	struct hlist_head *head = &e->l2_flows;
> > +	struct hlist_node *n;
> > +
> > +	lockdep_assert_held(&ppe_lock);
> > +
> > +	rhashtable_remove_fast(&ppe->l2_flows, &e->l2_node,
> > +			       airoha_l2_flow_table_params);
> > +	hlist_for_each_entry_safe(e, n, head, l2_subflow_node)
> > +		airoha_ppe_foe_remove_flow(ppe, e);
> > +}
> > +
> >  static void airoha_ppe_foe_flow_remove_entry(struct airoha_ppe *ppe,
> >  					     struct airoha_flow_table_entry *e)
> >  {
> >  	lockdep_assert_held(&ppe_lock);
> >  
> > -	if (e->type == FLOW_TYPE_L2) {
> > -		rhashtable_remove_fast(&ppe->l2_flows, &e->l2_node,
> > -				       airoha_l2_flow_table_params);
> > -	} else {
> > -		hlist_del_init(&e->list);
> > -		if (e->hash != 0xffff) {
> > -			e->data.ib1 &= ~AIROHA_FOE_IB1_BIND_STATE;
> > -			e->data.ib1 |= FIELD_PREP(AIROHA_FOE_IB1_BIND_STATE,
> > -						  AIROHA_FOE_STATE_INVALID);
> > -			airoha_ppe_foe_commit_entry(ppe, &e->data, e->hash);
> > -			e->hash = 0xffff;
> > -		}
> > -	}
> > +	if (e->type == FLOW_TYPE_L2)
> > +		airoha_ppe_foe_remove_l2_flow(ppe, e);
> > +	else
> > +		airoha_ppe_foe_remove_flow(ppe, e);
> 
> It's not a hard request, more of a question: wouldn't it be better to
> introduce "airoha_ppe_foe_remove_l2_flow()" and
> "airoha_ppe_foe_remove_flow()" in the patch #2?
> It looks like reorganizing the code can be part of the preliminary
> patch and the current patch can just add the feature, e.g. L2_SUBFLOW.

ack, fine for me. I will fix it in v2.

Regards,
Lorenzo

> 
> Thanks,
> Michal
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH net-next 2/3] net: airoha: Add airoha_ppe_foe_flow_remove_entry_locked()

[Lorenzo Bianconi]

[-- Attachment #1: Type: text/plain, Size: 991 bytes --]

> On Mon, Apr 07, 2025 at 04:18:31PM +0200, Lorenzo Bianconi wrote:
> > Introduce airoha_ppe_foe_flow_remove_entry_locked utility routine
> > in order to run airoha_ppe_foe_flow_remove_entry holding ppe_lock.
> > This is a preliminary patch to L2 offloading support to airoha_eth
> > driver.
> > 
> > Signed-off-by: Lorenzo Bianconi <lorenzo@kernel.org>
> 
> Could you please explain the reason of introducing the *_remove_entry_locked
> function if "airoha_ppe_foe_flow_remove_entry()" is still never called out of
> "airoha_ppe_foe_flow_remove_entry_locked()" context (at least in this
> series)?
> I would expect that it can be useful if you have an use case when you want
> to call "airoha_ppe_foe_flow_remove_entry()" from another function that
> has already taken the lock, but I haven't found such a context.

ack, you are right. I guess we can drop
airoha_ppe_foe_flow_remove_entry_locked(). I will fix it in v2.

Regards,
Lorenzo

> 
> Thanks,
> Michal
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]