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

[Jason Wang <jasowang@redhat.com>]

On 2017年08月19日 14:38, Koichiro Den wrote:
> Facing the possible unbounded delay relying on freeing on xmit path,
> we also better to invoke and clear the upper layer zerocopy callback
> beforehand to keep them from waiting for unbounded duration in vain.
> For instance, this removes the possible deadlock in the case that the
> upper layer is a zerocopy-enabled vhost-net.
> This does not apply if napi_tx is enabled since it will be called in
> reasonale time.
>
> Signed-off-by: Koichiro Den <den@klaipeden.com>
> ---
>   drivers/net/virtio_net.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 4302f313d9a7..f7deaa5b7b50 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -1290,6 +1290,14 @@ static netdev_tx_t start_xmit(struct sk_buff *skb, struct net_device *dev)
>   
>   	/* 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;
> +		}
>   		skb_orphan(skb);
>   		nf_reset(skb);
>   	}


Interesting, deadlock could be treated as a a radical case of the 
discussion here https://patchwork.kernel.org/patch/3787671/.

git grep tells more similar skb_orphan() cases. Do we need to change 
them all (or part)?

Actually, we may meet similar issues at many other places (e.g netem). 
Need to consider a complete solution for this. Figuring out all places 
that could delay a packet is a method.

Thanks