Re: inet_hash_connect: source port allocation

Eric Dumazet
2010-11-29T19:07:40Z

Le lundi 29 novembre 2010 à 19:46 +0100, Eric Dumazet a écrit :
> Le lundi 29 novembre 2010 à 18:29 +0000, John Haxby a écrit :
>
> > Sorry, I think I phrased my question badly.
> >
> > inet_csk_get_port() starts its search for a free port with
> >
> > smallest_rover = rover = net_random() % remaining + low;
> >
> > whereas __inet_hash_connect() basically misses out that call to
> > net_random() so you get a predictable port number.
> >
> > Is there any good reason why that is the case?
> >
>
> It seems random select was done at bind() time only in commit
> 6df716340da3a6f ([TCP/DCCP]: Randomize port selection)
>
> It probably should be done in autobind too.
>
>

I'll test following patch :

diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
index 1b344f3..65c3702 100644
--- a/net/ipv4/inet_hashtables.c
+++ b/net/ipv4/inet_hashtables.c
@@ -466,20 +466,18 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
int twrefcnt = 1;

if (!snum) {
- int i, remaining, low, high, port;
- static u32 hint;
- u32 offset = hint + port_offset;
+ int remaining, low, high, port;
struct hlist_node *node;
struct inet_timewait_sock *tw = NULL;

inet_get_local_port_range(&low, &high);
remaining = (high - low) + 1;
+ port = net_random() % remaining + low;

local_bh_disable();
- for (i = 1; i <= remaining; i++) {
- port = low + (i + offset) % remaining;
+ do {
if (inet_is_reserved_local_port(port))
- continue;
+ goto next_nolock;
head = &hinfo->bhash[inet_bhashfn(net, port,
hinfo->bhash_size)];
spin_lock(&head->lock);
@@ -510,16 +508,17 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
tb->fastreuse = -1;
goto ok;

- next_port:
+next_port:
spin_unlock(&head->lock);
- }
+next_nolock:
+ if (++port > high)
+ port = low;
+ } while (--remaining > 0);
local_bh_enable();

return -EADDRNOTAVAIL;

ok:
- hint += i;
-
/* Head lock still held and bh's disabled */
inet_bind_hash(sk, tb, port);
if (sk_unhashed(sk)) {