From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B1675A2EFC for ; Tue, 15 Oct 2019 07:36:17 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 9C5611C223; Tue, 15 Oct 2019 07:36:16 +0200 (CEST) Received: from mga09.intel.com (mga09.intel.com [134.134.136.24]) by dpdk.org (Postfix) with ESMTP id 6E3731C220; Tue, 15 Oct 2019 07:36:14 +0200 (CEST) X-Amp-Result: UNKNOWN X-Amp-Original-Verdict: FILE UNKNOWN X-Amp-File-Uploaded: False Received: from orsmga007.jf.intel.com ([10.7.209.58]) by orsmga102.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 14 Oct 2019 22:36:13 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.67,298,1566889200"; d="scan'208";a="185706436" Received: from dpdk-virtio-tbie-2.sh.intel.com (HELO ___) ([10.67.104.74]) by orsmga007.jf.intel.com with ESMTP; 14 Oct 2019 22:36:11 -0700 Date: Tue, 15 Oct 2019 13:33:10 +0800 From: Tiwei Bie To: Andrew Rybchenko Cc: Marvin Liu , maxime.coquelin@redhat.com, zhihong.wang@intel.com, stephen@networkplumber.org, dev@dpdk.org, stable@dpdk.org, Kevin Traynor , Luca Boccassi Message-ID: <20191015053309.GA18809@___> References: <20190923140511.107939-1-yong.liu@intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.9.4 (2018-02-28) Subject: Re: [dpdk-dev] [PATCH] net/virtio: fix mbuf data and pkt length mismatch 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: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, Oct 14, 2019 at 06:15:42PM +0300, Andrew Rybchenko wrote: > > diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c > > index 27ead19fb..822cce06d 100644 > > --- a/drivers/net/virtio/virtio_rxtx.c > > +++ b/drivers/net/virtio/virtio_rxtx.c > > @@ -597,9 +597,8 @@ virtqueue_enqueue_xmit_inorder(struct virtnet_tx *txvq, > > dxp->cookie = (void *)cookies[i]; > > dxp->ndescs = 1; > > - hdr = (struct virtio_net_hdr *) > > - rte_pktmbuf_prepend(cookies[i], head_size); > > - cookies[i]->pkt_len -= head_size; > > + hdr = (struct virtio_net_hdr *)(char *)cookies[i]->buf_addr + > > + cookies[i]->data_off - head_size; > > /* if offload disabled, hdr is not zeroed yet, do it now */ > > if (!vq->hw->has_tx_offload) > > @@ -608,9 +607,10 @@ virtqueue_enqueue_xmit_inorder(struct virtnet_tx *txvq, > > virtqueue_xmit_offload(hdr, cookies[i], true); > > start_dp[idx].addr = VIRTIO_MBUF_DATA_DMA_ADDR(cookies[i], vq); > > As I understand the problem is here. It points to start of the packet > (Ethernet header) since data_off is not changed above now, but > should point to virtio_net_hdr before the packet. > > I think the patch fixes the bug in a wrong direction. It looks better > to simply remove > > cookies[i]->pkt_len -= head_size; > > above and care about real packet length difference in > virtio_update_packet_stats() or when it is called from Tx path. > > If it is OK for maintainers I'm ready to send patches to rollback back > this one and fix it as described above. Hi Andrew, Thanks for looking into this! Feel free to send your fix. PS. Another thing also needs to be noticed is that, after prepending the net hdr with rte_pktmbuf_prepend(), below code in virtio_update_packet_stats() won't be able to access the ether header as expected: https://github.com/DPDK/dpdk/blob/31b798a6f08e9b333b94b8bb26910209aa810b73/drivers/net/virtio/virtio_rxtx.c#L134-L140 Thanks, Tiwei