[PATCH net-next] octeontx2-af: add NPC TEID matching for GTP-U and GTP-C flows

[PATCH net-next] octeontx2-af: add NPC TEID matching for GTP-U and GTP-C flows

[Ratheesh Kannoth]

From: Suman Ghosh <sumang@marvell.com>

Add NPC_GTPU_TEID and NPC_GTPC_TEID key fields and wire them through
the AF MCAM path so ethtool Rx flow rules can match on the TEID of
GTP-U and GTP-C packets.

In otx2_prepare_flow_request(), when a UDP or TCP v4/v6 ntuple rule
targets the well-known GTP-U (2152) or GTP-C (2123) destination port,
take the TEID value and mask from h_ext.data[1] and install the
corresponding LE layer match in NPC.  Expose the new fields in debugfs
MCAM dumps.

Signed-off-by: Suman Ghosh <sumang@marvell.com>
Signed-off-by: Geetha sowjanya <gakula@marvell.com>
Signed-off-by: Sunil Goutham <sgoutham@marvell.com>
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>
---
 .../net/ethernet/marvell/octeontx2/af/mbox.h  |  2 ++
 .../net/ethernet/marvell/octeontx2/af/npc.h   |  2 ++
 .../marvell/octeontx2/af/rvu_debugfs.c        |  8 +++++++
 .../marvell/octeontx2/af/rvu_npc_fs.c         | 21 +++++++++++++++--
 .../marvell/octeontx2/nic/otx2_flows.c        | 23 +++++++++++++++++++
 5 files changed, 54 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
index 73f743e4a83d..1101e29328ac 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
@@ -1839,6 +1839,8 @@ struct flow_msg {
 		u8 next_header;
 	};
 	__be16 vlan_itci;
