[net] Octeontx2-af: Skip overlap check for SPI field

[net] Octeontx2-af: Skip overlap check for SPI field

[Hariprasad Kelam]

Octeontx2/CN10K silicon supports generating a 256-bit key per packet.
The specific fields to be extracted from a packet for key generation
are configurable via a Key Extraction (MKEX) Profile.

The AF driver scans the configured extraction profile to ensure that
fields from upper layers do not overwrite fields from lower layers in
the key.

Example Packet Field Layout:
LA: DMAC + SMAC
LB: VLAN
LC: IPv4/IPv6
LD: TCP/UDP

Valid MKEX Profile Configuration:

LA   -> DMAC   -> key_offset[0-5]
LC   -> SIP    -> key_offset[20-23]
LD   -> SPORT  -> key_offset[30-31]

Invalid MKEX profile configuration:

LA   -> DMAC   -> key_offset[0-5]
LC   -> SIP    -> key_offset[20-23]
LD   -> SPORT  -> key_offset[2-3]  // Overlaps with DMAC field

In another scenario, if the MKEX profile is configured to extract
the SPI field from both AH and ESP headers at the same key offset,
the driver rejecting this configuration. In a regular traffic,
ipsec packet will be having either AF(LD) or ESP (LE). This patch
relaxes the check for the same.

Fixes: 12aa0a3b93f3 ("octeontx2-af: Harden rule validation.")
Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c | 2 +-
 1 file changed, 1 insertion(+), 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 1b765045aa63..d8d491a01e5b 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
@@ -607,7 +607,7 @@ static void npc_set_features(struct rvu *rvu, int blkaddr, u8 intf)
 			*features &= ~BIT_ULL(NPC_OUTER_VID);
 
 	/* Set SPI flag only if AH/ESP and IPSEC_SPI are in the key */
