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