diff mbox series

[net-next] sfc: keep alive neighbour entries while a TC encap action is using them

Message ID 20230621121504.17004-1-edward.cree@amd.com (mailing list archive)
State Accepted
Commit 9a14f2e3dab106df7f27d1730cc540247317d4b9
Delegated to: Netdev Maintainers
Headers show
Series [net-next] sfc: keep alive neighbour entries while a TC encap action is using them | expand

Checks

Context Check Description
netdev/series_format success Single patches do not need cover letters
netdev/tree_selection success Clearly marked for net-next
netdev/fixes_present success Fixes tag not required for -next series
netdev/header_inline success No static functions without inline keyword in header files
netdev/build_32bit success Errors and warnings before: 9 this patch: 9
netdev/cc_maintainers success CCed 8 of 8 maintainers
netdev/build_clang success Errors and warnings before: 8 this patch: 8
netdev/verify_signedoff success Signed-off-by tag matches author and committer
netdev/deprecated_api success None detected
netdev/check_selftest success No net selftest shell script
netdev/verify_fixes success No Fixes tag
netdev/build_allmodconfig_warn success Errors and warnings before: 9 this patch: 9
netdev/checkpatch warning WARNING: Where possible, use lockdep_assert_held instead of assertions based on spin_is_locked WARNING: line length of 81 exceeds 80 columns WARNING: line length of 85 exceeds 80 columns
netdev/kdoc success Errors and warnings before: 0 this patch: 0
netdev/source_inline success Was 0 now: 0

Commit Message

edward.cree@amd.com June 21, 2023, 12:15 p.m. UTC
From: Edward Cree <ecree.xilinx@gmail.com>

When processing counter updates, if any action set using the newly
 incremented counter includes an encap action, prod the corresponding
 neighbouring entry to indicate to the neighbour cache that the entry
 is still in use and passing traffic.

Signed-off-by: Edward Cree <ecree.xilinx@gmail.com>
---
 drivers/net/ethernet/sfc/tc.c          | 20 ++++++++-
 drivers/net/ethernet/sfc/tc.h          |  1 +
 drivers/net/ethernet/sfc/tc_counters.c | 58 ++++++++++++++++++++++++++
 drivers/net/ethernet/sfc/tc_counters.h |  3 ++
 4 files changed, 81 insertions(+), 1 deletion(-)

Comments

patchwork-bot+netdevbpf@kernel.org June 23, 2023, 3 a.m. UTC | #1
Hello:

This patch was applied to netdev/net-next.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Wed, 21 Jun 2023 13:15:04 +0100 you wrote:
> From: Edward Cree <ecree.xilinx@gmail.com>
> 
> When processing counter updates, if any action set using the newly
>  incremented counter includes an encap action, prod the corresponding
>  neighbouring entry to indicate to the neighbour cache that the entry
>  is still in use and passing traffic.
> 
> [...]

Here is the summary with links:
  - [net-next] sfc: keep alive neighbour entries while a TC encap action is using them
    https://git.kernel.org/netdev/net-next/c/9a14f2e3dab1

You are awesome, thank you!
diff mbox series

Patch

diff --git a/drivers/net/ethernet/sfc/tc.c b/drivers/net/ethernet/sfc/tc.c
index 77acdb60381e..15ebd3973922 100644
--- a/drivers/net/ethernet/sfc/tc.c
+++ b/drivers/net/ethernet/sfc/tc.c
@@ -110,8 +110,13 @@  static void efx_tc_free_action_set(struct efx_nic *efx,
 		 */
 		list_del(&act->list);
 	}
