netdev - Re: [PATCH net-next v4 1/2] net: Add a second bind table hashed by port and address

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] [day] [month] [year] [list]

Date:   Tue, 17 May 2022 21:57:54 -0700
From:   Joanne Koong <joannelkoong@...il.com>
To:     Eric Dumazet <edumazet@...gle.com>
Cc:     netdev <netdev@...r.kernel.org>, Martin KaFai Lau <kafai@...com>,
        Jakub Kicinski <kuba@...nel.org>,
        David Miller <davem@...emloft.net>
Subject: Re: [PATCH net-next v4 1/2] net: Add a second bind table hashed by
 port and address

On Tue, May 17, 2022 at 9:59 AM Eric Dumazet <edumazet@...gle.com> wrote:
>
> On Thu, May 12, 2022 at 1:51 PM Joanne Koong <joannelkoong@...il.com> wrote:
> >
> > We currently have one tcp bind table (bhash) which hashes by port
> > number only. In the socket bind path, we check for bind conflicts by
> > traversing the specified port's inet_bind2_bucket while holding the
> > bucket's spinlock (see inet_csk_get_port() and inet_csk_bind_conflict()).
> >
> > In instances where there are tons of sockets hashed to the same port
> > at different addresses, checking for a bind conflict is time-intensive
> > and can cause softirq cpu lockups, as well as stops new tcp connections
> > since __inet_inherit_port() also contests for the spinlock.
> >
> > This patch proposes adding a second bind table, bhash2, that hashes by
> > port and ip address. Searching the bhash2 table leads to significantly
> > faster conflict resolution and less time holding the spinlock.
> >
> > Signed-off-by: Joanne Koong <joannelkoong@...il.com>
>
> Scary patch, but I could not find obvious issues with it.
> Let's give it a try, thanks !
>
> Reviewed-by: Eric Dumazet <edumazet@...gle.com>
Thanks for reviewing this code, Eric.

I will submit a v5 that tidies up some of the things flagged by
patchworks [1] (removing the inline in the "static inline bool
check_bind2_bucket_match(...)" function in net/ipv4/inet_hashtables.c
and adding line breaks to keep the lengths to 80 columns)

[1] https://patchwork.kernel.org/project/netdevbpf/patch/20220512205041.1208962-2-joannelkoong@gmail.com/