__inet_hash_connect port_offset parameter

[Crestez Dan Leonard <cdleonard@gmail.com>]

Hello,

I'm confused about the port_offset parameter to __inet_hash_connect.

When allocating the local port for an outgoing TCP connection the port search looks something like this:

    static u32 hint;
    u32 offset = hint + port_offset;

    inet_get_local_port_range(net, &low, &high);
    remaining = (high - low) + 1;

    for (i = 1; i <= remaining; i++) {
        port = low + (i + offset) % remaining;
        /* check port is free */

The port_offset is calculated for v4 and v6 based on a hash of src/dst addresses, presumably in order to improve security.

I see a few issues with this:
 - The port_offset is calculated even if the local port was already assigned via bind. This wastes a few cycles.
 - Keeping the last searched port as a static variable is a bad idea on multicore cpus. Starting a lot of connections to the same target will result in lock contention in the bind hash. This is probably only visible in highly synthetic tests.
 - When doing a port search at bind() time the search starts from "prandom_32()". Is this "less secure" for port allocation? I bet most applications are not aware of this difference.

Wouldn't it be better to use the same local port search mechanism at both bind (inet_csk_get_port) and connect (__inet_hash_connect) time, based on starting from a random point? It would also make connecting slightly faster.

Regards,
Leonard