Not only this simplifies the code a bit, it prepares the
environment for upcoming patches. The new
virNetDevBandwidthManipulateFilter() function is capable of both
removing a filter and adding a new one. At the same time! Yeah,
this is not currently used anywhere but look at the next commit
where you'll see it.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/util/virnetdevbandwidth.c | 142 +++++++++++++++++++++++++++++++-----------
1 file changed, 106 insertions(+), 36 deletions(-)
diff --git a/src/util/virnetdevbandwidth.c b/src/util/virnetdevbandwidth.c
index 943178b..c57c8c0 100644
--- a/src/util/virnetdevbandwidth.c
+++ b/src/util/virnetdevbandwidth.c
@@ -43,6 +43,107 @@ virNetDevBandwidthFree(virNetDevBandwidthPtr def)
VIR_FREE(def);
}
+/**
+ * virNetDevBandwidthManipulateFilter:
+ * @ifname: interface to operate on
+ * @ifmac_ptr: MAC of the interface to create filter over
+ * @id: filter ID
+ * @class_id: where to place traffic
+ * @remove_old: whether to remove the filter
+ * @create_new: whether to create the filter
+ *
+ * TC filters are as crucial for traffic shaping as QDiscs. While
+ * QDisc acts like black boxes deciding which packets should be
+ * held up and which should be sent immediately, it's filter who
+ * places a packet into the box. So, we may end up constructing a
+ * set of filters on a single device (e.g. a bridge) and filter
+ * the traffic into QDiscs based on the originating vNET device.
+ *
+ * Long story short, @ifname is the interface where the filter
+ * should be created. The @ifmac_ptr is the MAC address for which
+ * the filter should be created (usually different to the MAC
+ * address of @ifname). Then, like everything - even filters have
+ * an @id which should be unique (per @ifname). And @class_id
+ * tells into which QDisc should filter place the traffic.
+ *
+ * This function can be used for both, removing stale filter
+ * (@remove_old set to true) and creating new one (@create_new
+ * set to true). Both at once for the same price!
+ *
+ * Returns: 0 on success,
+ * -1 otherwise (with error reported).
+ */
+static int ATTRIBUTE_NONNULL(1)
+virNetDevBandwidthManipulateFilter(const char *ifname,
+ const virMacAddr *ifmac_ptr,
+ unsigned int id,
+ const char *class_id,
+ bool remove_old,
+ bool create_new)
+{
+ int ret = -1;
+ char *filter_id = NULL;
+ virCommandPtr cmd = NULL;
+ unsigned char ifmac[VIR_MAC_BUFLEN];
+ char *mac[2] = {NULL, NULL};
+
+ if (!(remove_old || create_new)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("filter creation API error"));
+ goto cleanup;
+ }
+
+ /* u32 filters must have 800:: prefix. Don't ask. */
+ if (virAsprintf(&filter_id, "800::%u", id) < 0)
+ goto cleanup;
+
+ if (remove_old) {
+ int cmd_ret = 0;
+
+ cmd = virCommandNew(TC);
+ virCommandAddArgList(cmd, "filter", "del", "dev",
ifname,
+ "prio", "2", "handle",
filter_id, "u32", NULL);
+
+ if (virCommandRun(cmd, &cmd_ret) < 0)
+ goto cleanup;
+
+ }
+
+ if (create_new) {
+ virMacAddrGetRaw(ifmac_ptr, ifmac);
+
+ if (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)
+ goto cleanup;
+
+ virCommandFree(cmd);
+ cmd = virCommandNew(TC);
+ /* Okay, this not nice. But since libvirt does not necessarily track
+ * 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",
ifname, "protocol", "ip",
+ "prio", "2", "handle",
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(filter_id);
+ virCommandFree(cmd);
+ return ret;
+}
+
/**
* virNetDevBandwidthSet:
@@ -416,19 +517,15 @@ virNetDevBandwidthPlug(const char *brname,
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) {
@@ -441,10 +538,6 @@ virNetDevBandwidthPlug(const char *brname,
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 :
@@ -468,31 +561,15 @@ virNetDevBandwidthPlug(const char *brname,
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)
+ if (virNetDevBandwidthManipulateFilter(brname, ifmac_ptr, id,
+ class_id, false, true) < 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);
@@ -517,7 +594,6 @@ virNetDevBandwidthUnplug(const char *brname,
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);
@@ -525,8 +601,7 @@ virNetDevBandwidthUnplug(const char *brname,
}
if (virAsprintf(&class_id, "1:%x", id) < 0 ||
- virAsprintf(&qdisc_id, "%x:", id) < 0 ||
- virAsprintf(&filter_id, "%u", id) < 0)
+ virAsprintf(&qdisc_id, "%x:", id) < 0)
goto cleanup;
cmd = virCommandNew(TC);
@@ -538,12 +613,8 @@ virNetDevBandwidthUnplug(const char *brname,
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)
+ if (virNetDevBandwidthManipulateFilter(brname, NULL, id,
+ NULL, true, false) < 0)
goto cleanup;
virCommandFree(cmd);
@@ -557,7 +628,6 @@ virNetDevBandwidthUnplug(const char *brname,
ret = 0;
cleanup:
- VIR_FREE(filter_id);
VIR_FREE(qdisc_id);
VIR_FREE(class_id);
virCommandFree(cmd);
--
2.0.5