[libvirt] [PATCH 0/1] port profile id

The following proof of concept patch attempts to show how we might support the use of port profile IDs. The ID would be associated with an interface on a VM and provided to the network infrastructure at VM start time. Since the interfaces by which the id can be provided are still in flux, the use function is only a stub. Dave David Allan (1): Initial POC of port profile id support docs/schemas/domain.rng | 8 ++++++++ src/conf/domain_conf.c | 12 ++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 3 +++ src/qemu/qemu_conf.c | 12 ++++++++++++ src/util/macvtap.c | 13 +++++++++++++ src/util/macvtap.h | 4 ++++ 7 files changed, 53 insertions(+), 0 deletions(-)

--- docs/schemas/domain.rng | 8 ++++++++ src/conf/domain_conf.c | 12 ++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 3 +++ src/qemu/qemu_conf.c | 12 ++++++++++++ src/util/macvtap.c | 13 +++++++++++++ src/util/macvtap.h | 4 ++++ 7 files changed, 53 insertions(+), 0 deletions(-) diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index d804301..5917f60 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -794,6 +794,11 @@ <ref name="bridgeMode"/> </attribute> </optional> + <optional> + <attribute name="profileid"> + <ref name="profileID"/> + </attribute> + </optional> <empty/> </element> <ref name="interface-options"/> @@ -1647,6 +1652,9 @@ <param name="pattern">(vepa|bridge|private)</param> </data> </define> + <define name="profileID"> + <data type="string"/> + </define> <define name="addrMAC"> <data type="string"> <param name="pattern">([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2}</param> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 66aa53e..ea2f8dd 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -479,6 +479,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def) case VIR_DOMAIN_NET_TYPE_DIRECT: VIR_FREE(def->data.direct.linkdev); + VIR_FREE(def->data.direct.profileid); break; case VIR_DOMAIN_NET_TYPE_USER: @@ -1801,6 +1802,7 @@ virDomainNetDefParseXML(virCapsPtr caps, char *internal = NULL; char *devaddr = NULL; char *mode = NULL; + char *profileid = NULL; virNWFilterHashTablePtr filterparams = NULL; if (VIR_ALLOC(def) < 0) { @@ -1843,6 +1845,7 @@ virDomainNetDefParseXML(virCapsPtr caps, xmlStrEqual(cur->name, BAD_CAST "source")) { dev = virXMLPropString(cur, "dev"); mode = virXMLPropString(cur, "mode"); + profileid = virXMLPropString(cur, "profileid"); } else if ((network == NULL) && ((def->type == VIR_DOMAIN_NET_TYPE_SERVER) || (def->type == VIR_DOMAIN_NET_TYPE_CLIENT) || @@ -2018,6 +2021,10 @@ virDomainNetDefParseXML(virCapsPtr caps, } else def->data.direct.mode = VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA; + if (profileid != NULL) { + def->data.direct.profileid = profileid; + } + def->data.direct.linkdev = dev; dev = NULL; @@ -2083,6 +2090,7 @@ cleanup: VIR_FREE(internal); VIR_FREE(devaddr); VIR_FREE(mode); + VIR_FREE(profileid); virNWFilterHashTableFree(filterparams); return def; @@ -5080,6 +5088,10 @@ virDomainNetDefFormat(virBufferPtr buf, def->data.direct.linkdev); virBufferVSprintf(buf, " mode='%s'", virDomainNetdevMacvtapTypeToString(def->data.direct.mode)); + if (def->data.direct.profileid) { + virBufferEscapeString(buf, " profileid='%s'", + def->data.direct.profileid); + } virBufferAddLit(buf, "/>\n"); break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index ed1a4ad..4c23a23 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -289,6 +289,7 @@ struct _virDomainNetDef { struct { char *linkdev; int mode; + char *profileid; } direct; } data; char *ifname; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index cc943f8..a14b114 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -702,3 +702,6 @@ virXPathLongLong; virXPathULongLong; virXPathLongHex; virXPathULongHex; + +# macvtap.h +sendPortProfileId; diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 8f6f7ec..f2880d3 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1469,6 +1469,18 @@ qemudPhysIfaceConnect(virConnectPtr conn, net->model && STREQ(net->model, "virtio")) vnet_hdr = 1; + /* Failure here is equivalent to the failure to plug in a physical + * network port. + * + * If this operation is non-blocking we will have a race between + * the VM starting and the interface coming up. + * + * If any of the subsequent operations fail, will we need to + * unwind the sending of the port profile id? */ + sendPortProfileId(net->data.direct.linkdev, + net->data.direct.mode, + net->data.direct.profileid); + rc = openMacvtapTap(conn, net->ifname, net->mac, linkdev, brmode, &res_ifname, vnet_hdr); if (rc >= 0) { diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 999e670..585e56b 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -43,6 +43,7 @@ # include "util.h" # include "memory.h" +# include "logging.h" # include "macvtap.h" # include "conf/domain_conf.h" # include "virterror_internal.h" @@ -673,6 +674,18 @@ configMacvtapTap(int tapfd, int vnet_hdr) } +int +sendPortProfileId(const char *linkdev, + int mode, + const char *profileid) +{ + VIR_DEBUG("Sending port profile id '%s' on physical device '%s' mode %d", + profileid, linkdev, mode); + + return 0; +} + + /** * openMacvtapTap: * Create an instance of a macvtap device and open its tap character diff --git a/src/util/macvtap.h b/src/util/macvtap.h index bd5f4d6..6cd7621 100644 --- a/src/util/macvtap.h +++ b/src/util/macvtap.h @@ -38,6 +38,10 @@ int openMacvtapTap(virConnectPtr conn, void delMacvtap(const char *ifname); +int sendPortProfileId(const char *linkdev, + int mode, + const char *profileid); + # endif /* WITH_MACVTAP */ # define MACVTAP_MODE_PRIVATE_STR "private" -- 1.7.0.1

On Tue, Mar 30, 2010 at 05:30:56PM -0400, David Allan wrote:
--- docs/schemas/domain.rng | 8 ++++++++ src/conf/domain_conf.c | 12 ++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 3 +++ src/qemu/qemu_conf.c | 12 ++++++++++++ src/util/macvtap.c | 13 +++++++++++++ src/util/macvtap.h | 4 ++++ 7 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index d804301..5917f60 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -794,6 +794,11 @@ <ref name="bridgeMode"/> </attribute> </optional> + <optional> + <attribute name="profileid"> + <ref name="profileID"/> + </attribute> + </optional> <empty/> </element> <ref name="interface-options"/> @@ -1647,6 +1652,9 @@ <param name="pattern">(vepa|bridge|private)</param> </data> </define> + <define name="profileID"> + <data type="string"/> + </define> <define name="addrMAC"> <data type="string"> <param name="pattern">([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2}</param>
I think this would be better off as a new element within interface, so that if we need more data associated with the profile we can provide it. eg perhaps something like <switchport profile='XYZ'/> Wouldn't this also be valid for the type=bridge networking mode, since that is connecting VMs to the LAN too. Regards, Daniel -- |: Red Hat, Engineering, London -o- http://people.redhat.com/berrange/ :| |: http://libvirt.org -o- http://virt-manager.org -o- http://deltacloud.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: GnuPG: 7D3B9505 -o- F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505 :|

On 03/31/2010 06:28 AM, Daniel P. Berrange wrote:
On Tue, Mar 30, 2010 at 05:30:56PM -0400, David Allan wrote:
--- docs/schemas/domain.rng | 8 ++++++++ src/conf/domain_conf.c | 12 ++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 3 +++ src/qemu/qemu_conf.c | 12 ++++++++++++ src/util/macvtap.c | 13 +++++++++++++ src/util/macvtap.h | 4 ++++ 7 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index d804301..5917f60 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -794,6 +794,11 @@ <ref name="bridgeMode"/> </attribute> </optional> +<optional> +<attribute name="profileid"> +<ref name="profileID"/> +</attribute> +</optional> <empty/> </element> <ref name="interface-options"/> @@ -1647,6 +1652,9 @@ <param name="pattern">(vepa|bridge|private)</param> </data> </define> +<define name="profileID"> +<data type="string"/> +</define> <define name="addrMAC"> <data type="string"> <param name="pattern">([a-fA-F0-9]{2}:){5}[a-fA-F0-9]{2}</param>
I think this would be better off as a new element within interface, so that if we need more data associated with the profile we can provide it. eg perhaps something like
<switchport profile='XYZ'/>
Wouldn't this also be valid for the type=bridge networking mode, since that is connecting VMs to the LAN too.
Agreed with both; an updated patch is attached. I also added a test for the new element. Dave

On Fri, 2010-04-02 at 11:42 -0400, Dave Allan wrote:
Agreed with both; an updated patch is attached. I also added a test for the new element.
Dave
Hi, unless I am missing something, you changed only the XML (<attribute name="profile">), but not the source code (profileid = virXMLPropString(cur, "profileid");) I also get some weird characters, which I don't understand: 13:13:05.475: debug : virDomainNetDefParseXML:2057 : profileid=a 13:13:19.668: debug : virDomainNetDefFormat:5153 : profileid=з� I added a few debug lines in domain_conf.c to produce above output. The first line is when libvirtd starts up and reads all domain files. The second line is when I tried to edit the domain xml via "virsh edit". -- Best regards, Gerhard Stenzel, ----------------------------------------------------------------------------------------------------------------------------------- IBM Deutschland Research & Development GmbH Vorsitzender des Aufsichtsrats: Martin Jetter Geschäftsführung: Dirk Wittkopp Sitz der Gesellschaft: Böblingen Registergericht: Amtsgericht Stuttgart, HRB 243294

libvir-list-bounces@redhat.com wrote on 04/02/2010 11:42:03 AM:
On 03/31/2010 06:28 AM, Daniel P. Berrange wrote:
On Tue, Mar 30, 2010 at 05:30:56PM -0400, David Allan wrote:
---
[...]
Wouldn't this also be valid for the type=bridge networking mode, since that is connecting VMs to the LAN too.
Agreed with both; an updated patch is attached. I also added a test for
the new element.
Dave
From a945107f047c7cd71f9c1b74fd74c47d8cdc3670 Mon Sep 17 00:00:00 2001
From: David Allan <dallan@redhat.com> Date: Fri, 12 Mar 2010 13:25:04 -0500 Subject: [PATCH 1/1] POC of port profile id support * Modified schema per DanPB's feedback * Added test for modified schema --- docs/schemas/domain.rng | 13 +++++++++++++ src/conf/domain_conf.c | 12 ++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 3 +++ src/qemu/qemu_conf.c | 12 ++++++++++++ src/util/macvtap.c | 13 +++++++++++++ src/util/macvtap.h | 4 ++++ tests/domainschemadata/portprofile.xml | 15 +++++++++++++++ 8 files changed, 73 insertions(+), 0 deletions(-) create mode 100644 tests/domainschemadata/portprofile.xml diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index 228665c..53280ce 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -816,6 +816,9 @@ </optional> <empty/> </element> + <optional> + <ref name="switchPort"/> + </optional> <ref name="interface-options"/> </interleave> </group> @@ -896,6 +899,16 @@ </optional> </interleave> </define> + <define name="switchPort"> + <element name="switchport"> + <optional> + <attribute name="profile"> + <text/> + </attribute> + </optional> + <empty/> + </element> + </define> <!-- An emulator description is just a path to the binary used for the task --> diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 067f053..d2b791f 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -481,6 +481,7 @@ void virDomainNetDefFree(virDomainNetDefPtr def) case VIR_DOMAIN_NET_TYPE_DIRECT: VIR_FREE(def->data.direct.linkdev); + VIR_FREE(def->data.direct.profileid); break; case VIR_DOMAIN_NET_TYPE_USER: @@ -1823,6 +1824,7 @@ virDomainNetDefParseXML(virCapsPtr caps, char *internal = NULL; char *devaddr = NULL; char *mode = NULL; + char *profileid = NULL; virNWFilterHashTablePtr filterparams = NULL; if (VIR_ALLOC(def) < 0) { @@ -1865,6 +1867,7 @@ virDomainNetDefParseXML(virCapsPtr caps, xmlStrEqual(cur->name, BAD_CAST "source")) { dev = virXMLPropString(cur, "dev"); mode = virXMLPropString(cur, "mode"); + profileid = virXMLPropString(cur, "profileid"); COMMENT: For the above code to be able to parse this + <source dev='eth0' mode='vepa'/> + <switchport profile='foo'/> I think you'll first need to find a switchport element. } else if ((network == NULL) && ((def->type == VIR_DOMAIN_NET_TYPE_SERVER) || (def->type == VIR_DOMAIN_NET_TYPE_CLIENT) || @@ -2040,6 +2043,10 @@ virDomainNetDefParseXML(virCapsPtr caps, } else def->data.direct.mode = VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA; + if (profileid != NULL) { + def->data.direct.profileid = profileid; + } + def->data.direct.linkdev = dev; dev = NULL; @@ -2105,6 +2112,7 @@ cleanup: VIR_FREE(internal); VIR_FREE(devaddr); VIR_FREE(mode); + VIR_FREE(profileid); virNWFilterHashTableFree(filterparams); return def; @@ -5131,6 +5139,10 @@ virDomainNetDefFormat(virBufferPtr buf, def->data.direct.linkdev); virBufferVSprintf(buf, " mode='%s'", virDomainNetdevMacvtapTypeToString(def->data.direct.mode)); + if (def->data.direct.profileid) { + virBufferEscapeString(buf, " profileid='%s'", + def->data.direct.profileid); + } COMMENT: You'd need to print a switchport element first. virBufferAddLit(buf, "/>\n"); break; diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index b789289..37a58a9 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -289,6 +289,7 @@ struct _virDomainNetDef { struct { char *linkdev; int mode; + char *profileid; } direct; } data; char *ifname; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 878eda3..253f935 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -703,3 +703,6 @@ virXPathLongLong; virXPathULongLong; virXPathLongHex; virXPathULongHex; + +# macvtap.h +sendPortProfileId; COMMENT: There's a separate libvirt_macvtap.syms file. Also, please prefix this function with macvtap -> macvtapSendPortProfileId diff --git a/src/qemu/qemu_conf.c b/src/qemu/qemu_conf.c index 55397cd..3cb3ce9 100644 --- a/src/qemu/qemu_conf.c +++ b/src/qemu/qemu_conf.c @@ -1469,6 +1469,18 @@ qemudPhysIfaceConnect(virConnectPtr conn, net->model && STREQ(net->model, "virtio")) vnet_hdr = 1; + /* Failure here is equivalent to the failure to plug in a physical + * network port. + * + * If this operation is non-blocking we will have a race between + * the VM starting and the interface coming up. + * + * If any of the subsequent operations fail, will we need to + * unwind the sending of the port profile id? */ + sendPortProfileId(net->data.direct.linkdev, + net->data.direct.mode, + net->data.direct.profileid); + COMMENT: I'd push this call into the openMacvtapTap function since it will be part of the setup of the Tap device, no? Stefan rc = openMacvtapTap(conn, net->ifname, net->mac, linkdev, brmode, &res_ifname, vnet_hdr); if (rc >= 0) { diff --git a/src/util/macvtap.c b/src/util/macvtap.c index 999e670..585e56b 100644 --- a/src/util/macvtap.c +++ b/src/util/macvtap.c @@ -43,6 +43,7 @@ # include "util.h" # include "memory.h" +# include "logging.h" # include "macvtap.h" # include "conf/domain_conf.h" # include "virterror_internal.h" @@ -673,6 +674,18 @@ configMacvtapTap(int tapfd, int vnet_hdr) } +int +sendPortProfileId(const char *linkdev, + int mode, + const char *profileid) +{ + VIR_DEBUG("Sending port profile id '%s' on physical device '%s' mode %d", + profileid, linkdev, mode); + + return 0; +} + + /** * openMacvtapTap: * Create an instance of a macvtap device and open its tap character diff --git a/src/util/macvtap.h b/src/util/macvtap.h index bd5f4d6..6cd7621 100644 --- a/src/util/macvtap.h +++ b/src/util/macvtap.h @@ -38,6 +38,10 @@ int openMacvtapTap(virConnectPtr conn, void delMacvtap(const char *ifname); +int sendPortProfileId(const char *linkdev, + int mode, + const char *profileid); + # endif /* WITH_MACVTAP */ # define MACVTAP_MODE_PRIVATE_STR "private" diff --git a/tests/domainschemadata/portprofile.xml b/tests/domainschemadata/portprofile.xml new file mode 100644 index 0000000..7152ffd --- /dev/null +++ b/tests/domainschemadata/portprofile.xml @@ -0,0 +1,15 @@ +<domain type='lxc'> + <name>portprofile</name> + <uuid>00000000-0000-0000-0000-000000000000</uuid> + <memory>1048576</memory> + <os> + <type>exe</type> + <init>/sh</init> + </os> + <devices> + <interface type='direct'> + <source dev='eth0' mode='vepa'/> + <switchport profile='foo'/> + </interface> + </devices> +</domain> -- 1.7.0.1

On Tue, 30 Mar 2010, David Allan wrote:
Date: Tue, 30 Mar 2010 17:30:56 -0400 From: David Allan <dallan@redhat.com> To: libvir-list@redhat.com Subject: [libvirt] [PATCH 1/1] Initial POC of port profile id support
--- docs/schemas/domain.rng | 8 ++++++++ src/conf/domain_conf.c | 12 ++++++++++++ src/conf/domain_conf.h | 1 + src/libvirt_private.syms | 3 +++ src/qemu/qemu_conf.c | 12 ++++++++++++ src/util/macvtap.c | 13 +++++++++++++ src/util/macvtap.h | 4 ++++ 7 files changed, 53 insertions(+), 0 deletions(-)
diff --git a/docs/schemas/domain.rng b/docs/schemas/domain.rng index d804301..5917f60 100644 --- a/docs/schemas/domain.rng +++ b/docs/schemas/domain.rng @@ -794,6 +794,11 @@ <ref name="bridgeMode"/> </attribute> </optional> + <optional> + <attribute name="profileid"> + <ref name="profileID"/> + </attribute> + </optional> <empty/> </element> <ref name="interface-options"/>
This is a good proposal. We need it to correctly configure the guests with VEPA mode. The profile-ID is required for the VEPA mode as per IEEE 802.1Qbg proposal, however, we can use it with bridging mode as well. How do we associate it with bridging mode if so desired?
@@ -1647,6 +1652,9 @@ <param name="pattern">(vepa|bridge|private)</param> </data> </define> + <define name="profileID"> + <data type="string"/> + </define> <define name="addrMAC">
The profile-ID, as per the TLV described comprises of the following fields: VSI Manager, VSI instance ID, VSI type, VSI type version. Though I agree libvirt need not know how to parse it or define the fields.
+ /* Failure here is equivalent to the failure to plug in a physical + * network port. + * + * If this operation is non-blocking we will have a race between + * the VM starting and the interface coming up. + * + * If any of the subsequent operations fail, will we need to + * unwind the sending of the port profile id? */ + sendPortProfileId(net->data.direct.linkdev, + net->data.direct.mode, + net->data.direct.profileid); +
This is good. The VSI Discovery and Configuration protocol(VDP) will need to be run between the bridge and the KVM host. The VDP daemon then receives the profile details along with the MAC/VLAN settings for the interface and initiates the VDP on its behalf with the switch. A synchronous wait for the VDP to successfully associate the MAC/VLAN with the switch seems the simplest unless the MACVTAP is configured in 'no carrier' state in the guest (Can that be done?). Otherwise the VM might start transmitting and loose packets while the VDP completes its registration. Blocking mode also makes error percolation easier as well. Vivek
# define MACVTAP_MODE_PRIVATE_STR "private" -- 1.7.0.1
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
__ Vivek Kashyap Linux Technology Center, IBM
participants (6)
-
Daniel P. Berrange
-
Dave Allan
-
David Allan
-
Gerhard Stenzel
-
Stefan Berger
-
Vivek Kashyap