[libvirt] [PATCH v2] vepa: parsing for 802.1Qb{g|h} XML

Below is David Alan's original patch with lots of changes. In particular, it now parses the following two XML descriptions, one for 802.1Qbg and 802.1Qbh and stored the data internally. The actual triggering of the switch setup protocol has not been implemented here but the relevant code to do that should go into the functions associatePortProfileId() and disassociatePortProfileId(). <interface type='direct'> <source dev='static' mode='vepa'/> <model type='virtio'/> <vsi type='802.1Qbg'> <parameters managerid='12' typeid='0x123456' typeidversion='1' instanceid='fa9b7fff-b0a0-4893-8e0e-beef4ff18f8f'/> </vsi> <filterref filter='clean-traffic'/> </interface> <interface type='direct'> <source dev='static' mode='vepa'/> <model type='virtio'/> <vsi type='802.1Qbh'> <parameters profileid='my_profile'/> </vsi> </interface> I'd suggest to use this patch as a base for triggering the setup protocol with the 802.1Qb{g|h} switch. Some of the changes from V1 to V2: - parser and XML generator have been separated into their own functions so they can be re-used elsewhere (passthrough case for example) - Adapted XML parser and generator support the above shown type (802.1Qbg, 802.1Qbh). - Adapted schema to above XML - Adapted test XML to above XML - Passing through the VM's UUID which seems to be necessary for 802.1Qbh -- sorry no host UUID - adding virtual function ID to association function, in case it's necessary to use (for SR-IOV) Signed-off-by: Stefan Berger <stefanb@us.ibm.com>
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 | 67 ++++++++++++++ src/conf/domain_conf.c | 150 +++++++++++++++++++++++++++++++++ src/conf/domain_conf.h | 35 +++++++ src/qemu/qemu_conf.c | 17 +-- src/qemu/qemu_conf.h | 5 - src/qemu/qemu_driver.c | 17 +-- src/util/macvtap.c | 150 +++++++++++++++++++++++++++++---- src/util/macvtap.h | 10 +- tests/domainschemadata/portprofile.xml | 30 ++++++ 9 files changed, 433 insertions(+), 48 deletions(-) create mode 100644 tests/domainschemadata/portprofile.xml Index: libvirt-acl/docs/schemas/domain.rng =================================================================== --- libvirt-acl.orig/docs/schemas/domain.rng +++ libvirt-acl/docs/schemas/domain.rng @@ -817,6 +817,9 @@ </optional> <empty/> </element> + <optional> + <ref name="vsiProfile"/> + </optional> <ref name="interface-options"/> </interleave> </group> @@ -902,6 +905,43 @@ </optional> </interleave> </define> + <define name="vsiProfile"> + <choice> + <group> + <element name="vsi"> + <attribute name="type"> + <value>802.1Qbg</value> + </attribute> + <element name="parameters"> + <attribute name="managerid"> + <ref name="uint8range"/> + </attribute> + <attribute name="typeid"> + <ref name="uint24range"/> + </attribute> + <attribute name="typeidversion"> + <ref name="uint8range"/> + </attribute> + <attribute name="instanceid"> + <ref name="UUID"/> + </attribute> + </element> + </element> + </group> + <group> + <element name="vsi"> + <attribute name="type"> + <value>802.1Qbh</value> + </attribute> + <element name="parameters"> + <attribute name="profileid"> + <ref name="vsiProfileID"/> + </attribute> + </element> + </element> + </group> + </choice> + </define> <!-- An emulator description is just a path to the binary used for the task --> @@ -1769,4 +1809,31 @@ <param name="pattern">[a-zA-Z0-9_\.:]+</param> </data> </define> + <define name="uint8range"> + <choice> + <data type="string"> + <param name="pattern">0x[0-9a-fA-F]{1,2}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">255</param> + </data> + </choice> + </define> + <define name="uint24range"> + <choice> + <data type="string"> + <param name="pattern">0x[0-9a-fA-F]{1,6}</param> + </data> + <data type="int"> + <param name="minInclusive">0</param> + <param name="maxInclusive">16777215</param> + </data> + </choice> + </define> + <define name="vsiProfileID"> + <data type="string"> + <param name="maxLength">39</param> + </data> + </define> </grammar> Index: libvirt-acl/src/conf/domain_conf.c =================================================================== --- libvirt-acl.orig/src/conf/domain_conf.c +++ libvirt-acl/src/conf/domain_conf.c @@ -242,6 +242,11 @@ VIR_ENUM_IMPL(virDomainNetdevMacvtap, VI "private", "bridge") +VIR_ENUM_IMPL(virVSI, VIR_VSI_TYPE_LAST, + "none", + "802.1Qbg", + "802.1Qbh") + VIR_ENUM_IMPL(virDomainClockOffset, VIR_DOMAIN_CLOCK_OFFSET_LAST, "utc", "localtime", @@ -1807,6 +1812,140 @@ isValidIfname(const char *ifname) { } +static void +virVSIProfileDefParseXML(xmlNodePtr node, + virVSIProfileDefPtr vsi) +{ + char *vsiType; + char *vsiManagerID = NULL; + char *vsiTypeID = NULL; + char *vsiTypeIDVersion = NULL; + char *vsiInstanceID = NULL; + char *vsiProfileID = NULL; + xmlNodePtr cur = node->children; + + vsiType = virXMLPropString(node, "type"); + if (!vsiType) + return; + + while (cur != NULL) { + if (xmlStrEqual(cur->name, BAD_CAST "parameters")) { + + vsiManagerID = virXMLPropString(cur, "managerid"); + vsiTypeID = virXMLPropString(cur, "typeid"); + vsiTypeIDVersion = virXMLPropString(cur, "typeidversion"); + vsiInstanceID = virXMLPropString(cur, "instanceid"); + vsiProfileID = virXMLPropString(cur, "profileid"); + + break; + } + + cur = cur->next; + } + + vsi->vsiType = VIR_VSI_NONE; + + switch (virVSITypeFromString(vsiType)) { + + case VIR_VSI_8021QBG: + if (vsiManagerID != NULL && vsiTypeID != NULL && + vsiTypeIDVersion != NULL && vsiInstanceID != NULL) { + unsigned int val; + + if ((virStrToLong_ui(vsiManagerID, NULL, 10, &val) && + virStrToLong_ui(vsiManagerID, NULL, 16, &val) ) || + val > 0xff) + break; + + vsi->u.vsi8021Qbg.managerID = (uint8_t)val; + + if ((virStrToLong_ui(vsiTypeID, NULL, 10, &val) && + virStrToLong_ui(vsiTypeID, NULL, 16, &val) ) || + val > 0xffffff) + break; + + vsi->u.vsi8021Qbg.typeID = (uint32_t)val; + + if ((virStrToLong_ui(vsiTypeIDVersion, NULL, 10, &val) && + virStrToLong_ui(vsiTypeIDVersion, NULL, 16, &val) ) || + val > 0xff) + break; + + vsi->u.vsi8021Qbg.typeIDVersion = (uint8_t)val; + + if (virUUIDParse(vsiInstanceID, vsi->u.vsi8021Qbg.instanceID)) + break; + + vsi->vsiType = VIR_VSI_8021QBG; + } + break; + + case VIR_VSI_8021QBH: + if (vsiProfileID != NULL) { + if (virStrcpyStatic(vsi->u.vsi8021Qbh.profileID, + vsiProfileID) != NULL) + vsi->vsiType = VIR_VSI_8021QBH; + } + break; + + + default: + case VIR_VSI_NONE: + case VIR_VSI_TYPE_LAST: + break; + } + + VIR_FREE(vsiManagerID); + VIR_FREE(vsiTypeID); + VIR_FREE(vsiTypeIDVersion); + VIR_FREE(vsiInstanceID); + VIR_FREE(vsiProfileID); + VIR_FREE(vsiType); +} + + +static void +virVSIProfileFormat(virBufferPtr buf, virVSIProfileDefPtr vsi, + const char *indent) +{ + char uuidstr[VIR_UUID_STRING_BUFLEN]; + + if (vsi->vsiType == VIR_VSI_NONE) + return; + + virBufferVSprintf(buf, "%s<vsi type='%s'>\n", + indent, virVSITypeToString(vsi->vsiType)); + + switch (vsi->vsiType) { + case VIR_VSI_NONE: + case VIR_VSI_TYPE_LAST: + break; + + case VIR_VSI_8021QBG: + virUUIDFormat(vsi->u.vsi8021Qbg.instanceID, + uuidstr); + virBufferVSprintf(buf, + "%s <parameters managerid='%d' typeid='%d' " + "typeidversion='%d' instanceid='%s'/>\n", + indent, + vsi->u.vsi8021Qbg.managerID, + vsi->u.vsi8021Qbg.typeID, + vsi->u.vsi8021Qbg.typeIDVersion, + uuidstr); + break; + + case VIR_VSI_8021QBH: + virBufferVSprintf(buf, + "%s <parameters profileid='%s'/>\n", + indent, + vsi->u.vsi8021Qbh.profileID); + break; + } + + virBufferVSprintf(buf, "%s</vsi>\n", indent); +} + + /* Parse the XML definition for a network interface * @param node XML nodeset to parse for net definition * @return 0 on success, -1 on failure @@ -1832,6 +1971,8 @@ virDomainNetDefParseXML(virCapsPtr caps, char *devaddr = NULL; char *mode = NULL; virNWFilterHashTablePtr filterparams = NULL; + virVSIProfileDef vsi; + bool vsiParsed = false; if (VIR_ALLOC(def) < 0) { virReportOOMError(); @@ -1873,6 +2014,11 @@ virDomainNetDefParseXML(virCapsPtr caps, xmlStrEqual(cur->name, BAD_CAST "source")) { dev = virXMLPropString(cur, "dev"); mode = virXMLPropString(cur, "mode"); + } else if ((vsiParsed == false) && + (def->type == VIR_DOMAIN_NET_TYPE_DIRECT) && + xmlStrEqual(cur->name, BAD_CAST "vsi")) { + virVSIProfileDefParseXML(cur, &vsi); + vsiParsed = true; } else if ((network == NULL) && ((def->type == VIR_DOMAIN_NET_TYPE_SERVER) || (def->type == VIR_DOMAIN_NET_TYPE_CLIENT) || @@ -2049,6 +2195,8 @@ virDomainNetDefParseXML(virCapsPtr caps, } else def->data.direct.mode = VIR_DOMAIN_NETDEV_MACVTAP_MODE_VEPA; + def->data.direct.vsiProfile = vsi; + def->data.direct.linkdev = dev; dev = NULL; @@ -5141,6 +5289,8 @@ virDomainNetDefFormat(virBufferPtr buf, virBufferVSprintf(buf, " mode='%s'", virDomainNetdevMacvtapTypeToString(def->data.direct.mode)); virBufferAddLit(buf, "/>\n"); + virVSIProfileFormat(buf, &def->data.direct.vsiProfile, + " "); break; case VIR_DOMAIN_NET_TYPE_USER: Index: libvirt-acl/src/conf/domain_conf.h =================================================================== --- libvirt-acl.orig/src/conf/domain_conf.h +++ libvirt-acl/src/conf/domain_conf.h @@ -259,6 +259,39 @@ enum virDomainNetdevMacvtapType { }; +enum virVSIType { + VIR_VSI_NONE, + VIR_VSI_8021QBG, + VIR_VSI_8021QBH, + + VIR_VSI_TYPE_LAST, +}; + +# ifdef IFLA_VF_PORT_PROFILE_MAX +# define LIBVIRT_IFLA_VF_PORT_PROFILE_MAX IFLA_VF_PORT_PROFILE_MAX +# else +# define LIBVIRT_IFLA_VF_PORT_PROFILE_MAX 40 +# endif + +/* profile data for macvtap (VEPA) */ +typedef struct _virVSIProfileDef virVSIProfileDef; +typedef virVSIProfileDef *virVSIProfileDefPtr; +struct _virVSIProfileDef { + enum virVSIType vsiType; + union { + struct { + uint8_t managerID; + uint32_t typeID; // 24 bit valid + uint8_t typeIDVersion; + unsigned char instanceID[VIR_UUID_BUFLEN]; + } vsi8021Qbg; + struct { + char profileID[LIBVIRT_IFLA_VF_PORT_PROFILE_MAX]; + } vsi8021Qbh; + } u; +}; + + /* Stores the virtual network interface configuration */ typedef struct _virDomainNetDef virDomainNetDef; typedef virDomainNetDef *virDomainNetDefPtr; @@ -290,6 +323,7 @@ struct _virDomainNetDef { struct { char *linkdev; int mode; + virVSIProfileDef vsiProfile; } direct; } data; char *ifname; @@ -1089,6 +1123,7 @@ VIR_ENUM_DECL(virDomainSeclabel) VIR_ENUM_DECL(virDomainClockOffset) VIR_ENUM_DECL(virDomainNetdevMacvtap) +VIR_ENUM_DECL(virVSI) VIR_ENUM_DECL(virDomainTimerName) VIR_ENUM_DECL(virDomainTimerTrack) Index: libvirt-acl/src/util/macvtap.c =================================================================== --- libvirt-acl.orig/src/util/macvtap.c +++ libvirt-acl/src/util/macvtap.c @@ -46,6 +46,7 @@ # include "util.h" # include "memory.h" +# include "logging.h" # include "macvtap.h" # include "interface.h" # include "conf/domain_conf.h" @@ -751,36 +752,127 @@ configMacvtapTap(int tapfd, int vnet_hdr /** + * associatePortProfile + * + * @linkdev: the link dev in case of macvtap + * @mac: the MAC address of the interface + * @mode: The vepa mode (private, bridge or vepa) + * @vsi: pointer to the object holding port profile parameters + * @vf: virtual function number, -1 if to be ignored + * @vmuuid : the UUID of the virtual machine + * + * Associate a port on a swtich with a profile. This function + * may notify a kernel driver or an external daemon to run + * the setup protocol. If profile parameters were not supplied + * by the user, then this function returns without doing + * anything. + * + * Returns 0 in case of success, != 0 otherwise with error + * having been reported. + */ +static int +associatePortProfileId(const char *linkdev, + const unsigned char *mac, + int mode, + const virVSIProfileDefPtr vsi, + int vf, + const unsigned char *vmuuid) +{ + int rc = 0; + VIR_DEBUG("Associating port profile '%p' on link device '%s' mode % d ", + vsi, linkdev, mode); + (void)mac; + (void)vf; + (void)vmuuid; + + switch (vsi->vsiType) { + case VIR_VSI_NONE: + case VIR_VSI_TYPE_LAST: + break; + + case VIR_VSI_8021QBG: + rc = -1; + break; + + case VIR_VSI_8021QBH: + rc = -1; + break; + } + + return rc; +} + + +/** + * disassociatePortProfile + * + * @linkdev: The link dev in case of a macvtap device + * @mac: The MAC address of the interface + * @vsi: point to object holding port profile parameters + * + * Returns 0 in case of success, != 0 otherwise with error + * having been reported. + */ +static int +disassociatePortProfileId(const char *linkdev, + const unsigned char *mac, + const virVSIProfileDefPtr vsi) +{ + int rc = 0; + VIR_DEBUG("Disassociating port profile id '%p' on link device '%s' ", + vsi, linkdev); + (void)mac; + + switch (vsi->vsiType) { + case VIR_VSI_NONE: + case VIR_VSI_TYPE_LAST: + break; + + case VIR_VSI_8021QBG: + rc = -1; + break; + + case VIR_VSI_8021QBH: + rc = -1; + break; + } + + return rc; +} + + +/** * openMacvtapTap: * Create an instance of a macvtap device and open its tap character * device. * @tgifname: Interface name that the macvtap is supposed to have. May * be NULL if this function is supposed to choose a name - * @macaddress: The MAC address for the macvtap device - * @linkdev: The interface name of the NIC to connect to the external bridge - * @mode_str: String describing the mode. Valid are 'bridge', 'vepa' and - * 'private'. + * @net: pointer to the virDomainNetDef object describing the direct + * type if an interface * @res_ifname: Pointer to a string pointer where the actual name of the * interface will be stored into if everything succeeded. It is up * to the caller to free the string. + * @vnet_hdr: Whether to enable IFF_VNET_HDR on the interface + * @vmuuid: The (raw) UUID of the VM * * Returns file descriptor of the tap device in case of success, * negative value otherwise with error reported. * + * Open a macvtap device and trigger the switch setup protocol + * if valid port profile parameters were provided. */ int openMacvtapTap(const char *tgifname, - const unsigned char *macaddress, - const char *linkdev, - int mode, + virDomainNetDefPtr net, char **res_ifname, - int vnet_hdr) + int vnet_hdr, + const unsigned char *vmuuid) { const char *type = "macvtap"; int c, rc; char ifname[IFNAMSIZ]; int retries, do_retry = 0; - uint32_t macvtapMode = macvtapModeFromInt(mode); + uint32_t macvtapMode = macvtapModeFromInt(net->data.direct.mode); const char *cr_ifname; int ifindex; @@ -797,7 +889,7 @@ openMacvtapTap(const char *tgifname, return -1; } cr_ifname = tgifname; - rc = link_add(type, macaddress, 6, tgifname, linkdev, + rc = link_add(type, net->mac, 6, tgifname, net->data.direct.linkdev, macvtapMode, &do_retry); if (rc) return -1; @@ -807,7 +899,8 @@ create_name: for (c = 0; c < 8192; c++) { snprintf(ifname, sizeof(ifname), MACVTAP_NAME_PATTERN, c); if (ifaceGetIndex(false, ifname, &ifindex) == ENODEV) { - rc = link_add(type, macaddress, 6, ifname, linkdev, + rc = link_add(type, net->mac, 6, ifname, + net->data.direct.linkdev, macvtapMode, &do_retry); if (rc == 0) break; @@ -820,6 +913,15 @@ create_name: cr_ifname = ifname; } + rc = associatePortProfileId(net->data.direct.linkdev, + net->mac, + net->data.direct.mode, + &net->data.direct.vsiProfile, + -1, + vmuuid); + if (rc != 0) + goto link_del_exit; + rc = ifaceUp(cr_ifname); if (rc != 0) { virReportSystemError(errno, @@ -828,7 +930,7 @@ create_name: "MAC address"), cr_ifname); rc = -1; - goto link_del_exit; + goto disassociate_exit; } rc = openTap(cr_ifname, 10); @@ -837,14 +939,19 @@ create_name: if (configMacvtapTap(rc, vnet_hdr) < 0) { close(rc); rc = -1; - goto link_del_exit; + goto disassociate_exit; } *res_ifname = strdup(cr_ifname); } else - goto link_del_exit; + goto disassociate_exit; return rc; +disassociate_exit: + disassociatePortProfileId(net->data.direct.linkdev, + net->mac, + &net->data.direct.vsiProfile); + link_del_exit: link_del(cr_ifname); @@ -854,14 +961,21 @@ link_del_exit: /** * delMacvtap: - * @ifname : The name of the macvtap interface + * @net: pointer to virDomainNetDef object * - * Delete an interface given its name. + * Delete an interface given its name. Disassociate + * it with the switch if port profile parameters + * were provided. */ void -delMacvtap(const char *ifname) +delMacvtap(virDomainNetDefPtr net) { - link_del(ifname); + if (net->ifname) { + disassociatePortProfileId(net->data.direct.linkdev, + net->mac, + &net->data.direct.vsiProfile); + link_del(net->ifname); + } } #endif Index: libvirt-acl/src/util/macvtap.h =================================================================== --- libvirt-acl.orig/src/util/macvtap.h +++ libvirt-acl/src/util/macvtap.h @@ -27,15 +27,15 @@ # if defined(WITH_MACVTAP) # include "internal.h" +# include "conf/domain_conf.h" int openMacvtapTap(const char *ifname, - const unsigned char *macaddress, - const char *linkdev, - int mode, + virDomainNetDefPtr net, char **res_ifname, - int vnet_hdr); + int vnet_hdr, + const unsigned char *vmuuid); -void delMacvtap(const char *ifname); +void delMacvtap(virDomainNetDefPtr net); # endif /* WITH_MACVTAP */ Index: libvirt-acl/tests/domainschemadata/portprofile.xml =================================================================== --- /dev/null +++ libvirt-acl/tests/domainschemadata/portprofile.xml @@ -0,0 +1,30 @@ +<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'/> + <vsi type='802.1Qbg'> + <parameters managerid='12' typeid='1193046' typeidversion='1' + instanceid='fa9b7fff-b0a0-4893-8e0e-beef4ff18f8f'/> + </vsi> + </interface> + <interface type='direct'> + <source dev='eth0' mode='vepa'/> + <vsi type='802.1Qbh'> + <parameters profileid='my_profile'/> + </vsi> + </interface> + <interface type='direct'> + <source dev='eth0' mode='vepa'/> + </interface> + <interface type='direct'> + <source dev='eth0' mode='vepa'/> + </interface> + </devices> +</domain> Index: libvirt-acl/src/qemu/qemu_conf.h =================================================================== --- libvirt-acl.orig/src/qemu/qemu_conf.h +++ libvirt-acl/src/qemu/qemu_conf.h @@ -271,9 +271,8 @@ qemudOpenVhostNet(virDomainNetDefPtr net int qemudPhysIfaceConnect(virConnectPtr conn, struct qemud_driver *driver, virDomainNetDefPtr net, - char *linkdev, - int brmode, - unsigned long long qemuCmdFlags); + unsigned long long qemuCmdFlags, + const unsigned char *vmuuid); int qemudProbeMachineTypes (const char *binary, virCapsGuestMachinePtr **machines, Index: libvirt-acl/src/qemu/qemu_driver.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_driver.c +++ libvirt-acl/src/qemu/qemu_driver.c @@ -3585,10 +3585,8 @@ static void qemudShutdownVMDaemon(struct def = vm->def; for (i = 0; i < def->nnets; i++) { virDomainNetDefPtr net = def->nets[i]; - if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) { - if (net->ifname) - delMacvtap(net->ifname); - } + if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) + delMacvtap(net); } #endif @@ -7175,9 +7173,8 @@ static int qemudDomainAttachNetDevice(vi } if ((tapfd = qemudPhysIfaceConnect(conn, driver, net, - net->data.direct.linkdev, - net->data.direct.mode, - qemuCmdFlags)) < 0) + qemuCmdFlags, + vm->def->uuid)) < 0) return -1; } @@ -8146,10 +8143,8 @@ qemudDomainDetachNetDevice(struct qemud_ virNWFilterTearNWFilter(detach); #if WITH_MACVTAP - if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) { - if (detach->ifname) - delMacvtap(detach->ifname); - } + if (detach->type == VIR_DOMAIN_NET_TYPE_DIRECT) + delMacvtap(detach); #endif if ((driver->macFilter) && (detach->ifname != NULL)) { Index: libvirt-acl/src/qemu/qemu_conf.c =================================================================== --- libvirt-acl.orig/src/qemu/qemu_conf.c +++ libvirt-acl/src/qemu/qemu_conf.c @@ -1465,9 +1465,8 @@ int qemudPhysIfaceConnect(virConnectPtr conn, struct qemud_driver *driver, virDomainNetDefPtr net, - char *linkdev, - int brmode, - unsigned long long qemuCmdFlags) + unsigned long long qemuCmdFlags, + const unsigned char *vmuuid) { int rc; #if WITH_MACVTAP @@ -1479,8 +1478,7 @@ qemudPhysIfaceConnect(virConnectPtr conn net->model && STREQ(net->model, "virtio")) vnet_hdr = 1; - rc = openMacvtapTap(net->ifname, net->mac, linkdev, brmode, - &res_ifname, vnet_hdr); + rc = openMacvtapTap(net->ifname, net, &res_ifname, vnet_hdr, vmuuid); if (rc >= 0) { VIR_FREE(net->ifname); net->ifname = res_ifname; @@ -1500,15 +1498,13 @@ qemudPhysIfaceConnect(virConnectPtr conn if (err) { close(rc); rc = -1; - delMacvtap(net->ifname); + delMacvtap(net); } } } #else (void)conn; (void)net; - (void)linkdev; - (void)brmode; (void)qemuCmdFlags; (void)driver; qemuReportError(VIR_ERR_INTERNAL_ERROR, @@ -4130,9 +4126,8 @@ int qemudBuildCommandLine(virConnectPtr goto no_memory; } else if (net->type == VIR_DOMAIN_NET_TYPE_DIRECT) { int tapfd = qemudPhysIfaceConnect(conn, driver, net, - net->data.direct.linkdev, - net->data.direct.mode, - qemuCmdFlags); + qemuCmdFlags, + def->uuid); if (tapfd < 0) goto error;

