BPF Archive mirror
 help / color / mirror / Atom feed
* [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full
@ 2024-04-12 12:25 Zheng Li
  2024-04-15 18:07 ` Jakub Kicinski
  2024-04-15 18:15 ` Simon Horman
  0 siblings, 2 replies; 9+ messages in thread
From: Zheng Li @ 2024-04-12 12:25 UTC (permalink / raw
  To: netdev, bpf, kuba, davem, jmorris; +Cc: James.Z.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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full
  2024-04-12 12:25 Zheng Li
@ 2024-04-15 18:07 ` Jakub Kicinski
  2024-04-15 18:15 ` Simon Horman
  1 sibling, 0 replies; 9+ messages in thread
From: Jakub Kicinski @ 2024-04-15 18:07 UTC (permalink / raw
  To: Zheng Li; +Cc: netdev, bpf, davem, jmorris, James.Z.Li

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full
  2024-04-12 12:25 Zheng Li
  2024-04-15 18:07 ` Jakub Kicinski
@ 2024-04-15 18:15 ` Simon Horman
  1 sibling, 0 replies; 9+ messages in thread
From: Simon Horman @ 2024-04-15 18:15 UTC (permalink / raw
  To: Zheng Li; +Cc: netdev, bpf, kuba, davem, jmorris, James.Z.Li

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);
}



^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full
@ 2024-04-16  9:53 Zheng Li
  2024-04-16 10:02 ` Eric Dumazet
  2024-04-18 10:20 ` patchwork-bot+netdevbpf
  0 siblings, 2 replies; 9+ messages in thread
From: Zheng Li @ 2024-04-16  9:53 UTC (permalink / raw
  To: netdev, bpf, davem, jmorris, edumazet, pabeni, kuba; +Cc: James.Z.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


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full
  2024-04-16  9:53 [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full Zheng Li
@ 2024-04-16 10:02 ` Eric Dumazet
  2024-04-16 10:36   ` Li, James Zheng
  2024-04-18 10:20 ` patchwork-bot+netdevbpf
  1 sibling, 1 reply; 9+ messages in thread
From: Eric Dumazet @ 2024-04-16 10:02 UTC (permalink / raw
  To: Zheng Li; +Cc: netdev, bpf, davem, jmorris, pabeni, kuba, James.Z.Li

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

^ permalink raw reply	[flat|nested] 9+ messages in thread

* RE: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full
  2024-04-16 10:02 ` Eric Dumazet
@ 2024-04-16 10:36   ` Li, James Zheng
  2024-04-18  9:33     ` Paolo Abeni
  0 siblings, 1 reply; 9+ messages in thread
From: Li, James Zheng @ 2024-04-16 10:36 UTC (permalink / raw
  To: Eric Dumazet, Zheng Li
  Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, davem@davemloft.net,
	jmorris@namei.org, pabeni@redhat.com, kuba@kernel.org




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.




^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full
  2024-04-16 10:36   ` Li, James Zheng
@ 2024-04-18  9:33     ` Paolo Abeni
  2024-04-18  9:45       ` Eric Dumazet
  0 siblings, 1 reply; 9+ messages in thread
From: Paolo Abeni @ 2024-04-18  9:33 UTC (permalink / raw
  To: Li, James Zheng, Eric Dumazet, Zheng Li
  Cc: netdev@vger.kernel.org, bpf@vger.kernel.org, davem@davemloft.net,
	jmorris@namei.org, kuba@kernel.org

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full
  2024-04-18  9:33     ` Paolo Abeni
@ 2024-04-18  9:45       ` Eric Dumazet
  0 siblings, 0 replies; 9+ messages in thread
From: Eric Dumazet @ 2024-04-18  9:45 UTC (permalink / raw
  To: Paolo Abeni
  Cc: Li, James Zheng, Zheng Li, netdev@vger.kernel.org,
	bpf@vger.kernel.org, davem@davemloft.net, jmorris@namei.org,
	kuba@kernel.org

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>

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full
  2024-04-16  9:53 [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full Zheng Li
  2024-04-16 10:02 ` Eric Dumazet
@ 2024-04-18 10:20 ` patchwork-bot+netdevbpf
  1 sibling, 0 replies; 9+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-04-18 10:20 UTC (permalink / raw
  To: Zheng Li; +Cc: netdev, bpf, davem, jmorris, edumazet, pabeni, kuba, James.Z.Li

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



^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2024-04-18 10:20 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-16  9:53 [PATCH] neighbour: guarantee the localhost connections be established successfully even the ARP table is full Zheng Li
2024-04-16 10:02 ` Eric Dumazet
2024-04-16 10:36   ` Li, James Zheng
2024-04-18  9:33     ` Paolo Abeni
2024-04-18  9:45       ` Eric Dumazet
2024-04-18 10:20 ` patchwork-bot+netdevbpf
  -- strict thread matches above, loose matches on Subject: below --
2024-04-12 12:25 Zheng Li
2024-04-15 18:07 ` Jakub Kicinski
2024-04-15 18:15 ` Simon Horman

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).