From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id ADB40C43387 for ; Thu, 17 Jan 2019 12:39:31 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id 85FC320657 for ; Thu, 17 Jan 2019 12:39:31 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1727414AbfAQMja (ORCPT ); Thu, 17 Jan 2019 07:39:30 -0500 Received: from mx1.redhat.com ([209.132.183.28]:50874 "EHLO mx1.redhat.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725882AbfAQMja (ORCPT ); Thu, 17 Jan 2019 07:39:30 -0500 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 606D880F88; Thu, 17 Jan 2019 12:39:29 +0000 (UTC) Received: from [10.72.12.16] (ovpn-12-16.pek2.redhat.com [10.72.12.16]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 781A1381A2; Thu, 17 Jan 2019 12:39:24 +0000 (UTC) Subject: Re: [PATCH net 2/7] virtio_net: Don't call free_old_xmit_skbs for xdp_frames To: Toshiaki Makita , "David S. Miller" , "Michael S. Tsirkin" Cc: netdev@vger.kernel.org, virtualization@lists.linux-foundation.org, Willem de Bruijn References: <1547724045-2726-1-git-send-email-makita.toshiaki@lab.ntt.co.jp> <1547724045-2726-3-git-send-email-makita.toshiaki@lab.ntt.co.jp> From: Jason Wang Message-ID: <7c8955f5-cd38-c26f-9932-0aa0be573b4e@redhat.com> Date: Thu, 17 Jan 2019 20:39:22 +0800 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <1547724045-2726-3-git-send-email-makita.toshiaki@lab.ntt.co.jp> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-Scanned-By: MIMEDefang 2.79 on 10.5.11.11 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.27]); Thu, 17 Jan 2019 12:39:29 +0000 (UTC) Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 2019/1/17 下午7:20, Toshiaki Makita wrote: > When napi_tx is enabled, virtnet_poll_cleantx() called > free_old_xmit_skbs() even for xdp send queue. > This is bogus since the queue has xdp_frames, not sk_buffs, thus mangled > device tx bytes counters because skb->len is meaningless value, and even > triggered oops due to general protection fault on freeing them. > > Since xdp send queues do not aquire locks, old xdp_frames should be > freed only in virtnet_xdp_xmit(), so just skip free_old_xmit_skbs() for > xdp send queues. > > Similarly virtnet_poll_tx() called free_old_xmit_skbs(). This NAPI > handler is called even without calling start_xmit() because cb for tx is > by default enabled. Once the handler is called, it enabled the cb again, > and then the handler would be called again. We don't need this handler > for XDP, so don't enable cb as well as not calling free_old_xmit_skbs(). > > Also, we need to disable tx NAPI when disabling XDP, so > virtnet_poll_tx() can safely access curr_queue_pairs and > xdp_queue_pairs, which are not atomically updated while disabling XDP. I suggest to split this into another patch or squash this part to patch 1. Other looks good. Thanks > > Fixes: b92f1e6751a6 ("virtio-net: transmit napi") > Fixes: 7b0411ef4aa6 ("virtio-net: clean tx descriptors from rx napi") > Signed-off-by: Toshiaki Makita > --- > drivers/net/virtio_net.c | 49 ++++++++++++++++++++++++++++++++---------------- > 1 file changed, 33 insertions(+), 16 deletions(-) > > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c > index a08da9e..7d35e6d 100644 > --- a/drivers/net/virtio_net.c > +++ b/drivers/net/virtio_net.c > @@ -1358,6 +1358,16 @@ static void free_old_xmit_skbs(struct send_queue *sq) > u64_stats_update_end(&sq->stats.syncp); > } > > +static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q) > +{ > + if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs)) > + return false; > + else if (q < vi->curr_queue_pairs) > + return true; > + else > + return false; > +} > + > static void virtnet_poll_cleantx(struct receive_queue *rq) > { > struct virtnet_info *vi = rq->vq->vdev->priv; > @@ -1365,7 +1375,7 @@ static void virtnet_poll_cleantx(struct receive_queue *rq) > struct send_queue *sq = &vi->sq[index]; > struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, index); > > - if (!sq->napi.weight) > + if (!sq->napi.weight || is_xdp_raw_buffer_queue(vi, index)) > return; > > if (__netif_tx_trylock(txq)) { > @@ -1442,8 +1452,16 @@ static int virtnet_poll_tx(struct napi_struct *napi, int budget) > { > struct send_queue *sq = container_of(napi, struct send_queue, napi); > struct virtnet_info *vi = sq->vq->vdev->priv; > - struct netdev_queue *txq = netdev_get_tx_queue(vi->dev, vq2txq(sq->vq)); > + unsigned int index = vq2txq(sq->vq); > + struct netdev_queue *txq; > > + if (unlikely(is_xdp_raw_buffer_queue(vi, index))) { > + /* We don't need to enable cb for XDP */ > + napi_complete_done(napi, 0); > + return 0; > + } > + > + txq = netdev_get_tx_queue(vi->dev, index); > __netif_tx_lock(txq, raw_smp_processor_id()); > free_old_xmit_skbs(sq); > __netif_tx_unlock(txq); > @@ -2402,9 +2420,12 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog, > } > > /* Make sure NAPI is not using any XDP TX queues for RX. */ > - if (netif_running(dev)) > - for (i = 0; i < vi->max_queue_pairs; i++) > + if (netif_running(dev)) { > + for (i = 0; i < vi->max_queue_pairs; i++) { > napi_disable(&vi->rq[i].napi); > + virtnet_napi_tx_disable(&vi->sq[i].napi); > + } > + } > > netif_set_real_num_rx_queues(dev, curr_qp + xdp_qp); > err = _virtnet_set_queues(vi, curr_qp + xdp_qp); > @@ -2423,16 +2444,22 @@ static int virtnet_xdp_set(struct net_device *dev, struct bpf_prog *prog, > } > if (old_prog) > bpf_prog_put(old_prog); > - if (netif_running(dev)) > + if (netif_running(dev)) { > virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi); > + virtnet_napi_tx_enable(vi, vi->sq[i].vq, > + &vi->sq[i].napi); > + } > } > > return 0; > > err: > if (netif_running(dev)) { > - for (i = 0; i < vi->max_queue_pairs; i++) > + for (i = 0; i < vi->max_queue_pairs; i++) { > virtnet_napi_enable(vi->rq[i].vq, &vi->rq[i].napi); > + virtnet_napi_tx_enable(vi, vi->sq[i].vq, > + &vi->sq[i].napi); > + } > } > if (prog) > bpf_prog_sub(prog, vi->max_queue_pairs - 1); > @@ -2615,16 +2642,6 @@ static void free_receive_page_frags(struct virtnet_info *vi) > put_page(vi->rq[i].alloc_frag.page); > } > > -static bool is_xdp_raw_buffer_queue(struct virtnet_info *vi, int q) > -{ > - if (q < (vi->curr_queue_pairs - vi->xdp_queue_pairs)) > - return false; > - else if (q < vi->curr_queue_pairs) > - return true; > - else > - return false; > -} > - > static void free_unused_bufs(struct virtnet_info *vi) > { > void *buf;