On Wed, May 12, 2010 at 12:13:14PM -0400, Stefan Berger wrote:
Below is David Alan's original patch with lots of changes.
In particular, it now parses the following two XML descriptions, one for 802.1Qbg and 802.1Qbh and stored the data internally. The actual triggering of the switch setup protocol has not been implemented here but the relevant code to do that should go into the functions associatePortProfileId() and disassociatePortProfileId().
<interface type='direct'> <source dev='static' mode='vepa'/> <model type='virtio'/> <vsi type='802.1Qbg'> <parameters managerid='12' typeid='0x123456' typeidversion='1' instanceid='fa9b7fff-b0a0-4893-8e0e-beef4ff18f8f'/> </vsi> <filterref filter='clean-traffic'/> </interface>
<interface type='direct'> <source dev='static' mode='vepa'/> <model type='virtio'/> <vsi type='802.1Qbh'> <parameters profileid='my_profile'/> </vsi> </interface>
I'd suggest to use this patch as a base for triggering the setup protocol with the 802.1Qb{g|h} switch.
Ok, I think the XML suggestion is pretty reasonable wrt what I can read about current state of the 802.* standards. My main question is how this applies to the Cisco VNLink capability that (IIUC) already exists in hardware today. It sounds like at an XML level it is pretty much wanting the same data as the 802.1Qbh case, so we could simply add a 3rd option that follows the scheme: <interface type='direct'> <source dev='static' mode='vepa'/> <model type='virtio'/> <vsi type='vnlink'> <parameters profileid='my_profile'/> </vsi> </interface> Internally this type='vnlink' would be something we key off to decide whether we need to also pass down a host UUID and other bits of info the Cisco stuff wants (guest UUID/name). 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 :|

