netdev - Re: [PATCH net-next 2/2] datagram, udp: Set local address and rehash socket atomically against lookup

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]

Message-ID: <CANn89i+PCsOHvd02nvM0oRjAXxPTgX6V1Y1-xfRL_43Ew9=H=w@mail.gmail.com>
Date: Thu, 5 Dec 2024 23:52:38 +0100
From: Eric Dumazet <edumazet@...gle.com>
To: David Gibson <david@...son.dropbear.id.au>
Cc: Stefano Brivio <sbrivio@...hat.com>, Willem de Bruijn <willemdebruijn.kernel@...il.com>, 
	netdev@...r.kernel.org, Kuniyuki Iwashima <kuniyu@...zon.com>, 
	Mike Manning <mvrmanning@...il.com>, Paul Holzinger <pholzing@...hat.com>, 
	Philo Lu <lulie@...ux.alibaba.com>, Cambda Zhu <cambda@...ux.alibaba.com>, 
	Fred Chen <fred.cc@...baba-inc.com>, Yubing Qiu <yubing.qiuyubing@...baba-inc.com>
Subject: Re: [PATCH net-next 2/2] datagram, udp: Set local address and rehash
 socket atomically against lookup

On Thu, Dec 5, 2024 at 11:32 PM David Gibson
<david@...son.dropbear.id.au> wrote:
>
> On Thu, Dec 05, 2024 at 05:35:52PM +0100, Eric Dumazet wrote:
> > On Wed, Dec 4, 2024 at 11:12 PM Stefano Brivio <sbrivio@...hat.com> wrote:
> [snip]
> > > diff --git a/net/ipv4/udp.c b/net/ipv4/udp.c
> > > index 6a01905d379f..8490408f6009 100644
> > > --- a/net/ipv4/udp.c
> > > +++ b/net/ipv4/udp.c
> > > @@ -639,18 +639,21 @@ struct sock *__udp4_lib_lookup(const struct net *net, __be32 saddr,
> > >                 int sdif, struct udp_table *udptable, struct sk_buff *skb)
> > >  {
> > >         unsigned short hnum = ntohs(dport);
> > > -       struct udp_hslot *hslot2;
> > > +       struct udp_hslot *hslot, *hslot2;
> > >         struct sock *result, *sk;
> > >         unsigned int hash2;
> > >
> > > +       hslot = udp_hashslot(udptable, net, hnum);
> > > +       spin_lock_bh(&hslot->lock);
> >
> > This is not acceptable.
> > UDP is best effort, packets can be dropped.
> > Please fix user application expectations.
>
> The packets aren't merely dropped, they're rejected with an ICMP Port
> Unreachable.

We made UDP stack scalable with RCU, it took years of work.

And this patch is bringing back the UDP stack to horrible performance
from more than a decade ago.
Everybody will go back to DPDK.

I am pretty certain this can be solved without using a spinlock in the
fast path.

Think about UDP DNS/QUIC servers, using SO_REUSEPORT and receiving
10,000,000 packets per second....

Changing source address on an UDP socket is highly unusual, we are not
going to slow down UDP for this case.

Application could instead open another socket, and would probably work
on old linux versions.

If the regression was recent, this would be considered as a normal regression,
but apparently nobody noticed for 10 years. This should be saying something...