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

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



On 12/11/2012 11:09 AM, Michal Privoznik wrote:
> 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;

Yeah, those are the changes I would have put with the earlier patch...

>  
>  
>  # 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;
> +}
ACK.


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