[libvirt] [PATCH 5/7] bandwitdh: Implement functions to enable and disable QoS
Laine Stump
laine at laine.org
Thu Jul 21 07:05:49 UTC 2011
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)
More information about the libvir-list
mailing list