Re: [Libvirt-cim] [PATCH] Patch to add network qos (bandwidth) support for KVM guests using tc

On 10/26/2011 06:42 AM, Gareth S. Bestor wrote: > # HG changeset patch > # User Gareth S. Bestor<bestor(a)us.ibm.com> > # Date 1319625716 25200 > # Node ID 862bcd60ae449932205b15ba42b37e06e6834de4 > # Parent 2714b9a5e842cc1e8c2ae182c2adef083cc1ef1a hg? Well, since not everyone is aware we've moved to git... > Patch to add network qos (bandwidth) support for KVM guests using tc nit: good to have a space here. > This patch adds network qos support into libvirt-cim for setting network > bandwidth limits on KVM guests' MAC addrs, using existing tc (traffic > control) > host support. Patch is a bit hack-ish because it is mostly for older > versions > of libvirt which do not have any qos support in libvirt itself. I am > finishing > a new, and cleaner, patch to libvirt-cim that will exploit the native > support > for qos present in latest libvirt versions. > Patch is a bit ugly because (1) it must manage all the qos data > associated with > KVM guest's network device completely *outside* of libvirt (since no > support > and existing mgmt data available to handle this in older libvirts). > And (2) because the tc commands themselves needed to query, retreive, > add and > delete tc bandwidth setting for a MAC addr are themselves quite messy > scripts. > This patch assume all the necessary tc performance classes have been > setup > externally. > Because it must screen-scrape tc command output to get > performance class and associated bandwith values, this patch will be very > sensitive to change in output format of tc commands. It has been tested > successfully on RHEL6.1. > I dont not recommend exploiting this new feature unless you know > what you are doing... nit: some of these lines appear >80

