2019-06-09 16:52:04

by Greg Kroah-Hartman

[permalink] [raw]
Subject: [PATCH 4.14 03/35] neighbor: Call __ipv4_neigh_lookup_noref in neigh_xmit

From: David Ahern <[email protected]>

[ Upstream commit 4b2a2bfeb3f056461a90bd621e8bd7d03fa47f60 ]

Commit cd9ff4de0107 changed the key for IFF_POINTOPOINT devices to
INADDR_ANY but neigh_xmit which is used for MPLS encapsulations was not
updated to use the altered key. The result is that every packet Tx does
a lookup on the gateway address which does not find an entry, a new one
is created only to find the existing one in the table right before the
insert since arp_constructor was updated to reset the primary key. This
is seen in the allocs and destroys counters:
ip -s -4 ntable show | head -10 | grep alloc

which increase for each packet showing the unnecessary overhread.

Fix by having neigh_xmit use __ipv4_neigh_lookup_noref for NEIGH_ARP_TABLE.

Fixes: cd9ff4de0107 ("ipv4: Make neigh lookup keys for loopback/point-to-point devices be INADDR_ANY")
Reported-by: Alan Maguire <[email protected]>
Signed-off-by: David Ahern <[email protected]>
Tested-by: Alan Maguire <[email protected]>
Signed-off-by: David S. Miller <[email protected]>
Signed-off-by: Greg Kroah-Hartman <[email protected]>
---
net/core/neighbour.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -30,6 +30,7 @@
#include <linux/times.h>
#include <net/net_namespace.h>
#include <net/neighbour.h>
+#include <net/arp.h>
#include <net/dst.h>
#include <net/sock.h>
#include <net/netevent.h>
@@ -2528,7 +2529,13 @@ int neigh_xmit(int index, struct net_dev
if (!tbl)
goto out;
rcu_read_lock_bh();
- neigh = __neigh_lookup_noref(tbl, addr, dev);
+ if (index == NEIGH_ARP_TABLE) {
+ u32 key = *((u32 *)addr);
+
+ neigh = __ipv4_neigh_lookup_noref(dev, key);
+ } else {
+ neigh = __neigh_lookup_noref(tbl, addr, dev);
+ }
if (!neigh)
neigh = __neigh_create(tbl, addr, dev, false);
err = PTR_ERR(neigh);



2019-06-10 01:12:00

by Nobuhiro Iwamatsu

[permalink] [raw]
Subject: Re: [PATCH 4.14 03/35] neighbor: Call __ipv4_neigh_lookup_noref in neigh_xmit

Hi,

On Sun, Jun 09, 2019 at 06:42:09PM +0200, Greg Kroah-Hartman wrote:
> From: David Ahern <[email protected]>
>
> [ Upstream commit 4b2a2bfeb3f056461a90bd621e8bd7d03fa47f60 ]
>
> Commit cd9ff4de0107 changed the key for IFF_POINTOPOINT devices to
> INADDR_ANY but neigh_xmit which is used for MPLS encapsulations was not
> updated to use the altered key. The result is that every packet Tx does
> a lookup on the gateway address which does not find an entry, a new one
> is created only to find the existing one in the table right before the
> insert since arp_constructor was updated to reset the primary key. This
> is seen in the allocs and destroys counters:
> ip -s -4 ntable show | head -10 | grep alloc
>
> which increase for each packet showing the unnecessary overhread.
>
> Fix by having neigh_xmit use __ipv4_neigh_lookup_noref for NEIGH_ARP_TABLE.
>
> Fixes: cd9ff4de0107 ("ipv4: Make neigh lookup keys for loopback/point-to-point devices be INADDR_ANY")
> Reported-by: Alan Maguire <[email protected]>
> Signed-off-by: David Ahern <[email protected]>
> Tested-by: Alan Maguire <[email protected]>
> Signed-off-by: David S. Miller <[email protected]>
> Signed-off-by: Greg Kroah-Hartman <[email protected]>
> ---

This commit also requires the following commit:

commit 9b3040a6aafd7898ece7fc7efcbca71e42aa8069
Author: David Ahern <[email protected]>
Date: Sun May 5 11:16:20 2019 -0700

ipv4: Define __ipv4_neigh_lookup_noref when CONFIG_INET is disabled

Define __ipv4_neigh_lookup_noref to return NULL when CONFIG_INET is disabled.

Fixes: 4b2a2bfeb3f0 ("neighbor: Call __ipv4_neigh_lookup_noref in neigh_xmit")
Reported-by: kbuild test robot <[email protected]>
Signed-off-by: David Ahern <[email protected]>
Signed-off-by: David S. Miller <[email protected]>

And this is also necessary for 4.4.y, 4.14.y, 4.19.y and 5.1.y.
Please apply this commit.

Best regards,
Nobuhiro

2019-06-10 01:18:35

by Nobuhiro Iwamatsu

[permalink] [raw]
Subject: RE: [PATCH 4.14 03/35] neighbor: Call __ipv4_neigh_lookup_noref in neigh_xmit

Hi again.

> -----Original Message-----
> From: [email protected]
> [mailto:[email protected]] On Behalf Of Nobuhiro Iwamatsu
> Sent: Monday, June 10, 2019 10:10 AM
> To: Greg Kroah-Hartman <[email protected]>
> Cc: [email protected]; [email protected]; Alan Maguire
> <[email protected]>; David Ahern <[email protected]>; David S.
> Miller <[email protected]>
> Subject: Re: [PATCH 4.14 03/35] neighbor: Call __ipv4_neigh_lookup_noref
> in neigh_xmit
>
> Hi,
>
> On Sun, Jun 09, 2019 at 06:42:09PM +0200, Greg Kroah-Hartman wrote:
> > From: David Ahern <[email protected]>
> >
> > [ Upstream commit 4b2a2bfeb3f056461a90bd621e8bd7d03fa47f60 ]
> >
> > Commit cd9ff4de0107 changed the key for IFF_POINTOPOINT devices to
> > INADDR_ANY but neigh_xmit which is used for MPLS encapsulations was
> > not updated to use the altered key. The result is that every packet
> Tx
> > does a lookup on the gateway address which does not find an entry, a
> > new one is created only to find the existing one in the table right
> > before the insert since arp_constructor was updated to reset the
> > primary key. This is seen in the allocs and destroys counters:
> > ip -s -4 ntable show | head -10 | grep alloc
> >
> > which increase for each packet showing the unnecessary overhread.
> >
> > Fix by having neigh_xmit use __ipv4_neigh_lookup_noref for
> NEIGH_ARP_TABLE.
> >
> > Fixes: cd9ff4de0107 ("ipv4: Make neigh lookup keys for
> > loopback/point-to-point devices be INADDR_ANY")
> > Reported-by: Alan Maguire <[email protected]>
> > Signed-off-by: David Ahern <[email protected]>
> > Tested-by: Alan Maguire <[email protected]>
> > Signed-off-by: David S. Miller <[email protected]>
> > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > ---
>
> This commit also requires the following commit:
>
> commit 9b3040a6aafd7898ece7fc7efcbca71e42aa8069
> Author: David Ahern <[email protected]>
> Date: Sun May 5 11:16:20 2019 -0700
>
> ipv4: Define __ipv4_neigh_lookup_noref when CONFIG_INET is disabled
>
> Define __ipv4_neigh_lookup_noref to return NULL when CONFIG_INET is
> disabled.
>
> Fixes: 4b2a2bfeb3f0 ("neighbor: Call __ipv4_neigh_lookup_noref in
> neigh_xmit")
> Reported-by: kbuild test robot <[email protected]>
> Signed-off-by: David Ahern <[email protected]>
> Signed-off-by: David S. Miller <[email protected]>
>
> And this is also necessary for 4.4.y, 4.14.y, 4.19.y and 5.1.y.

4.4.y, 4.9.y, 4.19.y and 5.1.y.

> Please apply this commit.
>
Best regards,
Nobuhiro

2019-06-10 05:57:18

by Greg Kroah-Hartman

[permalink] [raw]
Subject: Re: [PATCH 4.14 03/35] neighbor: Call __ipv4_neigh_lookup_noref in neigh_xmit

On Mon, Jun 10, 2019 at 01:13:16AM +0000, [email protected] wrote:
> Hi again.
>
> > -----Original Message-----
> > From: [email protected]
> > [mailto:[email protected]] On Behalf Of Nobuhiro Iwamatsu
> > Sent: Monday, June 10, 2019 10:10 AM
> > To: Greg Kroah-Hartman <[email protected]>
> > Cc: [email protected]; [email protected]; Alan Maguire
> > <[email protected]>; David Ahern <[email protected]>; David S.
> > Miller <[email protected]>
> > Subject: Re: [PATCH 4.14 03/35] neighbor: Call __ipv4_neigh_lookup_noref
> > in neigh_xmit
> >
> > Hi,
> >
> > On Sun, Jun 09, 2019 at 06:42:09PM +0200, Greg Kroah-Hartman wrote:
> > > From: David Ahern <[email protected]>
> > >
> > > [ Upstream commit 4b2a2bfeb3f056461a90bd621e8bd7d03fa47f60 ]
> > >
> > > Commit cd9ff4de0107 changed the key for IFF_POINTOPOINT devices to
> > > INADDR_ANY but neigh_xmit which is used for MPLS encapsulations was
> > > not updated to use the altered key. The result is that every packet
> > Tx
> > > does a lookup on the gateway address which does not find an entry, a
> > > new one is created only to find the existing one in the table right
> > > before the insert since arp_constructor was updated to reset the
> > > primary key. This is seen in the allocs and destroys counters:
> > > ip -s -4 ntable show | head -10 | grep alloc
> > >
> > > which increase for each packet showing the unnecessary overhread.
> > >
> > > Fix by having neigh_xmit use __ipv4_neigh_lookup_noref for
> > NEIGH_ARP_TABLE.
> > >
> > > Fixes: cd9ff4de0107 ("ipv4: Make neigh lookup keys for
> > > loopback/point-to-point devices be INADDR_ANY")
> > > Reported-by: Alan Maguire <[email protected]>
> > > Signed-off-by: David Ahern <[email protected]>
> > > Tested-by: Alan Maguire <[email protected]>
> > > Signed-off-by: David S. Miller <[email protected]>
> > > Signed-off-by: Greg Kroah-Hartman <[email protected]>
> > > ---
> >
> > This commit also requires the following commit:
> >
> > commit 9b3040a6aafd7898ece7fc7efcbca71e42aa8069
> > Author: David Ahern <[email protected]>
> > Date: Sun May 5 11:16:20 2019 -0700
> >
> > ipv4: Define __ipv4_neigh_lookup_noref when CONFIG_INET is disabled
> >
> > Define __ipv4_neigh_lookup_noref to return NULL when CONFIG_INET is
> > disabled.
> >
> > Fixes: 4b2a2bfeb3f0 ("neighbor: Call __ipv4_neigh_lookup_noref in
> > neigh_xmit")
> > Reported-by: kbuild test robot <[email protected]>
> > Signed-off-by: David Ahern <[email protected]>
> > Signed-off-by: David S. Miller <[email protected]>
> >
> > And this is also necessary for 4.4.y, 4.14.y, 4.19.y and 5.1.y.
>
> 4.4.y, 4.9.y, 4.19.y and 5.1.y.

Thanks for the information, now queued up everywhere.

greg k-h