Re: [PATCH] net/ice: fix L2TPv2 outer MAC address in training packet

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Apr 06, 2026 at 01:43:55PM +0530, Shaiq Wani wrote:
> flow rules with eth + l2tpv2 patterns 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. Also, ice_fdir_get_gen_prgm_pkt() uses the 'loc'
> pointer (past the L2TPv2 header) instead of 'pkt' (Ethernet offset 0).
> 
> Detect L2TPV2 in pre-scan, and use 'pkt' for MAC insertion in all
> four L2TPv2 training-packet cases.
> 
> Fixes: 733640dae75e ("net/ice: support L2TPv2 flow pattern matching")
> Fixes: bf662653976e ("net/ice/base: support L2TPv2 flow rule")
> Signed-off-by: Shaiq Wani <shaiq.wani@intel.com>
> ---
>  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) {

My concern with this change is that it makes the code different from all
the code surrouding it - all of which use "loc" rather than "pkt". While
there are some entries at the top of the function using pkt, almost all use
"loc". When do you need to use pkt vs loc? Should that be doucmented in a
comment in the function?

> 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;
>  		/* To align with shared code behavior, save gtpu outer
>  		 * fields in inner struct.
>  		 */

While I understand that both changes in this patch are to do with L2TP,
they actually appear as independent fixes. Consider splitting these into
two separate fix patches. [Note too how you have to describe them
separately in the commit log, and you have two fixlines for the patch]

/Bruce