Re: [libvirt] [PATCH v3 6/8] bandwidth: Create network bandwidth (un)plug functions

Network should be notified if we plug in or unplug an interface, so it can perform some action, e.g. set/unset network part of QoS. However, we are doing this in very early stage, so iface->ifname isn't filled in yet. So whenever we want to report an error, we must use a different identifier, e.g. the MAC address. --- src/conf/domain_conf.h | 1 + src/conf/network_conf.c | 18 ++++ src/conf/network_conf.h | 4 + src/libvirt_private.syms | 3 + src/network/bridge_driver.c | 212 ++++++++++++++++++++++++++++++++++++++++++- 5 files changed, 236 insertions(+), 2 deletions(-) diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h index 7ad5377..e6659cd 100644 --- a/src/conf/domain_conf.h +++ b/src/conf/domain_conf.h @@ -809,6 +809,7 @@ struct _virDomainActualNetDef { virNetDevVPortProfilePtr virtPortProfile; virNetDevBandwidthPtr bandwidth; virNetDevVlan vlan; + unsigned int class_id; /* class ID for bandwidth 'floor' */ }; /* Stores the virtual network interface configuration */ diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c index 3093418..ac326e1 100644 --- a/src/conf/network_conf.c +++ b/src/conf/network_conf.c @@ -47,6 +47,9 @@ #define MAX_BRIDGE_ID 256 #define VIR_FROM_THIS VIR_FROM_NETWORK +#define NEXT_FREE_CLASS_ID 3 +/* currently, /sbin/tc implementation allows up to 16 bits for minor class size */ +#define CLASS_ID_BITMAP_SIZE (1<<16) VIR_ENUM_IMPL(virNetworkForward, VIR_NETWORK_FORWARD_LAST, @@ -319,10 +322,25 @@ virNetworkAssignDef(virNetworkObjListPtr nets, virNetworkObjLock(network); network->def = def; + if (!(network->class_id = virBitmapNew(CLASS_ID_BITMAP_SIZE))) { + virReportOOMError(); + goto error; + } + + /* The first three class IDs are already taken */ + ignore_value(virBitmapSetBit(network->class_id, 0)); + ignore_value(virBitmapSetBit(network->class_id, 1)); + ignore_value(virBitmapSetBit(network->class_id, 2)); + + network->def = def; nets->objs[nets->count] = network; nets->count++; return network; +error: + virNetworkObjUnlock(network); + virNetworkObjFree(network); + return NULL; } diff --git a/src/conf/network_conf.h b/src/conf/network_conf.h index 949b3d2..364372d 100644 --- a/src/conf/network_conf.h +++ b/src/conf/network_conf.h @@ -38,6 +38,7 @@ # include "virnetdevvlan.h" # include "virmacaddr.h" # include "device_conf.h" +# include "bitmap.h" enum virNetworkForwardType { VIR_NETWORK_FORWARD_NONE = 0, @@ -226,6 +227,9 @@ struct _virNetworkObj { virNetworkDefPtr def; /* The current definition */ virNetworkDefPtr newDef; /* New definition to activate at shutdown */ + + virBitmapPtr class_id; /* bitmap of class IDs for QoS */ + unsigned long long floor_sum; /* sum of all 'floor'-s of attached NICs */ }; typedef struct _virNetworkObjList virNetworkObjList; diff --git a/src/libvirt_private.syms b/src/libvirt_private.syms index 499ab3b..6564676 100644 --- a/src/libvirt_private.syms +++ b/src/libvirt_private.syms @@ -1506,7 +1506,10 @@ virNetDevBandwidthClear; virNetDevBandwidthCopy; virNetDevBandwidthEqual; virNetDevBandwidthFree; +virNetDevBandwidthPlug; virNetDevBandwidthSet; +virNetDevBandwidthUnplug; +virNetDevBandwidthUpdateRate;

