[libvirt] [PATCH] esx: Support VLAN tags in virtual network port groups

--- src/esx/esx_network_driver.c | 38 +++++++++++++++++++++++++++++++++----- 1 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 09d46d3..2f5f1ab 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -489,7 +489,16 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) goto cleanup; } - hostPortGroupSpec->vlanId->value = 0; + if (def->portGroups[i].vlan.trunk) { + hostPortGroupSpec->vlanId->value = 4095; + } else if (def->portGroups[i].vlan.nTags == 1) { + hostPortGroupSpec->vlanId->value = *def->portGroups[i].vlan.tag; + } else if (def->portGroups[i].vlan.nTags > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Can apply one VLAN tag per port group only")); + } else { + hostPortGroupSpec->vlanId->value = 0; + } if (def->portGroups[i].bandwidth != NULL) { if (esxBandwidthToShapingPolicy @@ -695,6 +704,7 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) esxVI_String *hostPortGroupKey = NULL; esxVI_String *networkName = NULL; virNetworkDefPtr def; + virPortGroupDefPtr portGroup = NULL; if (esxVI_EnsureSession(priv->primary) < 0) { return NULL; @@ -824,9 +834,12 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) for (networkName = networkNameList; networkName != NULL; networkName = networkName->_next) { if (STREQ(networkName->value, hostPortGroup->spec->name)) { - def->portGroups[def->nPortGroups].name = strdup(networkName->value); + portGroup = &def->portGroups[def->nPortGroups]; + ++def->nPortGroups; + + portGroup->name = strdup(networkName->value); - if (def->portGroups[def->nPortGroups].name == NULL) { + if (portGroup->name == NULL) { virReportOOMError(); goto cleanup; } @@ -834,13 +847,28 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) if (hostPortGroup->spec->policy != NULL) { if (esxShapingPolicyToBandwidth (hostPortGroup->spec->policy->shapingPolicy, - &def->portGroups[def->nPortGroups].bandwidth) < 0) { + &portGroup->bandwidth) < 0) { ++def->nPortGroups; goto cleanup; } } - ++def->nPortGroups; + if (hostPortGroup->spec->vlanId->value > 0) { + if (hostPortGroup->spec->vlanId->value == 4095) { + portGroup->vlan.trunk = true; + } + + portGroup->vlan.nTags = 1; + + if (VIR_ALLOC_N(portGroup->vlan.tag, 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + *portGroup->vlan.tag = + hostPortGroup->spec->vlanId->value; + } + break; } } -- 1.7.4.1

On 08/18/2012 07:46 AM, Matthias Bolte wrote:
--- src/esx/esx_network_driver.c | 38 +++++++++++++++++++++++++++++++++----- 1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 09d46d3..2f5f1ab 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -489,7 +489,16 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) goto cleanup; }
- hostPortGroupSpec->vlanId->value = 0; + if (def->portGroups[i].vlan.trunk) { + hostPortGroupSpec->vlanId->value = 4095; + } else if (def->portGroups[i].vlan.nTags == 1) { + hostPortGroupSpec->vlanId->value = *def->portGroups[i].vlan.tag; + } else if (def->portGroups[i].vlan.nTags > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Can apply one VLAN tag per port group only")); + } else { + hostPortGroupSpec->vlanId->value = 0; + }
So in vmware you can specify that a port is to be used for trunking, (and you do that by specifying a tag of 4095), but can't limit the list of tags to allow - is that correct? The parser (and the RNG) currently requires at least one <tag> element in any <vlan> element. It sounds like we need to change that. Kyle or Ansis - does it make sense for Open vSwitch to have a port specified with trunk=yes and no list of specific tags? Aside from this, you are interpreting the <vlan> element in portgroups, but not the overlying <vlan> for the entire network (in practice this is what would be used if no portgroup was chosen, and no vlan info came from the domain's interface definition). If there isn't any practical way to implement this, you should check for it and log a CONFIG_UNSUPPORTED error if found.
if (def->portGroups[i].bandwidth != NULL) { if (esxBandwidthToShapingPolicy @@ -695,6 +704,7 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) esxVI_String *hostPortGroupKey = NULL; esxVI_String *networkName = NULL; virNetworkDefPtr def; + virPortGroupDefPtr portGroup = NULL;
if (esxVI_EnsureSession(priv->primary) < 0) { return NULL; @@ -824,9 +834,12 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) for (networkName = networkNameList; networkName != NULL; networkName = networkName->_next) { if (STREQ(networkName->value, hostPortGroup->spec->name)) { - def->portGroups[def->nPortGroups].name = strdup(networkName->value); + portGroup = &def->portGroups[def->nPortGroups]; + ++def->nPortGroups; + + portGroup->name = strdup(networkName->value);
- if (def->portGroups[def->nPortGroups].name == NULL) { + if (portGroup->name == NULL) { virReportOOMError(); goto cleanup; } @@ -834,13 +847,28 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) if (hostPortGroup->spec->policy != NULL) { if (esxShapingPolicyToBandwidth (hostPortGroup->spec->policy->shapingPolicy, - &def->portGroups[def->nPortGroups].bandwidth) < 0) { + &portGroup->bandwidth) < 0) { ++def->nPortGroups; goto cleanup; } }
- ++def->nPortGroups; + if (hostPortGroup->spec->vlanId->value > 0) { + if (hostPortGroup->spec->vlanId->value == 4095) { + portGroup->vlan.trunk = true; + } + + portGroup->vlan.nTags = 1; + + if (VIR_ALLOC_N(portGroup->vlan.tag, 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + *portGroup->vlan.tag = + hostPortGroup->spec->vlanId->value; + } + break; } }

On Sun, Aug 19, 2012 at 8:35 PM, Laine Stump <laine@laine.org> wrote:
On 08/18/2012 07:46 AM, Matthias Bolte wrote:
--- src/esx/esx_network_driver.c | 38 +++++++++++++++++++++++++++++++++----- 1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 09d46d3..2f5f1ab 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -489,7 +489,16 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) goto cleanup; }
- hostPortGroupSpec->vlanId->value = 0; + if (def->portGroups[i].vlan.trunk) { + hostPortGroupSpec->vlanId->value = 4095; + } else if (def->portGroups[i].vlan.nTags == 1) { + hostPortGroupSpec->vlanId->value = *def->portGroups[i].vlan.tag; + } else if (def->portGroups[i].vlan.nTags > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Can apply one VLAN tag per port group only")); + } else { + hostPortGroupSpec->vlanId->value = 0; + }
So in vmware you can specify that a port is to be used for trunking, (and you do that by specifying a tag of 4095), but can't limit the list of tags to allow - is that correct?
The parser (and the RNG) currently requires at least one <tag> element in any <vlan> element. It sounds like we need to change that.
Kyle or Ansis - does it make sense for Open vSwitch to have a port specified with trunk=yes and no list of specific tags?
If port type is not explicitly specified as access port (a.k.a. tagged-port), then Open vSwitch will assume that this is a trunk port. Also, if trunk list is empty, then Open vSwitch would interpret that as "trunk all VLANs". So, I guess, the answer would be "yes", if you want following configuration: ... <vlan trunk='yes'> </vlan> ... to become alias for "no vlan configuration specified at all" case?
Aside from this, you are interpreting the <vlan> element in portgroups, but not the overlying <vlan> for the entire network (in practice this is what would be used if no portgroup was chosen, and no vlan info came from the domain's interface definition). If there isn't any practical way to implement this, you should check for it and log a CONFIG_UNSUPPORTED error if found.
if (def->portGroups[i].bandwidth != NULL) { if (esxBandwidthToShapingPolicy @@ -695,6 +704,7 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) esxVI_String *hostPortGroupKey = NULL; esxVI_String *networkName = NULL; virNetworkDefPtr def; + virPortGroupDefPtr portGroup = NULL;
if (esxVI_EnsureSession(priv->primary) < 0) { return NULL; @@ -824,9 +834,12 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) for (networkName = networkNameList; networkName != NULL; networkName = networkName->_next) { if (STREQ(networkName->value, hostPortGroup->spec->name)) { - def->portGroups[def->nPortGroups].name = strdup(networkName->value); + portGroup = &def->portGroups[def->nPortGroups]; + ++def->nPortGroups; + + portGroup->name = strdup(networkName->value);
- if (def->portGroups[def->nPortGroups].name == NULL) { + if (portGroup->name == NULL) { virReportOOMError(); goto cleanup; } @@ -834,13 +847,28 @@ esxNetworkGetXMLDesc(virNetworkPtr network_, unsigned int flags) if (hostPortGroup->spec->policy != NULL) { if (esxShapingPolicyToBandwidth (hostPortGroup->spec->policy->shapingPolicy, - &def->portGroups[def->nPortGroups].bandwidth) < 0) { + &portGroup->bandwidth) < 0) { ++def->nPortGroups; goto cleanup; } }
- ++def->nPortGroups; + if (hostPortGroup->spec->vlanId->value > 0) { + if (hostPortGroup->spec->vlanId->value == 4095) { + portGroup->vlan.trunk = true; + } + + portGroup->vlan.nTags = 1; + + if (VIR_ALLOC_N(portGroup->vlan.tag, 1) < 0) { + virReportOOMError(); + goto cleanup; + } + + *portGroup->vlan.tag = + hostPortGroup->spec->vlanId->value; + } + break; } }

2012/8/20 Laine Stump <laine@laine.org>:
On 08/18/2012 07:46 AM, Matthias Bolte wrote:
--- src/esx/esx_network_driver.c | 38 +++++++++++++++++++++++++++++++++----- 1 files changed, 33 insertions(+), 5 deletions(-)
diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c index 09d46d3..2f5f1ab 100644 --- a/src/esx/esx_network_driver.c +++ b/src/esx/esx_network_driver.c @@ -489,7 +489,16 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml) goto cleanup; }
- hostPortGroupSpec->vlanId->value = 0; + if (def->portGroups[i].vlan.trunk) { + hostPortGroupSpec->vlanId->value = 4095; + } else if (def->portGroups[i].vlan.nTags == 1) { + hostPortGroupSpec->vlanId->value = *def->portGroups[i].vlan.tag; + } else if (def->portGroups[i].vlan.nTags > 1) { + virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s", + _("Can apply one VLAN tag per port group only")); + } else { + hostPortGroupSpec->vlanId->value = 0; + }
So in vmware you can specify that a port is to be used for trunking, (and you do that by specifying a tag of 4095), but can't limit the list of tags to allow - is that correct?
Yes.
Aside from this, you are interpreting the <vlan> element in portgroups, but not the overlying <vlan> for the entire network (in practice this is what would be used if no portgroup was chosen, and no vlan info came from the domain's interface definition). If there isn't any practical way to implement this, you should check for it and log a CONFIG_UNSUPPORTED error if found.
There is no way to not choose a portgroup in VMware, but I could use the following logic. If a portgroup has no <vlan> element but the network has one than use the network's <vlan> for the portgroup. -- Matthias Bolte http://photron.blogspot.com
participants (3)
-
Ansis Atteka
-
Laine Stump
-
Matthias Bolte