Re: [PATCH bpf-next v2 0/6] selftests/bpf: Various sockmap-related fixes

From: Jakub Sitnicki
Date: Wed Oct 09 2024 - 05:47:10 EST


I'm back after a short break. Sorry for delay.

On Wed, Oct 02, 2024 at 10:27 AM +02, Michal Luczaj wrote:
> On 9/27/24 11:15, Jakub Sitnicki wrote:
>> On Fri, Sep 27, 2024 at 12:54 AM +02, Michal Luczaj wrote:
>>> ...
>>> Here's a follow up: my guess is that some checks are missing. I'm not sure
>>> if it's the best approach, but this fixes things for me:
>>
>> So you have already found a bug with a negative test. Nice.
>>
>> Your patch makes sense to me.
>
> Great, I'll submit it properly.
>
> Another thing I've noticed is that unsupported (non-TCP) sk_msg redirects
> fail silently, i.e. send() is successful, then packet appears to be
> dropped, but because the BPF_SK_MSG_VERDICT program is never run, the
> verdict[SK_DROP] isn't updated. Is this by design?

That's curious. We don't override the proto::sendmsg callback for
protocols which don't support sk_msg redirects, like UDP:

https://elixir.bootlin.com/linux/v6.12-rc2/source/net/ipv4/udp_bpf.c#L114

The packet should get delivered to the peer socket as w/o sockmap.
I will have to double check that.

> Also, for unsupported af_vsock sk_skb-to-ingress we hit the warning:
>
> [ 233.396654] rx_queue is empty, but rx_bytes is non-zero
> [ 233.396702] WARNING: CPU: 11 PID: 40601 at net/vmw_vsock/virtio_transport_common.c:589 virtio_transport_stream_dequeue+0x2e5/0x2f0
>
> I'll try to fix that. Now, the series begin to grow long. Should the fixes
> come separately?

Thanks. And yes - if possible, better to push fixes separately. Because
they go through the bpf tree, and they will still land in the upcoming
-rc releases (and get backported).

While improvements go through bpf-next. Of course that sometimes makes
life more difficult if the improvements depend on some fixes...

Not sure if anything from bpf-next gets backported if it has a Fixes
tag. We can ask the stable kernel maintainers, if needed.