[PATCH net-next 0/7] neighbor: cleanups plus extack for add and delete

[PATCH net-next 0/7] neighbor: cleanups plus extack for add and delete

[David Ahern]

From: David Ahern <dsahern@gmail.com>

cleanups:
- remove open coding of key and hash functions for ipv4 and ipv6
  and then collapse hash functions
- collapse now unnecessary ___neigh_lookup_noref helper
- create helper for neigh hash computation
- remove duplicate lookup in neigh_add

After that add extack messages for neighbor add and delete.

David Ahern (7):
  neighbor: Remove open coding of key and hash functions
  neighbor: Fold ___neigh_lookup_noref into __neigh_lookup_noref
  net/ipv4: Move arp_hashfn into arp_hash
  net/ipv6: Move ndisc_hashfn to ndisc_hash
  neighbor: Create a neigh_hash helper
  neighbor: Skip the duplicate lookup in neigh_add
  neighbor: Add extack messages for add and delete commands

 include/net/arp.h       | 10 +------
 include/net/ndisc.h     | 12 +--------
 include/net/neighbour.h | 30 +++++++++------------
 net/core/filter.c       |  3 +--
 net/core/neighbour.c    | 72 ++++++++++++++++++++++++++++++-------------------
 net/ipv4/arp.c          |  5 +++-
 net/ipv6/ndisc.c        |  7 ++++-
 7 files changed, 71 insertions(+), 68 deletions(-)

-- 
2.11.0

[PATCH net-next 1/7] neighbor: Remove open coding of key and hash functions

[David Ahern]

From: David Ahern <dsahern@gmail.com>

___neigh_lookup_noref takes the key and hash functions as inputs, yet
those are part of the operations listed in the neigh_table which is
also passed as an arugment. Remove the open coding of these internal
implementations by converting uses of ___neigh_lookup_noref to
__neigh_lookup_noref.

For IPv4, arp_key_eq is essentially a call to neigh_key_eq32 and
arp_hash is a call to arp_hashfn. Similarly for IPv6, ndisc_key_eq
calls neigh_key_eq128 and ndisc_hash calls ndisc_hashfn. So the change
in helpers is a no-op.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/arp.h   | 2 +-
 include/net/ndisc.h | 2 +-
 net/core/filter.c   | 3 +--
 3 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/include/net/arp.h b/include/net/arp.h
index 977aabfcdc03..a5091f13cd3e 100644
--- a/include/net/arp.h
+++ b/include/net/arp.h
@@ -23,7 +23,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);
 }
 
 static inline struct neighbour *__ipv4_neigh_lookup(struct net_device *dev, u32 key)
diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index ddfbb591e2c5..c354345c679b 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -376,7 +376,7 @@ 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(struct net_device *dev, const void *pkey)
diff --git a/net/core/filter.c b/net/core/filter.c
index bd0df75dc7b6..f10cc675783c 100644
--- a/net/core/filter.c
+++ b/net/core/filter.c
@@ -4668,8 +4668,7 @@ static int bpf_ipv6_fib_lookup(struct net *net, struct bpf_fib_lookup *params,
 	 * not needed here. Can not use __ipv6_neigh_lookup_noref here
 	 * because we need to get nd_tbl via the stub
 	 */
-	neigh = ___neigh_lookup_noref(ipv6_stub->nd_tbl, neigh_key_eq128,
-				      ndisc_hashfn, dst, dev);
+	neigh = __neigh_lookup_noref(ipv6_stub->nd_tbl, dst, dev);
 	if (!neigh)
 		return BPF_FIB_LKUP_RET_NO_NEIGH;
 
-- 
2.11.0

[PATCH net-next 2/7] neighbor: Fold ___neigh_lookup_noref into __neigh_lookup_noref

[David Ahern]

From: David Ahern <dsahern@gmail.com>

There are no more direct callers of ___neigh_lookup_noref so no need
for it to be a standalone helper.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/neighbour.h | 22 +++++-----------------
 1 file changed, 5 insertions(+), 17 deletions(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index f58b384aa6c9..aac87bc2d96b 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -270,37 +270,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_bh(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_bh(nht->hash_buckets[hash_val]);
 	     n != NULL;
 	     n = rcu_dereference_bh(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);
-}
-
 void neigh_table_init(int index, struct neigh_table *tbl);
 int neigh_table_clear(int index, struct neigh_table *tbl);
 struct neighbour *neigh_lookup(struct neigh_table *tbl, const void *pkey,
-- 
2.11.0

[PATCH net-next 3/7] net/ipv4: Move arp_hashfn into arp_hash

[David Ahern]

From: David Ahern <dsahern@gmail.com>

There are no more direct references to arp_hashfn so fold it into
arp_hash, the hash callback for arp.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/arp.h | 8 --------
 net/ipv4/arp.c    | 5 ++++-
 2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/include/net/arp.h b/include/net/arp.h
index a5091f13cd3e..9f433c077b67 100644
--- a/include/net/arp.h
+++ b/include/net/arp.h
@@ -10,14 +10,6 @@
 
 extern struct neigh_table arp_tbl;
 
-static inline u32 arp_hashfn(const void *pkey, const struct net_device *dev, u32 *hash_rnd)
-{
-	u32 key = *(const u32 *)pkey;
-	u32 val = key ^ hash32_ptr(dev);
-
-	return val * hash_rnd[0];
-}
-
 static inline struct neighbour *__ipv4_neigh_lookup_noref(struct net_device *dev, u32 key)
 {
 	if (dev->flags & (IFF_LOOPBACK | IFF_POINTOPOINT))
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 850a6f13a082..6b88211287ae 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -213,7 +213,10 @@ static u32 arp_hash(const void *pkey,
 		    const struct net_device *dev,
 		    __u32 *hash_rnd)
 {
-	return arp_hashfn(pkey, dev, hash_rnd);
+	u32 key = *(const u32 *)pkey;
+	u32 val = key ^ hash32_ptr(dev);
+
+	return val * hash_rnd[0];
 }
 
 static bool arp_key_eq(const struct neighbour *neigh, const void *pkey)
-- 
2.11.0

[PATCH net-next 4/7] net/ipv6: Move ndisc_hashfn to ndisc_hash

[David Ahern]

From: David Ahern <dsahern@gmail.com>

There are no more direct references to ndisc_hashfn so fold it into
ndisc_hash, the hash callback for ndisc.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/ndisc.h | 10 ----------
 net/ipv6/ndisc.c    |  7 ++++++-
 2 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/include/net/ndisc.h b/include/net/ndisc.h
index c354345c679b..83a84f68901b 100644
--- a/include/net/ndisc.h
+++ b/include/net/ndisc.h
@@ -364,16 +364,6 @@ static inline u8 *ndisc_opt_addr_data(struct nd_opt_hdr *p,
 				     ndisc_addr_option_pad(dev->type));
 }
 
-static inline u32 ndisc_hashfn(const void *pkey, const struct net_device *dev, __u32 *hash_rnd)
-{
-	const u32 *p32 = pkey;
-
-	return (((p32[0] ^ hash32_ptr(dev)) * hash_rnd[0]) +
-		(p32[1] * hash_rnd[1]) +
-		(p32[2] * hash_rnd[2]) +
-		(p32[3] * hash_rnd[3]));
-}
-
 static inline struct neighbour *__ipv6_neigh_lookup_noref(struct net_device *dev, const void *pkey)
 {
 	return __neigh_lookup_noref(&nd_tbl, pkey, dev);
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index 659ecf4e4b3c..304a32b3c3f5 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -311,7 +311,12 @@ static u32 ndisc_hash(const void *pkey,
 		      const struct net_device *dev,
 		      __u32 *hash_rnd)
 {
-	return ndisc_hashfn(pkey, dev, hash_rnd);
+	const u32 *p32 = pkey;
+
+	return (((p32[0] ^ hash32_ptr(dev)) * hash_rnd[0]) +
+		 (p32[1] * hash_rnd[1]) +
+		 (p32[2] * hash_rnd[2]) +
+		 (p32[3] * hash_rnd[3]));
 }
 
 static bool ndisc_key_eq(const struct neighbour *n, const void *pkey)
-- 
2.11.0

[PATCH net-next 5/7] neighbor: Create a neigh_hash helper

[David Ahern]

From: David Ahern <dsahern@gmail.com>

Consolidate calculations of the neighbor hash into a single helper.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 include/net/neighbour.h | 10 +++++++++-
 net/core/neighbour.c    | 15 +++++----------
 2 files changed, 14 insertions(+), 11 deletions(-)

diff --git a/include/net/neighbour.h b/include/net/neighbour.h
index aac87bc2d96b..092493a8c91b 100644
--- a/include/net/neighbour.h
+++ b/include/net/neighbour.h
@@ -270,6 +270,14 @@ static inline bool neigh_key_eq128(const struct neighbour *n, const void *pkey)
 		(n32[2] ^ p32[2]) | (n32[3] ^ p32[3])) == 0;
 }
 
+static inline u32 neigh_hash(struct neigh_table *tbl,
+			     struct neigh_hash_table *nht,
+			     const void *pkey,
+			     struct net_device *dev)
+{
+	return tbl->hash(pkey, dev, nht->hash_rnd) >> (32 - nht->hash_shift);
+}
+
 static inline struct neighbour *__neigh_lookup_noref(struct neigh_table *tbl,
 						     const void *pkey,
 						     struct net_device *dev)
@@ -278,7 +286,7 @@ static inline struct neighbour *__neigh_lookup_noref(struct neigh_table *tbl,
 	struct neighbour *n;
 	u32 hash_val;
 
-	hash_val = tbl->hash(pkey, dev, nht->hash_rnd) >> (32 - nht->hash_shift);
+	hash_val = neigh_hash(tbl, nht, pkey, dev);
 	for (n = rcu_dereference_bh(nht->hash_buckets[hash_val]);
 	     n != NULL;
 	     n = rcu_dereference_bh(n->next)) {
diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 41954e42a2de..53e30c15882d 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -151,9 +151,8 @@ bool neigh_remove_one(struct neighbour *ndel, struct neigh_table *tbl)
 
 	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);
 
+	hash_val = neigh_hash(tbl, nht, pkey, ndel->dev);
 	np = &nht->hash_buckets[hash_val];
 	while ((n = rcu_dereference_protected(*np,
 					      lockdep_is_held(&tbl->lock)))) {
@@ -434,10 +433,7 @@ static struct neigh_hash_table *neigh_hash_grow(struct neigh_table *tbl,
 						   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);
+			hash = neigh_hash(tbl, new_nht, n->primary_key, n->dev);
 			next = rcu_dereference_protected(n->next,
 						lockdep_is_held(&tbl->lock));
 
@@ -485,9 +481,9 @@ struct neighbour *neigh_lookup_nodev(struct neigh_table *tbl, struct net *net,
 	NEIGH_CACHE_STAT_INC(tbl, lookups);
 
 	rcu_read_lock_bh();
-	nht = rcu_dereference_bh(tbl->nht);
-	hash_val = tbl->hash(pkey, NULL, nht->hash_rnd) >> (32 - nht->hash_shift);
 
+	nht = rcu_dereference_bh(tbl->nht);
+	hash_val = neigh_hash(tbl, nht, pkey, NULL);
 	for (n = rcu_dereference_bh(nht->hash_buckets[hash_val]);
 	     n != NULL;
 	     n = rcu_dereference_bh(n->next)) {
@@ -553,13 +549,12 @@ struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
 	if (atomic_read(&tbl->entries) > (1 << nht->hash_shift))
 		nht = neigh_hash_grow(tbl, nht->hash_shift + 1);
 
-	hash_val = tbl->hash(n->primary_key, dev, nht->hash_rnd) >> (32 - nht->hash_shift);
-
 	if (n->parms->dead) {
 		rc = ERR_PTR(-EINVAL);
 		goto out_tbl_unlock;
 	}
 
+	hash_val = neigh_hash(tbl, nht, n->primary_key, dev);
 	for (n1 = rcu_dereference_protected(nht->hash_buckets[hash_val],
 					    lockdep_is_held(&tbl->lock));
 	     n1 != NULL;
-- 
2.11.0

[PATCH net-next 6/7] neighbor: Skip the duplicate lookup in neigh_add

[David Ahern]

From: David Ahern <dsahern@gmail.com>

When adding a new neighbor via rtnetlink, neigh_add does a lookup
and if the result is NULL calls __neigh_lookup_errno to create a
new entry if the NLM_F_CREATE flag is set. But, __neigh_lookup_errno
calls neigh_lookup again before neigh_create; the neigh_lookup is
redundant.

Replace the call to __neigh_lookup_errno with a call to __neigh_create
to more efficiently achieve the same result and prepare for the next
patch.

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/neighbour.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 53e30c15882d..e324467e9a71 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1785,7 +1785,7 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
 			goto out;
 		}
 
-		neigh = __neigh_lookup_errno(tbl, dst, dev);
+		neigh = __neigh_create(tbl, dst, dev, true);
 		if (IS_ERR(neigh)) {
 			err = PTR_ERR(neigh);
 			goto out;
-- 
2.11.0

[PATCH net-next 7/7] neighbor: Add extack messages for add and delete commands

[David Ahern]

From: David Ahern <dsahern@gmail.com>

    Add extack messages for failures in neigh_add and neigh_delete.

    Also, require NDA_DST length to be exactly the key length for the
    table otherwise it is an unexpected address and can lead to unexpected
    entries. e.g., IPv4 table sent and IPv6 address (using a modified ip):
        $ ip neigh add 2001:db8:1::1 dev foo
        $ ip neigh ls dev foo
        32.1.13.184 dev foo lladdr 72:ed:f1:d9:20:9a PERMANENT

Signed-off-by: David Ahern <dsahern@gmail.com>
---
 net/core/neighbour.c | 55 +++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 39 insertions(+), 16 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index e324467e9a71..916a99fbb306 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -1132,8 +1132,9 @@ static void neigh_update_hhs(struct neighbour *neigh)
    Caller MUST hold reference count on the entry.
  */
 
-int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
-		 u32 flags, u32 nlmsg_pid)
+static int __neigh_update(struct neighbour *neigh, const u8 *lladdr,
+			  u8 new, u32 flags, u32 nlmsg_pid,
+			  struct netlink_ext_ack *extack)
 {
 	u8 old;
 	int err;
@@ -1150,8 +1151,10 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
 	if (!(flags & NEIGH_UPDATE_F_ADMIN) &&
 	    (old & (NUD_NOARP | NUD_PERMANENT)))
 		goto out;
-	if (neigh->dead)
+	if (neigh->dead) {
+		NL_SET_ERR_MSG(extack, "Neighbor entry is now dead");
 		goto out;
+	}
 
 	neigh_update_ext_learned(neigh, flags, &notify);
 
@@ -1188,8 +1191,10 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
 		   use it, otherwise discard the request.
 		 */
 		err = -EINVAL;
-		if (!(old & NUD_VALID))
+		if (!(old & NUD_VALID)) {
+			NL_SET_ERR_MSG(extack, "No link layer address given");
 			goto out;
+		}
 		lladdr = neigh->ha;
 	}
 
@@ -1302,6 +1307,12 @@ int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
 
 	return err;
 }
+
+int neigh_update(struct neighbour *neigh, const u8 *lladdr, u8 new,
+		 u32 flags, u32 nlmsg_pid)
+{
+	return __neigh_update(neigh, lladdr, new, flags, nlmsg_pid, NULL);
+}
 EXPORT_SYMBOL(neigh_update);
 
 /* Update the neigh to listen temporarily for probe responses, even if it is
@@ -1673,8 +1684,10 @@ static int neigh_delete(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out;
 
 	dst_attr = nlmsg_find_attr(nlh, sizeof(*ndm), NDA_DST);
-	if (dst_attr == NULL)
+	if (!dst_attr) {
+		NL_SET_ERR_MSG(extack, "Network address not specified");
 		goto out;
+	}
 
 	ndm = nlmsg_data(nlh);
 	if (ndm->ndm_ifindex) {
@@ -1689,8 +1702,10 @@ static int neigh_delete(struct sk_buff *skb, struct nlmsghdr *nlh,
 	if (tbl == NULL)
 		return -EAFNOSUPPORT;
 
-	if (nla_len(dst_attr) < (int)tbl->key_len)
+	if (nla_len(dst_attr) < (int)tbl->key_len) {
+		NL_SET_ERR_MSG(extack, "Invalid network address");
 		goto out;
+	}
 
 	if (ndm->ndm_flags & NTF_PROXY) {
 		err = pneigh_delete(tbl, net, nla_data(dst_attr), dev);
@@ -1706,10 +1721,9 @@ static int neigh_delete(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out;
 	}
 
-	err = neigh_update(neigh, NULL, NUD_FAILED,
-			   NEIGH_UPDATE_F_OVERRIDE |
-			   NEIGH_UPDATE_F_ADMIN,
-			   NETLINK_CB(skb).portid);
+	err = __neigh_update(neigh, NULL, NUD_FAILED,
+			     NEIGH_UPDATE_F_OVERRIDE | NEIGH_UPDATE_F_ADMIN,
+			     NETLINK_CB(skb).portid, extack);
 	write_lock_bh(&tbl->lock);
 	neigh_release(neigh);
 	neigh_remove_one(neigh, tbl);
@@ -1739,8 +1753,10 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out;
 
 	err = -EINVAL;
-	if (tb[NDA_DST] == NULL)
+	if (!tb[NDA_DST]) {
+		NL_SET_ERR_MSG(extack, "Network address not specified");
 		goto out;
+	}
 
 	ndm = nlmsg_data(nlh);
 	if (ndm->ndm_ifindex) {
@@ -1750,16 +1766,21 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
 			goto out;
 		}
 
-		if (tb[NDA_LLADDR] && nla_len(tb[NDA_LLADDR]) < dev->addr_len)
+		if (tb[NDA_LLADDR] && nla_len(tb[NDA_LLADDR]) < dev->addr_len) {
+			NL_SET_ERR_MSG(extack, "Invalid link address");
 			goto out;
+		}
 	}
 
 	tbl = neigh_find_table(ndm->ndm_family);
 	if (tbl == NULL)
 		return -EAFNOSUPPORT;
 
-	if (nla_len(tb[NDA_DST]) < (int)tbl->key_len)
+	if (nla_len(tb[NDA_DST]) != (int)tbl->key_len) {
+		NL_SET_ERR_MSG(extack, "Invalid network address");
 		goto out;
+	}
+
 	dst = nla_data(tb[NDA_DST]);
 	lladdr = tb[NDA_LLADDR] ? nla_data(tb[NDA_LLADDR]) : NULL;
 
@@ -1775,8 +1796,10 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
 		goto out;
 	}
 
-	if (dev == NULL)
+	if (!dev) {
+		NL_SET_ERR_MSG(extack, "Device not specified");
 		goto out;
+	}
 
 	neigh = neigh_lookup(tbl, dst, dev);
 	if (neigh == NULL) {
@@ -1812,8 +1835,8 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
 		neigh_event_send(neigh, NULL);
 		err = 0;
 	} else
-		err = neigh_update(neigh, lladdr, ndm->ndm_state, flags,
-				   NETLINK_CB(skb).portid);
+		err = __neigh_update(neigh, lladdr, ndm->ndm_state, flags,
+				     NETLINK_CB(skb).portid, extack);
 	neigh_release(neigh);
 
 out:
-- 
2.11.0

Re: [PATCH net-next 2/7] neighbor: Fold ___neigh_lookup_noref into __neigh_lookup_noref

[David Miller]

From: David Ahern <dsahern@kernel.org>
Date: Wed,  5 Dec 2018 15:34:09 -0800

> @@ -270,37 +270,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)
>  {

Sorry, we can't do this.

The whole point of how this is laid out is so that the entire hash traversal,
including the hash function, is expanded inline.

This demux is extremely critical on the output side, it must be the
smallest number of cycles possible.  It was the only way I could justify
not caching neigh entries in the routes any more when I wrote this code.

Even before retpoline, putting an indirect call here is painful.  With
retpoline it is deadly.

Please avoid removing the full inline expansion of the neigh lookup in the ipv6
and ipv4 data paths.

Thank you.

Re: [PATCH net-next 2/7] neighbor: Fold ___neigh_lookup_noref into __neigh_lookup_noref

[David Ahern]

On 12/5/18 5:44 PM, David Miller wrote:
> From: David Ahern <dsahern@kernel.org>
> Date: Wed,  5 Dec 2018 15:34:09 -0800
> 
>> @@ -270,37 +270,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)
>>  {
> 
> Sorry, we can't do this.
> 
> The whole point of how this is laid out is so that the entire hash traversal,
> including the hash function, is expanded inline.
> 
> This demux is extremely critical on the output side, it must be the
> smallest number of cycles possible.  It was the only way I could justify
> not caching neigh entries in the routes any more when I wrote this code.
> 
> Even before retpoline, putting an indirect call here is painful.  With
> retpoline it is deadly.
> 
> Please avoid removing the full inline expansion of the neigh lookup in the ipv6
> and ipv4 data paths.
> 

ok. patches 5-7 are not dependent on 1-4. Should I re-send outside of
this set?

Re: [PATCH net-next 2/7] neighbor: Fold ___neigh_lookup_noref into __neigh_lookup_noref

[David Ahern]

On 12/5/18 5:46 PM, David Ahern wrote:
> ok. patches 5-7 are not dependent on 1-4. Should I re-send outside of
> this set?

bleh. 5 is. I'll re-send.

Re: [PATCH net-next 2/7] neighbor: Fold ___neigh_lookup_noref into __neigh_lookup_noref

[David Miller]

From: David Ahern <dsahern@gmail.com>
Date: Wed, 5 Dec 2018 17:46:37 -0700

> ok. patches 5-7 are not dependent on 1-4. Should I re-send outside of
> this set?

Yes, please respin.

Thanks David.