[libvirt] [PATCH 2/3] support new forward mode of vlan for virtual network
John Ferlan
jferlan at redhat.com
Tue Jul 17 22:38:23 UTC 2018
On 07/05/2018 11:36 PM, Shi Lei wrote:
> Signed-off-by: Shi Lei <shilei.massclouds at gmx.com>
> ---
> src/conf/network_conf.c | 12 ++++---
> src/conf/network_conf.h | 1 +
> src/network/bridge_driver.c | 80 +++++++++++++++++++++++++++++++++++++++++----
> 3 files changed, 82 insertions(+), 11 deletions(-)
>
This would be missing RNG changes in docs/schemas/network.rng,
documentation changes in docs/formatnetwork.html.in in order to describe
things, and networkxml2* tests. Usually if you look at the last person
to add a new element to something it'll give you a good hint. In this
case, see commit id 25e8112d7 when "open" was added. I use gitk to chase
changes, but others use straight git - whatever works for you.
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index 630a87f..6e1de6c 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -50,7 +50,7 @@ VIR_ENUM_IMPL(virNetworkForward,
> VIR_NETWORK_FORWARD_LAST,
> "none", "nat", "route", "open",
> "bridge", "private", "vepa", "passthrough",
> - "hostdev")
> + "hostdev", "vlan")
>
> VIR_ENUM_IMPL(virNetworkBridgeMACTableManager,
> VIR_NETWORK_BRIDGE_MAC_TABLE_MANAGER_LAST,
> @@ -1876,6 +1876,7 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> */
> switch (def->forward.type) {
> case VIR_NETWORK_FORWARD_NONE:
> + case VIR_NETWORK_FORWARD_VLAN:
> break;
>
> case VIR_NETWORK_FORWARD_ROUTE:
> @@ -1963,9 +1964,10 @@ virNetworkDefParseXML(xmlXPathContextPtr ctxt)
> (def->forward.type != VIR_NETWORK_FORWARD_NONE &&
> def->forward.type != VIR_NETWORK_FORWARD_NAT &&
> def->forward.type != VIR_NETWORK_FORWARD_ROUTE &&
> - def->forward.type != VIR_NETWORK_FORWARD_OPEN)) {
> + def->forward.type != VIR_NETWORK_FORWARD_OPEN &&
> + def->forward.type != VIR_NETWORK_FORWARD_VLAN)) {
> virReportError(VIR_ERR_XML_ERROR,
> - _("mtu size only allowed in open, route, nat, "
> + _("mtu size only allowed in open, route, nat, vlan "
> "and isolated mode, not in %s (network '%s')"),
> virNetworkForwardTypeToString(def->forward.type),
> def->name);
> @@ -2474,6 +2476,7 @@ virNetworkDefFormatBuf(virBufferPtr buf,
> def->forward.type == VIR_NETWORK_FORWARD_NAT ||
> def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
> def->forward.type == VIR_NETWORK_FORWARD_OPEN ||
> + def->forward.type == VIR_NETWORK_FORWARD_VLAN ||
> def->bridge || def->macTableManager) {
>
> virBufferAddLit(buf, "<bridge");
> @@ -2481,7 +2484,8 @@ virNetworkDefFormatBuf(virBufferPtr buf,
> if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
> def->forward.type == VIR_NETWORK_FORWARD_NAT ||
> def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
> - def->forward.type == VIR_NETWORK_FORWARD_OPEN) {
> + def->forward.type == VIR_NETWORK_FORWARD_OPEN ||
> + def->forward.type == VIR_NETWORK_FORWARD_VLAN) {
> virBufferAsprintf(buf, " stp='%s' delay='%ld'",
> def->stp ? "on" : "off", def->delay);
> }
> diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h
> index 54c8ed1..47bb83e 100644
> --- a/src/conf/network_conf.h
> +++ b/src/conf/network_conf.h
> @@ -53,6 +53,7 @@ typedef enum {
> VIR_NETWORK_FORWARD_VEPA,
> VIR_NETWORK_FORWARD_PASSTHROUGH,
> VIR_NETWORK_FORWARD_HOSTDEV,
> + VIR_NETWORK_FORWARD_VLAN,
>
> VIR_NETWORK_FORWARD_LAST,
> } virNetworkForwardType;
> diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
> index da3c32e..314f78c 100644
> --- a/src/network/bridge_driver.c
> +++ b/src/network/bridge_driver.c
> @@ -451,6 +451,7 @@ networkUpdateState(virNetworkObjPtr obj,
> case VIR_NETWORK_FORWARD_NAT:
> case VIR_NETWORK_FORWARD_ROUTE:
> case VIR_NETWORK_FORWARD_OPEN:
> + case VIR_NETWORK_FORWARD_VLAN:
> /* If bridge doesn't exist, then mark it inactive */
> if (!(def->bridge && virNetDevExists(def->bridge) == 1))
> virNetworkObjSetActive(obj, false);
> @@ -2092,7 +2093,8 @@ networkRefreshDaemonsHelper(virNetworkObjPtr obj,
> ((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
> (def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
> (def->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
> - (def->forward.type == VIR_NETWORK_FORWARD_OPEN))) {
> + (def->forward.type == VIR_NETWORK_FORWARD_OPEN) ||
> + (def->forward.type == VIR_NETWORK_FORWARD_VLAN))) {
> /* Only the three L3 network types that are configured by
> * libvirt will have a dnsmasq or radvd daemon associated
> * with them. Here we send a SIGHUP to an existing
> @@ -2131,7 +2133,8 @@ networkReloadFirewallRulesHelper(virNetworkObjPtr obj,
> if (virNetworkObjIsActive(obj) &&
> ((def->forward.type == VIR_NETWORK_FORWARD_NONE) ||
> (def->forward.type == VIR_NETWORK_FORWARD_NAT) ||
> - (def->forward.type == VIR_NETWORK_FORWARD_ROUTE))) {
> + (def->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
> + (def->forward.type == VIR_NETWORK_FORWARD_VLAN))) {
> /* Only three of the L3 network types that are configured by
> * libvirt need to have iptables rules reloaded. The 4th L3
> * network type, forward='open', doesn't need this because it
> @@ -2513,6 +2516,27 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver,
> if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0)
> goto err5;
>
> + if (def->forward.type == VIR_NETWORK_FORWARD_VLAN) {
> + char vlanDevName[IFNAMSIZ];
char *vlanDevName = NULL; (needs to be at the top)
> + /* ifs[0].device.dev and vlan.tag[0] have been validated in networkValidate() */
> + if (virNetDevCreateVLanDev(def->forward.ifs[0].device.dev, def->vlan.tag[0],
> + vlanDevName, sizeof(vlanDevName)) < 0)
Make sure the vlanDevName aligns under the def->forward...
> + goto err5;
err5 is for bandwidth failure, maybe there needs to be a err6 for VLAN
and within that there will need to be a VIR_FREE(vlanDevName);
> +
> + if (virNetDevBridgeAddPort(def->bridge, vlanDevName) < 0) {
> + ignore_value(virNetDevDestroyVLanDev(def->forward.ifs[0].device.dev,
> + def->vlan.tag[0]));
> + goto err5;
> + }
> +
> + if (virNetDevSetOnline(vlanDevName, true) < 0) {
> + ignore_value(virNetDevBridgeRemovePort(def->bridge, vlanDevName));
> + ignore_value(virNetDevDestroyVLanDev(def->forward.ifs[0].device.dev,
> + def->vlan.tag[0]));
BTW: If all callers to virNetDevDestroyVLanDev are just going to be
wrapped in ignore_value(), then let's just make it a void and be done
with it.
> + goto err5;
> + }
VIR_FREE(vlanDevName);
> + }
> +
> VIR_FREE(macTapIfName);
> VIR_FREE(macMapFile);
>
> @@ -2576,6 +2600,18 @@ networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver,
> pid_t radvdPid;
> pid_t dnsmasqPid;
>
> + if (def->forward.type == VIR_NETWORK_FORWARD_VLAN) {
> + char vlanDevName[IFNAMSIZ];
char *vlanDevName = NULL;
> + ignore_value(virNetDevGetVLanDevName(def->forward.ifs[0].device.dev,
> + def->vlan.tag[0],
> + vlanDevName, sizeof(vlanDevName)));
don't think this can be ignored since it's used as input for other
functions that follow that may not expect a NULL vlanDevName
> +
> + ignore_value(virNetDevSetOnline(vlanDevName, false));
> + ignore_value(virNetDevBridgeRemovePort(def->bridge, vlanDevName));
> + ignore_value(virNetDevDestroyVLanDev(def->forward.ifs[0].device.dev,
I'm now having more thoughts related to whether virNetDevDestroyVLanDev
should take an ifname. Above you do the virNetDevGetVLanDevName call
and then again when calling virNetDevDestroyVLanDev, so why not just
have it take the vdname instead.
> + def->vlan.tag[0]));
VIR_FREE(vlanDevName);
John
> + }
> +
> if (def->bandwidth)
> virNetDevBandwidthClear(def->bridge);
>
> @@ -2719,6 +2755,7 @@ networkCreateInterfacePool(virNetworkDefPtr netdef)
> case VIR_NETWORK_FORWARD_NAT:
> case VIR_NETWORK_FORWARD_ROUTE:
> case VIR_NETWORK_FORWARD_OPEN:
> + case VIR_NETWORK_FORWARD_VLAN:
> case VIR_NETWORK_FORWARD_LAST:
> /* by definition these will never be encountered here */
> break;
> @@ -2817,6 +2854,7 @@ networkStartNetwork(virNetworkDriverStatePtr driver,
> case VIR_NETWORK_FORWARD_NAT:
> case VIR_NETWORK_FORWARD_ROUTE:
> case VIR_NETWORK_FORWARD_OPEN:
> + case VIR_NETWORK_FORWARD_VLAN:
> if (networkStartNetworkVirtual(driver, obj) < 0)
> goto cleanup;
> break;
> @@ -2899,6 +2937,7 @@ networkShutdownNetwork(virNetworkDriverStatePtr driver,
> case VIR_NETWORK_FORWARD_NAT:
> case VIR_NETWORK_FORWARD_ROUTE:
> case VIR_NETWORK_FORWARD_OPEN:
> + case VIR_NETWORK_FORWARD_VLAN:
> ret = networkShutdownNetworkVirtual(driver, obj);
> break;
>
> @@ -3276,7 +3315,8 @@ networkValidate(virNetworkDriverStatePtr driver,
> if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
> def->forward.type == VIR_NETWORK_FORWARD_NAT ||
> def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
> - def->forward.type == VIR_NETWORK_FORWARD_OPEN) {
> + def->forward.type == VIR_NETWORK_FORWARD_OPEN ||
> + def->forward.type == VIR_NETWORK_FORWARD_VLAN) {
>
> /* if no bridge name was given in the config, find a name
> * unused by any other libvirt networks and assign it.
> @@ -3447,7 +3487,8 @@ networkValidate(virNetworkDriverStatePtr driver,
> * a pool, and those using an Open vSwitch bridge.
> */
>
> - vlanAllowed = (def->forward.type == VIR_NETWORK_FORWARD_HOSTDEV ||
> + vlanAllowed = (def->forward.type == VIR_NETWORK_FORWARD_VLAN ||
> + def->forward.type == VIR_NETWORK_FORWARD_HOSTDEV ||
> def->forward.type == VIR_NETWORK_FORWARD_PASSTHROUGH ||
> (def->forward.type == VIR_NETWORK_FORWARD_BRIDGE &&
> def->virtPortProfile &&
> @@ -3530,6 +3571,28 @@ networkValidate(virNetworkDriverStatePtr driver,
> }
> }
> }
> +
> + if (def->forward.type == VIR_NETWORK_FORWARD_VLAN) {
> + if (virNetDevLoad8021Q() < 0)
> + return -1;
> +
> + if (def->forward.nifs != 1 ||
> + strlen(def->forward.ifs[0].device.dev) == 0) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("invalid dev in <forward mode='vlan' dev='xxx'> "
> + "of network '%s' with forward mode='%s'"),
> + def->name, virNetworkForwardTypeToString(def->forward.type));
> + return -1;
> + }
> + if (def->vlan.nTags != 1 || def->vlan.tag[0] >= 4096) {
> + virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> + _("unsupported vlan config. check <vlan><tag id='xxx'> "
> + "of network '%s' with forward mode='%s'"),
> + def->name, virNetworkForwardTypeToString(def->forward.type));
> + return -1;
> + }
> + }
> +
> return 0;
> }
>
> @@ -3757,7 +3820,8 @@ networkUpdate(virNetworkPtr net,
> */
> if (def->forward.type == VIR_NETWORK_FORWARD_NONE ||
> def->forward.type == VIR_NETWORK_FORWARD_NAT ||
> - def->forward.type == VIR_NETWORK_FORWARD_ROUTE) {
> + def->forward.type == VIR_NETWORK_FORWARD_ROUTE ||
> + def->forward.type == VIR_NETWORK_FORWARD_VLAN) {
> switch (section) {
> case VIR_NETWORK_SECTION_FORWARD:
> case VIR_NETWORK_SECTION_FORWARD_INTERFACE:
> @@ -4443,7 +4507,8 @@ networkAllocateActualDevice(virDomainDefPtr dom,
> if ((netdef->forward.type == VIR_NETWORK_FORWARD_NONE) ||
> (netdef->forward.type == VIR_NETWORK_FORWARD_NAT) ||
> (netdef->forward.type == VIR_NETWORK_FORWARD_ROUTE) ||
> - (netdef->forward.type == VIR_NETWORK_FORWARD_OPEN)) {
> + (netdef->forward.type == VIR_NETWORK_FORWARD_OPEN) ||
> + (netdef->forward.type == VIR_NETWORK_FORWARD_VLAN)) {
> /* for these forward types, the actual net type really *is*
> * NETWORK; we just keep the info from the portgroup in
> * iface->data.network.actual
> @@ -4701,7 +4766,8 @@ networkAllocateActualDevice(virDomainDefPtr dom,
> * mode) and openvswitch bridges. Otherwise log an error and
> * fail
> */
> - if (!(actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV ||
> + if (!(netdef->forward.type == VIR_NETWORK_FORWARD_VLAN ||
> + actualType == VIR_DOMAIN_NET_TYPE_HOSTDEV ||
> (actualType == VIR_DOMAIN_NET_TYPE_DIRECT &&
> virDomainNetGetActualDirectMode(iface)
> == VIR_NETDEV_MACVLAN_MODE_PASSTHRU) ||
>
More information about the libvir-list
mailing list