> > Signed-off-by: Gareth Bestor<bestor(a)us.ibm.com> > > diff -r 2714b9a5e842 -r 862bcd60ae44 src/Virt_RASD.c > --- a/src/Virt_RASD.c Mon Oct 03 02:51:26 2011 -0700 > +++ b/src/Virt_RASD.c Wed Oct 26 03:41:56 2011 -0700 > @@ -39,6 +39,16 @@ > #include "svpc_types.h" > #include "Virt_Device.h" > > +/* Network QoS support */ > +#define QOSCMD_MAC2BANDWIDTH "_ROOT=$(tc class show dev %s | awk > '($4==\"root\")\ > +{print $3}')\n _ID=$(tc filter show dev %s | awk 'BEGIN > {RS=\"\\nfilter\"} (NR>2)\ > +{m1=substr($24,1,2);m2=substr($24,3,2);m3=substr($24,5,2);m4=substr($24,7,2);\ > > +m5=substr($20,1,2);m6=substr($20,3,2);printf(\"%%s:%%s:%%s:%%s:%%s:%%s %%s\\n\",\ > > +m1,m2,m3,m4,m5,m6,$18)}' | awk -v mm=%s '($1==mm){print $2}')\n \ > +if [[ -n \"$_ID\" ]]; then\n tc class show dev %s | awk -v rr=$_ROOT > -v id=$_ID \ > +'($4==\"parent\"&& $5==rr&& $3==id){print \ > +substr($13,1,(index($13,\"Kbit\")-1))}'\n fi\n" > + It would be great to see a better comment for the above, but I have no idea what you might say to make that clearer. Since this patch has a limited life time, fine as-is. > const static CMPIBroker *_BROKER; > > static struct virt_device *_find_dev(struct virt_device *list, > @@ -449,7 +459,11 @@ > const struct virt_device *dev, > CMPIInstance *inst) > { > + FILE *pipe = NULL; > + char *cmd = NULL; > + uint64_t val = 0; > CMPIStatus s = {CMPI_RC_OK, NULL}; > + int i; > > CMSetProperty(inst, > "NetworkType", > @@ -467,6 +481,33 @@ > (CMPIValue *)dev->dev.net.source, > CMPI_chars); > > + /* Network QoS support */ > + if ((dev->dev.net.mac != NULL)&& (dev->dev.net.source != NULL)) { > + /* Get tc performance class bandwidth for this MAC addr */ > + i = asprintf(&cmd, QOSCMD_MAC2BANDWIDTH, dev->dev.net.source, > + dev->dev.net.source, > + dev->dev.net.mac, > + dev->dev.net.source); > + if (i == -1) > + goto out; > + > + if ((pipe = popen(cmd, "r")) != NULL) { > + if (fscanf(pipe, "%u", (unsigned int *)&val) == 1) { > + CU_DEBUG("pipe read. val = %d", val); > + > + CMSetProperty(inst, > + "Reservation", > + (CMPIValue *)&val, CMPI_uint64); > + CMSetProperty(inst, > + "AllocationUnits", > + (CMPIValue *)"KiloBits per Second", > + CMPI_chars); > + } > + pclose(pipe); > + } > + free(cmd); > + } > + > if ((dev->dev.net.source != NULL)&& > (STREQ(dev->dev.net.type, "direct"))) > CMSetProperty(inst, > @@ -498,6 +539,7 @@ > (CMPIValue *)dev->dev.net.poolid, > CMPI_chars); > > +out: > return s; > } > > diff -r 2714b9a5e842 -r 862bcd60ae44 > src/Virt_SettingsDefineCapabilities.c > --- a/src/Virt_SettingsDefineCapabilities.c Mon Oct 03 02:51:26 2011 > -0700 > +++ b/src/Virt_SettingsDefineCapabilities.c Wed Oct 26 03:41:56 2011 > -0700 > @@ -73,6 +73,11 @@ > #define POOL_RASD 1 > #define NEW_VOL_RASD 2 > > +/* QoS Network support */ > +#define QOSCMD_LISTCLASSES "_ROOT=$(tc class show dev %s | awk > '($4==\"root\")\ > +{print $3}')\n tc class show dev %s | awk -v rr=$_ROOT \ > +'($4==\"parent\"&& $5==rr){print $3\" \"$13}'\n" > + > static bool system_has_vt(virConnectPtr conn) > { > char *caps = NULL; > @@ -726,7 +731,7 @@ > } > > static CMPIStatus set_net_pool_props(const CMPIObjectPath *ref, > - const char *id, > + char *id, > uint16_t pool_type, > struct inst_list *list) > { > @@ -738,6 +743,7 @@ > const char *ip_stop = "192.168.122.254"; > int dev_count; > int i; > + char *tmp_str = NULL; > > /* Isolated network pools don't have a forward device */ > if (pool_type == NETPOOL_FORWARD_NONE) > @@ -750,7 +756,33 @@ > if ((inst == NULL) || (s.rc != CMPI_RC_OK)) > goto out; > > - CMSetProperty(inst, "InstanceID", (CMPIValue *)id, CMPI_chars); > + > + tmp_str = strtok(id, " "); > + if (tmp_str == NULL) { > + CU_DEBUG("Cannot set InstanceID"); > + goto out; > + } > + > + CU_DEBUG("InstanceID = %s", tmp_str); > + > + CMSetProperty(inst, "InstanceID", (CMPIValue *)tmp_str, > + CMPI_chars); > + tmp_str = strtok('\0', " "); > + if (tmp_str == NULL) { > + CU_DEBUG("Cannot set Reservation"); > + goto out; > + } > + > + CU_DEBUG("Reservation = %s", tmp_str); > + > + uint64_t val = atoi(tmp_str); > + > + CMSetProperty(inst, "Reservation", > + (CMPIValue *)&val, CMPI_uint64); > + > + CMSetProperty(inst, "AllocationUnits", > + (CMPIValue *)"Kilobits per Second", > + CMPI_chars); > > CMSetProperty(inst, "Address", > (CMPIValue *)addr, CMPI_chars); > @@ -779,11 +811,112 @@ > return s; > } > > +static char * get_bridge_name(virConnectPtr conn, const char *name) > +{ > + char *bridge = NULL; > + virNetworkPtr network = NULL; > + > + if (++name == NULL) > + goto out; > + > + CU_DEBUG("looking for network `%s'", name); > + network = virNetworkLookupByName(conn, name); > + if (network == NULL) { > + CU_DEBUG("Could not find network"); > + goto out; > + } > + > + bridge = virNetworkGetBridgeName(network); > + if (bridge == NULL) { > + CU_DEBUG("Could not find bridge"); > + } > + > + virNetworkFree(network); > + > + out: > + return bridge; > +} > + > +/* QoS Network support */ > +static CMPIStatus qos_hack( > + const CMPIObjectPath *ref, > + struct inst_list *list) > +{ > + CMPIStatus s = {CMPI_RC_OK, NULL}; > + virConnectPtr conn = NULL; > + FILE *pipe = NULL; > + char buffer[1024]; > + char *bridge = NULL; > + char *cmd = NULL; > + const char *val = NULL; > + int i; > + > + conn = connect_by_classname(_BROKER, CLASSNAME(ref),&s); > + if (s.rc != CMPI_RC_OK) { > + cu_statusf(_BROKER,&s, > + CMPI_RC_ERR_FAILED, > + "Could not get connection"); > + goto out; > + } > + > + if (cu_get_str_path(ref, "InstanceID",&val) != CMPI_RC_OK) { > + cu_statusf(_BROKER,&s, > + CMPI_RC_ERR_FAILED, > + "Could not get InstanceID"); > + goto out; > + } > + > + CU_DEBUG("InstanceID is %s", val); > + > + bridge = get_bridge_name(conn, strstr(val, "/")); > + if (bridge == NULL) > + goto out; > + > + i = asprintf(&cmd, QOSCMD_LISTCLASSES, bridge, bridge); > + if (i == -1) > + goto out; > + > + CU_DEBUG("qos_hack() cmd = %s", cmd); > + > + if ((pipe = popen(cmd, "r")) != NULL) { > + CU_DEBUG("pipe open"); > + while (fgets(buffer, sizeof(buffer), pipe) != NULL) { > + char *name = NULL; > + char *p = strstr(buffer, "Kbit"); > + > + /* trim string */ > + if (p) > + *p = '\0'; > + > + CU_DEBUG("qos hack buffer = %s", buffer); > + > + i = asprintf(&name, "Point/%s", buffer); > + if (i != -1) { > + s = set_net_pool_props(ref, name, > + NETPOOL_FORWARD_NONE, list); > + > + free(name); > + } > + } > + > + pclose(pipe); > + CU_DEBUG("pipe close"); > + } > + > + out: > + free(cmd); > + free(bridge); > + virConnectClose(conn); > + > + return s; > +} > + > + > static CMPIStatus net_pool_template(const CMPIObjectPath *ref, > int template_type, > struct inst_list *list) > { > - const char *id; > + char *id; > CMPIStatus s = {CMPI_RC_OK, NULL}; > int type[3] = {NETPOOL_FORWARD_NONE, > NETPOOL_FORWARD_NAT, > @@ -1949,7 +2082,10 @@ > goto out; > } > } > - > + > + if (type == CIM_RES_TYPE_NET) > + s = qos_hack(ref, list); > + > out: > return s; > } > @@ -2055,7 +2191,7 @@ > { > CMPIInstance *ref_inst = NULL; > uint16_t valuerole = SDC_ROLE_SUPPORTED; > - uint16_t valuerange; > + uint16_t valuerange = SDC_RANGE_POINT; > uint16_t ppolicy = SDC_POLICY_INDEPENDENT; > const char *iid = NULL; > > diff -r 2714b9a5e842 -r 862bcd60ae44 > src/Virt_VirtualSystemManagementService.c > --- a/src/Virt_VirtualSystemManagementService.c Mon Oct 03 02:51:26 > 2011 -0700 > +++ b/src/Virt_VirtualSystemManagementService.c Wed Oct 26 03:41:56 > 2011 -0700 > @@ -68,6 +68,35 @@ > #define RASD_IND_DELETED "ResourceAllocationSettingDataDeletedIndication" > #define RASD_IND_MODIFIED > "ResourceAllocationSettingDataModifiedIndication" > > +/* Network QoS support */ > +#define QOSCMD_BANDWIDTH2ID "_ROOT=$(tc class show dev %s | awk > '($4==\"root\")\ > +{print $3}')\n tc class show dev %s | awk -v rr=$_ROOT -v bw=%uKbit \ > +'($4==\"parent\"&& $5==rr&& $13==bw){print $3}'\n" > + > +#define QOSCMD_RMVM "MAC=%s; ME1=$(echo $MAC | awk -F ':' '{ print > $1$2$3$4 }'); \ > +ME2=$(echo $MAC | awk -F ':' '{ print $5$6 }'); MI1=$(echo $MAC | awk > -F ':' \ > +'{ print $1$2 }'); MI2=$(echo $MAC | awk -F ':' '{ print $3$4$5$6 }'); \ > +HDL=$(tc filter show dev %s | awk 'BEGIN {RS=\"\\nfilter\"} (NR>2)\ > +{m1=substr($24,1,2);m2=substr($24,3,2);m3=substr($24,5,2);m4=substr($24,7,2);\ > > +m5=substr($20,1,2);m6=substr($20,3,2);printf(\"%%s:%%s:%%s:%%s:%%s:%%s %%s\\n\",\ > > +m1,m2,m3,m4,m5,m6,$9)}' | awk -v mm=%s '($1==mm){print $2}'); \ > +U32=\"tc filter del dev %s protocol ip parent 1:0 prio 1 handle $HDL > u32\"; \ > +$U32 match u16 0x0800 0xFFFF at -2 match u16 0x$ME2 0xFFFF at -4 > match u32 \ > +0x$ME1 0xFFFFFFFF at -8 flowid %s; U32=\"tc filter del dev %s > protocol ip parent \ > +ffff: prio 50 u32\"; $U32 match u16 0x0800 0xFFFF at -2 match u32 > 0x$MI2 \ > +0xFFFFFFFF at -12 match u16 0x$MI1 0xFFFF at -14 police rate %uKbit > burst 15k \ > +drop\n" > + > +#define QOSCMD_ADDVM "MAC=%s; ME1=$(echo $MAC | awk -F ':' '{ print > $1$2$3$4 }'); \ > +ME2=$(echo $MAC | awk -F ':' '{ print $5$6 }'); MI1=$(echo $MAC | awk > -F ':' \ > +'{ print $1$2 }'); MI2=$(echo $MAC | awk -F ':' '{ print $3$4$5$6 }'); \ > +U32=\"tc filter add dev %s protocol ip parent 1:0 prio 1 u32\"; \ > +$U32 match u16 0x0800 0xFFFF at -2 match u16 0x$ME2 0xFFFF at -4 > match u32 \ > +0x$ME1 0xFFFFFFFF at -8 flowid %s; U32=\"tc filter add dev %s > protocol ip parent \ > +ffff: prio 50 u32\"; $U32 match u16 0x0800 0xFFFF at -2 match u32 > 0x$MI2 \ > +0xFFFFFFFF at -12 match u16 0x$MI1 0xFFFF at -14 police rate %uKbit > burst 15k \ > +drop\n" > + ouch!

