From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id F326E1B626 for ; Fri, 13 Oct 2017 14:47:40 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.13]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 20DAE60D1; Fri, 13 Oct 2017 12:47:40 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mx1.redhat.com 20DAE60D1 Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com Authentication-Results: ext-mx06.extmail.prod.ext.phx2.redhat.com; spf=fail smtp.mailfrom=maxime.coquelin@redhat.com Received: from [10.36.112.24] (ovpn-112-24.ams2.redhat.com [10.36.112.24]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 8CE4060A99; Fri, 13 Oct 2017 12:47:38 +0000 (UTC) To: "Yang, Zhiyong" , "dev@dpdk.org" Cc: "yliu@fridaylinux.org" , "Tan, Jianfeng" References: <20171011043935.16813-1-zhiyong.yang@intel.com> From: Maxime Coquelin Message-ID: <34d70b50-c35f-5eb2-7d30-19d35a1a43cb@redhat.com> Date: Fri, 13 Oct 2017 14:47:36 +0200 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.3.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit X-Scanned-By: MIMEDefang 2.79 on 10.5.11.13 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.30]); Fri, 13 Oct 2017 12:47:40 +0000 (UTC) Subject: Re: [dpdk-dev] [PATCH] 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: Fri, 13 Oct 2017 12:47:41 -0000 On 10/13/2017 02:44 PM, Yang, Zhiyong wrote: > Hi Maxime, > >> -----Original Message----- >> From: Maxime Coquelin [mailto:maxime.coquelin@redhat.com] >> Sent: Friday, October 13, 2017 6:06 PM >> To: Yang, Zhiyong ; dev@dpdk.org >> Cc: yliu@fridaylinux.org; Tan, Jianfeng >> Subject: Re: [PATCH] net/virtio: fix wrong TX pkt length stats >> >> Hi Zhiyong, >> >> On 10/11/2017 06:39 AM, Zhiyong Yang wrote: >>> In the function virtqueue_enqueue_xmit(), when can_push == 1 is true, >>> vtnet_hdr_size is added to pkt_len by calling rte_pktmbuf_prepend. >>> So, virtio header len should be subtracted before calling stats >>> function. >>> >>> Fixes: 58169a9c8153 ("net/virtio: support Tx checksum offload") >>> >>> Signed-off-by: Zhiyong Yang >>> --- >>> drivers/net/virtio/virtio_rxtx.c | 6 ++++++ >>> 1 file changed, 6 insertions(+) >>> >>> diff --git a/drivers/net/virtio/virtio_rxtx.c >>> b/drivers/net/virtio/virtio_rxtx.c >>> index 609b4138a..bf14f9a99 100644 >>> --- a/drivers/net/virtio/virtio_rxtx.c >>> +++ b/drivers/net/virtio/virtio_rxtx.c >>> @@ -1079,6 +1079,12 @@ virtio_xmit_pkts(void *tx_queue, struct rte_mbuf >> **tx_pkts, uint16_t nb_pkts) >>> /* Enqueue Packet buffers */ >>> virtqueue_enqueue_xmit(txvq, txm, slots, use_indirect, >> can_push); >>> >>> + /* In function virtqueue_enqueue_xmit(), when can_push == 1 >>> + * is true, vtnet_hdr_size is added to pkt_len of mbuf. So, it >>> + * should be subtracted before calling stats function. >>> + */ >>> + if (can_push == 1) >>> + txm->pkt_len -= txvq->vq->hw->vtnet_hdr_size; >>> txvq->stats.bytes += txm->pkt_len; >> >> I acknowledge the problem, but I don't like modifying pkt_len. >> This is not the case currently, but in case we want to do something with the >> mbuf later in virtio_xmit_cleanup(), it could be good to have the real length >> there. >> >> What about: >> >> if (can_push == 1) >> txvq->stats.bytes += txm->pkt_len - txvq->vq->hw->vtnet_hdr_size; else >> txvq->stats.bytes += txm->pkt_len; > > I don't like modifying pkt_len, too. > But We need to consider that some stats are updated in virtio_update_packet_stats() > In this function, pkt_len will be used further. Ha, good point! I think we have no better way then: Reviewed-by: Maxime Coquelin Thanks, Maxime > Thanks > Zhiyong > >> >> Thanks, >> Maxime >> >>> virtio_update_packet_stats(&txvq->stats, txm); >>> } >>>