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

[Matthias Bolte]

2012/9/30 Laine Stump <laine at laine.org>:
> On 09/09/2012 04:44 AM, Matthias Bolte wrote:
>> ---
>>
>> v2: Use network level VLAN config if there is no portgroup specific VLAN
>>     config given.
>>
>> v3: Add ESX_VLAN_TRUNK define for magic number 4095
>>
>>  src/esx/esx_network_driver.c |   70 +++++++++++++++++++++++++++++++++++++++---
>>  1 files changed, 65 insertions(+), 5 deletions(-)
>>
>> diff --git a/src/esx/esx_network_driver.c b/src/esx/esx_network_driver.c
>> index 09d46d3..21eabbe 100644
>> --- a/src/esx/esx_network_driver.c
>> +++ b/src/esx/esx_network_driver.c
>> @@ -45,6 +45,9 @@
>>   */
>>  verify(MD5_DIGEST_SIZE == VIR_UUID_BUFLEN);
>>
>> +/* ESX utilizes the VLAN ID of 4095 to mean that this port is in trunk mode */
>> +#define ESX_VLAN_TRUNK 4095
>> +
>>
>>
>>  static virDrvOpenStatus
>> @@ -489,7 +492,42 @@ esxNetworkDefineXML(virConnectPtr conn, const char *xml)
>>              goto cleanup;
>>          }
>>
>> -        hostPortGroupSpec->vlanId->value = 0;
>> +        if (def->portGroups[i].vlan.trunk) {
>> +            /* FIXME: Change this once tag-less trunk-mode is supported */
>
> I've lost the context (if there was any) of this comment. Is this
> something that libvirt needs to support? Or ESX?

libvirt.

>> +            if (def->portGroups[i].vlan.nTags != 1 ||
>> +                *def->portGroups[i].vlan.tag != ESX_VLAN_TRUNK) {
>> +                virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
>> +                               _("VLAN tag has to be %d in trunk mode"),
>> +                               ESX_VLAN_TRUNK);
>
> This seems a bit odd. If ESX requires  setting the vlan tag to 4095 to
> indicate trunk mode, why not just have trunk='yes' imply a tag id of
> 4095? While not exactly the same as trunk mode for Open vSwitch, it
> seems closer at least. (hmm, of course I think currently the parser
> requires at least one tag id, even for trunk='yes'. Maybe that should be
> relaxed in the parser, and enforced in each driver as necessary? Is that
> what the comment above is referring to? If so, I would say "go for it" :-))

That's the point. Currently libvirt requires at least one tag per
VLAN. An here's the patch that relaxes that:

https://www.redhat.com/archives/libvir-list/2012-October/msg00214.html

And here's v2 of this patch:

https://www.redhat.com/archives/libvir-list/2012-October/msg00215.html

-- 
Matthias Bolte
http://photron.blogspot.com