Hi Sivaprasad,

On 4/28/20 11:52 AM, Sivaprasad Tummala wrote:
> vhost buffer allocation is successful for packets that fit
> into a linear buffer. If it fails, vhost library is expected
> to drop the current buffer descriptor and skip to the next.
> 
> The patch fixes the error scenario by skipping to next descriptor.
> Note: Drop counters are not currently supported.

Fixes tag is missing here, and sta...@dpdk.org should be cc'ed if
necessary.

> Signed-off-by: Sivaprasad Tummala <sivaprasad.tumm...@intel.com>
> ---
>  lib/librte_vhost/virtio_net.c | 16 ++++++++++++++--
>  1 file changed, 14 insertions(+), 2 deletions(-)
> 
> diff --git a/lib/librte_vhost/virtio_net.c b/lib/librte_vhost/virtio_net.c
> index 37c47c7dc..b0d3a85c2 100644
> --- a/lib/librte_vhost/virtio_net.c
> +++ b/lib/librte_vhost/virtio_net.c
> @@ -1688,6 +1688,7 @@ virtio_dev_tx_split(struct virtio_net *dev, struct 
> vhost_virtqueue *vq,
>  {

You only fix split ring path, but not packed ring.

>       uint16_t i;
>       uint16_t free_entries;
> +     uint16_t dropped = 0;
>  
>       if (unlikely(dev->dequeue_zero_copy)) {
>               struct zcopy_mbuf *zmbuf, *next;
> @@ -1751,8 +1752,19 @@ virtio_dev_tx_split(struct virtio_net *dev, struct 
> vhost_virtqueue *vq,
>                       update_shadow_used_ring_split(vq, head_idx, 0);
>  
>               pkts[i] = virtio_dev_pktmbuf_alloc(dev, mbuf_pool, buf_len);
> -             if (unlikely(pkts[i] == NULL))
> +             if (unlikely(pkts[i] == NULL)) {
> +                     /*
> +                      * mbuf allocation fails for jumbo packets with
> +                      * linear buffer flag set. Drop this packet and
> +                      * proceed with the next available descriptor to
> +                      * avoid HOL blocking
> +                      */
> +                     VHOST_LOG_DATA(WARNING,
> +                             "Failed to allocate memory for mbuf. Packet 
> dropped!\n");

I think we need a better logging, otherwise it is going to flood the log
file quite rapidly if issue happens. Either some rate-limited logging or
warn-once would be better.

The warning message could be also improved, because when using linear
buffers, one would expect that the size of the mbufs could handle a
jumbo frame. So it should differentiate two

> +                     dropped += 1;
> +                     i++;
>                       break;
> +             }
>  
>               err = copy_desc_to_mbuf(dev, vq, buf_vec, nr_vec, pkts[i],
>                               mbuf_pool);
> @@ -1796,7 +1808,7 @@ virtio_dev_tx_split(struct virtio_net *dev, struct 
> vhost_virtqueue *vq,
>               }
>       }
>  
> -     return i;
> +     return (i - dropped);
>  }
>  
>  static __rte_always_inline int
> 

Reply via email to