Re: [PATCH net v3 1/2] net: core: propagate unreadable flag in skb_zerocopy

[Ilya Maximets <i.maximets@ovn.org>]

On 8/14/26 8:48 PM, Mina Almasry wrote:
> On Wed, Aug 12, 2026 at 8:52 AM Ilya Maximets <i.maximets@ovn.org> wrote:
>>
>> On 8/11/26 9:53 PM, Mina Almasry wrote:
>>> diff --git a/net/openvswitch/datapath.c b/net/openvswitch/datapath.c
>>> index ae69b2cabab9e..482893a5f67dc 100644
>>> --- a/net/openvswitch/datapath.c
>>> +++ b/net/openvswitch/datapath.c
>>> @@ -467,6 +467,9 @@ static int queue_userspace_packet(struct datapath *dp, struct sk_buff *skb,
>>>       if (!dp_ifindex)
>>>               return -ENODEV;
>>>
>>> +     if (!skb_frags_readable(skb))
>>> +             return -EFAULT;
>>> +
>>>       if (skb_vlan_tag_present(skb)) {
>>>               nskb = skb_clone(skb, GFP_ATOMIC);
>>>               if (!nskb)
>> FWIW, the devmem integration doesn't seem well-designed.
> 
> As is most of what I touch :P
> 
>>  I understand
>> that it is for performance, but IMO there should be a way to copy the
>> data on a slow path to avoid dropping the packets.  Clamping without
>> notifying the users that the packet is truncated is not a good solution.
>> Not for OVS, not for other parts of the kernel networking stack.  It's
>> a uAPI breakage.
>>
> 
> FWIW, it happens that the fallback-to-copy in the context of the
> devmem TCP seems to be useless to the userspace. As a matter of fact
> there is one current path where we fallback to copy/CPU memory
> (SCM_DEVMEM_LINEAR), and my users decided to write userspace to
> completely barf on that condition. We ended up rooting all the reasons
> SCM_DEVMEM_LINEAR could happen and preventing that (mostly flow
> steering failures in our case).
> 
> Broadcomm also added a devmem kselftest test case that fails on any
> SCM_DEVMEM_LINEAR as well, so I think they may have independently
> reached the same conclusion with their users.

Just for the context on how OVS works: the very first packet, e.g.
SYN, goes to userspace via this upcall mechanism, then ovs-vswitchd
runs it through the OpenFlow pipeline and figures out what actions
to take and where to forward.  Next it injects the packet back via
netlink request to execute those actions and in parallel it installs
a datapath flow into the kernel.  The next packet that matches the
installed datapath flow does not go to userspace and gets forwarded
inside the kernel.

So, in theory, very few packets go to userspace and the rest stay
in the kernel going through the fast path.  In this situation it
doesn't matter too much that the first packet takes the performance
hit as long as the rest are not.

Depending on the OpenFlow pipeline the syn+ack and the ack+psh may
need to go to userspace, out of which, I suppose the ack+psh is the
most problematic as it carries a large payload that will end up in
the unreadable memory.

In this situation, It seems to me that being able to copy the data
directly to the userspace would be useful as it would not have any
performance impact on the fast path and will allow openvswitch to
work normally for the most part without any changes to ovs-vswitchd
in userspace.

Best regards, Ilya Maximets.

> 
>> As it is, there is not much we can do here without extensive changes
>> in userspace applications, so for this OVS block:
>>
> 
> But, a future user could find such a fallback-to-cpu-mem useful. If
> anyone is reading this wondering how to implement that, these are my
> rough ideas:
> 
> + some dmabufs will support the dma_buf_vmap op which will map the
> dmabuf to the kernel space. if the dmabuf supports it, we could use
> that op to map the dmabuf memory to the kernel space, then copy the
> memory to a normal page allocated from a non-devmem page_pool, and
> create a readable skb based on that.
> + If the dmabuf does not support dma_buf_vmap, then I don't think
> there is any copy fallback we can do, sorry.
> + We'd need to design a uapi that tells the user that this particular
> chunk is in cpu memory. My guess is that recvmsg() needs to return if
> it notices a devmem/non-devmem skb boundary, and return early. Then
> the non-devmem skb can be given to the userspace with
> SCM_DEVMEM_LINEAR on the next recvmsg() call. Then the next recvmsg()
> call gets the next devmem skb without SCM_DEVMEM_LINEAR, etc. I think
> that would work.
> + We'd need the driver to maintain multiple page_pools per rx-queue
> then, I guess. One for the devmem, and one for the possible non-devmem
> fallback.
> 
>> Reviewed-by: Ilya Maximets <i.maximets@ovn.org>
> 
> Thank you! I'll submit another revision addressing the comments on the
> other patch.
>