[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20230621150058.59250-1-kuniyu@amazon.com>
Date: Wed, 21 Jun 2023 08:00:58 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <lmb@...valent.com>
CC: <andrii@...nel.org>, <ast@...nel.org>, <bpf@...r.kernel.org>,
<daniel@...earbox.net>, <davem@...emloft.net>,
<dsahern@...nel.org>, <edumazet@...gle.com>, <haoluo@...gle.com>,
<hemanthmalla@...il.com>, <joe@...d.net.nz>,
<john.fastabend@...il.com>, <jolsa@...nel.org>,
<kpsingh@...nel.org>, <kuba@...nel.org>, <kuniyu@...zon.com>,
<linux-kernel@...r.kernel.org>, <linux-kselftest@...r.kernel.org>,
<martin.lau@...ux.dev>, <mykolal@...com>, <netdev@...r.kernel.org>,
<pabeni@...hat.com>, <sdf@...gle.com>, <shuah@...nel.org>,
<song@...nel.org>, <willemdebruijn.kernel@...il.com>, <yhs@...com>
Subject: Re: [PATCH bpf-next v2 3/6] net: remove duplicate reuseport_lookup functions
From: Lorenz Bauer <lmb@...valent.com>
Date: Wed, 21 Jun 2023 09:01:15 +0100
> On Tue, Jun 20, 2023 at 7:31 PM Kuniyuki Iwashima <kuniyu@...zon.com> wrote:
> >
> > Good point. This is based on an assumption that all SO_REUSEPORT
> > sockets have the same score, which is wrong for two corner cases
> > if reuseport_has_conns() == true :
> >
> > 1) SO_INCOMING_CPU is set
> > -> selected sk might have +1 score
> >
> > 2) BPF prog returns ESTABLISHED and/or SO_INCOMING_CPU sk
> > -> selected sk will have more than 8
> >
> > Using the old score could trigger more lookups depending on the
> > order that sockets are created.
>
> So the result will still be correct, but it's less performant? Happy
> to fix a perf regression, but if the result is incorrect this might
> need a separate fix?
Right, the result is always correct.
If BPF prog selects a different socket per lookup, there is no
consistency, but it _is_ corret.
> I did some more digging. I think this was introduced by commit
> efc6b6f6c311 ("udp: Improve load balancing for SO_REUSEPORT.") which
> unfortunately ran into a merge conflict. That resulted in Dave Miller
> moving the bug around in commit a57066b1a019 ("Merge
> git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net"). Can you
> take a look and let me know if you think that is correct?
Yes, I should have updated the score too in efc6b6f6c311 to save
unneeded lookups. The conflict itself was resolved properly.
Powered by blists - more mailing lists