'tcp: bind() fix when many ports are bound' problem

'tcp: bind() fix when many ports are bound' problem

[Daniel Baluta]

Hi,

After a series of discussions [1], Eric provided
"tcp: bind() fix when many ports are bound" patch. ([2])

Anyhow, due to this problem ([3]) it was reverted.
Where there any follow ups on this patch?

I have spent some time looking at inet_csk_get_port with the
only conclusion that it's scary :D.

Should I work around patch "tcp: bind() fix when many ports are bound",
and try to fix problem [3], or is that a dead end?

thanks,
Daniel.

[1] http://kerneltrap.org/mailarchive/linux-netdev/2010/4/20/6275120
[2] http://kerneltrap.org/mailarchive/git-commits-head/2010/4/24/32191
[3] http://kerneltrap.org/mailarchive/linux-kernel/2010/4/28/4563937

Re: 'tcp: bind() fix when many ports are bound' problem

[Gaspar Chilingarov]

Hi there!

Well, that looks strange.

On my own side I've just put workaround (manually binding to all ports
in sequence :)
and moved production code to FreeBSD as it has better scalable network stack.

I can see the potential problem with that bind() problem on highly
loaded DNS servers/resolvers which establish tons of outgoing UDP
connections.

In some cases that connections could fail and as not receiving the
answer it is normal condition for DNS this will go totally unnoticed.

I don't think anyone will hit this bug in production environment
except the very high load applications.

/Gaspar

2011/1/4 Daniel Baluta <daniel.baluta@gmail.com>:
> Hi,
>
> After a series of discussions [1], Eric provided
> "tcp: bind() fix when many ports are bound" patch. ([2])
>
> Anyhow, due to this problem ([3]) it was reverted.
> Where there any follow ups on this patch?
>
> I have spent some time looking at inet_csk_get_port with the
> only conclusion that it's scary :D.
>
> Should I work around patch "tcp: bind() fix when many ports are bound",
> and try to fix problem [3], or is that a dead end?
>
> thanks,
> Daniel.
>
> [1] http://kerneltrap.org/mailarchive/linux-netdev/2010/4/20/6275120
> [2] http://kerneltrap.org/mailarchive/git-commits-head/2010/4/24/32191
> [3] http://kerneltrap.org/mailarchive/linux-kernel/2010/4/28/4563937
>



-- 
Gaspar Chilingarov

tel +37493 419763 (mobile - leave voice mail message)
icq 63174784
skype://gasparch
e mailto:nm@web.am mailto:gasparch@gmail.com
w http://gasparchilingarov.com/

Re: 'tcp: bind() fix when many ports are bound' problem

[Eric Dumazet]

Le mardi 04 janvier 2011 à 13:12 +0400, Gaspar Chilingarov a écrit :
> Hi there!
> 
> Well, that looks strange.
> 
> On my own side I've just put workaround (manually binding to all ports
> in sequence :)
> and moved production code to FreeBSD as it has better scalable network stack.
> 
> I can see the potential problem with that bind() problem on highly
> loaded DNS servers/resolvers which establish tons of outgoing UDP
> connections.
> 
> In some cases that connections could fail and as not receiving the
> answer it is normal condition for DNS this will go totally unnoticed.
> 
> I don't think anyone will hit this bug in production environment
> except the very high load applications.

Dont mix TCP and UDP, they are not the same.

Problem with TCP is you can have TIME_WAIT sockets, disallowing a port
to be reused. Not with UDP.

The connect() [without a previous bind()], or a sendto() [without a
previous bind()] problem is more an API problem.

When kernel autobinds an UDP socket [to get a local IP/port], there is a
problem on the selection of the local address : It must be ANY_ADDR
(0.0.0.0)

While for TCP, the IP address wont change for the whole session.
Problem is : The port can really be random, while the local address
comes from routing tables. To reach one destination, we usually use one
pref IP address, even if many are available.

If you dont bind() a socket before sending an UDP frame, kernel cannot
assume the local IP address wont change later (for other sent frames, if
routing takes another path), so must use the ANY address for the port
selection done in autobind. Max 2^16-1 choices.