"Daniel P. Berrange" <berrange@redhat.com> wrote on 05/12/2010 12:32:42 PM:
Ok, I think the XML suggestion is pretty reasonable wrt what I can read about current state of the 802.* standards. My main question is how this applies to the Cisco VNLink capability that (IIUC) already exists in hardware today.
It sounds like at an XML level it is pretty much wanting the same data as the 802.1Qbh case, so we could simply add a 3rd option that follows the scheme:
<interface type='direct'> <source dev='static' mode='vepa'/> <model type='virtio'/> <vsi type='vnlink'> <parameters profileid='my_profile'/> </vsi> </interface>
Internally this type='vnlink' would be something we key off to decide whether we need to also pass down a host UUID and other bits of info the Cisco stuff wants (guest UUID/name).
If someone wants to provide patches for triggering the setup for this 3rd technology then this should now be fairly easy to support. Stefan

On Wed, 2010-05-12 at 12:13 -0400, Stefan Berger wrote:
I'd suggest to use this patch as a base for triggering the setup protocol with the 802.1Qb{g|h} switch.
Here is a RFC patch, which demonstrates how libvirt could communicate with lldpad via the lldptool for the 802.1Qbg case. Please note, that there is currently no public available version of lldptool which accepts this command line. This is also work in progress. This applies on top of the patch Stefan just posted. Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> Index: libvirt/src/util/macvtap.c =================================================================== --- libvirt.orig/src/util/macvtap.c +++ libvirt/src/util/macvtap.c @@ -750,6 +750,107 @@ configMacvtapTap(int tapfd, int vnet_hdr return 0; } +# define ASSOCIATE 0x02 +# define DEASSOCIATE 0x03 +# define LLDPTOOL_NAME "lldptool" + +static int +setPortProfileId(const char *linkdev, + const unsigned char *mac, + int mode, + const virVSIProfileDefPtr vsi) +{ + char macaddr[VIR_MAC_STRING_BUFLEN]; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + char rootifname[IFNAMSIZ]; + static char *lldptool; + char *modestr = NULL; + int vlanid = 0; + int rc; + int status = 0; +# define NUM_PARAMS 8 + const char *argv[NUM_PARAMS] = {NULL, }; + int argc = 0; + + virFormatMacAddr(mac, macaddr); + + rc = ifaceGetRootIface(-1, linkdev, rootifname); + if (rc != 0) { + macvtapError(VIR_ERR_INTERNAL_ERROR, + _("cannot get root interface for %s"), + linkdev); + return rc; + } + VIR_DEBUG("root iface of %s is %s\n", linkdev, rootifname); + + ifaceGetVlanID(linkdev, &vlanid); + VIR_DEBUG("vlan id of %s is %d\n", linkdev, vlanid); + + if (lldptool == NULL) { + lldptool = virFindFileInPath(LLDPTOOL_NAME); + if (lldptool == NULL) { + macvtapError(VIR_ERR_INTERNAL_ERROR, + _("unable to find %s"), LLDPTOOL_NAME); + return -1; + } + } + + virFormatMacAddr(mac, macaddr); + virUUIDFormat(vsi->u.vsi8021Qbg.instanceID, uuidstr); + + VIR_DEBUG("setting port profile id '%p' with mode %d on physical device '%s' mac '%s' vlan '%d'\n", + vsi, mode, rootifname, macaddr, vlanid); + + /* example syntax: + * lldptool -T -i eth2 -V vdp \ + * mode=<mode>,<mgrid>,<typeid>,<typeidversion>,<uuid> + */ + + if ((virAsprintf(&modestr, "mode=%d,%d,%d,%d,%s,%s,%d", mode, + vsi->u.vsi8021Qbg.managerID, + vsi->u.vsi8021Qbg.typeID, + vsi->u.vsi8021Qbg.typeIDVersion, + uuidstr, + macaddr, + vlanid) < 0 )) { + rc = -1; + goto err_exit; + } + + argv[argc++] = lldptool; + argv[argc++] = "-T"; + argv[argc++] = "-i"; + argv[argc++] = rootifname; + argv[argc++] = "-V"; + argv[argc++] = "vdp"; + argv[argc++] = modestr; + argv[argc ] = NULL; + + VIR_DEBUG("argc = %d\n", argc); + if (argc >= NUM_PARAMS) { + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + _("too many parameters in array")); + rc = 1; + goto err_exit; + } +# undef NUM_PARAMS + + rc = virRun(argv, &status); + status >>= 8; + + VIR_DEBUG("Result from running %s: rc = %d, status = %d\n", + lldptool, rc, status); + +err_exit: + VIR_FREE(modestr); + + if (rc) + return rc; + + return status; + +} + /** * associatePortProfile @@ -791,7 +892,10 @@ associatePortProfileId(const char *linkd break; case VIR_VSI_8021QBG: - rc = -1; + rc = setPortProfileId(linkdev, + mac, + ASSOCIATE, + vsi); break; case VIR_VSI_8021QBH: @@ -829,7 +933,10 @@ disassociatePortProfileId(const char *li break; case VIR_VSI_8021QBG: - rc = -1; + rc = setPortProfileId(linkdev, + mac, + DEASSOCIATE, + vsi); break; case VIR_VSI_8021QBH: -- 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

