[v6,10/15] vhost: create descriptor mapping function

Message ID 20180702081629.29258-11-maxime.coquelin@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Maxime Coquelin
Headers
Series Vhost: add support to packed ring layout |

Checks

Context Check Description
ci/Intel-compilation fail Compilation issues

Commit Message

Maxime Coquelin July 2, 2018, 8:16 a.m. UTC
  Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
---
 lib/librte_vhost/virtio_net.c | 61 ++++++++++++++++++++++++++-----------------
 1 file changed, 37 insertions(+), 24 deletions(-)
  

Comments

Tiwei Bie July 4, 2018, 5:56 a.m. UTC | #1
On Mon, Jul 02, 2018 at 10:16:24AM +0200, Maxime Coquelin wrote:
> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
> ---
>  lib/librte_vhost/virtio_net.c | 61 ++++++++++++++++++++++++++-----------------
>  1 file changed, 37 insertions(+), 24 deletions(-)
> 
> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
> index f0e2e6a1f..64664b7de 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -292,6 +292,37 @@ virtio_enqueue_offload(struct rte_mbuf *m_buf, struct virtio_net_hdr *net_hdr)
>  	}
>  }
>  
> +static __rte_always_inline int
> +map_one_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
> +		struct buf_vector *buf_vec, uint16_t *vec_idx,
> +		uint64_t desc_iova, uint64_t desc_len, uint8_t perm)
> +{
> +	uint16_t vec_id = *vec_idx;
> +
> +	while (desc_len) {
> +		uint64_t desc_addr;
> +		uint64_t desc_chunck_len = desc_len;
> +
> +		desc_addr = vhost_iova_to_vva(dev, vq,
> +				desc_iova,
> +				&desc_chunck_len,
> +				perm);
> +		if (unlikely(!desc_addr))
> +			return -1;
> +
> +		buf_vec[vec_id].buf_iova = desc_iova;
> +		buf_vec[vec_id].buf_addr = desc_addr;
> +		buf_vec[vec_id].buf_len  = desc_chunck_len;
> +
> +		desc_len -= desc_chunck_len;
> +		desc_iova += desc_chunck_len;
> +		vec_id++;

FYI, a rebase on top of the latest "vhost: generalize
buffer vectors" series is needed to make sure that
access buf_vec[vec_id] won't cause overflow.

> +	}
> +	*vec_idx = vec_id;
> +
> +	return 0;
> +}
> +
[...]
  
Maxime Coquelin July 4, 2018, 4:18 p.m. UTC | #2
On 07/04/2018 07:56 AM, Tiwei Bie wrote:
> On Mon, Jul 02, 2018 at 10:16:24AM +0200, Maxime Coquelin wrote:
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>   lib/librte_vhost/virtio_net.c | 61 ++++++++++++++++++++++++++-----------------
>>   1 file changed, 37 insertions(+), 24 deletions(-)
>>
>> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
>> index f0e2e6a1f..64664b7de 100644
>> --- a/lib/librte_vhost/virtio_net.c
>> +++ b/lib/librte_vhost/virtio_net.c
>> @@ -292,6 +292,37 @@ virtio_enqueue_offload(struct rte_mbuf *m_buf, struct virtio_net_hdr *net_hdr)
>>   	}
>>   }
>>   
>> +static __rte_always_inline int
>> +map_one_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
>> +		struct buf_vector *buf_vec, uint16_t *vec_idx,
>> +		uint64_t desc_iova, uint64_t desc_len, uint8_t perm)
>> +{
>> +	uint16_t vec_id = *vec_idx;
>> +
>> +	while (desc_len) {
>> +		uint64_t desc_addr;
>> +		uint64_t desc_chunck_len = desc_len;
>> +
>> +		desc_addr = vhost_iova_to_vva(dev, vq,
>> +				desc_iova,
>> +				&desc_chunck_len,
>> +				perm);
>> +		if (unlikely(!desc_addr))
>> +			return -1;
>> +
>> +		buf_vec[vec_id].buf_iova = desc_iova;
>> +		buf_vec[vec_id].buf_addr = desc_addr;
>> +		buf_vec[vec_id].buf_len  = desc_chunck_len;
>> +
>> +		desc_len -= desc_chunck_len;
>> +		desc_iova += desc_chunck_len;
>> +		vec_id++;
> 
> FYI, a rebase on top of the latest "vhost: generalize
> buffer vectors" series is needed to make sure that
> access buf_vec[vec_id] won't cause overflow.

Right, this is fixed now that I rebased.

Thanks,
Maxime

>> +	}
>> +	*vec_idx = vec_id;
>> +
>> +	return 0;
>> +}
>> +
> [...]
>
  