> const static CMPIBroker *_BROKER; > > enum ResourceAction { > @@ -76,6 +105,104 @@ > RESOURCE_MOD, > }; > > +/* Network QoS support */ > +static CMPIStatus add_qos_for_mac(const uint64_t qos, > + const char *mac, > + const char *bridge) > +{ > + CMPIStatus s = {CMPI_RC_OK, NULL}; > + char *cmd = NULL; > + int j; > + FILE *pipe = NULL; > + char id[16] = ""; /* should be adequate to hold short tc class > ids... */ > + > + /* Find tc performance class id which matches requested qos > bandwidth */ > + j = asprintf(&cmd, QOSCMD_BANDWIDTH2ID, bridge, > + bridge, > + (unsigned int)qos); > + if (j == -1) > + goto out; > + CU_DEBUG("add_qos_for_mac(): cmd = %s", cmd); > + > + if ((pipe = popen(cmd, "r")) != NULL) { > + if (fgets(id, sizeof(id), pipe) != NULL) { > + /* Strip off trailing newline */ > + char *p = index(id, '\n'); > + if (p) *p = '\0'; > + } > + pclose(pipe); > + } > + free(cmd); > + CU_DEBUG("qos id = '%s'", id); > + > + /* Add tc performance class id for this MAC addr */ > + j = asprintf(&cmd, QOSCMD_ADDVM, mac, > + bridge, > + id, > + bridge, > + (unsigned int)qos); > + if (j == -1) > + goto out; > + CU_DEBUG("add_qos_for_mac(): cmd = %s", cmd); > + > + if (WEXITSTATUS(system(cmd)) != 0) > + CU_DEBUG("add_qos_for_mac(): qos add failed."); > + > + out: > + free(cmd); > + return s; > +} > + > +/* Network QoS support */ > +static CMPIStatus remove_qos_for_mac(const uint64_t qos, > + const char *mac, > + const char *bridge) > +{ > + CMPIStatus s = {CMPI_RC_OK, NULL}; > + char *cmd = NULL; > + int j; > + FILE *pipe = NULL; > + char id[16] = ""; /* should be adequate to hold short tc class > ids... */ > + > + /* Find tc performance class id which matches requested qos > bandwidth */ > + j = asprintf(&cmd, QOSCMD_BANDWIDTH2ID, bridge, > + bridge, > + (unsigned int)qos); > + if (j == -1) > + goto out; > + CU_DEBUG("remove_qos_for_mac(): cmd = %s", cmd); > + > + if ((pipe = popen(cmd, "r")) != NULL) { > + if (fgets(id, sizeof(id), pipe) != NULL) { > + /* Strip off trailing newline */ > + char *p = index(id, '\n'); > + if (p) *p = '\0'; > + } > + pclose(pipe); > + } > + free(cmd); > + CU_DEBUG("qos id = '%s'", id); > + > + /* Remove tc perf class id for this MAC; ignore errors when none > exists */ > + j = asprintf(&cmd, QOSCMD_RMVM, mac, > + bridge, > + mac, > + bridge, > + id, > + bridge, > + (unsigned int)qos); > + if (j == -1) > + goto out; > + CU_DEBUG("remove_qos_for_mac(): cmd = %s", cmd); > + > + if (WEXITSTATUS(system(cmd)) != 0) > + CU_DEBUG("remove_qos_for_mac(): qos remove failed; ignoring..."); > + > + out: > + free(cmd); > + return s; > +} > + > static CMPIStatus check_uuid_in_use(const CMPIObjectPath *ref, > struct domain *domain) > { > @@ -1518,6 +1645,8 @@ > } else if (type == CIM_RES_TYPE_NET) { > struct virt_device dev; > int ncount = count + domain->dev_net_ct; > + uint64_t qos_val = 0; > + const char *qos_unitstr; > > memset(&dev, 0, sizeof(dev)); > msg = rasd_to_vdev(inst, > @@ -1529,6 +1658,20 @@ > domain->dev_net, > ncount, > &domain->dev_net_ct); > + > + /* Network QoS support */ > + if (((&dev)->dev.net.mac != NULL)&& > + ((&dev)->dev.net.source != NULL)&& > + (cu_get_u64_prop(inst, "Reservation",&qos_val) == CMPI_RC_OK)&& > + (cu_get_str_prop(inst, "AllocationUnits",&qos_unitstr) == CMPI_RC_OK)&& > + STREQ(qos_unitstr,"KiloBits per Second")) { > + remove_qos_for_mac(qos_val, > + (&dev)->dev.net.mac, > + (&dev)->dev.net.source); > + add_qos_for_mac(qos_val, > + (&dev)->dev.net.mac, > + (&dev)->dev.net.source); > + } > } else if (type == CIM_RES_TYPE_GRAPHICS) { > struct virt_device dev; > int gcount = count + domain->dev_graphics_ct; > @@ -2590,10 +2733,28 @@ > (type == CIM_RES_TYPE_INPUT)) > cu_statusf(_BROKER,&s, CMPI_RC_OK, ""); > else { > + uint64_t qos_val = 0; > + const char *qos_unitstr; > + > s = _resource_dynamic(dominfo, > dev, > RESOURCE_MOD, > CLASSNAME(op)); > + > + /* Network QoS support */ > + if ((type == CIM_RES_TYPE_NET)&& > + (dev->dev.net.mac != NULL)&& > + (dev->dev.net.source != NULL)&& > + (cu_get_u64_prop(rasd, "Reservation",&qos_val) == CMPI_RC_OK)&& > + (cu_get_str_prop(rasd, "AllocationUnits",&qos_unitstr) == CMPI_RC_OK)&& > + STREQ(qos_unitstr,"KiloBits per Second")) { > + remove_qos_for_mac(qos_val, > + dev->dev.net.mac, > + dev->dev.net.source); > + s = add_qos_for_mac(qos_val, > + dev->dev.net.mac, > + dev->dev.net.source); > + } > } > break; > } > > _______________________________________________ > Libvirt-cim mailing list > Libvirt-cim(a)redhat.com > https://www.redhat.com/mailman/listinfo/libvirt-cim > +1 and pushed. Gareth, please post some cimtests for this so we can ensure the function performs the same from this version to the future version based on libvirt APIs. Thanks.