Re: [PATCH 2/2] tap: initialize TAPState->enabled according to the actual state of queue

[Jason Wang]

在 2022/6/14 19:21, Andrey Ryabinin 写道:
> Currently TAPState->enabled initialized as true. If fd was passed to qemu
> in a disabled state it will cause an assert at the attempt to detach queue
> in virtio_net_set_queues():
>
> virtio_net_set_queues() :
>              r = peer_detach() -> tap_disable():
>                                      if (s->enabled == 0) {
>                                         return 0;
>                                     } else {
>                                        //Will return an error.
>                                         ret = tap_fd_disable(s->fd);
>                                         ...
>                                        return ret;
>              assert(!r);
>
> Initialize ->enabled according to the actual state of fd to fix this.
>
> Signed-off-by: Andrey Ryabinin <arbn@yandex-team.com>
> ---
>   net/tap-bsd.c     |  5 +++++
>   net/tap-linux.c   | 12 ++++++++++++
>   net/tap-solaris.c |  5 +++++
>   net/tap.c         |  2 +-
>   net/tap_int.h     |  1 +
>   5 files changed, 24 insertions(+), 1 deletion(-)
>
> diff --git a/net/tap-bsd.c b/net/tap-bsd.c
> index 005ce05c6e0..8c21f058c8c 100644
> --- a/net/tap-bsd.c
> +++ b/net/tap-bsd.c
> @@ -217,6 +217,11 @@ int tap_probe_vnet_hdr_len(int fd, int len)
>       return 0;
>   }
>   
> +bool tap_probe_enabled(int fd)
> +{
> +    return true;
> +}
> +
>   void tap_fd_set_vnet_hdr_len(int fd, int len)
>   {
>   }
> diff --git a/net/tap-linux.c b/net/tap-linux.c
> index 304ff45071d..6078ba03af6 100644
> --- a/net/tap-linux.c
> +++ b/net/tap-linux.c
> @@ -193,6 +193,18 @@ int tap_probe_vnet_hdr_len(int fd, int len)
>       return 1;
>   }
>   
> +bool tap_probe_enabled(int fd)
> +{
> +    struct ifreq ifr;
> +
> +    if (ioctl(fd, TUNGETIFF, &ifr) != 0) {
> +        error_report("TUNGETIFF ioctl() failed: %s",
> +                     strerror(errno));
> +        return false;
> +    }
> +    return !(ifr.ifr_flags & IFF_DETACH_QUEUE);


Is it better to check IFF_MULT_QUEUE before this to unbreak the old host?

Thanks


> +}
> +
>   void tap_fd_set_vnet_hdr_len(int fd, int len)
>   {
>       if (ioctl(fd, TUNSETVNETHDRSZ, &len) == -1) {
> diff --git a/net/tap-solaris.c b/net/tap-solaris.c
> index a44f8805c23..ccaa3334882 100644
> --- a/net/tap-solaris.c
> +++ b/net/tap-solaris.c
> @@ -221,6 +221,11 @@ int tap_probe_vnet_hdr_len(int fd, int len)
>       return 0;
>   }
>   
> +bool tap_probe_enabled(int fd)
> +{
> +    return true;
> +}
> +
>   void tap_fd_set_vnet_hdr_len(int fd, int len)
>   {
>   }
> diff --git a/net/tap.c b/net/tap.c
> index b3ddfd4a74b..799f8ec7c76 100644
> --- a/net/tap.c
> +++ b/net/tap.c
> @@ -399,7 +399,7 @@ static TAPState *net_tap_fd_init(NetClientState *peer,
>       s->host_vnet_hdr_len = vnet_hdr ? sizeof(struct virtio_net_hdr) : 0;
>       s->using_vnet_hdr = false;
>       s->has_ufo = tap_probe_has_ufo(s->fd);
> -    s->enabled = true;
> +    s->enabled = tap_probe_enabled(s->fd);
>       tap_set_offload(&s->nc, 0, 0, 0, 0, 0);
>       /*
>        * Make sure host header length is set correctly in tap:
> diff --git a/net/tap_int.h b/net/tap_int.h
> index 547f8a5a28f..b8fc3dfbfa7 100644
> --- a/net/tap_int.h
> +++ b/net/tap_int.h
> @@ -37,6 +37,7 @@ void tap_set_sndbuf(int fd, const NetdevTapOptions *tap, 
> Error **errp);
>   int tap_probe_vnet_hdr(int fd, Error **errp);
>   int tap_probe_vnet_hdr_len(int fd, int len);
>   int tap_probe_has_ufo(int fd);
> +bool tap_probe_enabled(int fd);
>   void tap_fd_set_offload(int fd, int csum, int tso4, int tso6, int ecn, int 
> ufo);
>   void tap_fd_set_vnet_hdr_len(int fd, int len);
>   int tap_fd_set_vnet_le(int fd, int vnet_is_le);