[PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[Zheng Li]

From: Zheng Li <James.Z.Li@Dell.com>

Inter-process communication on localhost should be established successfully even the ARP table is full,
many processes on server machine use the localhost to communicate such as command-line interface (CLI),
servers hope all CLI commands can be executed successfully even the arp table is full.
Right now CLI commands got timeout when the arp table is full.
Set the parameter of exempt_from_gc to be true for LOOPBACK net device to
keep localhost neigh in arp table, not removed by gc.

the steps of reproduced:
server with "gc_thresh3 = 1024" setting, ping server from more than 1024 IPv4 addresses,
run "ssh localhost" on console interface, then the command will get timeout.

Signed-off-by: Zheng Li <James.Z.Li@Dell.com>
---
 net/core/neighbour.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 552719c3bbc3..d96dee3d4af6 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -734,7 +734,10 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
 struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
 				 struct net_device *dev, bool want_ref)
 {
-	return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
+	if (dev->flags & IFF_LOOPBACK)
+		return ___neigh_create(tbl, pkey, dev, 0, true, want_ref);
+	else
+		return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
 }
 EXPORT_SYMBOL(__neigh_create);
 
-- 
2.17.1

Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[Jakub Kicinski]

On Fri, 12 Apr 2024 20:25:38 +0800 Zheng Li wrote:
> Inter-process communication on localhost should be established successfully even the ARP table is full,
> many processes on server machine use the localhost to communicate such as command-line interface (CLI),
> servers hope all CLI commands can be executed successfully even the arp table is full.
> Right now CLI commands got timeout when the arp table is full.
> Set the parameter of exempt_from_gc to be true for LOOPBACK net device to
> keep localhost neigh in arp table, not removed by gc.
> 
> the steps of reproduced:
> server with "gc_thresh3 = 1024" setting, ping server from more than 1024 IPv4 addresses,
> run "ssh localhost" on console interface, then the command will get timeout.

Please repost this and CC appropriate maintainers.

scripts/get_maintainer doesn't bite.
-- 
pw-bot: cr

Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[Simon Horman]

On Fri, Apr 12, 2024 at 08:25:38PM +0800, Zheng Li wrote:
> From: Zheng Li <James.Z.Li@Dell.com>
> 
> Inter-process communication on localhost should be established successfully even the ARP table is full,
> many processes on server machine use the localhost to communicate such as command-line interface (CLI),
> servers hope all CLI commands can be executed successfully even the arp table is full.
> Right now CLI commands got timeout when the arp table is full.
> Set the parameter of exempt_from_gc to be true for LOOPBACK net device to
> keep localhost neigh in arp table, not removed by gc.
> 
> the steps of reproduced:
> server with "gc_thresh3 = 1024" setting, ping server from more than 1024 IPv4 addresses,
> run "ssh localhost" on console interface, then the command will get timeout.
> 
> Signed-off-by: Zheng Li <James.Z.Li@Dell.com>
> ---
>  net/core/neighbour.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 552719c3bbc3..d96dee3d4af6 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -734,7 +734,10 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
>  struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
>  				 struct net_device *dev, bool want_ref)
>  {
> -	return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
> +	if (dev->flags & IFF_LOOPBACK)
> +		return ___neigh_create(tbl, pkey, dev, 0, true, want_ref);
> +	else
> +		return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
>  }
>  EXPORT_SYMBOL(__neigh_create);

Hi James,

I'm not commenting on the merits of your patch - I'm sure
the maintainers will do so when you repost as Jakub has suggested.

But wile looking at this it seemed to me that the following might
be a cleaner way to express your change.

struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
				 struct net_device *dev, bool want_ref)
{
	bool exempt_from_gc = !!(dev->flags & IFF_LOOPBACK);

	return ___neigh_create(tbl, pkey, dev, 0, exempt_from_gc,  want_ref);
}

[PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[Zheng Li]

From: Zheng Li <James.Z.Li@Dell.com>

Inter-process communication on localhost should be established successfully
even the ARP table is full, many processes on server machine use the
localhost to communicate such as command-line interface (CLI),
servers hope all CLI commands can be executed successfully even the arp
table is full. Right now CLI commands got timeout when the arp table is
full. Set the parameter of exempt_from_gc to be true for LOOPBACK net
device to keep localhost neigh in arp table, not removed by gc.

the steps of reproduced:
server with "gc_thresh3 = 1024" setting, ping server from more than 1024
same netmask Lan IPv4 addresses, run "ssh localhost" on console interface,
then the command will get timeout.

Signed-off-by: Zheng Li <James.Z.Li@Dell.com>
---
 net/core/neighbour.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 552719c3bbc3..47d07b122f7a 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -734,7 +734,9 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
 struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
 				 struct net_device *dev, bool want_ref)
 {
-	return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
+	bool exempt_from_gc = !!(dev->flags & IFF_LOOPBACK);
+
+	return ___neigh_create(tbl, pkey, dev, 0, exempt_from_gc, want_ref);
 }
 EXPORT_SYMBOL(__neigh_create);
 
-- 
2.17.1

Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[Eric Dumazet]

On Tue, Apr 16, 2024 at 11:54 AM Zheng Li <lizheng043@gmail.com> wrote:
>
> From: Zheng Li <James.Z.Li@Dell.com>
>
> Inter-process communication on localhost should be established successfully
> even the ARP table is full, many processes on server machine use the
> localhost to communicate such as command-line interface (CLI),
> servers hope all CLI commands can be executed successfully even the arp
> table is full. Right now CLI commands got timeout when the arp table is
> full. Set the parameter of exempt_from_gc to be true for LOOPBACK net
> device to keep localhost neigh in arp table, not removed by gc.
>
> the steps of reproduced:
> server with "gc_thresh3 = 1024" setting, ping server from more than 1024
> same netmask Lan IPv4 addresses, run "ssh localhost" on console interface,
> then the command will get timeout.
>
> Signed-off-by: Zheng Li <James.Z.Li@Dell.com>
> ---
>  net/core/neighbour.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 552719c3bbc3..47d07b122f7a 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -734,7 +734,9 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
>  struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
>                                  struct net_device *dev, bool want_ref)
>  {
> -       return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
> +       bool exempt_from_gc = !!(dev->flags & IFF_LOOPBACK);
> +
> +       return ___neigh_create(tbl, pkey, dev, 0, exempt_from_gc, want_ref);
>  }
>  EXPORT_SYMBOL(__neigh_create);
>

Hmmm...

Loopback IPv4 can hold 2^24 different addresses, that is 16384 * 1024

RE: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[Li, James Zheng]

Internal Use - Confidential
-----Original Message-----
From: Eric Dumazet <edumazet@google.com>
Sent: Tuesday, April 16, 2024 6:02 PM
To: Zheng Li <lizheng043@gmail.com>
Cc: netdev@vger.kernel.org; bpf@vger.kernel.org; davem@davemloft.net; jmorris@namei.org; pabeni@redhat.com; kuba@kernel.org; Li, James Zheng <James.Z.Li@Dell.com>
Subject: Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full


[EXTERNAL EMAIL]

On Tue, Apr 16, 2024 at 11:54 AM Zheng Li <lizheng043@gmail.com> wrote:
>
> From: Zheng Li <James.Z.Li@Dell.com>
>
> Inter-process communication on localhost should be established
> successfully even the ARP table is full, many processes on server
> machine use the localhost to communicate such as command-line
> interface (CLI), servers hope all CLI commands can be executed
> successfully even the arp table is full. Right now CLI commands got
> timeout when the arp table is full. Set the parameter of
> exempt_from_gc to be true for LOOPBACK net device to keep localhost neigh in arp table, not removed by gc.
>
> the steps of reproduced:
> server with "gc_thresh3 = 1024" setting, ping server from more than
> 1024 same netmask Lan IPv4 addresses, run "ssh localhost" on console
> interface, then the command will get timeout.
>
> Signed-off-by: Zheng Li <James.Z.Li@Dell.com>
> ---
>  net/core/neighbour.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c index
> 552719c3bbc3..47d07b122f7a 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -734,7 +734,9 @@ ___neigh_create(struct neigh_table *tbl, const
> void *pkey,  struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
>                                  struct net_device *dev, bool
> want_ref)  {
> -       return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
> +       bool exempt_from_gc = !!(dev->flags & IFF_LOOPBACK);
> +
> +       return ___neigh_create(tbl, pkey, dev, 0, exempt_from_gc,
> + want_ref);
>  }
>  EXPORT_SYMBOL(__neigh_create);
>

> Hmmm...

> Loopback IPv4 can hold 2^24 different addresses, that is 16384 * 1024

