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

[Willem de Bruijn <willemdebruijn.kernel@gmail.com>]

>> > >         /* 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.

>> 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.
>