* [PATCH net-next v2] tcp: bind() use stronger condition for bind_conflict
@ 2012-04-06 7:47 Alexandru Copot
2012-04-13 5:36 ` Daniel Baluta
2012-04-13 7:30 ` Eric Dumazet
0 siblings, 2 replies; 7+ messages in thread
From: Alexandru Copot @ 2012-04-06 7:47 UTC (permalink / raw)
To: davem, eric.dumazet
Cc: fbl, kuznet, jmorris, yoshfuji, kaber, netdev, Alex Copot, Daniel Baluta
From: Alex Copot <alex.mihai.c@gmail.com>
We must try harder to get unique (addr, port) pairs when
doing port autoselection for sockets with SO_REUSEADDR
option set.
We achieve this by adding a relaxation parameter to
inet_csk_bind_conflict. When 'relax' parameter is off
we return a conflict whenever the current searched
pair (addr, port) is not unique.
This tries to address the problems reported in patch:
8d238b25b1ec22a73b1c2206f111df2faaff8285
Revert "tcp: bind() fix when many ports are bound"
Tests where ran for creating and binding(0) many sockets
on 100 IPs. The results are, on average:
* 60000 sockets, 600 ports / IP:
* 0.210 s, 620 (IP, port) duplicates without patch
* 0.219 s, no duplicates with patch
* 100000 sockets, 1000 ports / IP:
* 0.371 s, 1720 duplicates without patch
* 0.373 s, no duplicates with patch
* 200000 sockets, 2000 ports / IP:
* 0.766 s, 6900 duplicates without patch
* 0.768 s, no duplicates with patch
* 500000 sockets, 5000 ports / IP:
* 2.227 s, 41500 duplicates without patch
* 2.284 s, no duplicates with patch
Signed-off-by: Alex Copot <alex.mihai.c@gmail.com>
Signed-off-by: Daniel Baluta <dbaluta@ixiacom.com>
---
Changes from v1:
- Fixed coding style
- Replaced int flag with bool
- Added test results
---
include/net/inet6_connection_sock.h | 2 +-
include/net/inet_connection_sock.h | 4 ++--
net/ipv4/inet_connection_sock.c | 17 +++++++++++++----
net/ipv6/inet6_connection_sock.c | 2 +-
4 files changed, 17 insertions(+), 8 deletions(-)
diff --git a/include/net/inet6_connection_sock.h b/include/net/inet6_connection_sock.h
index 3207e58..1866a67 100644
--- a/include/net/inet6_connection_sock.h
+++ b/include/net/inet6_connection_sock.h
@@ -23,7 +23,7 @@ struct sock;
struct sockaddr;
extern int inet6_csk_bind_conflict(const struct sock *sk,
- const struct inet_bind_bucket *tb);
+ const struct inet_bind_bucket *tb, bool relax);
extern struct dst_entry* inet6_csk_route_req(struct sock *sk,
const struct request_sock *req);
diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
index dbf9aab..46c9e2c 100644
--- a/include/net/inet_connection_sock.h
+++ b/include/net/inet_connection_sock.h
@@ -60,7 +60,7 @@ struct inet_connection_sock_af_ops {
#endif
void (*addr2sockaddr)(struct sock *sk, struct sockaddr *);
int (*bind_conflict)(const struct sock *sk,
- const struct inet_bind_bucket *tb);
+ const struct inet_bind_bucket *tb, bool relax);
};
/** inet_connection_sock - INET connection oriented sock
@@ -245,7 +245,7 @@ extern struct request_sock *inet_csk_search_req(const struct sock *sk,
const __be32 raddr,
const __be32 laddr);
extern int inet_csk_bind_conflict(const struct sock *sk,
- const struct inet_bind_bucket *tb);
+ const struct inet_bind_bucket *tb, bool relax);
extern int inet_csk_get_port(struct sock *sk, unsigned short snum);
extern struct dst_entry* inet_csk_route_req(struct sock *sk,
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 19d66ce..13ef772 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -53,7 +53,7 @@ void inet_get_local_port_range(int *low, int *high)
EXPORT_SYMBOL(inet_get_local_port_range);
int inet_csk_bind_conflict(const struct sock *sk,
- const struct inet_bind_bucket *tb)
+ const struct inet_bind_bucket *tb, bool relax)
{
struct sock *sk2;
struct hlist_node *node;
@@ -79,6 +79,13 @@ int inet_csk_bind_conflict(const struct sock *sk,
sk2_rcv_saddr == sk_rcv_saddr(sk))
break;
}
+ if (!relax && reuse && sk2->sk_reuse &&
+ sk2->sk_state != TCP_LISTEN) {
+ const __be32 sk2_rcv_saddr = sk_rcv_saddr(sk2);
+ if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) ||
+ sk2_rcv_saddr == sk_rcv_saddr(sk))
+ break;
+ }
}
}
return node != NULL;
@@ -122,12 +129,13 @@ 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, false)) {
snum = smallest_rover;
goto tb_found;
}
}
- if (!inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) {
+ if (!inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb, false)) {
snum = rover;
goto tb_found;
}
@@ -178,12 +186,13 @@ tb_found:
goto success;
} else {
ret = 1;
- if (inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) {
+ if (inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb, true)) {
if (sk->sk_reuse && sk->sk_state != TCP_LISTEN &&
smallest_size != -1 && --attempts >= 0) {
spin_unlock(&head->lock);
goto again;
}
+
goto fail_unlock;
}
}
diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index 02dd203..e6cee52 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -28,7 +28,7 @@
#include <net/inet6_connection_sock.h>
int inet6_csk_bind_conflict(const struct sock *sk,
- const struct inet_bind_bucket *tb)
+ const struct inet_bind_bucket *tb, bool relax)
{
const struct sock *sk2;
const struct hlist_node *node;
--
1.7.9.6
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2] tcp: bind() use stronger condition for bind_conflict
2012-04-06 7:47 [PATCH net-next v2] tcp: bind() use stronger condition for bind_conflict Alexandru Copot
@ 2012-04-13 5:36 ` Daniel Baluta
2012-04-13 5:46 ` David Miller
2012-04-13 5:53 ` Eric Dumazet
2012-04-13 7:30 ` Eric Dumazet
1 sibling, 2 replies; 7+ messages in thread
From: Daniel Baluta @ 2012-04-13 5:36 UTC (permalink / raw)
To: alex.mihai.c, davem, eric.dumazet
Cc: fbl, kuznet, jmorris, yoshfuji, kaber, netdev
On Fri, Apr 6, 2012 at 10:47 AM, Alexandru Copot <alex.mihai.c@gmail.com> wrote:
> From: Alex Copot <alex.mihai.c@gmail.com>
>
> We must try harder to get unique (addr, port) pairs when
> doing port autoselection for sockets with SO_REUSEADDR
> option set.
>
> We achieve this by adding a relaxation parameter to
> inet_csk_bind_conflict. When 'relax' parameter is off
> we return a conflict whenever the current searched
> pair (addr, port) is not unique.
>
> This tries to address the problems reported in patch:
> 8d238b25b1ec22a73b1c2206f111df2faaff8285
> Revert "tcp: bind() fix when many ports are bound"
>
> Tests where ran for creating and binding(0) many sockets
> on 100 IPs. The results are, on average:
>
> * 60000 sockets, 600 ports / IP:
> * 0.210 s, 620 (IP, port) duplicates without patch
> * 0.219 s, no duplicates with patch
> * 100000 sockets, 1000 ports / IP:
> * 0.371 s, 1720 duplicates without patch
> * 0.373 s, no duplicates with patch
> * 200000 sockets, 2000 ports / IP:
> * 0.766 s, 6900 duplicates without patch
> * 0.768 s, no duplicates with patch
> * 500000 sockets, 5000 ports / IP:
> * 2.227 s, 41500 duplicates without patch
> * 2.284 s, no duplicates with patch
>
> Signed-off-by: Alex Copot <alex.mihai.c@gmail.com>
> Signed-off-by: Daniel Baluta <dbaluta@ixiacom.com>
>
> ---
> Changes from v1:
> - Fixed coding style
> - Replaced int flag with bool
> - Added test results
> ---
> include/net/inet6_connection_sock.h | 2 +-
> include/net/inet_connection_sock.h | 4 ++--
> net/ipv4/inet_connection_sock.c | 17 +++++++++++++----
> net/ipv6/inet6_connection_sock.c | 2 +-
> 4 files changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/include/net/inet6_connection_sock.h b/include/net/inet6_connection_sock.h
> index 3207e58..1866a67 100644
> --- a/include/net/inet6_connection_sock.h
> +++ b/include/net/inet6_connection_sock.h
> @@ -23,7 +23,7 @@ struct sock;
> struct sockaddr;
>
> extern int inet6_csk_bind_conflict(const struct sock *sk,
> - const struct inet_bind_bucket *tb);
> + const struct inet_bind_bucket *tb, bool relax);
>
> extern struct dst_entry* inet6_csk_route_req(struct sock *sk,
> const struct request_sock *req);
> diff --git a/include/net/inet_connection_sock.h b/include/net/inet_connection_sock.h
> index dbf9aab..46c9e2c 100644
> --- a/include/net/inet_connection_sock.h
> +++ b/include/net/inet_connection_sock.h
> @@ -60,7 +60,7 @@ struct inet_connection_sock_af_ops {
> #endif
> void (*addr2sockaddr)(struct sock *sk, struct sockaddr *);
> int (*bind_conflict)(const struct sock *sk,
> - const struct inet_bind_bucket *tb);
> + const struct inet_bind_bucket *tb, bool relax);
> };
>
> /** inet_connection_sock - INET connection oriented sock
> @@ -245,7 +245,7 @@ extern struct request_sock *inet_csk_search_req(const struct sock *sk,
> const __be32 raddr,
> const __be32 laddr);
> extern int inet_csk_bind_conflict(const struct sock *sk,
> - const struct inet_bind_bucket *tb);
> + const struct inet_bind_bucket *tb, bool relax);
> extern int inet_csk_get_port(struct sock *sk, unsigned short snum);
>
> extern struct dst_entry* inet_csk_route_req(struct sock *sk,
> diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
> index 19d66ce..13ef772 100644
> --- a/net/ipv4/inet_connection_sock.c
> +++ b/net/ipv4/inet_connection_sock.c
> @@ -53,7 +53,7 @@ void inet_get_local_port_range(int *low, int *high)
> EXPORT_SYMBOL(inet_get_local_port_range);
>
> int inet_csk_bind_conflict(const struct sock *sk,
> - const struct inet_bind_bucket *tb)
> + const struct inet_bind_bucket *tb, bool relax)
> {
> struct sock *sk2;
> struct hlist_node *node;
> @@ -79,6 +79,13 @@ int inet_csk_bind_conflict(const struct sock *sk,
> sk2_rcv_saddr == sk_rcv_saddr(sk))
> break;
> }
> + if (!relax && reuse && sk2->sk_reuse &&
> + sk2->sk_state != TCP_LISTEN) {
> + const __be32 sk2_rcv_saddr = sk_rcv_saddr(sk2);
> + if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) ||
> + sk2_rcv_saddr == sk_rcv_saddr(sk))
> + break;
> + }
> }
> }
> return node != NULL;
> @@ -122,12 +129,13 @@ 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, false)) {
> snum = smallest_rover;
> goto tb_found;
> }
> }
> - if (!inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) {
> + if (!inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb, false)) {
> snum = rover;
> goto tb_found;
> }
> @@ -178,12 +186,13 @@ tb_found:
> goto success;
> } else {
> ret = 1;
> - if (inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb)) {
> + if (inet_csk(sk)->icsk_af_ops->bind_conflict(sk, tb, true)) {
> if (sk->sk_reuse && sk->sk_state != TCP_LISTEN &&
> smallest_size != -1 && --attempts >= 0) {
> spin_unlock(&head->lock);
> goto again;
> }
> +
> goto fail_unlock;
> }
> }
> diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
> index 02dd203..e6cee52 100644
> --- a/net/ipv6/inet6_connection_sock.c
> +++ b/net/ipv6/inet6_connection_sock.c
> @@ -28,7 +28,7 @@
> #include <net/inet6_connection_sock.h>
>
> int inet6_csk_bind_conflict(const struct sock *sk,
> - const struct inet_bind_bucket *tb)
> + const struct inet_bind_bucket *tb, bool relax)
> {
> const struct sock *sk2;
> const struct hlist_node *node;
> --
> 1.7.9.6
>
> --
Eric, David can you have a look on this?
thanks,
Daniel.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2] tcp: bind() use stronger condition for bind_conflict
2012-04-13 5:36 ` Daniel Baluta
@ 2012-04-13 5:46 ` David Miller
2012-04-13 5:53 ` Eric Dumazet
1 sibling, 0 replies; 7+ messages in thread
From: David Miller @ 2012-04-13 5:46 UTC (permalink / raw)
To: daniel.baluta
Cc: alex.mihai.c, eric.dumazet, fbl, kuznet, jmorris, yoshfuji,
kaber, netdev
From: Daniel Baluta <daniel.baluta@gmail.com>
Date: Fri, 13 Apr 2012 08:36:31 +0300
> Eric, David can you have a look on this?
It's in the patchwork queue as you can plainly see:
http://patchwork.ozlabs.org/patch/151099/
you therefore never, ever, need to ask me questions like this.
I just haven't had time to look at the patch, but when I do you'll be
the first to know.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2] tcp: bind() use stronger condition for bind_conflict
2012-04-13 5:36 ` Daniel Baluta
2012-04-13 5:46 ` David Miller
@ 2012-04-13 5:53 ` Eric Dumazet
2012-04-13 6:12 ` Daniel Baluta
1 sibling, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2012-04-13 5:53 UTC (permalink / raw)
To: Daniel Baluta
Cc: alex.mihai.c, davem, fbl, kuznet, jmorris, yoshfuji, kaber, netdev
On Fri, 2012-04-13 at 08:36 +0300, Daniel Baluta wrote:
> Eric, David can you have a look on this?
Hi Daniel
I can, but can you provide me the sources of the tests you ran ?
(I am kind of busy these days)
Thanks
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2] tcp: bind() use stronger condition for bind_conflict
2012-04-13 5:53 ` Eric Dumazet
@ 2012-04-13 6:12 ` Daniel Baluta
0 siblings, 0 replies; 7+ messages in thread
From: Daniel Baluta @ 2012-04-13 6:12 UTC (permalink / raw)
To: Eric Dumazet
Cc: alex.mihai.c, davem, fbl, kuznet, jmorris, yoshfuji, kaber, netdev
On Fri, Apr 13, 2012 at 8:53 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Fri, 2012-04-13 at 08:36 +0300, Daniel Baluta wrote:
>
>> Eric, David can you have a look on this?
>
>
> Hi Daniel
>
> I can, but can you provide me the sources of the tests you ran ?
>
> (I am kind of busy these days)
Sure. You can find the sources here [1].
* prepare-test.sh, sets up 100 interfaces.
* ./testcon -c <connections> -i <ips>, runs the test.
The results shown in patch description are obtained as follows:
* 60000 sockets, 600 ports / IP:
** ./testcon -c 60000 -i 100
* 100000 sockets, 1000 ports / IP:
** ./testcon -c 100000 -i 100
* 200000 sockets, 2000 ports / IP:
** ./testcon -c 200000 -i 100
* 500000 sockets, 5000 ports / IP:
** ./testcon -c 500000 -i 100
thanks,
Daniel.
[1] http://ixlabs.cs.pub.ro/gitweb/?p=port-allocation.git;a=tree;f=testbind;hb=master
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2] tcp: bind() use stronger condition for bind_conflict
2012-04-06 7:47 [PATCH net-next v2] tcp: bind() use stronger condition for bind_conflict Alexandru Copot
2012-04-13 5:36 ` Daniel Baluta
@ 2012-04-13 7:30 ` Eric Dumazet
2012-04-13 8:12 ` Alexandru Copot
1 sibling, 1 reply; 7+ messages in thread
From: Eric Dumazet @ 2012-04-13 7:30 UTC (permalink / raw)
To: Alexandru Copot
Cc: davem, fbl, kuznet, jmorris, yoshfuji, kaber, netdev, Daniel Baluta
Le vendredi 06 avril 2012 à 10:47 +0300, Alexandru Copot a écrit :
> From: Alex Copot <alex.mihai.c@gmail.com>
>
> We must try harder to get unique (addr, port) pairs when
> doing port autoselection for sockets with SO_REUSEADDR
> option set.
>
...
Excellent patch Alex !
Two minor coding style problems in this part :
> + if (!relax && reuse && sk2->sk_reuse &&
> + sk2->sk_state != TCP_LISTEN) {
> + const __be32 sk2_rcv_saddr = sk_rcv_saddr(sk2);
> + if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) ||
> + sk2_rcv_saddr == sk_rcv_saddr(sk))
> + break;
> + }
> }
Correct indentation, and one empty line after variable declaration :
if (!relax && reuse && sk2->sk_reuse &&
sk2->sk_state != TCP_LISTEN) {
const __be32 sk2_rcv_saddr = sk_rcv_saddr(sk2);
if (!sk2_rcv_saddr || !sk_rcv_saddr(sk) ||
sk2_rcv_saddr == sk_rcv_saddr(sk))
break;
}
if (condition_one ||
condition_two) {
foo();
instead of
if (condition_on ||
condition_two)
foo();
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH net-next v2] tcp: bind() use stronger condition for bind_conflict
2012-04-13 7:30 ` Eric Dumazet
@ 2012-04-13 8:12 ` Alexandru Copot
0 siblings, 0 replies; 7+ messages in thread
From: Alexandru Copot @ 2012-04-13 8:12 UTC (permalink / raw)
To: Eric Dumazet
Cc: davem, fbl, kuznet, jmorris, yoshfuji, kaber, netdev, Daniel Baluta
On Fri, Apr 13, 2012 at 10:30 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> Excellent patch Alex !
Thanks, Eric !
> Two minor coding style problems in this part :
> Correct indentation, and one empty line after variable declaration :
I will fix those. That empty line is missing because that's the way it
was in the code above my changes, but I will add it.
Should I send a v3 or reply here with the new patch ?
Alex Copot
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2012-04-13 8:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2012-04-06 7:47 [PATCH net-next v2] tcp: bind() use stronger condition for bind_conflict Alexandru Copot
2012-04-13 5:36 ` Daniel Baluta
2012-04-13 5:46 ` David Miller
2012-04-13 5:53 ` Eric Dumazet
2012-04-13 6:12 ` Daniel Baluta
2012-04-13 7:30 ` Eric Dumazet
2012-04-13 8:12 ` Alexandru Copot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.