Re: [libvirt] [PATCH v2 5/7] virNetDevMacVLanTapSetup: Allow enabling of IFF_MULTI_QUEUE

* Re: [libvirt] [PATCH v2 5/7] virNetDevMacVLanTapSetup: Allow enabling of IFF_MULTI_QUEUE
       [not found] ` <8aca20adcba7e96950d16427fbd0a88d13fd08f0.1449732911.git.mprivozn@redhat.com>
@ 2015-12-14 10:23   ` Ian Campbell
       [not found]   ` <1450088639.16856.6.camel@citrix.com>
  1 sibling, 0 replies; 3+ messages in thread
From: Ian Campbell @ 2015-12-14 10:23 UTC (permalink / raw)
  To: Michal Privoznik, libvir-list; +Cc: xen-devel

Hello,

On Thu, 2015-12-10 at 08:38 +0100, Michal Privoznik wrote:
> Like we are doing for TUN/TAP devices, we should do the same for
> macvtaps. Although, it's not as critical as in that case, we
> should do it for the consistency.
> 
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>

This has triggered a build failure on amd64+i386+armhf within the Xen
automated test framework (which uses Debian Wheezy as the build
environment), I doubt it is in any way Xen specific though:

util/virnetdevmacvlan.c: In function 'virNetDevMacVLanTapSetup':
util/virnetdevmacvlan.c:338:26: error: 'IFF_MULTI_QUEUE' undeclared (first use in this function)
util/virnetdevmacvlan.c:338:26: note: each undeclared identifier is reported only once for each function it appears in

I'm not sure where that definition is supposed to come from, so I can't
tell if it is a missing #include in this code or an out of date header on
the Debian system.

Full logs are at
http://logs.test-lab.xenproject.org/osstest/logs/65756/
http://logs.test-lab.xenproject.org/osstest/logs/65756/build-amd64-libvirt/5.ts-libvirt-build.log
http://lists.xen.org/archives/html/xen-devel/2015-12/msg01470.html

But TBH there isn't much more of use than the above.

Cheers,
Ian.

> ---
>  src/util/virnetdevmacvlan.c | 40 ++++++++++++++++++++++-----------------
> -
>  1 file changed, 22 insertions(+), 18 deletions(-)
> 
> diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
> index c4d0d53..76fd542 100644
> --- a/src/util/virnetdevmacvlan.c
> +++ b/src/util/virnetdevmacvlan.c
> @@ -289,24 +289,26 @@ virNetDevMacVLanTapOpen(const char *ifname,
>   * @tapfd: array of file descriptors of the macvtap tap
>   * @tapfdSize: number of file descriptors in @tapfd
>   * @vnet_hdr: whether to enable or disable IFF_VNET_HDR
> + * @multiqueue: whether to enable or disable IFF_MULTI_QUEUE
> + *
> + * Turn the IFF_VNET_HDR flag, if requested and available, make sure
> it's
> + * off in the other cases. Similarly, IFF_MULTI_QUEUE is enabled if
> + * requested. However, if requested and failed to set, it is considered
> a
> + * fatal error (as opposed to @vnet_hdr).
>   *
> - * Turn the IFF_VNET_HDR flag, if requested and available, make sure
> - * it's off in the other cases.
>   * A fatal error is defined as the VNET_HDR flag being set but it cannot
>   * be turned off for some reason. This is reported with -1. Other fatal
>   * error is not being able to read the interface flags. In that case the
>   * macvtap device should not be used.
>   *
> - * Returns 0 on success, -1 in case of fatal error, error code
> otherwise.
> + * Returns 0 on success, -1 in case of fatal error.
>   */
>  static int
> -virNetDevMacVLanTapSetup(int *tapfd, size_t tapfdSize, bool vnet_hdr)
> +virNetDevMacVLanTapSetup(int *tapfd, size_t tapfdSize, bool vnet_hdr,
> bool multiqueue)
>  {
>      unsigned int features;
>      struct ifreq ifreq;
>      short new_flags = 0;
> -    int rc_on_fail = 0;
> -    const char *errmsg = NULL;
>      size_t i;
>  
>      for (i = 0; i < tapfdSize; i++) {
> @@ -320,27 +322,29 @@ virNetDevMacVLanTapSetup(int *tapfd, size_t
> tapfdSize, bool vnet_hdr)
>  
>          new_flags = ifreq.ifr_flags;
>  
> -        if ((ifreq.ifr_flags & IFF_VNET_HDR) && !vnet_hdr) {
> -            new_flags = ifreq.ifr_flags & ~IFF_VNET_HDR;
> -            rc_on_fail = -1;
> -            errmsg = _("cannot clean IFF_VNET_HDR flag on macvtap tap");
> -        } else if ((ifreq.ifr_flags & IFF_VNET_HDR) == 0 && vnet_hdr) {
> +        if (vnet_hdr) {
>              if (ioctl(tapfd[i], TUNGETFEATURES, &features) < 0) {
>                  virReportSystemError(errno, "%s",
>                                       _("cannot get feature flags on
> macvtap tap"));
>                  return -1;
>              }
> -            if ((features & IFF_VNET_HDR)) {
> -                new_flags = ifreq.ifr_flags | IFF_VNET_HDR;
> -                errmsg = _("cannot set IFF_VNET_HDR flag on macvtap
> tap");
> -            }
> +            if (features & IFF_VNET_HDR)
> +                new_flags |= IFF_VNET_HDR;
> +        } else {
> +            new_flags &= ~IFF_VNET_HDR;
>          }
>  
> +        if (multiqueue)
> +            new_flags |= IFF_MULTI_QUEUE;
> +        else
> +            new_flags &= ~IFF_MULTI_QUEUE;
> +
>          if (new_flags != ifreq.ifr_flags) {
>              ifreq.ifr_flags = new_flags;
>              if (ioctl(tapfd[i], TUNSETIFF, &ifreq) < 0) {
> -                virReportSystemError(errno, "%s", errmsg);
> -                return rc_on_fail;
> +                virReportSystemError(errno, "%s",
> +                                     _("unable to set vnet or multiqueue
> flags on macvtap"));
> +                return -1;
>              }
>          }
>      }
> @@ -852,7 +856,7 @@ int virNetDevMacVLanCreateWithVPortProfile(const char
> *tgifname,
>          if (virNetDevMacVLanTapOpen(cr_ifname, &rc, 1, 10) < 0)
>              goto disassociate_exit;
>  
> -        if (virNetDevMacVLanTapSetup(&rc, 1, vnet_hdr) < 0) {
> +        if (virNetDevMacVLanTapSetup(&rc, 1, vnet_hdr, false) < 0) {
>              VIR_FORCE_CLOSE(rc); /* sets rc to -1 */
>              goto disassociate_exit;
>          }
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

^ permalink raw reply	[flat|nested] 3+ messages in thread