There is only one Loopback neigh "0.0.0.0 dev lo lladdr 00:00:00:00:00:00 NOARP" existing even you have configured 2^24 different addresses on the loopback device.

Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[Paolo Abeni]

On Tue, 2024-04-16 at 10:36 +0000, Li, James Zheng wrote:
> On Tuesday, April 16, 2024 6:02 PM Eric Dumazet <edumazet@google.com> wrote:
> > Hmmm...
> 
> > Loopback IPv4 can hold 2^24 different addresses, that is 16384 * 1024
> 
> There is only one Loopback neigh "0.0.0.0 dev lo lladdr 00:00:00:00:00:00 NOARP"
> existing even you have configured 2^24 different addresses on the loopback device.

Eric, I think James is right, in __ipv4_neigh_lookup_noref():

	if (dev->flags & (IFF_LOOPBACK | IFF_POINTOPOINT))
                key = INADDR_ANY;

	return ___neigh_lookup_noref(&arp_tbl, neigh_key_eq32, arp_hashfn, &key, dev);

So there should be at most one neigh entry over the loopback device.
The patch looks safe to me, am I missing something?

Thanks,

Paolo

Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[Eric Dumazet]

On Thu, Apr 18, 2024 at 11:33 AM Paolo Abeni <pabeni@redhat.com> wrote:
>
> On Tue, 2024-04-16 at 10:36 +0000, Li, James Zheng wrote:
> > On Tuesday, April 16, 2024 6:02 PM Eric Dumazet <edumazet@google.com> wrote:
> > > Hmmm...
> >
> > > Loopback IPv4 can hold 2^24 different addresses, that is 16384 * 1024
> >
> > There is only one Loopback neigh "0.0.0.0 dev lo lladdr 00:00:00:00:00:00 NOARP"
> > existing even you have configured 2^24 different addresses on the loopback device.
>
> Eric, I think James is right, in __ipv4_neigh_lookup_noref():
>
>         if (dev->flags & (IFF_LOOPBACK | IFF_POINTOPOINT))
>                 key = INADDR_ANY;
>
>         return ___neigh_lookup_noref(&arp_tbl, neigh_key_eq32, arp_hashfn, &key, dev);
>
> So there should be at most one neigh entry over the loopback device.
> The patch looks safe to me, am I missing something?

This seems fine, thanks.

It is unfortunate ip command does not seem to display these
neighbours, for some reason.

(I am about to send a series of three patches to remove RTNL from "ip
neighbour show")

Reviewed-by: Eric Dumazet <edumazet@google.com>

Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net-next.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 16 Apr 2024 17:53:43 +0800 you wrote:
> From: Zheng Li <James.Z.Li@Dell.com>
> 
> Inter-process communication on localhost should be established successfully
> even the ARP table is full, many processes on server machine use the
> localhost to communicate such as command-line interface (CLI),
> servers hope all CLI commands can be executed successfully even the arp
> table is full. Right now CLI commands got timeout when the arp table is
> full. Set the parameter of exempt_from_gc to be true for LOOPBACK net
> device to keep localhost neigh in arp table, not removed by gc.
> 
> [...]

Here is the summary with links:
  - neighbour: guarantee the localhost connections be established successfully even the ARP table is full
    https://git.kernel.org/netdev/net-next/c/eabf425bc6ad

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

[PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[Zheng Li]

From: Zheng Li <James.Z.Li@Dell.com>

Inter-process communication on localhost should be established successfully even the ARP table is full,
many processes on server machine use the localhost to communicate such as command-line interface (CLI),
servers hope all CLI commands can be executed successfully even the arp table is full.
Right now CLI commands got timeout when the arp table is full.
Set the parameter of exempt_from_gc to be true for LOOPBACK net device to
keep localhost neigh in arp table, not removed by gc.

the steps of reproduced:
server with "gc_thresh3 = 1024" setting, ping server from more than 1024 IPv4 addresses,
run "ssh localhost" on console interface, then the command will get timeout.

Signed-off-by: Zheng Li <James.Z.Li@Dell.com>
---
 net/core/neighbour.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/core/neighbour.c b/net/core/neighbour.c
index 552719c3bbc3..d96dee3d4af6 100644
--- a/net/core/neighbour.c
+++ b/net/core/neighbour.c
@@ -734,7 +734,10 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
 struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
 				 struct net_device *dev, bool want_ref)
 {
-	return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
+	if (dev->flags & IFF_LOOPBACK)
+		return ___neigh_create(tbl, pkey, dev, 0, true, want_ref);
+	else
+		return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
 }
 EXPORT_SYMBOL(__neigh_create);
 
-- 
2.17.1

Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[Ratheesh Kannoth]

On 2024-03-11 at 17:54:01, Zheng Li (lizheng043@gmail.com) wrote:
>
> Inter-process communication on localhost should be established successfully even the ARP table is full,
> many processes on server machine use the localhost to communicate such as command-line interface (CLI),
> servers hope all CLI commands can be executed successfully even the arp table is full.
> Right now CLI commands got timeout when the arp table is full.
> Set the parameter of exempt_from_gc to be true for LOOPBACK net device to
> keep localhost neigh in arp table, not removed by gc.
>
> the steps of reproduced:
> server with "gc_thresh3 = 1024" setting, ping server from more than 1024 IPv4 addresses,
> run "ssh localhost" on console interface, then the command will get timeout.
it does not look correct to me. why gc has to behave differently for loopback devices.
why can't a higher gc_thresh3 value (fine tuned to your use case) wont solve the issue ?
can't you add localhost arp entry statically and get rid of gc issue ?

>
> Signed-off-by: Zheng Li <James.Z.Li@Dell.com>
> ---
>  net/core/neighbour.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 552719c3bbc3..d96dee3d4af6 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -734,7 +734,10 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
>  struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
>  				 struct net_device *dev, bool want_ref)
>  {
> -	return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
> +	if (dev->flags & IFF_LOOPBACK)
> +		return ___neigh_create(tbl, pkey, dev, 0, true, want_ref);
> +	else
> +		return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
>  }
>  EXPORT_SYMBOL(__neigh_create);
>
> --
> 2.17.1
>

Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full

