* [dpdk-dev] [PATCH 0/2] Fix packed ring header len
@ 2020-10-01 10:11 Maxime Coquelin
2020-10-01 10:11 ` [dpdk-dev] [PATCH 1/2] vhost: fix Virtio-net header len with packed ring Maxime Coquelin
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: Maxime Coquelin @ 2020-10-01 10:11 UTC (permalink / raw)
To: dev, chenbo.xia, yong.liu; +Cc: Maxime Coquelin
First patch is fixing packed ring header len. Issue was
reported by Marvin.
Second patch is a (very) light optimization, to make used
directly of the static hdr len value in packed-ring only
functions.
Maxime Coquelin (2):
vhost: fix Virtio-net header len with packed ring
vhost: use fixed Virtio-net header len packed ring
lib/librte_vhost/vhost_user.c | 4 +++-
lib/librte_vhost/virtio_net.c | 11 ++++++-----
2 files changed, 9 insertions(+), 6 deletions(-)
--
2.26.2
^ permalink raw reply [flat|nested] 7+ messages in thread* [dpdk-dev] [PATCH 1/2] vhost: fix Virtio-net header len with packed ring 2020-10-01 10:11 [dpdk-dev] [PATCH 0/2] Fix packed ring header len Maxime Coquelin @ 2020-10-01 10:11 ` Maxime Coquelin 2020-10-09 6:33 ` Xia, Chenbo 2020-10-01 10:11 ` [dpdk-dev] [PATCH 2/2] vhost: use fixed Virtio-net header len " Maxime Coquelin 2020-10-09 7:23 ` [dpdk-dev] [PATCH 0/2] Fix packed ring header len Maxime Coquelin 2 siblings, 1 reply; 7+ messages in thread From: Maxime Coquelin @ 2020-10-01 10:11 UTC (permalink / raw) To: dev, chenbo.xia, yong.liu; +Cc: Maxime Coquelin, stable In case packed ring layout has been negotiated, but neither Version 1 nor mergeable buffers, the Virtio-net header len is assigned to the legacy devices value, which is wrong. This patch fixes this with using the proper len as devices using packed ring are not legacy devices. Fixes: a922401f35cc ("vhost: add Rx support for packed ring") Fixes: ae999ce49dcb ("vhost: add Tx support for packed ring") Cc: stable@dpdk.org Reported-by: Marvin Liu <yong.liu@intel.com> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- lib/librte_vhost/vhost_user.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c index 4deceb3e00..5d1fb9e863 100644 --- a/lib/librte_vhost/vhost_user.c +++ b/lib/librte_vhost/vhost_user.c @@ -341,7 +341,9 @@ vhost_user_set_features(struct virtio_net **pdev, struct VhostUserMsg *msg, dev->features = features; if (dev->features & - ((1 << VIRTIO_NET_F_MRG_RXBUF) | (1ULL << VIRTIO_F_VERSION_1))) { + ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | + (1ULL << VIRTIO_F_VERSION_1) | + (1ULL << VIRTIO_F_RING_PACKED))) { dev->vhost_hlen = sizeof(struct virtio_net_hdr_mrg_rxbuf); } else { dev->vhost_hlen = sizeof(struct virtio_net_hdr); -- 2.26.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH 1/2] vhost: fix Virtio-net header len with packed ring 2020-10-01 10:11 ` [dpdk-dev] [PATCH 1/2] vhost: fix Virtio-net header len with packed ring Maxime Coquelin @ 2020-10-09 6:33 ` Xia, Chenbo 0 siblings, 0 replies; 7+ messages in thread From: Xia, Chenbo @ 2020-10-09 6:33 UTC (permalink / raw) To: Maxime Coquelin, dev, Liu, Yong; +Cc: stable > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Thursday, October 1, 2020 6:12 PM > To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Liu, Yong > <yong.liu@intel.com> > Cc: Maxime Coquelin <maxime.coquelin@redhat.com>; stable@dpdk.org > Subject: [PATCH 1/2] vhost: fix Virtio-net header len with packed ring > > In case packed ring layout has been negotiated, but neither > Version 1 nor mergeable buffers, the Virtio-net header len > is assigned to the legacy devices value, which is wrong. > > This patch fixes this with using the proper len as devices > using packed ring are not legacy devices. > > Fixes: a922401f35cc ("vhost: add Rx support for packed ring") > Fixes: ae999ce49dcb ("vhost: add Tx support for packed ring") > Cc: stable@dpdk.org > > Reported-by: Marvin Liu <yong.liu@intel.com> > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > --- > lib/librte_vhost/vhost_user.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/lib/librte_vhost/vhost_user.c b/lib/librte_vhost/vhost_user.c > index 4deceb3e00..5d1fb9e863 100644 > --- a/lib/librte_vhost/vhost_user.c > +++ b/lib/librte_vhost/vhost_user.c > @@ -341,7 +341,9 @@ vhost_user_set_features(struct virtio_net **pdev, > struct VhostUserMsg *msg, > > dev->features = features; > if (dev->features & > - ((1 << VIRTIO_NET_F_MRG_RXBUF) | (1ULL << VIRTIO_F_VERSION_1))) > { > + ((1ULL << VIRTIO_NET_F_MRG_RXBUF) | > + (1ULL << VIRTIO_F_VERSION_1) | > + (1ULL << VIRTIO_F_RING_PACKED))) { > dev->vhost_hlen = sizeof(struct virtio_net_hdr_mrg_rxbuf); > } else { > dev->vhost_hlen = sizeof(struct virtio_net_hdr); > -- > 2.26.2 Reviewed-by: Chenbo Xia <chenbo.xia@intel.com> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [dpdk-dev] [PATCH 2/2] vhost: use fixed Virtio-net header len packed ring 2020-10-01 10:11 [dpdk-dev] [PATCH 0/2] Fix packed ring header len Maxime Coquelin 2020-10-01 10:11 ` [dpdk-dev] [PATCH 1/2] vhost: fix Virtio-net header len with packed ring Maxime Coquelin @ 2020-10-01 10:11 ` Maxime Coquelin 2020-10-09 6:36 ` Xia, Chenbo 2020-10-09 7:23 ` [dpdk-dev] [PATCH 0/2] Fix packed ring header len Maxime Coquelin 2 siblings, 1 reply; 7+ messages in thread From: Maxime Coquelin @ 2020-10-01 10:11 UTC (permalink / raw) To: dev, chenbo.xia, yong.liu; +Cc: Maxime Coquelin This small optimization uses static the Virtio-net header len in packed datapath, since Virtio-net header cannot be the legacy one in case of packed ring. Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> --- lib/librte_vhost/virtio_net.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c index 0a0bea1a5a..5865fa5f65 100644 --- a/lib/librte_vhost/virtio_net.c +++ b/lib/librte_vhost/virtio_net.c @@ -1147,7 +1147,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev, uint16_t buf_id = 0; uint32_t len = 0; uint16_t desc_count; - uint32_t size = pkt->pkt_len + dev->vhost_hlen; + uint32_t size = pkt->pkt_len + sizeof(struct virtio_net_hdr_mrg_rxbuf); uint16_t num_buffers = 0; uint32_t buffer_len[vq->size]; uint16_t buffer_buf_id[vq->size]; @@ -1262,7 +1262,7 @@ virtio_dev_rx_batch_packed(struct virtio_net *dev, uint16_t avail_idx = vq->last_avail_idx; uint64_t desc_addrs[PACKED_BATCH_SIZE]; struct virtio_net_hdr_mrg_rxbuf *hdrs[PACKED_BATCH_SIZE]; - uint32_t buf_offset = dev->vhost_hlen; + uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); uint64_t lens[PACKED_BATCH_SIZE]; uint16_t ids[PACKED_BATCH_SIZE]; uint16_t i; @@ -1308,7 +1308,8 @@ virtio_dev_rx_batch_packed(struct virtio_net *dev, rte_prefetch0((void *)(uintptr_t)desc_addrs[i]); hdrs[i] = (struct virtio_net_hdr_mrg_rxbuf *) (uintptr_t)desc_addrs[i]; - lens[i] = pkts[i]->pkt_len + dev->vhost_hlen; + lens[i] = pkts[i]->pkt_len + + sizeof(struct virtio_net_hdr_mrg_rxbuf); } vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) @@ -2277,7 +2278,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev, struct virtio_net_hdr *hdr; uint64_t lens[PACKED_BATCH_SIZE]; uint64_t buf_lens[PACKED_BATCH_SIZE]; - uint32_t buf_offset = dev->vhost_hlen; + uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); uint16_t flags, i; if (unlikely(avail_idx & PACKED_BATCH_MASK)) @@ -2354,7 +2355,7 @@ virtio_dev_tx_batch_packed(struct virtio_net *dev, struct rte_mbuf **pkts) { uint16_t avail_idx = vq->last_avail_idx; - uint32_t buf_offset = dev->vhost_hlen; + uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); uintptr_t desc_addrs[PACKED_BATCH_SIZE]; uint16_t ids[PACKED_BATCH_SIZE]; uint16_t i; -- 2.26.2 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] vhost: use fixed Virtio-net header len packed ring 2020-10-01 10:11 ` [dpdk-dev] [PATCH 2/2] vhost: use fixed Virtio-net header len " Maxime Coquelin @ 2020-10-09 6:36 ` Xia, Chenbo 2020-10-09 7:23 ` Maxime Coquelin 0 siblings, 1 reply; 7+ messages in thread From: Xia, Chenbo @ 2020-10-09 6:36 UTC (permalink / raw) To: Maxime Coquelin, dev, Liu, Yong Hi Maxime, > -----Original Message----- > From: Maxime Coquelin <maxime.coquelin@redhat.com> > Sent: Thursday, October 1, 2020 6:12 PM > To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Liu, Yong > <yong.liu@intel.com> > Cc: Maxime Coquelin <maxime.coquelin@redhat.com> > Subject: [PATCH 2/2] vhost: use fixed Virtio-net header len packed ring > > This small optimization uses static the Virtio-net header len Better use 'the static Virtio-net header length' here when you apply the patch 😊. With this fix: Reviewed-by: Chenbo Xia <chenbo.xia@intel.com> > in packed datapath, since Virtio-net header cannot be the > legacy one in case of packed ring. > > Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> > --- > lib/librte_vhost/virtio_net.c | 11 ++++++----- > 1 file changed, 6 insertions(+), 5 deletions(-) > > diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c > index 0a0bea1a5a..5865fa5f65 100644 > --- a/lib/librte_vhost/virtio_net.c > +++ b/lib/librte_vhost/virtio_net.c > @@ -1147,7 +1147,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev, > uint16_t buf_id = 0; > uint32_t len = 0; > uint16_t desc_count; > - uint32_t size = pkt->pkt_len + dev->vhost_hlen; > + uint32_t size = pkt->pkt_len + sizeof(struct > virtio_net_hdr_mrg_rxbuf); > uint16_t num_buffers = 0; > uint32_t buffer_len[vq->size]; > uint16_t buffer_buf_id[vq->size]; > @@ -1262,7 +1262,7 @@ virtio_dev_rx_batch_packed(struct virtio_net *dev, > uint16_t avail_idx = vq->last_avail_idx; > uint64_t desc_addrs[PACKED_BATCH_SIZE]; > struct virtio_net_hdr_mrg_rxbuf *hdrs[PACKED_BATCH_SIZE]; > - uint32_t buf_offset = dev->vhost_hlen; > + uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); > uint64_t lens[PACKED_BATCH_SIZE]; > uint16_t ids[PACKED_BATCH_SIZE]; > uint16_t i; > @@ -1308,7 +1308,8 @@ virtio_dev_rx_batch_packed(struct virtio_net *dev, > rte_prefetch0((void *)(uintptr_t)desc_addrs[i]); > hdrs[i] = (struct virtio_net_hdr_mrg_rxbuf *) > (uintptr_t)desc_addrs[i]; > - lens[i] = pkts[i]->pkt_len + dev->vhost_hlen; > + lens[i] = pkts[i]->pkt_len + > + sizeof(struct virtio_net_hdr_mrg_rxbuf); > } > > vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) > @@ -2277,7 +2278,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net > *dev, > struct virtio_net_hdr *hdr; > uint64_t lens[PACKED_BATCH_SIZE]; > uint64_t buf_lens[PACKED_BATCH_SIZE]; > - uint32_t buf_offset = dev->vhost_hlen; > + uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); > uint16_t flags, i; > > if (unlikely(avail_idx & PACKED_BATCH_MASK)) > @@ -2354,7 +2355,7 @@ virtio_dev_tx_batch_packed(struct virtio_net *dev, > struct rte_mbuf **pkts) > { > uint16_t avail_idx = vq->last_avail_idx; > - uint32_t buf_offset = dev->vhost_hlen; > + uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); > uintptr_t desc_addrs[PACKED_BATCH_SIZE]; > uint16_t ids[PACKED_BATCH_SIZE]; > uint16_t i; > -- > 2.26.2 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH 2/2] vhost: use fixed Virtio-net header len packed ring 2020-10-09 6:36 ` Xia, Chenbo @ 2020-10-09 7:23 ` Maxime Coquelin 0 siblings, 0 replies; 7+ messages in thread From: Maxime Coquelin @ 2020-10-09 7:23 UTC (permalink / raw) To: Xia, Chenbo, dev, Liu, Yong Welcome back Chenbo, On 10/9/20 8:36 AM, Xia, Chenbo wrote: > Hi Maxime, > >> -----Original Message----- >> From: Maxime Coquelin <maxime.coquelin@redhat.com> >> Sent: Thursday, October 1, 2020 6:12 PM >> To: dev@dpdk.org; Xia, Chenbo <chenbo.xia@intel.com>; Liu, Yong >> <yong.liu@intel.com> >> Cc: Maxime Coquelin <maxime.coquelin@redhat.com> >> Subject: [PATCH 2/2] vhost: use fixed Virtio-net header len packed ring >> >> This small optimization uses static the Virtio-net header len > > Better use 'the static Virtio-net header length' here when you apply the > patch 😊. With this fix: I did the change when applying, thanks! Maxime > Reviewed-by: Chenbo Xia <chenbo.xia@intel.com> > >> in packed datapath, since Virtio-net header cannot be the >> legacy one in case of packed ring. >> >> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com> >> --- >> lib/librte_vhost/virtio_net.c | 11 ++++++----- >> 1 file changed, 6 insertions(+), 5 deletions(-) >> >> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c >> index 0a0bea1a5a..5865fa5f65 100644 >> --- a/lib/librte_vhost/virtio_net.c >> +++ b/lib/librte_vhost/virtio_net.c >> @@ -1147,7 +1147,7 @@ vhost_enqueue_single_packed(struct virtio_net *dev, >> uint16_t buf_id = 0; >> uint32_t len = 0; >> uint16_t desc_count; >> - uint32_t size = pkt->pkt_len + dev->vhost_hlen; >> + uint32_t size = pkt->pkt_len + sizeof(struct >> virtio_net_hdr_mrg_rxbuf); >> uint16_t num_buffers = 0; >> uint32_t buffer_len[vq->size]; >> uint16_t buffer_buf_id[vq->size]; >> @@ -1262,7 +1262,7 @@ virtio_dev_rx_batch_packed(struct virtio_net *dev, >> uint16_t avail_idx = vq->last_avail_idx; >> uint64_t desc_addrs[PACKED_BATCH_SIZE]; >> struct virtio_net_hdr_mrg_rxbuf *hdrs[PACKED_BATCH_SIZE]; >> - uint32_t buf_offset = dev->vhost_hlen; >> + uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); >> uint64_t lens[PACKED_BATCH_SIZE]; >> uint16_t ids[PACKED_BATCH_SIZE]; >> uint16_t i; >> @@ -1308,7 +1308,8 @@ virtio_dev_rx_batch_packed(struct virtio_net *dev, >> rte_prefetch0((void *)(uintptr_t)desc_addrs[i]); >> hdrs[i] = (struct virtio_net_hdr_mrg_rxbuf *) >> (uintptr_t)desc_addrs[i]; >> - lens[i] = pkts[i]->pkt_len + dev->vhost_hlen; >> + lens[i] = pkts[i]->pkt_len + >> + sizeof(struct virtio_net_hdr_mrg_rxbuf); >> } >> >> vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) >> @@ -2277,7 +2278,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net >> *dev, >> struct virtio_net_hdr *hdr; >> uint64_t lens[PACKED_BATCH_SIZE]; >> uint64_t buf_lens[PACKED_BATCH_SIZE]; >> - uint32_t buf_offset = dev->vhost_hlen; >> + uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); >> uint16_t flags, i; >> >> if (unlikely(avail_idx & PACKED_BATCH_MASK)) >> @@ -2354,7 +2355,7 @@ virtio_dev_tx_batch_packed(struct virtio_net *dev, >> struct rte_mbuf **pkts) >> { >> uint16_t avail_idx = vq->last_avail_idx; >> - uint32_t buf_offset = dev->vhost_hlen; >> + uint32_t buf_offset = sizeof(struct virtio_net_hdr_mrg_rxbuf); >> uintptr_t desc_addrs[PACKED_BATCH_SIZE]; >> uint16_t ids[PACKED_BATCH_SIZE]; >> uint16_t i; >> -- >> 2.26.2 > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [dpdk-dev] [PATCH 0/2] Fix packed ring header len 2020-10-01 10:11 [dpdk-dev] [PATCH 0/2] Fix packed ring header len Maxime Coquelin 2020-10-01 10:11 ` [dpdk-dev] [PATCH 1/2] vhost: fix Virtio-net header len with packed ring Maxime Coquelin 2020-10-01 10:11 ` [dpdk-dev] [PATCH 2/2] vhost: use fixed Virtio-net header len " Maxime Coquelin @ 2020-10-09 7:23 ` Maxime Coquelin 2 siblings, 0 replies; 7+ messages in thread From: Maxime Coquelin @ 2020-10-09 7:23 UTC (permalink / raw) To: dev, chenbo.xia, yong.liu On 10/1/20 12:11 PM, Maxime Coquelin wrote: > First patch is fixing packed ring header len. Issue was > reported by Marvin. > > Second patch is a (very) light optimization, to make used > directly of the static hdr len value in packed-ring only > functions. > > Maxime Coquelin (2): > vhost: fix Virtio-net header len with packed ring > vhost: use fixed Virtio-net header len packed ring > > lib/librte_vhost/vhost_user.c | 4 +++- > lib/librte_vhost/virtio_net.c | 11 ++++++----- > 2 files changed, 9 insertions(+), 6 deletions(-) > Applied to dpdk-next-virtio/main. Thanks, Maxime ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2020-10-09 7:23 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-10-01 10:11 [dpdk-dev] [PATCH 0/2] Fix packed ring header len Maxime Coquelin 2020-10-01 10:11 ` [dpdk-dev] [PATCH 1/2] vhost: fix Virtio-net header len with packed ring Maxime Coquelin 2020-10-09 6:33 ` Xia, Chenbo 2020-10-01 10:11 ` [dpdk-dev] [PATCH 2/2] vhost: use fixed Virtio-net header len " Maxime Coquelin 2020-10-09 6:36 ` Xia, Chenbo 2020-10-09 7:23 ` Maxime Coquelin 2020-10-09 7:23 ` [dpdk-dev] [PATCH 0/2] Fix packed ring header len Maxime Coquelin
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.