* Gerhard Stenzel (gstenzel@linux.vnet.ibm.com) wrote:
On Wed, 2010-05-12 at 12:13 -0400, Stefan Berger wrote:
I'd suggest to use this patch as a base for triggering the setup protocol with the 802.1Qb{g|h} switch.
Here is a RFC patch, which demonstrates how libvirt could communicate with lldpad via the lldptool for the 802.1Qbg case. Please note, that there is currently no public available version of lldptool which accepts this command line. This is also work in progress.
Can this be made a library instead of an exec() based cmdline interface? thanks, -chris

On Wed, May 12, 2010 at 09:47:17AM -0700, Chris Wright wrote:
* Gerhard Stenzel (gstenzel@linux.vnet.ibm.com) wrote:
On Wed, 2010-05-12 at 12:13 -0400, Stefan Berger wrote:
I'd suggest to use this patch as a base for triggering the setup protocol with the 802.1Qb{g|h} switch.
Here is a RFC patch, which demonstrates how libvirt could communicate with lldpad via the lldptool for the 802.1Qbg case. Please note, that there is currently no public available version of lldptool which accepts this command line. This is also work in progress.
Can this be made a library instead of an exec() based cmdline interface?
Hum, actually from a libvirt deployment POV, depending on an unstable library is way worse than depending on a command line interface. I.e. the library would make sense only if we had some serious garantee of stabilities, API/ABI garantees, etc ... In the absence of someone firmly commiting to this, a CLI is less dangerous. So at least in a first step an exec() based interface sounds the right approach to me. Daniel -- Daniel Veillard | libxml Gnome XML XSLT toolkit http://xmlsoft.org/ daniel@veillard.com | Rpmfind RPM search engine http://rpmfind.net/ http://veillard.com/ | virtualization library http://libvirt.org/

