RE: [PATCH iwl-next 1/2] ice: fix TC flower filters matching more than the ip_proto key

[Loktionov, Aleksandr]

> -----Original Message-----
> From: Petr Oros <poros@xxxxxxxxxx>
> Sent: Thursday, September 10, 2026 5:48 PM
> To: netdev@xxxxxxxxxxxxxxx
> Cc: Oros, Petr <poros@xxxxxxxxxx>; Nguyen, Anthony L
> <anthony.l.nguyen@xxxxxxxxx>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@xxxxxxxxx>; Andrew Lunn <andrew+netdev@xxxxxxx>;
> David S. Miller <davem@xxxxxxxxxxxxx>; Eric Dumazet
> <edumazet@xxxxxxxxxx>; Jakub Kicinski <kuba@xxxxxxxxxx>; Paolo Abeni
> <pabeni@xxxxxxxxxx>; Kushwaha, Priyalee <priyalee.kushwaha@xxxxxxxxx>;
> Kiran Patil <kiran.patil@xxxxxxxxx>; Drewek, Wojciech
> <wojciech.drewek@xxxxxxxxx>; Michal Swiatkowski
> <michal.swiatkowski@xxxxxxxxxxxxxxx>; intel-wired-
> lan@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: [PATCH iwl-next 1/2] ice: fix TC flower filters matching more
> than the ip_proto key
> 
> ice_parse_cls_flower() stores the ip_proto key from the flow rule but
> never programs a matching lookup unless the filter also matches on L4
> ports or the L2TPv3 session ID. A filter like:
> 
>   tc filter add dev $pf ingress protocol ip flower skip_sw \
>       ip_proto udp action drop
> 
> is silently programmed into the hardware as a match on eth_type ipv4
> alone and drops every IPv4 packet, not just UDP.
> 
> Program the IP protocol match through the protocol field of the IPv4
> header lookup and the next header field of the IPv6 header lookup, the
> same lookups that are already used for ToS and TTL. The OS default and
> comms DDP packages provide no profile that extracts the IPv6 next
> header word, so the IPv6 rule programming currently fails with
> "Required profiles not found" and the filter falls back to software
> evaluation instead of over-matching, and the offload starts working
> with a DDP package that can extract it. Note that the lookup matches
> the next header byte of the base IPv6 header, so packets carrying
> extension headers are not matched in hardware and fall back to
> software evaluation, which under-matches only for skip_sw filters.
> 
> GTP tunnel and PPPoE filters rewrite the parsed ethertype, the IP
> header lookups are not available there, so reject an unconsumed
> ip_proto for them instead of silently widening the match. Filters
> where ip_proto is implied by an L4 ports or L2TPv3 session ID lookup
> are not affected. Based on an earlier unapplied patch from Michal
> Swiatkowski that implemented the IPv4 part [1].
> 
> Link: https://lore.kernel.org/intel-wired-lan/20240222123956.2393-3-
> michal.swiatkowski@xxxxxxxxxxxxxxx/ [1]
> Fixes: 0d08a441fb1a ("ice: ndo_setup_tc implementation for PF")
> Signed-off-by: Petr Oros <poros@xxxxxxxxxx>
> ---
>  drivers/net/ethernet/intel/ice/ice_tc_lib.c | 37 +++++++++++++++++++-
> -  drivers/net/ethernet/intel/ice/ice_tc_lib.h |  1 +
>  2 files changed, 35 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> index d20357c0412731..fbd8cbad150a98 100644
> --- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> @@ -78,7 +78,8 @@ static int ice_tc_count_lkups(u32 flags, struct
> ice_tc_flower_fltr *fltr)
>  		     ICE_TC_FLWR_FIELD_DEST_IPV6 |
> ICE_TC_FLWR_FIELD_SRC_IPV6))
>  		lkups_cnt++;
> 
> -	if (flags & (ICE_TC_FLWR_FIELD_IP_TOS |
> ICE_TC_FLWR_FIELD_IP_TTL))
> +	if (flags & (ICE_TC_FLWR_FIELD_IP_TOS |
> ICE_TC_FLWR_FIELD_IP_TTL |
> +		     ICE_TC_FLWR_FIELD_IP_PROTO))
>  		lkups_cnt++;
> 
>  	/* are L2TPv3 options specified? */
> @@ -552,7 +553,8 @@ ice_tc_fill_rules(struct ice_hw *hw, u32 flags,
>  	}
> 
>  	if (headers->l2_key.n_proto == htons(ETH_P_IP) &&
> -	    (flags & (ICE_TC_FLWR_FIELD_IP_TOS |
> ICE_TC_FLWR_FIELD_IP_TTL))) {
> +	    (flags & (ICE_TC_FLWR_FIELD_IP_TOS |
> ICE_TC_FLWR_FIELD_IP_TTL |
> +		      ICE_TC_FLWR_FIELD_IP_PROTO))) {
>  		list[i].type = ice_proto_type_from_ipv4(inner);
> 
>  		if (flags & ICE_TC_FLWR_FIELD_IP_TOS) { @@ -567,11
> +569,19 @@ ice_tc_fill_rules(struct ice_hw *hw, u32 flags,
>  				headers->l3_mask.ttl;
>  		}
> 
> +		if (flags & ICE_TC_FLWR_FIELD_IP_PROTO) {
> +			list[i].h_u.ipv4_hdr.protocol =
> +				headers->l3_key.ip_proto;
> +			list[i].m_u.ipv4_hdr.protocol =
> +				headers->l3_mask.ip_proto;
> +		}
> +
>  		i++;
>  	}
> 
>  	if (headers->l2_key.n_proto == htons(ETH_P_IPV6) &&
> -	    (flags & (ICE_TC_FLWR_FIELD_IP_TOS |
> ICE_TC_FLWR_FIELD_IP_TTL))) {
> +	    (flags & (ICE_TC_FLWR_FIELD_IP_TOS |
> ICE_TC_FLWR_FIELD_IP_TTL |
> +		      ICE_TC_FLWR_FIELD_IP_PROTO))) {
>  		struct ice_ipv6_hdr *hdr_h, *hdr_m;
> 
>  		hdr_h = &list[i].h_u.ipv6_hdr;
> @@ -592,6 +602,11 @@ ice_tc_fill_rules(struct ice_hw *hw, u32 flags,
>  			hdr_m->hop_limit = headers->l3_mask.ttl;
>  		}
> 
> +		if (flags & ICE_TC_FLWR_FIELD_IP_PROTO) {
> +			hdr_h->next_hdr = headers->l3_key.ip_proto;
> +			hdr_m->next_hdr = headers->l3_mask.ip_proto;
> +		}
> +
>  		i++;
>  	}
> 
> @@ -1737,6 +1752,9 @@ ice_parse_cls_flower(struct net_device
> *filter_dev, struct ice_vsi *vsi,
>  		headers->l2_key.n_proto = cpu_to_be16(n_proto_key);
>  		headers->l2_mask.n_proto = cpu_to_be16(n_proto_mask);
>  		headers->l3_key.ip_proto = match.key->ip_proto;
> +		headers->l3_mask.ip_proto = match.mask->ip_proto;
> +		if (match.mask->ip_proto)
> +			fltr->flags |= ICE_TC_FLWR_FIELD_IP_PROTO;
>  	}
> 
>  	if (flow_rule_match_key(rule, FLOW_DISSECTOR_KEY_ETH_ADDRS)) {
> @@ -1910,6 +1928,19 @@ ice_parse_cls_flower(struct net_device
> *filter_dev, struct ice_vsi *vsi,
>  		}
>  	}
> 
> +	if (fltr->flags & (ICE_TC_FLWR_FIELD_DEST_L4_PORT |
> +			   ICE_TC_FLWR_FIELD_SRC_L4_PORT |
> +			   ICE_TC_FLWR_FIELD_L2TPV3_SESSID))
> +		fltr->flags &= ~ICE_TC_FLWR_FIELD_IP_PROTO;
> +
> +	if ((fltr->flags & ICE_TC_FLWR_FIELD_IP_PROTO) &&
> +	    headers->l2_key.n_proto != htons(ETH_P_IP) &&
> +	    headers->l2_key.n_proto != htons(ETH_P_IPV6)) {
> +		NL_SET_ERR_MSG_MOD(fltr->extack,
> +				   "IP protocol match is not supported with
> GTP or PPPoE");
> +		return -EOPNOTSUPP;
> +	}
> +
>  	/* Ingress filter on representor results in an egress filter in
> HW
>  	 * and vice versa
>  	 */
> diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.h
> b/drivers/net/ethernet/intel/ice/ice_tc_lib.h
> index 8a3ab2f22af9ba..752af65e70b7bf 100644
> --- a/drivers/net/ethernet/intel/ice/ice_tc_lib.h
> +++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.h
> @@ -38,6 +38,7 @@
>  #define ICE_TC_FLWR_FIELD_CVLAN_PRIO		BIT(28)
>  #define ICE_TC_FLWR_FIELD_VLAN_TPID		BIT(29)
>  #define ICE_TC_FLWR_FIELD_PFCP_OPTS		BIT(30)
> +#define ICE_TC_FLWR_FIELD_IP_PROTO		BIT(31)
> 
>  #define ICE_TC_FLOWER_MASK_32   0xFFFFFFFF
> 
> --
> 2.55.0

Reviewed-by: Aleksandr Loktionov <aleksandr.loktionov@xxxxxxxxx>