[PATCH 0 of 4 v3 RESEND] xl: add support for vif rate limiting

[PATCH 0 of 4 v3 RESEND] xl: add support for vif rate limiting

[Mathieu Gagné]

Hi,

This patch series implements the required plumbering for vif rate limiting in libxl/xl.

The first patch (already applied) fixes trivial indentation issues and introduces no functional changes.

The second patch (already applied) implements dry-run for `xl network-attach` for debugging and testing purposes.

The third patch adds the required plumbering in libxl/xl to add vif rate limiting support. It uses the `rate` option syntax from xm/xend, ensuring full backward compatiblity.

The final patch adds the "check-xl-vif-parse" test script which tests various `rate` syntax.

Changes in v3:
- Move xlu_cfg_init from parse_vif_rate to main_networkattach
- Add xlu_cfg_destroy to main_networkattach
- Use %"PRIuXX" instead of %llu and %lu

Changes in v2:
- Don't default to unlimited in case of overflow/underflow or invalid syntax in rate
- Add error handling
- Remove use of matches in regex which weren't used anyway
- Add docs about the syntax
- Add note in docs explaining that limitations are netback implementation specific
- Modify test cases according to new behavior

--
Mathieu

[PATCH 1 of 4 v3 RESEND] xl: cleanup indentation

[Mathieu Gagné]

Signed-off-by: Mathieu Gagné <mgagne@iweb.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -847,10 +847,10 @@ static void parse_config_data(const char
                 nic->script = strdup(default_vifscript);
             }
 
-	    if (default_bridge) {
-		free(nic->bridge);
-		nic->bridge = strdup(default_bridge);
-	    }
+            if (default_bridge) {
+                free(nic->bridge);
+                nic->bridge = strdup(default_bridge);
+            }
 
             p = strtok(buf2, ",");
             if (!p)
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 2 of 4 v3 RESEND] xl: xl network-attach -N (dry run) option

[Mathieu Gagné]

Add dryrun for testing and debugging purposes.

Signed-off-by: Mathieu Gagné <mgagne@iweb.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>
Acked-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com>

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -4916,6 +4916,16 @@ int main_networkattach(int argc, char **
             return 1;
         }
     }
