From cea3715b2e9a8faf9ed3a6d6699c03b52cfa769b Mon Sep 17 00:00:00 2001 From: Michal Privoznik Date: Mon, 2 Oct 2017 14:12:44 +0200 Subject: [PATCH] QoS: Set classes and filters in proper direction Similarly to previous patch, for some types of interface domain and host are on the same side of RX/TX barrier. In that case, we need to set up the QoS differently. Well, swapped. Signed-off-by: Michal Privoznik Reviewed-by: John Ferlan --- src/lxc/lxc_driver.c | 3 ++- src/lxc/lxc_process.c | 3 ++- src/network/bridge_driver.c | 2 +- src/qemu/qemu_command.c | 3 ++- src/qemu/qemu_driver.c | 6 +++-- src/qemu/qemu_hotplug.c | 6 +++-- src/util/virnetdevbandwidth.c | 43 ++++++++++++++++++++++++---------- src/util/virnetdevbandwidth.h | 3 ++- tests/virnetdevbandwidthtest.c | 2 +- 9 files changed, 48 insertions(+), 23 deletions(-) diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c index f0a22ce3eb..6ad61bdb76 100644 --- a/src/lxc/lxc_driver.c +++ b/src/lxc/lxc_driver.c @@ -3981,7 +3981,8 @@ lxcDomainAttachDeviceNetLive(virConnectPtr conn, actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { if (virNetDevSupportBandwidth(actualType)) { - if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false) < 0) + if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, + !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; } else { VIR_WARN("setting bandwidth on interfaces of " diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c index 05f1dec4c4..efd8a69000 100644 --- a/src/lxc/lxc_process.c +++ b/src/lxc/lxc_process.c @@ -601,7 +601,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr conn, actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { if (virNetDevSupportBandwidth(type)) { - if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false) < 0) + if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, + !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; } else { VIR_WARN("setting bandwidth on interfaces of " diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index e8d093a314..530d00ff36 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -2445,7 +2445,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr driver, VIR_FORCE_CLOSE(tapfd); } - if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true) < 0) + if (virNetDevBandwidthSet(def->bridge, def->bandwidth, true, true) < 0) goto err5; VIR_FREE(macTapIfName); diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 72318672ed..c9ba920689 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -8578,7 +8578,8 @@ qemuBuildInterfaceCommandLine(virQEMUDriverPtr driver, actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { if (virNetDevSupportBandwidth(actualType)) { - if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false) < 0) + if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, + !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; } else { VIR_WARN("setting bandwidth on interfaces of " diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index dcad01a01d..fd4075814a 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -11182,11 +11182,13 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom, if (!networkBandwidthChangeAllowed(net, newBandwidth)) goto endjob; - if (virNetDevBandwidthSet(net->ifname, newBandwidth, false) < 0 || + if (virNetDevBandwidthSet(net->ifname, newBandwidth, false, + !virDomainNetTypeSharesHostView(net)) < 0 || networkBandwidthUpdate(net, newBandwidth) < 0) { ignore_value(virNetDevBandwidthSet(net->ifname, net->bandwidth, - false)); + false, + !virDomainNetTypeSharesHostView(net))); goto endjob; } diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c index b77731df08..2b9427df1a 100644 --- a/src/qemu/qemu_hotplug.c +++ b/src/qemu/qemu_hotplug.c @@ -1209,7 +1209,8 @@ qemuDomainAttachNetDevice(virQEMUDriverPtr driver, actualBandwidth = virDomainNetGetActualBandwidth(net); if (actualBandwidth) { if (virNetDevSupportBandwidth(actualType)) { - if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false) < 0) + if (virNetDevBandwidthSet(net->ifname, actualBandwidth, false, + !virDomainNetTypeSharesHostView(net)) < 0) goto cleanup; } else { VIR_WARN("setting bandwidth on interfaces of " @@ -3362,7 +3363,8 @@ qemuDomainChangeNet(virQEMUDriverPtr driver, if (needBandwidthSet) { if (virNetDevBandwidthSet(newdev->ifname, virDomainNetGetActualBandwidth(newdev), - false) < 0) + false, + !virDomainNetTypeSharesHostView(newdev)) < 0) goto cleanup; needReplaceDevDef = true; } diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c index 4e4ac617be..9aebe56f40 100644 --- a/src/util/virnetdevbandwidth.c +++ b/src/util/virnetdevbandwidth.c @@ -174,6 +174,7 @@ virNetDevBandwidthManipulateFilter(const char *ifname, * @ifname: on which interface * @bandwidth: rates to set (may be NULL) * @hierarchical_class: whether to create hierarchical class + * @swapped: true if IN/OUT should be set contrariwise * * This function enables QoS on specified interface * and set given traffic limits for both, incoming @@ -182,14 +183,23 @@ virNetDevBandwidthManipulateFilter(const char *ifname, * hierarchical class. It is used to guarantee minimal * throughput ('floor' attribute in NIC). * + * If @swapped is set, the IN part of @bandwidth is set on + * @ifname's TX, and vice versa. If it is not set, IN is set on + * RX and OUT on TX. This is because for some types of interfaces + * domain and the host live on the same side of the interface (so + * domain's RX/TX is host's RX/TX), and for some it's swapped + * (domain's RX/TX is hosts's TX/RX). + * * Return 0 on success, -1 otherwise. */ int virNetDevBandwidthSet(const char *ifname, virNetDevBandwidthPtr bandwidth, - bool hierarchical_class) + bool hierarchical_class, + bool swapped) { int ret = -1; + virNetDevBandwidthRatePtr rx = NULL, tx = NULL; /* From domain POV */ virCommandPtr cmd = NULL; char *average = NULL; char *peak = NULL; @@ -215,16 +225,24 @@ virNetDevBandwidthSet(const char *ifname, return -1; } + if (swapped) { + rx = bandwidth->out; + tx = bandwidth->in; + } else { + rx = bandwidth->in; + tx = bandwidth->out; + } + virNetDevBandwidthClear(ifname); - if (bandwidth->in && bandwidth->in->average) { - if (virAsprintf(&average, "%llukbps", bandwidth->in->average) < 0) + if (tx && tx->average) { + if (virAsprintf(&average, "%llukbps", tx->average) < 0) goto cleanup; - if (bandwidth->in->peak && - (virAsprintf(&peak, "%llukbps", bandwidth->in->peak) < 0)) + if (tx->peak && + (virAsprintf(&peak, "%llukbps", tx->peak) < 0)) goto cleanup; - if (bandwidth->in->burst && - (virAsprintf(&burst, "%llukb", bandwidth->in->burst) < 0)) + if (tx->burst && + (virAsprintf(&burst, "%llukb", tx->burst) < 0)) goto cleanup; cmd = virCommandNew(TC); @@ -303,7 +321,7 @@ virNetDevBandwidthSet(const char *ifname, virCommandAddArgList(cmd, "class", "add", "dev", ifname, "parent", "1:", "classid", "1:1", "htb", "rate", average, "ceil", peak ? peak : average, NULL); - virNetDevBandwidthCmdAddOptimalQuantum(cmd, bandwidth->in); + virNetDevBandwidthCmdAddOptimalQuantum(cmd, tx); if (virCommandRun(cmd, NULL) < 0) goto cleanup; } @@ -319,7 +337,7 @@ virNetDevBandwidthSet(const char *ifname, if (burst) virCommandAddArgList(cmd, "burst", burst, NULL); - virNetDevBandwidthCmdAddOptimalQuantum(cmd, bandwidth->in); + virNetDevBandwidthCmdAddOptimalQuantum(cmd, tx); if (virCommandRun(cmd, NULL) < 0) goto cleanup; @@ -347,11 +365,10 @@ virNetDevBandwidthSet(const char *ifname, VIR_FREE(burst); } - if (bandwidth->out) { - if (virAsprintf(&average, "%llukbps", bandwidth->out->average) < 0) + if (rx) { + if (virAsprintf(&average, "%llukbps", rx->average) < 0) goto cleanup; - if (virAsprintf(&burst, "%llukb", bandwidth->out->burst ? - bandwidth->out->burst : bandwidth->out->average) < 0) + if (virAsprintf(&burst, "%llukb", rx->burst ? rx->burst : rx->average) < 0) goto cleanup; virCommandFree(cmd); diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h index 64f35372e4..63ac608970 100644 --- a/src/util/virnetdevbandwidth.h +++ b/src/util/virnetdevbandwidth.h @@ -45,7 +45,8 @@ void virNetDevBandwidthFree(virNetDevBandwidthPtr def); int virNetDevBandwidthSet(const char *ifname, virNetDevBandwidthPtr bandwidth, - bool hierarchical_class) + bool hierarchical_class, + bool swapped) ATTRIBUTE_RETURN_CHECK; int virNetDevBandwidthClear(const char *ifname); int virNetDevBandwidthCopy(virNetDevBandwidthPtr *dest, diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c index a8f4ab1966..2b031d55e6 100644 --- a/tests/virnetdevbandwidthtest.c +++ b/tests/virnetdevbandwidthtest.c @@ -81,7 +81,7 @@ testVirNetDevBandwidthSet(const void *data) virCommandSetDryRun(&buf, NULL, NULL); - if (virNetDevBandwidthSet(iface, band, info->hierarchical_class) < 0) + if (virNetDevBandwidthSet(iface, band, info->hierarchical_class, true) < 0) goto cleanup; if (!(actual_cmd = virBufferContentAndReset(&buf))) {