[PATCH iwl-next 0/2] ice: fix TC flower filter priority violations

[PATCH iwl-next 0/2] ice: fix TC flower filter priority violations

[Petr Oros]

Two fixes for TC flower offload in the legacy switch mode. Both
address the same user visible failure, all traffic on a port silently
disappears after installing a filter chain that mixes offloadable and
non offloadable filters.

The first fix covers filters matching on ip_proto alone. The driver
never programmed the protocol lookup, so a filter like "ip_proto udp
action drop" was installed in hardware as a match on eth_type ipv4 and
dropped every IPv4 packet.

The second fix covers the interaction between software only filters and
offloaded drop filters. The E810 switch gives drop rules absolute
precedence over forwarding rules regardless of recipe priority, so a
lower priority drop offloaded into hardware overrides any higher
priority filter that stayed in software. The driver now tracks filters
it could not offload and refuses to offload a drop filter that would
bypass one of them, keeping the drop functional in software instead.

Tested on E810 with the OS default and comms DDP packages, including
the original reproducer from the report, L2TPv3 pass and drop chains
with and without session ID matching.

Petr Oros (2):
  ice: fix TC flower filters matching more than the ip_proto key
  ice: don't offload drop filters that bypass higher priority filters

 drivers/net/ethernet/intel/ice/ice.h        |   1 +
 drivers/net/ethernet/intel/ice/ice_tc_lib.c | 187 +++++++++++++++++++-
 drivers/net/ethernet/intel/ice/ice_tc_lib.h |  22 +++
 3 files changed, 201 insertions(+), 9 deletions(-)

-- 
2.55.0

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

[Petr Oros]

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@linux.intel.com/ [1]
Fixes: 0d08a441fb1a ("ice: ndo_setup_tc implementation for PF")
Signed-off-by: Petr Oros <poros@redhat.com>
---
 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

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@redhat.com>
> Sent: Thursday, September 10, 2026 5:48 PM
> To: netdev@vger.kernel.org
> Cc: Oros, Petr <poros@redhat.com>; Nguyen, Anthony L
> <anthony.l.nguyen@intel.com>; Kitszel, Przemyslaw
> <przemyslaw.kitszel@intel.com>; Andrew Lunn <andrew+netdev@lunn.ch>;
> David S. Miller <davem@davemloft.net>; Eric Dumazet
> <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni
> <pabeni@redhat.com>; Kushwaha, Priyalee <priyalee.kushwaha@intel.com>;
> Kiran Patil <kiran.patil@intel.com>; Drewek, Wojciech
> <wojciech.drewek@intel.com>; Michal Swiatkowski
> <michal.swiatkowski@linux.intel.com>; intel-wired-
> lan@lists.osuosl.org; linux-kernel@vger.kernel.org
> 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@linux.intel.com/ [1]
> Fixes: 0d08a441fb1a ("ice: ndo_setup_tc implementation for PF")
> Signed-off-by: Petr Oros <poros@redhat.com>
> ---
>  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@intel.com>

[PATCH iwl-next 2/2] ice: don't offload drop filters that bypass higher priority filters

[Petr Oros]

The E810 switch gives drop rules absolute precedence over forwarding
rules regardless of recipe priority (verified on E810 in every
combination of installation order and match specificity). TC however
requires filters to be evaluated in priority order, so when a higher
priority filter stays software-only because its match or action cannot
be offloaded while a lower priority drop filter is offloaded, the
hardware drops packets the higher priority filter should have seen:

  tc filter add dev $pf ingress prio 1 protocol ip flower \
      ip_proto l2tp action pass
  tc filter add dev $pf ingress prio 2 protocol ip flower action drop

The pass filter is rejected (unsupported action), the drop filter is
offloaded and the hardware drops all IPv4 traffic including the L2TP
packets the prio 1 filter should accept.

Track filters that were presented to the driver but not offloaded and
refuse to offload a drop filter when a higher priority filter on the
same device block and direction is software-only or is offloaded with a
forwarding action, which the hardware drop would equally override. The
refused drop filter keeps working in software. Filters are compared
only within one device block since TC priorities are not ordered across
blocks. Filters with skip_sw bypass the check because the user
explicitly requested hardware-only operation and refusing would fail
the filter add entirely, and the tracking applies to the legacy switch
mode only, switchdev pipelines manage rule priorities themselves.

