netdev - Re: [PATCH] tcp: avoid unnecessary loop if even ports are used up

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 20 Feb 2021 13:44:02 -0800 (PST)
From:   David Miller <davem@...emloft.net>
To:     redsky110@...il.com
Cc:     edumazet@...gle.com, netdev@...r.kernel.org
Subject: Re: [PATCH] tcp: avoid unnecessary loop if even ports are used up

From: Honglei Wang <redsky110@...il.com>
Date: Sat, 20 Feb 2021 19:03:56 +0800

> We are getting port for connect() from even ports firstly now. This
> makes bind() users have more available slots at odd part. But there is a
> problem here when the even ports are used up. This happens when there
> is a flood of short life cycle connections. In this scenario, it starts
> getting ports from the odd part, but each requirement has to walk all of
> the even port and the related hash buckets (it probably gets nothing
> before the workload pressure's gone) before go to the odd part. This
> makes the code path __inet_hash_connect()->__inet_check_established()
> and the locks there hot.
> 
> This patch tries to improve the strategy so we can go faster when the
> even part is used up. It'll record the last gotten port was odd or even,
> if it's an odd one, it means there is no available even port for us and
> we probably can't get an even port this time, neither. So we just walk
> 1/16 of the whole even ports. If we can get one in this way, it probably
> means there are more available even part, we'll go back to the old
> strategy and walk all of them when next connect() comes. If still can't
> get even port in the 1/16 part, we just go to the odd part directly and
> avoid doing unnecessary loop.
> 
> Signed-off-by: Honglei Wang <redsky110@...il.com>
> ---
>  net/ipv4/inet_hashtables.c | 21 +++++++++++++++++++--
>  1 file changed, 19 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/inet_hashtables.c b/net/ipv4/inet_hashtables.c
> index 45fb450b4522..c95bf5cf9323 100644
> --- a/net/ipv4/inet_hashtables.c
> +++ b/net/ipv4/inet_hashtables.c
> @@ -721,9 +721,10 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
>  	struct net *net = sock_net(sk);
>  	struct inet_bind_bucket *tb;
>  	u32 remaining, offset;
> -	int ret, i, low, high;
> +	int ret, i, low, high, span;
>  	static u32 hint;
>  	int l3mdev;
> +	static bool last_port_is_odd;
>  
>  	if (port) {
>  		head = &hinfo->bhash[inet_bhashfn(net, port,
> @@ -756,8 +757,19 @@ int __inet_hash_connect(struct inet_timewait_death_row *death_row,
>  	 */
>  	offset &= ~1U;
>  other_parity_scan:
> +	/* If the last available port is odd, it means
> +	 * we walked all of the even ports, but got
> +	 * nothing last time. It's telling us the even
> +	 * part is busy to get available port. In this
> +	 * case, we can go a bit faster.
> +	 */
> +	if (last_port_is_odd && !(offset & 1) && remaining > 32)

The first time this executes, won't last_port_is_odd be uninitialized?