From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <yliu@fridaylinux.org>
Received: from new2-smtp.messagingengine.com (new2-smtp.messagingengine.com
[66.111.4.224]) by dpdk.org (Postfix) with ESMTP id 40F051B615;
Mon, 23 Oct 2017 15:21:50 +0200 (CEST)
Received: from compute1.internal (compute1.nyi.internal [10.202.2.41])
by mailnew.nyi.internal (Postfix) with ESMTP id 75D6D1016;
Mon, 23 Oct 2017 09:21:49 -0400 (EDT)
Received: from frontend1 ([10.202.2.160])
by compute1.internal (MEProxy); Mon, 23 Oct 2017 09:21:49 -0400
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=fridaylinux.org;
h=cc:content-type:date:from:in-reply-to:message-id:mime-version
:references:subject:to:x-me-sender:x-me-sender:x-sasl-enc; s=
fm1; bh=ewqamxMlPZ0Ej3h9+KdpIjimYROYCObAK6j+Uy9BSVE=; b=Z5yFrCra
wqZ0q78txL1NjyXAisgckTQyZay+BV2gGi3y4k+ejocB/THlSAbgG6czDxiIMbcc
QCYZHn37spmRpG+4HQY6EAPDxTBEL86FIdWmcQCUkMW1NViEpMkrZT2NU5uP9vjL
ZBWGueRc6RqknDjT3oR2bIeiOfC/lwO6yN/gPTI2Np/SRc3+yTGQU5Zx0/dKy6Xp
2r9eeKeb0uMOZNY462Gyrp+PYFoRTb0FuYxy+mZR97Z460cxATzvzEc6+61wtQYm
3yGglLiddYoriOH0FmZAN+8lh7Vd7YU3Sh1j4N5HgforVSnhvQYqRRVkl7PSGUiW
DZ+qNiIkBC6u+A==
DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=
messagingengine.com; h=cc:content-type:date:from:in-reply-to
:message-id:mime-version:references:subject:to:x-me-sender
:x-me-sender:x-sasl-enc; s=fm1; bh=ewqamxMlPZ0Ej3h9+KdpIjimYROYC
ObAK6j+Uy9BSVE=; b=IoVkFCocg78SwANXWqJIIBAXvGMjh9rfhKwnFfMAHRIfC
+Rwq4zoFVJHqtZB53VQFENwYvABU7WxKYez/EJJMKBppqvWF92E3vjjU+T4FeOvA
qIe9diUfR1AinsFca2LkBtqbFQaKRnegkLVXp0GJHMAtC6dADMndCqS0xbN9Eu9z
IN6WhgF6AZIErsEEaIzDgU5FRYnNKxCGoJHQiH5Ob64LkKlT5+Y6p/dz5w7RU03H
N/52cWRkzmasU2bKps4pQno7Jcfa3AfsSRuTsUumhJdyBcPkYdWxd8loWwQgUg4Z
gwxUjDlhYjTA9m8mAk0O+rQoCY48cFrXqwTmhMVtA==
X-ME-Sender: <xms:7eztWUP-zKYfpPPimC40DpxTDX-EwcQFbA3GZ5k-hE2Ln5ofGN-Pgw>
Received: from yliu-home (unknown [180.158.56.237])
by mail.messagingengine.com (Postfix) with ESMTPA id F13647F91D;
Mon, 23 Oct 2017 09:21:47 -0400 (EDT)
Date: Mon, 23 Oct 2017 21:21:44 +0800
From: Yuanhan Liu <yliu@fridaylinux.org>
To: Zhiyong Yang <zhiyong.yang@intel.com>
Cc: dev@dpdk.org, stable@dpdk.org, maxime.coquelin@redhat.com
Message-ID: <20171023132143.GI1545@yliu-home>
References: <20171011043935.16813-1-zhiyong.yang@intel.com>
<20171023064036.56821-1-zhiyong.yang@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20171023064036.56821-1-zhiyong.yang@intel.com>
User-Agent: Mutt/1.5.24 (2015-08-30)
Subject: Re: [dpdk-stable] [PATCH v2] net/virtio: fix wrong TX pkt length
stats
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/stable>,
<mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/stable>,
<mailto:stable-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 23 Oct 2017 13:21:51 -0000
On Mon, Oct 23, 2017 at 02:40:36PM +0800, Zhiyong Yang wrote:
> In the function virtqueue_enqueue_xmit(), when can_push is true,
> vtnet_hdr_size is added to pkt_len by calling rte_pktmbuf_prepend.
> So, virtio header length should be subtracted before calling stats
> function.
>
> Fixes: 58169a9c8153 ("net/virtio: support Tx checksum offload")
>
> Cc: stable@dpdk.org
> Cc: yliu@fridaylinux.org
> Cc: maxime.coquelin@redhat.com
> Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com>
> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>
> Changes in V2:
> Put code in the function virtqueue_enqueue_xmit() according to
> yuanhan's comments.
>
> drivers/net/virtio/virtio_rxtx.c | 7 +++++++
> 1 file changed, 7 insertions(+)
>
> diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c
> index 2cf82fef4..f28751e07 100644
> --- a/drivers/net/virtio/virtio_rxtx.c
> +++ b/drivers/net/virtio/virtio_rxtx.c
> @@ -396,6 +396,13 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie,
> vq->vq_desc_tail_idx = idx;
> vq->vq_free_cnt = (uint16_t)(vq->vq_free_cnt - needed);
> vq_update_avail_ring(vq, head_idx);
> +
> + /* when can_push is true, vtnet_hdr_size is added to pkt_len
> + * of mbuf. It should be subtracted in order to make stats function
> + * work in the right way.
> + */
> + if (can_push)
> + cookie->pkt_len -= head_size;
I actually meant to put it inside the "if (can_push)" clause, so that you
don't have to add yet another if. The another reason I wanted you to do
the move is, rte_pktmbuf_prepend (which did the magic) is exactly invoked
inside this if. Such move puts logical related code tight together. At least,
you could make the comment simpler: you don't have to say "when can_push
is true ...". Instead, it could be:
/*
* rte_pktmbuf_prepend() counts the hdr size to the pkt length,
* which is wrong. Below subtract restores the correct pkt size.
*/
--yliu