* [dpdk-stable] [PATCH v2] net/virtio: fix wrong TX pkt length stats [not found] <20171011043935.16813-1-zhiyong.yang@intel.com> @ 2017-10-23 6:40 ` Zhiyong Yang 2017-10-23 13:21 ` Yuanhan Liu 2017-10-24 3:06 ` [dpdk-stable] [PATCH v3] " Zhiyong Yang 0 siblings, 2 replies; 5+ messages in thread From: Zhiyong Yang @ 2017-10-23 6:40 UTC (permalink / raw) To: dev; +Cc: stable, yliu, maxime.coquelin, Zhiyong Yang 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 <zhiyong.yang@intel.com> Reviewed-by: Maxime Coquelin <maxime.coquelin@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; } void -- 2.13.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-stable] [PATCH v2] net/virtio: fix wrong TX pkt length stats 2017-10-23 6:40 ` [dpdk-stable] [PATCH v2] net/virtio: fix wrong TX pkt length stats Zhiyong Yang @ 2017-10-23 13:21 ` Yuanhan Liu 2017-10-24 2:05 ` Yang, Zhiyong 2017-10-24 3:06 ` [dpdk-stable] [PATCH v3] " Zhiyong Yang 1 sibling, 1 reply; 5+ messages in thread From: Yuanhan Liu @ 2017-10-23 13:21 UTC (permalink / raw) To: Zhiyong Yang; +Cc: dev, stable, maxime.coquelin 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 <zhiyong.yang@intel.com> > Reviewed-by: Maxime Coquelin <maxime.coquelin@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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-stable] [PATCH v2] net/virtio: fix wrong TX pkt length stats 2017-10-23 13:21 ` Yuanhan Liu @ 2017-10-24 2:05 ` Yang, Zhiyong 0 siblings, 0 replies; 5+ messages in thread From: Yang, Zhiyong @ 2017-10-24 2:05 UTC (permalink / raw) To: Yuanhan Liu; +Cc: dev, stable, maxime.coquelin Yuanhan, > -----Original Message----- > From: Yuanhan Liu [mailto:yliu@fridaylinux.org] > Sent: Monday, October 23, 2017 9:22 PM > To: Yang, Zhiyong <zhiyong.yang@intel.com> > Cc: dev@dpdk.org; stable@dpdk.org; maxime.coquelin@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: stable@dpdk.org > > Cc: yliu@fridaylinux.org > > Cc: maxime.coquelin@redhat.com > > Signed-off-by: Zhiyong Yang <zhiyong.yang@intel.com> > > Reviewed-by: Maxime Coquelin <maxime.coquelin@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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [dpdk-stable] [PATCH v3] net/virtio: fix wrong TX pkt length stats 2017-10-23 6:40 ` [dpdk-stable] [PATCH v2] net/virtio: fix wrong TX pkt length stats Zhiyong Yang 2017-10-23 13:21 ` Yuanhan Liu @ 2017-10-24 3:06 ` Zhiyong Yang 2017-10-24 8:46 ` Yuanhan Liu 1 sibling, 1 reply; 5+ messages in thread From: Zhiyong Yang @ 2017-10-24 3:06 UTC (permalink / raw) To: dev; +Cc: stable, yliu, maxime.coquelin, Zhiyong Yang In the function virtqueue_enqueue_xmit(), when can_push is true, vtnet_hdr_size is added to pkt_len by calling rte_pktmbuf_prepend. which is wrong for pkt stats, 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 <zhiyong.yang@intel.com> Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- Changes in V3: Move code inside "if (can_push)" clause and simplify comments. Changes in V2: Put code in the function virtqueue_enqueue_xmit() according to yuanhan's comments. drivers/net/virtio/virtio_rxtx.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/drivers/net/virtio/virtio_rxtx.c b/drivers/net/virtio/virtio_rxtx.c index 2cf82fef4..ac4055d45 100644 --- a/drivers/net/virtio/virtio_rxtx.c +++ b/drivers/net/virtio/virtio_rxtx.c @@ -299,6 +299,10 @@ virtqueue_enqueue_xmit(struct virtnet_tx *txvq, struct rte_mbuf *cookie, /* prepend cannot fail, checked by caller */ hdr = (struct virtio_net_hdr *) rte_pktmbuf_prepend(cookie, head_size); + /* rte_pktmbuf_prepend() counts the hdr size to the pkt length, + * which is wrong. Below subtract restores correct pkt size. + */ + cookie->pkt_len -= head_size; /* if offload disabled, it is not zeroed below, do it now */ if (offload == 0) { ASSIGN_UNLESS_EQUAL(hdr->csum_start, 0); -- 2.13.3 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [dpdk-stable] [PATCH v3] net/virtio: fix wrong TX pkt length stats 2017-10-24 3:06 ` [dpdk-stable] [PATCH v3] " Zhiyong Yang @ 2017-10-24 8:46 ` Yuanhan Liu 0 siblings, 0 replies; 5+ messages in thread From: Yuanhan Liu @ 2017-10-24 8:46 UTC (permalink / raw) To: Zhiyong Yang; +Cc: dev, stable, maxime.coquelin On Tue, Oct 24, 2017 at 11:06:14AM +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. > which is wrong for pkt stats, 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 <zhiyong.yang@intel.com> > Reviewed-by: Maxime Coquelin <maxime.coquelin@redhat.com> Applied to dpdk-next-virtio. Thanks. --yliu ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2017-10-24 8:46 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <20171011043935.16813-1-zhiyong.yang@intel.com>
2017-10-23 6:40 ` [dpdk-stable] [PATCH v2] net/virtio: fix wrong TX pkt length stats Zhiyong Yang
2017-10-23 13:21 ` Yuanhan Liu
2017-10-24 2:05 ` Yang, Zhiyong
2017-10-24 3:06 ` [dpdk-stable] [PATCH v3] " Zhiyong Yang
2017-10-24 8:46 ` Yuanhan Liu
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).