Re: [PATCH net-next v2 1/2] tcp: Consider every port when connecting with IP_LOCAL_PORT_RANGE

[Jakub Sitnicki <jakub@cloudflare.com>]

On Tue, Jul 08, 2025 at 04:38 AM -07, Eric Dumazet wrote:
> On Thu, Jul 3, 2025 at 8:59 AM Jakub Sitnicki <jakub@cloudflare.com> wrote:

[...]

>> @@ -1070,6 +1107,8 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
>>                         if (!inet_bind_bucket_match(tb, net, port, l3mdev))
>>                                 continue;
>>                         if (tb->fastreuse >= 0 || tb->fastreuseport >= 0) {
>> +                               if (unlikely(local_ports))
>> +                                       break; /* optimistic assumption */
>
> I find this quite pessimistic :/
>
> It seems you had some internal code before my recent change (86c2bc293b8130
> "tcp: use RCU lookup in __inet_hash_connect()") ?
>
> Instead, make the RCU changes so that check_bound() can be called from RCU,
> and call it here before taking the decision to break off this loop.

Thanks for taking a look. I appreciate it.

That was intentional. Perhaps a bad call on my side.

My thinking here was that since we're already short on ephemeral ports
when these "blocked buckets" become an issue, then I wouldn't want to
dismiss a potential port in case the socket blocking it from reuse due
to src (IP, port) conflict goes away before we get to the second
(locked) phase of the search.

But then again, in hindsight, if we run into ephemeral port depletion,
then we must be under pressure from outgoing connections, so it seems
like a bad idea to put more stress on the bucket lock.

I will rework it as suggested.

>>                                 rcu_read_unlock();
>>                                 goto next_port;
>>                         }

[...]