If you have 100 IP addresses on your machine, it doesnt change this ANY
selection [for UDP] at all.

If you need more than 2^16 local endpoints and you have more than one
external IP address, the only portable way is to use bind() yourself and
manage a pool of [tuples]. Well, this is not true for some old OSes
(Solaris 2.5.1 comes to mind with TCP sockets)

Re: 'tcp: bind() fix when many ports are bound' problem

[Daniel Baluta]

On Tue, Jan 4, 2011 at 1:22 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mardi 04 janvier 2011 à 13:12 +0400, Gaspar Chilingarov a écrit :
>> Hi there!
>>
>> Well, that looks strange.
>>
>> On my own side I've just put workaround (manually binding to all ports
>> in sequence :)
>> and moved production code to FreeBSD as it has better scalable network stack.
>>
>> I can see the potential problem with that bind() problem on highly
>> loaded DNS servers/resolvers which establish tons of outgoing UDP
>> connections.
>>
>> In some cases that connections could fail and as not receiving the
>> answer it is normal condition for DNS this will go totally unnoticed.
>>
>> I don't think anyone will hit this bug in production environment
>> except the very high load applications.
>
> Dont mix TCP and UDP, they are not the same.
>
> Problem with TCP is you can have TIME_WAIT sockets, disallowing a port
> to be reused. Not with UDP.

Isn't SO_REUSEADDR supposed to fix this problem?

Anyhow, inet_csk_get_port the function used by bind(0) performs bad.

Short reminder:
100 IP addr  - 70K sockets created => 700 sockets per IP.
bind(0) for all sockets will results in a large number of
(port, addr) duplicates although there are more than 30000 ports available.

This wouldn't be so bad if you won't try to connect duplicates to the same
remote (addr, port) which will result in a connect failure.

Eric's patch introduced the restriction 'forbid two reuse enabled sockets
to bind on same (addr, port) tuple with a (non ANY addr)'.

Well, I think this will break rule #2 from inet_hastable.h:
"
If all sockets have sk->sk_reuse set, and none of them are in
TCP_LISTEN state, the port may be shared.
"
and also caused problem ([1]), don't really know if they are the same.

An attempt, to fix this was to "always allow a reuse listen if
no other listen is already active on the same IP".

The problem with this fix, is that at the moment of bind() we
don't know what will be the usage of this socket. It can be,
bind -> connect or bind -> listen.
>
> The connect() [without a previous bind()], or a sendto() [without a
> previous bind()] problem is more an API problem.

Can you share your thoughts on this?

Going back to my first email, are there any follow ups on your
"tcp: bind() fix when many ports are bound" patch. I've searched
netdev archives but no luck. I might have missed something.

I really appreciate your help.

thanks,
Daniel.

[PATCH] tcp: disallow bind() to reuse addr/port

[Eric Dumazet]

Le mercredi 05 janvier 2011 à 11:00 +0200, Daniel Baluta a écrit :

> Going back to my first email, are there any follow ups on your
> "tcp: bind() fix when many ports are bound" patch. I've searched
> netdev archives but no luck. I might have missed something.
> 
> I really appreciate your help.
> 

I believe following patch should solve the problem.

Thanks for reminding us this issue !

I checked FreeBSD : It doesnt allow two sockets (in CLOSE state) bound
on same addr/port, even if both have REUSEADDR set



[PATCH] tcp: disallow bind() to reuse addr/port

inet_csk_bind_conflict() logic currently disallows a bind() if
it finds a friend socket (a socket bound on same address/port)
satisfying a set of conditions :

1) Current (to be bound) socket doesnt have sk_reuse set
OR
2) other socket doesnt have sk_reuse set
OR
3) other socket is in LISTEN state

We should add the CLOSE state in the 3) condition, in order to avoid two
REUSEADDR sockets in CLOSE state with same local address/port, since
this can deny further operations.

Note : a prior patch tried to address the problem in a different (and
buggy) way. (commit fda48a0d7a8412ced tcp: bind() fix when many ports
are bound).

