|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: <unknown - reviewing series.patch>
Author: Shaiq Wani <shaiq.wani at intel.com>
Subject: [PATCH] net/ice: fix FDIR session ID offset for L-bit L2TPv2 types

This patch fixes a bug where the driver incorrectly used UDP payload
offset 12 for the L2TPv2 session_id field when the Length bit is set,
which actually places it at offset 14. The fix wires up the previously
unused ICE_FLOW_FIELD_IDX_L2TPV2_LEN_SESS_ID and selects 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

[ ... ]

> @@ -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)
> +					input_set_o |= ICE_INSET_L2TPV2OIP_LEN_SESSION_ID;
> +				else
> +					input_set_o |= ICE_INSET_L2TPV2OIP_SESSION_ID;
>  			}

Can this logic overflow if flags_version contains unexpected values? The
function ice_fdir_parse_pattern() appears to accept flags_version from
l2tpv2_spec->hdr.common.flags_version without validation. Does this code
handle cases where flags_version has invalid or unexpected L2TPv2 type
values?

Does the nested if-statement under the existing compound condition follow
the coding style for operator placement? The DPDK coding style guide
requires operators at the beginning of continuation lines. This nested
if-statement appears to place the logical OR operators at the end of
lines rather than at the beginning of the next line.

> 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)