Yuanhan, > -----Original Message----- > From: Yuanhan Liu [mailto:y...@fridaylinux.org] > Sent: Monday, October 23, 2017 9:22 PM > To: Yang, Zhiyong <zhiyong.y...@intel.com> > Cc: dev@dpdk.org; sta...@dpdk.org; maxime.coque...@redhat.com > Subject: Re: [PATCH v2] net/virtio: fix wrong TX pkt length stats > > 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: sta...@dpdk.org > > Cc: y...@fridaylinux.org > > Cc: maxime.coque...@redhat.com > > Signed-off-by: Zhiyong Yang <zhiyong.y...@intel.com> > > Reviewed-by: Maxime Coquelin <maxime.coque...@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
You are right. I mistakenly think that cookie->pkt_len will be used to transmit pkts . Actually cookie->data_len will is used when filling the descriptors. No problem. V3 will be sent soon. Zhiyong
