[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [libvirt] [PATCH 5/7] bandwitdh: Implement functions to enable and disable QoS



On 07/18/2011 04:05 PM, Michal Privoznik wrote:
These function executes 'tc' with appropriate arguments to set
desired QoS setting on interface or bridge during its creation.
---
  configure.ac                |    4 +
  src/libvirt_private.syms    |    2 +
  src/network/bridge_driver.c |   12 +++
  src/qemu/qemu_command.c     |   10 +++-
  src/util/macvtap.c          |   12 +++-
  src/util/macvtap.h          |    5 +-
  src/util/network.c          |  161 +++++++++++++++++++++++++++++++++++++++++++
  src/util/network.h          |    3 +
  8 files changed, 205 insertions(+), 4 deletions(-)

diff --git a/configure.ac b/configure.ac
index e9d5be4..2b864cf 100644
--- a/configure.ac
+++ b/configure.ac
@@ -165,6 +165,8 @@ AC_PATH_PROG([RADVD], [radvd], [radvd],
  	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
  AC_PATH_PROG([BRCTL], [brctl], [brctl],
  	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
+AC_PATH_PROG([TC], [tc], [tc],
+    [/sbin:/usr/sbin:/usr/local/sbin:$PATH])


You may also want to add a note to the comment in libvirt.spec.in where we add "Requires: iproute" that we now need it for /sbin/tc, in addition to /sbin/ip.


  AC_PATH_PROG([UDEVADM], [udevadm], [],
  	[/sbin:/usr/sbin:/usr/local/sbin:$PATH])
  AC_PATH_PROG([UDEVSETTLE], [udevsettle], [],
@@ -178,6 +180,8 @@ AC_DEFINE_UNQUOTED([RADVD],["$RADVD"],
          [Location or name of the radvd program])
  AC_DEFINE_UNQUOTED([BRCTL],["$BRCTL"],
          [Location or name of the brctl program (see bridge-utils)])
+AC_DEFINE_UNQUOTED([TC],["$TC"],
+        [Location or name of the tc profram (see iproute2)])
  if test -n "$UDEVADM"; then
    AC_DEFINE_UNQUOTED([UDEVADM],["$UDEVADM"],
          [Location or name of the udevadm program])
diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms
index 1cc9bca..ff398d7 100644
--- a/src/libvirt_private.syms
+++ b/src/libvirt_private.syms
@@ -702,6 +702,8 @@ nlComm;
  # network.h
  virBandwidrhDefFormat;
  virBandwidthDefParseNode;
+virBandwidthDisable;
+virBandwidthEnable;
  virSocketAddrBroadcast;
  virSocketAddrBroadcastByPrefix;
  virSocketAddrIsNetmask;
diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
index 0a12bc0..47f9799 100644
--- a/src/network/bridge_driver.c
+++ b/src/network/bridge_driver.c
@@ -1804,6 +1804,13 @@ networkStartNetworkDaemon(struct network_driver *driver,
      if (v6present&&  networkStartRadvd(network)<  0)
          goto err4;

In my vswitch patches I've renamed networkStartNetworkDaemon to networkStartNetworkVirtual, and added separate functions for host bridge-based networks and macvtap networks, so 1) there will be a slight merge conflict, and 2) you will want to add a call to virBandwidthEnable at least to the host bridge version (I'm not sure how you can implement a network-wide bandwidth limit for the macvtap-based networks, since they can have multiple interfaces.

Also, what happens when libvirt is restarted? Do we need to "reload" the tc configuration as we do with iptables rules (in case some other application has messed with them)?


+    if (virBandwidthEnable(&network->def->bandwidth, network->def->bridge)<  0) {
+        networkReportError(VIR_ERR_INTERNAL_ERROR,
+                           _("cannot set bandwidth limits on %s"),
+                           network->def->bridge);
+        goto err5;
+    }
+
      /* Persist the live configuration now we have bridge info  */
      if (virNetworkSaveConfig(NETWORK_STATE_DIR, network->def)<  0) {
          goto err5;
@@ -1895,6 +1902,11 @@ static int networkShutdownNetworkDaemon(struct network_driver *driver,
      unlink(stateFile);
      VIR_FREE(stateFile);

+    if (virBandwidthDisable(network->def->bridge, true)<  0) {
+        VIR_WARN("Failed to disable QoS on %s",
+                 network->def->name);
+    }
+
      if (network->radvdPid>  0) {
          char *radvdpidbase;

diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index a3bce4a..78cb865 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -128,7 +128,7 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
      rc = openMacvtapTap(net->ifname, net->mac, net->data.direct.linkdev,
                          net->data.direct.mode, vnet_hdr, def->uuid,
                          &net->data.direct.virtPortProfile,&res_ifname,
-                        vmop, driver->stateDir);
+                        vmop, driver->stateDir,&net->bandwidth);
      if (rc>= 0) {
          virDomainAuditNetDevice(def, net, res_ifname, true);
          VIR_FREE(net->ifname);
@@ -291,6 +291,14 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
          }
      }

+    if (virBandwidthEnable(&net->bandwidth, net->ifname)<  0) {
+        qemuReportError(VIR_ERR_INTERNAL_ERROR,
+                        _("cannot set bandwidth limits on %s"),
+                        net->ifname);
+        VIR_FORCE_CLOSE(tapfd);
+        goto cleanup;
+    }
+
      if (tapfd>= 0) {
          if ((net->filter)&&  (net->ifname)) {
              err = virDomainConfNWFilterInstantiate(conn, net);
diff --git a/src/util/macvtap.c b/src/util/macvtap.c
index 30343c8..bb7784c 100644
--- a/src/util/macvtap.c
+++ b/src/util/macvtap.c
@@ -266,7 +266,8 @@ openMacvtapTap(const char *tgifname,
                 virVirtualPortProfileParamsPtr virtPortProfile,
                 char **res_ifname,
                 enum virVMOperationType vmOp,
-               char *stateDir)
+               char *stateDir,
+               virBandwidthPtr bandwidth)
  {
      const char *type = "macvtap";
      int c, rc;
@@ -360,6 +361,15 @@ create_name:
      } else
          goto disassociate_exit;

+    if (virBandwidthEnable(bandwidth, cr_ifname)<  0) {
+        macvtapError(VIR_ERR_INTERNAL_ERROR,
+                     _("cannot set bandwidth limits on %s"),
+                     cr_ifname);
+        rc = -1;
+        goto disassociate_exit;
+    }
+
+

I thought somebody had said something earlier about bandwitdh control not working on macvtap interfaces... Have you tried this and seen it work?

      return rc;

  disassociate_exit:
diff --git a/src/util/macvtap.h b/src/util/macvtap.h
index 1b85989..84bbc92 100644
--- a/src/util/macvtap.h
+++ b/src/util/macvtap.h
@@ -24,7 +24,7 @@
  # define __UTIL_MACVTAP_H__

  # include<config.h>
-
+# include "network.h"

  enum virVirtualPortType {
      VIR_VIRTUALPORT_NONE,
@@ -95,7 +95,8 @@ int openMacvtapTap(const char *ifname,
                     virVirtualPortProfileParamsPtr virtPortProfile,
                     char **res_ifname,
                     enum virVMOperationType vmop,
-                   char *stateDir);
+                   char *stateDir,
+                   virBandwidthPtr bandwidth);

  void delMacvtap(const char *ifname,
                  const unsigned char *macaddress,
diff --git a/src/util/network.c b/src/util/network.c
index 58c0492..0a5652c 100644
--- a/src/util/network.c
+++ b/src/util/network.c
@@ -15,6 +15,7 @@
  #include "network.h"
  #include "util.h"
  #include "virterror_internal.h"
+#include "command.h"

  #define VIR_FROM_THIS VIR_FROM_NONE
  #define virSocketError(code, ...)                                       \
@@ -858,3 +859,163 @@ virBandwidrhDefFormat(virBufferPtr buf,
  cleanup:
      return ret;
  }
+
+/**
+ * virBandwidthEnable:
+ * @bandwidth: rates to set
+ * @iface: on which interface
+ *
+ * This function enables QoS on specified interface
+ * and set given traffic limits for both, incoming
+ * and outgoing traffic. Any previous setting get
+ * overwritten.
+ *
+ * Return 0 on success, -1 otherwise.
+ */
+int
+virBandwidthEnable(virBandwidthPtr bandwidth,
+                   const char *iface)
+{
+    int ret = -1;
+    virCommandPtr cmd = NULL;
+    char *average = NULL;
+    char *peak = NULL;
+    char *burst = NULL;
+
+    if (!bandwidth || !iface)
+        return -1;
+
+    if (!bandwidth->in.average&&
+        !bandwidth->out.average) {
+        /* nothing to be enabled */
+        ret = 0;
+        goto cleanup;
+    }
+
+    if (virBandwidthDisable(iface, true)<  0)
+        goto cleanup;
+
+    if (bandwidth->in.average) {
+        if (virAsprintf(&average, "%lukbps", bandwidth->in.average)<  0)
+            goto cleanup;
+        if (bandwidth->in.peak&&
+            (virAsprintf(&peak, "%lukbps", bandwidth->in.peak)<  0))
+            goto cleanup;
+        if (bandwidth->in.burst&&
+            (virAsprintf(&burst, "%lukb", bandwidth->in.burst)<  0))
+            goto cleanup;
+
+        cmd = virCommandNew(TC);
+        virCommandAddArgList(cmd, "qdisc", "add", "dev", iface, "root",
+                             "handle", "1:", "htb", "default", "1", NULL);
+        if (virCommandRun(cmd, NULL)<  0)
+            goto cleanup;
+
+        virCommandFree(cmd);
+        cmd = virCommandNew(TC);
+            virCommandAddArgList(cmd,"class", "add", "dev", iface, "parent",
+                                 "1:", "classid", "1:1", "htb", NULL);
+        virCommandAddArgList(cmd, "rate", average, NULL);
+
+        if (peak)
+            virCommandAddArgList(cmd, "ceil", peak, NULL);
+        if (burst)
+            virCommandAddArgList(cmd, "burst", burst, NULL);
+
+        if (virCommandRun(cmd, NULL)<  0)
+            goto cleanup;
+
+        virCommandFree(cmd);
+        cmd = virCommandNew(TC);
+            virCommandAddArgList(cmd,"filter", "add", "dev", iface, "parent",
+                                 "1:0", "protocol", "ip", "handle", "1", "fw",
+                                 "flowid", "1", NULL);


I haven't used tc, but I trust these are all the right commands :-)


+
+        if (virCommandRun(cmd, NULL)<  0)
+            goto cleanup;
+
+        virCommandFree(cmd);
+        VIR_FREE(average);
+        VIR_FREE(peak);
+        VIR_FREE(burst);
+    }
+
+    if (bandwidth->out.average) {
+        if (virAsprintf(&average, "%lukbps", bandwidth->out.average)<  0)
+            goto cleanup;
+        if (virAsprintf(&burst, "%lukb", bandwidth->out.burst ?
+                        bandwidth->out.burst : bandwidth->out.average)<  0)
+            goto cleanup;
+
+        cmd = virCommandNew(TC);
+            virCommandAddArgList(cmd, "qdisc", "add", "dev", iface,
+                                 "ingress", NULL);
+
+        if (virCommandRun(cmd, NULL)<  0)
+            goto cleanup;
+
+        virCommandFree(cmd);
+        cmd = virCommandNew(TC);
+        virCommandAddArgList(cmd, "filter", "add", "dev", iface, "parent",
+                             "ffff:", "protocol", "ip", "u32", "match", "ip",
+                             "src", "0.0.0.0/0", "police", "rate", average,
+                             "burst", burst, "drop", "flowid", ":1", NULL);
+
+        if (virCommandRun(cmd, NULL)<  0)
+            goto cleanup;
+    }
+
+    ret = 0;
+
+cleanup:
+    virCommandFree(cmd);
+    VIR_FREE(average);
+    VIR_FREE(peak);
+    VIR_FREE(burst);
+    return ret;
+}
+
+/**
+ * virBandwidthDisable:
+ * @iface: on which interface
+ * @may_fail: should be unsuccessful disable considered fatal?
+ *
+ * This function tries to disable QoS on specified interface
+ * by deleting root and ingress qdisc. However, this may fail
+ * if we try to remove the default one.
+ *
+ * Return 0 on success, -1 otherwise.
+ */
+int
+virBandwidthDisable(const char *iface,
+                    bool may_fail)
+{
+    int ret = -1;
+    int status;
+    virCommandPtr cmd = NULL;
+
+    if (!iface)
+        return -1;
+
+    cmd = virCommandNew(TC);
+    virCommandAddArgList(cmd, "qdisc", "del", "dev", iface, "root", NULL);
+
+    if ((virCommandRun(cmd,&status)<  0) ||
+        (!may_fail&&  status))
+        goto cleanup;
+
+    virCommandFree(cmd);
+
+    cmd = virCommandNew(TC);
+    virCommandAddArgList(cmd, "qdisc",  "del", "dev", iface, "ingress", NULL);
+
+    if ((virCommandRun(cmd,&status)<  0) ||
+        (!may_fail&&  status))
+        goto cleanup;
+
+    ret = 0;
+
+cleanup:
+    virCommandFree(cmd);
+    return ret;
+}
diff --git a/src/util/network.h b/src/util/network.h
index 98e3082..17eab40 100644
--- a/src/util/network.h
+++ b/src/util/network.h
@@ -110,4 +110,7 @@ int virBandwidthDefParseNode(xmlNodePtr node, virBandwidthPtr def);
  int virBandwidrhDefFormat(virBufferPtr buf,
                            virBandwidthPtr def,
                            const char *indent);
+
+int virBandwidthEnable(virBandwidthPtr bandwidth, const char *iface);
+int virBandwidthDisable(const char *iface, bool may_fail);
  #endif /* __VIR_NETWORK_H__ */

ACK modulo any rebasing that will need to be done (and assuming that nothing needs to be done in parallel with reloading iptables filters)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]