[RFC v3 PATCH 0/4] iproute2 bridge vlan support
As requested by Laine, I have converted the code to use netlink rather than executing bridge vlan commands. I have also checked it compiles under FreeBSD. Description ----------- The iproute2 bridge command supports the capability for VLAN filtering that allows each interface connected to a standard linux bridge to be configured to use one or more VLANs. For simple setups, this capability is enough to allow virtual machines or containers to be put onto separate VLANs without creating multiple bridges and VLANs on the host. The first patch adds a new function virNetDevBridgeSetupVlans() that will, given a virNetDevVlan structure, execute the required bridge vlan commands to configure the given interface accordingly. The second patch updates the virNetDevBridgeAddPort() function to allow a virNetDevVlan parameter to be passed, and to call the virNetDevBridgeSetupVlans() function. The third patch updates the lxc and tap code to pass the virNetDevLan parameter from the configuration and to update the XML domain and network validation to permit the VLAN-related tags for standard bridges. The fourth patch updates documentation to match the new capability. Changes since v2 ---------------- - Convert to use netlink rather than executing bridge vlan commands. - Add unsupported on this platform error message on FreeBSD. Changes since v1 ---------------- - Fix bug in virNetDevSetupVlans where bridge port has no native vlan. - Update bridge network validation to permit vlan configuration. - Update documentation to match the functionality. - Tweak some of the commit descriptions for clarity. Usage example ------------- Configure the host with systemd-networkd as follows: /etc/systemd/network/br0.netdev (br0.network not shown) [NetDev] Name=br0 Kind=bridge MACAddress=xx:xx:xx:xx:xx:xx [Bridge] VLANFiltering=on /etc/systemd/network/eno1.network [Match] Name=eno1 [Network] Bridge=br0 [Link] MTUBytes=9000 [BridgeVLAN] VLAN=40 [BridgeVLAN] VLAN=60 Then add <vlan> tags into the lxc or qemu config: lxc interface definition: <interface type='bridge'> <mac address='xx:xx:xx:xx:xx:xx'/> <source bridge='br0'/> <vlan> <tag id='40'/> </vlan> </interface> qemu interface definition: <interface type='network'> <mac address='xx:xx:xx:xx:xx:xx'/> <source network='br0'/> <vlan> <tag id='60'/> </vlan> <model type='virtio'/> <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </interface> Then, after starting them, you will see the following $ sudo bridge vlan port vlan-id eno1 1 PVID Egress Untagged 40 60 br0 1 PVID Egress Untagged vnet0 60 PVID Egress Untagged vnet1 40 PVID Egress Untagged Regards, Leigh Brown (4): util: add netlink bridge vlan filtering util: Add vlan support to virNetDevBridgeAddPort Enable vlan support for standard linux bridges docs: standard linux bridges now support vlans docs/formatdomain.rst | 37 +++++++++--------- docs/formatnetwork.rst | 45 +++++++++++----------- src/conf/domain_validate.c | 3 +- src/lxc/lxc_process.c | 3 +- src/network/bridge_driver.c | 13 ++++--- src/util/virnetdevbridge.c | 75 +++++++++++++++++++++++++++++++++++-- src/util/virnetdevbridge.h | 4 +- src/util/virnetdevtap.c | 2 +- src/util/virnetlink.c | 66 ++++++++++++++++++++++++++++++++ src/util/virnetlink.h | 7 ++++ 10 files changed, 202 insertions(+), 53 deletions(-) -- 2.39.5
Enable capability to add and remove vlan filters for a standard linux bridge using netlink. New function virNetlinkBridgeVlanFilterSet can be used to add or remove a vlan filter to a given bridge interface. Signed-off-by: Leigh Brown <leigh@solinno.co.uk> --- src/util/virnetlink.c | 66 +++++++++++++++++++++++++++++++++++++++++++ src/util/virnetlink.h | 7 +++++ 2 files changed, 73 insertions(+) diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 24cd69a385..206646d9d7 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -701,6 +701,72 @@ virNetlinkDelLink(const char *ifname, virNetlinkTalkFallback fallback) return 0; } +/** + * virNetlinkBridgeVlanFilterSet: + * + * @ifname: name of the link + * @cmd: netlink command, either RTM_SETLINK or RTM_DELLINK + * @flags: flags to use when adding the vlan filter + * @vid: vlan id to add or remove + * @error: netlink error code + * + * Add or remove a vlan filter from an interface associated with a + * bridge. + * + * Returns 0 on success, -1 on error. Additionally, if the @error is + * non-zero, then a netlink failure occurred, but no error message + * is generated leaving it up to the caller to handle the condition. + */ +int +virNetlinkBridgeVlanFilterSet(const char *ifname, + int cmd, + const unsigned short flags, + const short vid, + int *error) +{ + struct ifinfomsg ifm = { .ifi_family = PF_BRIDGE }; + struct bridge_vlan_info vinfo = { .flags = flags, .vid = vid }; + struct nlattr *afspec = NULL; + g_autoptr(virNetlinkMsg) nl_msg = NULL; + g_autofree struct nlmsghdr *resp = NULL; + unsigned int resp_len = 0; + + *error = 0; + + if (vid < 1 || vid > 4095) { + virReportError(ERANGE, _("vlanid out of range: %1$d"), vid); + return -1; + } + + if (!(cmd == RTM_SETLINK || cmd == RTM_DELLINK)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid vlan filter command %1$d"), cmd); + return -1; + } + + if (virNetDevGetIndex(ifname, &ifm.ifi_index) < 0) + return -1; + + nl_msg = virNetlinkMsgNew(cmd, NLM_F_REQUEST); + + NETLINK_MSG_APPEND(nl_msg, sizeof(ifm), &ifm); + + NETLINK_MSG_NEST_START(nl_msg, afspec, IFLA_AF_SPEC); + NETLINK_MSG_PUT(nl_msg, IFLA_BRIDGE_VLAN_INFO, sizeof(vinfo), &vinfo); + NETLINK_MSG_NEST_END(nl_msg, afspec); + + if (virNetlinkTalk(ifname, nl_msg, 0, 0, &resp, &resp_len, error, NULL) < 0) + return -1; + + if (resp->nlmsg_type != NLMSG_ERROR && resp->nlmsg_type != NLMSG_DONE) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed netlink response message")); + return -1; + } + + return 0; +} + /** * virNetlinkGetNeighbor: * diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h index 75192f645f..327fb426a1 100644 --- a/src/util/virnetlink.h +++ b/src/util/virnetlink.h @@ -25,6 +25,7 @@ #if defined(WITH_LIBNL) # include <netlink/msg.h> +# include <linux/if_bridge.h> typedef struct nl_msg virNetlinkMsg; G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetlinkMsg, nlmsg_free); @@ -76,6 +77,12 @@ typedef int (*virNetlinkTalkFallback)(const char *ifname); int virNetlinkDelLink(const char *ifname, virNetlinkTalkFallback fallback); +int virNetlinkBridgeVlanFilterSet(const char *ifname, + int cmd, + const unsigned short flags, + const short vid, + int *error); + int virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen); int virNetlinkDumpLink(const char *ifname, int ifindex, -- 2.39.5
On 1/2/25 4:29 AM, Leigh Brown wrote:
Enable capability to add and remove vlan filters for a standard linux bridge using netlink.
New function virNetlinkBridgeVlanFilterSet can be used to add or remove a vlan filter to a given bridge interface.
Signed-off-by: Leigh Brown <leigh@solinno.co.uk>
Reviewed-by: Laine Stump <laine@redhat.com>
--- src/util/virnetlink.c | 66 +++++++++++++++++++++++++++++++++++++++++++ src/util/virnetlink.h | 7 +++++ 2 files changed, 73 insertions(+)
diff --git a/src/util/virnetlink.c b/src/util/virnetlink.c index 24cd69a385..206646d9d7 100644 --- a/src/util/virnetlink.c +++ b/src/util/virnetlink.c @@ -701,6 +701,72 @@ virNetlinkDelLink(const char *ifname, virNetlinkTalkFallback fallback) return 0; }
+/** + * virNetlinkBridgeVlanFilterSet: + * + * @ifname: name of the link + * @cmd: netlink command, either RTM_SETLINK or RTM_DELLINK + * @flags: flags to use when adding the vlan filter + * @vid: vlan id to add or remove + * @error: netlink error code + * + * Add or remove a vlan filter from an interface associated with a + * bridge. + * + * Returns 0 on success, -1 on error. Additionally, if the @error is + * non-zero, then a netlink failure occurred, but no error message + * is generated leaving it up to the caller to handle the condition. + */ +int +virNetlinkBridgeVlanFilterSet(const char *ifname, + int cmd, + const unsigned short flags, + const short vid, + int *error) +{ + struct ifinfomsg ifm = { .ifi_family = PF_BRIDGE }; + struct bridge_vlan_info vinfo = { .flags = flags, .vid = vid }; + struct nlattr *afspec = NULL; + g_autoptr(virNetlinkMsg) nl_msg = NULL; + g_autofree struct nlmsghdr *resp = NULL; + unsigned int resp_len = 0; + + *error = 0; + + if (vid < 1 || vid > 4095) { + virReportError(ERANGE, _("vlanid out of range: %1$d"), vid); + return -1; + } + + if (!(cmd == RTM_SETLINK || cmd == RTM_DELLINK)) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Invalid vlan filter command %1$d"), cmd); + return -1; + } + + if (virNetDevGetIndex(ifname, &ifm.ifi_index) < 0) + return -1; + + nl_msg = virNetlinkMsgNew(cmd, NLM_F_REQUEST); + + NETLINK_MSG_APPEND(nl_msg, sizeof(ifm), &ifm); + + NETLINK_MSG_NEST_START(nl_msg, afspec, IFLA_AF_SPEC); + NETLINK_MSG_PUT(nl_msg, IFLA_BRIDGE_VLAN_INFO, sizeof(vinfo), &vinfo); + NETLINK_MSG_NEST_END(nl_msg, afspec); + + if (virNetlinkTalk(ifname, nl_msg, 0, 0, &resp, &resp_len, error, NULL) < 0) + return -1; + + if (resp->nlmsg_type != NLMSG_ERROR && resp->nlmsg_type != NLMSG_DONE) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("malformed netlink response message")); + return -1; + } + + return 0; +} + /** * virNetlinkGetNeighbor: * diff --git a/src/util/virnetlink.h b/src/util/virnetlink.h index 75192f645f..327fb426a1 100644 --- a/src/util/virnetlink.h +++ b/src/util/virnetlink.h @@ -25,6 +25,7 @@ #if defined(WITH_LIBNL)
# include <netlink/msg.h> +# include <linux/if_bridge.h>
typedef struct nl_msg virNetlinkMsg; G_DEFINE_AUTOPTR_CLEANUP_FUNC(virNetlinkMsg, nlmsg_free); @@ -76,6 +77,12 @@ typedef int (*virNetlinkTalkFallback)(const char *ifname);
int virNetlinkDelLink(const char *ifname, virNetlinkTalkFallback fallback);
+int virNetlinkBridgeVlanFilterSet(const char *ifname, + int cmd, + const unsigned short flags, + const short vid, + int *error); + int virNetlinkGetErrorCode(struct nlmsghdr *resp, unsigned int recvbuflen);
int virNetlinkDumpLink(const char *ifname, int ifindex,
Add virNetDevBridgeSetupVlans function to configures a bridge interface using the passed virNetDevVlan struct. Add virVlan parameter to the Linux version of virNetDevBridgeAddPort and call virNetDevBridgeSetupVlans to set up the required vlan configuration. Update callers of virNetDevBridgeAddPort to pass NULL for now. Signed-off-by: Leigh Brown <leigh@solinno.co.uk> --- src/lxc/lxc_process.c | 2 +- src/util/virnetdevbridge.c | 75 ++++++++++++++++++++++++++++++++++++-- src/util/virnetdevbridge.h | 4 +- src/util/virnetdevtap.c | 2 +- 4 files changed, 76 insertions(+), 7 deletions(-) diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index c2982244f0..7c760cec40 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -289,7 +289,7 @@ virLXCProcessSetupInterfaceTap(virDomainDef *vm, vport, virDomainNetGetActualVlan(net)) < 0) return NULL; } else { - if (virNetDevBridgeAddPort(brname, parentVeth) < 0) + if (virNetDevBridgeAddPort(brname, parentVeth, NULL) < 0) return NULL; if (virDomainNetGetActualPortOptionsIsolated(net) == VIR_TRISTATE_BOOL_YES && diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 5fd88f3195..80f028e9b7 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -313,6 +313,65 @@ 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) + goto err_delete; + + // 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 err_add; + } + + 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 err_add; + } + } 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 err_add; + } + + return 0; + + err_add: + if (error != 0) + virReportSystemError(-error, + _("error adding vlan filter to interface %1$s"), + ifname); + return -1; + + err_delete: + if (error != 0) + virReportSystemError(-error, + _("error removing vlan filter from interface %1$s"), + ifname); + return -1; +} + #else int @@ -593,7 +652,8 @@ int virNetDevBridgeDelete(const char *brname G_GNUC_UNUSED) */ #if defined(WITH_STRUCT_IFREQ) && defined(SIOCBRADDIF) int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) { struct ifreq ifr; VIR_AUTOCLOSE fd = -1; @@ -613,14 +673,20 @@ int virNetDevBridgeAddPort(const char *brname, return -1; } - return 0; + return virNetDevBridgeSetupVlans(ifname, virtVlan); } #elif defined(WITH_BSD_BRIDGE_MGMT) int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) { struct ifbreq req = { 0 }; + if (virtVlan) { + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; + } + if (virStrcpyStatic(req.ifbr_ifsname, ifname) < 0) { virReportSystemError(ERANGE, _("Network interface name '%1$s' is too long"), @@ -638,7 +704,8 @@ int virNetDevBridgeAddPort(const char *brname, } #else int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) { virReportSystemError(ENOSYS, _("Unable to add bridge %1$s port %2$s"), brname, ifname); diff --git a/src/util/virnetdevbridge.h b/src/util/virnetdevbridge.h index db4099bf0b..5f51656abe 100644 --- a/src/util/virnetdevbridge.h +++ b/src/util/virnetdevbridge.h @@ -20,6 +20,7 @@ #include "internal.h" #include "virmacaddr.h" +#include "virnetdevvlan.h" int virNetDevBridgeCreate(const char *brname, const virMacAddr *mac) @@ -28,7 +29,8 @@ int virNetDevBridgeDelete(const char *brname) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; int virNetDevBridgeRemovePort(const char *brname, diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 2701ba6dfc..a9573eb8e1 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -483,7 +483,7 @@ virNetDevTapAttachBridge(const char *tapname, return -1; } } else { - if (virNetDevBridgeAddPort(brname, tapname) < 0) + if (virNetDevBridgeAddPort(brname, tapname, NULL) < 0) return -1; if (isolatedPort == VIR_TRISTATE_BOOL_YES && -- 2.39.5
On 1/2/25 4:29 AM, Leigh Brown wrote:
Add virNetDevBridgeSetupVlans function to configures a bridge interface using the passed virNetDevVlan struct.
Add virVlan parameter to the Linux version of virNetDevBridgeAddPort and call virNetDevBridgeSetupVlans to set up the required vlan configuration.
Update callers of virNetDevBridgeAddPort to pass NULL for now.
Signed-off-by: Leigh Brown <leigh@solinno.co.uk>
--- src/lxc/lxc_process.c | 2 +- src/util/virnetdevbridge.c | 75 ++++++++++++++++++++++++++++++++++++-- src/util/virnetdevbridge.h | 4 +- src/util/virnetdevtap.c | 2 +- 4 files changed, 76 insertions(+), 7 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index c2982244f0..7c760cec40 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -289,7 +289,7 @@ virLXCProcessSetupInterfaceTap(virDomainDef *vm, vport, virDomainNetGetActualVlan(net)) < 0) return NULL; } else { - if (virNetDevBridgeAddPort(brname, parentVeth) < 0) + if (virNetDevBridgeAddPort(brname, parentVeth, NULL) < 0) return NULL;
if (virDomainNetGetActualPortOptionsIsolated(net) == VIR_TRISTATE_BOOL_YES && diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 5fd88f3195..80f028e9b7 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -313,6 +313,65 @@ 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) + goto err_delete; + + // 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;
I think some people aren't as pedantic about it as I am (since I often encounter braceless code like this), but the libvirt coding guidelines (at https://libvirt.org/coding-style.html) do require adding { } when the conditional expression itself takes up multiple lines at the same indentation level, even if the code inside the conditional is just a single line.
+ + if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, flags, + virtVlan->nativeTag, &error) < 0) + goto err_add;
(For some reason, the coding guidelines *don't* require braces in this case (they say it's optional because the difference in indentation "makes it obvious it is a single line"), but I still prefer braces in this case as well)
+ } + + 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 err_add; + } + } 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 err_add; + }
(NB: I'm assuming that the netlink commands you're sending actually do the correct thing (i.e. identical to what is done for the same config when connecting to an OVS bridge) in all the cases (trunking, etc), as I don't use vlans myself. What you have looks like it makes sense though :-))
+ + return 0; + + err_add: + if (error != 0) + virReportSystemError(-error, + _("error adding vlan filter to interface %1$s"), + ifname); + return -1; + + err_delete: + if (error != 0) + virReportSystemError(-error, + _("error removing vlan filter from interface %1$s"), + ifname); + return -1;
Although there are a few other instances of a goto label with a name other than "cleanup" or "error", we try to keep it to those as much as possible. Since there's only a single "goto err_delete", maybe we could just move that error reporting and return up to the place that jumps to it (and then rename err_add to error)? (see the "Use of goto" section in the coding-style document I referenced above) If you are okay with these two changes and want to make them today, I can push an updated version tonight, or if you're okay with them but don't have the time to make those changes today, I can do it for you before pushing; let me know one way or the other. Assuming those changes: Reviewed-by: Laine Stump <laine@redhat.com>
+} +
#else int @@ -593,7 +652,8 @@ int virNetDevBridgeDelete(const char *brname G_GNUC_UNUSED) */ #if defined(WITH_STRUCT_IFREQ) && defined(SIOCBRADDIF) int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) { struct ifreq ifr; VIR_AUTOCLOSE fd = -1; @@ -613,14 +673,20 @@ int virNetDevBridgeAddPort(const char *brname, return -1; }
- return 0; + return virNetDevBridgeSetupVlans(ifname, virtVlan); } #elif defined(WITH_BSD_BRIDGE_MGMT) int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) { struct ifbreq req = { 0 };
+ if (virtVlan) { + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; + } + if (virStrcpyStatic(req.ifbr_ifsname, ifname) < 0) { virReportSystemError(ERANGE, _("Network interface name '%1$s' is too long"), @@ -638,7 +704,8 @@ int virNetDevBridgeAddPort(const char *brname, } #else int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) { virReportSystemError(ENOSYS, _("Unable to add bridge %1$s port %2$s"), brname, ifname); diff --git a/src/util/virnetdevbridge.h b/src/util/virnetdevbridge.h index db4099bf0b..5f51656abe 100644 --- a/src/util/virnetdevbridge.h +++ b/src/util/virnetdevbridge.h @@ -20,6 +20,7 @@
#include "internal.h" #include "virmacaddr.h" +#include "virnetdevvlan.h"
int virNetDevBridgeCreate(const char *brname, const virMacAddr *mac) @@ -28,7 +29,8 @@ int virNetDevBridgeDelete(const char *brname) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT;
int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;
int virNetDevBridgeRemovePort(const char *brname, diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 2701ba6dfc..a9573eb8e1 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -483,7 +483,7 @@ virNetDevTapAttachBridge(const char *tapname, return -1; } } else { - if (virNetDevBridgeAddPort(brname, tapname) < 0) + if (virNetDevBridgeAddPort(brname, tapname, NULL) < 0) return -1;
if (isolatedPort == VIR_TRISTATE_BOOL_YES &&
Hello, On 2025-01-08 07:26, Laine Stump wrote:
On 1/2/25 4:29 AM, Leigh Brown wrote:
Add virNetDevBridgeSetupVlans function to configures a bridge interface using the passed virNetDevVlan struct.
Add virVlan parameter to the Linux version of virNetDevBridgeAddPort and call virNetDevBridgeSetupVlans to set up the required vlan configuration.
Update callers of virNetDevBridgeAddPort to pass NULL for now.
Signed-off-by: Leigh Brown <leigh@solinno.co.uk>
--- src/lxc/lxc_process.c | 2 +- src/util/virnetdevbridge.c | 75 ++++++++++++++++++++++++++++++++++++-- src/util/virnetdevbridge.h | 4 +- src/util/virnetdevtap.c | 2 +- 4 files changed, 76 insertions(+), 7 deletions(-)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index c2982244f0..7c760cec40 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -289,7 +289,7 @@ virLXCProcessSetupInterfaceTap(virDomainDef *vm, vport, virDomainNetGetActualVlan(net)) < 0) return NULL; } else { - if (virNetDevBridgeAddPort(brname, parentVeth) < 0) + if (virNetDevBridgeAddPort(brname, parentVeth, NULL) < 0) return NULL; if (virDomainNetGetActualPortOptionsIsolated(net) == VIR_TRISTATE_BOOL_YES && diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 5fd88f3195..80f028e9b7 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c @@ -313,6 +313,65 @@ 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) + goto err_delete; + + // 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;
I think some people aren't as pedantic about it as I am (since I often encounter braceless code like this), but the libvirt coding guidelines (at https://libvirt.org/coding-style.html) do require adding { } when the conditional expression itself takes up multiple lines at the same indentation level, even if the code inside the conditional is just a single line.
I will update.
+ + if (virNetlinkBridgeVlanFilterSet(ifname, RTM_SETLINK, flags, + virtVlan->nativeTag, &error) < 0) + goto err_add;
(For some reason, the coding guidelines *don't* require braces in this case (they say it's optional because the difference in indentation "makes it obvious it is a single line"), but I still prefer braces in this case as well)
I will update.
+ } + + 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 err_add; + } + } 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 err_add; + }
(NB: I'm assuming that the netlink commands you're sending actually do the correct thing (i.e. identical to what is done for the same config when connecting to an OVS bridge) in all the cases (trunking, etc), as I don't use vlans myself. What you have looks like it makes sense though :-))
I've not used OVS but I read the documentation and I believe the behaviour is the same.
+ + return 0; + + err_add: + if (error != 0) + virReportSystemError(-error, + _("error adding vlan filter to interface %1$s"), + ifname); + return -1; + + err_delete: + if (error != 0) + virReportSystemError(-error, + _("error removing vlan filter from interface %1$s"), + ifname); + return -1;
Although there are a few other instances of a goto label with a name other than "cleanup" or "error", we try to keep it to those as much as possible. Since there's only a single "goto err_delete", maybe we could just move that error reporting and return up to the place that jumps to it (and then rename err_add to error)? (see the "Use of goto" section in the coding-style document I referenced above)
If you are okay with these two changes and want to make them today, I can push an updated version tonight, or if you're okay with them but don't have the time to make those changes today, I can do it for you before pushing; let me know one way or the other.
Thanks, I will make the changes and send an updated version.
Assuming those changes:
Reviewed-by: Laine Stump <laine@redhat.com>
+} + #else int @@ -593,7 +652,8 @@ int virNetDevBridgeDelete(const char *brname G_GNUC_UNUSED) */ #if defined(WITH_STRUCT_IFREQ) && defined(SIOCBRADDIF) int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) { struct ifreq ifr; VIR_AUTOCLOSE fd = -1; @@ -613,14 +673,20 @@ int virNetDevBridgeAddPort(const char *brname, return -1; } - return 0; + return virNetDevBridgeSetupVlans(ifname, virtVlan); } #elif defined(WITH_BSD_BRIDGE_MGMT) int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) { struct ifbreq req = { 0 }; + if (virtVlan) { + virReportSystemError(ENOSYS, "%s", _("Not supported on this platform")); + return -1; + } + if (virStrcpyStatic(req.ifbr_ifsname, ifname) < 0) { virReportSystemError(ERANGE, _("Network interface name '%1$s' is too long"), @@ -638,7 +704,8 @@ int virNetDevBridgeAddPort(const char *brname, } #else int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) { virReportSystemError(ENOSYS, _("Unable to add bridge %1$s port %2$s"), brname, ifname); diff --git a/src/util/virnetdevbridge.h b/src/util/virnetdevbridge.h index db4099bf0b..5f51656abe 100644 --- a/src/util/virnetdevbridge.h +++ b/src/util/virnetdevbridge.h @@ -20,6 +20,7 @@ #include "internal.h" #include "virmacaddr.h" +#include "virnetdevvlan.h" int virNetDevBridgeCreate(const char *brname, const virMacAddr *mac) @@ -28,7 +29,8 @@ int virNetDevBridgeDelete(const char *brname) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT; int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT; int virNetDevBridgeRemovePort(const char *brname, diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 2701ba6dfc..a9573eb8e1 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -483,7 +483,7 @@ virNetDevTapAttachBridge(const char *tapname, return -1; } } else { - if (virNetDevBridgeAddPort(brname, tapname) < 0) + if (virNetDevBridgeAddPort(brname, tapname, NULL) < 0) return -1; if (isolatedPort == VIR_TRISTATE_BOOL_YES &&
The CI pipeline just found a minor problem - see below: On 1/2/25 4:29 AM, Leigh Brown wrote:
Add virNetDevBridgeSetupVlans function to configures a bridge interface using the passed virNetDevVlan struct.
Add virVlan parameter to the Linux version of virNetDevBridgeAddPort and call virNetDevBridgeSetupVlans to set up the required vlan configuration.
Update callers of virNetDevBridgeAddPort to pass NULL for now.
Signed-off-by: Leigh Brown <leigh@solinno.co.uk> ---
[...]
diff --git a/src/util/virnetdevbridge.c b/src/util/virnetdevbridge.c index 5fd88f3195..80f028e9b7 100644 --- a/src/util/virnetdevbridge.c +++ b/src/util/virnetdevbridge.c
[...]
@@ -638,7 +704,8 @@ int virNetDevBridgeAddPort(const char *brname, } #else int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan)
The last arg (virtVlan) needs a G_GNUC_UNUSED, otherwise the CI build fails for MacOS.
{ virReportSystemrror(ENOSYS, _("Unable to add bridge %1$s port %2$s"), brname, ifname); diff --git a/src/util/virnetdevbridge.h b/src/util/virnetdevbridge.h index db4099bf0b..5f51656abe 100644 --- a/src/util/virnetdevbridge.h +++ b/src/util/virnetdevbridge.h @@ -20,6 +20,7 @@
#include "internal.h" #include "virmacaddr.h" +#include "virnetdevvlan.h"
int virNetDevBridgeCreate(const char *brname, const virMacAddr *mac) @@ -28,7 +29,8 @@ int virNetDevBridgeDelete(const char *brname) ATTRIBUTE_NONNULL(1) G_GNUC_WARN_UNUSED_RESULT;
int virNetDevBridgeAddPort(const char *brname, - const char *ifname) + const char *ifname, + const virNetDevVlan *virtVlan) ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) G_GNUC_WARN_UNUSED_RESULT;
int virNetDevBridgeRemovePort(const char *brname, diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index 2701ba6dfc..a9573eb8e1 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -483,7 +483,7 @@ virNetDevTapAttachBridge(const char *tapname, return -1; } } else { - if (virNetDevBridgeAddPort(brname, tapname) < 0) + if (virNetDevBridgeAddPort(brname, tapname, NULL) < 0) return -1;
if (isolatedPort == VIR_TRISTATE_BOOL_YES &&
Adjust domain and network validation to permit standard linux bridges to allow vlan configuration. Update calls to virNetDevBridgeAddPort to pass the vlan configuration. Signed-off-by: Leigh Brown <leigh@solinno.co.uk> --- src/conf/domain_validate.c | 3 ++- src/lxc/lxc_process.c | 3 ++- src/network/bridge_driver.c | 13 ++++++++----- src/util/virnetdevtap.c | 2 +- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 1034bb57f5..c7a79a0277 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2077,7 +2077,8 @@ virDomainActualNetDefValidate(const virDomainNetDef *net) (actualType == VIR_DOMAIN_NET_TYPE_DIRECT && virDomainNetGetActualDirectMode(net) == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) || (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE && - vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) { + vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) || + (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE && !vport))) { virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("interface %1$s - vlan tag not supported for this connection type"), macstr); diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 7c760cec40..c427b38605 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -289,7 +289,8 @@ virLXCProcessSetupInterfaceTap(virDomainDef *vm, vport, virDomainNetGetActualVlan(net)) < 0) return NULL; } else { - if (virNetDevBridgeAddPort(brname, parentVeth, NULL) < 0) + if (virNetDevBridgeAddPort(brname, parentVeth, + virDomainNetGetActualVlan(net)) < 0) return NULL; if (virDomainNetGetActualPortOptionsIsolated(net) == VIR_TRISTATE_BOOL_YES && diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ce793c12ef..8f47ef2574 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2999,7 +2999,8 @@ networkValidate(virNetworkDriverState *driver, /* The only type of networks that currently support transparent * vlan configuration are those using hostdev sr-iov devices from - * a pool, and those using an Open vSwitch bridge. + * a pool, those using an Open vSwitch bridge, and standard linux + * bridges. */ vlanAllowed = (def->forward.type == VIR_NETWORK_FORWARD_HOSTDEV || @@ -3007,15 +3008,17 @@ networkValidate(virNetworkDriverState *driver, (def->forward.type == VIR_NETWORK_FORWARD_BRIDGE && def->virtPortProfile && def->virtPortProfile->virtPortType - == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)); + == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) || + (def->forward.type == VIR_NETWORK_FORWARD_BRIDGE && + !def->virtPortProfile)); vlanUsed = def->vlan.nTags > 0; for (i = 0; i < def->nPortGroups; i++) { if (vlanUsed || def->portGroups[i].vlan.nTags > 0) { /* anyone using this portgroup will get a vlan tag. Verify - * that they will also be using an openvswitch connection, - * as that is the only type of network that currently - * supports a vlan tag. + * that they will also be using an openvswitch connection + * or a standard linux bridge as they are the only types of + * network that currently support a vlan tag. */ if (def->portGroups[i].virtPortProfile) { if (def->forward.type != VIR_NETWORK_FORWARD_BRIDGE || diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index a9573eb8e1..1dc77f0f5c 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -483,7 +483,7 @@ virNetDevTapAttachBridge(const char *tapname, return -1; } } else { - if (virNetDevBridgeAddPort(brname, tapname, NULL) < 0) + if (virNetDevBridgeAddPort(brname, tapname, virtVlan) < 0) return -1; if (isolatedPort == VIR_TRISTATE_BOOL_YES && -- 2.39.5
On 1/2/25 4:29 AM, Leigh Brown wrote:
Adjust domain and network validation to permit standard linux bridges to allow vlan configuration.
Update calls to virNetDevBridgeAddPort to pass the vlan configuration.
Signed-off-by: Leigh Brown <leigh@solinno.co.uk> --- src/conf/domain_validate.c | 3 ++- src/lxc/lxc_process.c | 3 ++- src/network/bridge_driver.c | 13 ++++++++----- src/util/virnetdevtap.c | 2 +- 4 files changed, 13 insertions(+), 8 deletions(-)
diff --git a/src/conf/domain_validate.c b/src/conf/domain_validate.c index 1034bb57f5..c7a79a0277 100644 --- a/src/conf/domain_validate.c +++ b/src/conf/domain_validate.c @@ -2077,7 +2077,8 @@ virDomainActualNetDefValidate(const virDomainNetDef *net) (actualType == VIR_DOMAIN_NET_TYPE_DIRECT && virDomainNetGetActualDirectMode(net) == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) || (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE && - vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH))) { + vport && vport->virtPortType == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) || + (actualType == VIR_DOMAIN_NET_TYPE_BRIDGE && !vport))) {
it's pre-existing, but while changing the first line, the double space in "vport && " should be made a single space. Alsi I think it would look more consistent if the new line have "!vport" first, e.g.: (!vport && actualType == VIR_DOMAIN_NET_TYPE_BRIDGE))) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, _("interface %1$s - vlan tag not supported for this connection type"), macstr); diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 7c760cec40..c427b38605 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -289,7 +289,8 @@ virLXCProcessSetupInterfaceTap(virDomainDef *vm, vport, virDomainNetGetActualVlan(net)) < 0) return NULL; } else { - if (virNetDevBridgeAddPort(brname, parentVeth, NULL) < 0) + if (virNetDevBridgeAddPort(brname, parentVeth, + virDomainNetGetActualVlan(net)) < 0)
It's okay to leave the above as a single line - long ago we used to strictly limit to 80 character lines, but these days the coding-style says to "aim for 80-90 characters, with a maximum of 100" (except for error message strings, which should always be kept on a single line, no matter how long)
return NULL;
if (virDomainNetGetActualPortOptionsIsolated(net) == VIR_TRISTATE_BOOL_YES && diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index ce793c12ef..8f47ef2574 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2999,7 +2999,8 @@ networkValidate(virNetworkDriverState *driver,
/* The only type of networks that currently support transparent * vlan configuration are those using hostdev sr-iov devices from - * a pool, and those using an Open vSwitch bridge. + * a pool, those using an Open vSwitch bridge, and standard linux + * bridges. */
vlanAllowed = (def->forward.type == VIR_NETWORK_FORWARD_HOSTDEV || @@ -3007,15 +3008,17 @@ networkValidate(virNetworkDriverState *driver, (def->forward.type == VIR_NETWORK_FORWARD_BRIDGE && def->virtPortProfile && def->virtPortProfile->virtPortType - == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH)); + == VIR_NETDEV_VPORT_PROFILE_OPENVSWITCH) || + (def->forward.type == VIR_NETWORK_FORWARD_BRIDGE && + !def->virtPortProfile));
vlanUsed = def->vlan.nTags > 0; for (i = 0; i < def->nPortGroups; i++) { if (vlanUsed || def->portGroups[i].vlan.nTags > 0) { /* anyone using this portgroup will get a vlan tag. Verify - * that they will also be using an openvswitch connection, - * as that is the only type of network that currently - * supports a vlan tag. + * that they will also be using an openvswitch connection + * or a standard linux bridge as they are the only types of + * network that currently support a vlan tag. */ if (def->portGroups[i].virtPortProfile) { if (def->forward.type != VIR_NETWORK_FORWARD_BRIDGE || diff --git a/src/util/virnetdevtap.c b/src/util/virnetdevtap.c index a9573eb8e1..1dc77f0f5c 100644 --- a/src/util/virnetdevtap.c +++ b/src/util/virnetdevtap.c @@ -483,7 +483,7 @@ virNetDevTapAttachBridge(const char *tapname, return -1; } } else { - if (virNetDevBridgeAddPort(brname, tapname, NULL) < 0) + if (virNetDevBridgeAddPort(brname, tapname, virtVlan) < 0) return -1;
if (isolatedPort == VIR_TRISTATE_BOOL_YES &&
Update domain XML and network XML documentation to describe how standard linux bridges support the VLAN configuration. Signed-off-by: Leigh Brown <leigh@solinno.co.uk> --- docs/formatdomain.rst | 37 +++++++++++++++++----------------- docs/formatnetwork.rst | 45 +++++++++++++++++++++--------------------- 2 files changed, 42 insertions(+), 40 deletions(-) diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 8d787ef59a..90b025508a 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -6039,28 +6039,29 @@ VLAN tags to apply to the guest's network traffic :since:`Since 0.10.0`. Network connections that support guest-transparent VLAN tagging include ``type='bridge'`` interfaces connected to an Open vSwitch bridge, SRIOV -Virtual Functions (VF) used via ``type='hostdev'`` (direct device assignment) -and, :since:`since 1.3.5`, SRIOV VFs used via ``type='direct'`` with -``mode='passthrough'`` (macvtap "passthru" mode). All other -connection types, including standard linux bridges and libvirt's own virtual +Virtual Functions (VF) used via ``type='hostdev'`` (direct device assignment), +:since:`since 1.3.5`, SRIOV VFs used via ``type='direct'`` with +``mode='passthrough'`` (macvtap "passthru" mode) and, :since:`since 11.0.0` +standard linux bridges. Other connection types, including libvirt's own virtual networks, **do not** support it. 802.1Qbh (vn-link) and 802.1Qbg (VEPA) switches provide their own way (outside of libvirt) to tag guest traffic onto a specific VLAN. Each tag is given in a separate ``<tag>`` subelement of ``<vlan>`` (for example: ``<tag id='42'/>``). For VLAN trunking of multiple tags (which is -supported only on Open vSwitch connections), multiple ``<tag>`` subelements can -be specified, which implies that the user wants to do VLAN trunking on the -interface for all the specified tags. In the case that VLAN trunking of a single -tag is desired, the optional attribute ``trunk='yes'`` can be added to the -toplevel ``<vlan>`` element to differentiate trunking of a single tag from -normal tagging. - -For network connections using Open vSwitch it is also possible to configure -'native-tagged' and 'native-untagged' VLAN modes :since:`Since 1.1.0`. This is -done with the optional ``nativeMode`` attribute on the ``<tag>`` subelement: -``nativeMode`` may be set to 'tagged' or 'untagged'. The ``id`` attribute of the -``<tag>`` subelement containing ``nativeMode`` sets which VLAN is considered to -be the "native" VLAN for this interface, and the ``nativeMode`` attribute -determines whether or not traffic for that VLAN will be tagged. +supported on Open vSwitch connections and standard linux bridges), multiple +``<tag>`` subelements can be specified, which implies that the user wants to do +VLAN trunking on the interface for all the specified tags. In the case that VLAN +trunking of a single tag is desired, the optional attribute ``trunk='yes'`` can +be added to the toplevel ``<vlan>`` element to differentiate trunking of a +single tag from normal tagging. + +For network connections using Open vSwitch and standard linux bridges it is also +possible to configure 'native-tagged' and 'native-untagged' VLAN modes +:since:`Since 1.1.0`. This is done with the optional ``nativeMode`` attribute on +the ``<tag>`` subelement: ``nativeMode`` may be set to 'tagged' or 'untagged'. +The ``id`` attribute of the ``<tag>`` subelement containing ``nativeMode`` sets +which VLAN is considered to be the "native" VLAN for this interface, and the +``nativeMode`` attribute determines whether or not traffic for that VLAN will be +tagged. Isolating guests' network traffic from each other diff --git a/docs/formatnetwork.rst b/docs/formatnetwork.rst index 9b4ecbf31d..053fe6ad56 100644 --- a/docs/formatnetwork.rst +++ b/docs/formatnetwork.rst @@ -520,28 +520,29 @@ VLAN tags to apply to the guest's network traffic :since:`Since 0.10.0`. Network connections that support guest-transparent VLAN tagging include ``type='bridge'`` interfaces connected to an Open vSwitch bridge, SRIOV -Virtual Functions (VF) used via ``type='hostdev'`` (direct device assignment) -and, :since:`since 1.3.5`, SRIOV VFs used via ``type='direct'`` with -``mode='passthrough'`` (macvtap "passthru" mode). All other -connection types, including standard linux bridges and libvirt's own virtual -networks, **do not** support it. 802.1Qbh (vn-link) and 802.1Qbg (VEPA) switches -provide their own way (outside of libvirt) to tag guest traffic onto a specific -VLAN. Each tag is given in a separate ``<tag>`` subelement of ``<vlan>`` (for -example: ``<tag id='42'/>``). For VLAN trunking of multiple tags (which is -supported only on Open vSwitch connections), multiple ``<tag>`` subelements can -be specified, which implies that the user wants to do VLAN trunking on the -interface for all the specified tags. In the case that VLAN trunking of a single -tag is desired, the optional attribute ``trunk='yes'`` can be added to the -toplevel ``<vlan>`` element to differentiate trunking of a single tag from -normal tagging. - -For network connections using Open vSwitch it is also possible to configure -'native-tagged' and 'native-untagged' VLAN modes :since:`Since 1.1.0`. This is -done with the optional ``nativeMode`` attribute on the ``<tag>`` subelement: -``nativeMode`` may be set to 'tagged' or 'untagged'. The ``id`` attribute of the -``<tag>`` subelement containing ``nativeMode`` sets which VLAN is considered to -be the "native" VLAN for this interface, and the ``nativeMode`` attribute -determines whether or not traffic for that VLAN will be tagged. +Virtual Functions (VF) used via ``type='hostdev'`` (direct device assignment), +:since:`since 1.3.5`, SRIOV VFs used via ``type='direct'`` with +``mode='passthrough'`` (macvtap "passthru" mode) and, :since:`since 11.0.0`, +standard linux bridges. All other connection types, including libvirt's own +virtual networks, **do not** support it. 802.1Qbh (vn-link) and 802.1Qbg (VEPA) +switches provide their own way (outside of libvirt) to tag guest traffic onto a +specific VLAN. Each tag is given in a separate ``<tag>`` subelement of +``<vlan>`` (for example: ``<tag id='42'/>``). For VLAN trunking of multiple +tags (which is supported on Open vSwitch connections and standard linux +bridges), multiple ``<tag>`` subelements can be specified, which implies that +the user wants to do VLAN trunking on the interface for all the specified tags. +In the case that VLAN trunking of a single tag is desired, the optional +attribute ``trunk='yes'`` can be added to the toplevel ``<vlan>`` element to +differentiate trunking of a single tag from normal tagging. + +For network connections using Open vSwitch :since:`since 1.1.10` and standard +linux bridges :since:`since 11.0.0` it is also possible to configure +'native-tagged' and 'native-untagged' VLAN modes. This is done with the optional +``nativeMode`` attribute on the ``<tag>`` subelement: ``nativeMode`` may be set +to 'tagged' or 'untagged'. The ``id`` attribute of the ``<tag>`` subelement +containing ``nativeMode`` sets which VLAN is considered to be the "native" VLAN +for this interface, and the ``nativeMode`` attribute determines whether or not +traffic for that VLAN will be tagged. ``<vlan>`` elements can also be specified in a ``<portgroup>`` element, as well as directly in a domain's ``<interface>`` element. In the case that a vlan tag -- 2.39.5
On 1/2/25 4:29 AM, Leigh Brown wrote:
Update domain XML and network XML documentation to describe how standard linux bridges support the VLAN configuration.
Signed-off-by: Leigh Brown <leigh@solinno.co.uk>
Reviewed-by: Laine Stump <laine@redhat.com> We'll also need a separate patch to add a blurb to NEWS.rst, but that can be done during the week of freeze.
--- docs/formatdomain.rst | 37 +++++++++++++++++----------------- docs/formatnetwork.rst | 45 +++++++++++++++++++++--------------------- 2 files changed, 42 insertions(+), 40 deletions(-)
diff --git a/docs/formatdomain.rst b/docs/formatdomain.rst index 8d787ef59a..90b025508a 100644 --- a/docs/formatdomain.rst +++ b/docs/formatdomain.rst @@ -6039,28 +6039,29 @@ VLAN tags to apply to the guest's network traffic :since:`Since 0.10.0`.
Network connections that support guest-transparent VLAN tagging include ``type='bridge'`` interfaces connected to an Open vSwitch bridge, SRIOV -Virtual Functions (VF) used via ``type='hostdev'`` (direct device assignment) -and, :since:`since 1.3.5`, SRIOV VFs used via ``type='direct'`` with -``mode='passthrough'`` (macvtap "passthru" mode). All other -connection types, including standard linux bridges and libvirt's own virtual +Virtual Functions (VF) used via ``type='hostdev'`` (direct device assignment), +:since:`since 1.3.5`, SRIOV VFs used via ``type='direct'`` with +``mode='passthrough'`` (macvtap "passthru" mode) and, :since:`since 11.0.0` +standard linux bridges. Other connection types, including libvirt's own virtual networks, **do not** support it. 802.1Qbh (vn-link) and 802.1Qbg (VEPA) switches provide their own way (outside of libvirt) to tag guest traffic onto a specific VLAN. Each tag is given in a separate ``<tag>`` subelement of ``<vlan>`` (for example: ``<tag id='42'/>``). For VLAN trunking of multiple tags (which is -supported only on Open vSwitch connections), multiple ``<tag>`` subelements can -be specified, which implies that the user wants to do VLAN trunking on the -interface for all the specified tags. In the case that VLAN trunking of a single -tag is desired, the optional attribute ``trunk='yes'`` can be added to the -toplevel ``<vlan>`` element to differentiate trunking of a single tag from -normal tagging. - -For network connections using Open vSwitch it is also possible to configure -'native-tagged' and 'native-untagged' VLAN modes :since:`Since 1.1.0`. This is -done with the optional ``nativeMode`` attribute on the ``<tag>`` subelement: -``nativeMode`` may be set to 'tagged' or 'untagged'. The ``id`` attribute of the -``<tag>`` subelement containing ``nativeMode`` sets which VLAN is considered to -be the "native" VLAN for this interface, and the ``nativeMode`` attribute -determines whether or not traffic for that VLAN will be tagged. +supported on Open vSwitch connections and standard linux bridges), multiple +``<tag>`` subelements can be specified, which implies that the user wants to do +VLAN trunking on the interface for all the specified tags. In the case that VLAN +trunking of a single tag is desired, the optional attribute ``trunk='yes'`` can +be added to the toplevel ``<vlan>`` element to differentiate trunking of a +single tag from normal tagging. + +For network connections using Open vSwitch and standard linux bridges it is also +possible to configure 'native-tagged' and 'native-untagged' VLAN modes +:since:`Since 1.1.0`. This is done with the optional ``nativeMode`` attribute on +the ``<tag>`` subelement: ``nativeMode`` may be set to 'tagged' or 'untagged'. +The ``id`` attribute of the ``<tag>`` subelement containing ``nativeMode`` sets +which VLAN is considered to be the "native" VLAN for this interface, and the +``nativeMode`` attribute determines whether or not traffic for that VLAN will be +tagged.
Isolating guests' network traffic from each other diff --git a/docs/formatnetwork.rst b/docs/formatnetwork.rst index 9b4ecbf31d..053fe6ad56 100644 --- a/docs/formatnetwork.rst +++ b/docs/formatnetwork.rst @@ -520,28 +520,29 @@ VLAN tags to apply to the guest's network traffic :since:`Since 0.10.0`.
Network connections that support guest-transparent VLAN tagging include ``type='bridge'`` interfaces connected to an Open vSwitch bridge, SRIOV -Virtual Functions (VF) used via ``type='hostdev'`` (direct device assignment) -and, :since:`since 1.3.5`, SRIOV VFs used via ``type='direct'`` with -``mode='passthrough'`` (macvtap "passthru" mode). All other -connection types, including standard linux bridges and libvirt's own virtual -networks, **do not** support it. 802.1Qbh (vn-link) and 802.1Qbg (VEPA) switches -provide their own way (outside of libvirt) to tag guest traffic onto a specific -VLAN. Each tag is given in a separate ``<tag>`` subelement of ``<vlan>`` (for -example: ``<tag id='42'/>``). For VLAN trunking of multiple tags (which is -supported only on Open vSwitch connections), multiple ``<tag>`` subelements can -be specified, which implies that the user wants to do VLAN trunking on the -interface for all the specified tags. In the case that VLAN trunking of a single -tag is desired, the optional attribute ``trunk='yes'`` can be added to the -toplevel ``<vlan>`` element to differentiate trunking of a single tag from -normal tagging. - -For network connections using Open vSwitch it is also possible to configure -'native-tagged' and 'native-untagged' VLAN modes :since:`Since 1.1.0`. This is -done with the optional ``nativeMode`` attribute on the ``<tag>`` subelement: -``nativeMode`` may be set to 'tagged' or 'untagged'. The ``id`` attribute of the -``<tag>`` subelement containing ``nativeMode`` sets which VLAN is considered to -be the "native" VLAN for this interface, and the ``nativeMode`` attribute -determines whether or not traffic for that VLAN will be tagged. +Virtual Functions (VF) used via ``type='hostdev'`` (direct device assignment), +:since:`since 1.3.5`, SRIOV VFs used via ``type='direct'`` with +``mode='passthrough'`` (macvtap "passthru" mode) and, :since:`since 11.0.0`, +standard linux bridges. All other connection types, including libvirt's own +virtual networks, **do not** support it. 802.1Qbh (vn-link) and 802.1Qbg (VEPA) +switches provide their own way (outside of libvirt) to tag guest traffic onto a +specific VLAN. Each tag is given in a separate ``<tag>`` subelement of +``<vlan>`` (for example: ``<tag id='42'/>``). For VLAN trunking of multiple +tags (which is supported on Open vSwitch connections and standard linux +bridges), multiple ``<tag>`` subelements can be specified, which implies that +the user wants to do VLAN trunking on the interface for all the specified tags. +In the case that VLAN trunking of a single tag is desired, the optional +attribute ``trunk='yes'`` can be added to the toplevel ``<vlan>`` element to +differentiate trunking of a single tag from normal tagging. + +For network connections using Open vSwitch :since:`since 1.1.10` and standard +linux bridges :since:`since 11.0.0` it is also possible to configure +'native-tagged' and 'native-untagged' VLAN modes. This is done with the optional +``nativeMode`` attribute on the ``<tag>`` subelement: ``nativeMode`` may be set +to 'tagged' or 'untagged'. The ``id`` attribute of the ``<tag>`` subelement +containing ``nativeMode`` sets which VLAN is considered to be the "native" VLAN +for this interface, and the ``nativeMode`` attribute determines whether or not +traffic for that VLAN will be tagged.
``<vlan>`` elements can also be specified in a ``<portgroup>`` element, as well as directly in a domain's ``<interface>`` element. In the case that a vlan tag
On 1/2/25 4:29 AM, Leigh Brown wrote:
As requested by Laine, I have converted the code to use netlink rather than executing bridge vlan commands. I have also checked it compiles under FreeBSD.
Just a few minor formatting nits that I've pointed out, but otherwise this looks good! If you want to respin between now and this evening I can go over it again and push it tonight, or if you don't have the time just let me know today and I can take care of them before pushing. We will also need a separate patch to add it to NEWS.rst, as well as a change to qemuDomainChangeNet() (because it assumes anything with a vlan is an OVS bridge, and when the vlan config changes it will try to call an OVS-specific function (which will obviously fail). But in the case of a standard bridge we can just re-attach the tap device to the bridge and the vlan settings will automatically be updated. That patch can be safely pushed during freeze though, and I'll write it, since that function is a bit messy, and it's already clear in my mind what needs to be done. Thanks for the contribution! (and sorry for not looking at it during the last two weeks - I was completely disconnected from all work-related email).
Description ----------- The iproute2 bridge command supports the capability for VLAN filtering that allows each interface connected to a standard linux bridge to be configured to use one or more VLANs. For simple setups, this capability is enough to allow virtual machines or containers to be put onto separate VLANs without creating multiple bridges and VLANs on the host.
The first patch adds a new function virNetDevBridgeSetupVlans() that will, given a virNetDevVlan structure, execute the required bridge vlan commands to configure the given interface accordingly.
The second patch updates the virNetDevBridgeAddPort() function to allow a virNetDevVlan parameter to be passed, and to call the virNetDevBridgeSetupVlans() function.
The third patch updates the lxc and tap code to pass the virNetDevLan parameter from the configuration and to update the XML domain and network validation to permit the VLAN-related tags for standard bridges.
The fourth patch updates documentation to match the new capability.
Changes since v2 ---------------- - Convert to use netlink rather than executing bridge vlan commands. - Add unsupported on this platform error message on FreeBSD.
Changes since v1 ---------------- - Fix bug in virNetDevSetupVlans where bridge port has no native vlan. - Update bridge network validation to permit vlan configuration. - Update documentation to match the functionality. - Tweak some of the commit descriptions for clarity.
Usage example ------------- Configure the host with systemd-networkd as follows:
/etc/systemd/network/br0.netdev (br0.network not shown)
[NetDev] Name=br0 Kind=bridge MACAddress=xx:xx:xx:xx:xx:xx [Bridge] VLANFiltering=on
/etc/systemd/network/eno1.network
[Match] Name=eno1 [Network] Bridge=br0 [Link] MTUBytes=9000 [BridgeVLAN] VLAN=40 [BridgeVLAN] VLAN=60
Then add <vlan> tags into the lxc or qemu config:
lxc interface definition: <interface type='bridge'> <mac address='xx:xx:xx:xx:xx:xx'/> <source bridge='br0'/> <vlan> <tag id='40'/> </vlan> </interface>
qemu interface definition: <interface type='network'> <mac address='xx:xx:xx:xx:xx:xx'/> <source network='br0'/> <vlan> <tag id='60'/> </vlan> <model type='virtio'/> <address type='pci' domain='0x0000' bus='0x01' slot='0x00' function='0x0'/> </interface>
Then, after starting them, you will see the following
$ sudo bridge vlan port vlan-id eno1 1 PVID Egress Untagged 40 60 br0 1 PVID Egress Untagged vnet0 60 PVID Egress Untagged vnet1 40 PVID Egress Untagged
Regards,
Leigh Brown (4): util: add netlink bridge vlan filtering util: Add vlan support to virNetDevBridgeAddPort Enable vlan support for standard linux bridges docs: standard linux bridges now support vlans
docs/formatdomain.rst | 37 +++++++++--------- docs/formatnetwork.rst | 45 +++++++++++----------- src/conf/domain_validate.c | 3 +- src/lxc/lxc_process.c | 3 +- src/network/bridge_driver.c | 13 ++++--- src/util/virnetdevbridge.c | 75 +++++++++++++++++++++++++++++++++++-- src/util/virnetdevbridge.h | 4 +- src/util/virnetdevtap.c | 2 +- src/util/virnetlink.c | 66 ++++++++++++++++++++++++++++++++ src/util/virnetlink.h | 7 ++++ 10 files changed, 202 insertions(+), 53 deletions(-)
participants (3)
-
Laine Stump -
Laine Stump -
Leigh Brown