-	if (npc_check_field(rvu, blkaddr, NPC_IPSEC_SPI, intf) &&
+	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.34.1

Re: [net] Octeontx2-af: Skip overlap check for SPI field

[Simon Horman]

On Fri, Jul 25, 2025 at 12:18:02PM +0530, Hariprasad Kelam wrote:
> Octeontx2/CN10K silicon supports generating a 256-bit key per packet.
> The specific fields to be extracted from a packet for key generation
> are configurable via a Key Extraction (MKEX) Profile.
> 
> The AF driver scans the configured extraction profile to ensure that
> fields from upper layers do not overwrite fields from lower layers in
> the key.
> 
> Example Packet Field Layout:
> LA: DMAC + SMAC
> LB: VLAN
> LC: IPv4/IPv6
> LD: TCP/UDP
> 
> Valid MKEX Profile Configuration:
> 
> LA   -> DMAC   -> key_offset[0-5]
> LC   -> SIP    -> key_offset[20-23]
> LD   -> SPORT  -> key_offset[30-31]
> 
> Invalid MKEX profile configuration:
> 
> LA   -> DMAC   -> key_offset[0-5]
> LC   -> SIP    -> key_offset[20-23]
> LD   -> SPORT  -> key_offset[2-3]  // Overlaps with DMAC field
> 
> In another scenario, if the MKEX profile is configured to extract
> the SPI field from both AH and ESP headers at the same key offset,
> the driver rejecting this configuration. In a regular traffic,
> ipsec packet will be having either AF(LD) or ESP (LE). This patch

Should "AF" be "AH ?

> relaxes the check for the same.
> 
> Fixes: 12aa0a3b93f3 ("octeontx2-af: Harden rule validation.")
> Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
> ---
>  drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c | 2 +-
>  1 file changed, 1 insertion(+), 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 1b765045aa63..d8d491a01e5b 100644
> --- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> +++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
> @@ -607,7 +607,7 @@ static void npc_set_features(struct rvu *rvu, int blkaddr, u8 intf)
>  			*features &= ~BIT_ULL(NPC_OUTER_VID);
>  
>  	/* Set SPI flag only if AH/ESP and IPSEC_SPI are in the key */
> -	if (npc_check_field(rvu, blkaddr, NPC_IPSEC_SPI, intf) &&
> +	if (npc_is_field_present(rvu, NPC_IPSEC_SPI, intf) &&

As this checks now differs in form from that of other's in this function,
perhaps expanding the comment above is warranted.

>  	    (*features & (BIT_ULL(NPC_IPPROTO_ESP) | BIT_ULL(NPC_IPPROTO_AH))))
>  		*features |= BIT_ULL(NPC_IPSEC_SPI);
>  
> -- 
> 2.34.1
> 

[net] Octeontx2-af: Skip overlap check for SPI field

[Hariprasad Kelam]

Currently, the AF driver scans the mkex profile to identify all
supported features. This process also involves checking for any
fields that might overlap with each other.

For example, NPC_TCP_SPORT field offset within the key should
not overlap with NPC_DMAC/NPC_SIP_IPV4 or any other field.

However, there are situations where some overlap is unavoidable.
For instance, when extracting the SPI field, the same key offset might
be used by both the AH and ESP layers. This patch addresses this
specific scenario by skipping the overlap check and instead, adds
a warning message to the user.

Fixes: 12aa0a3b93f3 ("octeontx2-af: Harden rule validation.")
Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>
---
 drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c | 4 +++-
 1 file changed, 3 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 1b765045aa63..163cbce8575f 100644
--- a/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
+++ b/drivers/net/ethernet/marvell/octeontx2/af/rvu_npc_fs.c
@@ -606,8 +606,10 @@ 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);
 
+	if (npc_check_overlap(rvu, blkaddr, NPC_IPSEC_SPI, 0, intf))
+		dev_warn(rvu->dev, "Overlap detected the field NPC_IPSEC_SPI\n");
 	/* Set SPI flag only if AH/ESP and IPSEC_SPI are in the key */
-	if (npc_check_field(rvu, blkaddr, NPC_IPSEC_SPI, intf) &&
+	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.34.1

Re: [net] Octeontx2-af: Skip overlap check for SPI field

[Simon Horman]

On Sun, May 25, 2025 at 03:28:54PM +0530, Hariprasad Kelam wrote:
> Currently, the AF driver scans the mkex profile to identify all
> supported features. This process also involves checking for any
> fields that might overlap with each other.
> 
> For example, NPC_TCP_SPORT field offset within the key should
> not overlap with NPC_DMAC/NPC_SIP_IPV4 or any other field.
> 
> However, there are situations where some overlap is unavoidable.
> For instance, when extracting the SPI field, the same key offset might
> be used by both the AH and ESP layers. This patch addresses this
> specific scenario by skipping the overlap check and instead, adds
> a warning message to the user.
> 
> Fixes: 12aa0a3b93f3 ("octeontx2-af: Harden rule validation.")
> Signed-off-by: Hariprasad Kelam <hkelam@marvell.com>

Reviewed-by: Simon Horman <horms@kernel.org>

Re: [net] Octeontx2-af: Skip overlap check for SPI field

[Jakub Kicinski]

On Sun, 25 May 2025 15:28:54 +0530 Hariprasad Kelam wrote:
> Currently, the AF driver scans the mkex profile to identify all
> supported features. This process also involves checking for any
> fields that might overlap with each other.
> 
> For example, NPC_TCP_SPORT field offset within the key should
> not overlap with NPC_DMAC/NPC_SIP_IPV4 or any other field.
> 
> However, there are situations where some overlap is unavoidable.
> For instance, when extracting the SPI field, the same key offset might
> be used by both the AH and ESP layers. This patch addresses this
> specific scenario by skipping the overlap check and instead, adds
> a warning message to the user.

The commit message doesn't really explain what the implications for
field overlap are. The warning is also very uninformative. The user
will see the warning, find the commit which added it, and should be
able to more or less figure out what the implications are.
-- 
pw-bot: cr