diff mbox series

ice: identify aRFS flows using L3/L4 dissector info

Message ID 20230314230401.44098-1-ahmed.zaki@intel.com
State Changes Requested
Delegated to: Anthony Nguyen
Headers show
Series ice: identify aRFS flows using L3/L4 dissector info | expand

Commit Message

Ahmed Zaki March 14, 2023, 11:04 p.m. UTC 
The flow ID passed to ice_rx_flow_steer() is computed like this:

    flow_id = skb_get_hash(skb) & flow_table->mask;

With smaller aRFS tables (for example, size 256) and higher number of
flows, there is a good chance of flow ID collisions where two or more
different flows are using the same flow ID. This results in the aRFS
destination queue constantly changing for all flows sharing that ID.

Use the full L3/L4 flow dissector info to identify the steered flow
instead of the passed flow ID.

Fixes: 28bf26724fdb ("ice: Implement aRFS")
Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
---
 drivers/net/ethernet/intel/ice/ice_arfs.c | 44 +++++++++++++++++++++--
 1 file changed, 41 insertions(+), 3 deletions(-)

Comments

Arland, ArpanaX April 4, 2023, 6:33 p.m. UTC | #1 
> -----Original Message-----
> From: Intel-wired-lan <intel-wired-lan-bounces@osuosl.org> On Behalf Of Zaki, Ahmed
> Sent: Wednesday, March 15, 2023 4:34 AM
> To: intel-wired-lan@lists.osuosl.org
> Subject: [Intel-wired-lan] [PATCH] ice: identify aRFS flows using L3/L4 dissector info
>
> The flow ID passed to ice_rx_flow_steer() is computed like this:
>
>   flow_id = skb_get_hash(skb) & flow_table->mask;
>
> With smaller aRFS tables (for example, size 256) and higher number of flows, there is a good chance of flow ID collisions where two or more different flows are using the same flow ID. This results in the aRFS destination queue constantly changing for all flows sharing that ID.
>
> Use the full L3/L4 flow dissector info to identify the steered flow instead of the passed flow ID.
>
> Fixes: 28bf26724fdb ("ice: Implement aRFS")
> Signed-off-by: Ahmed Zaki <ahmed.zaki@intel.com>
> ---
>  drivers/net/ethernet/intel/ice/ice_arfs.c | 44 +++++++++++++++++++++--
>  1 file changed, 41 insertions(+), 3 deletions(-)
>

Tested-by: Arpana Arland <arpanax.arland@intel.com> (A Contingent worker at Intel)
diff mbox series

Patch

diff --git a/drivers/net/ethernet/intel/ice/ice_arfs.c b/drivers/net/ethernet/intel/ice/ice_arfs.c
index fba178e07600..d7ae64d21e01 100644
--- a/drivers/net/ethernet/intel/ice/ice_arfs.c
+++ b/drivers/net/ethernet/intel/ice/ice_arfs.c
@@ -345,6 +345,44 @@  ice_arfs_build_entry(struct ice_vsi *vsi, const struct flow_keys *fk,
 	return arfs_entry;
 }
 
+/**
+ * ice_arfs_cmp - compare flow to a saved ARFS entry's filter info
+ * @fltr_info: filter info of the saved ARFS entry
+ * @fk: flow dissector keys
+ *
+ * Caller must hold arfs_lock if @fltr_info belongs to arfs_fltr_list
+ */
+static bool
+ice_arfs_cmp(struct ice_fdir_fltr *fltr_info, const struct flow_keys *fk)
+{
+	bool is_ipv4;
+
+	if (!fltr_info || !fk)
+		return false;
+
+	is_ipv4 = (fltr_info->flow_type == ICE_FLTR_PTYPE_NONF_IPV4_UDP ||
+		fltr_info->flow_type == ICE_FLTR_PTYPE_NONF_IPV4_TCP);
+
+	if (fk->basic.n_proto == htons(ETH_P_IP) && is_ipv4)
+		return (fltr_info->ip.v4.proto == fk->basic.ip_proto &&
+			fltr_info->ip.v4.src_port == fk->ports.src &&
+			fltr_info->ip.v4.dst_port == fk->ports.dst &&
+			fltr_info->ip.v4.src_ip == fk->addrs.v4addrs.src &&
+			fltr_info->ip.v4.dst_ip == fk->addrs.v4addrs.dst);
+	else if (fk->basic.n_proto == htons(ETH_P_IPV6) && !is_ipv4)
+		return (fltr_info->ip.v6.proto == fk->basic.ip_proto &&
+			fltr_info->ip.v6.src_port == fk->ports.src &&
+			fltr_info->ip.v6.dst_port == fk->ports.dst &&
+			!memcmp(&fltr_info->ip.v6.src_ip,
+				&fk->addrs.v6addrs.src,
+				sizeof(struct in6_addr)) &&
+			!memcmp(&fltr_info->ip.v6.dst_ip,
+				&fk->addrs.v6addrs.dst,
+				sizeof(struct in6_addr)));
+
+	return false;
+}
+
 /**
  * ice_arfs_is_perfect_flow_set - Check to see if perfect flow is set
  * @hw: pointer to HW structure
@@ -436,17 +474,17 @@  ice_rx_flow_steer(struct net_device *netdev, const struct sk_buff *skb,
 
 	/* choose the aRFS list bucket based on skb hash */
 	idx = skb_get_hash_raw(skb) & ICE_ARFS_LST_MASK;
+
 	/* search for entry in the bucket */
 	spin_lock_bh(&vsi->arfs_lock);
 	hlist_for_each_entry(arfs_entry, &vsi->arfs_fltr_list[idx],
 			     list_entry) {
-		struct ice_fdir_fltr *fltr_info;
+		struct ice_fdir_fltr *fltr_info = &arfs_entry->fltr_info;
 
 		/* keep searching for the already existing arfs_entry flow */
-		if (arfs_entry->flow_id != flow_id)
+		if (!ice_arfs_cmp(fltr_info, &fk))
 			continue;
 
-		fltr_info = &arfs_entry->fltr_info;
 		ret = fltr_info->fltr_id;
 
 		if (fltr_info->q_index == rxq_idx ||