The check does not analyze match overlap, so it is conservative, and it
runs only at drop offload time. A conflicting filter installed after a
drop filter was already offloaded cannot un-offload it, and filters the
driver never saw (added while hw-tc-offload was off) are invisible to
it. Installing filters in priority order with offload enabled avoids
both.

Fixes: 0d08a441fb1a ("ice: ndo_setup_tc implementation for PF")
Signed-off-by: Petr Oros <poros@redhat.com>
---
 drivers/net/ethernet/intel/ice/ice.h        |   1 +
 drivers/net/ethernet/intel/ice/ice_tc_lib.c | 150 +++++++++++++++++++-
 drivers/net/ethernet/intel/ice/ice_tc_lib.h |  21 +++
 3 files changed, 166 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/intel/ice/ice.h b/drivers/net/ethernet/intel/ice/ice.h
index db3c7015c56c43..6c596e5a315175 100644
--- a/drivers/net/ethernet/intel/ice/ice.h
+++ b/drivers/net/ethernet/intel/ice/ice.h
@@ -643,6 +643,7 @@ struct ice_pf {
 	 */
 	u16 num_dmac_chnl_fltrs;
 	struct hlist_head tc_flower_fltr_list;
+	struct hlist_head tc_sw_fltr_list;
 
 	u64 supported_rxdids;
 
diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
index fbd8cbad150a98..7cfc8941b7ba57 100644
--- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
+++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
@@ -2269,6 +2269,112 @@ ice_find_tc_flower_fltr(struct ice_pf *pf, unsigned long cookie)
 	return NULL;
 }
 
