netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net-next 0/2] Improve neigh_flush_dev performance
@ 2024-10-01  5:09 Gilad Naaman
  2024-10-01  5:09 ` [PATCH net-next 1/2] Convert neighbour-table to use hlist Gilad Naaman
  2024-10-01  5:09 ` [PATCH net-next 2/2] Create netdev->neighbour association Gilad Naaman
  0 siblings, 2 replies; 7+ messages in thread
From: Gilad Naaman @ 2024-10-01  5:09 UTC (permalink / raw)
  To: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Gilad Naaman

This patchsets improves the performance of neigh_flush_dev.

Currently, the only way to implement it requires traversing
all neighbours known to the kernel, across all network-namespaces.

This means that some flows are slowed down as a function of neighbour-scale,
even if the specific link they're handling has little to no neighbours.

In order to solve this, this patchset adds a netdev->neighbours list,
as well as making the original linked-list doubly-, so that it is
possible to unlink neighbours without traversing the hash-bucket to
obtain the previous neighbour.

The original use-case we encountered was mass-deletion of links (12K
VLANs) while there are 50K ARPs and 50K NDPs in the system; though the
slowdowns would also appear when the links are set down.


Gilad Naaman (2):
  Convert neighbour-table to use hlist
  Create netdev->neighbour association

 .../networking/net_cachelines/net_device.rst  |   1 +
 include/linux/netdevice.h                     |   3 +
 include/net/neighbour.h                       |  18 +-
 include/net/neighbour_tables.h                |  13 ++
 net/core/neighbour.c                          | 221 +++++++++---------
 5 files changed, 138 insertions(+), 118 deletions(-)
 create mode 100644 include/net/neighbour_tables.h

-- 
2.46.0


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH net-next 1/2] Convert neighbour-table to use hlist
  2024-10-01  5:09 [PATCH net-next 0/2] Improve neigh_flush_dev performance Gilad Naaman
@ 2024-10-01  5:09 ` Gilad Naaman
  2024-10-03 11:23   ` Simon Horman
  2024-10-04  1:41   ` kernel test robot
  2024-10-01  5:09 ` [PATCH net-next 2/2] Create netdev->neighbour association Gilad Naaman
  1 sibling, 2 replies; 7+ messages in thread
From: Gilad Naaman @ 2024-10-01  5:09 UTC (permalink / raw)
  To: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Gilad Naaman

Use doubly-linked instead of singly-linked list when linking neighbours,
so that it is possible to remove neighbours without traversing the
entire table.

