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

[Ian Campbell <ian.campbell@xxxxxxxxxx>]

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@xxxxxxxxxx>

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@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel