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

[Holda, Patryk]

> -----Original Message-----
> From: Petr Oros <poros@xxxxxxxxxx>
> Sent: Wednesday, September 16, 2026 4:42 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>; Loktionov, Aleksandr
> <aleksandr.loktionov@xxxxxxxxx>; Simon Horman <horms@xxxxxxxxxx>; intel-
> wired-lan@xxxxxxxxxxxxxxxx; linux-kernel@xxxxxxxxxxxxxxx
> Subject: [PATCH iwl-next v2 2/2] ice: don't offload drop filters that bypass
> higher priority filters
> 
> 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, and a shared block bound to multiple
> devices is tracked once per binding. 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@xxxxxxxxxx>
> ---
> v2:
> - track one entry per cookie and filter_dev pair instead of one per
>   cookie, so a shared block bound to multiple devices of one PF is
>   protected on every binding and each DESTROY invocation removes its
>   own entry instead of returning -EINVAL for the later bindings
>   (sashiko review via Simon Horman)
> - pass the filter device to ice_del_cls_flower() for the lookup above
> 
> v1: https://lore.kernel.org/all/20260910154824.3603687-3-
> poros@xxxxxxxxxx/
>  drivers/net/ethernet/intel/ice/ice.h        |   1 +
>  drivers/net/ethernet/intel/ice/ice_main.c   |   2 +-
>  drivers/net/ethernet/intel/ice/ice_repr.c   |   2 +-
>  drivers/net/ethernet/intel/ice/ice_tc_lib.c | 159 +++++++++++++++++++-
> drivers/net/ethernet/intel/ice/ice_tc_lib.h |  23 ++-
>  5 files changed, 177 insertions(+), 10 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_main.c
> b/drivers/net/ethernet/intel/ice/ice_main.c
> index 3b76eb58666c07..3e6976cbb45c22 100644
> --- a/drivers/net/ethernet/intel/ice/ice_main.c
> +++ b/drivers/net/ethernet/intel/ice/ice_main.c
> @@ -8392,7 +8392,7 @@ ice_setup_tc_cls_flower(struct ice_netdev_priv
> *np,
>  	case FLOW_CLS_REPLACE:
>  		return ice_add_cls_flower(filter_dev, vsi, cls_flower, ingress);
>  	case FLOW_CLS_DESTROY:
> -		return ice_del_cls_flower(vsi, cls_flower);
> +		return ice_del_cls_flower(filter_dev, vsi, cls_flower);
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/drivers/net/ethernet/intel/ice/ice_repr.c
> b/drivers/net/ethernet/intel/ice/ice_repr.c
> index 096566c697f444..312a1bda460035 100644
> --- a/drivers/net/ethernet/intel/ice/ice_repr.c
> +++ b/drivers/net/ethernet/intel/ice/ice_repr.c
> @@ -223,7 +223,7 @@ ice_repr_setup_tc_cls_flower(struct ice_repr *repr,
>  		return ice_add_cls_flower(repr->netdev, repr->src_vsi, flower,
>  					  true);
>  	case FLOW_CLS_DESTROY:
> -		return ice_del_cls_flower(repr->src_vsi, flower);
> +		return ice_del_cls_flower(repr->netdev, repr->src_vsi, flower);
>  	default:
>  		return -EINVAL;
>  	}
> diff --git a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> index fbd8cbad150a98..fec6970a74cfbf 100644
> --- a/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> +++ b/drivers/net/ethernet/intel/ice/ice_tc_lib.c
> @@ -2269,6 +2269,117 @@ 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
> + * @filter_dev: device the filter was requested on
> + * @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,
> +				   struct net_device *filter_dev,
> +				   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 ||
> +		    sw_fltr->filter_dev != filter_dev)
> +			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 &&
> +		    sw_fltr->filter_dev == filter_dev)
> +			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 +2395,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 +2425,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,28
> +2436,51 @@ 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;
>  }
> 
>  /**
>   * ice_del_cls_flower - delete TC flower filters
> + * @netdev: Pointer to filter device
>   * @vsi: Pointer to VSI
>   * @cls_flower: Pointer to struct flow_cls_offload
>   */
>  int
> -ice_del_cls_flower(struct ice_vsi *vsi, struct flow_cls_offload *cls_flower)
> +ice_del_cls_flower(struct net_device *netdev, struct ice_vsi *vsi,
> +		   struct flow_cls_offload *cls_flower)
>  {
>  	struct ice_tc_flower_fltr *fltr;
>  	struct ice_pf *pf = vsi->back;
>  	int err;
> 
> +	if (ice_tc_untrack_sw_fltr(pf, netdev, 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..47444b7dadd257 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
>  	 */
> @@ -214,7 +235,7 @@ static inline int ice_chnl_dmac_fltr_cnt(struct ice_pf
> *pf)  struct ice_vsi *ice_locate_vsi_using_queue(struct ice_vsi *vsi, int queue);
> int ice_add_cls_flower(struct net_device *netdev, struct ice_vsi *vsi,
>  		       struct flow_cls_offload *cls_flower, bool ingress); -int
> ice_del_cls_flower(struct ice_vsi *vsi,
> +int ice_del_cls_flower(struct net_device *netdev, struct ice_vsi *vsi,
>  		       struct flow_cls_offload *cls_flower);  void
> ice_replay_tc_fltrs(struct ice_pf *pf);  bool ice_is_tunnel_supported(struct
> net_device *dev);
> --
> 2.55.0

Tested-by: Patryk Holda <patryk.holda@xxxxxxxxx>