Re: [PATCH 4/7] vhost: improve Vhost layer logs

[Maxime Coquelin <maxime.coquelin@redhat.com>]

On 1/4/22 15:48, David Marchand wrote:
> On Thu, Dec 23, 2021 at 9:37 AM Maxime Coquelin
> <maxime.coquelin@redhat.com> wrote:
>>
>> This patch prepends Vhost logs with the Vhost-user socket
>> patch when available to ease filtering logs for a given port.
>>
>> Signed-off-by: Maxime Coquelin <maxime.coquelin@redhat.com>
>> ---
>>   lib/vhost/vhost.c | 104 +++++++++++++++++++++++-----------------------
>>   1 file changed, 51 insertions(+), 53 deletions(-)
>>
>> diff --git a/lib/vhost/vhost.c b/lib/vhost/vhost.c
>> index e52d7f7bb6..dac0915ac0 100644
>> --- a/lib/vhost/vhost.c
>> +++ b/lib/vhost/vhost.c
>> @@ -58,9 +58,8 @@ __vhost_iova_to_vva(struct virtio_net *dev, struct vhost_virtqueue *vq,
>>
>>                  vhost_user_iotlb_pending_insert(dev, vq, iova, perm);
>>                  if (vhost_user_iotlb_miss(dev, iova, perm)) {
>> -                       VHOST_LOG_CONFIG(ERR,
>> -                               "IOTLB miss req failed for IOVA 0x%" PRIx64 "\n",
>> -                               iova);
>> +                       VHOST_LOG_DATA(ERR, "(%s) IOTLB miss req failed for IOVA 0x%" PRIx64 "\n",
>> +                               dev->ifname, iova);
> 
> This change from CONFIG to DATA looks unrelated, and a separate fix
> (which could go with another fix, see below).
> 

I'm adding this one and the below one you spotted in a dedicated patch.

>>                          vhost_user_iotlb_pending_remove(vq, iova, 1, perm);
>>                  }
>>
> 
> [snip]
> 
>>
>> @@ -422,9 +421,9 @@ translate_log_addr(struct virtio_net *dev, struct vhost_virtqueue *vq,
>>                  gpa = hva_to_gpa(dev, hva, exp_size);
>>                  if (!gpa) {
>>                          VHOST_LOG_CONFIG(ERR,
> 
> translate_log_addr is called from vring_translate which is called from datapath.
> So this log should be _DATA, in a separate fix.
> 
> 
>> -                               "VQ: Failed to find GPA for log_addr: 0x%"
>> +                               "(%s) failed to find GPA for log_addr: 0x%"
>>                                  PRIx64 " hva: 0x%" PRIx64 "\n",
>> -                               log_addr, hva);
>> +                               dev->ifname, log_addr, hva);
>>                          return 0;
>>                  }
>>                  return gpa;
>> @@ -551,16 +550,15 @@ init_vring_queue(struct virtio_net *dev, uint32_t vring_idx)
>>          int numa_node = SOCKET_ID_ANY;
>>
>>          if (vring_idx >= VHOST_MAX_VRING) {
>> -               VHOST_LOG_CONFIG(ERR,
>> -                               "Failed not init vring, out of bound (%d)\n",
>> -                               vring_idx);
>> +               VHOST_LOG_CONFIG(ERR, "(%s) failed not init vring, out of bound (%d)\n",
> 
> failed to*

Fixed in this patch.

> 
>> +                               dev->ifname, vring_idx);
>>                  return;
>>          }
>>
>>          vq = dev->virtqueue[vring_idx];
>>          if (!vq) {
>> -               VHOST_LOG_CONFIG(ERR, "Virtqueue not allocated (%d)\n",
>> -                               vring_idx);
>> +               VHOST_LOG_CONFIG(ERR, "(%s) virtqueue not allocated (%d)\n",
>> +                               dev->ifname, vring_idx);
>>                  return;
>>          }
>>
> 
> The rest lgtm.
> 
> 

Thanks,
Maxime