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@xxxxxxxxxxx) wrote:
> From: Hariprasad Kelam <hkelam@xxxxxxxxxxx>
>
> 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@xxxxxxxxxxx>
> Signed-off-by: Ratheesh Kannoth <rkannoth@xxxxxxxxxxx>

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.