* Daniel Veillard (veillard@redhat.com) wrote:
On Wed, May 12, 2010 at 09:47:17AM -0700, Chris Wright wrote:
* Gerhard Stenzel (gstenzel@linux.vnet.ibm.com) wrote:
On Wed, 2010-05-12 at 12:13 -0400, Stefan Berger wrote:
I'd suggest to use this patch as a base for triggering the setup protocol with the 802.1Qb{g|h} switch.
Here is a RFC patch, which demonstrates how libvirt could communicate with lldpad via the lldptool for the 802.1Qbg case. Please note, that there is currently no public available version of lldptool which accepts this command line. This is also work in progress.
Can this be made a library instead of an exec() based cmdline interface?
Hum, actually from a libvirt deployment POV, depending on an unstable library is way worse than depending on a command line interface. I.e. the library would make sense only if we had some serious garantee of stabilities, API/ABI garantees, etc ... In the absence of someone firmly commiting to this, a CLI is less dangerous. So at least in a first step an exec() based interface sounds the right approach to me.
Fair enough (you're in way better position to see the implications). I know other bits had moved to library interfaces, so thought I'd make the suggestion. Main thing that is worth pointing out is this is moving away from a single netlink based message interface, and towards a messaging interface per type (VNLink, Qbg...) thanks, -chris

Gerhard Stenzel <gstenzel@linux.vnet.ibm.com> wrote on 05/12/2010 12:34:09 PM:
On Wed, 2010-05-12 at 12:13 -0400, Stefan Berger wrote:
I'd suggest to use this patch as a base for triggering the setup protocol with the 802.1Qb{g|h} switch.
Here is a RFC patch, which demonstrates how libvirt could communicate with lldpad via the lldptool for the 802.1Qbg case. Please note, that there is currently no public available version of lldptool which accepts this command line. This is also work in progress.
[...]
+ virFormatMacAddr(mac, macaddr); + + rc = ifaceGetRootIface(-1, linkdev, rootifname); + if (rc != 0) { + macvtapError(VIR_ERR_INTERNAL_ERROR, + _("cannot get root interface for %s"), + linkdev); + return rc; + } + VIR_DEBUG("root iface of %s is %s\n", linkdev, rootifname); + + ifaceGetVlanID(linkdev, &vlanid); + VIR_DEBUG("vlan id of %s is %d\n", linkdev, vlanid);
I am wondering whether it would not be 'more general' to have lldpad determine the root interface and handling bonding interfaces on the way rather than libvirt trying to determine that here. So we would just pass the linkdev or the even the macvtap to lldptool. The same would be true for finding the VLAN ID. For both the code I posted here on libvirt mailing list could be used in lldpad. For now I won't check it in. Stefan

* Stefan Berger (stefanb@us.ibm.com) wrote:
Gerhard Stenzel <gstenzel@linux.vnet.ibm.com> wrote on 05/12/2010 12:34:09 PM:
On Wed, 2010-05-12 at 12:13 -0400, Stefan Berger wrote:
I'd suggest to use this patch as a base for triggering the setup protocol with the 802.1Qb{g|h} switch.
Here is a RFC patch, which demonstrates how libvirt could communicate with lldpad via the lldptool for the 802.1Qbg case. Please note, that there is currently no public available version of lldptool which accepts this command line. This is also work in progress.
[...]
+ virFormatMacAddr(mac, macaddr); + + rc = ifaceGetRootIface(-1, linkdev, rootifname); + if (rc != 0) { + macvtapError(VIR_ERR_INTERNAL_ERROR, + _("cannot get root interface for %s"), + linkdev); + return rc; + } + VIR_DEBUG("root iface of %s is %s\n", linkdev, rootifname); + + ifaceGetVlanID(linkdev, &vlanid); + VIR_DEBUG("vlan id of %s is %d\n", linkdev, vlanid);
I am wondering whether it would not be 'more general' to have lldpad determine the root interface and handling bonding interfaces on the way rather than libvirt trying to determine that here.
I think so, esp. since we will need to have bonding details in lldpad to handle some active/backup failover cases.
So we would just pass the linkdev or the even the macvtap to lldptool. The same would be true for finding the VLAN ID. For both the code I posted here on libvirt mailing list could be used in lldpad. For now I won't check it in.

On Wed, 2010-05-12 at 13:21 -0400, Stefan Berger wrote:
I am wondering whether it would not be 'more general' to have lldpad determine the root interface and handling bonding interfaces on the way rather than libvirt trying to determine that here. So we would just pass the linkdev or the even the macvtap to lldptool. The same would be true for finding the VLAN ID. For both the code I posted here on libvirt mailing list could be used in lldpad. For now I won't check it in.
If that is the preferred way ... fine with me. -- 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

On Wed, 2010-05-12 at 21:08 +0200, Gerhard Stenzel wrote:
I am wondering whether it would not be 'more general' to have lldpad determine the root interface and handling bonding interfaces on the way rather than libvirt trying to determine that here. So we would just pass
On Wed, 2010-05-12 at 13:21 -0400, Stefan Berger wrote: the
linkdev or the even the macvtap to lldptool. The same would be true for finding the VLAN ID. For both the code I posted here on libvirt mailing list could be used in lldpad. For now I won't check it in.
If that is the preferred way ... fine with me.
Just in case, here is a version taking care of that: Here is a RFC patch, which demonstrates how libvirt could communicate with lldpad via the lldptool for the 802.1Qbg case. Please note, that there is currently no public available version of lldptool which accepts this command line. This is also work in progress. Signed-off-by: Gerhard Stenzel <gerhard.stenzel@de.ibm.com> Index: libvirt/src/util/macvtap.c =================================================================== --- libvirt.orig/src/util/macvtap.c +++ libvirt/src/util/macvtap.c @@ -750,6 +750,92 @@ configMacvtapTap(int tapfd, int vnet_hdr return 0; } +# define ASSOCIATE 0x02 +# define DEASSOCIATE 0x03 +# define LLDPTOOL_NAME "lldptool" + +static int +setPortProfileId(const char *linkdev, + const unsigned char *mac, + int mode, + const virVSIProfileDefPtr vsi) +{ + char macaddr[VIR_MAC_STRING_BUFLEN]; + char uuidstr[VIR_UUID_STRING_BUFLEN]; + static char *lldptool; + char *modestr = NULL; + int rc; + int status = 0; +# define NUM_PARAMS 8 + const char *argv[NUM_PARAMS] = {NULL, }; + int argc = 0; + + virFormatMacAddr(mac, macaddr); + + if (lldptool == NULL) { + lldptool = virFindFileInPath(LLDPTOOL_NAME); + if (lldptool == NULL) { + macvtapError(VIR_ERR_INTERNAL_ERROR, + _("unable to find %s"), LLDPTOOL_NAME); + return -1; + } + } + + virFormatMacAddr(mac, macaddr); + virUUIDFormat(vsi->u.vsi8021Qbg.instanceID, uuidstr); + + VIR_DEBUG("setting typeid '%d' with mode '%d' on link '%s' mac '% s'\n", + vsi->u.vsi8021Qbg.typeID, mode, linkdev, macaddr); + + /* example syntax: + * lldptool -T -i eth2 -V vdp \ + * mode=<mode>,<mgrid>,<typeid>,<typeidversion>,<uuid> + */ + + if ((virAsprintf(&modestr, "mode=%d,%d,%d,%d,%s,%s", mode, + vsi->u.vsi8021Qbg.managerID, + vsi->u.vsi8021Qbg.typeID, + vsi->u.vsi8021Qbg.typeIDVersion, + uuidstr, + macaddr) < 0 )) { + rc = -1; + goto err_exit; + } + + argv[argc++] = lldptool; + argv[argc++] = "-T"; + argv[argc++] = "-i"; + argv[argc++] = linkdev; + argv[argc++] = "-V"; + argv[argc++] = "vdp"; + argv[argc++] = modestr; + argv[argc ] = NULL; + + VIR_DEBUG("argc = %d\n", argc); + if (argc >= NUM_PARAMS) { + macvtapError(VIR_ERR_INTERNAL_ERROR, "%s", + _("too many parameters in array")); + rc = 1; + goto err_exit; + } +# undef NUM_PARAMS + + rc = virRun(argv, &status); + status >>= 8; + + VIR_DEBUG("Result from running %s: rc = %d, status = %d\n", + lldptool, rc, status); + +err_exit: + VIR_FREE(modestr); + + if (rc) + return rc; + + return status; + +} + /** * associatePortProfile @@ -791,7 +877,10 @@ associatePortProfileId(const char *linkd break; case VIR_VSI_8021QBG: - rc = -1; + rc = setPortProfileId(linkdev, + mac, + ASSOCIATE, + vsi); break; case VIR_VSI_8021QBH: @@ -829,7 +918,10 @@ disassociatePortProfileId(const char *li break; case VIR_VSI_8021QBG: - rc = -1; + rc = setPortProfileId(linkdev, + mac, + DEASSOCIATE, + vsi); break; case VIR_VSI_8021QBH: -- 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
participants (5)
-
Chris Wright
-
Daniel P. Berrange
-
Daniel Veillard
-
Gerhard Stenzel
-
Stefan Berger