Re: [PATCH 3/3] tap: add die() on vhost error

[Eugenio Perez Martin <eperezma@redhat.com>]

On Thu, Apr 3, 2025 at 3:51 AM David Gibson <david@gibson.dropbear.id.au> wrote:
>
> On Tue, Apr 01, 2025 at 01:38:09PM +0200, Eugenio Pérez wrote:
> > In case the kernel needs to signal an error.
> >
> > Signed-off-by: Eugenio Pérez <eperezma@redhat.com>
> > ---
> >  epoll_type.h |  2 ++
> >  passt.c      |  4 ++++
> >  passt.h      |  1 +
> >  tap.c        | 16 ++++++++++++++++
> >  4 files changed, 23 insertions(+)
> >
> > diff --git a/epoll_type.h b/epoll_type.h
> > index 6284c79..6b320db 100644
> > --- a/epoll_type.h
> > +++ b/epoll_type.h
> > @@ -46,6 +46,8 @@ enum epoll_type {
> >       EPOLL_TYPE_REPAIR,
> >       /* vhost-kernel call socket */
> >       EPOLL_TYPE_VHOST_CALL,
> > +     /* vhost-kernel error socket */
> > +     EPOLL_TYPE_VHOST_ERROR,
> >
> >       EPOLL_NUM_TYPES,
> >  };
> > diff --git a/passt.c b/passt.c
> > index 19c5d5f..2779e0b 100644
> > --- a/passt.c
> > +++ b/passt.c
> > @@ -80,6 +80,7 @@ char *epoll_type_str[] = {
> >       [EPOLL_TYPE_REPAIR_LISTEN]      = "TCP_REPAIR helper listening socket",
> >       [EPOLL_TYPE_REPAIR]             = "TCP_REPAIR helper socket",
> >       [EPOLL_TYPE_VHOST_CALL]         = "vhost-kernel call socket",
> > +     [EPOLL_TYPE_VHOST_ERROR]        = "vhost-kernel error socket",
> >  };
> >  static_assert(ARRAY_SIZE(epoll_type_str) == EPOLL_NUM_TYPES,
> >             "epoll_type_str[] doesn't match enum epoll_type");
> > @@ -361,6 +362,9 @@ loop:
> >               case EPOLL_TYPE_VHOST_CALL:
> >                       vhost_call_cb(&c, ref, &now);
> >                       break;
> > +             case EPOLL_TYPE_VHOST_ERROR:
> > +                     die("Error on vhost-kernel socket");
> > +                     break;
> >               default:
> >                       /* Can't happen */
> >                       ASSERT(0);
> > diff --git a/passt.h b/passt.h
> > index eb5aa03..9e42f3b 100644
> > --- a/passt.h
> > +++ b/passt.h
> > @@ -307,6 +307,7 @@ struct ctx {
> >
> >               int kick_fd;
> >               int call_fd;
> > +             int err_fd;
> >       } vq[2];
> >
> >       int no_dns;
> > diff --git a/tap.c b/tap.c
> > index fbe83aa..b02d3da 100644
> > --- a/tap.c
> > +++ b/tap.c
> > @@ -1552,6 +1552,22 @@ static int tap_ns_tun(void *arg)
> >               if (rc < 0)
> >                       die_perror("Failed to add call eventfd to epoll");
> >               c->vq[i].call_fd = file.fd;
> > +
> > +             file.fd = eventfd(0, EFD_NONBLOCK | EFD_CLOEXEC);
> > +             if (file.fd < 0)
> > +                     die_perror("Failed to create error eventfd");
> > +
> > +             rc = ioctl(vhost_fd, VHOST_SET_VRING_ERR, &file);
> > +             if (rc < 0)
> > +                     die_perror(
> > +                             "VHOST_SET_VRING_ERR ioctl on /dev/vhost-net failed");
> > +
> > +             ref.type = EPOLL_TYPE_VHOST_ERROR, ref.fd = file.fd;
> > +             ev.data.u64 = ref.u64;
> > +             rc = epoll_ctl(c->epollfd, EPOLL_CTL_ADD, ref.fd, &ev);
> > +             if (rc < 0)
> > +                     die_perror("Failed to add error eventfd to epoll");
> > +             c->vq[i].err_fd = file.fd;
>
> If it's possible to re-initialize vhost, eventually it would be nice
> to do so eventually, but for now die()ing is perfectly reasonable.
>

That's a good point, adding TODO!