[PATCH v2 2/2] vhost: add new mbuf allocation failure statistic

[Maxime Coquelin]

On 2/1/24 09:10, David Marchand wrote:
> On Wed, Jan 31, 2024 at 8:53 PM Maxime Coquelin
> <maxime.coquelin at redhat.com> wrote:
>>
>> This patch introduces a new, per virtqueue, mbuf allocation
>> failure statistic. It can be useful to troubleshoot packets
>> drops due to insufficient mempool size or memory leaks.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin at redhat.com>
> 
> Having a stat for such situation will be useful.
> 
> I just have one comment, though it is not really related to this change itself.
> 
> [snip]
> 
>> diff --git a/lib/vhost/virtio_net.c b/lib/vhost/virtio_net.c
>> index 9951842b9f..1359c5fb1f 100644
>> --- a/lib/vhost/virtio_net.c
>> +++ b/lib/vhost/virtio_net.c
>> @@ -2996,6 +2996,7 @@ desc_to_mbuf(struct virtio_net *dev, struct vhost_virtqueue *vq,
>>                  if (mbuf_avail == 0) {
>>                          cur = rte_pktmbuf_alloc(mbuf_pool);
>>                          if (unlikely(cur == NULL)) {
>> +                               vq->stats.mbuf_alloc_failed++;
>>                                  VHOST_DATA_LOG(dev->ifname, ERR,
>>                                          "failed to allocate memory for mbuf.");
> 
> This error log here is scary as it means the datapath can be slowed
> down for each multisegment mbuf in the event of a mbuf (maybe
> temporary) shortage.
> Besides no other mbuf allocation in the vhost library datapath
> generates such log.
> 
> I would remove it, probably in a separate patch.
> WDYT?

Agree, we should not have such log in the datapath.
And now that we have the stat, it is even less useful.

Regards,
Maxime

> 
>>                                  goto error;
> 
>