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

[David Marchand <david.marchand@redhat.com>]

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).


>                         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*


> +                               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.


-- 
David Marchand