DPDK patches and discussions
 help / color / mirror / Atom feed
* [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	[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	[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

* 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

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git