virNetDevBridgeSetupVlans() calls virNetlinkBridgeVlanFilterSet(), which requires libnl. Use the function only when libnl is available to avoid breaking builds. Signed-off-by: Akihiko Odaki <akihiko.odaki(a)daynix.com&gt;

--- src/util/virnetdevbridge.c | 122 ++++++++++++++++++++++----------------------- 1 file changed, 61 insertions(+), 61 deletions(-) diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index c79d0c79b7b16a0a070b5013ed7df9d0a63b1c38..70e82a0029634ddf44c2b933577bf783c89f035c 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -313,66 +313,6 @@ virNetDevBridgePortSetIsolated(const char *brname, return virNetDevBridgePortSet(brname, ifname, "isolated", enable ? 1 : 0); } -static int -virNetDevBridgeSetupVlans(const char *ifname, const virNetDevVlan *virtVlan) -{ - int error = 0; - unsigned short flags; - - if (!virtVlan || !virtVlan->nTags) - return 0; - - // The interface will have been automatically added to vlan 1, so remove it - if (virNetlinkBridgeVlanFilterSet(ifname, RTM_DELLINK, 0, 1, &error) < 0) { - if (error != 0) { - virReportSystemError(-error, - _("error removing vlan filter from interface %1$s"), - ifname); - } - return -1; - } - - // If trunk mode, add the native VLAN then add the others, if any - if (virtVlan->trunk) { - size_t i; - - if (virtVlan->nativeTag) { - flags = BRIDGE_VLAN_INFO_PVID; - if (virtVlan->nativeMode == VIR_NATIVE_VLAN_MODE_UNTAGGED || - virtVlan->nativeMode == VIR_NATIVE_VLAN_MODE_DEFAULT) { - flags |= BRIDGE_VLAN_INFO_UNTAGGED; - } - - if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, flags, - virtVlan->nativeTag, &error) < 0) { - goto error; - } - } - - for (i = 0; i < virtVlan->nTags; i++) { - if (virtVlan->tag[i] != virtVlan->nativeTag) - if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, 0, - virtVlan->tag[i], &error) < 0) { - goto error; - } - } - } else { - // In native mode, add the single VLAN as pvid untagged - flags = BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED; - if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, flags, - virtVlan->tag[0], &error) < 0) { - goto error; - } - } - - return 0; - - error: - if (error != 0) - virReportSystemError(-error, _("error adding vlan filter to interface %1$s"), ifname); - return -1; -} - #else int @@ -651,7 +591,67 @@ int virNetDevBridgeDelete(const char *brname G_GNUC_UNUSED) * * Returns 0 in case of success or an errno code in case of failure. */ -#if defined(WITH_STRUCT_IFREQ) && defined(SIOCBRADDIF) +#if defined(WITH_STRUCT_IFREQ) && defined(SIOCBRADDIF) && defined(WITH_LIBNL) +static int +virNetDevBridgeSetupVlans(const char *ifname, const virNetDevVlan *virtVlan) +{ + int error = 0; + unsigned short flags; + + if (!virtVlan || !virtVlan->nTags) + return 0; + + // The interface will have been automatically added to vlan 1, so remove it + if (virNetlinkBridgeVlanFilterSet(ifname, RTM_DELLINK, 0, 1, &error) < 0) { + if (error != 0) { + virReportSystemError(-error, + _("error removing vlan filter from interface %1$s"), + ifname); + } + return -1; + } + + // If trunk mode, add the native VLAN then add the others, if any + if (virtVlan->trunk) { + size_t i; + + if (virtVlan->nativeTag) { + flags = BRIDGE_VLAN_INFO_PVID; + if (virtVlan->nativeMode == VIR_NATIVE_VLAN_MODE_UNTAGGED || + virtVlan->nativeMode == VIR_NATIVE_VLAN_MODE_DEFAULT) { + flags |= BRIDGE_VLAN_INFO_UNTAGGED; + } + + if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, flags, + virtVlan->nativeTag, &error) < 0) { + goto error; + } + } + + for (i = 0; i < virtVlan->nTags; i++) { + if (virtVlan->tag[i] != virtVlan->nativeTag) + if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, 0, + virtVlan->tag[i], &error) < 0) { + goto error; + } + } + } else { + // In native mode, add the single VLAN as pvid untagged + flags = BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED; + if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, flags, + virtVlan->tag[0], &error) < 0) { + goto error; + } + } + + return 0; + + error: + if (error != 0) + virReportSystemError(-error, _("error adding vlan filter to interface %1$s"), ifname); + return -1; +} + int virNetDevBridgeAddPort(const char *brname, const char *ifname, const virNetDevVlan *virtVlan) --- base-commit: e5299ddf86121d3c792ca271ffcb54900eb19dc3 change-id: 20250304-nl-605e05b8c5f2 Best regards,

On 2025/03/07 22:11, Laine Stump wrote: > On 3/7/25 6:48 AM, Akihiko Odaki wrote: >> virNetDevBridgeSetupVlans() calls virNetlinkBridgeVlanFilterSet(), >> which requires libnl. Use the function only when libnl is available >> to avoid breaking builds. >> >> Signed-off-by: Akihiko Odaki <akihiko.odaki(a)daynix.com&gt; > > I'm curious what is the Linux platform where we don't have libnl > available? I didn't have libnl3-devel installed. Another option is to require libnl3-devel in Mesa; it will give nicer error messages to anyone trying to build libvirt without libnl and allows us to drop WITH_LIBNL checks.

> > Also, in this case you've removed the ability to add ports to a > bridge, even if the request has no vlan info. If there really are > Linux distros that don't have libnl then we don't want to completely > disable the ability to add a tap to a bridge - instead we should move > the check for vlan == NULL from the top of virNetDevBridgeSetupVlans() > to the place where virNetDevBridgeSetupVlans() is called (in > virNetDevBridgeAddPort()). Then virNetDeBridgeSetupVlans should be > placed inside  an #if defined(WITH_LIBNL) that has an #else > alternative implementation just reporting that vlans can't be set on > host bridge ports on this platform - this entire set of 2 > virNetDevBridgeeSetupVlan() functions should be defined immediately > preceding the definition of virNetDebBridgeAddPort(), inside the same > #if as that function. > > So in short, you should have: > > #if defined(WITH_STRUCT_IFREQ) && defined(SIOCBRADDIF) > # if defined(WITH_LIBNL) > static int > virNetDevBridgeSetupVlans() > { >    /* existing virNetDevBridgeSetupVlans() body except NULL check */ > } > # else > static int > virNetDevBridgeSetupVlans() > { >     virReportSystemError(ENOSYS, "%s", >                           _("Cannot configure vlans on host bridge ports >                             on this platform")); >                           /* *un*break previous two lines!! */ >      return -1;} > } > # fi > > > int > virNetDevBridgeAddPort(...) > { >    /* existing body of this function, except at the end do this: */ >    if (virtVlan && virtVlan->nTags) >        return virNetDevBridgeSetupVlans(ifname, virtVlan); >    else >        return 0; > } > #elif defined(WITH_BSD_BRIDGE_MGMT) > ... > > > This way you'll stil have the ability to add ports to a bridge when > libnl isn't available, but will get a runtime error if someone tries > to configure vlan stuff for a port. > >> --- >>   src/util/virnetdevbridge.c | 122 +++++++++++++++++++++ >> +----------------------- >>   1 file changed, 61 insertions(+), 61 deletions(-) >> >> diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c >> index >> c79d0c79b7b16a0a070b5013ed7df9d0a63b1c38..70e82a0029634ddf44c2b933577bf783c89f035c 100644 >> --- a/src/util/virnetdevbridge.c >> +++ b/src/util/virnetdevbridge.c >> @@ -313,66 +313,6 @@ virNetDevBridgePortSetIsolated(const char *brname, >>       return virNetDevBridgePortSet(brname, ifname, "isolated", >> enable ? 1 : 0); >>   } >> -static int >> -virNetDevBridgeSetupVlans(const char *ifname, const virNetDevVlan >> *virtVlan) >> -{ >> -    int error = 0; >> -    unsigned short flags; >> - >> -    if (!virtVlan || !virtVlan->nTags) >> -        return 0; >> - >> -    // The interface will have been automatically added to vlan 1, >> so remove it >> -    if (virNetlinkBridgeVlanFilterSet(ifname, RTM_DELLINK, 0, 1, >> &error) < 0) { >> -        if (error != 0) { >> -            virReportSystemError(-error, >> -                                 _("error removing vlan filter from >> interface %1$s"), >> -                                 ifname); >> -        } >> -        return -1; >> -    } >> - >> -    // If trunk mode, add the native VLAN then add the others, if any >> -    if (virtVlan->trunk) { >> -        size_t i; >> - >> -        if (virtVlan->nativeTag) { >> -            flags = BRIDGE_VLAN_INFO_PVID; >> -            if (virtVlan->nativeMode == >> VIR_NATIVE_VLAN_MODE_UNTAGGED || >> -                virtVlan->nativeMode == VIR_NATIVE_VLAN_MODE_DEFAULT) { >> -                flags |= BRIDGE_VLAN_INFO_UNTAGGED; >> -            } >> - >> -            if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, >> flags, >> -                                              virtVlan->nativeTag, >> &error) < 0) { >> -                goto error; >> -            } >> -        } >> - >> -        for (i = 0; i < virtVlan->nTags; i++) { >> -            if (virtVlan->tag[i] != virtVlan->nativeTag) >> -                if (virNetlinkBridgeVlanFilterSet(ifname, >> RTM_SETLINK, 0, >> -                                                  virtVlan->tag[i], >> &error) < 0) { >> -                    goto error; >> -                } >> -        } >> -    } else { >> -        // In native mode, add the single VLAN as pvid untagged >> -        flags = BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED; >> -        if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, flags, >> -                                          virtVlan->tag[0], &error) >> < 0) { >> -            goto error; >> -        } >> -    } >> - >> -    return 0; >> - >> - error: >> -    if (error != 0) >> -        virReportSystemError(-error, _("error adding vlan filter to >> interface %1$s"), ifname); >> -    return -1; >> -} >> - >>   #else >>   int >> @@ -651,7 +591,67 @@ int virNetDevBridgeDelete(const char *brname >> G_GNUC_UNUSED) >>    * >>    * Returns 0 in case of success or an errno code in case of failure. >>    */ >> -#if defined(WITH_STRUCT_IFREQ) && defined(SIOCBRADDIF) >> +#if defined(WITH_STRUCT_IFREQ) && defined(SIOCBRADDIF) && >> defined(WITH_LIBNL) >> +static int >> +virNetDevBridgeSetupVlans(const char *ifname, const virNetDevVlan >> *virtVlan) >> +{ >> +    int error = 0; >> +    unsigned short flags; >> + >> +    if (!virtVlan || !virtVlan->nTags) >> +        return 0; >> + >> +    // The interface will have been automatically added to vlan 1, >> so remove it >> +    if (virNetlinkBridgeVlanFilterSet(ifname, RTM_DELLINK, 0, 1, >> &error) < 0) { >> +        if (error != 0) { >> +            virReportSystemError(-error, >> +                                 _("error removing vlan filter from >> interface %1$s"), >> +                                 ifname); >> +        } >> +        return -1; >> +    } >> + >> +    // If trunk mode, add the native VLAN then add the others, if any >> +    if (virtVlan->trunk) { >> +        size_t i; >> + >> +        if (virtVlan->nativeTag) { >> +            flags = BRIDGE_VLAN_INFO_PVID; >> +            if (virtVlan->nativeMode == >> VIR_NATIVE_VLAN_MODE_UNTAGGED || >> +                virtVlan->nativeMode == VIR_NATIVE_VLAN_MODE_DEFAULT) { >> +                flags |= BRIDGE_VLAN_INFO_UNTAGGED; >> +            } >> + >> +            if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, >> flags, >> +                                              virtVlan->nativeTag, >> &error) < 0) { >> +                goto error; >> +            } >> +        } >> + >> +        for (i = 0; i < virtVlan->nTags; i++) { >> +            if (virtVlan->tag[i] != virtVlan->nativeTag) >> +                if (virNetlinkBridgeVlanFilterSet(ifname, >> RTM_SETLINK, 0, >> +                                                  virtVlan->tag[i], >> &error) < 0) { >> +                    goto error; >> +                } >> +        } >> +    } else { >> +        // In native mode, add the single VLAN as pvid untagged >> +        flags = BRIDGE_VLAN_INFO_PVID | BRIDGE_VLAN_INFO_UNTAGGED; >> +        if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, flags, >> +                                          virtVlan->tag[0], &error) >> < 0) { >> +            goto error; >> +        } >> +    } >> + >> +    return 0; >> + >> + error: >> +    if (error != 0) >> +        virReportSystemError(-error, _("error adding vlan filter to >> interface %1$s"), ifname); >> +    return -1; >> +} >> + >>   int virNetDevBridgeAddPort(const char *brname, >>                              const char *ifname, >>                              const virNetDevVlan *virtVlan) >> >> --- >> base-commit: e5299ddf86121d3c792ca271ffcb54900eb19dc3 >> change-id: 20250304-nl-605e05b8c5f2 >> >> Best regards, >