[James Lee]

loopback neigh is a special device in the neighbour system which is
used by all local communications and state is NUD_NOARP.
Any setting value of gc_thresh3 might encounter arp table be full,
manually increasing gc_thresh3 can resolve this issue for every time,
but we hope this issue automatically be resolved in Linux kernel for
all local communications whenever ARP table is full, rather than
manually operation as a workaround.


Ratheesh Kannoth <rkannoth@marvell.com> 于2024年3月11日周一 21:51写道：
>
> On 2024-03-11 at 17:54:01, Zheng Li (lizheng043@gmail.com) wrote:
> >
> > Inter-process communication on localhost should be established successfully even the ARP table is full,
> > many processes on server machine use the localhost to communicate such as command-line interface (CLI),
> > servers hope all CLI commands can be executed successfully even the arp table is full.
> > Right now CLI commands got timeout when the arp table is full.
> > Set the parameter of exempt_from_gc to be true for LOOPBACK net device to
> > keep localhost neigh in arp table, not removed by gc.
> >
> > the steps of reproduced:
> > server with "gc_thresh3 = 1024" setting, ping server from more than 1024 IPv4 addresses,
> > run "ssh localhost" on console interface, then the command will get timeout.
> it does not look correct to me. why gc has to behave differently for loopback devices.
> why can't a higher gc_thresh3 value (fine tuned to your use case) wont solve the issue ?
> can't you add localhost arp entry statically and get rid of gc issue ?
>
> >
> > Signed-off-by: Zheng Li <James.Z.Li@Dell.com>
> > ---
> >  net/core/neighbour.c | 5 ++++-
> >  1 file changed, 4 insertions(+), 1 deletion(-)
> >
> > diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> > index 552719c3bbc3..d96dee3d4af6 100644
> > --- a/net/core/neighbour.c
> > +++ b/net/core/neighbour.c
> > @@ -734,7 +734,10 @@ ___neigh_create(struct neigh_table *tbl, const void *pkey,
> >  struct neighbour *__neigh_create(struct neigh_table *tbl, const void *pkey,
> >                                struct net_device *dev, bool want_ref)
> >  {
> > -     return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
> > +     if (dev->flags & IFF_LOOPBACK)
> > +             return ___neigh_create(tbl, pkey, dev, 0, true, want_ref);
> > +     else
> > +             return ___neigh_create(tbl, pkey, dev, 0, false, want_ref);
> >  }
> >  EXPORT_SYMBOL(__neigh_create);
> >
> > --
> > 2.17.1
> >