Reported-by: Gaspar Chilingarov <gasparch@gmail.com>
Reported-by: Daniel Baluta <daniel.baluta@gmail.com>
Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
---
 net/ipv4/inet_connection_sock.c  |    5 +++--
 net/ipv6/inet6_connection_sock.c |    2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 25e3181..9f6d585 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -73,7 +73,7 @@ int inet_csk_bind_conflict(const struct sock *sk,
 		     !sk2->sk_bound_dev_if ||
 		     sk->sk_bound_dev_if == sk2->sk_bound_dev_if)) {
 			if (!reuse || !sk2->sk_reuse ||
-			    sk2->sk_state == TCP_LISTEN) {
+			    ((1 << sk2->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))) {
 				const __be32 sk2_rcv_saddr = sk_rcv_saddr(sk2);
 				if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) ||
 				    sk2_rcv_saddr == sk_rcv_saddr(sk))
@@ -122,7 +122,8 @@ again:
 					    (tb->num_owners < smallest_size || smallest_size == -1)) {
 						smallest_size = tb->num_owners;
 						smallest_rover = rover;
-						if (atomic_read(&hashinfo->bsockets) > (high - low) + 1) {
+						if (atomic_read(&hashinfo->bsockets) > (high - low) + 1 && 
+						    !inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) {
 							spin_unlock(&head->lock);
 							snum = smallest_rover;
 							goto have_snum;
diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index e46305d..d144e62 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -44,7 +44,7 @@ int inet6_csk_bind_conflict(const struct sock *sk,
 		     !sk2->sk_bound_dev_if ||
 		     sk->sk_bound_dev_if == sk2->sk_bound_dev_if) &&
 		    (!sk->sk_reuse || !sk2->sk_reuse ||
-		     sk2->sk_state == TCP_LISTEN) &&
+		     ((1 << sk2->sk_state) & (TCPF_LISTEN | TCPF_CLOSE))) &&
 		     ipv6_rcv_saddr_equal(sk, sk2))
 			break;
 	}

Re: [PATCH] tcp: disallow bind() to reuse addr/port

[Daniel Baluta]

On Tue, Jan 11, 2011 at 1:14 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Le mercredi 05 janvier 2011 à 11:00 +0200, Daniel Baluta a écrit :
>
>> Going back to my first email, are there any follow ups on your
>> "tcp: bind() fix when many ports are bound" patch. I've searched
>> netdev archives but no luck. I might have missed something.
>>
>> I really appreciate your help.
>>
>
> I believe following patch should solve the problem.

I have tested the patch and it works.

>
> Thanks for reminding us this issue !

thanks,
Daniel.

Re: [PATCH] tcp: disallow bind() to reuse addr/port

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Tue, 11 Jan 2011 12:14:22 +0100

> [PATCH] tcp: disallow bind() to reuse addr/port
> 
> inet_csk_bind_conflict() logic currently disallows a bind() if
> it finds a friend socket (a socket bound on same address/port)
> satisfying a set of conditions :
> 
> 1) Current (to be bound) socket doesnt have sk_reuse set
> OR
> 2) other socket doesnt have sk_reuse set
> OR
> 3) other socket is in LISTEN state
> 
> We should add the CLOSE state in the 3) condition, in order to avoid two
> REUSEADDR sockets in CLOSE state with same local address/port, since
> this can deny further operations.
> 
> Note : a prior patch tried to address the problem in a different (and
> buggy) way. (commit fda48a0d7a8412ced tcp: bind() fix when many ports
> are bound).
> 
> Reported-by: Gaspar Chilingarov <gasparch@gmail.com>
> Reported-by: Daniel Baluta <daniel.baluta@gmail.com>
> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>

Applied, thanks.

Re: [PATCH] tcp: disallow bind() to reuse addr/port

[George B.]