Signed-off-by: Gilad Naaman <gnaaman@drivenets.com>
---
 include/net/neighbour.h |   8 +--
 net/core/neighbour.c    | 123 ++++++++++++++--------------------------
 2 files changed, 45 insertions(+), 86 deletions(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index a44f262a7384..77a4aa53aecb 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -135,7 +135,7 @@ struct neigh_statistics {
 #define NEIGH_CACHE_STAT_INC(tbl, field) this_cpu_inc((tbl)->stats->field)
 
 struct neighbour {
-	struct neighbour __rcu	*next;
+	struct hlist_node __rcu list;
 	struct neigh_table	*tbl;
 	struct neigh_parms	*parms;
 	unsigned long		confirmed;
@@ -190,7 +190,7 @@ struct pneigh_entry {
 #define NEIGH_NUM_HASH_RND	4
 
 struct neigh_hash_table {
-	struct neighbour __rcu	**hash_buckets;
+	struct hlist_head __rcu	*hash_buckets;
 	unsigned int		hash_shift;
 	__u32			hash_rnd[NEIGH_NUM_HASH_RND];
 	struct rcu_head		rcu;
@@ -304,9 +304,9 @@ static inline struct neighbour *___neigh_lookup_noref(
 	u32 hash_val;
 
 	hash_val = hash(pkey, dev, nht->hash_rnd) >> (32 - nht->hash_shift);
-	for (n = rcu_dereference(nht->hash_buckets[hash_val]);
+	for (n = (struct neighbour *)rcu_dereference(nht->hash_buckets[hash_val].first);
 	     n != NULL;
-	     n = rcu_dereference(n->next)) {
+	     n = (struct neighbour *)rcu_dereference(n->list.next)) {
 		if (n->dev == dev && key_eq(n, pkey))
 			return n;
 	}
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 77b819cd995b..5b48ed1fdcf0 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -37,6 +37,7 @@
 #include <linux/string.h>
 #include <linux/log2.h>
 #include <linux/inetdevice.h>
+#include <linux/rculist.h>
 #include <net/addrconf.h>
 
 #include <trace/events/neigh.h>
@@ -205,18 +206,13 @@ static void neigh_update_flags(struct neighbour *neigh, u32 flags, int *notify,
 	}
 }
 
-static bool neigh_del(struct neighbour *n, struct neighbour __rcu **np,
-		      struct neigh_table *tbl)
+static bool neigh_del(struct neighbour *n, struct neigh_table *tbl)
 {
 	bool retval = false;
 
 	write_lock(&n->lock);
 	if (refcount_read(&n->refcnt) == 1) {
-		struct neighbour *neigh;
-
-		neigh = rcu_dereference_protected(n->next,
-						  lockdep_is_held(&tbl->lock));
-		rcu_assign_pointer(*np, neigh);
+		hlist_del_rcu(&n->list);
 		neigh_mark_dead(n);
 		retval = true;
 	}
@@ -228,25 +224,7 @@ static bool neigh_del(struct neighbour *n, struct neighbour __rcu **np,
 
 bool neigh_remove_one(struct neighbour *ndel, struct neigh_table *tbl)
 {
-	struct neigh_hash_table *nht;
-	void *pkey = ndel->primary_key;
-	u32 hash_val;
-	struct neighbour *n;
-	struct neighbour __rcu **np;
-
-	nht = rcu_dereference_protected(tbl->nht,
-					lockdep_is_held(&tbl->lock));
-	hash_val = tbl->hash(pkey, ndel->dev, nht->hash_rnd);
-	hash_val = hash_val >> (32 - nht->hash_shift);
-
-	np = &nht->hash_buckets[hash_val];
-	while ((n = rcu_dereference_protected(*np,
-					      lockdep_is_held(&tbl->lock)))) {
-		if (n == ndel)
-			return neigh_del(n, np, tbl);
-		np = &n->next;
-	}
-	return false;
+	return neigh_del(ndel, tbl);
 }
 
 static int neigh_forced_gc(struct neigh_table *tbl)
@@ -388,21 +366,20 @@ static void neigh_flush_dev(struct neigh_table *tbl, struct net_device *dev,
 
 	for (i = 0; i < (1 << nht->hash_shift); i++) {
 		struct neighbour *n;
-		struct neighbour __rcu **np = &nht->hash_buckets[i];
+		struct neighbour __rcu **np =
+			(struct neighbour __rcu **)&nht->hash_buckets[i].first;
 
 		while ((n = rcu_dereference_protected(*np,
 					lockdep_is_held(&tbl->lock))) != NULL) {
 			if (dev && n->dev != dev) {
-				np = &n->next;
+				np = (struct neighbour __rcu **)&n->list.next;
 				continue;
 			}
 			if (skip_perm && n->nud_state & NUD_PERMANENT) {
-				np = &n->next;
+				np = (struct neighbour __rcu **)&n->list.next;
 				continue;
 			}
-			rcu_assign_pointer(*np,
-				   rcu_dereference_protected(n->next,
-						lockdep_is_held(&tbl->lock)));
+			hlist_del_rcu(&n->list);
 			write_lock(&n->lock);
 			neigh_del_timer(n);
 			neigh_mark_dead(n);
@@ -530,9 +507,9 @@ static void neigh_get_hash_rnd(u32 *x)
 
 static struct neigh_hash_table *neigh_hash_alloc(unsigned int shift)
 {
-	size_t size = (1 << shift) * sizeof(struct neighbour *);
+	size_t size = (1 << shift) * sizeof(struct hlist_head);
 	struct neigh_hash_table *ret;
-	struct neighbour __rcu **buckets;
+	struct hlist_head __rcu *buckets;
 	int i;
 
 	ret = kmalloc(sizeof(*ret), GFP_ATOMIC);
@@ -541,7 +518,7 @@ static struct neigh_hash_table *neigh_hash_alloc(unsigned int shift)
 	if (size <= PAGE_SIZE) {
 		buckets = kzalloc(size, GFP_ATOMIC);
 	} else {
-		buckets = (struct neighbour __rcu **)
+		buckets = (struct hlist_head __rcu *)
 			  __get_free_pages(GFP_ATOMIC | __GFP_ZERO,
 					   get_order(size));
 		kmemleak_alloc(buckets, size, 1, GFP_ATOMIC);
@@ -562,8 +539,8 @@ static void neigh_hash_free_rcu(struct rcu_head *head)
 	struct neigh_hash_table *nht = container_of(head,
 						    struct neigh_hash_table,
 						    rcu);
-	size_t size = (1 << nht->hash_shift) * sizeof(struct neighbour *);
-	struct neighbour __rcu **buckets = nht->hash_buckets;
+	size_t size = (1 << nht->hash_shift) * sizeof(struct hlist_head);
+	struct hlist_head __rcu *buckets = nht->hash_buckets;
 
 	if (size <= PAGE_SIZE) {
 		kfree(buckets);
@@ -591,22 +568,18 @@ static struct neigh_hash_table *neigh_hash_grow(struct neigh_table *tbl,
 	for (i = 0; i < (1 << old_nht->hash_shift); i++) {
 		struct neighbour *n, *next;
 
-		for (n = rcu_dereference_protected(old_nht->hash_buckets[i],
-						   lockdep_is_held(&tbl->lock));
+		for (n = (struct neighbour *)
+			rcu_dereference_protected(old_nht->hash_buckets[i].first,
+						  lockdep_is_held(&tbl->lock));
 		     n != NULL;
 		     n = next) {
 			hash = tbl->hash(n->primary_key, n->dev,
 					 new_nht->hash_rnd);
 
 			hash >>= (32 - new_nht->hash_shift);
-			next = rcu_dereference_protected(n->next,
-						lockdep_is_held(&tbl->lock));
-
-			rcu_assign_pointer(n->next,
-					   rcu_dereference_protected(
-						new_nht->hash_buckets[hash],
-						lockdep_is_held(&tbl->lock)));
-			rcu_assign_pointer(new_nht->hash_buckets[hash], n);
+			next = (struct neighbour *)hlist_next_rcu(&n->list);
+			hlist_del_rcu(&n->list);
+			hlist_add_head_rcu(&n->list, &new_nht->hash_buckets[hash]);
 		}
 	}
 
@@ -693,11 +666,10 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
 		goto out_tbl_unlock;
 	}
 
-	for (n1 = rcu_dereference_protected(nht->hash_buckets[hash_val],
-					    lockdep_is_held(&tbl->lock));
-	     n1 != NULL;
-	     n1 = rcu_dereference_protected(n1->next,
-			lockdep_is_held(&tbl->lock))) {
+	hlist_for_each_entry_rcu(n1,
+				 &nht->hash_buckets[hash_val],
+				 list,
+				 lockdep_is_held(&tbl->lock)) {
 		if (dev == n1->dev && !memcmp(n1->primary_key, n->primary_key, key_len)) {
 			if (want_ref)
 				neigh_hold(n1);
@@ -713,10 +685,7 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
 		list_add_tail(&n->managed_list, &n->tbl->managed_list);
 	if (want_ref)
 		neigh_hold(n);
-	rcu_assign_pointer(n->next,
-			   rcu_dereference_protected(nht->hash_buckets[hash_val],
-						     lockdep_is_held(&tbl->lock)));
-	rcu_assign_pointer(nht->hash_buckets[hash_val], n);
+	hlist_add_head_rcu(&n->list, &nht->hash_buckets[hash_val]);
 	write_unlock_bh(&tbl->lock);
 	neigh_dbg(2, "neigh %p is created\n", n);
 	rc = n;
@@ -976,7 +945,7 @@ static void neigh_periodic_work(struct work_struct *work)
 		goto out;
 
 	for (i = 0 ; i < (1 << nht->hash_shift); i++) {
-		np = &nht->hash_buckets[i];
+		np = (struct neighbour __rcu **)&nht->hash_buckets[i].first;
 
 		while ((n = rcu_dereference_protected(*np,
 				lockdep_is_held(&tbl->lock))) != NULL) {
@@ -999,9 +968,7 @@ static void neigh_periodic_work(struct work_struct *work)
 			    (state == NUD_FAILED ||
 			     !time_in_range_open(jiffies, n->used,
 						 n->used + NEIGH_VAR(n->parms, GC_STALETIME)))) {
-				rcu_assign_pointer(*np,
-					rcu_dereference_protected(n->next,
-						lockdep_is_held(&tbl->lock)));
+				hlist_del_rcu(&n->list);
 				neigh_mark_dead(n);
 				write_unlock(&n->lock);
 				neigh_cleanup_and_release(n);
@@ -1010,7 +977,7 @@ static void neigh_periodic_work(struct work_struct *work)
 			write_unlock(&n->lock);
 
 next_elt:
-			np = &n->next;
+			np = (struct neighbour __rcu **)&n->list.next;
 		}
 		/*
 		 * It's fine to release lock here, even if hash table
@@ -2728,9 +2695,7 @@ static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
 	for (h = s_h; h < (1 << nht->hash_shift); h++) {
 		if (h > s_h)
 			s_idx = 0;
-		for (n = rcu_dereference(nht->hash_buckets[h]), idx = 0;
-		     n != NULL;
-		     n = rcu_dereference(n->next)) {
+		hlist_for_each_entry_rcu(n, &nht->hash_buckets[h], list) {
 			if (idx < s_idx || !net_eq(dev_net(n->dev), net))
 				goto next;
 			if (neigh_ifindex_filtered(n->dev, filter->dev_idx) ||
@@ -3097,9 +3062,7 @@ void neigh_for_each(struct neigh_table *tbl, void (*cb)(struct neighbour *, void
 	for (chain = 0; chain < (1 << nht->hash_shift); chain++) {
 		struct neighbour *n;
 
-		for (n = rcu_dereference(nht->hash_buckets[chain]);
-		     n != NULL;
-		     n = rcu_dereference(n->next))
+		hlist_for_each_entry_rcu(n, &nht->hash_buckets[chain], list)
 			cb(n, cookie);
 	}
 	read_unlock_bh(&tbl->lock);
@@ -3120,7 +3083,7 @@ void __neigh_for_each_release(struct neigh_table *tbl,
 		struct neighbour *n;
 		struct neighbour __rcu **np;
 
-		np = &nht->hash_buckets[chain];
+		np = (struct neighbour __rcu **)&nht->hash_buckets[chain].first;
 		while ((n = rcu_dereference_protected(*np,
 					lockdep_is_held(&tbl->lock))) != NULL) {
 			int release;
@@ -3128,12 +3091,10 @@ void __neigh_for_each_release(struct neigh_table *tbl,
 			write_lock(&n->lock);
 			release = cb(n);
 			if (release) {
-				rcu_assign_pointer(*np,
-					rcu_dereference_protected(n->next,
-						lockdep_is_held(&tbl->lock)));
+				hlist_del_rcu(&n->list);
 				neigh_mark_dead(n);
 			} else
-				np = &n->next;
+				np = (struct neighbour __rcu **)&n->list.next;
 			write_unlock(&n->lock);
 			if (release)
 				neigh_cleanup_and_release(n);
@@ -3200,25 +3161,21 @@ static struct neighbour *neigh_get_first(struct seq_file *seq)
 
 	state->flags &= ~NEIGH_SEQ_IS_PNEIGH;
 	for (bucket = 0; bucket < (1 << nht->hash_shift); bucket++) {
-		n = rcu_dereference(nht->hash_buckets[bucket]);
-
-		while (n) {
+		hlist_for_each_entry_rcu(n, &nht->hash_buckets[bucket], list) {
 			if (!net_eq(dev_net(n->dev), net))
-				goto next;
+				continue;
 			if (state->neigh_sub_iter) {
 				loff_t fakep = 0;
 				void *v;
 
 				v = state->neigh_sub_iter(state, n, &fakep);
 				if (!v)
-					goto next;
+					continue;
 			}
 			if (!(state->flags & NEIGH_SEQ_SKIP_NOARP))
 				break;
 			if (READ_ONCE(n->nud_state) & ~NUD_NOARP)
 				break;
-next:
-			n = rcu_dereference(n->next);
 		}
 
 		if (n)
@@ -3242,7 +3199,8 @@ static struct neighbour *neigh_get_next(struct seq_file *seq,
 		if (v)
 			return n;
 	}
-	n = rcu_dereference(n->next);
+
+	n = (struct neighbour *)hlist_next_rcu(&n->list);
 
 	while (1) {
 		while (n) {
@@ -3260,7 +3218,8 @@ static struct neighbour *neigh_get_next(struct seq_file *seq,
 			if (READ_ONCE(n->nud_state) & ~NUD_NOARP)
 				break;
 next:
-			n = rcu_dereference(n->next);
+
+			n = (struct neighbour *)hlist_next_rcu(&n->list);
 		}
 
 		if (n)
@@ -3269,7 +3228,7 @@ static struct neighbour *neigh_get_next(struct seq_file *seq,
 		if (++state->bucket >= (1 << nht->hash_shift))
 			break;
 
-		n = rcu_dereference(nht->hash_buckets[state->bucket]);
+		n = (struct neighbour *)hlist_first_rcu(&nht->hash_buckets[state->bucket]);
 	}
 
 	if (n && pos)
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH net-next 2/2] Create netdev->neighbour association
  2024-10-01  5:09 [PATCH net-next 0/2] Improve neigh_flush_dev performance Gilad Naaman
  2024-10-01  5:09 ` [PATCH net-next 1/2] Convert neighbour-table to use hlist Gilad Naaman
@ 2024-10-01  5:09 ` Gilad Naaman
  2024-10-03 11:26   ` Simon Horman
  1 sibling, 1 reply; 7+ messages in thread
From: Gilad Naaman @ 2024-10-01  5:09 UTC (permalink / raw)
  To: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni
  Cc: Gilad Naaman

Create a mapping between a netdev and its neighoburs,
allowing for much cheaper flushes.

Signed-off-by: Gilad Naaman <gnaaman@drivenets.com>
---
 .../networking/net_cachelines/net_device.rst  |   1 +
 include/linux/netdevice.h                     |   3 +
 include/net/neighbour.h                       |  10 +-
 include/net/neighbour_tables.h                |  13 +++
 net/core/neighbour.c                          | 100 +++++++++++++-----
 5 files changed, 94 insertions(+), 33 deletions(-)
 create mode 100644 include/net/neighbour_tables.h

diff --git a/Documentation/networking/net_cachelines/net_device.rst b/Documentation/networking/net_cachelines/net_device.rst
index 22b07c814f4a..510c407d7268 100644
--- a/Documentation/networking/net_cachelines/net_device.rst
+++ b/Documentation/networking/net_cachelines/net_device.rst
@@ -183,3 +183,4 @@ struct_devlink_port*                devlink_port
 struct_dpll_pin*                    dpll_pin                                                        
 struct hlist_head                   page_pools
 struct dim_irq_moder*               irq_moder
+struct hlist_head                   neighbours[3]
diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index e87b5e488325..7b24a792280c 100644
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -52,6 +52,7 @@
 #include <net/net_trackers.h>
 #include <net/net_debug.h>
 #include <net/dropreason-core.h>
+#include <net/neighbour_tables.h>
 
 struct netpoll_info;
 struct device;
@@ -2399,6 +2400,8 @@ struct net_device {
 	/** @irq_moder: dim parameters used if IS_ENABLED(CONFIG_DIMLIB). */
 	struct dim_irq_moder	*irq_moder;
 
+	struct hlist_head neighbours[NEIGH_NR_TABLES];
+
 	u8			priv[] ____cacheline_aligned
 				       __counted_by(priv_len);
 } ____cacheline_aligned;
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 77a4aa53aecb..580c2d00e4d5 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -29,6 +29,7 @@
 #include <linux/sysctl.h>
 #include <linux/workqueue.h>
 #include <net/rtnetlink.h>
+#include <net/neighbour_tables.h>
 
 /*
  * NUD stands for "neighbor unreachability detection"
@@ -136,6 +137,7 @@ struct neigh_statistics {
 
 struct neighbour {
 	struct hlist_node __rcu list;
+	struct hlist_node __rcu dev_list;
 	struct neigh_table	*tbl;
 	struct neigh_parms	*parms;
 	unsigned long		confirmed;
@@ -236,14 +238,6 @@ struct neigh_table {
 	struct pneigh_entry	**phash_buckets;
 };
 
-enum {
-	NEIGH_ARP_TABLE = 0,
-	NEIGH_ND_TABLE = 1,
-	NEIGH_DN_TABLE = 2,
-	NEIGH_NR_TABLES,
-	NEIGH_LINK_TABLE = NEIGH_NR_TABLES /* Pseudo table for neigh_xmit */
-};
-
 static inline int neigh_parms_family(struct neigh_parms *p)
 {
 	return p->tbl->family;
diff --git a/include/net/neighbour_tables.h b/include/net/neighbour_tables.h
new file mode 100644
index 000000000000..ad98b49d58db
--- /dev/null
+++ b/include/net/neighbour_tables.h
@@ -0,0 +1,13 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _NET_NEIGHBOUR_TABLES_H
+#define _NET_NEIGHBOUR_TABLES_H
+
+enum {
+	NEIGH_ARP_TABLE = 0,
+	NEIGH_ND_TABLE = 1,
+	NEIGH_DN_TABLE = 2,
+	NEIGH_NR_TABLES,
+	NEIGH_LINK_TABLE = NEIGH_NR_TABLES /* Pseudo table for neigh_xmit */
+};
+
+#endif
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 5b48ed1fdcf0..f3a9a220b343 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -62,6 +62,20 @@ static int pneigh_ifdown_and_unlock(struct neigh_table *tbl,
 static const struct seq_operations neigh_stat_seq_ops;
 #endif
 
+static int family_to_neightbl_index(int family)
+{
+	switch (family) {
+	case AF_INET:
+		return NEIGH_ARP_TABLE;
+	case AF_INET6:
+		return NEIGH_ND_TABLE;
+	case AF_DECnet:
+		return NEIGH_DN_TABLE;
+	default:
+		return -1;
+	}
+}
+
 /*
    Neighbour hash table buckets are protected with rwlock tbl->lock.
 
@@ -213,6 +227,7 @@ static bool neigh_del(struct neighbour *n, struct neigh_table *tbl)
 	write_lock(&n->lock);
 	if (refcount_read(&n->refcnt) == 1) {
 		hlist_del_rcu(&n->list);
+		hlist_del_rcu(&n->dev_list);
 		neigh_mark_dead(n);
 		retval = true;
 	}
@@ -355,12 +370,63 @@ static void pneigh_queue_purge(struct sk_buff_head *list, struct net *net,
 	}
 }
 
+static void _neigh_flush_free_neigh(struct neighbour *n)
+{
+	hlist_del_rcu(&n->list);
+	hlist_del_rcu(&n->dev_list);
+	write_lock(&n->lock);
+	neigh_del_timer(n);
+	neigh_mark_dead(n);
+	if (refcount_read(&n->refcnt) != 1) {
+		/* The most unpleasant situation.
+		 * We must destroy neighbour entry,
+		 * but someone still uses it.
+		 *
+		 * The destroy will be delayed until
+		 * the last user releases us, but
+		 * we must kill timers etc. and move
+		 * it to safe state.
+		 */
+		__skb_queue_purge(&n->arp_queue);
+		n->arp_queue_len_bytes = 0;
+		WRITE_ONCE(n->output, neigh_blackhole);
+		if (n->nud_state & NUD_VALID)
+			n->nud_state = NUD_NOARP;
+		else
+			n->nud_state = NUD_NONE;
+		neigh_dbg(2, "neigh %p is stray\n", n);
+	}
+	write_unlock(&n->lock);
+	neigh_cleanup_and_release(n);
+}
+
+static void neigh_flush_dev_fast(struct neigh_table *tbl, struct hlist_node __rcu *next,
+				 bool skip_perm)
+{
+	struct neighbour *n;
+
+	while (next) {
+		n = container_of(next, struct neighbour, dev_list);
+		next = hlist_next_rcu(next);
+		if (skip_perm && n->nud_state & NUD_PERMANENT)
+			continue;
+
+		_neigh_flush_free_neigh(n);
+	}
+}
+
 static void neigh_flush_dev(struct neigh_table *tbl, struct net_device *dev,
 			    bool skip_perm)
 {
 	int i;
 	struct neigh_hash_table *nht;
 
+	i = family_to_neightbl_index(tbl->family);
+	if (i != -1) {
+		neigh_flush_dev_fast(tbl, hlist_first_rcu(&dev->neighbours[i]), skip_perm);
+		return;
+	}
+
 	nht = rcu_dereference_protected(tbl->nht,
 					lockdep_is_held(&tbl->lock));
 
@@ -379,31 +445,8 @@ static void neigh_flush_dev(struct neigh_table *tbl, struct net_device *dev,
 				np = (struct neighbour __rcu **)&n->list.next;
 				continue;
 			}
-			hlist_del_rcu(&n->list);
-			write_lock(&n->lock);
-			neigh_del_timer(n);
-			neigh_mark_dead(n);
-			if (refcount_read(&n->refcnt) != 1) {
-				/* The most unpleasant situation.
-				   We must destroy neighbour entry,
-				   but someone still uses it.
-
-				   The destroy will be delayed until
-				   the last user releases us, but
-				   we must kill timers etc. and move
-				   it to safe state.
-				 */
-				__skb_queue_purge(&n->arp_queue);
-				n->arp_queue_len_bytes = 0;
-				WRITE_ONCE(n->output, neigh_blackhole);
-				if (n->nud_state & NUD_VALID)
-					n->nud_state = NUD_NOARP;
-				else
-					n->nud_state = NUD_NONE;
-				neigh_dbg(2, "neigh %p is stray\n", n);
-			}
-			write_unlock(&n->lock);
-			neigh_cleanup_and_release(n);
+
+			_neigh_flush_free_neigh(n);
 		}
 	}
 }
@@ -686,6 +729,11 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
 	if (want_ref)
 		neigh_hold(n);
 	hlist_add_head_rcu(&n->list, &nht->hash_buckets[hash_val]);
+
+	error = family_to_neightbl_index(tbl->family);
+	if (error != -1)
+		hlist_add_head_rcu(&n->dev_list, &dev->neighbours[error]);
+
 	write_unlock_bh(&tbl->lock);
 	neigh_dbg(2, "neigh %p is created\n", n);
 	rc = n;
@@ -969,6 +1017,7 @@ static void neigh_periodic_work(struct work_struct *work)
 			     !time_in_range_open(jiffies, n->used,
 						 n->used + NEIGH_VAR(n->parms, GC_STALETIME)))) {
 				hlist_del_rcu(&n->list);
+				hlist_del_rcu(&n->dev_list);
 				neigh_mark_dead(n);
 				write_unlock(&n->lock);
 				neigh_cleanup_and_release(n);
@@ -3092,6 +3141,7 @@ void __neigh_for_each_release(struct neigh_table *tbl,
 			release = cb(n);
 			if (release) {
 				hlist_del_rcu(&n->list);
+				hlist_del_rcu(&n->dev_list);
 				neigh_mark_dead(n);
 			} else
 				np = (struct neighbour __rcu **)&n->list.next;
-- 
2.46.0


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next 1/2] Convert neighbour-table to use hlist
  2024-10-01  5:09 ` [PATCH net-next 1/2] Convert neighbour-table to use hlist Gilad Naaman
@ 2024-10-03 11:23   ` Simon Horman
  2024-10-04 15:06     ` Gilad Naaman
  2024-10-04  1:41   ` kernel test robot
  1 sibling, 1 reply; 7+ messages in thread
From: Simon Horman @ 2024-10-03 11:23 UTC (permalink / raw)
  To: Gilad Naaman
  Cc: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni

On Tue, Oct 01, 2024 at 05:09:56AM +0000, Gilad Naaman wrote:
> Use doubly-linked instead of singly-linked list when linking neighbours,
> so that it is possible to remove neighbours without traversing the
> entire table.
> 
> Signed-off-by: Gilad Naaman <gnaaman@drivenets.com>

Hi Gilad,

This is not a full review, but rather some feedback to take into account
once a proper review arrives.

> ---
>  include/net/neighbour.h |   8 +--
>  net/core/neighbour.c    | 123 ++++++++++++++--------------------------
>  2 files changed, 45 insertions(+), 86 deletions(-)
> 
> diff --git a/include/net/neighbour.h b/include/net/neighbour.h
> index a44f262a7384..77a4aa53aecb 100644
> --- a/include/net/neighbour.h
> +++ b/include/net/neighbour.h
> @@ -135,7 +135,7 @@ struct neigh_statistics {
>  #define NEIGH_CACHE_STAT_INC(tbl, field) this_cpu_inc((tbl)->stats->field)
>  
>  struct neighbour {
> -	struct neighbour __rcu	*next;
> +	struct hlist_node __rcu list;
>  	struct neigh_table	*tbl;
>  	struct neigh_parms	*parms;
>  	unsigned long		confirmed;

Sparse is having a bit of a cow with rcu changes introduced by this patch.
Please take a look at that.

...

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next 2/2] Create netdev->neighbour association
  2024-10-01  5:09 ` [PATCH net-next 2/2] Create netdev->neighbour association Gilad Naaman
@ 2024-10-03 11:26   ` Simon Horman
  0 siblings, 0 replies; 7+ messages in thread
From: Simon Horman @ 2024-10-03 11:26 UTC (permalink / raw)
  To: Gilad Naaman
  Cc: netdev, David S. Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni

On Tue, Oct 01, 2024 at 05:09:57AM +0000, Gilad Naaman wrote:
> Create a mapping between a netdev and its neighoburs,
> allowing for much cheaper flushes.
> 
> Signed-off-by: Gilad Naaman <gnaaman@drivenets.com>

Hi Gilad,

As per my comment on patch 1/2, This is not a full review, but rather some
feedback to take into account once a proper review arrives.

...

> diff --git a/Documentation/networking/net_cachelines/net_device.rst b/Documentation/networking/net_cachelines/net_device.rst

...

> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h

...

> @@ -2399,6 +2400,8 @@ struct net_device {
>  	/** @irq_moder: dim parameters used if IS_ENABLED(CONFIG_DIMLIB). */
>  	struct dim_irq_moder	*irq_moder;
>  
> +	struct hlist_head neighbours[NEIGH_NR_TABLES];
> +

Please add an entry for neighbours in the Kernel doc for this
structure, which is immediately above it.

This is flagged by ./scripts/kernel-doc -none, and W=1 allmodconfig builds.

>  	u8			priv[] ____cacheline_aligned
>  				       __counted_by(priv_len);
>  } ____cacheline_aligned;

...

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next 1/2] Convert neighbour-table to use hlist
  2024-10-01  5:09 ` [PATCH net-next 1/2] Convert neighbour-table to use hlist Gilad Naaman
  2024-10-03 11:23   ` Simon Horman
@ 2024-10-04  1:41   ` kernel test robot
  1 sibling, 0 replies; 7+ messages in thread
From: kernel test robot @ 2024-10-04  1:41 UTC (permalink / raw)
  To: Gilad Naaman, netdev, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni
  Cc: oe-kbuild-all, Gilad Naaman

Hi Gilad,

kernel test robot noticed the following build warnings:

[auto build test WARNING on net-next/main]

url:    https://github.com/intel-lab-lkp/linux/commits/Gilad-Naaman/Convert-neighbour-table-to-use-hlist/20241001-131234
base:   net-next/main
patch link:    https://lore.kernel.org/r/20241001050959.1799151-2-gnaaman%40drivenets.com
patch subject: [PATCH net-next 1/2] Convert neighbour-table to use hlist
config: x86_64-randconfig-121-20241004 (https://download.01.org/0day-ci/archive/20241004/202410040908.loCFe95v-lkp@intel.com/config)
compiler: clang version 18.1.8 (https://github.com/llvm/llvm-project 3b5b5c1ec4a3095ab096dd780e84d7ab81f3d7ff)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20241004/202410040908.loCFe95v-lkp@intel.com/reproduce)

If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202410040908.loCFe95v-lkp@intel.com/

sparse warnings: (new ones prefixed by >>)
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:215:32: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected struct hlist_node *n @@     got struct hlist_node [noderef] * @@
   net/core/neighbour.c:215:32: sparse:     expected struct hlist_node *n
   net/core/neighbour.c:215:32: sparse:     got struct hlist_node [noderef] *
   net/core/neighbour.c:370:26: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:382:40: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected struct hlist_node *n @@     got struct hlist_node [noderef] * @@
   net/core/neighbour.c:382:40: sparse:     expected struct hlist_node *n
   net/core/neighbour.c:382:40: sparse:     got struct hlist_node [noderef] *
>> net/core/neighbour.c:519:25: sparse: sparse: incorrect type in assignment (different address spaces) @@     expected struct hlist_head [noderef] __rcu *buckets @@     got void *_res @@
   net/core/neighbour.c:519:25: sparse:     expected struct hlist_head [noderef] __rcu *buckets
   net/core/neighbour.c:519:25: sparse:     got void *_res
>> net/core/neighbour.c:524:32: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const *ptr @@     got struct hlist_head [noderef] __rcu *[assigned] buckets @@
   net/core/neighbour.c:524:32: sparse:     expected void const *ptr
   net/core/neighbour.c:524:32: sparse:     got struct hlist_head [noderef] __rcu *[assigned] buckets
>> net/core/neighbour.c:546:23: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const *objp @@     got struct hlist_head [noderef] __rcu *buckets @@
   net/core/neighbour.c:546:23: sparse:     expected void const *objp
   net/core/neighbour.c:546:23: sparse:     got struct hlist_head [noderef] __rcu *buckets
>> net/core/neighbour.c:548:31: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected void const *ptr @@     got struct hlist_head [noderef] __rcu *buckets @@
   net/core/neighbour.c:548:31: sparse:     expected void const *ptr
   net/core/neighbour.c:548:31: sparse:     got struct hlist_head [noderef] __rcu *buckets
>> net/core/neighbour.c:572:25: sparse: sparse: incompatible types in comparison expression (different address spaces):
   net/core/neighbour.c:572:25: sparse:    struct hlist_node [noderef] __rcu *
   net/core/neighbour.c:572:25: sparse:    struct hlist_node *
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
>> net/core/neighbour.c:669:9: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:688:29: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected struct hlist_node *n @@     got struct hlist_node [noderef] * @@
>> net/core/neighbour.c:688:56: sparse: sparse: incorrect type in argument 2 (different address spaces) @@     expected struct hlist_head *h @@     got struct hlist_head [noderef] __rcu * @@
   net/core/neighbour.c:948:23: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:971:48: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected struct hlist_node *n @@     got struct hlist_node [noderef] * @@
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:2698:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3065:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3086:23: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3094:48: sparse: sparse: incorrect type in argument 1 (different modifiers) @@     expected struct hlist_node *n @@     got struct hlist_node [noderef] * @@
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3164:17: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3203:14: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3222:30: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3231:41: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:3231:22: sparse: sparse: cast removes address space '__rcu' of expression
   net/core/neighbour.c:429:9: sparse: sparse: context imbalance in '__neigh_ifdown' - wrong count at exit
>> net/core/neighbour.c:572:25: sparse: sparse: dereference of noderef expression
   net/core/neighbour.c: note: in included file:
>> include/net/neighbour.h:307:38: sparse: sparse: cast removes address space '__rcu' of expression
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *
--
   net/core/filter.c:1423:39: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sock_filter const *filter @@     got struct sock_filter [noderef] __user *filter @@
   net/core/filter.c:1423:39: sparse:     expected struct sock_filter const *filter
   net/core/filter.c:1423:39: sparse:     got struct sock_filter [noderef] __user *filter
   net/core/filter.c:1501:39: sparse: sparse: incorrect type in argument 1 (different address spaces) @@     expected struct sock_filter const *filter @@     got struct sock_filter [noderef] __user *filter @@
   net/core/filter.c:1501:39: sparse:     expected struct sock_filter const *filter
   net/core/filter.c:1501:39: sparse:     got struct sock_filter [noderef] __user *filter
   net/core/filter.c:2340:45: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __be32 [usertype] daddr @@     got unsigned int [usertype] ipv4_nh @@
   net/core/filter.c:2340:45: sparse:     expected restricted __be32 [usertype] daddr
   net/core/filter.c:2340:45: sparse:     got unsigned int [usertype] ipv4_nh
   net/core/filter.c:11048:31: sparse: sparse: symbol 'cg_skb_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:11054:27: sparse: sparse: symbol 'cg_skb_prog_ops' was not declared. Should it be static?
   net/core/filter.c:11098:31: sparse: sparse: symbol 'cg_sock_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:11104:27: sparse: sparse: symbol 'cg_sock_prog_ops' was not declared. Should it be static?
   net/core/filter.c:11107:31: sparse: sparse: symbol 'cg_sock_addr_verifier_ops' was not declared. Should it be static?
   net/core/filter.c:11113:27: sparse: sparse: symbol 'cg_sock_addr_prog_ops' was not declared. Should it be static?
   net/core/filter.c:1943:43: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __wsum [usertype] diff @@     got unsigned long long [usertype] to @@
   net/core/filter.c:1943:43: sparse:     expected restricted __wsum [usertype] diff
   net/core/filter.c:1943:43: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:1946:36: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __be16 [usertype] old @@     got unsigned long long [usertype] from @@
   net/core/filter.c:1946:36: sparse:     expected restricted __be16 [usertype] old
   net/core/filter.c:1946:36: sparse:     got unsigned long long [usertype] from
   net/core/filter.c:1946:42: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected restricted __be16 [usertype] new @@     got unsigned long long [usertype] to @@
   net/core/filter.c:1946:42: sparse:     expected restricted __be16 [usertype] new
   net/core/filter.c:1946:42: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:1949:36: sparse: sparse: incorrect type in argument 2 (different base types) @@     expected restricted __be32 [usertype] from @@     got unsigned long long [usertype] from @@
   net/core/filter.c:1949:36: sparse:     expected restricted __be32 [usertype] from
   net/core/filter.c:1949:36: sparse:     got unsigned long long [usertype] from
   net/core/filter.c:1949:42: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected restricted __be32 [usertype] to @@     got unsigned long long [usertype] to @@
   net/core/filter.c:1949:42: sparse:     expected restricted __be32 [usertype] to
   net/core/filter.c:1949:42: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:1994:59: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected restricted __wsum [usertype] diff @@     got unsigned long long [usertype] to @@
   net/core/filter.c:1994:59: sparse:     expected restricted __wsum [usertype] diff
   net/core/filter.c:1994:59: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:1997:52: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected restricted __be16 [usertype] from @@     got unsigned long long [usertype] from @@
   net/core/filter.c:1997:52: sparse:     expected restricted __be16 [usertype] from
   net/core/filter.c:1997:52: sparse:     got unsigned long long [usertype] from
   net/core/filter.c:1997:58: sparse: sparse: incorrect type in argument 4 (different base types) @@     expected restricted __be16 [usertype] to @@     got unsigned long long [usertype] to @@
   net/core/filter.c:1997:58: sparse:     expected restricted __be16 [usertype] to
   net/core/filter.c:1997:58: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:2000:52: sparse: sparse: incorrect type in argument 3 (different base types) @@     expected restricted __be32 [usertype] from @@     got unsigned long long [usertype] from @@
   net/core/filter.c:2000:52: sparse:     expected restricted __be32 [usertype] from
   net/core/filter.c:2000:52: sparse:     got unsigned long long [usertype] from
   net/core/filter.c:2000:58: sparse: sparse: incorrect type in argument 4 (different base types) @@     expected restricted __be32 [usertype] to @@     got unsigned long long [usertype] to @@
   net/core/filter.c:2000:58: sparse:     expected restricted __be32 [usertype] to
   net/core/filter.c:2000:58: sparse:     got unsigned long long [usertype] to
   net/core/filter.c:2050:16: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned long long @@     got restricted __wsum [assigned] [usertype] ret @@
   net/core/filter.c:2050:16: sparse:     expected unsigned long long
   net/core/filter.c:2050:16: sparse:     got restricted __wsum [assigned] [usertype] ret
   net/core/filter.c:2072:35: sparse: sparse: incorrect type in return expression (different base types) @@     expected unsigned long long @@     got restricted __wsum [usertype] csum @@
   net/core/filter.c:2072:35: sparse:     expected unsigned long long
   net/core/filter.c:2072:35: sparse:     got restricted __wsum [usertype] csum
   net/core/filter.c: note: in included file (through include/net/dst.h, include/net/sock.h, include/linux/sock_diag.h):
>> include/net/neighbour.h:307:38: sparse: sparse: cast removes address space '__rcu' of expression
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *
>> include/net/neighbour.h:307:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:307:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:307:38: sparse:    struct hlist_node *
   include/net/neighbour.h:309:38: sparse: sparse: incompatible types in comparison expression (different address spaces):
   include/net/neighbour.h:309:38: sparse:    struct hlist_node [noderef] __rcu *
   include/net/neighbour.h:309:38: sparse:    struct hlist_node *

vim +/__rcu +669 net/core/neighbour.c

   507	
   508	static struct neigh_hash_table *neigh_hash_alloc(unsigned int shift)
   509	{
   510		size_t size = (1 << shift) * sizeof(struct hlist_head);
   511		struct neigh_hash_table *ret;
   512		struct hlist_head __rcu *buckets;
   513		int i;
   514	
   515		ret = kmalloc(sizeof(*ret), GFP_ATOMIC);
   516		if (!ret)
   517			return NULL;
   518		if (size <= PAGE_SIZE) {
 > 519			buckets = kzalloc(size, GFP_ATOMIC);
   520		} else {
   521			buckets = (struct hlist_head __rcu *)
   522				  __get_free_pages(GFP_ATOMIC | __GFP_ZERO,
   523						   get_order(size));
 > 524			kmemleak_alloc(buckets, size, 1, GFP_ATOMIC);
   525		}
   526		if (!buckets) {
   527			kfree(ret);
   528			return NULL;
   529		}
   530		ret->hash_buckets = buckets;
   531		ret->hash_shift = shift;
   532		for (i = 0; i < NEIGH_NUM_HASH_RND; i++)
   533			neigh_get_hash_rnd(&ret->hash_rnd[i]);
   534		return ret;
   535	}
   536	
   537	static void neigh_hash_free_rcu(struct rcu_head *head)
   538	{
   539		struct neigh_hash_table *nht = container_of(head,
   540							    struct neigh_hash_table,
   541							    rcu);
   542		size_t size = (1 << nht->hash_shift) * sizeof(struct hlist_head);
   543		struct hlist_head __rcu *buckets = nht->hash_buckets;
   544	
   545		if (size <= PAGE_SIZE) {
 > 546			kfree(buckets);
   547		} else {
 > 548			kmemleak_free(buckets);
   549			free_pages((unsigned long)buckets, get_order(size));
   550		}
   551		kfree(nht);
   552	}
   553	
   554	static struct neigh_hash_table *neigh_hash_grow(struct neigh_table *tbl,
   555							unsigned long new_shift)
   556	{
   557		unsigned int i, hash;
   558		struct neigh_hash_table *new_nht, *old_nht;
   559	
   560		NEIGH_CACHE_STAT_INC(tbl, hash_grows);
   561	
   562		old_nht = rcu_dereference_protected(tbl->nht,
   563						    lockdep_is_held(&tbl->lock));
   564		new_nht = neigh_hash_alloc(new_shift);
   565		if (!new_nht)
   566			return old_nht;
   567	
   568		for (i = 0; i < (1 << old_nht->hash_shift); i++) {
   569			struct neighbour *n, *next;
   570	
   571			for (n = (struct neighbour *)
 > 572				rcu_dereference_protected(old_nht->hash_buckets[i].first,
   573							  lockdep_is_held(&tbl->lock));
   574			     n != NULL;
   575			     n = next) {
   576				hash = tbl->hash(n->primary_key, n->dev,
   577						 new_nht->hash_rnd);
   578	
   579				hash >>= (32 - new_nht->hash_shift);
   580				next = (struct neighbour *)hlist_next_rcu(&n->list);
   581				hlist_del_rcu(&n->list);
   582				hlist_add_head_rcu(&n->list, &new_nht->hash_buckets[hash]);
   583			}
   584		}
   585	
   586		rcu_assign_pointer(tbl->nht, new_nht);
   587		call_rcu(&old_nht->rcu, neigh_hash_free_rcu);
   588		return new_nht;
   589	}
   590	
   591	struct neighbour *neigh_lookup(struct neigh_table *tbl, const void *pkey,
   592				       struct net_device *dev)
   593	{
   594		struct neighbour *n;
   595	
   596		NEIGH_CACHE_STAT_INC(tbl, lookups);
   597	
   598		rcu_read_lock();
   599		n = __neigh_lookup_noref(tbl, pkey, dev);
   600		if (n) {
   601			if (!refcount_inc_not_zero(&n->refcnt))
   602				n = NULL;
   603			NEIGH_CACHE_STAT_INC(tbl, hits);
   604		}
   605	
   606		rcu_read_unlock();
   607		return n;
   608	}
   609	EXPORT_SYMBOL(neigh_lookup);
   610	
   611	static struct neighbour *
   612	___neigh_create(struct neigh_table *tbl, const void *pkey,
   613			struct net_device *dev, u32 flags,
   614			bool exempt_from_gc, bool want_ref)
   615	{
   616		u32 hash_val, key_len = tbl->key_len;
   617		struct neighbour *n1, *rc, *n;
   618		struct neigh_hash_table *nht;
   619		int error;
   620	
   621		n = neigh_alloc(tbl, dev, flags, exempt_from_gc);
   622		trace_neigh_create(tbl, dev, pkey, n, exempt_from_gc);
   623		if (!n) {
   624			rc = ERR_PTR(-ENOBUFS);
   625			goto out;
   626		}
   627	
   628		memcpy(n->primary_key, pkey, key_len);
   629		n->dev = dev;
   630		netdev_hold(dev, &n->dev_tracker, GFP_ATOMIC);
   631	
   632		/* Protocol specific setup. */
   633		if (tbl->constructor &&	(error = tbl->constructor(n)) < 0) {
   634			rc = ERR_PTR(error);
   635			goto out_neigh_release;
   636		}
   637	
   638		if (dev->netdev_ops->ndo_neigh_construct) {
   639			error = dev->netdev_ops->ndo_neigh_construct(dev, n);
   640			if (error < 0) {
   641				rc = ERR_PTR(error);
   642				goto out_neigh_release;
   643			}
   644		}
   645	
   646		/* Device specific setup. */
   647		if (n->parms->neigh_setup &&
   648		    (error = n->parms->neigh_setup(n)) < 0) {
   649			rc = ERR_PTR(error);
   650			goto out_neigh_release;
   651		}
   652	
   653		n->confirmed = jiffies - (NEIGH_VAR(n->parms, BASE_REACHABLE_TIME) << 1);
   654	
   655		write_lock_bh(&tbl->lock);
   656		nht = rcu_dereference_protected(tbl->nht,
   657						lockdep_is_held(&tbl->lock));
   658	
   659		if (atomic_read(&tbl->entries) > (1 << nht->hash_shift))
   660			nht = neigh_hash_grow(tbl, nht->hash_shift + 1);
   661	
   662		hash_val = tbl->hash(n->primary_key, dev, nht->hash_rnd) >> (32 - nht->hash_shift);
   663	
   664		if (n->parms->dead) {
   665			rc = ERR_PTR(-EINVAL);
   666			goto out_tbl_unlock;
   667		}
   668	
 > 669		hlist_for_each_entry_rcu(n1,
   670					 &nht->hash_buckets[hash_val],
   671					 list,
   672					 lockdep_is_held(&tbl->lock)) {
   673			if (dev == n1->dev && !memcmp(n1->primary_key, n->primary_key, key_len)) {
   674				if (want_ref)
   675					neigh_hold(n1);
   676				rc = n1;
   677				goto out_tbl_unlock;
   678			}
   679		}
   680	
   681		n->dead = 0;
   682		if (!exempt_from_gc)
   683			list_add_tail(&n->gc_list, &n->tbl->gc_list);
   684		if (n->flags & NTF_MANAGED)
   685			list_add_tail(&n->managed_list, &n->tbl->managed_list);
   686		if (want_ref)
   687			neigh_hold(n);
 > 688		hlist_add_head_rcu(&n->list, &nht->hash_buckets[hash_val]);
   689		write_unlock_bh(&tbl->lock);
   690		neigh_dbg(2, "neigh %p is created\n", n);
   691		rc = n;
   692	out:
   693		return rc;
   694	out_tbl_unlock:
   695		write_unlock_bh(&tbl->lock);
   696	out_neigh_release:
   697		if (!exempt_from_gc)
   698			atomic_dec(&tbl->gc_entries);
   699		neigh_release(n);
   700		goto out;
   701	}
   702	

-- 
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH net-next 1/2] Convert neighbour-table to use hlist
  2024-10-03 11:23   ` Simon Horman
@ 2024-10-04 15:06     ` Gilad Naaman
  0 siblings, 0 replies; 7+ messages in thread
From: Gilad Naaman @ 2024-10-04 15:06 UTC (permalink / raw)
  To: horms; +Cc: davem, edumazet, gnaaman, kuba, netdev, pabeni

Hey Simon,

Thank you for the notes, I'll make sure that Sparse is happy.

For some reason I can't get it to accept `hlist_for_each_entry_rcu`,
complaining that the cast removes `__rcu`, (even if I add `__rcu` to the
`pos` variable definition):


 > static int neigh_dump_table(struct neigh_table *tbl, struct sk_buff *skb,
 > 			    struct netlink_callback *cb,
 > 			    struct neigh_dump_filter *filter)
 > {
 > 	struct net *net = sock_net(skb->sk);
 > 	struct neighbour __rcu *n;

<snip>

Line 2752:
 > 		hlist_for_each_entry_rcu(n, &nht->hash_buckets[h], list) {

Sparse output:
 > net/core/neighbour.c:2752:17: sparse: warning: cast removes address space '__rcu' of expression

Do you know if this is solveable without reverting to the previous style
of iteration?

Thank you

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-10-04 15:06 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-10-01  5:09 [PATCH net-next 0/2] Improve neigh_flush_dev performance Gilad Naaman
2024-10-01  5:09 ` [PATCH net-next 1/2] Convert neighbour-table to use hlist Gilad Naaman
2024-10-03 11:23   ` Simon Horman
2024-10-04 15:06     ` Gilad Naaman
2024-10-04  1:41   ` kernel test robot
2024-10-01  5:09 ` [PATCH net-next 2/2] Create netdev->neighbour association Gilad Naaman
2024-10-03 11:26   ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).