+/**
+ * ice_tc_fltr_is_drop - check if a filter carries a drop action
+ * @cls_flower: offload request describing the filter
+ *
+ * Return: true if any action of the filter is a drop, false otherwise.
+ */
+static bool ice_tc_fltr_is_drop(struct flow_cls_offload *cls_flower)
+{
+	struct flow_rule *rule = flow_cls_offload_flow_rule(cls_flower);
+	struct flow_action_entry *act;
+	int i;
+
+	if (cls_flower->classid)
+		return false;
+
+	flow_action_for_each(i, act, &rule->action)
+		if (act->id == FLOW_ACTION_DROP)
+			return true;
+
+	return false;
+}
+
+/**
+ * ice_tc_untrack_sw_fltr - forget a tracked software-only filter
+ * @pf: pointer to PF structure
+ * @cookie: unique filter identifier from the offload request
+ *
+ * Return: true if the filter was tracked, false otherwise.
+ */
+static bool ice_tc_untrack_sw_fltr(struct ice_pf *pf, unsigned long cookie)
+{
+	struct ice_tc_sw_fltr *sw_fltr;
+
+	hlist_for_each_entry(sw_fltr, &pf->tc_sw_fltr_list, node) {
+		if (sw_fltr->cookie != cookie)
+			continue;
+
+		hlist_del(&sw_fltr->node);
+		kfree(sw_fltr);
+		return true;
+	}
+
+	return false;
+}
+
+/**
+ * ice_tc_track_sw_fltr - remember a filter that was not offloaded
+ * @pf: pointer to PF structure
+ * @filter_dev: device the filter was requested on
+ * @cls_flower: offload request describing the filter
+ * @direction: block direction the filter was requested for
+ */
+static void ice_tc_track_sw_fltr(struct ice_pf *pf,
+				 struct net_device *filter_dev,
+				 struct flow_cls_offload *cls_flower,
+				 enum ice_eswitch_fltr_direction direction)
+{
+	struct ice_tc_sw_fltr *sw_fltr;
+
+	hlist_for_each_entry(sw_fltr, &pf->tc_sw_fltr_list, node)
+		if (sw_fltr->cookie == cls_flower->cookie)
+			return;
+
+	sw_fltr = kzalloc_obj(*sw_fltr);
+	if (!sw_fltr)
+		return;
+
+	sw_fltr->cookie = cls_flower->cookie;
+	sw_fltr->filter_dev = filter_dev;
+	sw_fltr->prio = cls_flower->common.prio;
+	sw_fltr->direction = direction;
+	sw_fltr->is_drop = ice_tc_fltr_is_drop(cls_flower);
+	hlist_add_head(&sw_fltr->node, &pf->tc_sw_fltr_list);
+}
+
+/**
+ * ice_tc_drop_bypasses_fltr - check if a drop rule would bypass a filter
+ * @pf: pointer to PF structure
+ * @filter_dev: device the drop filter is requested on
+ * @prio: TC priority of the drop filter
+ * @direction: block direction of the drop filter
+ *
+ * Return: true if such a filter exists, false otherwise.
+ */
+static bool
+ice_tc_drop_bypasses_fltr(struct ice_pf *pf, struct net_device *filter_dev,
+			  u32 prio, enum ice_eswitch_fltr_direction direction)
+{
+	struct ice_tc_flower_fltr *fltr;
+	struct ice_tc_sw_fltr *sw_fltr;
+
+	hlist_for_each_entry(sw_fltr, &pf->tc_sw_fltr_list, node)
+		if (sw_fltr->filter_dev == filter_dev &&
+		    sw_fltr->direction == direction && sw_fltr->prio < prio &&
+		    !sw_fltr->is_drop)
+			return true;
+
+	hlist_for_each_entry(fltr, &pf->tc_flower_fltr_list, tc_flower_node)
+		if (fltr->filter_dev == filter_dev &&
+		    fltr->direction == direction && fltr->prio < prio &&
+		    fltr->action.fltr_act != ICE_DROP_PACKET)
+			return true;
+
+	return false;
+}
+
 /**
  * ice_add_cls_flower - add TC flower filters
  * @netdev: Pointer to filter device
@@ -2284,14 +2390,24 @@ int ice_add_cls_flower(struct net_device *netdev, struct ice_vsi *vsi,
 {
 	struct netlink_ext_ack *extack = cls_flower->common.extack;
 	struct net_device *vsi_netdev = vsi->netdev;
+	enum ice_eswitch_fltr_direction direction;
 	struct ice_tc_flower_fltr *fltr;
 	struct ice_pf *pf = vsi->back;
+	bool track_sw_fltrs;
 	int err;
 
-	if (ice_is_reset_in_progress(pf->state))
-		return -EBUSY;
-	if (test_bit(ICE_FLAG_FW_LLDP_AGENT, pf->flags))
-		return -EINVAL;
+	direction = ingress ? ICE_ESWITCH_FLTR_INGRESS :
+			      ICE_ESWITCH_FLTR_EGRESS;
+	track_sw_fltrs = !ice_is_eswitch_mode_switchdev(pf);
+
+	if (ice_is_reset_in_progress(pf->state)) {
+		err = -EBUSY;
+		goto track_sw;
+	}
+	if (test_bit(ICE_FLAG_FW_LLDP_AGENT, pf->flags)) {
+		err = -EINVAL;
+		goto track_sw;
+	}
 
 	if (ice_is_port_repr_netdev(netdev))
 		vsi_netdev = netdev;
@@ -2304,7 +2420,8 @@ int ice_add_cls_flower(struct net_device *netdev, struct ice_vsi *vsi,
 		 */
 		if (netdev == vsi_netdev)
 			NL_SET_ERR_MSG_MOD(extack, "can't apply TC flower filters, turn ON hw-tc-offload and try again");
-		return -EINVAL;
+		err = -EINVAL;
+		goto track_sw;
 	}
 
 	/* avoid duplicate entries, if exists - return error */
@@ -2314,14 +2431,32 @@ int ice_add_cls_flower(struct net_device *netdev, struct ice_vsi *vsi,
 		return -EEXIST;
 	}
 