Patch

diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
index f0e2e6a1f..64664b7de 100644
--- a/lib/librte_vhost/virtio_net.c
+++ b/lib/librte_vhost/virtio_net.c
@@ -292,6 +292,37 @@  virtio_enqueue_offload(struct rte_mbuf *m_buf, struct virtio_net_hdr *net_hdr)
 	}
 }
 
+static __rte_always_inline int
+map_one_desc(struct virtio_net *dev, struct vhost_virtqueue *vq,
+		struct buf_vector *buf_vec, uint16_t *vec_idx,
+		uint64_t desc_iova, uint64_t desc_len, uint8_t perm)
+{
+	uint16_t vec_id = *vec_idx;
+
+	while (desc_len) {
+		uint64_t desc_addr;
+		uint64_t desc_chunck_len = desc_len;
+
+		desc_addr = vhost_iova_to_vva(dev, vq,
+				desc_iova,
+				&desc_chunck_len,
+				perm);
+		if (unlikely(!desc_addr))
+			return -1;
+
+		buf_vec[vec_id].buf_iova = desc_iova;
+		buf_vec[vec_id].buf_addr = desc_addr;
+		buf_vec[vec_id].buf_len  = desc_chunck_len;
+
+		desc_len -= desc_chunck_len;
+		desc_iova += desc_chunck_len;
+		vec_id++;
+	}
+	*vec_idx = vec_id;
+
+	return 0;
+}
+
 static __rte_always_inline int
 fill_vec_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 			 uint32_t avail_idx, uint16_t *vec_idx,
@@ -301,7 +332,7 @@  fill_vec_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 	uint16_t idx = vq->avail->ring[avail_idx & (vq->size - 1)];
 	uint16_t vec_id = *vec_idx;
 	uint32_t len    = 0;
-	uint64_t dlen, desc_avail, desc_iova;
+	uint64_t dlen;
 	struct vring_desc *descs = vq->desc;
 	struct vring_desc *idesc = NULL;
 
@@ -339,30 +370,12 @@  fill_vec_buf_split(struct virtio_net *dev, struct vhost_virtqueue *vq,
 		}
 
 		len += descs[idx].len;
-		desc_avail = descs[idx].len;
-		desc_iova = descs[idx].addr;
-
-		while (desc_avail) {
-			uint64_t desc_addr;
-			uint64_t desc_chunck_len = desc_avail;
-
-			desc_addr = vhost_iova_to_vva(dev, vq,
-					desc_iova,
-					&desc_chunck_len,
-					perm);
-			if (unlikely(!desc_addr)) {
-				free_ind_table(idesc);
-				return -1;
-			}
-
-			buf_vec[vec_id].buf_iova = desc_iova;
-			buf_vec[vec_id].buf_addr = desc_addr;
-			buf_vec[vec_id].buf_len  = desc_chunck_len;
-			buf_vec[vec_id].desc_idx = idx;
 
-			desc_avail -= desc_chunck_len;
-			desc_iova += desc_chunck_len;
-			vec_id++;
+		if (unlikely(map_one_desc(dev, vq, buf_vec, &vec_id,
+						descs[idx].addr, descs[idx].len,
+						perm))) {
+			free_ind_table(idesc);
+			return -1;
 		}
 
 		if ((descs[idx].flags & VRING_DESC_F_NEXT) == 0)