|SUCCESS| net/ice: fix L2TPv2 outer MAC address in training packet

[dpdklab at iol.unh.edu]

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

_AI Code Review_

=== Patch Review: series.patch (via Claude) ===
Review date: 2026-04-06

This patch addresses a resource-assignment issue in L2TPv2 flow pattern
matching. I have reviewed it against the DPDK guidelines and have the
following comments:

git sha: Not available (patch file format)
Author: Shaiq Wani <shaiq.wani at intel.com>
Subject: [PATCH] net/ice: fix L2TPv2 outer MAC address in training packet

The commit fixes incorrect placement of outer MAC addresses in L2TPv2
training packets by detecting L2TPv2 tunnel type during pre-scan and using
the correct base pointer for MAC insertion.

> diff --git a/drivers/net/intel/ice/base/ice_fdir.c b/drivers/net/intel/ice/base/ice_fdir.c
> index 2c0cb99854..e1d8b65972 100644
> --- a/drivers/net/intel/ice/base/ice_fdir.c
> +++ b/drivers/net/intel/ice/base/ice_fdir.c
> @@ -4599,16 +4599,16 @@ ice_fdir_get_gen_prgm_pkt(struct ice_hw *hw, struct ice_fdir_fltr *input,
>  				     input->ip.v6.tc);
>  		break;
>  	case ICE_FLTR_PTYPE_NONF_IPV4_L2TPV2_CONTROL:
> -		ice_pkt_insert_mac_addr(loc, input->ext_data_outer.dst_mac);
> -		ice_pkt_insert_mac_addr(loc + ETH_ALEN,
> +		ice_pkt_insert_mac_addr(pkt, input->ext_data_outer.dst_mac);
> +		ice_pkt_insert_mac_addr(pkt + ETH_ALEN,
>  					input->ext_data_outer.src_mac);

The patch changes pointer arithmetic from 'loc' to 'pkt' for MAC address
insertion. Does ice_pkt_insert_mac_addr() verify that 'pkt + ETH_ALEN'
stays within the allocated packet buffer bounds in all cases?

>  		ice_pkt_insert_u16(loc, ICE_IPV4_L2TPV2_LEN_SESS_ID_OFFSET,
>  				   input->l2tpv2_data.session_id);
>  		break;
>  	case ICE_FLTR_PTYPE_NONF_IPV4_L2TPV2:
>  	case ICE_FLTR_PTYPE_NONF_IPV4_L2TPV2_PPP:
> -		ice_pkt_insert_mac_addr(loc, input->ext_data_outer.dst_mac);
> -		ice_pkt_insert_mac_addr(loc + ETH_ALEN,
> +		ice_pkt_insert_mac_addr(pkt, input->ext_data_outer.dst_mac);
> +		ice_pkt_insert_mac_addr(pkt + ETH_ALEN,
>  					input->ext_data_outer.src_mac);
>  		flags_version = BE16_TO_CPU(input->l2tpv2_data.flags_version);
>  		if (flags_version & ICE_L2TPV2_FLAGS_LEN) {

[ ... ]

> diff --git a/drivers/net/intel/ice/ice_fdir_filter.c b/drivers/net/intel/ice/ice_fdir_filter.c
> index 3522d77123..1cbc613020 100644
> --- a/drivers/net/intel/ice/ice_fdir_filter.c
> +++ b/drivers/net/intel/ice/ice_fdir_filter.c
> @@ -1911,6 +1911,8 @@ ice_fdir_parse_pattern(__rte_unused struct ice_adapter *ad,
>  	for (item = pattern; item->type != RTE_FLOW_ITEM_TYPE_END; item++) {
>  		if (item->type == RTE_FLOW_ITEM_TYPE_VXLAN)
>  			tunnel_type = ICE_FDIR_TUNNEL_TYPE_VXLAN;
> +		if (item->type == RTE_FLOW_ITEM_TYPE_L2TPV2)
> +			tunnel_type = ICE_FDIR_TUNNEL_TYPE_L2TPV2;

According to the DPDK C coding style, comparisons to NULL and 0 should be
explicit. Does this code follow the guideline? The comparison with
RTE_FLOW_ITEM_TYPE_VXLAN on the previous line should also use an explicit
comparison operator:

        if (item->type == RTE_FLOW_ITEM_TYPE_VXLAN)

Additionally, can tunnel_type be overwritten if both VXLAN and L2TPV2
patterns are present? Should these conditions be mutually exclusive using
else-if, or is there validation elsewhere that prevents conflicting tunnel
types?

>  		/* To align with shared code behavior, save gtpu outer
>  		 * fields in inner struct.
>  		 */