[PATCH] net: remove ___neigh_lookup_noref().

[PATCH] net: remove ___neigh_lookup_noref().

[YangXin]

key_eq() and hash() are functions of struct neigh_table, so we just need to call tbl->key_eq() and tbl->hash(), instead of passing them in as parameters. 

And if those two parameters were removed,  ___neigh_lookup_noref() would be pointless, so I replaced ___neigh_lookup_noref() with __neigh_lookup_noref().

Signed-off-by: YangXin <yx.0xffff@gmail.com>
---
Last time I comitted this patch, Mr Dumazet said "this might defeat inlining.".
So I compiled kernel on my computer with defconfig, made sure that this patch would not lead __neigh_lookup_noref() fail to inline.

My enviroment:
Debian12
gcc version:
gcc (Debian 12.2.0-14) 12.2.0

 include/net/arp.h       |  2 +-
 include/net/ndisc.h     |  5 ++---
 include/net/neighbour.h | 22 +++++-----------------
 3 files changed, 8 insertions(+), 21 deletions(-)

diff --git a/include/net/arp.h b/include/net/arp.h
index e8747e0713c7..e274c7fc5020 100644
--- a/include/net/arp.h
+++ b/include/net/arp.h
@@ -24,7 +24,7 @@ static inline struct neighbour *__ipv4_neigh_lookup_noref(struct net_device *dev
 	if (dev->flags & (IFF_LOOPBACK | IFF_POINTOPOINT))
 		key = INADDR_ANY;
 
-	return ___neigh_lookup_noref(&arp_tbl, neigh_key_eq32, arp_hashfn, &key, dev);
+	return __neigh_lookup_noref(&arp_tbl, &key, dev);
 }
 #else
 static inline
diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index 9bbdf6eaa942..8e0ba9a87e4d 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -380,15 +380,14 @@ static inline u32 ndisc_hashfn(const void *pkey, const struct net_device *dev, _
 
 static inline struct neighbour *__ipv6_neigh_lookup_noref(struct net_device *dev, const void *pkey)
 {
-	return ___neigh_lookup_noref(&nd_tbl, neigh_key_eq128, ndisc_hashfn, pkey, dev);
+	return __neigh_lookup_noref(&nd_tbl, pkey, dev);
 }
 
 static inline
 struct neighbour *__ipv6_neigh_lookup_noref_stub(struct net_device *dev,
 						 const void *pkey)
 {
-	return ___neigh_lookup_noref(ipv6_stub->nd_tbl, neigh_key_eq128,
-				     ndisc_hashfn, pkey, dev);
+	return __neigh_lookup_noref(ipv6_stub->nd_tbl, pkey, dev);
 }
 
 static inline struct neighbour *__ipv6_neigh_lookup(struct net_device *dev, const void *pkey)
diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index 0d28172193fa..434c9e7c7ea7 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -290,37 +290,25 @@ static inline bool neigh_key_eq128(const struct neighbour *n, const void *pkey)
 		(n32[2] ^ p32[2]) | (n32[3] ^ p32[3])) == 0;
 }
 
-static inline struct neighbour *___neigh_lookup_noref(
-	struct neigh_table *tbl,
-	bool (*key_eq)(const struct neighbour *n, const void *pkey),
-	__u32 (*hash)(const void *pkey,
-		      const struct net_device *dev,
-		      __u32 *hash_rnd),
-	const void *pkey,
-	struct net_device *dev)
+static inline struct neighbour *__neigh_lookup_noref(struct neigh_table *tbl,
+						     const void *pkey,
+						     struct net_device *dev)
 {
 	struct neigh_hash_table *nht = rcu_dereference(tbl->nht);
 	struct neighbour *n;
 	u32 hash_val;
 
-	hash_val = hash(pkey, dev, nht->hash_rnd) >> (32 - nht->hash_shift);
+	hash_val = tbl->hash(pkey, dev, nht->hash_rnd) >> (32 - nht->hash_shift);
 	for (n = rcu_dereference(nht->hash_buckets[hash_val]);
 	     n != NULL;
 	     n = rcu_dereference(n->next)) {
-		if (n->dev == dev && key_eq(n, pkey))
+		if (n->dev == dev && tbl->key_eq(n, pkey))
 			return n;
 	}
 
 	return NULL;
 }
 
-static inline struct neighbour *__neigh_lookup_noref(struct neigh_table *tbl,
-						     const void *pkey,
-						     struct net_device *dev)
-{
-	return ___neigh_lookup_noref(tbl, tbl->key_eq, tbl->hash, pkey, dev);
-}
-
 static inline void neigh_confirm(struct neighbour *n)
 {
 	if (n) {
-- 
2.33.0

Re: [PATCH] net: remove ___neigh_lookup_noref().

[Eric Dumazet]

On Tue, Dec 5, 2023 at 8:10 PM YangXin <yx.0xffff@gmail.com> wrote:
>
> key_eq() and hash() are functions of struct neigh_table, so we just need to call tbl->key_eq() and tbl->hash(), instead of passing them in as parameters.
>
> And if those two parameters were removed,  ___neigh_lookup_noref() would be pointless, so I replaced ___neigh_lookup_noref() with __neigh_lookup_noref().
>
> Signed-off-by: YangXin <yx.0xffff@gmail.com>
> ---
> Last time I comitted this patch, Mr Dumazet said "this might defeat inlining.".
> So I compiled kernel on my computer with defconfig, made sure that this patch would not lead __neigh_lookup_noref() fail to inline.

Not sure how you checked, but I found the opposite.

This patch adds additional indirect function calls, with additional
RETPOLINE costs.

Look at ip_neigh_gw4() disassembly before/after your patch.