|SUCCESS| net/ice: fix FDIR session ID offset for L-bit L2TPv2 types

[dpdklab at iol.unh.edu]

Test-Label: ai-code-review
Test-Status: SUCCESS
https://dpdk.org/patch/162775

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-03-30

git sha: (from series.patch)
Author: Shaiq Wani <shaiq.wani at intel.com>

net/ice: fix FDIR session ID offset for L-bit L2TPv2 types

L2TPv2 types with the Length bit set have a 2-byte Length field before
Tunnel ID, placing session_id at UDP payload offset 14, not 12. The
driver always used offset 12, causing FDIR to match on wrong bytes.

Wire up ICE_FLOW_FIELD_IDX_L2TPV2_LEN_SESS_ID which was defined but
never used, and select it based on the L-bit in flags_version.

> diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c
> index 3522d77123..bb41f91168 100644
> --- a/drivers/net/intel/ice/ice_fdir_filter.c
> +++ b/drivers/net/intel/ice/ice_fdir_filter.c
> @@ -110,7 +110,8 @@
>  	ICE_INSET_NAT_T_ESP_SPI)
>  
>  #define ICE_FDIR_INSET_L2TPV2 (\
> -	ICE_INSET_SMAC | ICE_INSET_DMAC | ICE_INSET_L2TPV2OIP_SESSION_ID)
> +	ICE_INSET_SMAC | ICE_INSET_DMAC | ICE_INSET_L2TPV2OIP_SESSION_ID | \
> +	ICE_INSET_L2TPV2OIP_LEN_SESSION_ID)
>  

[ ... ]

> @@ -2598,7 +2600,14 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
>  			     l2tpv2_mask->hdr.type1.session_id == UINT16_MAX) ||
>  			    (flags_version == RTE_L2TPV2_MSG_TYPE_DATA_L_S_O &&
>  			     l2tpv2_mask->hdr.type0.session_id == UINT16_MAX)) {
> -				input_set_o |= ICE_INSET_L2TPV2OIP_SESSION_ID;
> +				if (flags_version == RTE_L2TPV2_MSG_TYPE_CONTROL ||
> +				    flags_version == RTE_L2TPV2_MSG_TYPE_DATA_L ||
> +				    flags_version == RTE_L2TPV2_MSG_TYPE_DATA_L_S ||
> +				    flags_version == RTE_L2TPV2_MSG_TYPE_DATA_L_O ||
> +				    flags_version == RTE_L2TPV2_MSG_TYPE_DATA_L_S_O)

In ice_fdir_parse_pattern(), does this condition have proper operator
precedence? The if statement appears to be at the same nesting level as
the surrounding parenthesized OR expressions for session_id checks. Is
the brace placement correct, or should the inner if condition be checked
before the session_id mask validation?

Looking at the code structure:

  if ((flags_version == X && mask == MAX) ||
      (flags_version == Y && mask == MAX)) {
      if (flags_version == A || ... || flags_version == E)
          input_set_o |= LEN_SESSION_ID;
      else
          input_set_o |= SESSION_ID;
  }

Does this logic correctly handle all L2TPv2 message types? The outer
condition already filters for specific flags_version values
(RTE_L2TPV2_MSG_TYPE_DATA_S and RTE_L2TPV2_MSG_TYPE_DATA_L_S_O). Can
the inner if condition match message types not already validated by the
outer condition?

> +					input_set_o |= ICE_INSET_L2TPV2OIP_LEN_SESSION_ID;
> +				else
> +					input_set_o |= ICE_INSET_L2TPV2OIP_SESSION_ID;
>  			}
>  
>  			tunnel_type = ICE_FDIR_TUNNEL_TYPE_L2TPV2;

[ ... ]

> diff --git a/drivers/net/intel/ice/ice_generic_flow.h b/drivers/net/intel/ice/ice_generic_flow.h
> index 1b5514d5df..c0eed84610 100644
> --- a/drivers/net/intel/ice/ice_generic_flow.h
> +++ b/drivers/net/intel/ice/ice_generic_flow.h
> @@ -66,6 +66,7 @@
>  #define ICE_IP_PK_ID			BIT_ULL(40)
>  #define ICE_IP_FRAG_OFS			BIT_ULL(39)
>  #define ICE_L2TPV2OIP_SESSION_ID	BIT_ULL(38)
> +#define ICE_L2TPV2OIP_LEN_SESSION_ID	BIT_ULL(37)

Does using bit 37 conflict with any existing bit definitions? Are there
any gaps in the bit allocation that should be filled first, or is bit
37 intentionally chosen for a specific reason?