Re: [PATCH v2] network: Add network bandwidth support for ethernet interfaces

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

 



Should I recreate this patch on the top of the latest tree and resubmit?
Or, is there anything that I missed out? Any feedback will be highly
appreciated. Thanks.

Anirban

On 9/26/14, 10:52 AM, "Anirban Chakraborty" <abchak@xxxxxxxxxxx> wrote:

>V2:
>Addressed comments raised in review of V1.
>Consolidate calls to virNetDevBandwidthSet.
>Clear bandwidth settings when the interface is detached or domain
>destroyed.
>
>V1:
>Ethernet interfaces in libvirt currently do not support bandwidth setting.
>For example, following xml file for an interface will not apply these
>settings to corresponding qdiscs.
>
>    <interface type="ethernet">
>      <mac address="02:36:1d:18:2a:e4"/>
>      <model type="virtio"/>
>      <script path=""/>
>      <target dev="tap361d182a-e4"/>
>      <bandwidth>
>        <inbound average="984" peak="1024" burst="64"/>
>        <outbound average="2000" peak="2048" burst="128"/>
>      </bandwidth>
>    </interface>
>
>Signed-off-by: Anirban Chakraborty <abchak@xxxxxxxxxxx>
>
>---
> src/lxc/lxc_process.c          | 26 +++++++++++++-------------
> src/network/bridge_driver.c    |  7 ++++---
> src/qemu/qemu_command.c        |  9 ++++-----
> src/qemu/qemu_driver.c         | 22 +++++++++++++++++++++-
> src/qemu/qemu_hotplug.c        | 14 +++++++++++++-
> src/util/virnetdevbandwidth.c  | 23 ++++++++++++++++++++---
> src/util/virnetdevbandwidth.h  |  7 ++++---
> src/util/virnetdevmacvlan.c    | 10 ----------
> src/util/virnetdevmacvlan.h    |  1 -
> tests/virnetdevbandwidthtest.c |  3 ++-
> 10 files changed, 81 insertions(+), 41 deletions(-)
>
>diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
>index ed30c37..7f7e4ad 100644
>--- a/src/lxc/lxc_process.c
>+++ b/src/lxc/lxc_process.c
>@@ -274,11 +274,6 @@ char
>*virLXCProcessSetupInterfaceBridged(virConnectPtr conn,
>     if (virNetDevSetOnline(parentVeth, true) < 0)
>         goto cleanup;
>
>-    if (virNetDevBandwidthSet(net->ifname,
>-                              virDomainNetGetActualBandwidth(net),
>-                              false) < 0)
>-        goto cleanup;
>-
>     if (net->filter &&
>         virDomainConfNWFilterInstantiate(conn, vm->uuid, net) < 0)
>         goto cleanup;
>@@ -300,6 +295,7 @@ char *virLXCProcessSetupInterfaceDirect(virConnectPtr
>conn,
>     virNetDevBandwidthPtr bw;
>     virNetDevVPortProfilePtr prof;
>     virLXCDriverConfigPtr cfg = virLXCDriverGetConfig(driver);
>+    const char *linkdev = virDomainNetGetActualDirectDev(net);
>
>     /* XXX how todo bandwidth controls ?
>      * Since the 'net-ifname' is about to be moved to a different
>@@ -329,16 +325,15 @@ char
>*virLXCProcessSetupInterfaceDirect(virConnectPtr conn,
>
>     if (virNetDevMacVLanCreateWithVPortProfile(
>             net->ifname, &net->mac,
>-            virDomainNetGetActualDirectDev(net),
>+            linkdev,
>             virDomainNetGetActualDirectMode(net),
>             false, def->uuid,
>-            virDomainNetGetActualVirtPortProfile(net),
>+            prof,
>             &res_ifname,
>             VIR_NETDEV_VPORT_PROFILE_OP_CREATE,
>             cfg->stateDir,
>-            virDomainNetGetActualBandwidth(net), 0) < 0)
>+            0) < 0)
>         goto cleanup;
>-
>     ret = res_ifname;
>
>  cleanup:
>@@ -368,6 +363,7 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
>conn,
>     int ret = -1;
>     size_t i;
>     size_t niface = 0;
>+    int actualType;
>
>     for (i = 0; i < def->nnets; i++) {
>         char *veth = NULL;
>@@ -381,7 +377,8 @@ static int virLXCProcessSetupInterfaces(virConnectPtr
>conn,
>         if (VIR_EXPAND_N(*veths, *nveths, 1) < 0)
>             goto cleanup;
>
>-        switch (virDomainNetGetActualType(def->nets[i])) {
>+        actualType = virDomainNetGetActualType(def->nets[i]);
>+        switch (actualType) {
>         case VIR_DOMAIN_NET_TYPE_NETWORK: {
>             virNetworkPtr network;
>             char *brname = NULL;
>@@ -444,11 +441,14 @@ static int
>virLXCProcessSetupInterfaces(virConnectPtr conn,
>         case VIR_DOMAIN_NET_TYPE_LAST:
>             virReportError(VIR_ERR_INTERNAL_ERROR,
>                            _("Unsupported network type %s"),
>-                           virDomainNetTypeToString(
>-                               virDomainNetGetActualType(def->nets[i])
>-                               ));
>+                           virDomainNetTypeToString(actualType));
>             goto cleanup;
>         }
>+        /* set network bandwidth */
>+        if (virNetDevBandwidthSet(def->nets[i]->ifname,
>+                virDomainNetGetActualBandwidth(def->nets[i]), false,
>+                actualType) < 0)
>+           goto cleanup;
>
>         (*veths)[(*nveths)-1] = veth;
>
>diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c
>index 979fb13..2e1f821 100644
>--- a/src/network/bridge_driver.c
>+++ b/src/network/bridge_driver.c
>@@ -2082,7 +2082,8 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr
>driver,
>     }
>
>     if (virNetDevBandwidthSet(network->def->bridge,
>-                              network->def->bandwidth, true) < 0)
>+                              network->def->bandwidth, true,
>+                              VIR_DOMAIN_NET_TYPE_BRIDGE) < 0)
>         goto err5;
>
>     VIR_FREE(macTapIfName);
>@@ -2090,7 +2091,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr
>driver,
>     return 0;
>
>  err5:
>-    virNetDevBandwidthClear(network->def->bridge);
>+    virNetDevBandwidthClear(network->def->bridge,
>VIR_DOMAIN_NET_TYPE_BRIDGE);
>
>  err4:
>     if (!save_err)
>@@ -2137,7 +2138,7 @@ networkStartNetworkVirtual(virNetworkDriverStatePtr
>driver,
> static int networkShutdownNetworkVirtual(virNetworkDriverStatePtr driver
>ATTRIBUTE_UNUSED,
>                                          virNetworkObjPtr network)
> {
>-    virNetDevBandwidthClear(network->def->bridge);
>+    virNetDevBandwidthClear(network->def->bridge,
>VIR_DOMAIN_NET_TYPE_BRIDGE);
>
>     if (network->radvdPid > 0) {
>         char *radvdpidbase;
>diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
>index eb72451..a92fb29 100644
>--- a/src/qemu/qemu_command.c
>+++ b/src/qemu/qemu_command.c
>@@ -191,7 +191,6 @@ qemuPhysIfaceConnect(virDomainDefPtr def,
>         virDomainNetGetActualVirtPortProfile(net),
>         &res_ifname,
>         vmop, cfg->stateDir,
>-        virDomainNetGetActualBandwidth(net),
>         macvlan_create_flags);
>     if (rc >= 0) {
>         virDomainAuditNetDevice(def, net, res_ifname, true);
>@@ -371,10 +370,6 @@ qemuNetworkIfaceConnect(virDomainDefPtr def,
>                                   &net->mac) < 0)
>         goto cleanup;
>
>-    if (virNetDevBandwidthSet(net->ifname,
>-                              virDomainNetGetActualBandwidth(net),
>-                              false) < 0)
>-        goto cleanup;
>
>     if (net->filter &&
>         virDomainConfNWFilterInstantiate(conn, def->uuid, net) < 0) {
>@@ -7392,6 +7387,10 @@ qemuBuildInterfaceCommandLine(virCommandPtr cmd,
>         if (tapfd[0] < 0)
>             goto cleanup;
>     }
>+    /* Set bandwidth  */
>+    if (virNetDevBandwidthSet(net->ifname,
>virDomainNetGetActualBandwidth(net),
>+           false, actualType) < 0)
>+        goto cleanup;
>
>     if ((actualType == VIR_DOMAIN_NET_TYPE_NETWORK ||
>          actualType == VIR_DOMAIN_NET_TYPE_BRIDGE ||
>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>index 117138a..02eb680 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -186,6 +186,9 @@ qemuVMFilterRebuild(virDomainObjListIterator iter,
>void *data)
>     return virDomainObjListForEach(qemu_driver->domains, iter, data);
> }
>
>+static void
>+qemuDomainClearNetBandwidth(virDomainObjPtr vm);
>+
> static virNWFilterCallbackDriver qemuCallbackDriver = {
>     .name = QEMU_DRIVER_NAME,
>     .vmFilterRebuild = qemuVMFilterRebuild,
>@@ -2196,6 +2199,9 @@ qemuDomainDestroyFlags(virDomainPtr dom,
>     if (virDomainDestroyFlagsEnsureACL(dom->conn, vm->def) < 0)
>         goto cleanup;
>
>+    /* Clear network bandwidth settings */
>+    qemuDomainClearNetBandwidth(vm);
>+
>     qemuDomainSetFakeReboot(driver, vm, false);
>
>
>@@ -10205,7 +10211,8 @@ qemuDomainSetInterfaceParameters(virDomainPtr dom,
>                    sizeof(*newBandwidth->out));
>         }
>
>-        if (virNetDevBandwidthSet(net->ifname, newBandwidth, false) < 0)
>+        if (virNetDevBandwidthSet(net->ifname, newBandwidth, false,
>+                virDomainNetGetActualType(net)) < 0)
>             goto cleanup;
>
>         virNetDevBandwidthFree(net->bandwidth);
>@@ -18216,6 +18223,19 @@ qemuNodeAllocPages(virConnectPtr conn,
>                           startCell, cellCount, add);
> }
>
>+/* Clear the bandwidth setting of all the network interfaces of a vm */
>+static void
>+qemuDomainClearNetBandwidth(virDomainObjPtr vm)
>+{
>+    size_t i;
>+    int type;
>+
>+    for (i = 0; i < vm->def->nnets; i++) {
>+        type = virDomainNetGetActualType(vm->def->nets[i]);
>+        virNetDevBandwidthClear(vm->def->nets[i]->ifname, type);
>+    }
>+}
>+
>
> static virDriver qemuDriver = {
>     .no = VIR_DRV_QEMU,
>diff --git a/src/qemu/qemu_hotplug.c b/src/qemu/qemu_hotplug.c
>index d631887..4002afb 100644
>--- a/src/qemu/qemu_hotplug.c
>+++ b/src/qemu/qemu_hotplug.c
>@@ -947,6 +947,10 @@ int qemuDomainAttachNetDevice(virConnectPtr conn,
>         if (qemuOpenVhostNet(vm->def, net, priv->qemuCaps, vhostfd,
>&vhostfdSize) < 0)
>             goto cleanup;
>     }
>+    /* Set bandwidth  */
>+    if (virNetDevBandwidthSet(net->ifname,
>virDomainNetGetActualBandwidth(net),
>+            false, actualType) < 0)
>+        goto cleanup;
>
>     for (i = 0; i < tapfdSize; i++) {
>         if (virSecurityManagerSetTapFDLabel(driver->securityManager,
>@@ -2214,7 +2218,7 @@ qemuDomainChangeNet(virQEMUDriverPtr driver,
>     if (needBandwidthSet) {
>         if (virNetDevBandwidthSet(newdev->ifname,
>                                   virDomainNetGetActualBandwidth(newdev),
>-                                  false) < 0)
>+                                  false, newType) < 0)
>             goto cleanup;
>         needReplaceDevDef = true;
>     }
>@@ -3481,6 +3485,7 @@ qemuDomainDetachNetDevice(virConnectPtr conn,
>     virDomainNetDefPtr detach = NULL;
>     qemuDomainObjPrivatePtr priv = vm->privateData;
>     int rc;
>+    int actualType;
>
>     if ((detachidx = virDomainNetFindIdx(vm->def, dev->data.net)) < 0)
>         goto cleanup;
>@@ -3517,6 +3522,13 @@ qemuDomainDetachNetDevice(virConnectPtr conn,
>         }
>     }
>
>+    actualType = virDomainNetGetActualType(detach);
>+    if (virNetDevBandwidthClear(detach->ifname, actualType) < 0) {
>+        virReportError(VIR_ERR_OPERATION_FAILED,
>+                       _("cannot clear bandwidth setting for device :
>%s"),
>+                       detach->ifname);
>+        goto cleanup;
>+    }
>     qemuDomainMarkDeviceForRemoval(vm, &detach->info);
>
>     qemuDomainObjEnterMonitor(driver, vm);
>diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
>index 5fa231a..34a224e 100644
>--- a/src/util/virnetdevbandwidth.c
>+++ b/src/util/virnetdevbandwidth.c
>@@ -27,6 +27,7 @@
> #include "viralloc.h"
> #include "virerror.h"
> #include "virstring.h"
>+#include "domain_conf.h"
>
> #define VIR_FROM_THIS VIR_FROM_NONE
>
>@@ -47,6 +48,7 @@ virNetDevBandwidthFree(virNetDevBandwidthPtr def)
>  * @ifname: on which interface
>  * @bandwidth: rates to set (may be NULL)
>  * @hierarchical_class: whether to create hierarchical class
>+ * @type: interface type
>  *
>  * This function enables QoS on specified interface
>  * and set given traffic limits for both, incoming
>@@ -60,7 +62,8 @@ virNetDevBandwidthFree(virNetDevBandwidthPtr def)
> int
> virNetDevBandwidthSet(const char *ifname,
>                       virNetDevBandwidthPtr bandwidth,
>-                      bool hierarchical_class)
>+                      bool hierarchical_class,
>+                      int type)
> {
>     int ret = -1;
>     virCommandPtr cmd = NULL;
>@@ -74,7 +77,14 @@ virNetDevBandwidthSet(const char *ifname,
>         goto cleanup;
>     }
>
>-    virNetDevBandwidthClear(ifname);
>+    if (type != VIR_DOMAIN_NET_TYPE_ETHERNET &&
>+            type != VIR_DOMAIN_NET_TYPE_BRIDGE &&
>+            type != VIR_DOMAIN_NET_TYPE_NETWORK &&
>+            type != VIR_DOMAIN_NET_TYPE_DIRECT)
>+        /* bandwidth not set for interfaces other than the above */
>+        return 0;
>+
>+    virNetDevBandwidthClear(ifname, type);
>
>     if (bandwidth->in && bandwidth->in->average) {
>         if (virAsprintf(&average, "%llukbps", bandwidth->in->average) <
>0)
>@@ -245,6 +255,7 @@ virNetDevBandwidthSet(const char *ifname,
> /**
>  * virNetDevBandwidthClear:
>  * @ifname: on which interface
>+ * @type: interface tyoe
>  *
>  * This function tries to disable QoS on specified interface
>  * by deleting root and ingress qdisc. However, this may fail
>@@ -253,12 +264,18 @@ virNetDevBandwidthSet(const char *ifname,
>  * Return 0 on success, -1 otherwise.
>  */
> int
>-virNetDevBandwidthClear(const char *ifname)
>+virNetDevBandwidthClear(const char *ifname, int type)
> {
>     int ret = 0;
>     int dummy; /* for ignoring the exit status */
>     virCommandPtr cmd = NULL;
>
>+    if (type != VIR_DOMAIN_NET_TYPE_ETHERNET &&
>+            type != VIR_DOMAIN_NET_TYPE_BRIDGE &&
>+            type != VIR_DOMAIN_NET_TYPE_NETWORK &&
>+            type != VIR_DOMAIN_NET_TYPE_DIRECT)
>+        return ret;
>+
>     cmd = virCommandNew(TC);
>     virCommandAddArgList(cmd, "qdisc", "del", "dev", ifname, "root",
>NULL);
>
>diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
>index 4606a07..cc43444 100644
>--- a/src/util/virnetdevbandwidth.h
>+++ b/src/util/virnetdevbandwidth.h
>@@ -43,11 +43,12 @@ struct _virNetDevBandwidth {
>
> void virNetDevBandwidthFree(virNetDevBandwidthPtr def);
>
>-int virNetDevBandwidthSet(const char *ifname,
>+int virNetDevBandwidthSet(const char *name,
>                           virNetDevBandwidthPtr bandwidth,
>-                          bool hierarchical_class)
>+                          bool hierarchical_class,
>+                          int type)
>     ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
>-int virNetDevBandwidthClear(const char *ifname)
>+int virNetDevBandwidthClear(const char *name, int type)
>     ATTRIBUTE_NONNULL(1);
> int virNetDevBandwidthCopy(virNetDevBandwidthPtr *dest,
>                            const virNetDevBandwidth *src)
>diff --git a/src/util/virnetdevmacvlan.c b/src/util/virnetdevmacvlan.c
>index c83341c..956a96b 100644
>--- a/src/util/virnetdevmacvlan.c
>+++ b/src/util/virnetdevmacvlan.c
>@@ -811,7 +811,6 @@ int virNetDevMacVLanCreateWithVPortProfile(const char
>*tgifname,
>                                            char **res_ifname,
>                                            virNetDevVPortProfileOp vmOp,
>                                            char *stateDir,
>-                                           virNetDevBandwidthPtr
>bandwidth,
>                                            unsigned int flags)
> {
>     const char *type = (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP) ?
>@@ -925,14 +924,6 @@ int virNetDevMacVLanCreateWithVPortProfile(const
>char *tgifname,
>         rc = 0;
>     }
>
>-    if (virNetDevBandwidthSet(cr_ifname, bandwidth, false) < 0) {
>-        if (flags & VIR_NETDEV_MACVLAN_CREATE_WITH_TAP)
>-            VIR_FORCE_CLOSE(rc); /* sets rc to -1 */
>-        else
>-            rc = -1;
>-        goto disassociate_exit;
>-    }
>-
>     if (vmOp == VIR_NETDEV_VPORT_PROFILE_OP_CREATE ||
>         vmOp == VIR_NETDEV_VPORT_PROFILE_OP_RESTORE) {
>         /* Only directly register upon a create or restore (restarting
>@@ -1076,7 +1067,6 @@ int virNetDevMacVLanCreateWithVPortProfile(const
>char *ifname ATTRIBUTE_UNUSED,
>                                            char **res_ifname
>ATTRIBUTE_UNUSED,
>                                            virNetDevVPortProfileOp vmop
>ATTRIBUTE_UNUSED,
>                                            char *stateDir
>ATTRIBUTE_UNUSED,
>-                                           virNetDevBandwidthPtr
>bandwidth ATTRIBUTE_UNUSED,
>                                            unsigned int flags)
> {
>     virCheckFlags(0, -1);
>diff --git a/src/util/virnetdevmacvlan.h b/src/util/virnetdevmacvlan.h
>index 41aa4e2..f08d32b 100644
>--- a/src/util/virnetdevmacvlan.h
>+++ b/src/util/virnetdevmacvlan.h
>@@ -68,7 +68,6 @@ int virNetDevMacVLanCreateWithVPortProfile(const char
>*ifname,
>                                            char **res_ifname,
>                                            virNetDevVPortProfileOp vmop,
>                                            char *stateDir,
>-                                           virNetDevBandwidthPtr
>bandwidth,
>                                            unsigned int flags)
>     ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(6)
>     ATTRIBUTE_NONNULL(8) ATTRIBUTE_NONNULL(10) ATTRIBUTE_RETURN_CHECK;
>diff --git a/tests/virnetdevbandwidthtest.c
>b/tests/virnetdevbandwidthtest.c
>index 384991e..db1f557 100644
>--- a/tests/virnetdevbandwidthtest.c
>+++ b/tests/virnetdevbandwidthtest.c
>@@ -79,7 +79,8 @@ testVirNetDevBandwidthSet(const void *data)
>
>     virCommandSetDryRun(&buf, NULL, NULL);
>
>-    if (virNetDevBandwidthSet(iface, band, info->hierarchical_class) < 0)
>+    if (virNetDevBandwidthSet(iface, band, info->hierarchical_class,
>+           VIR_DOMAIN_NET_TYPE_ETHERNET) < 0)
>         goto cleanup;
>
>     if (!(actual_cmd = virBufferContentAndReset(&buf))) {
>-- 
>1.9.1
>
>--
>libvir-list mailing list
>libvir-list@xxxxxxxxxx
>https://www.redhat.com/mailman/listinfo/libvir-list


--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]