+	if (track_sw_fltrs && !cls_flower->common.skip_sw &&
+	    ice_tc_fltr_is_drop(cls_flower) &&
+	    ice_tc_drop_bypasses_fltr(pf, netdev, cls_flower->common.prio,
+				      direction)) {
+		NL_SET_ERR_MSG_MOD(extack,
+				   "Drop filter not offloaded because it would bypass a higher priority filter");
+		err = -EOPNOTSUPP;
+		goto track_sw;
+	}
+
 	/* prep and add TC-flower filter in HW */
 	err = ice_add_tc_fltr(netdev, vsi, cls_flower, &fltr, ingress);
 	if (err)
-		return err;
+		goto track_sw;
+
+	fltr->filter_dev = netdev;
+	fltr->prio = cls_flower->common.prio;
 
 	/* add filter into an ordered list */
 	hlist_add_head(&fltr->tc_flower_node, &pf->tc_flower_fltr_list);
 	return 0;
+
+track_sw:
+	if (track_sw_fltrs && !cls_flower->common.skip_sw)
+		ice_tc_track_sw_fltr(pf, netdev, cls_flower, direction);
+	return err;
 }
 
 /**
@@ -2336,6 +2471,9 @@ ice_del_cls_flower(struct ice_vsi *vsi, struct flow_cls_offload *cls_flower)
 	struct ice_pf *pf = vsi->back;
 	int err;
 
+	if (ice_tc_untrack_sw_fltr(pf, cls_flower->cookie))
+		return 0;
+
 	/* find filter */
 	fltr = ice_find_tc_flower_fltr(pf, cls_flower->cookie);
 	if (!fltr) {
diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.h b/drivers/net/ethernet/intel/ice/ice_tc_lib.h
index 752af65e70b7bf..37d6f4100ecfbc 100644
--- a/drivers/net/ethernet/intel/ice/ice_tc_lib.h
+++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.h
@@ -139,12 +139,33 @@ enum ice_eswitch_fltr_direction {
 	ICE_ESWITCH_FLTR_EGRESS,
 };
 
+/**
+ * struct ice_tc_sw_fltr - filter presented to the driver but not offloaded
+ * @node: node in the pf->tc_sw_fltr_list
+ * @cookie: unique filter identifier from the offload request
+ * @filter_dev: device the filter was requested on
+ * @prio: TC priority, lower value is evaluated first
+ * @direction: block direction the filter was requested for
+ * @is_drop: the filter carries a drop action
+ */
+struct ice_tc_sw_fltr {
+	struct hlist_node node;
+	unsigned long cookie;
+	struct net_device *filter_dev;
+	u32 prio;
+	enum ice_eswitch_fltr_direction direction;
+	bool is_drop;
+};
+
 struct ice_tc_flower_fltr {
 	struct hlist_node tc_flower_node;
 
 	/* cookie becomes filter_rule_id if rule is added successfully */
 	unsigned long cookie;
 
+	struct net_device *filter_dev;
+	u32 prio;
+
 	/* add_adv_rule returns information like recipe ID, rule_id. Store
 	 * those values since they are needed to remove advanced rule
 	 */
-- 
2.55.0

Re: [PATCH iwl-next 2/2] ice: don't offload drop filters that bypass higher priority filters

[Simon Horman]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/#/patchset/20260910154824.3603687-2-poros@redhat.com?part=2
---
ice: don't offload drop filters that bypass higher priority filters

This commit tracks software-only TC filters to prevent the hardware from
incorrectly offloading lower priority drop rules that would override them.
It tracks un-offloaded filters within a device block and rejects conflicting
drop offloads.

> diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> --- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
[ ... ]
> @@ -2269,6 +2269,112 @@ ice_find_tc_flower_fltr(struct ice_pf *pf, unsigned long cookie)
[ ... ]
> +static void ice_tc_track_sw_fltr(struct ice_pf *pf,
> +				 struct net_device *filter_dev,
> +				 struct flow_cls_offload *cls_flower,
> +				 enum ice_eswitch_fltr_direction direction)
> +{
> +	struct ice_tc_sw_fltr *sw_fltr;
> +
> +	hlist_for_each_entry(sw_fltr, &pf->tc_sw_fltr_list, node)
> +		if (sw_fltr->cookie == cls_flower->cookie)
> +			return;

[Severity: High]
Does this early return break tracking for TC shared blocks?

When a shared block is bound to multiple interfaces, the driver will only
track the cookie for the first interface because this check returns early
when it sees the same cookie.

> +
> +	sw_fltr = kzalloc_obj(*sw_fltr);
[ ... ]
> +static bool
> +ice_tc_drop_bypasses_fltr(struct ice_pf *pf, struct net_device *filter_dev,
> +			  u32 prio, enum ice_eswitch_fltr_direction direction)
> +{
> +	struct ice_tc_flower_fltr *fltr;
> +	struct ice_tc_sw_fltr *sw_fltr;
> +
> +	hlist_for_each_entry(sw_fltr, &pf->tc_sw_fltr_list, node)
> +		if (sw_fltr->filter_dev == filter_dev &&

[Severity: High]
Because the tracking entry is bound strictly to the first interface's
filter_dev, won't this check fail to protect other ports sharing the same
block?

> +		    sw_fltr->direction == direction && sw_fltr->prio < prio &&
> +		    !sw_fltr->is_drop)
> +			return true;
[ ... ]
> @@ -2336,6 +2471,9 @@ ice_del_cls_flower(struct ice_vsi *vsi, struct flow_cls_offload *cls_flower)
>  	struct ice_pf *pf = vsi->back;
>  	int err;
>  
> +	if (ice_tc_untrack_sw_fltr(pf, cls_flower->cookie))
> +		return 0;
> +

[Severity: High]
When deleting a filter from a shared block, the first net_device will delete
the unique tracking entry here. Won't subsequent callback invocations for the
other ports in the shared block fall through to the hardware failure path
and incorrectly return -EINVAL?

>  	/* find filter */
>  	fltr = ice_find_tc_flower_fltr(pf, cls_flower->cookie);
>  	if (!fltr) {

Later in ice_del_cls_flower(), the filter is removed and freed:

drivers/net/ethernet/intel/ice/ice_tc_lib.c:ice_del_cls_flower() {
    ...
	/* delete filter from an ordered list */
	hlist_del(&fltr->tc_flower_node);

	/* free the filter node */
	kfree(fltr);
    ...
}

[Severity: High]
This is a pre-existing issue, but is it safe to modify tc_flower_fltr_list
and free the filter here?

While this runs under the rtnl_lock, the background device reset task
(ice_reset_subtask) calls ice_rem_all_chnl_fltrs(), which iterates over
pf->tc_flower_fltr_list and frees filters using kfree() without holding
the rtnl_lock or any driver-specific lock protecting the list.

Could this concurrent access during a hardware reset result in list corruption
or a use-after-free?

Re: [PATCH iwl-next 2/2] ice: don't offload drop filters that bypass higher priority filters

[Petr Oros]

On 9/12/26 10:36, Simon Horman wrote:
> This is an AI-generated review of your patch. The human sending this
> email has considered the AI review valid, or at least plausible.
> Full review at: https://sashiko.dev/#/patchset/20260910154824.3603687-2-poros@redhat.com?part=2
> ---
> ice: don't offload drop filters that bypass higher priority filters
>
> This commit tracks software-only TC filters to prevent the hardware from
> incorrectly offloading lower priority drop rules that would override them.
> It tracks un-offloaded filters within a device block and rejects conflicting
> drop offloads.
>
>> diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
>> --- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
>> +++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> [ ... ]
>> @@ -2269,6 +2269,112 @@ ice_find_tc_flower_fltr(struct ice_pf *pf, unsigned long cookie)
> [ ... ]
>> +static void ice_tc_track_sw_fltr(struct ice_pf *pf,
>> +				 struct net_device *filter_dev,
>> +				 struct flow_cls_offload *cls_flower,
>> +				 enum ice_eswitch_fltr_direction direction)
>> +{
>> +	struct ice_tc_sw_fltr *sw_fltr;
>> +
>> +	hlist_for_each_entry(sw_fltr, &pf->tc_sw_fltr_list, node)
>> +		if (sw_fltr->cookie == cls_flower->cookie)
>> +			return;
> [Severity: High]
> Does this early return break tracking for TC shared blocks?
>
> When a shared block is bound to multiple interfaces, the driver will only
> track the cookie for the first interface because this check returns early
> when it sees the same cookie.
Correct. For a block bound to multiple devices of one PF only the first
binding's filter_dev is remembered, so the guard protects the first
binding only. The impact is a missed refusal on the other bindings,
never a false refusal, but it is easy to do better. I will rework the
tracking in v2 to keep one entry per cookie and filter_dev pair, so
every binding replay creates its own entry and the guard covers all of
them.
>
>> +
>> +	sw_fltr = kzalloc_obj(*sw_fltr);
> [ ... ]
>> +static bool
>> +ice_tc_drop_bypasses_fltr(struct ice_pf *pf, struct net_device *filter_dev,
>> +			  u32 prio, enum ice_eswitch_fltr_direction direction)
>> +{
>> +	struct ice_tc_flower_fltr *fltr;
>> +	struct ice_tc_sw_fltr *sw_fltr;
>> +
>> +	hlist_for_each_entry(sw_fltr, &pf->tc_sw_fltr_list, node)
>> +		if (sw_fltr->filter_dev == filter_dev &&
> [Severity: High]
> Because the tracking entry is bound strictly to the first interface's
> filter_dev, won't this check fail to protect other ports sharing the same
> block?
Yes, same root cause as above, fixed by the same v2 rework. The
per device comparison itself has to stay because TC priorities are not
ordered across blocks, only the tracking granularity was wrong.
>
>> +		    sw_fltr->direction == direction && sw_fltr->prio < prio &&
>> +		    !sw_fltr->is_drop)
>> +			return true;
> [ ... ]
>> @@ -2336,6 +2471,9 @@ ice_del_cls_flower(struct ice_vsi *vsi, struct flow_cls_offload *cls_flower)
>>   	struct ice_pf *pf = vsi->back;
>>   	int err;
>>   
>> +	if (ice_tc_untrack_sw_fltr(pf, cls_flower->cookie))
>> +		return 0;
>> +
> [Severity: High]
> When deleting a filter from a shared block, the first net_device will delete
> the unique tracking entry here. Won't subsequent callback invocations for the
> other ports in the shared block fall through to the hardware failure path
> and incorrectly return -EINVAL?
They will, though that is what every DESTROY for an unknown cookie
returned before this patch too, so nothing gets worse. With the v2
per binding tracking each DESTROY invocation removes its own entry and
returns 0, which fixes this for tracked filters as a side effect.
>
>>   	/* find filter */
>>   	fltr = ice_find_tc_flower_fltr(pf, cls_flower->cookie);
>>   	if (!fltr) {
> Later in ice_del_cls_flower(), the filter is removed and freed:
>
> drivers/net/ethernet/intel/ice/ice_tc_lib.c:ice_del_cls_flower() {
>      ...
> 	/* delete filter from an ordered list */
> 	hlist_del(&fltr->tc_flower_node);
>
> 	/* free the filter node */
> 	kfree(fltr);
>      ...
> }
>
> [Severity: High]
> This is a pre-existing issue, but is it safe to modify tc_flower_fltr_list
> and free the filter here?
>
> While this runs under the rtnl_lock, the background device reset task
> (ice_reset_subtask) calls ice_rem_all_chnl_fltrs(), which iterates over
> pf->tc_flower_fltr_list and frees filters using kfree() without holding
> the rtnl_lock or any driver-specific lock protecting the list.
>
> Could this concurrent access during a hardware reset result in list corruption
> or a use-after-free?
Confirmed, this race is real and pre-existing. ice_prepare_for_reset()
runs from the service task without RTNL and for a non PFR reset with
mqprio active it calls ice_remove_q_channels() with rem_fltr true,
which walks and frees tc_flower_fltr_list entries concurrently with
the RTNL serialized cls_flower paths. It predates this series and the
list reader added here runs in the same RTNL context as the existing
readers, so this series does not widen it. I will address it in a
separate fix.

I will send v2 of this series with the per binding tracking rework.

Regards,

Petr