-	if (act->count)
+	if (act->count) {
+		spin_lock_bh(&act->count->cnt->lock);
+		if (!list_empty(&act->count_user))
+			list_del(&act->count_user);
+		spin_unlock_bh(&act->count->cnt->lock);
 		efx_tc_flower_put_counter_index(efx, act->count);
+	}
 	if (act->encap_md) {
 		list_del(&act->encap_user);
 		efx_tc_flower_release_encap_md(efx, act->encap_md);
@@ -796,6 +801,7 @@  static int efx_tc_flower_replace_foreign(struct efx_nic *efx,
 					goto release;
 				}
 				act->count = ctr;
+				INIT_LIST_HEAD(&act->count_user);
 			}
 
 			if (!efx_tc_flower_action_order_ok(act, EFX_TC_AO_DELIVER)) {
@@ -1083,6 +1089,7 @@  static int efx_tc_flower_replace(struct efx_nic *efx,
 				goto release;
 			}
 			act->count = ctr;
+			INIT_LIST_HEAD(&act->count_user);
 		}
 
 		switch (fa->id) {
@@ -1120,6 +1127,17 @@  static int efx_tc_flower_replace(struct efx_nic *efx,
 				list_add_tail(&act->encap_user, &encap->users);
 				act->dest_mport = encap->dest_mport;
 				act->deliver = 1;
+				if (act->count && !WARN_ON(!act->count->cnt)) {
+					/* This counter is used by an encap
+					 * action, which needs a reference back
+					 * so it can prod neighbouring whenever
+					 * traffic is seen.
+					 */
+					spin_lock_bh(&act->count->cnt->lock);
+					list_add_tail(&act->count_user,
+						      &act->count->cnt->users);
+					spin_unlock_bh(&act->count->cnt->lock);
+				}
 				rc = efx_mae_alloc_action_set(efx, act);
 				if (rc) {
 					NL_SET_ERR_MSG_MOD(extack, "Failed to write action set to hw (encap)");
diff --git a/drivers/net/ethernet/sfc/tc.h b/drivers/net/ethernet/sfc/tc.h
index 607429f8bb28..1549c3df43bb 100644
--- a/drivers/net/ethernet/sfc/tc.h
+++ b/drivers/net/ethernet/sfc/tc.h
@@ -38,6 +38,7 @@  struct efx_tc_action_set {
 	struct efx_tc_encap_action *encap_md; /* entry in tc_encap_ht table */
 	struct list_head encap_user; /* entry on encap_md->users list */
 	struct efx_tc_action_set_list *user; /* Only populated if encap_md */
+	struct list_head count_user; /* entry on counter->users list, if encap */
 	u32 dest_mport;
 	u32 fw_id; /* index of this entry in firmware actions table */
 	struct list_head list;
diff --git a/drivers/net/ethernet/sfc/tc_counters.c b/drivers/net/ethernet/sfc/tc_counters.c
index d1a91d54c6bb..979f49058a0c 100644
--- a/drivers/net/ethernet/sfc/tc_counters.c
+++ b/drivers/net/ethernet/sfc/tc_counters.c
@@ -9,6 +9,7 @@ 
  */
 
 #include "tc_counters.h"
+#include "tc_encap_actions.h"
 #include "mae_counter_format.h"
 #include "mae.h"
 #include "rx_common.h"
@@ -31,6 +32,15 @@  static void efx_tc_counter_free(void *ptr, void *__unused)
 {
 	struct efx_tc_counter *cnt = ptr;
 
+	WARN_ON(!list_empty(&cnt->users));
+	/* We'd like to synchronize_rcu() here, but unfortunately we aren't
+	 * removing the element from the hashtable (it's not clear that's a
+	 * safe thing to do in an rhashtable_free_and_destroy free_fn), so
+	 * threads could still be obtaining new pointers to *cnt if they can
+	 * race against this function at all.
+	 */
+	flush_work(&cnt->work);
+	EFX_WARN_ON_PARANOID(spin_is_locked(&cnt->lock));
 	kfree(cnt);
 }
 
@@ -74,6 +84,49 @@  void efx_tc_fini_counters(struct efx_nic *efx)
 	rhashtable_free_and_destroy(&efx->tc->counter_ht, efx_tc_counter_free, NULL);
 }
 
+static void efx_tc_counter_work(struct work_struct *work)
+{
+	struct efx_tc_counter *cnt = container_of(work, struct efx_tc_counter, work);
+	struct efx_tc_encap_action *encap;
+	struct efx_tc_action_set *act;
+	unsigned long touched;
+	struct neighbour *n;
+
+	spin_lock_bh(&cnt->lock);
+	touched = READ_ONCE(cnt->touched);
+
+	list_for_each_entry(act, &cnt->users, count_user) {
+		encap = act->encap_md;
+		if (!encap)
+			continue;
+		if (!encap->neigh) /* can't happen */
+			continue;
+		if (time_after_eq(encap->neigh->used, touched))
+			continue;
+		encap->neigh->used = touched;
+		/* We have passed traffic using this ARP entry, so
+		 * indicate to the ARP cache that it's still active
+		 */
+		if (encap->neigh->dst_ip)
+			n = neigh_lookup(&arp_tbl, &encap->neigh->dst_ip,
+					 encap->neigh->egdev);
+		else
+#if IS_ENABLED(CONFIG_IPV6)
+			n = neigh_lookup(ipv6_stub->nd_tbl,
+					 &encap->neigh->dst_ip6,
+					 encap->neigh->egdev);
+#else
+			n = NULL;
+#endif
+		if (!n)
+			continue;
+
+		neigh_event_send(n, NULL);
+		neigh_release(n);
+	}
+	spin_unlock_bh(&cnt->lock);
+}
+
 /* Counter allocation */
 
 static struct efx_tc_counter *efx_tc_flower_allocate_counter(struct efx_nic *efx,
@@ -87,12 +140,14 @@  static struct efx_tc_counter *efx_tc_flower_allocate_counter(struct efx_nic *efx
 		return ERR_PTR(-ENOMEM);
 
 	spin_lock_init(&cnt->lock);
+	INIT_WORK(&cnt->work, efx_tc_counter_work);
 	cnt->touched = jiffies;
 	cnt->type = type;
 
 	rc = efx_mae_allocate_counter(efx, cnt);
 	if (rc)
 		goto fail1;
+	INIT_LIST_HEAD(&cnt->users);
 	rc = rhashtable_insert_fast(&efx->tc->counter_ht, &cnt->linkage,
 				    efx_tc_counter_ht_params);
 	if (rc)
@@ -126,6 +181,7 @@  static void efx_tc_flower_release_counter(struct efx_nic *efx,
 		netif_warn(efx, hw, efx->net_dev,
 			   "Failed to free MAE counter %u, rc %d\n",
 			   cnt->fw_id, rc);
+	WARN_ON(!list_empty(&cnt->users));
 	/* This doesn't protect counter updates coming in arbitrarily long
 	 * after we deleted the counter.  The RCU just ensures that we won't
 	 * free the counter while another thread has a pointer to it.
@@ -133,6 +189,7 @@  static void efx_tc_flower_release_counter(struct efx_nic *efx,
 	 * is handled by the generation count.
 	 */
 	synchronize_rcu();
+	flush_work(&cnt->work);
 	EFX_WARN_ON_PARANOID(spin_is_locked(&cnt->lock));
 	kfree(cnt);
 }
@@ -302,6 +359,7 @@  static void efx_tc_counter_update(struct efx_nic *efx,
 		cnt->touched = jiffies;
 	}
 	spin_unlock_bh(&cnt->lock);
+	schedule_work(&cnt->work);
 out:
 	rcu_read_unlock();
 }
diff --git a/drivers/net/ethernet/sfc/tc_counters.h b/drivers/net/ethernet/sfc/tc_counters.h
index 8fc7c4bbb29c..41e57f34b763 100644
--- a/drivers/net/ethernet/sfc/tc_counters.h
+++ b/drivers/net/ethernet/sfc/tc_counters.h
@@ -32,6 +32,9 @@  struct efx_tc_counter {
 	u64 old_packets, old_bytes; /* Values last time passed to userspace */
 	/* jiffies of the last time we saw packets increase */
 	unsigned long touched;
+	struct work_struct work; /* For notifying encap actions */
+	/* owners of corresponding count actions */
+	struct list_head users;
 };
 
 struct efx_tc_counter_index {