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

[Wayne Xia]

于 2011-10-27 23:07, Chip Vincent 写道:
>
>
> On 10/26/2011 06:42 AM, Gareth S. Bestor wrote:
>> # HG changeset patch
>> # User Gareth S. Bestor<bestor at 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
>
the code seems not that nice because code was not devided for CIM model
and hos command. Later I would post some code in Libxkutil for
executing commands on host, which is also needed for VLAN.

>>
>> Signed-off-by: Gareth Bestor<bestor at 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!
>
an example of text used here would be good to say what it is doing.
>> 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 at 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.
>


-- 
Best Regards

Wayne Xia
mail:xiawenc at linux.vnet.ibm.com
tel:86-010-82450803