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

[Jakub Sitnicki <jakub@cloudflare.com>]

On Thu, Jul 17, 2025 at 11:23 AM +02, Paolo Abeni wrote:
> On 7/14/25 6:03 PM, Jakub Sitnicki wrote:
>> Solution
>> --------
>> 
>> If there is no IP address conflict with any socket bound to a given local
>> port, then from the protocol's perspective, the port can be safely shared.
>> 
>> With that in mind, modify the port search during connect(), that is
>> __inet_hash_connect, to consider all bind buckets (ports) when looking for
>> a local port for egress.
>> 
>> To achieve this, add an extra walk over bhash2 buckets for the port to
>> check for IP conflicts. The additional walk is not free, so perform it only
>> once per port - during the second phase of conflict checking, when the
>> bhash bucket is locked.
>> 
>> We enable this changed behavior only if the IP_LOCAL_PORT_RANGE socket
>> option is set. The rationale is that users are likely to care about using
>> every possible local port only when they have deliberately constrained the
>> ephemeral port range.
>
> I'm not a big fan of piggybacking additional semantic on existing
> socketopt, have you considered a new one?

That's a fair point. Though a dedicated sysctl seems more appropriate in
this case. Akin to how we have ip_autobind_reuse to enable amore
aggresive port sharing strategy on bind() side. How does that sound?

>
> At very least you will need to update the man page.
>
>
>> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
>> index d3ce6d0a514e..9d8a9c7c8274 100644
>> --- a/net/ipv4/inet_hashtables.c
>> +++ b/net/ipv4/inet_hashtables.c
>> @@ -1005,6 +1005,52 @@ EXPORT_IPV6_MOD(inet_bhash2_reset_saddr);
>>  #define INET_TABLE_PERTURB_SIZE (1 << CONFIG_INET_TABLE_PERTURB_ORDER)
>>  static u32 *table_perturb;
>>  
>> +/* True on source address conflict with another socket. False otherwise. */
>> +static inline bool check_bind2_bucket(const struct sock *sk,
>> +				      const struct inet_bind2_bucket *tb2)
>
> Please no inline function in c files.

Will fix.

Thanks for feedback.