# virnetdevbridge.h diff --git a/src/network/bridge_driver.c b/src/network/bridge_driver.c index 58f1d2e..0bee453 100644 --- a/src/network/bridge_driver.c +++ b/src/network/bridge_driver.c @@ -121,6 +121,11 @@ static int networkShutdownNetworkExternal(struct network_driver *driver, static void networkReloadIptablesRules(struct network_driver *driver); static void networkRefreshDaemons(struct network_driver *driver); +static int networkPlugBandwidth(virNetworkObjPtr net, + virDomainNetDefPtr iface); +static int networkUnplugBandwidth(virNetworkObjPtr net, + virDomainNetDefPtr iface); + static struct network_driver *driverState = NULL; static char * @@ -3491,6 +3496,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) enum virDomainNetType actualType = iface->type; virNetworkObjPtr network = NULL; virNetworkDefPtr netdef = NULL; + virNetDevBandwidthPtr bandwidth = NULL; virPortGroupDefPtr portgroup = NULL; virNetDevVPortProfilePtr virtport = iface->virtPortProfile; virNetDevVlanPtr vlan = NULL; @@ -3529,7 +3535,13 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) * (already in NetDef). Otherwise, if there is bandwidth info in * the portgroup, fill that into the ActualDef. */ - if (portgroup && !iface->bandwidth) { + + if (iface->bandwidth) + bandwidth = iface->bandwidth; + else if (portgroup && portgroup->bandwidth) + bandwidth = portgroup->bandwidth; + + if (bandwidth) { if (!iface->data.network.actual && (VIR_ALLOC(iface->data.network.actual) < 0)) { virReportOOMError(); @@ -3537,7 +3549,7 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) } if (virNetDevBandwidthCopy(&iface->data.network.actual->bandwidth, - portgroup->bandwidth) < 0) + bandwidth) < 0) goto error; } @@ -3550,6 +3562,10 @@ networkAllocateActualDevice(virDomainNetDefPtr iface) */ if (iface->data.network.actual) iface->data.network.actual->type = VIR_DOMAIN_NET_TYPE_NETWORK; + + if (networkPlugBandwidth(network, iface) < 0) + goto error; + } else if ((netdef->forwardType == VIR_NETWORK_FORWARD_BRIDGE) && netdef->bridge) { @@ -4061,6 +4077,12 @@ networkReleaseActualDevice(virDomainNetDefPtr iface) } netdef = network->def; + if ((netdef->forwardType == VIR_NETWORK_FORWARD_NONE || + netdef->forwardType == VIR_NETWORK_FORWARD_NAT || + netdef->forwardType == VIR_NETWORK_FORWARD_ROUTE) && + networkUnplugBandwidth(network, iface) < 0) + goto error; + if ((!iface->data.network.actual) || ((actualType != VIR_DOMAIN_NET_TYPE_DIRECT) && (actualType != VIR_DOMAIN_NET_TYPE_HOSTDEV))) { @@ -4264,3 +4286,189 @@ cleanup: error: goto cleanup; } + +/** + * networkCheckBandwidth: + * @net: network QoS + * @iface: interface QoS + * @new_rate: new rate for non guaranteed class + * + * Returns: -1 if plugging would overcommit network QoS + * 0 if plugging is safe (@new_rate updated) + * 1 if no QoS is set (@new_rate untouched) + */ +static int +networkCheckBandwidth(virNetworkObjPtr net, + virDomainNetDefPtr iface, + unsigned long long *new_rate) +{ + int ret = -1; + virNetDevBandwidthPtr netBand = net->def->bandwidth; + virNetDevBandwidthPtr ifaceBand = iface->bandwidth; + unsigned long long tmp_floor_sum = net->floor_sum; + unsigned long long tmp_new_rate = 0; + char ifmac[VIR_MAC_STRING_BUFLEN]; + + if (!ifaceBand || !ifaceBand->in || !ifaceBand->in->floor || + !netBand || !netBand->in) + return 1; + + virMacAddrFormat(&iface->mac, ifmac); + + tmp_new_rate = netBand->in->average; + tmp_floor_sum += ifaceBand->in->floor; + + /* check against peak */ + if (netBand->in->peak) { + tmp_new_rate = netBand->in->peak; + if (tmp_floor_sum > netBand->in->peak) { + virReportError(VIR_ERR_OPERATION_INVALID, + _("Cannot plug '%s' interface into '%s' because it " + "would overcommit 'peak' on network '%s'"), + ifmac, + net->def->bridge, + net->def->name); + goto cleanup; + } + } else if (tmp_floor_sum > netBand->in->average) { + /* tmp_floor_sum can be between 'average' and 'peak' iff 'peak' is set. + * Otherwise, tmp_floor_sum must be below 'average'. */ + virReportError(VIR_ERR_OPERATION_INVALID, + _("Cannot plug '%s' interface into '%s' because it " + "would overcommit 'average' on network '%s'"), + ifmac, + net->def->bridge, + net->def->name); + goto cleanup; + } + + *new_rate = tmp_new_rate; + ret = 0; + +cleanup: + return ret; +} + +/** + * networkNextClassID: + * @net: network object + * + * Find next free class ID. @net is supposed + * to be locked already. If there is a free ID, + * it is marked as used and returned. + * + * Returns next free class ID or -1 if none is available. + */ +static ssize_t +networkNextClassID(virNetworkObjPtr net) +{ + size_t ret = 0; + bool is_set = false; + + while (virBitmapGetBit(net->class_id, ret, &is_set) == 0 && is_set) + ret++; + + if (is_set || virBitmapSetBit(net->class_id, ret) < 0) + return -1; + + return ret; +} + +static int +networkPlugBandwidth(virNetworkObjPtr net, + virDomainNetDefPtr iface) +{ + int ret = -1; + int plug_ret; + unsigned long long new_rate = 0; + ssize_t class_id = 0; + char ifmac[VIR_MAC_STRING_BUFLEN]; + + if ((plug_ret = networkCheckBandwidth(net, iface, &new_rate)) < 0) { + /* helper reported error */ + goto cleanup; + } + + if (plug_ret > 0) { + /* no QoS needs to be set; claim success */ + ret = 0; + goto cleanup; + } + + virMacAddrFormat(&iface->mac, ifmac); + if (iface->type != VIR_DOMAIN_NET_TYPE_NETWORK || + !iface->data.network.actual) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("Cannot set bandwidth on interface '%s' of type %d"), + ifmac, iface->type); + goto cleanup; + } + + /* generate new class_id */ + if ((class_id = networkNextClassID(net)) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("Could not generate next class ID")); + goto cleanup; + } + + plug_ret = virNetDevBandwidthPlug(net->def->bridge, + net->def->bandwidth, + &iface->mac, + iface->bandwidth, + class_id); + if (plug_ret < 0) { + ignore_value(virNetDevBandwidthUnplug(net->def->bridge, class_id)); + goto cleanup; + } + + /* QoS was set, generate new class ID */ + iface->data.network.actual->class_id = class_id; + /* update sum of 'floor'-s of attached NICs */ + net->floor_sum += iface->bandwidth->in->floor; + /* update rate for non guaranteed NICs */ + new_rate -= net->floor_sum; + if (virNetDevBandwidthUpdateRate(net->def->bridge, "1:2", + net->def->bandwidth, new_rate) < 0) + VIR_WARN("Unable to update rate for 1:2 class on %s bridge", + net->def->bridge); + + ret = 0; + +cleanup: + return ret; +} + +static int +networkUnplugBandwidth(virNetworkObjPtr net, + virDomainNetDefPtr iface) +{ + int ret = 0; + unsigned long long new_rate; + + if (iface->data.network.actual && + iface->data.network.actual->class_id) { + /* we must remove class from bridge */ + new_rate = net->def->bandwidth->in->average; + + if (net->def->bandwidth->in->peak > 0) + new_rate = net->def->bandwidth->in->peak; + + ret = virNetDevBandwidthUnplug(net->def->bridge, + iface->data.network.actual->class_id); + if (ret < 0) + goto cleanup; + /* update sum of 'floor'-s of attached NICs */ + net->floor_sum -= iface->bandwidth->in->floor; + /* update rate for non guaranteed NICs */ + new_rate -= net->floor_sum; + if (virNetDevBandwidthUpdateRate(net->def->bridge, "1:2", + net->def->bandwidth, new_rate) < 0) + VIR_WARN("Unable to update rate for 1:2 class on %s bridge", + net->def->bridge); + /* no class is associated any longer */ + iface->data.network.actual->class_id = 0; + } + +cleanup: + return ret; +}