Hi Maxime, > -----Original Message----- > From: Maxime Coquelin [mailto:maxime.coque...@redhat.com] > Sent: Friday, October 13, 2017 6:06 PM > To: Yang, Zhiyong <zhiyong.y...@intel.com>; dev@dpdk.org > Cc: y...@fridaylinux.org; Tan, Jianfeng <jianfeng....@intel.com> > 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 <zhiyong.y...@intel.com> > > --- > > 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. Thanks Zhiyong > > Thanks, > Maxime > > > virtio_update_packet_stats(&txvq->stats, txm); > > } > >
