From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by dpdk.org (Postfix) with ESMTP id 2D2E0239; Tue, 24 Oct 2017 04:05:35 +0200 (CEST) Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga105.jf.intel.com with ESMTP; 23 Oct 2017 19:05:32 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.43,424,1503385200"; d="scan'208";a="166709131" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga006.fm.intel.com with ESMTP; 23 Oct 2017 19:05:32 -0700 Received: from fmsmsx116.amr.corp.intel.com (10.18.116.20) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 23 Oct 2017 19:05:32 -0700 Received: from bgsmsx153.gar.corp.intel.com (10.224.23.4) by fmsmsx116.amr.corp.intel.com (10.18.116.20) with Microsoft SMTP Server (TLS) id 14.3.319.2; Mon, 23 Oct 2017 19:05:31 -0700 Received: from bgsmsx101.gar.corp.intel.com ([169.254.1.200]) by BGSMSX153.gar.corp.intel.com ([169.254.2.63]) with mapi id 14.03.0319.002; Tue, 24 Oct 2017 07:35:29 +0530 From: "Yang, Zhiyong" To: Yuanhan Liu CC: "dev@dpdk.org" , "stable@dpdk.org" , "maxime.coquelin@redhat.com" Thread-Topic: [PATCH v2] net/virtio: fix wrong TX pkt length stats Thread-Index: AQHTS8neecKLjTyVz0ylWUNzwNqBHaLxD/YAgAEs9WA= Date: Tue, 24 Oct 2017 02:05:28 +0000 Message-ID: References: <20171011043935.16813-1-zhiyong.yang@intel.com> <20171023064036.56821-1-zhiyong.yang@intel.com> <20171023132143.GI1545@yliu-home> In-Reply-To: <20171023132143.GI1545@yliu-home> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.223.10.10] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v2] net/virtio: fix wrong TX pkt length stats X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 24 Oct 2017 02:05:36 -0000 Yuanhan, > -----Original Message----- > From: Yuanhan Liu [mailto:yliu@fridaylinux.org] > Sent: Monday, October 23, 2017 9:22 PM > To: Yang, Zhiyong > Cc: dev@dpdk.org; stable@dpdk.org; maxime.coquelin@redhat.com > Subject: Re: [PATCH v2] net/virtio: fix wrong TX pkt length stats >=20 > 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 > > Reviewed-by: Maxime Coquelin > > --- > > > > 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 =3D idx; > > vq->vq_free_cnt =3D (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 -=3D head_size; >=20 > 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 t= he > comment simpler: you don't have to say "when can_push is true ...". Inste= ad, it > could be: >=20 > /* > * rte_pktmbuf_prepend() counts the hdr size to the pkt length, > * which is wrong. Below subtract restores the correct pkt size. > */ >=20 > --yliu You are right. I mistakenly think that cookie->pkt_len will be used to tra= nsmit pkts . Actually cookie->data_len will is used when filling the descriptors. No problem. V3 will be sent soon. Zhiyong