On Tue, Jan 11, 2011 at 2:03 PM, David Miller <davem@davemloft.net> wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Tue, 11 Jan 2011 12:14:22 +0100
>
>> [PATCH] tcp: disallow bind() to reuse addr/port
>>
>> inet_csk_bind_conflict() logic currently disallows a bind() if
>> it finds a friend socket (a socket bound on same address/port)
>> satisfying a set of conditions :
>>
>> 1) Current (to be bound) socket doesnt have sk_reuse set
>> OR
>> 2) other socket doesnt have sk_reuse set
>> OR
>> 3) other socket is in LISTEN state
>>
>> We should add the CLOSE state in the 3) condition, in order to avoid two
>> REUSEADDR sockets in CLOSE state with same local address/port, since
>> this can deny further operations.
>>
>> Note : a prior patch tried to address the problem in a different (and
>> buggy) way. (commit fda48a0d7a8412ced tcp: bind() fix when many ports
>> are bound).
>>
>> Reported-by: Gaspar Chilingarov <gasparch@gmail.com>
>> Reported-by: Daniel Baluta <daniel.baluta@gmail.com>
>> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
>
> Applied, thanks.
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>

OK, just saw this, so please disregard my earlier.

George

Re: [PATCH] tcp: disallow bind() to reuse addr/port

[Eric Dumazet]

Le mercredi 27 avril 2011 à 10:37 -0700, George B. a écrit :
> On Tue, Jan 11, 2011 at 2:03 PM, David Miller <davem@davemloft.net> wrote:
> > From: Eric Dumazet <eric.dumazet@gmail.com>
> > Date: Tue, 11 Jan 2011 12:14:22 +0100
> >
> >> [PATCH] tcp: disallow bind() to reuse addr/port
> >>
> >> inet_csk_bind_conflict() logic currently disallows a bind() if
> >> it finds a friend socket (a socket bound on same address/port)
> >> satisfying a set of conditions :
> >>
> >> 1) Current (to be bound) socket doesnt have sk_reuse set
> >> OR
> >> 2) other socket doesnt have sk_reuse set
> >> OR
> >> 3) other socket is in LISTEN state
> >>
> >> We should add the CLOSE state in the 3) condition, in order to avoid two
> >> REUSEADDR sockets in CLOSE state with same local address/port, since
> >> this can deny further operations.
> >>
> >> Note : a prior patch tried to address the problem in a different (and
> >> buggy) way. (commit fda48a0d7a8412ced tcp: bind() fix when many ports
> >> are bound).
> >>
> >> Reported-by: Gaspar Chilingarov <gasparch@gmail.com>
> >> Reported-by: Daniel Baluta <daniel.baluta@gmail.com>
> >> Signed-off-by: Eric Dumazet <eric.dumazet@gmail.com>
> >
> > Applied, thanks.
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> OK, just saw this, so please disregard my earlier.

Hmm... you'll discover this patch was reverted, because it broke some
applications.

So your problem remains.

Re: [PATCH] tcp: disallow bind() to reuse addr/port

[George B.]

>>
>> OK, just saw this, so please disregard my earlier.
>
> Hmm... you'll discover this patch was reverted, because it broke some
> applications.
>
> So your problem remains.

Just to clarify, both the previous patch from last year *AND* this
patch in January were reverted?  I can't seem to find anything showing
the new one being reverted and am now confused.

Re: [PATCH] tcp: disallow bind() to reuse addr/port

[Eric Dumazet]

Le mercredi 27 avril 2011 à 10:54 -0700, George B. a écrit :
> >>
> >> OK, just saw this, so please disregard my earlier.
> >
> > Hmm... you'll discover this patch was reverted, because it broke some
> > applications.
> >
> > So your problem remains.
> 
> Just to clarify, both the previous patch from last year *AND* this
> patch in January were reverted?  I can't seem to find anything showing
> the new one being reverted and am now confused.

Yes, all patches were reverted.

Last revert was very recent : 3e8c806a08c7beecd972e7ce15c

http://git.kernel.org/?p=linux/kernel/git/torvalds/linux-2.6.git;a=commit;h=3e8c806a08c7beecd972e7ce15c570b9aba64baa

Revert "tcp: disallow bind() to reuse addr/port"

This reverts commit c191a836a908d1dd6b40c503741f91b914de3348.

