[PATCH v2 net-next] octeontx2-af: npc: Warn on NPC_IPSEC_SPI key overlap

[PATCH v2 net-next] octeontx2-af: npc: Warn on NPC_IPSEC_SPI key overlap

[Ratheesh Kannoth]

From: Hariprasad Kelam <hkelam@marvell.com>

When scanning the MKEX profile to determine supported NPC features, warn
if the SPI extraction field overlaps with other key fields. AH and ESP
may legitimately use the same key offset for SPI, so continue to
advertise NPC_IPSEC_SPI via npc_is_field_present() instead of treating
the overlap as a hard failure.

Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

---
v1 -> v2: Addressed Simon comments
	https://lore.kernel.org/netdev/20260720171857.278396-2-horms@kernel.org/
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

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 91b5947dae06..d422bdd5e8f8 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
@@ -729,7 +729,12 @@ static void npc_set_features(struct rvu *rvu, int blkaddr, u8 intf)
 		if (!npc_check_field(rvu, blkaddr, NPC_LB, intf))
 			*features &= ~BIT_ULL(NPC_OUTER_VID);
 
-	/* Allow extracting SPI field from AH and ESP headers at same offset */
+	/* Warn on unrelated MKEX fields colliding with SPI key bits.  AH/ESP
+	 * sharing the same SPI key offset is valid; use npc_is_field_present(),
+	 * not npc_check_field(), to advertise the feature.
+	 */
+	if (npc_check_overlap(rvu, blkaddr, NPC_IPSEC_SPI, 0, intf))
+		dev_warn(rvu->dev, "Overlap detected the field NPC_IPSEC_SPI\n");
 	if (npc_is_field_present(rvu, NPC_IPSEC_SPI, intf) &&
 	    (*features & (BIT_ULL(NPC_IPPROTO_ESP) | BIT_ULL(NPC_IPPROTO_AH))))
 		*features |= BIT_ULL(NPC_IPSEC_SPI);
-- 
2.43.0

Re: [PATCH v2 net-next] octeontx2-af: npc: Warn on NPC_IPSEC_SPI key overlap

[Ratheesh Kannoth]

On 2026-07-21 at 12:33:03, Ratheesh Kannoth (rkannoth@marvell.com) wrote:
> From: Hariprasad Kelam <hkelam@marvell.com>
>
> When scanning the MKEX profile to determine supported NPC features, warn
> if the SPI extraction field overlaps with other key fields. AH and ESP
> may legitimately use the same key offset for SPI, so continue to
> advertise NPC_IPSEC_SPI via npc_is_field_present() instead of treating
> the overlap as a hard failure.
>
> Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
> Signed-off-by: Ratheesh Kannoth <rkannoth@marvell.com>

https://sashiko.dev/#/patchset/20260721070303.986740-1-rkannoth%40marvell.com wrote

>> 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
>[ ... ]
>> -	/* Allow extracting SPI field from AH and ESP headers at same offset */
>> +	/* Warn on unrelated MKEX fields colliding with SPI key bits.  AH/ESP
>> +	 * sharing the same SPI key offset is valid; use npc_is_field_present(),
>> +	 * not npc_check_field(), to advertise the feature.
>> +	 */
>> +	if (npc_check_overlap(rvu, blkaddr, NPC_IPSEC_SPI, 0, intf))
>> +		dev_warn(rvu->dev, "Overlap detected the field NPC_IPSEC_SPI\n");
>
>Will this warning spuriously trigger for valid AH and ESP configurations?
>
>The NPC_IPSEC_SPI field extracts data from two different layers, NPC_LID_LD for
>AH and NPC_LID_LE for ESP. However, rx_key_fields[NPC_IPSEC_SPI].layer_mdata
>only stores the metadata of the last scanned layer.
>
>When npc_check_overlap() processes the AH layer, npc_is_same() evaluates to
>false because the LIDs differ. Then npc_check_overlap_fields() compares
>their kw_masks, which intersect because the SPI kw_mask accumulated both AH
>and ESP extraction bits.
>
>This appears to cause npc_check_overlap() to return true for its own layer
>component, warning unconditionally on valid profiles and contradicting the
>goal of ignoring legitimate AH/ESP overlap.

npc_check_overlap() is to check whether there is an overlap for the extracted key in meta data.
We can ignore sashiko comment.

>
>>  	if (npc_is_field_present(rvu, NPC_IPSEC_SPI, intf) &&
>>  	    (*features & (BIT_ULL(NPC_IPPROTO_ESP) | BIT_ULL(NPC_IPPROTO_AH))))
>>  		*features |= BIT_ULL(NPC_IPSEC_SPI);
>
>Does bypassing the overlap check here allow corrupted key extraction?
>
>If npc_check_overlap() detects a true collision with a completely unrelated
>MKEX field, such as VLAN or DMAC, this logic merely prints the warning above
>but still continues to set the NPC_IPSEC_SPI feature bit.
>
>This seems to advertise support for the feature even when the hardware MKEX
>profile has genuinely overlapping extraction keys that could misclassify
>packets and incorrectly steer IPsec traffic.

Evenif overlap detected, system functions for most of the traffic, so print warning and continue.
We can ignore sashiko comment.