+	__be32 gtpu_teid;
+	__be32 gtpc_teid;
 #define OTX2_FLOWER_MASK_MPLS_LB		GENMASK(31, 12)
 #define OTX2_FLOWER_MASK_MPLS_TC		GENMASK(11, 9)
 #define OTX2_FLOWER_MASK_MPLS_BOS		BIT(8)
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/npc.h b/drivers/net/ethernet/marvell/octeontx2/af/npc.h
index 719b3618eeb5..33277fc3d27e 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/npc.h
+++ b/drivers/net/ethernet/marvell/octeontx2/af/npc.h
@@ -214,6 +214,8 @@ enum key_fields {
 	NPC_DPORT_UDP,
 	NPC_SPORT_SCTP,
 	NPC_DPORT_SCTP,
+	NPC_GTPU_TEID,
+	NPC_GTPC_TEID,
 	NPC_IPSEC_SPI,
 	NPC_MPLS1_LBTCBOS,
 	NPC_MPLS1_TTL,
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
index 3456313d3b3c..bb690890606c 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
@@ -3437,6 +3437,14 @@ static void rvu_dbg_npc_mcam_show_flows(struct seq_file *s,
 			seq_printf(s, "%d ", rule->packet.icmp_code);
 			seq_printf(s, "mask 0x%x\n", rule->mask.icmp_code);
 			break;
+		case NPC_GTPU_TEID:
+			seq_printf(s, "%d ", ntohl(rule->packet.gtpu_teid));
+			seq_printf(s, "mask 0x%x\n", ntohl(rule->mask.gtpu_teid));
+			break;
+		case NPC_GTPC_TEID:
+			seq_printf(s, "%d ", ntohl(rule->packet.gtpc_teid));
+			seq_printf(s, "mask 0x%x\n", ntohl(rule->mask.gtpc_teid));
+			break;
 		default:
 			seq_puts(s, "\n");
 			break;
diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
index d422bdd5e8f8..1f9af70cf2f9 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
@@ -44,6 +44,8 @@ static const char * const npc_flow_names[] = {
 	[NPC_SPORT_SCTP] = "sctp source port",
 	[NPC_DPORT_SCTP] = "sctp destination port",
 	[NPC_LXMB]	= "Mcast/Bcast header ",
+	[NPC_GTPU_TEID]	= "gtp-u teid ",
+	[NPC_GTPC_TEID]	= "gtp-c teid ",
 	[NPC_IPSEC_SPI] = "SPI ",
 	[NPC_MPLS1_LBTCBOS] = "lse depth 1 label tc bos",
 	[NPC_MPLS1_TTL]     = "lse depth 1 ttl",
@@ -661,6 +663,8 @@ do {									       \
 	NPC_SCAN_HDR(NPC_VLAN_TAG1, NPC_LID_LB, NPC_LT_LB_CTAG, 2, 2);
 	NPC_SCAN_HDR(NPC_VLAN_TAG2, NPC_LID_LB, NPC_LT_LB_STAG_QINQ, 2, 2);
 	NPC_SCAN_HDR(NPC_VLAN_TAG3, NPC_LID_LB, NPC_LT_LB_STAG_QINQ, 6, 2);
+	NPC_SCAN_HDR(NPC_GTPU_TEID, NPC_LID_LE, NPC_LT_LE_GTPU, 4, 4);
+	NPC_SCAN_HDR(NPC_GTPC_TEID, NPC_LID_LE, NPC_LT_LE_GTPC, 4, 4);
 	NPC_SCAN_HDR(NPC_DMAC, NPC_LID_LA, la_ltype, la_start, 6);
 
 	NPC_SCAN_HDR(NPC_IPSEC_SPI, NPC_LID_LD, NPC_LT_LD_AH, 4, 4);
@@ -720,9 +724,10 @@ static void npc_set_features(struct rvu *rvu, int blkaddr, u8 intf)
 		*features |= BIT_ULL(NPC_IPPROTO_ICMP6);
 	}
 
-	/* for ESP, check if corresponding layer type is present in the key */
+	/* for ESP/GTP-U/GTP-C check if corresponding layer type is present in the key */
 	if (npc_check_field(rvu, blkaddr, NPC_LE, intf))
-		*features |= BIT_ULL(NPC_IPPROTO_ESP);
+		*features |= BIT_ULL(NPC_IPPROTO_ESP) | BIT_ULL(NPC_GTPU_TEID) |
+			     BIT_ULL(NPC_GTPC_TEID);
 
 	/* for vlan corresponding layer type should be in the key */
 	if (*features & BIT_ULL(NPC_OUTER_VID))
@@ -1113,6 +1118,14 @@ void npc_update_flow(struct rvu *rvu, struct mcam_entry_mdata *mdata,
 		npc_update_entry(rvu, NPC_LE, mdata, NPC_LT_LE_ESP,
 				 0, ~0ULL, 0, intf);
 
+	if (features & BIT_ULL(NPC_GTPU_TEID))
+		npc_update_entry(rvu, NPC_LE, mdata, NPC_LT_LE_GTPU,
+				 0, ~0ULL, 0, intf);
+
+	if (features & BIT_ULL(NPC_GTPC_TEID))
+		npc_update_entry(rvu, NPC_LE, mdata, NPC_LT_LE_GTPC,
+				 0, ~0ULL, 0, intf);
+
 	if (features & BIT_ULL(NPC_LXMB)) {
 		output->lxmb = is_broadcast_ether_addr(pkt->dmac) ? 2 : 1;
 		npc_update_entry(rvu, NPC_LXMB, mdata, output->lxmb, 0,
@@ -1209,6 +1222,10 @@ do {									      \
 
 	NPC_WRITE_FLOW(NPC_IPFRAG_IPV6, next_header, pkt->next_header, 0,
 		       mask->next_header, 0);
+	NPC_WRITE_FLOW(NPC_GTPU_TEID, gtpu_teid, ntohl(pkt->gtpu_teid), 0,
+		       ntohl(mask->gtpu_teid), 0);
+	NPC_WRITE_FLOW(NPC_GTPC_TEID, gtpc_teid, ntohl(pkt->gtpc_teid), 0,
+		       ntohl(mask->gtpc_teid), 0);
 	npc_update_ipv6_flow(rvu, mdata, features, pkt, mask, output, intf);
 	npc_update_vlan_features(rvu, mdata, features, intf);
 
diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
index 99d78fc5a2c4..59ea2da33ae2 100644
--- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
+++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
@@ -11,6 +11,8 @@
 #include "otx2_common.h"
 
 #define OTX2_DEFAULT_ACTION	0x1
+#define GTPU_PORT		2152
+#define GTPC_PORT		2123
 
 struct otx2_flow {
 	struct ethtool_rx_flow_spec flow_spec;
@@ -1036,8 +1038,29 @@ static int otx2_prepare_flow_request(struct ethtool_rx_flow_spec *fsp,
 		return -EOPNOTSUPP;
 	}
 	if (fsp->flow_type & FLOW_EXT) {
+		int skip_user_def = false;
 		u16 vlan_etype;
 
+		switch (flow_type) {
+		case UDP_V4_FLOW:
+		case UDP_V6_FLOW:
+		case TCP_V4_FLOW:
+		case TCP_V6_FLOW:
+			if (ntohs(pkt->dport) == GTPU_PORT) {
+				/* Check for GTP-U packets */
+				skip_user_def = true;
+				pkt->gtpu_teid = fsp->h_ext.data[1];
+				pmask->gtpu_teid = fsp->m_ext.data[1];
+				req->features |= BIT_ULL(NPC_GTPU_TEID);
+			} else if (ntohs(pkt->dport) == GTPC_PORT) {
+				/* Check for GTP-C packets */
+				skip_user_def = true;
+				pkt->gtpc_teid = fsp->h_ext.data[1];
+				pmask->gtpc_teid = fsp->m_ext.data[1];
+				req->features |= BIT_ULL(NPC_GTPC_TEID);
+			}
+		}
+
 		if (fsp->m_ext.vlan_etype) {
 			/* Partial masks not supported */
 			if (be16_to_cpu(fsp->m_ext.vlan_etype) != 0xFFFF)
-- 
2.43.0

Re: [PATCH net-next] octeontx2-af: add NPC TEID matching for GTP-U and GTP-C flows

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/#/patchset/20260810050436.1822566-1-rkannoth@marvell.com?part=1
---
--- Patch [1]: [PATCH net-next] octeontx2-af: add NPC TEID matching for GTP-U and GTP-C flows ---
commit 98f9555a96987d26df070fd9f55b613312f67f14
Author: Suman Ghosh <sumang@marvell.com>
Subject: octeontx2-af: add NPC TEID matching for GTP-U and GTP-C flows

This patch adds NPC_GTPU_TEID and NPC_GTPC_TEID key fields to support
ethtool Rx flow rules matching on GTP-U and GTP-C TEIDs. It wires these
fields through the AF MCAM path and exposes them in debugfs.

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> --- a/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/mbox.h
> @@ -1839,6 +1839,8 @@ struct flow_msg {
>  		u8 next_header;
>  	};
>  	__be16 vlan_itci;
> +	__be32 gtpu_teid;
> +	__be32 gtpc_teid;

[Severity: Critical]
Is it safe to insert these new fields in the middle of struct flow_msg? 

Since this structure is embedded in the npc_install_flow_req mailbox message,
wouldn't this shift the memory offsets of all subsequent fields (like mpls_lse
and the trailing fields in npc_install_flow_req)? This might break the PF/VF
mailbox ABI for older VFs communicating with a newer PF, potentially allowing
corrupted state.

>  #define OTX2_FLOWER_MASK_MPLS_LB		GENMASK(31, 12)
>  #define OTX2_FLOWER_MASK_MPLS_TC		GENMASK(11, 9)
>  #define OTX2_FLOWER_MASK_MPLS_BOS		BIT(8)

[ ... ]

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/npc.h b/drivers/net/ethernet/marvell/octeontx2/af/npc.h
> --- a/drivers/net/ethernet/marvell/octeontx2/af/npc.h
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/npc.h
> @@ -214,6 +214,8 @@ enum key_fields {
>  	NPC_DPORT_UDP,
>  	NPC_SPORT_SCTP,
>  	NPC_DPORT_SCTP,
> +	NPC_GTPU_TEID,
> +	NPC_GTPC_TEID,

[Severity: Critical]
Could inserting these enum values in the middle break the PF/VF mailbox ABI
bitmask? 

The integer values of enum key_fields are used as bit indices for the
req->features bitmask sent over the mailbox. Inserting here shifts the integer
values of NPC_IPSEC_SPI and all subsequent fields. An older VF requesting an
IPSec rule using the old bit index might have its request misinterpreted by the
new PF as a GTP rule.

>  	NPC_IPSEC_SPI,
>  	NPC_MPLS1_LBTCBOS,
>  	NPC_MPLS1_TTL,

[ ... ]

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_debugfs.c
> @@ -3437,6 +3437,14 @@ static void rvu_dbg_npc_mcam_show_flows(struct seq_file *s,
>  			seq_printf(s, "%d ", rule->packet.icmp_code);
>  			seq_printf(s, "mask 0x%x\n", rule->mask.icmp_code);
>  			break;
> +		case NPC_GTPU_TEID:
> +			seq_printf(s, "%d ", ntohl(rule->packet.gtpu_teid));
> +			seq_printf(s, "mask 0x%x\n", ntohl(rule->mask.gtpu_teid));

[Severity: Low]
Does the %d format specifier display the unsigned 32-bit TEID correctly here?
If a TEID has its most significant bit set, it appears it will be printed as a
negative number instead of an unsigned value. Using %u might be preferred.

> +			break;

[ ... ]

> diff --git a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c

[ ... ]

> @@ -720,9 +724,10 @@ static void npc_set_features(struct rvu *rvu, int blkaddr, u8 intf)
>  		*features |= BIT_ULL(NPC_IPPROTO_ICMP6);
>  	}
>  
> -	/* for ESP, check if corresponding layer type is present in the key */
> +	/* for ESP/GTP-U/GTP-C check if corresponding layer type is present in the key */
>  	if (npc_check_field(rvu, blkaddr, NPC_LE, intf))
> -		*features |= BIT_ULL(NPC_IPPROTO_ESP);
> +		*features |= BIT_ULL(NPC_IPPROTO_ESP) | BIT_ULL(NPC_GTPU_TEID) |
> +			     BIT_ULL(NPC_GTPC_TEID);

[Severity: High]
Is it intentional to advertise BIT_ULL(NPC_GTPU_TEID) and
BIT_ULL(NPC_GTPC_TEID) without specifically validating them via
npc_check_field()?

If the hardware Key Extraction (KEX) profile doesn't actually extract the
TEID bytes, claiming support for these features could result in rules being
accepted by otx2_prepare_flow_request() but silently ignored when programming
them in npc_update_entry().

[ ... ]

> diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_flows.c

[ ... ]

> @@ -1036,8 +1038,29 @@ static int otx2_prepare_flow_request(struct ethtool_rx_flow_spec *fsp,
>  		return -EOPNOTSUPP;
>  	}
>  	if (fsp->flow_type & FLOW_EXT) {
> +		int skip_user_def = false;
>  		u16 vlan_etype;
>  
> +		switch (flow_type) {
> +		case UDP_V4_FLOW:
> +		case UDP_V6_FLOW:
> +		case TCP_V4_FLOW:
> +		case TCP_V6_FLOW:
> +			if (ntohs(pkt->dport) == GTPU_PORT) {

[Severity: Medium]
Should we also verify that the port mask is exact (0xFFFF) before inferring
the GTP protocol?

If a user configures a broad UDP matching rule with a base port that aligns
with 2152 under its mask, this logic might misinterpret it as a GTP rule
and apply TEID features incorrectly.

> +				/* Check for GTP-U packets */
> +				skip_user_def = true;

[Severity: Medium]
Is the skip_user_def variable used anywhere after being set to true?

If a user requests a GTP flow rule with a TEID of 1 (which equals
OTX2_DEFAULT_ACTION), the fallback else if condition later in this function
checks if fsp->h_ext.data[1] == cpu_to_be32(OTX2_DEFAULT_ACTION). Because
skip_user_def is never evaluated, wouldn't this incorrectly override the
requested Rx queue action with NIX_RX_ACTION_DEFAULT?

> +				pkt->gtpu_teid = fsp->h_ext.data[1];
> +				pmask->gtpu_teid = fsp->m_ext.data[1];
> +				req->features |= BIT_ULL(NPC_GTPU_TEID);
> +			} else if (ntohs(pkt->dport) == GTPC_PORT) {
> +				/* Check for GTP-C packets */
> +				skip_user_def = true;
> +				pkt->gtpc_teid = fsp->h_ext.data[1];
> +				pmask->gtpc_teid = fsp->m_ext.data[1];
> +				req->features |= BIT_ULL(NPC_GTPC_TEID);
> +			}
> +		}
> +

[ ... ]