+
+    if (dryrun_only) {
+        char *json = libxl_device_nic_to_json(ctx, &nic);
+        printf("vif: %s\n", json);
+        free(json);
+        libxl_device_nic_dispose(&nic);
+        if (ferror(stdout) || fflush(stdout)) { perror("stdout"); exit(-1); }
+        return 0;
+    }
+
     if (libxl_device_nic_add(ctx, domid, &nic)) {
         fprintf(stderr, "libxl_device_nic_add failed.\n");
         return 1;
diff --git a/tools/libxl/xl_cmdtable.c b/tools/libxl/xl_cmdtable.c
--- a/tools/libxl/xl_cmdtable.c
+++ b/tools/libxl/xl_cmdtable.c
@@ -288,7 +288,7 @@ struct cmd_spec cmd_table[] = {
       "",
     },
     { "network-attach",
-      &main_networkattach, 0,
+      &main_networkattach, 1,
       "Create a new virtual network device",
       "<Domain> [type=<type>] [mac=<mac>] [bridge=<bridge>] "
       "[ip=<ip>] [script=<script>] [backend=<BackDomain>] [vifname=<name>] "
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 3 of 4 v3 RESEND] xl: add support for vif rate limiting

[Mathieu Gagné]

The `rate` keyword specifies the rate at which the outgoing traffic
will be limited to. The default if this keyword is not specified
is unlimited.

The `rate` keyword supports an optional replenishment interval
parameter for specifying the granularity of credit replenishment.
It determines the frequency at which the vif transmission credit
is replenished. The default interval is 50ms.

For example:

        'rate=10Mb/s'
        'rate=250KB/s'
        'rate=1MB/s@20ms'

Signed-off-by: Mathieu Gagné <mgagne@iweb.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/docs/misc/xl-network-configuration.markdown b/docs/misc/xl-network-configuration.markdown
--- a/docs/misc/xl-network-configuration.markdown
+++ b/docs/misc/xl-network-configuration.markdown
@@ -122,5 +122,34 @@ Specifies the backend domain which this 
 defaults to domain 0. Specifying another domain requires setting up a
 driver domain which is outside the scope of this document.
 
+### rate
+
+Specifies the rate at which the outgoing traffic will be limited to.
+The default if this keyword is not specified is unlimited.
+
+The rate may be specified as "<RATE>/s" or optionally "<RATE>/s@<INTERVAL>".
+
+  * `RATE` is in bytes and can accept suffixes:
+      * GB, MB, KB, B for bytes.
+      * Gb, Mb, Kb, b for bits.
+  * `INTERVAL` is in microseconds and can accept suffixes: ms, us, s.
+    It determines the frequency at which the vif transmission credit
+    is replenished. The default is 50ms.
+
+Vif rate limiting is credit-based. It means that for "1MB/s@20ms", the
+available credit will be equivalent of the traffic you would have done
+at "1MB/s" during 20ms. This will results in a credit of 20,000 bytes
+replenished every 20,000 us.
+
+For example:
+
+        'rate=10Mb/s' -- meaning up to 10 megabits every second
+        'rate=250KB/s' -- meaning up to 250 kilobytes every second
+        'rate=1MB/s@20ms' -- meaning 20,000 bytes in every 20 millisecond period
+
+NOTE: The actual underlying limits of rate limiting are dependent
+on the underlying netback implementation.
+
+
 [oui]: http://en.wikipedia.org/wiki/Organizationally_Unique_Identifier
 [net]: http://wiki.xen.org/wiki/HostConfiguration/Networking
diff --git a/tools/libxl/Makefile b/tools/libxl/Makefile
--- a/tools/libxl/Makefile
+++ b/tools/libxl/Makefile
@@ -61,7 +61,7 @@ LIBXL_OBJS += _libxl_types.o libxl_flask
 AUTOINCS= libxlu_cfg_y.h libxlu_cfg_l.h _libxl_list.h
 AUTOSRCS= libxlu_cfg_y.c libxlu_cfg_l.c
 LIBXLU_OBJS = libxlu_cfg_y.o libxlu_cfg_l.o libxlu_cfg.o \
-	libxlu_disk_l.o libxlu_disk.o libxlu_pci.o
+	libxlu_disk_l.o libxlu_disk.o libxlu_vif.o libxlu_pci.o
 $(LIBXLU_OBJS): CFLAGS += $(CFLAGS_libxenctrl) # For xentoollog.h
 
 CLIENTS = xl testidl
diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c
--- a/tools/libxl/libxl.c
+++ b/tools/libxl/libxl.c
@@ -1854,6 +1854,13 @@ int libxl_device_nic_add(libxl_ctx *ctx,
         flexarray_append(back, libxl__strdup(gc, nic->ip));
     }
 
+    if (nic->rate_interval_usecs > 0) {
+        flexarray_append(back, "rate");
+        flexarray_append(back, libxl__sprintf(gc, "%"PRIu64",%"PRIu32"",
+                            nic->rate_bytes_per_interval,
+                            nic->rate_interval_usecs));
+    }
+
     flexarray_append(back, "bridge");
     flexarray_append(back, libxl__strdup(gc, nic->bridge));
     flexarray_append(back, "handle");
diff --git a/tools/libxl/libxl_types.idl b/tools/libxl/libxl_types.idl
--- a/tools/libxl/libxl_types.idl
+++ b/tools/libxl/libxl_types.idl
@@ -343,6 +343,8 @@ libxl_device_nic = Struct("device_nic", 
     ("ifname", string),
     ("script", string),
     ("nictype", libxl_nic_type),
+    ("rate_bytes_per_interval", uint64),
+    ("rate_interval_usecs", uint32),
     ])
 
 libxl_device_pci = Struct("device_pci", [
diff --git a/tools/libxl/libxlu_internal.h b/tools/libxl/libxlu_internal.h
--- a/tools/libxl/libxlu_internal.h
+++ b/tools/libxl/libxlu_internal.h
@@ -17,9 +17,11 @@
 #define LIBXLU_INTERNAL_H
 
 #include <stdio.h>
+#include <stdlib.h>
 #include <errno.h>
 #include <string.h>
 #include <assert.h>
+#include <regex.h>
 
 #define XLU_ConfigList XLU_ConfigSetting
 
diff --git a/tools/libxl/libxlu_vif.c b/tools/libxl/libxlu_vif.c
new file mode 100644
--- /dev/null
+++ b/tools/libxl/libxlu_vif.c
@@ -0,0 +1,141 @@
+#include "libxl_osdeps.h" /* must come before any other headers */
+#include "libxlu_internal.h"
+
+static const char *vif_bytes_per_sec_re = "^[0-9]+[GMK]?[Bb]/s$";
+static const char *vif_internal_usec_re = "^[0-9]+[mu]?s?$";
+
+static void xlu__vif_err(XLU_Config *cfg, const char *msg, const char *rate) {
+    fprintf(cfg->report,
+            "%s: config parsing error in vif: %s in `%s'\n",
+            cfg->filename, msg, rate);
+}
+
+static int vif_parse_rate_bytes_per_sec(XLU_Config *cfg, const char *bytes,
+                                        uint64_t *bytes_per_sec)
+{
+    regex_t rec;
+    uint64_t tmp = 0;
+    const char *p;
+    int rc = 0;
+
+    regcomp(&rec, vif_bytes_per_sec_re, REG_EXTENDED|REG_NOSUB);
+    if (regexec(&rec, bytes, 0, NULL, 0)) {
+        xlu__vif_err(cfg, "invalid rate", bytes);
+        rc = EINVAL;
+        goto out;
+    }
+
+    p = bytes;
+    tmp = strtoull(p, (char**)&p, 0);
+    if (tmp == 0 || tmp > UINT32_MAX || errno == ERANGE) {
+        xlu__vif_err(cfg, "rate overflow", bytes);
+        rc = EOVERFLOW;
+        goto out;
+    }
+
+    if (*p == 'G')
+       tmp *= 1000 * 1000 * 1000;
+    else if (*p == 'M')
+       tmp *= 1000 * 1000;
+    else if (*p == 'K')
+       tmp *= 1000;
+    if (*p == 'b' || *(p+1) == 'b')
+       tmp /= 8;
+
+    *bytes_per_sec = tmp;
+
+out:
+    regfree(&rec);
+    return rc;
+}
+
+static int vif_parse_rate_interval_usecs(XLU_Config *cfg, const char *interval,
+                                         uint32_t *interval_usecs)
+{
+    regex_t rec;
+    uint64_t tmp = 0;
+    const char *p;
+    int rc = 0;
+
+    regcomp(&rec, vif_internal_usec_re, REG_EXTENDED|REG_NOSUB);
+    if (regexec(&rec, interval, 0, NULL, 0)) {
+        xlu__vif_err(cfg, "invalid replenishment interval", interval);
+        rc = EINVAL;
+        goto out;
+    }
+
+    p = interval;
+    tmp = strtoull(p, (char**)&p, 0);
+    if (tmp == 0 || tmp > UINT32_MAX || errno == ERANGE) {
+        xlu__vif_err(cfg, "replenishment interval overflow", interval);
+        rc = EOVERFLOW;
+        goto out;
+    }
+
+    if (*p == 's' || *p == '\0')
+        tmp *= 1000 * 1000;
+    else if (*p == 'm')
+        tmp *= 1000;
+
+    if (tmp > UINT32_MAX) {
+        xlu__vif_err(cfg, "replenishment interval overflow", interval);
+        rc = EOVERFLOW;
+        goto out;
+    }
+
+    *interval_usecs = (uint32_t) tmp;
+
+out:
+    regfree(&rec);
+    return rc;
+}
+
+int xlu_vif_parse_rate(XLU_Config *cfg, const char *rate, libxl_device_nic *nic)
+{
+    uint64_t bytes_per_sec = 0;
+    uint64_t bytes_per_interval = 0;
+    uint32_t interval_usecs = 50000UL; /* Default to 50ms */
+    char *ratetok, *tmprate;
+    int rc = 0;
+
+    tmprate = strdup(rate);
+    if (!strcmp(tmprate,"")) {
+        xlu__vif_err(cfg, "no rate specified", rate);
+        rc = EINVAL;
+        goto out;
+    }
+
+    ratetok = strtok(tmprate, "@");
+    rc = vif_parse_rate_bytes_per_sec(cfg, ratetok, &bytes_per_sec);
+    if (rc) goto out;
+
+    ratetok = strtok(NULL, "@");
+    if (ratetok != NULL) {
+        rc = vif_parse_rate_interval_usecs(cfg, ratetok, &interval_usecs);
+        if (rc) goto out;
+    }
+
+    if (interval_usecs != 0 && (bytes_per_sec > (UINT64_MAX / interval_usecs))) {
+        xlu__vif_err(cfg, "rate overflow", rate);
+        rc = EOVERFLOW;
+        goto out;
+    }
+
+    bytes_per_interval =
+        (((uint64_t) bytes_per_sec * (uint64_t) interval_usecs) / 1000000UL);
+
+    nic->rate_interval_usecs = interval_usecs;
+    nic->rate_bytes_per_interval = bytes_per_interval;
+
+out:
+    free(tmprate);
+    return rc;
+}
+
+/*
+ * Local variables:
+ * mode: C
+ * c-basic-offset: 4
+ * indent-tabs-mode: nil
+ * End:
+ */
diff --git a/tools/libxl/libxlutil.h b/tools/libxl/libxlutil.h
--- a/tools/libxl/libxlutil.h
+++ b/tools/libxl/libxlutil.h
@@ -94,6 +94,13 @@ int xlu_disk_parse(XLU_Config *cfg, int 
 int xlu_pci_parse_bdf(XLU_Config *cfg, libxl_device_pci *pcidev, const char *str);
 
 
+/*
+ * Vif rate parsing.
+ */
+
+int xlu_vif_parse_rate(XLU_Config *cfg, const char *rate,
+                       libxl_device_nic *nic);
+
 #endif /* LIBXLUTIL_H */
 
 /*
diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -403,6 +403,19 @@ static void parse_disk_config(XLU_Config
     parse_disk_config_multistring(config, 1, &spec, disk);
 }
 
+static void parse_vif_rate(XLU_Config **config, const char *rate,
+                           libxl_device_nic *nic)
+{
+    int e;
+
+    e = xlu_vif_parse_rate(*config, rate, nic);
+    if (e == EINVAL || e == EOVERFLOW) exit(-1);
+    if (e) {
+        fprintf(stderr,"xlu_vif_parse_rate failed: %s\n",strerror(errno));
+        exit(-1);
+    }
+}
+
 static void split_string_into_string_list(const char *str,
                                           const char *delim,
                                           libxl_string_list *psl)
@@ -906,7 +919,7 @@ static void parse_config_data(const char
                         nic->backend_domid = 0;
                     }
                 } else if (!strcmp(p, "rate")) {
-                    fprintf(stderr, "the rate parameter for vifs is currently not supported\n");
+                    parse_vif_rate(&config, (p2 + 1), nic);
                 } else if (!strcmp(p, "accel")) {
                     fprintf(stderr, "the accel parameter for vifs is currently not supported\n");
                 }
@@ -4855,6 +4868,7 @@ int main_networkattach(int argc, char **
 {
     int opt;
     libxl_device_nic nic;
+    XLU_Config *config = 0;
     char *endptr, *oparg;
     const char *tok;
     int i;
@@ -4872,6 +4886,13 @@ int main_networkattach(int argc, char **
         fprintf(stderr, "%s is an invalid domain identifier\n", argv[optind]);
         return 1;
     }
+
+    config= xlu_cfg_init(stderr, "command line");
+    if (!config) {
+        fprintf(stderr, "Failed to allocate for configuration\n");
+        return 1;
+    }
+
     libxl_device_nic_init(&nic);
     for (argv += optind+1, argc -= optind+1; argc > 0; ++argv, --argc) {
         if (MATCH_OPTION("type", *argv, oparg)) {
@@ -4910,6 +4931,7 @@ int main_networkattach(int argc, char **
         } else if (MATCH_OPTION("model", *argv, oparg)) {
             replace_string(&nic.model, oparg);
         } else if (MATCH_OPTION("rate", *argv, oparg)) {
+            parse_vif_rate(&config, oparg, &nic);
         } else if (MATCH_OPTION("accel", *argv, oparg)) {
         } else {
             fprintf(stderr, "unrecognized argument `%s'\n", *argv);
@@ -4931,6 +4953,7 @@ int main_networkattach(int argc, char **
         return 1;
     }
     libxl_device_nic_dispose(&nic);
+    xlu_cfg_destroy(config);
     return 0;
 }
 
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[PATCH 4 of 4 v3 RESEND] xl: add "check-xl-vif-parse" test script

[Mathieu Gagné]

This test script runs "xl -N network-attach 0 <foobar>" against various
rate syntax and checks that the output is as expected.

Signed-off-by: Mathieu Gagné <mgagne@iweb.com>
Acked-by: Ian Campbell <ian.campbell@citrix.com>

diff --git a/tools/libxl/check-xl-vif-parse b/tools/libxl/check-xl-vif-parse
new file mode 100755
--- /dev/null
+++ b/tools/libxl/check-xl-vif-parse
@@ -0,0 +1,209 @@
+#!/bin/bash
+
+set -e
+
+if [ -x ./xl ] ; then
+    export LD_LIBRARY_PATH=.
+    XL=./xl
+else
+    XL=xl
+fi
+
+fprefix=tmp.check-xl-vif-parse
+
+expected () {
+    cat >$fprefix.expected
+}
+
+failures=0
+
+one () {
+    expected_rc=$1; shift
+    printf "test case %s...\n" "$*"
+    set +e
+    ${XL} -N network-attach 0 "$@" </dev/null >$fprefix.actual 2>/dev/null
+    actual_rc=$?
+    diff -u $fprefix.expected $fprefix.actual
+    diff_rc=$?
+    set -e
+    if [ $actual_rc != $expected_rc ] || [ $diff_rc != 0 ]; then
+        echo >&2 "test case \`$*' failed ($actual_rc $diff_rc)"
+        failures=$(( $failures + 1 ))
+    fi
+}
+
+complete () {
+    if [ "$failures" = 0 ]; then
+        echo all ok.; exit 0
+    else
+        echo "$failures tests failed."; exit 1
+    fi
+}
+
+e=255
+
+
+#---------- test data ----------
+
+# test invalid vif config
+expected </dev/null
+one 1 foo
+
+# test invalid rate units
+expected </dev/null
+one $e rate=foo
+one $e rate=foo
+one $e rate=10MB
+one $e rate=10MB/m
+one $e rate=10ZB
+one $e rate=10ZB/s
+one $e rate=10ZB/m
+
+# test b/s and B/s rate units
+expected <<END
+vif: {
+    "backend_domid": 0,
+    "devid": 0,
+    "mtu": 0,
+    "model": null,
+    "mac": "00:00:00:00:00:00",
+    "ip": null,
+    "bridge": null,
+    "ifname": null,
+    "script": null,
+    "nictype": null,
+    "rate_bytes_per_interval": 100000,
+    "rate_interval_usecs": 50000
+}
+
+END
+
+one 0 rate=16000000b/s
+one 0 rate=16000000b/s@50ms
+one 0 rate=2000000B/s
+one 0 rate=2000000B/s@50ms
+
+# test Kb/s and KB/s rate units
+expected <<END
+vif: {
+    "backend_domid": 0,
+    "devid": 0,
+    "mtu": 0,
+    "model": null,
+    "mac": "00:00:00:00:00:00",
+    "ip": null,
+    "bridge": null,
+    "ifname": null,
+    "script": null,
+    "nictype": null,
+    "rate_bytes_per_interval": 100,
+    "rate_interval_usecs": 50000
+}
+
+END
+one 0 rate=16Kb/s
+one 0 rate=16Kb/s@50ms
+one 0 rate=2KB/s
+one 0 rate=2KB/s@50ms
+
+# test Mb/s and MB/s rate units
+expected <<END
+vif: {
+    "backend_domid": 0,
+    "devid": 0,
+    "mtu": 0,
+    "model": null,
+    "mac": "00:00:00:00:00:00",
+    "ip": null,
+    "bridge": null,
+    "ifname": null,
+    "script": null,
+    "nictype": null,
+    "rate_bytes_per_interval": 100000,
+    "rate_interval_usecs": 50000
+}
+
+END
+one 0 rate=16Mb/s
+one 0 rate=16Mb/s@50ms
+one 0 rate=2MB/s
+one 0 rate=2MB/s@50ms
+
+# test Gb/s and GB/s rate units
+expected <<END
+vif: {
+    "backend_domid": 0,
+    "devid": 0,
+    "mtu": 0,
+    "model": null,
+    "mac": "00:00:00:00:00:00",
+    "ip": null,
+    "bridge": null,
+    "ifname": null,
+    "script": null,
+    "nictype": null,
+    "rate_bytes_per_interval": 50000000,
+    "rate_interval_usecs": 50000
+}
+
+END
+one 0 rate=8Gb/s
+one 0 rate=8Gb/s@50ms
+one 0 rate=1GB/s
+one 0 rate=1GB/s@50ms
+
+# test rate overflow
+expected </dev/null
+one $e rate=4294967296b/s
+one $e rate=4294967296Kb/s
+one $e rate=4294967296Mb/s
+one $e rate=4294967296Gb/s
+
+# test rate underflow
+expected </dev/null
+one $e rate=0B/s
+
+# test invalid replenishment interval
+expected </dev/null
+one $e rate=10Mb/s@foo
+one $e rate=10Mb/s@10h
+one $e rate=10MB/s@foo
+one $e rate=10MB/s@10h
+
+# test replenishment interval in seconds
+expected <<END
+vif: {
+    "backend_domid": 0,
+    "devid": 0,
+    "mtu": 0,
+    "model": null,
+    "mac": "00:00:00:00:00:00",
+    "ip": null,
+    "bridge": null,
+    "ifname": null,
+    "script": null,
+    "nictype": null,
+    "rate_bytes_per_interval": 10000000,
+    "rate_interval_usecs": 1000000
+}
+
+END
+one 0 rate=80Mb/s@1s
+one 0 rate=10MB/s@1s
+
+# test replenishment interval overflow
+expected </dev/null
+one $e rate=1B/s@4294967296us
+one $e rate=1B/s@4294968ms
+one $e rate=1B/s@4295s
+
+# test replenishment interval underflow
+expected </dev/null
+one $e rate=1B/s@0us
+
+# test rate limiting resulting in overflow
+expected </dev/null
+one $e rate=4294967295GB/s@5us
+one $e rate=4296MB/s@4294s
+
+complete
_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel