[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [ovs-dev] [PATCH] openvswitch: Orphan frags before sending to userspace via Netlink to avoid guest stall



On 11/03/14 19:41, Zoltan Kiss wrote:
On 07/03/14 17:59, Thomas Graf wrote:
On 03/07/2014 06:28 PM, Pravin Shelar wrote:
Problem is mapping SKBTX_DEV_ZEROCOPY pages to userspace. skb_zerocopy
is not doing that.

Unless I missing something, Current netlink code can not handle
skb-frags with zero copy. So we have to copy skb anyways and no need
to orphan-frags here.
If you are planning on handling skb-frags without copying then
skb_orphan_frags should be done in netlink.

If you look at the second part of skb_zerocopy() this is exactly what
it is doing unless the target skb has sufficient linear space
preallocated. At least unless mmap is enabled in which case we would
have to copy again until we have implemented a way to pass page refs
via the nl ring buffer.

So I think Zoltan is correct in orphaning frags that come from f.e.
a tun device via zerocopy_sg_from_iovec().

Now as I'm checking how Netlink works, I might be wrong at some parts :)
skb_zerocopy correctly add the frags to the user_skb we are sending
upwards, however when the userspace receive it in netlink_recvmsg(), it
gets copied to the supplied buffer anyway. Is that correct? In which
case we don't need to worry that userspace will sit on that page
indefinitely. However we have to worry about userspace not calling recv
on that Netlink socket, so in the end we still need skb_orphan_frags,
just for a different reason :)
We can put skb_orphan_frags into skb_zerocopy, skb_clone also do that.

However with Netlink mmapped IO, we should take a different approach,
and instead of calling skb_orphan_frags we should make sure user_skb can
hold any skb we get from the kernel, and copy the frags there. Even if
we would be able to pass page refs to userspace through the ring buffer
(AFAIK currently we can't), it would be fragile to just pass kernel
pages directly to userspace, even if they came without the
SKBTX_DEV_ZEROCOPY flag. And I think it would be quite rare that we need
that copy anyway, because the flow setup usually happens with small
packets without frags.
If we choose the above approach with Netlink mmap, we don't need
skb_orphan_frags, in fact

I spent some time to think about this mmaped scenaria, and discussed it with others: the conclusion is that it shouldn't be a big problem to expose local kernel pages through the frags array as I thought before. So OVS can get along with passing refs to those pages in the shared ring. However skb_orphan_frags would be still necessary in skb_zerocopy, for the same reason as now. Should I post a new patch which does calls orphan_frags in zerocopy? Or do you have any other opinion?

Zoli


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.