Re: [PATCH net-next] virtio-net: invoke zerocopy callback on xmit path if no tx napi

On Tue, 2017-08-22 at 13:19 -0400, Willem de Bruijn wrote:

quoted

quoted

quoted

quoted

        /* Don't wait up for transmitted skbs to be freed. */
        if (!use_napi) {
+               if (skb_shinfo(skb)->tx_flags & SKBTX_DEV_ZEROCOPY) {
+                       struct ubuf_info *uarg;
+                       uarg = skb_shinfo(skb)->destructor_arg;
+                       if (uarg->callback)
+                           uarg->callback(uarg, true);
+                       skb_shinfo(skb)->destructor_arg = NULL;
+                       skb_shinfo(skb)->tx_flags &=
~SKBTX_DEV_ZEROCOPY;
+               }

Instead of open coding, this can use skb_zcopy_clear.

It is not correct to only send the zerocopy completion here, as
the skb will still be shared with the nic. The only safe approach
to notifying early is to create a copy with skb_orphan_frags_rx.
That will call skb_zcopy_clear(skb, false) internally. But the
copy will be very expensive.

I noticed this email after my last post. I cannot not imagine how it could
be
unsafe in virtio case. Sorry could you explain a bit more?

A guest process sends a packet with MSG_ZEROCOPY to the
virtio-net device. As soon as the process receives the completion
notification, it is allowed to reuse the memory backing the packet.

A call to skb_zcopy_clear in virtio-net start_xmit will notify the
process that it is allowed to reuse the memory. But the user pages
are still linked into the skb frags and are about to be shared with
the host os.

quoted

quoted

Is the deadlock you refer to the case where tx interrupts are
disabled, so transmit completions are only handled in start_xmit
and somehow the socket(s) are unable to send new data? The
question is what is blocking them. If it is zerocopy, it may be a
too low optmem_max or hitting the per-user locked pages limit.
We may have to keep interrupts enabled when zerocopy skbs
are in flight.

Even if we keep interrupts enabled, We miss the completion without
start_xmit.
And it's also likely that the next start_xmit depends on the completion
itself
as I described in my last post.

Thanks for the explanation, I misinterpreted the "nic" part, now it's clear.
Sorry about bothering.