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@daynix.com>
I'm curious what is the Linux platform where we don't have libnl available? 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,