It causes known regressions for programs that expect to be able to use
SO_REUSEADDR to shutdown a socket, then successfully rebind another
socket to the same ID.

Programs such as haproxy and amavisd expect this to work.

This should fix kernel bugzilla 32832.

Re: [PATCH] tcp: disallow bind() to reuse addr/port

[George B.]

> It causes known regressions for programs that expect to be able to use
> SO_REUSEADDR to shutdown a socket, then successfully rebind another
> socket to the same ID.
>
> Programs such as haproxy and amavisd expect this to work.
>
> This should fix kernel bugzilla 32832.


Thank you very much for the clarification.  It just seems on the
surface like it should be a simple problem (don't they all, at
first?).  Instead of checking to see if we have more than the number
of ephemeral ports in use globally, see if we have more than that
number in use on the requested IP address.  The problem I am having is
if the number of ports in use globally is greater than the number of
configured ephemeral ports, I can't open a socket on a specific source
IP even though that IP has plenty of ports available.  It would seem
like a simple bounds checking problem.

Thanks again for taking the time to respond.

George

Re: 'tcp: bind() fix when many ports are bound' problem

[George B.]

On Wed, Jan 5, 2011 at 1:00 AM, Daniel Baluta <daniel.baluta@gmail.com> wrote:
>
> On Tue, Jan 4, 2011 at 1:22 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> > Le mardi 04 janvier 2011 à 13:12 +0400, Gaspar Chilingarov a écrit :
> >> Hi there!
> >>
> >> Well, that looks strange.
> >>
> >> On my own side I've just put workaround (manually binding to all ports
> >> in sequence :)
> >> and moved production code to FreeBSD as it has better scalable network stack.
> >>
> >> I can see the potential problem with that bind() problem on highly
> >> loaded DNS servers/resolvers which establish tons of outgoing UDP
> >> connections.
> >>
> >> In some cases that connections could fail and as not receiving the
> >> answer it is normal condition for DNS this will go totally unnoticed.
> >>
> >> I don't think anyone will hit this bug in production environment
> >> except the very high load applications.
> >
> > Dont mix TCP and UDP, they are not the same.
> >
> > Problem with TCP is you can have TIME_WAIT sockets, disallowing a port
> > to be reused. Not with UDP.
>
> Isn't SO_REUSEADDR supposed to fix this problem?
>
> Anyhow, inet_csk_get_port the function used by bind(0) performs bad.
>
> Short reminder:
> 100 IP addr  - 70K sockets created => 700 sockets per IP.
> bind(0) for all sockets will results in a large number of
> (port, addr) duplicates although there are more than 30000 ports available.
>
> This wouldn't be so bad if you won't try to connect duplicates to the same
> remote (addr, port) which will result in a connect failure.
>
> Eric's patch introduced the restriction 'forbid two reuse enabled sockets
> to bind on same (addr, port) tuple with a (non ANY addr)'.
>
> Well, I think this will break rule #2 from inet_hastable.h:
> "
> If all sockets have sk->sk_reuse set, and none of them are in
> TCP_LISTEN state, the port may be shared.
> "
> and also caused problem ([1]), don't really know if they are the same.
>
> An attempt, to fix this was to "always allow a reuse listen if
> no other listen is already active on the same IP".
>
> The problem with this fix, is that at the moment of bind() we
> don't know what will be the usage of this socket. It can be,
> bind -> connect or bind -> listen.
> >
> > The connect() [without a previous bind()], or a sendto() [without a
> > previous bind()] problem is more an API problem.
>
> Can you share your thoughts on this?
>
> Going back to my first email, are there any follow ups on your
> "tcp: bind() fix when many ports are bound" patch. I've searched
> netdev archives but no luck. I might have missed something.
>
> I really appreciate your help.
>
> thanks,
> Daniel.
> --

This is also causing a problem for me in a very high load application
where more than 64K sockets are being sourced from multiple IP
addresses.

I, too, would like to know if this has been followed up on.  The old
patch that was reverted was actually working well for us, we never
actually hit the TIME_WAIT problem but we are hitting the problem of
not being able to source a connection from an IP when the global
number of connections is >64K or so.