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

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



On 12/11/2012 11:09 AM, Michal Privoznik wrote:
> These set bridge part of QoS when bringing domain's interface up.
> Long story short, if there's a 'floor' set, a new QoS class is created.
> ClassID MUST be unique within the bridge and should be kept for
> unplug phase.
> ---
>  po/POTFILES.in                |    1 +
>  src/util/virnetdevbandwidth.c |  182 +++++++++++++++++++++++++++++++++++++++++
>  src/util/virnetdevbandwidth.h |   14 +++
>  3 files changed, 197 insertions(+), 0 deletions(-)
>
> diff --git a/po/POTFILES.in b/po/POTFILES.in
> index 586aa2b..d47f445 100644
> --- a/po/POTFILES.in
> +++ b/po/POTFILES.in
> @@ -158,6 +158,7 @@ src/util/virinitctl.c
>  src/util/virkeyfile.c
>  src/util/virlockspace.c
>  src/util/virnetdev.c
> +src/util/virnetdevbandwidth.c
>  src/util/virnetdevbridge.c
>  src/util/virnetdevmacvlan.c
>  src/util/virnetdevopenvswitch.c
> diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
> index 71c272e..f6f2a86 100644
> --- a/src/util/virnetdevbandwidth.c
> +++ b/src/util/virnetdevbandwidth.c
> @@ -352,3 +352,185 @@ virNetDevBandwidthEqual(virNetDevBandwidthPtr a,
>  
>      return true;
>  }
> +
> +/*
> + * virNetDevBandwidthPlug:
> + * @brname: name of the bridge
> + * @net_bandwidth: QoS settings on @brname
> + * @ifmac: MAC of interface
> + * @bandwidth: QoS settings for interface
> + * @id: unique ID (MUST be greater than 2)
> + *
> + * Set bridge part of interface QoS settings, e.g. guaranteed
> + * bandwidth.  @id is an unique ID (among @brname) from which
> + * other identifiers for class, qdisc and filter are derived.
> + * However, two classes were already set up (by
> + * virNetDevBandwidthSet). That's why this @id MUST be greater
> + * than 2. You may want to keep passed @id, as it is used later
> + * by virNetDevBandwidthUnplug.
> + *
> + * Returns:
> + * 0 if QoS set successfully
> + * -1 otherwise.
> + */
> +int
> +virNetDevBandwidthPlug(const char *brname,
> +                       virNetDevBandwidthPtr net_bandwidth,
> +                       const virMacAddrPtr ifmac_ptr,
> +                       virNetDevBandwidthPtr bandwidth,
> +                       unsigned int id)
> +{
> +    int ret = -1;
> +    virCommandPtr cmd = NULL;
> +    char *class_id = NULL;
> +    char *qdisc_id = NULL;
> +    char *filter_id = NULL;
> +    char *floor = NULL;
> +    char *ceil = NULL;
> +    unsigned char ifmac[VIR_MAC_BUFLEN];
> +    char ifmacStr[VIR_MAC_STRING_BUFLEN];
> +    char *mac[2] = {NULL, NULL};
> +
> +    if (id <= 2) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid class ID %d"), id);
> +        return -1;
> +    }
> +
> +    virMacAddrGetRaw(ifmac_ptr, ifmac);
> +    virMacAddrFormat(ifmac_ptr, ifmacStr);
> +
> +    if (!net_bandwidth || !net_bandwidth->in) {
> +        virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
> +                       _("Bridge '%s' has no QoS set, therefore "
> +                         "unable to set 'floor' on '%s'"),
> +                       brname, ifmacStr);
> +        return -1;
> +    }
> +
> +    if (virAsprintf(&class_id, "1:%x", id) < 0 ||
> +        virAsprintf(&qdisc_id, "%x:", id) < 0 ||
> +        virAsprintf(&filter_id, "%u", id) < 0 ||
> +        virAsprintf(&mac[0], "0x%02x%02x%02x%02x", ifmac[2],
> +                    ifmac[3], ifmac[4], ifmac[5]) < 0 ||
> +        virAsprintf(&mac[1], "0x%02x%02x", ifmac[0], ifmac[1]) < 0 ||
> +        virAsprintf(&floor, "%llukbps", bandwidth->in->floor) < 0 ||
> +        virAsprintf(&ceil, "%llukbps", net_bandwidth->in->peak ?
> +                    net_bandwidth->in->peak :
> +                    net_bandwidth->in->average) < 0) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    cmd = virCommandNew(TC);
> +    virCommandAddArgList(cmd, "class", "add", "dev", brname, "parent", "1:1",
> +                         "classid", class_id, "htb", "rate", floor,
> +                         "ceil", ceil, NULL);
> +
> +    if (virCommandRun(cmd, NULL) < 0)
> +        goto cleanup;
> +
> +    virCommandFree(cmd);
> +    cmd = virCommandNew(TC);
> +    virCommandAddArgList(cmd, "qdisc", "add", "dev", brname, "parent",
> +                         class_id, "handle", qdisc_id, "sfq", "perturb",
> +                         "10", NULL);
> +
> +    if (virCommandRun(cmd, NULL) < 0)
> +        goto cleanup;
> +
> +    virCommandFree(cmd);
> +    cmd = virCommandNew(TC);
> +    /* Okay, this not nice. But since libvirt does not know anything about
> +     * interface IP address(es), and tc fw filter simply refuse to use ebtables
> +     * marks, we need to use u32 selector to match MAC address.
> +     * If libvirt will ever know something, remove this FIXME
> +     */
> +    virCommandAddArgList(cmd, "filter", "add", "dev", brname, "protocol", "ip",
> +                         "prio", filter_id, "u32",
> +                         "match", "u16", "0x0800", "0xffff", "at", "-2",
> +                         "match", "u32", mac[0], "0xffffffff", "at", "-12",
> +                         "match", "u16", mac[1], "0xffff", "at", "-14",
> +                         "flowid", class_id, NULL);
> +
> +    if (virCommandRun(cmd, NULL) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    VIR_FREE(mac[1]);
> +    VIR_FREE(mac[0]);
> +    VIR_FREE(ceil);
> +    VIR_FREE(floor);
> +    VIR_FREE(filter_id);
> +    VIR_FREE(qdisc_id);
> +    VIR_FREE(class_id);
> +    virCommandFree(cmd);
> +    return ret;
> +}
> +
> +/*
> + * virNetDevBandwidthUnplug:
> + * @brname: from which bridge are we unplugging
> + * @id: unique identifier (MUST be greater than 2)
> + *
> + * Remove QoS settings from bridge.
> + *
> + * Returns 0 on success, -1 otherwise.
> + */
> +int
> +virNetDevBandwidthUnplug(const char *brname,
> +                         unsigned int id)
> +{
> +    int ret = -1;
> +    int cmd_ret = 0;
> +    virCommandPtr cmd = NULL;
> +    char *class_id = NULL;
> +    char *qdisc_id = NULL;
> +    char *filter_id = NULL;
> +
> +    if (id <= 2) {
> +        virReportError(VIR_ERR_INTERNAL_ERROR, _("Invalid class ID %d"), id);
> +        return -1;
> +    }
> +
> +    if (virAsprintf(&class_id, "1:%x", id) < 0 ||
> +        virAsprintf(&qdisc_id, "%x:", id) < 0 ||
> +        virAsprintf(&filter_id, "%u", id) < 0) {
> +        virReportOOMError();
> +        goto cleanup;
> +    }
> +
> +    cmd = virCommandNew(TC);
> +    virCommandAddArgList(cmd, "qdisc", "del", "dev", brname,
> +                         "handle", qdisc_id, NULL);
> +
> +    /* Don't threat tc errors as fatal, but
> +     * try to remove as much as possible */
> +    if (virCommandRun(cmd, &cmd_ret) < 0)
> +        goto cleanup;
> +
> +    virCommandFree(cmd);
> +    cmd = virCommandNew(TC);
> +    virCommandAddArgList(cmd, "filter", "del", "dev", brname,
> +                         "prio", filter_id, NULL);
> +
> +    if (virCommandRun(cmd, &cmd_ret) < 0)
> +        goto cleanup;
> +
> +    cmd = virCommandNew(TC);
> +    virCommandAddArgList(cmd, "class", "del", "dev", brname,
> +                         "classid", class_id, NULL);
> +
> +    if (virCommandRun(cmd, &cmd_ret) < 0)
> +        goto cleanup;
> +
> +    ret = 0;
> +
> +cleanup:
> +    VIR_FREE(filter_id);
> +    VIR_FREE(qdisc_id);
> +    VIR_FREE(class_id);
> +    virCommandFree(cmd);
> +    return ret;
> +}
> diff --git a/src/util/virnetdevbandwidth.h b/src/util/virnetdevbandwidth.h
> index d308ab2..5c9bce3 100644
> --- a/src/util/virnetdevbandwidth.h
> +++ b/src/util/virnetdevbandwidth.h
> @@ -24,6 +24,7 @@
>  # define __VIR_NETDEV_BANDWIDTH_H__
>  
>  # include "internal.h"
> +# include "virmacaddr.h"
>  
>  typedef struct _virNetDevBandwidthRate virNetDevBandwidthRate;
>  typedef virNetDevBandwidthRate *virNetDevBandwidthRatePtr;
> @@ -53,4 +54,17 @@ int virNetDevBandwidthCopy(virNetDevBandwidthPtr *dest, const virNetDevBandwidth
>  
>  bool virNetDevBandwidthEqual(virNetDevBandwidthPtr a, virNetDevBandwidthPtr b);
>  
> +int virNetDevBandwidthPlug(const char *brname,
> +                           virNetDevBandwidthPtr net_bandwidth,
> +                           const virMacAddrPtr ifmac_ptr,
> +                           virNetDevBandwidthPtr bandwidth,
> +                           unsigned int id)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2)
> +    ATTRIBUTE_NONNULL(3) ATTRIBUTE_NONNULL(4)
> +    ATTRIBUTE_RETURN_CHECK;
> +
> +int virNetDevBandwidthUnplug(const char *brname,
> +                             unsigned int id)
> +    ATTRIBUTE_NONNULL(1) ATTRIBUTE_RETURN_CHECK;
> +
>  #endif /* __VIR_NETDEV_BANDWIDTH_H__ */

You should add virNetDevBandwidth plug to libvirt_private.syms during
this patch, but since you're not actually using it yet, it doesn't cause
a make failure, (and you *do* add it in on the patch where you use it)
so I'll let it slide :-)

ACK.


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