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

dpdklab at iol.unh.edu dpdklab at iol.unh.edu
Mon Apr 6 13:50:56 CEST 2026


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

I will review this DPDK patch for compliance with coding standards and guidelines.

```
commit (git sha not provided in patch file)
Author: Shaiq Wani <shaiq.wani at intel.com>

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

This patch fixes flow rules with eth + l2tpv2 patterns that fail to
match because outer MACs are written to the wrong location in the
training packet. The pre-scan loop does not detect L2TPV2, so
tunnel_type is still 0 when the ETH item is parsed and MACs land in
ext_data (inner) instead of ext_data_outer.

>  drivers/net/intel/ice/base/ice_fdir.c   | 16 ++++++++--------
>  drivers/net/intel/ice/ice_fdir_filter.c |  2 ++
>  2 files changed, 10 insertions(+), 8 deletions(-)
> 
> 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);
>  		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) {
> @@ -4622,16 +4622,16 @@ ice_fdir_get_gen_prgm_pkt(struct ice_hw *hw, struct ice_fdir_fltr *input,
>  		}
>  		break;
>  	case ICE_FLTR_PTYPE_NONF_IPV6_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);
>  		ice_pkt_insert_u16(loc, ICE_IPV6_L2TPV2_LEN_SESS_ID_OFFSET,
>  				   input->l2tpv2_data.session_id);
>  		break;
>  	case ICE_FLTR_PTYPE_NONF_IPV6_L2TPV2:
>  	case ICE_FLTR_PTYPE_NONF_IPV6_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;

Does the lack of braces on this if statement violate the coding style
guide? DPDK requires braces on all control statements even when they are
single line bodies. Looking at the existing VXLAN check above, it also
lacks braces, but that does not make this addition compliant with the
documented standard.

Additionally, does this control flow create correct behavior when both
VXLAN and L2TPV2 items appear in the same pattern? Would tunnel_type be
overwritten sequentially, potentially misclassifying the tunnel type?

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


More information about the test-report mailing list