Use bulk alloc/free in virtio packed
cover Use bulk alloc/free in virtio packed
Use the faster bulk versions of alloc and free in virtio packed path.
Balazs Nemeth (4):
vhost: move allocation of mbuf outside of packet enqueue
vhost: perform all mbuf allocations in one loop
vhost: allocate and free packets in bulk
vhost: remove unnecessary level of indirection
lib/librte_vhost/virtio_net.c | 56 ++++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 17 deletions(-)
--
2.30.2
PATCH 1/4 vhost: move allocation of mbuf outside of packet enqueue
In preparation for subsequent patches, move mbuf allocation out and rename virtio_dev_pktmbuf_alloc to virtio_dev_pktmbuf_prep. This function now receives an already allocated mbuf pointer.
--- lib/librte_vhost/virtio_net.c | 54 ++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 14 deletions(-)
lib/librte_vhost/virtio_net.c +40 -14
@@ -2166,6 +2166,23 @@ virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp,return NULL ; } +static __rte_always_inline int +virtio_dev_pktmbuf_prep (struct virtio_net *dev,struct rte_mbuf *pkt, +uint32_t data_len) +{ +if (rte_pktmbuf_tailroom (pkt) >= data_len) +return 0 ; + +/* attach an external buffer if supported */ +if (dev->extbuf && !virtio_dev_extbuf_alloc (pkt, data_len)) +return 0 ; + +/* check if chained buffers are allowed */ +if (!dev->linearbuf) +return 0 ; +return 1 ; +} + static __rte_noinline uint16_tvirtio_dev_tx_split (struct virtio_net *dev,struct vhost_virtqueue *vq,struct rte_mempool *mbuf_pool,struct rte_mbuf **pkts,uint16_t count)
@@ -2259,7 +2276,6 @@ virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline int vhost_reserve_avail_batch_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, - struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t avail_idx, uintptr_t *desc_addrs,
@@ -2304,8 +2320,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev, }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) { - pkts[i] =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, lens[i]); -if (!pkts[i]) +if (virtio_dev_pktmbuf_prep (dev, pkts[i], lens[i])) goto free_buf; }
@@ -2326,16 +2341,12 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev,return 0 ; free_buf: -for (i =0 ; i <PACKED_BATCH_SIZE ; i++) -rte_pktmbuf_free (pkts[i]); -return -1 ; } static __rte_always_inline intvirtio_dev_tx_batch_packed (struct virtio_net *dev,struct vhost_virtqueue *vq, -struct rte_mempool *mbuf_pool,struct rte_mbuf **pkts) {uint16_t avail_idx = vq->last_avail_idx;
@@ -2345,8 +2356,8 @@ virtio_dev_tx_batch_packed(struct virtio_net *dev,uint16_t ids[PACKED_BATCH_SIZE ];uint16_t i; -if (vhost_reserve_avail_batch_packed (dev, vq, mbuf_pool, pkts, - avail_idx, desc_addrs, ids)) +if (vhost_reserve_avail_batch_packed (dev, vq, pkts, avail_idx, + desc_addrs, ids))return -1 ;vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE )
@@ -2396,8 +2407,8 @@ vhost_dequeue_single_packed(struct virtio_net *dev, VHOST_ACCESS_RO) < 0))return -1 ; - *pkts =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, buf_len); -if (unlikely (*pkts ==NULL )) { + +if (unlikely (virtio_dev_pktmbuf_prep (dev, *pkts, buf_len))) {if (!allocerr_warned) { VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d from %s on %s.\n",
@@ -2416,7 +2427,6 @@ vhost_dequeue_single_packed(struct virtio_net *dev, dev->ifname); allocerr_warned = true; } -rte_pktmbuf_free (*pkts);return -1 ; }
@@ -2459,22 +2469,38 @@ virtio_dev_tx_packed(struct virtio_net *dev, {uint32_t pkt_idx =0 ;uint32_t remained = count; +uint16_t i; do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]); if (remained >= PACKED_BATCH_SIZE) { - if (!virtio_dev_tx_batch_packed(dev, vq, mbuf_pool, + vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) { + pkts[pkt_idx + i] = + rte_pktmbuf_alloc(mbuf_pool); + } + + if (!virtio_dev_tx_batch_packed(dev, vq, &pkts[pkt_idx])) { pkt_idx += PACKED_BATCH_SIZE; remained -= PACKED_BATCH_SIZE; + continue; + } else { + vhost_for_each_try_unroll(i, 0, + PACKED_BATCH_SIZE) { + rte_pktmbuf_free(pkts[pkt_idx + i]); + } } } + pkts[pkt_idx] = rte_pktmbuf_alloc(mbuf_pool); + if (virtio_dev_tx_single_packed(dev, vq, mbuf_pool, - &pkts[pkt_idx])) + &pkts[pkt_idx])) { + rte_pktmbuf_free(pkts[pkt_idx]); break; + } pkt_idx++; remained--;
Signature · 1 line
2.30.2
PATCH v2 1/4 vhost: move allocation of mbuf outside of packet enqueue
In preparation for subsequent patches, move mbuf allocation out and rename virtio_dev_pktmbuf_alloc to virtio_dev_pktmbuf_prep. This function now receives an already allocated mbuf pointer.
--- lib/librte_vhost/virtio_net.c | 54 ++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 14 deletions(-)
lib/librte_vhost/virtio_net.c +40 -14
@@ -2166,6 +2166,23 @@ virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp,return NULL ; } +static __rte_always_inline int +virtio_dev_pktmbuf_prep (struct virtio_net *dev,struct rte_mbuf *pkt, +uint32_t data_len) +{ +if (rte_pktmbuf_tailroom (pkt) >= data_len) +return 0 ; + +/* attach an external buffer if supported */ +if (dev->extbuf && !virtio_dev_extbuf_alloc (pkt, data_len)) +return 0 ; + +/* check if chained buffers are allowed */ +if (!dev->linearbuf) +return 0 ; +return 1 ; +} + static __rte_noinline uint16_tvirtio_dev_tx_split (struct virtio_net *dev,struct vhost_virtqueue *vq,struct rte_mempool *mbuf_pool,struct rte_mbuf **pkts,uint16_t count)
@@ -2259,7 +2276,6 @@ virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline int vhost_reserve_avail_batch_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, - struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t avail_idx, uintptr_t *desc_addrs,
@@ -2304,8 +2320,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev, }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) { - pkts[i] =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, lens[i]); -if (!pkts[i]) +if (virtio_dev_pktmbuf_prep (dev, pkts[i], lens[i])) goto free_buf; }
@@ -2326,16 +2341,12 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev,return 0 ; free_buf: -for (i =0 ; i <PACKED_BATCH_SIZE ; i++) -rte_pktmbuf_free (pkts[i]); -return -1 ; } static __rte_always_inline intvirtio_dev_tx_batch_packed (struct virtio_net *dev,struct vhost_virtqueue *vq, -struct rte_mempool *mbuf_pool,struct rte_mbuf **pkts) {uint16_t avail_idx = vq->last_avail_idx;
@@ -2345,8 +2356,8 @@ virtio_dev_tx_batch_packed(struct virtio_net *dev,uint16_t ids[PACKED_BATCH_SIZE ];uint16_t i; -if (vhost_reserve_avail_batch_packed (dev, vq, mbuf_pool, pkts, - avail_idx, desc_addrs, ids)) +if (vhost_reserve_avail_batch_packed (dev, vq, pkts, avail_idx, + desc_addrs, ids))return -1 ;vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE )
@@ -2396,8 +2407,8 @@ vhost_dequeue_single_packed(struct virtio_net *dev, VHOST_ACCESS_RO) < 0))return -1 ; - *pkts =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, buf_len); -if (unlikely (*pkts ==NULL )) { + +if (unlikely (virtio_dev_pktmbuf_prep (dev, *pkts, buf_len))) {if (!allocerr_warned) { VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d from %s on %s.\n",
@@ -2416,7 +2427,6 @@ vhost_dequeue_single_packed(struct virtio_net *dev, dev->ifname); allocerr_warned = true; } -rte_pktmbuf_free (*pkts);return -1 ; }
@@ -2459,22 +2469,38 @@ virtio_dev_tx_packed(struct virtio_net *dev, {uint32_t pkt_idx =0 ;uint32_t remained = count; +uint16_t i; do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]); if (remained >= PACKED_BATCH_SIZE) { - if (!virtio_dev_tx_batch_packed(dev, vq, mbuf_pool, + vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) { + pkts[pkt_idx + i] = + rte_pktmbuf_alloc(mbuf_pool); + } + + if (!virtio_dev_tx_batch_packed(dev, vq, &pkts[pkt_idx])) { pkt_idx += PACKED_BATCH_SIZE; remained -= PACKED_BATCH_SIZE; + continue; + } else { + vhost_for_each_try_unroll(i, 0, + PACKED_BATCH_SIZE) { + rte_pktmbuf_free(pkts[pkt_idx + i]); + } } } + pkts[pkt_idx] = rte_pktmbuf_alloc(mbuf_pool); + if (virtio_dev_tx_single_packed(dev, vq, mbuf_pool, - &pkts[pkt_idx])) + &pkts[pkt_idx])) { + rte_pktmbuf_free(pkts[pkt_idx]); break; + } pkt_idx++; remained--;
Signature · 1 line
2.30.2
Re: [dpdk-dev] [PATCH v2 1/4] vhost: move allocation of mbuf outside of packet enqueue
Hi Balazs,
Hint for future revisions, please add a cover letter when multiple patches, it makes the series handling for the maintainer easier. Also, please use the MAINTAINERS file to add the other maintainers.
Quoted 21 lines · Balazs Nemeth
In preparation for subsequent patches, move mbuf allocation out and rename
virtio_dev_pktmbuf_alloctovirtio_dev_pktmbuf_prep. This function now receives an already allocated mbuf pointer.Signed-off-by Balazs Nemeth <[email protected]>--- lib/librte_vhost/virtio_net.c | 54 ++++++++++++++++++++++++++--------- 1 file changed, 40 insertions(+), 14 deletions(-)lib/librte_vhost/virtio_net.c +4
-0@@ -2166,6 +2166,23 @@ virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp,return NULL ; } +static __rte_always_inline int +virtio_dev_pktmbuf_prep (struct virtio_net *dev,struct rte_mbuf *pkt, +uint32_t data_len) +{
lib/librte_vhost/virtio_net.c virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp, · line 2182
Quoted 10 lines
lib/librte_vhost/virtio_net.c +10
-0+if (rte_pktmbuf_tailroom (pkt) >= data_len) +return 0 ; + +/* attach an external buffer if supported */ +if (dev->extbuf && !virtio_dev_extbuf_alloc (pkt, data_len)) +return 0 ; + +/* check if chained buffers are allowed */ +if (!dev->linearbuf) +return 0 ;
Add new line here.
lib/librte_vhost/virtio_net.c virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp, · line 2183
Quoted 1 line
lib/librte_vhost/virtio_net.c +1
-0+return 1 ;
Maybe return a negative value for consistency.
Quoted 24 lines
lib/librte_vhost/virtio_net.c +3
-3+} + static __rte_noinline uint16_tvirtio_dev_tx_split (struct virtio_net *dev,struct vhost_virtqueue *vq,struct rte_mempool *mbuf_pool,struct rte_mbuf **pkts,uint16_t count)@@ -2259,7 +2276,6 @@ virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline int vhost_reserve_avail_batch_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, - struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t avail_idx, uintptr_t *desc_addrs,@@ -2304,8 +2320,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev, }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) { - pkts[i] =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, lens[i]); -if (!pkts[i]) +if (virtio_dev_pktmbuf_prep (dev, pkts[i], lens[i])) goto free_buf; }
lib/librte_vhost/virtio_net.c vhost_reserve_avail_batch_packed(struct virtio_net *dev, · line 2343
Quoted 3 lines
lib/librte_vhost/virtio_net.c +0
-0@@ -2326,16 +2341,12 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev,return 0 ; free_buf:
It is no more freeing here, so better to rename the label.
Quoted 47 lines
lib/librte_vhost/virtio_net.c +4
-9-for (i =0 ; i <PACKED_BATCH_SIZE ; i++) -rte_pktmbuf_free (pkts[i]); -return -1 ; } static __rte_always_inline intvirtio_dev_tx_batch_packed (struct virtio_net *dev,struct vhost_virtqueue *vq, -struct rte_mempool *mbuf_pool,struct rte_mbuf **pkts) {uint16_t avail_idx = vq->last_avail_idx;@@ -2345,8 +2356,8 @@ virtio_dev_tx_batch_packed(struct virtio_net *dev,uint16_t ids[PACKED_BATCH_SIZE ];uint16_t i; -if (vhost_reserve_avail_batch_packed (dev, vq, mbuf_pool, pkts, - avail_idx, desc_addrs, ids)) +if (vhost_reserve_avail_batch_packed (dev, vq, pkts, avail_idx, + desc_addrs, ids))return -1 ;vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE )@@ -2396,8 +2407,8 @@ vhost_dequeue_single_packed(struct virtio_net *dev, VHOST_ACCESS_RO) < 0))return -1 ; - *pkts =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, buf_len); -if (unlikely (*pkts ==NULL )) { + +if (unlikely (virtio_dev_pktmbuf_prep (dev, *pkts, buf_len))) {if (!allocerr_warned) { VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d from %s on %s.\n",@@ -2416,7 +2427,6 @@ vhost_dequeue_single_packed(struct virtio_net *dev, dev->ifname); allocerr_warned = true; } -rte_pktmbuf_free (*pkts);return -1 ; }@@ -2459,22 +2469,38 @@ virtio_dev_tx_packed(struct virtio_net *dev, {uint32_t pkt_idx =0 ;uint32_t remained = count;
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2480
Quoted 10 lines
lib/librte_vhost/virtio_net.c +4
-1+uint16_t i; do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]); if (remained >= PACKED_BATCH_SIZE) { - if (!virtio_dev_tx_batch_packed(dev, vq, mbuf_pool, + vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) { + pkts[pkt_idx + i] = + rte_pktmbuf_alloc(mbuf_pool);
No check on whether the alloc succeeded?
Also, we recently move to up to 100 chars lines, so maybe it can fit in a single one here now.
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2484
Quoted 4 lines
lib/librte_vhost/virtio_net.c +3
-0+ } + +if (!virtio_dev_tx_batch_packed (dev, vq, &pkts[pkt_idx])) {
Ditto
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2491
Quoted 7 lines
lib/librte_vhost/virtio_net.c +4
-0pkt_idx +=PACKED_BATCH_SIZE ; remained -=PACKED_BATCH_SIZE ; +continue ; + }else { +vhost_for_each_try_unroll (i,0 , +PACKED_BATCH_SIZE ) {
Same here
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2497
Quoted 6 lines
lib/librte_vhost/virtio_net.c +3
-0+rte_pktmbuf_free (pkts[pkt_idx + i]); + } } } + pkts[pkt_idx] =rte_pktmbuf_alloc (mbuf_pool);
Here also you may want to ensure the allocation succeeded.
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2506
Quoted 11 lines
lib/librte_vhost/virtio_net.c +4
-1+if (virtio_dev_tx_single_packed (dev, vq, mbuf_pool, - &pkts[pkt_idx])) + &pkts[pkt_idx])) { +rte_pktmbuf_free (pkts[pkt_idx]);break ; + } pkt_idx++; remained--;
Maxime
PATCH v3 1/1 vhost: allocate and free packets in bulk
In reply to Balazs NemethMove allocation out further and perform all allocation in bulk. The same goes for freeing packets. In the process, also rename virtio_dev_pktmbuf_alloc to virtio_dev_pktmbuf_prep. This function now receives an already allocated mbuf pointer.
--- lib/librte_vhost/virtio_net.c | 58 +++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 20 deletions(-)
lib/librte_vhost/virtio_net.c +38 -20
@@ -2168,6 +2168,24 @@ virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp,return NULL ; } +static __rte_always_inline int +virtio_dev_pktmbuf_prep (struct virtio_net *dev,struct rte_mbuf *pkt, +uint32_t data_len) +{ +if (rte_pktmbuf_tailroom (pkt) >= data_len) +return 0 ; + +/* attach an external buffer if supported */ +if (dev->extbuf && !virtio_dev_extbuf_alloc (pkt, data_len)) +return 0 ; + +/* check if chained buffers are allowed */ +if (!dev->linearbuf) +return 0 ; + +return -1 ; +} + static __rte_noinline uint16_tvirtio_dev_tx_split (struct virtio_net *dev,struct vhost_virtqueue *vq,struct rte_mempool *mbuf_pool,struct rte_mbuf **pkts,uint16_t count)
@@ -2261,7 +2279,6 @@ virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline int vhost_reserve_avail_batch_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, - struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t avail_idx, uintptr_t *desc_addrs,
@@ -2306,9 +2323,8 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev, }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) { - pkts[i] =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, lens[i]); -if (!pkts[i]) - goto free_buf; +if (virtio_dev_pktmbuf_prep (dev, pkts[i], lens[i])) + goto err; }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE )
@@ -2316,7 +2332,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev,vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) {if (unlikely (buf_lens[i] < (lens[i] - buf_offset))) - goto free_buf; + goto err; }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) {
@@ -2327,17 +2343,13 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev,return 0 ; -free_buf: -for (i =0 ; i <PACKED_BATCH_SIZE ; i++) -rte_pktmbuf_free (pkts[i]); - +err:return -1 ; } static __rte_always_inline intvirtio_dev_tx_batch_packed (struct virtio_net *dev,struct vhost_virtqueue *vq, -struct rte_mempool *mbuf_pool,struct rte_mbuf **pkts) {uint16_t avail_idx = vq->last_avail_idx;
@@ -2347,8 +2359,8 @@ virtio_dev_tx_batch_packed(struct virtio_net *dev,uint16_t ids[PACKED_BATCH_SIZE ];uint16_t i; -if (vhost_reserve_avail_batch_packed (dev, vq, mbuf_pool, pkts, - avail_idx, desc_addrs, ids)) +if (vhost_reserve_avail_batch_packed (dev, vq, pkts, avail_idx, + desc_addrs, ids))return -1 ;vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE )
@@ -2381,7 +2393,7 @@ static __rte_always_inline int vhost_dequeue_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts, + struct rte_mbuf *pkts, uint16_t *buf_id, uint16_t *desc_count) {
@@ -2398,8 +2410,8 @@ vhost_dequeue_single_packed(struct virtio_net *dev, VHOST_ACCESS_RO) < 0))return -1 ; - *pkts =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, buf_len); -if (unlikely (*pkts ==NULL )) { + +if (unlikely (virtio_dev_pktmbuf_prep (dev, pkts, buf_len))) {if (!allocerr_warned) { VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d from %s on %s.\n",
@@ -2409,7 +2421,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev,return -1 ; } - err =copy_desc_to_mbuf (dev, vq, buf_vec, nr_vec, *pkts, + err =copy_desc_to_mbuf (dev, vq, buf_vec, nr_vec, pkts, mbuf_pool);if (unlikely (err)) {if (!allocerr_warned) {
@@ -2418,7 +2430,6 @@ vhost_dequeue_single_packed(struct virtio_net *dev, dev->ifname); allocerr_warned = true; } -rte_pktmbuf_free (*pkts);return -1 ; }
@@ -2429,7 +2440,7 @@ static __rte_always_inline int virtio_dev_tx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts) + struct rte_mbuf *pkts) { uint16_t buf_id, desc_count = 0;
@@ -2462,26 +2473,33 @@ virtio_dev_tx_packed(struct virtio_net *dev,uint32_t pkt_idx =0 ;uint32_t remained = count; +if (rte_pktmbuf_alloc_bulk (mbuf_pool, pkts, count)) +return 0 ; +do {rte_prefetch0 (&vq->desc_packed[vq->last_avail_idx]);if (remained >=PACKED_BATCH_SIZE ) { -if (!virtio_dev_tx_batch_packed (dev, vq, mbuf_pool, +if (!virtio_dev_tx_batch_packed (dev, vq, &pkts[pkt_idx])) { pkt_idx +=PACKED_BATCH_SIZE ; remained -=PACKED_BATCH_SIZE ; +continue ; } }if (virtio_dev_tx_single_packed (dev, vq, mbuf_pool, - &pkts[pkt_idx])) + pkts[pkt_idx]))break ; pkt_idx++; remained--; }while (remained); +if (pkt_idx != count) +rte_pktmbuf_free_bulk (&pkts[pkt_idx], count - pkt_idx); +if (vq->shadow_used_idx) {do_data_copy_dequeue (vq);
Signature · 1 line
2.30.2
Re: [dpdk-dev] [PATCH v3] vhost: allocate and free packets in bulk
Quoted 10 lines · Balazs Nemeth
Move allocation out further and perform all allocation in bulk. The same goes for freeing packets. In the process, also rename
virtio_dev_pktmbuf_alloctovirtio_dev_pktmbuf_prep. This function now receives an already allocated mbuf pointer.Signed-off-by Balazs Nemeth <[email protected]>--- lib/librte_vhost/virtio_net.c | 58 +++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 20 deletions(-)
Thanks,
Maxime
Re: [dpdk-dev] [PATCH v3] vhost: allocate and free packets in bulk
In reply to Balazs NemethQuoted 7 lines · Balazs Nemeth
Move allocation out further and perform all allocation in bulk. The same goes for freeing packets. In the process, also rename
virtio_dev_pktmbuf_alloctovirtio_dev_pktmbuf_prep. This function now receives an already allocated mbuf pointer.Signed-off-by Balazs Nemeth <[email protected]>
The title should indicate we are only touching the tx packed path.
What about tx split?
If it is too complex to rework, this can wait.
Quoted 20 lines
--- lib/librte_vhost/virtio_net.c | 58 +++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 20 deletions(-)lib/librte_vhost/virtio_net.c +8
-0@@ -2168,6 +2168,24 @@ virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp,return NULL ; } +static __rte_always_inline int +virtio_dev_pktmbuf_prep (struct virtio_net *dev,struct rte_mbuf *pkt, +uint32_t data_len) +{ +if (rte_pktmbuf_tailroom (pkt) >= data_len) +return 0 ; + +/* attach an external buffer if supported */
lib/librte_vhost/virtio_net.c virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp, · line 2188
Quoted 10 lines
lib/librte_vhost/virtio_net.c +10
-0+if (dev->extbuf && !virtio_dev_extbuf_alloc (pkt, data_len)) +return 0 ; + +/* check if chained buffers are allowed */ +if (!dev->linearbuf) +return 0 ; + +return -1 ; +} +
If virtio_dev_pktmbuf_alloc() uses this new helper, we avoid duplicating the logic.
Quoted 3 lines
lib/librte_vhost/virtio_net.c +0
-0static __rte_noinline uint16_tvirtio_dev_tx_split (struct virtio_net *dev,struct vhost_virtqueue *vq,struct rte_mempool *mbuf_pool,struct rte_mbuf **pkts,uint16_t count)
[snip]
Quoted 16 lines
lib/librte_vhost/virtio_net.c +4
-1@@ -2429,7 +2440,7 @@ static __rte_always_inline int virtio_dev_tx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts) + struct rte_mbuf *pkts) { uint16_t buf_id, desc_count = 0;@@ -2462,26 +2473,33 @@ virtio_dev_tx_packed(struct virtio_net *dev,uint32_t pkt_idx =0 ;uint32_t remained = count; +if (rte_pktmbuf_alloc_bulk (mbuf_pool, pkts, count)) +return 0 ; +
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2487
Quoted 10 lines
lib/librte_vhost/virtio_net.c +2
-1do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]); if (remained >= PACKED_BATCH_SIZE) { - if (!virtio_dev_tx_batch_packed(dev, vq, mbuf_pool, + if (!virtio_dev_tx_batch_packed(dev, vq, &pkts[pkt_idx])) { pkt_idx += PACKED_BATCH_SIZE; remained -= PACKED_BATCH_SIZE; +
No need for the extra line.
Quoted 9 lines
lib/librte_vhost/virtio_net.c +1
-1continue ; } }if (virtio_dev_tx_single_packed (dev, vq, mbuf_pool, - &pkts[pkt_idx])) + pkts[pkt_idx]))break ; pkt_idx++;
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2505
Quoted 10 lines
lib/librte_vhost/virtio_net.c +3
-0remained--; }while (remained); +if (pkt_idx != count) +rte_pktmbuf_free_bulk (&pkts[pkt_idx], count - pkt_idx); +if (vq->shadow_used_idx) {do_data_copy_dequeue (vq);
With those comments addressed,
Thanks Balazs!
Signature · 1 line
David Marchand
Re: [dpdk-dev] [PATCH v3] vhost: allocate and free packets in bulk
Hi David,
I was also thinking about using the same idea for tx split so that virtio_dev_pktmbuf_alloc can be removed completely. I don't have those patches yet. However, I can make virtio_dev_pktmbuf_alloc use virtio_dev_pktmbuf_prep for this patch that addresses the packed version and submit other patches for tx split later. Would that work for now?
Regards,
Balazs
Quoted 121 lines · David Marchand
Quoted 7 lines · Balazs Nemeth
Move allocation out further and perform all allocation in bulk. The same goes for freeing packets. In the process, also rename
virtio_dev_pktmbuf_alloctovirtio_dev_pktmbuf_prep. This function now receives an already allocated mbuf pointer.Signed-off-by Balazs Nemeth <[email protected]>The title should indicate we are only touching the tx packed path.
What about tx split?
If it is too complex to rework, this can wait.Review
Quoted 5 lines
--- lib/librte_vhost/virtio_net.c | 58 +++++++++++++++++++++++------------ 1 file changed, 38 insertions(+), 20 deletions(-)(quoted diff) +0
-0
b/lib/librte_vhost/virtio_net.clib/librte_vhost/virtio_net.c virtio_dev_pktmbuf_alloc(struct virtio_net *dev,
Quoted 4 lines
index
ff39878609..d6d5636e0f100644lib/librte_vhost/virtio_net.c +0
-0@@ -2168,6 +2168,24 @@ virtio_dev_pktmbuf_alloc(struct virtio_net *dev,struct
rte_mempool*mp,Quoted 11 lines
lib/librte_vhost/virtio_net.c +8
-0return NULL ; } +static __rte_always_inline int +virtio_dev_pktmbuf_prep (struct virtio_net *dev,struct rte_mbuf *pkt, +uint32_t data_len) +{ +if (rte_pktmbuf_tailroom (pkt) >= data_len) +return 0 ; + +/* attach an external buffer if supported */ lib/librte_vhost/virtio_net.c virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp, · line 2188
Quoted 10 lines
lib/librte_vhost/virtio_net.c +10
-0+if (dev->extbuf && !virtio_dev_extbuf_alloc (pkt, data_len)) +return 0 ; + +/* check if chained buffers are allowed */ +if (!dev->linearbuf) +return 0 ; + +return -1 ; +} +If
virtio_dev_pktmbuf_alloc()uses this new helper, we avoid duplicating the logic.lib/librte_vhost/virtio_net.c virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp,
Quoted 3 lines
lib/librte_vhost/virtio_net.c +0
-0static __rte_noinline uint16_tvirtio_dev_tx_split (struct virtio_net *dev,struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_tcount)
[snip]
Quoted 11 lines
lib/librte_vhost/virtio_net.c +1
-1@@ -2429,7 +2440,7 @@ static __rte_always_inline int virtio_dev_tx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts) + struct rte_mbuf *pkts) { uint16_t buf_id, desc_count = 0;@@ -2462,26 +2473,33 @@ virtio_dev_tx_packed(struct virtio_net *dev,uint32_t pkt_idx =0 ;lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2469
Quoted 10 lines
lib/librte_vhost/virtio_net.c +3
-1uint32_t remained = count; +if (rte_pktmbuf_alloc_bulk (mbuf_pool, pkts, count)) +return 0 ; + do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]); if (remained >= PACKED_BATCH_SIZE) { - if (!virtio_dev_tx_batch_packed(dev, vq,
mbuf_pool,lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2484
Quoted 2 lines
lib/librte_vhost/virtio_net.c +1
-0+if (!virtio_dev_tx_batch_packed (dev, vq, &pkts[pkt_idx])){
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2487
Quoted 3 lines
lib/librte_vhost/virtio_net.c +1
-0pkt_idx +=PACKED_BATCH_SIZE ; remained -=PACKED_BATCH_SIZE ; +No need for the extra line.
Quoted 9 lines
lib/librte_vhost/virtio_net.c +1
-1continue ; } }if (virtio_dev_tx_single_packed (dev, vq, mbuf_pool, - &pkts[pkt_idx])) + pkts[pkt_idx]))break ; pkt_idx++;lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2505
Quoted 10 lines
lib/librte_vhost/virtio_net.c +3
-0remained--; }while (remained); +if (pkt_idx != count) +rte_pktmbuf_free_bulk (&pkts[pkt_idx], count - pkt_idx); +if (vq->shadow_used_idx) {do_data_copy_dequeue (vq);With those comments addressed,
Reviewed-by David Marchand <[email protected]>Thanks Balazs!
--
David Marchand
Re: [dpdk-dev] [PATCH v3] vhost: allocate and free packets in bulk
Hi Balazs,
Quoted 8 lines · Balazs Nemeth
Hi David,
I was also thinking about using the same idea for tx split so that
virtio_dev_pktmbuf_alloccan be removed completely. I don't have those patches yet. However, I can makevirtio_dev_pktmbuf_allocusevirtio_dev_pktmbuf_prepfor this patch that addresses the packed version and submit other patches for tx split later. Would that work for now?
I think so.
That would be great to have the same for split ring, but more rework may be necessary than for packed ring. So I'm fine if the split ring part is done after this release.
Thanks,
Maxime
Quoted 133 lines
Regards,
BalazsOn Fri, Apr 16, 2021 at 11:05 AM David Marchand <[email protected] <mailto:[email protected]>> wrote:
On Fri, Apr 16, 2021 at 10:18 AM Balazs Nemeth <[email protected] <mailto:[email protected]>> wrote: > > Move allocation out further and perform all allocation in bulk. The same > goes for freeing packets. In the process, also rename > virtio_dev_pktmbuf_alloc to virtio_dev_pktmbuf_prep. This > function now receives an already allocated mbuf pointer. > > Signed-off-by: Balazs Nemeth <[email protected] <mailto:[email protected]>> The title should indicate we are only touching the tx packed path. What about tx split? If it is too complex to rework, this can wait. > --- > lib/librte_vhost/virtio_net.c | 58 +++++++++++++++++++++++------------ > 1 file changed, 38 insertions(+), 20 deletions(-) > > diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c > index ff39878609..d6d5636e0f 100644 > --- a/lib/librte_vhost/virtio_net.c > +++ b/lib/librte_vhost/virtio_net.c > @@ -2168,6 +2168,24 @@ virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp, > return NULL; > } > > +static __rte_always_inline int > +virtio_dev_pktmbuf_prep(struct virtio_net *dev, struct rte_mbuf *pkt, > + uint32_t data_len) > +{ > + if (rte_pktmbuf_tailroom(pkt) >= data_len) > + return 0; > + > + /* attach an external buffer if supported */ > + if (dev->extbuf && !virtio_dev_extbuf_alloc(pkt, data_len)) > + return 0; > + > + /* check if chained buffers are allowed */ > + if (!dev->linearbuf) > + return 0; > + > + return -1; > +} > + If virtio_dev_pktmbuf_alloc() uses this new helper, we avoid duplicating the logic. > static __rte_noinline uint16_t > virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, > struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t count) [snip] > @@ -2429,7 +2440,7 @@ static __rte_always_inline int > virtio_dev_tx_single_packed(struct virtio_net *dev, > struct vhost_virtqueue *vq, > struct rte_mempool *mbuf_pool, > - struct rte_mbuf **pkts) > + struct rte_mbuf *pkts) > { > > uint16_t buf_id, desc_count = 0; > @@ -2462,26 +2473,33 @@ virtio_dev_tx_packed(struct virtio_net *dev, > uint32_t pkt_idx = 0; > uint32_t remained = count; > > + if (rte_pktmbuf_alloc_bulk(mbuf_pool, pkts, count)) > + return 0; > + > do { > rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]); > > if (remained >= PACKED_BATCH_SIZE) { > - if (!virtio_dev_tx_batch_packed(dev, vq, mbuf_pool, > + if (!virtio_dev_tx_batch_packed(dev, vq, > &pkts[pkt_idx])) { > pkt_idx += PACKED_BATCH_SIZE; > remained -= PACKED_BATCH_SIZE; > + No need for the extra line. > continue; > } > } > > if (virtio_dev_tx_single_packed(dev, vq, mbuf_pool, > - &pkts[pkt_idx])) > + pkts[pkt_idx])) > break; > pkt_idx++; > remained--; > > } while (remained); > > + if (pkt_idx != count) > + rte_pktmbuf_free_bulk(&pkts[pkt_idx], count - pkt_idx); > + > if (vq->shadow_used_idx) { > do_data_copy_dequeue(vq); > With those comments addressed, Reviewed-by: David Marchand <[email protected] <mailto:[email protected]>> Thanks Balazs! -- David Marchand
Re: [dpdk-dev] [PATCH v3] vhost: allocate and free packets in bulk
In reply to Balazs NemethQuoted 1 line · Balazs Nemeth
I was also thinking about using the same idea for tx split so that
virtio_dev_pktmbuf_alloccan be removed completely. I don't have those patches yet. However, I can makevirtio_dev_pktmbuf_allocusevirtio_dev_pktmbuf_prepfor this patch that addresses the packed version and submit other patches for tx split later. Would that work for now?
Ok for me.
And revisit the tx split part in 21.08 :-).
Signature · 1 line
David Marchand
PATCH v4 1/1 vhost: allocate and free packets in bulk
In reply to Balazs NemethMove allocation out further and perform all allocation in bulk. The same goes for freeing packets. In the process, also introduce virtio_dev_pktmbuf_prep and make virtio_dev_pktmbuf_alloc use that.
--- lib/librte_vhost/virtio_net.c | 80 +++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 36 deletions(-)
lib/librte_vhost/virtio_net.c +44 -36
@@ -2134,6 +2134,24 @@ virtio_dev_extbuf_alloc(struct rte_mbuf *pkt, uint32_t size)return 0 ; } +static __rte_always_inline int +virtio_dev_pktmbuf_prep (struct virtio_net *dev,struct rte_mbuf *pkt, +uint32_t data_len) +{ +if (rte_pktmbuf_tailroom (pkt) >= data_len) +return 0 ; + +/* attach an external buffer if supported */ +if (dev->extbuf && !virtio_dev_extbuf_alloc (pkt, data_len)) +return 0 ; + +/* check if chained buffers are allowed */ +if (!dev->linearbuf) +return 0 ; + +return -1 ; +} +/* * Allocate a host supported pktmbuf. */
@@ -2149,23 +2167,15 @@ virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp,return NULL ; } -if (rte_pktmbuf_tailroom (pkt) >= data_len) -return pkt; +if (virtio_dev_pktmbuf_prep (dev, pkt, data_len)) { +/* Data doesn't fit into the buffer and the host supports +* only linear buffers +*/ +rte_pktmbuf_free (pkt); +return NULL ; + } -/* attach an external buffer if supported */ -if (dev->extbuf && !virtio_dev_extbuf_alloc (pkt, data_len)) -return pkt; - -/* check if chained buffers are allowed */ -if (!dev->linearbuf) -return pkt; - -/* Data doesn't fit into the buffer and the host supports -* only linear buffers -*/ -rte_pktmbuf_free (pkt); - -return NULL ; + return pkt }static __rte_noinline uint16_t
@@ -2261,7 +2271,6 @@ virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline int vhost_reserve_avail_batch_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, - struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t avail_idx, uintptr_t *desc_addrs,
@@ -2306,9 +2315,8 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev, }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) { - pkts[i] =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, lens[i]); -if (!pkts[i]) - goto free_buf; +if (virtio_dev_pktmbuf_prep (dev, pkts[i], lens[i])) + goto err; }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE )
@@ -2316,7 +2324,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev,vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) {if (unlikely (buf_lens[i] < (lens[i] - buf_offset))) - goto free_buf; + goto err; }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) {
@@ -2327,17 +2335,13 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev,return 0 ; -free_buf: -for (i =0 ; i <PACKED_BATCH_SIZE ; i++) -rte_pktmbuf_free (pkts[i]); - +err:return -1 ; } static __rte_always_inline intvirtio_dev_tx_batch_packed (struct virtio_net *dev,struct vhost_virtqueue *vq, -struct rte_mempool *mbuf_pool,struct rte_mbuf **pkts) {uint16_t avail_idx = vq->last_avail_idx;
@@ -2347,8 +2351,8 @@ virtio_dev_tx_batch_packed(struct virtio_net *dev,uint16_t ids[PACKED_BATCH_SIZE ];uint16_t i; -if (vhost_reserve_avail_batch_packed (dev, vq, mbuf_pool, pkts, - avail_idx, desc_addrs, ids)) +if (vhost_reserve_avail_batch_packed (dev, vq, pkts, avail_idx, + desc_addrs, ids))return -1 ;vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE )
@@ -2381,7 +2385,7 @@ static __rte_always_inline int vhost_dequeue_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts, + struct rte_mbuf *pkts, uint16_t *buf_id, uint16_t *desc_count) {
@@ -2398,8 +2402,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev, VHOST_ACCESS_RO) < 0))return -1 ; - *pkts =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, buf_len); -if (unlikely (*pkts ==NULL )) { +if (unlikely (virtio_dev_pktmbuf_prep (dev, pkts, buf_len))) {if (!allocerr_warned) { VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d from %s on %s.\n",
@@ -2409,7 +2412,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev,return -1 ; } - err =copy_desc_to_mbuf (dev, vq, buf_vec, nr_vec, *pkts, + err =copy_desc_to_mbuf (dev, vq, buf_vec, nr_vec, pkts, mbuf_pool);if (unlikely (err)) {if (!allocerr_warned) {
@@ -2418,7 +2421,6 @@ vhost_dequeue_single_packed(struct virtio_net *dev, dev->ifname); allocerr_warned = true; } -rte_pktmbuf_free (*pkts);return -1 ; }
@@ -2429,7 +2431,7 @@ static __rte_always_inline int virtio_dev_tx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts) + struct rte_mbuf *pkts) { uint16_t buf_id, desc_count = 0;
@@ -2462,11 +2464,14 @@ virtio_dev_tx_packed(struct virtio_net *dev,uint32_t pkt_idx =0 ;uint32_t remained = count; +if (rte_pktmbuf_alloc_bulk (mbuf_pool, pkts, count)) +return 0 ; + do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]); if (remained >= PACKED_BATCH_SIZE) { - if (!virtio_dev_tx_batch_packed(dev, vq, mbuf_pool, + if (!virtio_dev_tx_batch_packed(dev, vq, &pkts[pkt_idx])) { pkt_idx += PACKED_BATCH_SIZE; remained -= PACKED_BATCH_SIZE;
@@ -2475,13 +2480,16 @@ virtio_dev_tx_packed(struct virtio_net *dev, }if (virtio_dev_tx_single_packed (dev, vq, mbuf_pool, - &pkts[pkt_idx])) + pkts[pkt_idx]))break ; pkt_idx++; remained--; }while (remained); +if (pkt_idx != count) +rte_pktmbuf_free_bulk (&pkts[pkt_idx], count - pkt_idx); +if (vq->shadow_used_idx) {do_data_copy_dequeue (vq);
Signature · 1 line
2.30.2
PATCH v5 1/1 vhost: allocate and free packets in bulk in Tx packed
Move allocation out further and perform all allocation in bulk. The same goes for freeing packets. In the process, also introduce virtio_dev_pktmbuf_prep and make virtio_dev_pktmbuf_alloc use that.
--- lib/librte_vhost/virtio_net.c | 80 +++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 36 deletions(-)
lib/librte_vhost/virtio_net.c +44 -36
@@ -2134,6 +2134,24 @@ virtio_dev_extbuf_alloc(struct rte_mbuf *pkt, uint32_t size)return 0 ; } +static __rte_always_inline int +virtio_dev_pktmbuf_prep (struct virtio_net *dev,struct rte_mbuf *pkt, +uint32_t data_len) +{ +if (rte_pktmbuf_tailroom (pkt) >= data_len) +return 0 ; + +/* attach an external buffer if supported */ +if (dev->extbuf && !virtio_dev_extbuf_alloc (pkt, data_len)) +return 0 ; + +/* check if chained buffers are allowed */ +if (!dev->linearbuf) +return 0 ; + +return -1 ; +} +/* * Allocate a host supported pktmbuf. */
@@ -2149,23 +2167,15 @@ virtio_dev_pktmbuf_alloc(struct virtio_net *dev, struct rte_mempool *mp,return NULL ; } -if (rte_pktmbuf_tailroom (pkt) >= data_len) -return pkt; +if (virtio_dev_pktmbuf_prep (dev, pkt, data_len)) { +/* Data doesn't fit into the buffer and the host supports +* only linear buffers +*/ +rte_pktmbuf_free (pkt); +return NULL ; + } -/* attach an external buffer if supported */ -if (dev->extbuf && !virtio_dev_extbuf_alloc (pkt, data_len)) -return pkt; - -/* check if chained buffers are allowed */ -if (!dev->linearbuf) -return pkt; - -/* Data doesn't fit into the buffer and the host supports -* only linear buffers -*/ -rte_pktmbuf_free (pkt); - -return NULL ; +return pkt; }static __rte_noinline uint16_t
@@ -2261,7 +2271,6 @@ virtio_dev_tx_split(struct virtio_net *dev, struct vhost_virtqueue *vq, static __rte_always_inline int vhost_reserve_avail_batch_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, - struct rte_mempool *mbuf_pool, struct rte_mbuf **pkts, uint16_t avail_idx, uintptr_t *desc_addrs,
@@ -2306,9 +2315,8 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev, }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) { - pkts[i] =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, lens[i]); -if (!pkts[i]) - goto free_buf; +if (virtio_dev_pktmbuf_prep (dev, pkts[i], lens[i])) + goto err; }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE )
@@ -2316,7 +2324,7 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev,vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) {if (unlikely (buf_lens[i] < (lens[i] - buf_offset))) - goto free_buf; + goto err; }vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE ) {
@@ -2327,17 +2335,13 @@ vhost_reserve_avail_batch_packed(struct virtio_net *dev,return 0 ; -free_buf: -for (i =0 ; i <PACKED_BATCH_SIZE ; i++) -rte_pktmbuf_free (pkts[i]); - +err:return -1 ; } static __rte_always_inline intvirtio_dev_tx_batch_packed (struct virtio_net *dev,struct vhost_virtqueue *vq, -struct rte_mempool *mbuf_pool,struct rte_mbuf **pkts) {uint16_t avail_idx = vq->last_avail_idx;
@@ -2347,8 +2351,8 @@ virtio_dev_tx_batch_packed(struct virtio_net *dev,uint16_t ids[PACKED_BATCH_SIZE ];uint16_t i; -if (vhost_reserve_avail_batch_packed (dev, vq, mbuf_pool, pkts, - avail_idx, desc_addrs, ids)) +if (vhost_reserve_avail_batch_packed (dev, vq, pkts, avail_idx, + desc_addrs, ids))return -1 ;vhost_for_each_try_unroll (i,0 ,PACKED_BATCH_SIZE )
@@ -2381,7 +2385,7 @@ static __rte_always_inline int vhost_dequeue_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts, + struct rte_mbuf *pkts, uint16_t *buf_id, uint16_t *desc_count) {
@@ -2398,8 +2402,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev, VHOST_ACCESS_RO) < 0))return -1 ; - *pkts =virtio_dev_pktmbuf_alloc (dev, mbuf_pool, buf_len); -if (unlikely (*pkts ==NULL )) { +if (unlikely (virtio_dev_pktmbuf_prep (dev, pkts, buf_len))) {if (!allocerr_warned) { VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d from %s on %s.\n",
@@ -2409,7 +2412,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev,return -1 ; } - err =copy_desc_to_mbuf (dev, vq, buf_vec, nr_vec, *pkts, + err =copy_desc_to_mbuf (dev, vq, buf_vec, nr_vec, pkts, mbuf_pool);if (unlikely (err)) {if (!allocerr_warned) {
@@ -2418,7 +2421,6 @@ vhost_dequeue_single_packed(struct virtio_net *dev, dev->ifname); allocerr_warned = true; } -rte_pktmbuf_free (*pkts);return -1 ; }
@@ -2429,7 +2431,7 @@ static __rte_always_inline int virtio_dev_tx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts) + struct rte_mbuf *pkts) { uint16_t buf_id, desc_count = 0;
@@ -2462,11 +2464,14 @@ virtio_dev_tx_packed(struct virtio_net *dev,uint32_t pkt_idx =0 ;uint32_t remained = count; +if (rte_pktmbuf_alloc_bulk (mbuf_pool, pkts, count)) +return 0 ; + do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]); if (remained >= PACKED_BATCH_SIZE) { - if (!virtio_dev_tx_batch_packed(dev, vq, mbuf_pool, + if (!virtio_dev_tx_batch_packed(dev, vq, &pkts[pkt_idx])) { pkt_idx += PACKED_BATCH_SIZE; remained -= PACKED_BATCH_SIZE;
@@ -2475,13 +2480,16 @@ virtio_dev_tx_packed(struct virtio_net *dev, }if (virtio_dev_tx_single_packed (dev, vq, mbuf_pool, - &pkts[pkt_idx])) + pkts[pkt_idx]))break ; pkt_idx++; remained--; }while (remained); +if (pkt_idx != count) +rte_pktmbuf_free_bulk (&pkts[pkt_idx], count - pkt_idx); +if (vq->shadow_used_idx) {do_data_copy_dequeue (vq);
Signature · 1 line
2.30.2
Re: [dpdk-dev] [PATCH v5] vhost: allocate and free packets in bulk in Tx packed
Quoted 6 lines · Balazs Nemeth
Move allocation out further and perform all allocation in bulk. The same goes for freeing packets. In the process, also introduce
virtio_dev_pktmbuf_prepand makevirtio_dev_pktmbuf_allocuse that.Signed-off-by Balazs Nemeth <[email protected]>
Signature · 1 line
David Marchand
Re: [dpdk-dev] [PATCH v5] vhost: allocate and free packets in bulk in Tx packed
In reply to Balazs NemethQuoted 9 lines · Balazs Nemeth
Move allocation out further and perform all allocation in bulk. The same goes for freeing packets. In the process, also introduce
virtio_dev_pktmbuf_prepand makevirtio_dev_pktmbuf_allocuse that.Signed-off-by Balazs Nemeth <[email protected]>--- lib/librte_vhost/virtio_net.c | 80 +++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 36 deletions(-)
Thanks,
Maxime
Re: [dpdk-dev] [PATCH v5] vhost: allocate and free packets in bulk in Tx packed
Quoted 10 lines
-----Original Message-----
From: dev <[email protected]> On Behalf Of Maxime Coquelin
Sent: Wednesday, April 21, 2021 3:49 PM
To: Balazs Nemeth <[email protected]>; [email protected]
Cc: [email protected]
Subject: Re: [dpdk-dev] [PATCH v5] vhost: allocate and free packets in bulk in Tx packed
Quoted 10 lines
Quoted 9 lines · Balazs Nemeth
Move allocation out further and perform all allocation in bulk. The same goes for freeing packets. In the process, also introduce
virtio_dev_pktmbuf_prepand makevirtio_dev_pktmbuf_allocuse that.Signed-off-by Balazs Nemeth <[email protected]>--- lib/librte_vhost/virtio_net.c | 80 +++++++++++++++++++---------------- 1 file changed, 44 insertions(+), 36 deletions(-)
Patch applied to next-virtio/main with conflict resolved.
Thanks
PATCH v2 2/4 vhost: perform all mbuf allocations in one loop
In reply to Balazs NemethMove allocation out further and perform all allocation in one loop. The same goes for freeing packets. This is to prepare for use of bulk versions of these functions.
--- lib/librte_vhost/virtio_net.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)
lib/librte_vhost/virtio_net.c +6 -12
@@ -2471,14 +2471,13 @@ virtio_dev_tx_packed(struct virtio_net *dev,uint32_t remained = count;uint16_t i; +for (i =0 ; i < count; ++i) + pkts[i] =rte_pktmbuf_alloc (mbuf_pool); + do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]); if (remained >= PACKED_BATCH_SIZE) { - vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) { - pkts[pkt_idx + i] = - rte_pktmbuf_alloc(mbuf_pool); - } if (!virtio_dev_tx_batch_packed(dev, vq, &pkts[pkt_idx])) {
@@ -2486,19 +2485,11 @@ virtio_dev_tx_packed(struct virtio_net *dev, remained -=PACKED_BATCH_SIZE ;continue ; - }else { -vhost_for_each_try_unroll (i,0 , -PACKED_BATCH_SIZE ) { -rte_pktmbuf_free (pkts[pkt_idx + i]); - } } } - pkts[pkt_idx] =rte_pktmbuf_alloc (mbuf_pool); -if (virtio_dev_tx_single_packed (dev, vq, mbuf_pool, &pkts[pkt_idx])) { -rte_pktmbuf_free (pkts[pkt_idx]);break ; } pkt_idx++;
@@ -2506,6 +2497,9 @@ virtio_dev_tx_packed(struct virtio_net *dev, }while (remained); +for (i = pkt_idx; i < count; ++i) +rte_pktmbuf_free (pkts[i]); +if (vq->shadow_used_idx) {do_data_copy_dequeue (vq);
Signature · 1 line
2.30.2
Re: [dpdk-dev] [PATCH v2 2/4] vhost: perform all mbuf allocations in one loop
Quoted 14 lines · Balazs Nemeth
Move allocation out further and perform all allocation in one loop. The same goes for freeing packets. This is to prepare for use of bulk versions of these functions.
Signed-off-by Balazs Nemeth <[email protected]>--- lib/librte_vhost/virtio_net.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)lib/librte_vhost/virtio_net.c +0
-0
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2475
Quoted 5 lines
lib/librte_vhost/virtio_net.c +2
-0@@ -2471,14 +2471,13 @@ virtio_dev_tx_packed(struct virtio_net *dev,uint32_t remained = count;uint16_t i; +for (i =0 ; i < count; ++i) + pkts[i] =rte_pktmbuf_alloc (mbuf_pool);
Same comment as for previous patch, we need to check whether allocation succeeded.
PATCH v2 3/4 vhost: allocate and free packets in bulk
In reply to Balazs NemethNow that all allocation and freeing has been moved together, use the faster bulk versions instead of handling packets one by one.
--- lib/librte_vhost/virtio_net.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)
lib/librte_vhost/virtio_net.c +6 -5
@@ -2469,10 +2469,10 @@ virtio_dev_tx_packed(struct virtio_net *dev, {uint32_t pkt_idx =0 ;uint32_t remained = count; -uint16_t i; -for (i =0 ; i < count; ++i) - pkts[i] =rte_pktmbuf_alloc (mbuf_pool); +if (rte_pktmbuf_alloc_bulk (mbuf_pool, pkts, count)) { +return 0 ; + } do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);
@@ -2497,8 +2497,9 @@ virtio_dev_tx_packed(struct virtio_net *dev, }while (remained); -for (i = pkt_idx; i < count; ++i) -rte_pktmbuf_free (pkts[i]); +if (pkt_idx != count) { +rte_pktmbuf_free_bulk (&pkts[pkt_idx], count - pkt_idx); + }if (vq->shadow_used_idx) {do_data_copy_dequeue (vq);
Signature · 1 line
2.30.2
Re: [dpdk-dev] [PATCH v2 3/4] vhost: allocate and free packets in bulk
Quoted 13 lines · Balazs Nemeth
Now that all allocation and freeing has been moved together, use the faster bulk versions instead of handling packets one by one.
Signed-off-by Balazs Nemeth <[email protected]>--- lib/librte_vhost/virtio_net.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)lib/librte_vhost/virtio_net.c +0
-0
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2475
Quoted 10 lines
lib/librte_vhost/virtio_net.c +3
-3@@ -2469,10 +2469,10 @@ virtio_dev_tx_packed(struct virtio_net *dev, {uint32_t pkt_idx =0 ;uint32_t remained = count; -uint16_t i; -for (i =0 ; i < count; ++i) - pkts[i] =rte_pktmbuf_alloc (mbuf_pool); +if (rte_pktmbuf_alloc_bulk (mbuf_pool, pkts, count)) { +return 0 ; + }
OK, get it now :)
Maybe just add a comment in previous patches that it is going to be handled when moving to bulk allocation.
Quoted 16 lines
lib/librte_vhost/virtio_net.c +3
-2do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);@@ -2497,8 +2497,9 @@ virtio_dev_tx_packed(struct virtio_net *dev, }while (remained); -for (i = pkt_idx; i < count; ++i) -rte_pktmbuf_free (pkts[i]); +if (pkt_idx != count) { +rte_pktmbuf_free_bulk (&pkts[pkt_idx], count - pkt_idx); + }if (vq->shadow_used_idx) {do_data_copy_dequeue (vq);
Re: [dpdk-dev] [PATCH v2 3/4] vhost: allocate and free packets in bulk
Quoted 19 lines · Maxime Coquelin
Quoted 18 lines · Balazs Nemeth
Now that all allocation and freeing has been moved together, use the faster bulk versions instead of handling packets one by one.
Signed-off-by Balazs Nemeth <[email protected]>--- lib/librte_vhost/virtio_net.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)lib/librte_vhost/virtio_net.c +0
-1@@ -2469,10 +2469,10 @@ virtio_dev_tx_packed(struct virtio_net *dev, {uint32_t pkt_idx =0 ;uint32_t remained = count; -uint16_t i;
Quoted 9 lines
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2475
Quoted 5 lines
lib/librte_vhost/virtio_net.c +3
-2-for (i =0 ; i < count; ++i) - pkts[i] =rte_pktmbuf_alloc (mbuf_pool); +if (rte_pktmbuf_alloc_bulk (mbuf_pool, pkts, count)) { +return 0 ; + }OK, get it now :)
Maybe just add a comment in previous patches that it is going to be handled when moving to bulk allocation.
Looking at the whole series, it is easy to read and understand as a single patch.
And it avoids this intermediate issue.
Signature · 1 line
David Marchand
Re: [dpdk-dev] [PATCH v2 3/4] vhost: allocate and free packets in bulk
Quoted 31 lines · David Marchand
Quoted 19 lines · Maxime Coquelin
Quoted 18 lines · Balazs Nemeth
Now that all allocation and freeing has been moved together, use the faster bulk versions instead of handling packets one by one.
Signed-off-by Balazs Nemeth <[email protected]>--- lib/librte_vhost/virtio_net.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)lib/librte_vhost/virtio_net.c +0
-1@@ -2469,10 +2469,10 @@ virtio_dev_tx_packed(struct virtio_net *dev, {uint32_t pkt_idx =0 ;uint32_t remained = count; -uint16_t i;Quoted 9 lines
lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2475
Quoted 5 lines
lib/librte_vhost/virtio_net.c +3
-2-for (i =0 ; i < count; ++i) - pkts[i] =rte_pktmbuf_alloc (mbuf_pool); +if (rte_pktmbuf_alloc_bulk (mbuf_pool, pkts, count)) { +return 0 ; + }OK, get it now :)
Maybe just add a comment in previous patches that it is going to be handled when moving to bulk allocation.
Quoted 3 lines
Looking at the whole series, it is easy to read and understand as a single patch.
And it avoids this intermediate issue.
I agree with David, better to squash the first three patches.
Thanks,
Maxime
Re: [dpdk-dev] [PATCH v2 3/4] vhost: allocate and free packets in bulk
Quoted 34 lines · Maxime Coquelin
Quoted 31 lines · David Marchand
Quoted 28 lines · Maxime Coquelin
Quoted 13 lines · Balazs Nemeth
Now that all allocation and freeing has been moved together, use the faster bulk versions instead of handling packets one by one.
Signed-off-by Balazs Nemeth <[email protected]>--- lib/librte_vhost/virtio_net.c | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-)lib/librte_vhost/virtio_net.c +0
-0lib/librte_vhost/virtio_net.c virtio_dev_tx_packed(struct virtio_net *dev, · line 2475
Quoted 10 lines
lib/librte_vhost/virtio_net.c +3
-3@@ -2469,10 +2469,10 @@ virtio_dev_tx_packed(struct virtio_net *dev, {uint32_t pkt_idx =0 ;uint32_t remained = count; -uint16_t i; -for (i =0 ; i < count; ++i) - pkts[i] =rte_pktmbuf_alloc (mbuf_pool); +if (rte_pktmbuf_alloc_bulk (mbuf_pool, pkts, count)) { +return 0 ; + }OK, get it now :)
Maybe just add a comment in previous patches that it is going to be handled when moving to bulk allocation.
Quoted 5 lines
Quoted 3 lines
Looking at the whole series, it is easy to read and understand as a single patch.
And it avoids this intermediate issue.I agree with David, better to squash the first three patches.
As David told me off-list, all the series can be squashed since the last patch is a consequence of doing bulk allocation.
Cheers,
Maxime
Quoted 3 lines
Thanks,
Maxime
PATCH v2 4/4 vhost: remove unnecessary level of indirection
In reply to Balazs NemethThere is no need to pass a pointer to an mbuf pointer.
--- lib/librte_vhost/virtio_net.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
lib/librte_vhost/virtio_net.c +5 -5
@@ -2390,7 +2390,7 @@ static __rte_always_inline int vhost_dequeue_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts, + struct rte_mbuf *pkts, uint16_t *buf_id, uint16_t *desc_count) {
@@ -2408,7 +2408,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev,return -1 ; -if (unlikely (virtio_dev_pktmbuf_prep (dev, *pkts, buf_len))) { +if (unlikely (virtio_dev_pktmbuf_prep (dev, pkts, buf_len))) {if (!allocerr_warned) { VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d from %s on %s.\n",
@@ -2418,7 +2418,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev,return -1 ; } - err =copy_desc_to_mbuf (dev, vq, buf_vec, nr_vec, *pkts, + err =copy_desc_to_mbuf (dev, vq, buf_vec, nr_vec, pkts, mbuf_pool);if (unlikely (err)) {if (!allocerr_warned) {
@@ -2437,7 +2437,7 @@ static __rte_always_inline int virtio_dev_tx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts) + struct rte_mbuf *pkts) { uint16_t buf_id, desc_count = 0;
@@ -2489,7 +2489,7 @@ virtio_dev_tx_packed(struct virtio_net *dev, }if (virtio_dev_tx_single_packed (dev, vq, mbuf_pool, - &pkts[pkt_idx])) { + pkts[pkt_idx])) {break ; } pkt_idx++;
Signature · 1 line
2.30.2
Re: [dpdk-dev] [PATCH v2 4/4] vhost: remove unnecessary level of indirection
Quoted 7 lines · Balazs Nemeth
There is no need to pass a pointer to an mbuf pointer.
Signed-off-by Balazs Nemeth <[email protected]>--- lib/librte_vhost/virtio_net.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
Thanks,
Maxime
PATCH 2/4 vhost: perform all mbuf allocations in one loop
In reply to Balazs NemethMove allocation out further and perform all allocation in one loop. The same goes for freeing packets. This is to prepare for use of bulk versions of these functions.
--- lib/librte_vhost/virtio_net.c | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-)
lib/librte_vhost/virtio_net.c +6 -12
@@ -2471,14 +2471,13 @@ virtio_dev_tx_packed(struct virtio_net *dev,uint32_t remained = count;uint16_t i; +for (i =0 ; i < count; ++i) + pkts[i] =rte_pktmbuf_alloc (mbuf_pool); + do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]); if (remained >= PACKED_BATCH_SIZE) { - vhost_for_each_try_unroll(i, 0, PACKED_BATCH_SIZE) { - pkts[pkt_idx + i] = - rte_pktmbuf_alloc(mbuf_pool); - } if (!virtio_dev_tx_batch_packed(dev, vq, &pkts[pkt_idx])) {
@@ -2486,19 +2485,11 @@ virtio_dev_tx_packed(struct virtio_net *dev, remained -=PACKED_BATCH_SIZE ;continue ; - }else { -vhost_for_each_try_unroll (i,0 , -PACKED_BATCH_SIZE ) { -rte_pktmbuf_free (pkts[pkt_idx + i]); - } } } - pkts[pkt_idx] =rte_pktmbuf_alloc (mbuf_pool); -if (virtio_dev_tx_single_packed (dev, vq, mbuf_pool, &pkts[pkt_idx])) { -rte_pktmbuf_free (pkts[pkt_idx]);break ; } pkt_idx++;
@@ -2506,6 +2497,9 @@ virtio_dev_tx_packed(struct virtio_net *dev, }while (remained); +for (i = pkt_idx; i < count; ++i) +rte_pktmbuf_free (pkts[i]); +if (vq->shadow_used_idx) {do_data_copy_dequeue (vq);
Signature · 1 line
2.30.2
PATCH 3/4 vhost: allocate and free packets in bulk
In reply to Balazs NemethNow that all allocation and freeing has been moved together, use the faster bulk versions instead of handling packets one by one.
--- lib/librte_vhost/virtio_net.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-)
lib/librte_vhost/virtio_net.c +6 -4
@@ -2471,8 +2471,9 @@ virtio_dev_tx_packed(struct virtio_net *dev,uint32_t remained = count;uint16_t i; -for (i =0 ; i < count; ++i) - pkts[i] =rte_pktmbuf_alloc (mbuf_pool); +if (rte_pktmbuf_alloc_bulk (mbuf_pool, pkts, count)) { +return 0 ; + } do { rte_prefetch0(&vq->desc_packed[vq->last_avail_idx]);
@@ -2497,8 +2498,9 @@ virtio_dev_tx_packed(struct virtio_net *dev, }while (remained); -for (i = pkt_idx; i < count; ++i) -rte_pktmbuf_free (pkts[i]); +if (pkt_idx != count) { +rte_pktmbuf_free_bulk (&pkts[pkt_idx], count - pkt_idx); + }if (vq->shadow_used_idx) {do_data_copy_dequeue (vq);
Signature · 1 line
2.30.2
PATCH 4/4 vhost: remove unnecessary level of indirection
In reply to Balazs NemethThere is no need to pass a pointer to an mbuf pointer.
--- lib/librte_vhost/virtio_net.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-)
lib/librte_vhost/virtio_net.c +5 -5
@@ -2390,7 +2390,7 @@ static __rte_always_inline int vhost_dequeue_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts, + struct rte_mbuf *pkts, uint16_t *buf_id, uint16_t *desc_count) {
@@ -2408,7 +2408,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev,return -1 ; -if (unlikely (virtio_dev_pktmbuf_prep (dev, *pkts, buf_len))) { +if (unlikely (virtio_dev_pktmbuf_prep (dev, pkts, buf_len))) {if (!allocerr_warned) { VHOST_LOG_DATA(ERR, "Failed mbuf alloc of size %d from %s on %s.\n",
@@ -2418,7 +2418,7 @@ vhost_dequeue_single_packed(struct virtio_net *dev,return -1 ; } - err =copy_desc_to_mbuf (dev, vq, buf_vec, nr_vec, *pkts, + err =copy_desc_to_mbuf (dev, vq, buf_vec, nr_vec, pkts, mbuf_pool);if (unlikely (err)) {if (!allocerr_warned) {
@@ -2437,7 +2437,7 @@ static __rte_always_inline int virtio_dev_tx_single_packed(struct virtio_net *dev, struct vhost_virtqueue *vq, struct rte_mempool *mbuf_pool, - struct rte_mbuf **pkts) + struct rte_mbuf *pkts) { uint16_t buf_id, desc_count = 0;
@@ -2490,7 +2490,7 @@ virtio_dev_tx_packed(struct virtio_net *dev, }if (virtio_dev_tx_single_packed (dev, vq, mbuf_pool, - &pkts[pkt_idx])) { + pkts[pkt_idx])) {break ; } pkt_idx++;
Signature · 1 line
2.30.2