[PATCH v2 0/2] ipv4: Make neigh lookup keys for loopback/point-to-point devices be INADDR_ANY

This used to be the previous behavior in older kernels but became broken in
a263b3093641f (ipv4: Make neigh lookups directly in output packet path)
and then later removed because it was broken in 0bb4087cbec0 (ipv4: Fix neigh
lookup keying over loopback/point-to-point devices)

Not having this results in there being an arp entry for every remote ip
address that the device talks to.  Given a fairly active device it can
cause the arp table to become huge and/or having to add/purge large number
of entires to keep within table size thresholds.

$ ip -4 neigh show nud noarp | grep tun | wc -l
55850

$ lnstat -k arp_cache:entries,arp_cache:allocs,arp_cache:destroys -c 10
arp_cach|arp_cach|arp_cach|
 entries|  allocs|destroys|
   81493|620166816|620126069|
  101867|   10186|       0|
  113854|    5993|       0|
  118773|    2459|       0|
   27937|   18579|   63998|
   39256|    5659|       0|
   56231|    8487|       0|
   65602|    4685|       0|
   79697|    7047|       0|
   90733|    5517|       0|

v2: 
 - fixes coding style issues

Jim Westfall (2):
  net: Allow neigh contructor functions ability to modify the
    primary_key
  ipv4: Make neigh lookup keys for loopback/point-to-point devices be
    INADDR_ANY

 include/net/arp.h    | 3 +++
 net/core/neighbour.c | 4 ++--
 net/ipv4/arp.c       | 7 ++++++-
 3 files changed, 11 insertions(+), 3 deletions(-)

-- 
2.15.1

Use n->primary_key instead of pkey to account for the possibility that a neigh
constructor function may have modified the primary_key value.

Signed-off-by: Jim Westfall <redacted>
---
 net/core/neighbour.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index d1f5fe986edd..7f831711b6e0 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c

@@ -532,7 +532,7 @@ 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(pkey, dev, nht->hash_rnd) >> (32 - nht->hash_shift);
+	hash_val = tbl->hash(n->primary_key, dev, nht->hash_rnd) >> (32 - nht->hash_shift);
 
 	if (n->parms->dead) {
 		rc = ERR_PTR(-EINVAL);

@@ -544,7 +544,7 @@ struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
 	     n1 != NULL;
 	     n1 = rcu_dereference_protected(n1->next,
 			lockdep_is_held(&tbl->lock))) {
-		if (dev == n1->dev && !memcmp(n1->primary_key, pkey, key_len)) {
+		if (dev == n1->dev && !memcmp(n1->primary_key, n->primary_key, key_len)) {
 			if (want_ref)
 				neigh_hold(n1);
 			rc = n1;

-- 
2.15.1

Map all lookup neigh keys to INADDR_ANY for loopback/point-to-point devices
to avoid making an entry for every remote ip the device needs to talk to.

This used the be the old behavior but became broken in a263b3093641f
(ipv4: Make neigh lookups directly in output packet path) and later removed
in 0bb4087cbec0 (ipv4: Fix neigh lookup keying over loopback/point-to-point
devices) because it was broken.

Signed-off-by: Jim Westfall <redacted>
---
 include/net/arp.h | 3 +++
 net/ipv4/arp.c    | 7 ++++++-
 2 files changed, 9 insertions(+), 1 deletion(-)

diff --git a/include/net/arp.h b/include/net/arp.h
index dc8cd47f883b..977aabfcdc03 100644
--- a/include/net/arp.h
+++ b/include/net/arp.h

@@ -20,6 +20,9 @@ static inline u32 arp_hashfn(const void *pkey, const struct net_device *dev, u32
 
 static inline struct neighbour *__ipv4_neigh_lookup_noref(struct net_device *dev, u32 key)
 {
+	if (dev->flags & (IFF_LOOPBACK | IFF_POINTOPOINT))
+		key = INADDR_ANY;
+
 	return ___neigh_lookup_noref(&arp_tbl, neigh_key_eq32, arp_hashfn, &key, dev);
 }
 

diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index a8d7c5a9fb05..6c231b43974d 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c

@@ -223,11 +223,16 @@ static bool arp_key_eq(const struct neighbour *neigh, const void *pkey)
 
 static int arp_constructor(struct neighbour *neigh)
 {
-	__be32 addr = *(__be32 *)neigh->primary_key;
+	__be32 addr;
 	struct net_device *dev = neigh->dev;
 	struct in_device *in_dev;
 	struct neigh_parms *parms;
+	u32 inaddr_any = INADDR_ANY;
 
+	if (dev->flags & (IFF_LOOPBACK | IFF_POINTOPOINT))
+		memcpy(neigh->primary_key, &inaddr_any, arp_tbl.key_len);
+
+	addr = *(__be32 *)neigh->primary_key;
 	rcu_read_lock();
 	in_dev = __in_dev_get_rcu(dev);
 	if (!in_dev) {

-- 
2.15.1

From: Jim Westfall <redacted>
Date: Sun, 14 Jan 2018 04:18:49 -0800

This used to be the previous behavior in older kernels but became broken in
a263b3093641f (ipv4: Make neigh lookups directly in output packet path)
and then later removed because it was broken in 0bb4087cbec0 (ipv4: Fix neigh
lookup keying over loopback/point-to-point devices)

Not having this results in there being an arp entry for every remote ip
address that the device talks to.  Given a fairly active device it can
cause the arp table to become huge and/or having to add/purge large number
of entires to keep within table size thresholds.

...

v2: 
 - fixes coding style issues

Series applied and queued up for -stable, thank you.

David Miller [off-list ref] wrote [01.15.18]:

From: Jim Westfall <redacted>
Date: Sun, 14 Jan 2018 04:18:49 -0800

quoted

This used to be the previous behavior in older kernels but became broken in
a263b3093641f (ipv4: Make neigh lookups directly in output packet path)
and then later removed because it was broken in 0bb4087cbec0 (ipv4: Fix neigh
lookup keying over loopback/point-to-point devices)

Not having this results in there being an arp entry for every remote ip
address that the device talks to.  Given a fairly active device it can
cause the arp table to become huge and/or having to add/purge large number
of entires to keep within table size thresholds.

...

quoted

v2: 
 - fixes coding style issues

Series applied and queued up for -stable, thank you.

Thanks for applying these.  We see the same type of behavior with ipv6  
over point-to-point interfaces and I would like to fix these as well by 
mapping all the ndisc_cache entries to in6addr_any.  However my knowledge 
of ndisc is limited and I'm unclear if its safe to assume ndisc, like 
arp, would never exist on the point-to-point interface.

Thanks
jim

From: Jim Westfall <redacted>
Date: Mon, 15 Jan 2018 13:42:38 -0800

David Miller [off-list ref] wrote [01.15.18]:

quoted

From: Jim Westfall <redacted>
Date: Sun, 14 Jan 2018 04:18:49 -0800

quoted

This used to be the previous behavior in older kernels but became broken in
a263b3093641f (ipv4: Make neigh lookups directly in output packet path)
and then later removed because it was broken in 0bb4087cbec0 (ipv4: Fix neigh
lookup keying over loopback/point-to-point devices)

Not having this results in there being an arp entry for every remote ip
address that the device talks to.  Given a fairly active device it can
cause the arp table to become huge and/or having to add/purge large number
of entires to keep within table size thresholds.

...

quoted

v2: 
 - fixes coding style issues

Series applied and queued up for -stable, thank you.

Thanks for applying these.  We see the same type of behavior with ipv6  
over point-to-point interfaces and I would like to fix these as well by 
mapping all the ndisc_cache entries to in6addr_any.  However my knowledge 
of ndisc is limited and I'm unclear if its safe to assume ndisc, like 
arp, would never exist on the point-to-point interface.

Ok, hopefully some ipv6 experts can chime in.

Thank you.