9:17 a.m.
Cleanup virnetdevbandwidthtest and introduce new test cases.
However, Martin suggested (after glancing over the last patch) that
arguments could be stored in a file (just like qemuxml2argvtest does).
I'm not against that, but QEMU cmd line args change more often than
those of tc/ovs-vsctl. But if desired I can do the change (either as a
follow up or in v2).
Michal Prívozník (4):
virnetdevbandwidthtest: Drop unnecessary brackets
virnetdevbandwidthtest: Drop unused testMinimalStruct
virnetdevbandwidthtest: Reformat TC cmd line
virnetdevbandwidthtest: Test QoS for OVS
tests/virnetdevbandwidthtest.c | 171 +++++++++++++++++++++------------
1 file changed, 109 insertions(+), 62 deletions(-)
--
2.35.1
9:17 a.m.
New subject: [PATCH 1/4] virnetdevbandwidthtest: Drop unnecessary brackets
Some cases that call DO_TEST_SET() macro wrap each argument in
curved brackets. This is unnecessary, drop the brackets.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/virnetdevbandwidthtest.c | 88 +++++++++++++++++-----------------
1 file changed, 44 insertions(+), 44 deletions(-)
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
index eb4f47dfce..419793a4ae 100644
--- a/tests/virnetdevbandwidthtest.c
+++ b/tests/virnetdevbandwidthtest.c
@@ -117,53 +117,53 @@ mymain(void)
DO_TEST_SET("<bandwidth/>", NULL);
- DO_TEST_SET(("<bandwidth>"
- " <inbound average='1024'/>"
- "</bandwidth>"),
- (TC " qdisc del dev eth0 root\n"
- TC " qdisc del dev eth0 ingress\n"
- TC " qdisc add dev eth0 root handle 1: htb default 1\n"
- TC " class add dev eth0 parent 1: classid 1:1 htb rate 1024kbps
quantum 87\n"
- TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb
10\n"
- TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw
flowid 1\n"));
+ DO_TEST_SET("<bandwidth>"
+ " <inbound average='1024'/>"
+ "</bandwidth>",
+ TC " qdisc del dev eth0 root\n"
+ TC " qdisc del dev eth0 ingress\n"
+ TC " qdisc add dev eth0 root handle 1: htb default 1\n"
+ TC " class add dev eth0 parent 1: classid 1:1 htb rate 1024kbps
quantum 87\n"
+ TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
+ TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw
flowid 1\n");
- DO_TEST_SET(("<bandwidth>"
- " <outbound average='1024'/>"
- "</bandwidth>"),
- (TC " qdisc del dev eth0 root\n"
- TC " qdisc del dev eth0 ingress\n"
- TC " qdisc add dev eth0 ingress\n"
- TC " filter add dev eth0 parent ffff: protocol all u32 match u32 0
0 "
- "police rate 1024kbps burst 1024kb mtu 64kb drop flowid
:1\n"));
+ DO_TEST_SET("<bandwidth>"
+ " <outbound average='1024'/>"
+ "</bandwidth>",
+ TC " qdisc del dev eth0 root\n"
+ TC " qdisc del dev eth0 ingress\n"
+ TC " qdisc add dev eth0 ingress\n"
+ TC " filter add dev eth0 parent ffff: protocol all u32 match u32 0 0
"
+ "police rate 1024kbps burst 1024kb mtu 64kb drop flowid
:1\n");
- DO_TEST_SET(("<bandwidth>"
- " <inbound average='1' peak='2'
floor='3' burst='4'/>"
- " <outbound average='5' peak='6'
burst='7'/>"
- "</bandwidth>"),
- (TC " qdisc del dev eth0 root\n"
- TC " qdisc del dev eth0 ingress\n"
- TC " qdisc add dev eth0 root handle 1: htb default 1\n"
- TC " class add dev eth0 parent 1: classid 1:1 htb rate 1kbps ceil
2kbps burst 4kb quantum 1\n"
- TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb
10\n"
- TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw
flowid 1\n"
- TC " qdisc add dev eth0 ingress\n"
- TC " filter add dev eth0 parent ffff: protocol all u32 match u32 0
0 "
- "police rate 5kbps burst 7kb mtu 64kb drop flowid :1\n"));
+ DO_TEST_SET("<bandwidth>"
+ " <inbound average='1' peak='2'
floor='3' burst='4'/>"
+ " <outbound average='5' peak='6'
burst='7'/>"
+ "</bandwidth>",
+ TC " qdisc del dev eth0 root\n"
+ TC " qdisc del dev eth0 ingress\n"
+ TC " qdisc add dev eth0 root handle 1: htb default 1\n"
+ TC " class add dev eth0 parent 1: classid 1:1 htb rate 1kbps ceil
2kbps burst 4kb quantum 1\n"
+ TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
+ TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw
flowid 1\n"
+ TC " qdisc add dev eth0 ingress\n"
+ TC " filter add dev eth0 parent ffff: protocol all u32 match u32 0 0
"
+ "police rate 5kbps burst 7kb mtu 64kb drop flowid :1\n");
- DO_TEST_SET(("<bandwidth>"
- " <inbound average='4294967295'/>"
- " <outbound average='4294967295'/>"
- "</bandwidth>"),
- (TC " qdisc del dev eth0 root\n"
- TC " qdisc del dev eth0 ingress\n"
- TC " qdisc add dev eth0 root handle 1: htb default 1\n"
- TC " class add dev eth0 parent 1: classid 1:1 htb rate
4294967295kbps quantum 366503875\n"
- TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb
10\n"
- TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw
flowid 1\n"
- TC " qdisc add dev eth0 ingress\n"
- TC " filter add dev eth0 parent ffff: protocol all u32 match
"
- "u32 0 0 police rate 4294967295kbps burst 4194303kb mtu 64kb
"
- "drop flowid :1\n"));
+ DO_TEST_SET("<bandwidth>"
+ " <inbound average='4294967295'/>"
+ " <outbound average='4294967295'/>"
+ "</bandwidth>",
+ TC " qdisc del dev eth0 root\n"
+ TC " qdisc del dev eth0 ingress\n"
+ TC " qdisc add dev eth0 root handle 1: htb default 1\n"
+ TC " class add dev eth0 parent 1: classid 1:1 htb rate
4294967295kbps quantum 366503875\n"
+ TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
+ TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw
flowid 1\n"
+ TC " qdisc add dev eth0 ingress\n"
+ TC " filter add dev eth0 parent ffff: protocol all u32 match "
+ "u32 0 0 police rate 4294967295kbps burst 4194303kb mtu 64kb "
+ "drop flowid :1\n");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.35.1
9:17 a.m.
New subject: [PATCH 2/4] virnetdevbandwidthtest: Drop unused testMinimalStruct
The last usage of the testMinimalStruct struct was removed in
v1.2.2-rc1~206 which forgot to remove the struct as well. Remove
it now.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/virnetdevbandwidthtest.c | 6 ------
1 file changed, 6 deletions(-)
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
index 419793a4ae..f0873743b4 100644
--- a/tests/virnetdevbandwidthtest.c
+++ b/tests/virnetdevbandwidthtest.c
@@ -26,12 +26,6 @@
#define VIR_FROM_THIS VIR_FROM_NONE
-struct testMinimalStruct {
- const char *expected_result;
- const char *band1;
- const char *band2;
-};
-
struct testSetStruct {
const char *band;
const char *exp_cmd;
--
2.35.1
9:17 a.m.
New subject: [PATCH 3/4] virnetdevbandwidthtest: Reformat TC cmd line
Our coding style expects a long line to be broken into shorter
lines which are then aligned on the first character, for
instance:
"some string that's broken "
"into multiple lines"
However, one can argue that there are few cases where shifting
the alignment makes the code more readable. And this is the case
of expected cmd line for DO_TEST_SET() where a long cmd line can
be aligned on the arguments rather than the binary:
TC " filter ..."
" police ..."
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/virnetdevbandwidthtest.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
index f0873743b4..51e5ec016b 100644
--- a/tests/virnetdevbandwidthtest.c
+++ b/tests/virnetdevbandwidthtest.c
@@ -127,8 +127,8 @@ mymain(void)
TC " qdisc del dev eth0 root\n"
TC " qdisc del dev eth0 ingress\n"
TC " qdisc add dev eth0 ingress\n"
- TC " filter add dev eth0 parent ffff: protocol all u32 match u32 0 0
"
- "police rate 1024kbps burst 1024kb mtu 64kb drop flowid
:1\n");
+ TC " filter add dev eth0 parent ffff: protocol all u32 match u32 0
0"
+ " police rate 1024kbps burst 1024kb mtu 64kb drop flowid
:1\n");
DO_TEST_SET("<bandwidth>"
" <inbound average='1' peak='2'
floor='3' burst='4'/>"
@@ -141,8 +141,8 @@ mymain(void)
TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw
flowid 1\n"
TC " qdisc add dev eth0 ingress\n"
- TC " filter add dev eth0 parent ffff: protocol all u32 match u32 0 0
"
- "police rate 5kbps burst 7kb mtu 64kb drop flowid :1\n");
+ TC " filter add dev eth0 parent ffff: protocol all u32 match u32 0
0"
+ " police rate 5kbps burst 7kb mtu 64kb drop flowid :1\n");
DO_TEST_SET("<bandwidth>"
" <inbound average='4294967295'/>"
@@ -155,9 +155,9 @@ mymain(void)
TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw
flowid 1\n"
TC " qdisc add dev eth0 ingress\n"
- TC " filter add dev eth0 parent ffff: protocol all u32 match "
- "u32 0 0 police rate 4294967295kbps burst 4194303kb mtu 64kb "
- "drop flowid :1\n");
+ TC " filter add dev eth0 parent ffff: protocol all u32 match"
+ " u32 0 0 police rate 4294967295kbps burst 4194303kb mtu
64kb"
+ " drop flowid :1\n");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.35.1
9:17 a.m.
New subject: [PATCH 4/4] virnetdevbandwidthtest: Test QoS for OVS
Ever since v7.6.0-rc1~235 we can use ovs-vsctl to set QoS instead
of tc. However, we don't have a test that's verifying generated
cmd line for ovs-vsctl.
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
tests/virnetdevbandwidthtest.c | 85 +++++++++++++++++++++++++++-------
1 file changed, 69 insertions(+), 16 deletions(-)
diff --git a/tests/virnetdevbandwidthtest.c b/tests/virnetdevbandwidthtest.c
index 51e5ec016b..d5e4092fc3 100644
--- a/tests/virnetdevbandwidthtest.c
+++ b/tests/virnetdevbandwidthtest.c
@@ -22,13 +22,17 @@
#define LIBVIRT_VIRCOMMANDPRIV_H_ALLOW
#include "vircommandpriv.h"
#include "virnetdevbandwidth.h"
+#include "virnetdevopenvswitch.h"
#include "netdev_bandwidth_conf.c"
#define VIR_FROM_THIS VIR_FROM_NONE
struct testSetStruct {
const char *band;
- const char *exp_cmd;
+ const char *exp_cmd_tc;
+ const char *exp_cmd_ovs;
+ bool ovs;
+ const unsigned char *uuid;
const char *iface;
const bool hierarchical_class;
};
@@ -59,6 +63,7 @@ testVirNetDevBandwidthSet(const void *data)
{
const struct testSetStruct *info = data;
const char *iface = info->iface;
+ const char *exp_cmd = NULL;
g_autoptr(virNetDevBandwidth) band = NULL;
g_auto(virBuffer) buf = VIR_BUFFER_INITIALIZER;
g_autofree char *actual_cmd = NULL;
@@ -72,17 +77,24 @@ testVirNetDevBandwidthSet(const void *data)
virCommandSetDryRun(dryRunToken, &buf, false, false, NULL, NULL);
- if (virNetDevBandwidthSet(iface, band, info->hierarchical_class, true) < 0)
- return -1;
+ if (info->ovs) {
+ exp_cmd = info->exp_cmd_ovs;
+ if (virNetDevOpenvswitchInterfaceSetQos(iface, band, info->uuid, true) <
0)
+ return -1;
+ } else {
+ exp_cmd = info->exp_cmd_tc;
+ if (virNetDevBandwidthSet(iface, band, info->hierarchical_class, true) <
0)
+ return -1;
+ }
if (!(actual_cmd = virBufferContentAndReset(&buf))) {
/* This is interesting, no command has been executed.
* Maybe that's expected, actually. */
}
- if (STRNEQ_NULLABLE(info->exp_cmd, actual_cmd)) {
+ if (STRNEQ_NULLABLE(exp_cmd, actual_cmd)) {
virTestDifference(stderr,
- NULLSTR(info->exp_cmd),
+ NULLSTR(exp_cmd),
NULLSTR(actual_cmd));
return -1;
}
@@ -94,22 +106,37 @@ static int
mymain(void)
{
int ret = 0;
+ unsigned char uuid[VIR_UUID_BUFLEN] = { 0 };
-#define DO_TEST_SET(Band, Exp_cmd, ...) \
+#define VMUUID "c1018351-a229-4209-9faf-42446e0b53e5"
+
+ if (virUUIDParse(VMUUID, uuid) < 0)
+ return -1;
+
+#define DO_TEST_SET(Band, Exp_cmd_tc, Exp_cmd_ovs, ...) \
do { \
struct testSetStruct data = {.band = Band, \
- .exp_cmd = Exp_cmd, \
+ .exp_cmd_tc = Exp_cmd_tc, \
+ .exp_cmd_ovs = Exp_cmd_ovs, \
+ .ovs = false, \
+ .uuid = uuid, \
__VA_ARGS__}; \
- if (virTestRun("virNetDevBandwidthSet", \
+ if (virTestRun("virNetDevBandwidthSet TC", \
testVirNetDevBandwidthSet, \
- &data) < 0) \
+ &data) < 0) { \
ret = -1; \
+ } \
+ data.ovs = true; \
+ if (virTestRun("virNetDevBandwidthSet OVS", \
+ testVirNetDevBandwidthSet, \
+ &data) < 0) { \
+ ret = -1; \
+ } \
} while (0)
+ DO_TEST_SET(NULL, NULL, NULL);
- DO_TEST_SET(NULL, NULL);
-
- DO_TEST_SET("<bandwidth/>", NULL);
+ DO_TEST_SET("<bandwidth/>", NULL, NULL);
DO_TEST_SET("<bandwidth>"
" <inbound average='1024'/>"
@@ -119,7 +146,14 @@ mymain(void)
TC " qdisc add dev eth0 root handle 1: htb default 1\n"
TC " class add dev eth0 parent 1: classid 1:1 htb rate 1024kbps
quantum 87\n"
TC " qdisc add dev eth0 parent 1:1 handle 2: sfq perturb 10\n"
- TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw
flowid 1\n");
+ TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw
flowid 1\n",
+ OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find queue
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"'\n"
+ OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find qos
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"'\n"
+ OVS_VSCTL " --timeout=5 set port eth0 qos=@qos1
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"' --"
+ " --id=@qos1 create qos type=linux-htb
other_config:min-rate=8192000 queues:0=@queue0 'external-ids:vm-id=\""
VMUUID "\"'"
+ " 'external-ids:ifname=\"eth0\"'
--"
+ " --id=@queue0 create queue other_config:min-rate=8192000
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"'\n"
+ OVS_VSCTL " --timeout=5 set Interface eth0 ingress_policing_rate=0
ingress_policing_burst=0\n");
DO_TEST_SET("<bandwidth>"
" <outbound average='1024'/>"
@@ -128,7 +162,10 @@ mymain(void)
TC " qdisc del dev eth0 ingress\n"
TC " qdisc add dev eth0 ingress\n"
TC " filter add dev eth0 parent ffff: protocol all u32 match u32 0
0"
- " police rate 1024kbps burst 1024kb mtu 64kb drop flowid
:1\n");
+ " police rate 1024kbps burst 1024kb mtu 64kb drop flowid
:1\n",
+ OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find queue
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"'\n"
+ OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find qos
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"'\n"
+ OVS_VSCTL " --timeout=5 set Interface eth0
ingress_policing_rate=8192\n");
DO_TEST_SET("<bandwidth>"
" <inbound average='1' peak='2'
floor='3' burst='4'/>"
@@ -142,7 +179,15 @@ mymain(void)
TC " filter add dev eth0 parent 1:0 protocol all prio 1 handle 1 fw
flowid 1\n"
TC " qdisc add dev eth0 ingress\n"
TC " filter add dev eth0 parent ffff: protocol all u32 match u32 0
0"
- " police rate 5kbps burst 7kb mtu 64kb drop flowid :1\n");
+ " police rate 5kbps burst 7kb mtu 64kb drop flowid :1\n",
+ OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find queue
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"'\n"
+ OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find qos
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"'\n"
+ OVS_VSCTL " --timeout=5 set port eth0 qos=@qos1
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"' --"
+ " --id=@qos1 create qos type=linux-htb
other_config:min-rate=8000 other_config:burst=32768 other_config:max-rate=16000"
+ " queues:0=@queue0 'external-ids:vm-id=\""
VMUUID "\"' 'external-ids:ifname=\"eth0\"' --"
+ " --id=@queue0 create queue other_config:min-rate=8000
other_config:burst=32768 other_config:max-rate=16000"
+ " 'external-ids:vm-id=\"" VMUUID
"\"' 'external-ids:ifname=\"eth0\"'\n"
+ OVS_VSCTL " --timeout=5 set Interface eth0 ingress_policing_rate=40
ingress_policing_burst=56\n");
DO_TEST_SET("<bandwidth>"
" <inbound average='4294967295'/>"
@@ -157,7 +202,15 @@ mymain(void)
TC " qdisc add dev eth0 ingress\n"
TC " filter add dev eth0 parent ffff: protocol all u32 match"
" u32 0 0 police rate 4294967295kbps burst 4194303kb mtu
64kb"
- " drop flowid :1\n");
+ " drop flowid :1\n",
+ OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find queue
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"'\n"
+ OVS_VSCTL " --timeout=5 --no-heading --columns=_uuid find qos
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"'\n"
+ OVS_VSCTL " --timeout=5 set port eth0 qos=@qos1
'external-ids:vm-id=\"" VMUUID "\"'
'external-ids:ifname=\"eth0\"' --"
+ " --id=@qos1 create qos type=linux-htb
other_config:min-rate=34359738360000"
+ " queues:0=@queue0 'external-ids:vm-id=\""
VMUUID "\"' 'external-ids:ifname=\"eth0\"' --"
+ " --id=@queue0 create queue
other_config:min-rate=34359738360000 'external-ids:vm-id=\"" VMUUID
"\"'"
+ "
'external-ids:ifname=\"eth0\"'\n"
+ OVS_VSCTL " --timeout=5 set Interface eth0
ingress_policing_rate=34359738360\n");
return ret == 0 ? EXIT_SUCCESS : EXIT_FAILURE;
}
--
2.35.1
3:56 p.m.
On Tue, Jun 28, 2022 at 04:17:31PM +0200, Michal Privoznik wrote:
Cleanup virnetdevbandwidthtest and introduce new test cases.
However, Martin suggested (after glancing over the last patch) that
arguments could be stored in a file (just like qemuxml2argvtest does).
I'm not against that, but QEMU cmd line args change more often than
those of tc/ovs-vsctl. But if desired I can do the change (either as a
follow up or in v2).
I, personally, do not need that, I just figured it could be nicer for
someone.
Anyway, good that you added the tests, I remember the units were not fun
to figure out with the inconsistencies wrt tc, so it's good we have it
in the tests too now.
Michal Prívozník (4):
virnetdevbandwidthtest: Drop unnecessary brackets
virnetdevbandwidthtest: Drop unused testMinimalStruct
virnetdevbandwidthtest: Reformat TC cmd line
virnetdevbandwidthtest: Test QoS for OVS
Reviewed-by: Martin Kletzander <mkletzan(a)redhat.com>
tests/virnetdevbandwidthtest.c | 171 +++++++++++++++++++++------------
1 file changed, 109 insertions(+), 62 deletions(-)
--
2.35.1
877
days inactive
877
days old
5 comments
2 participants
participants (2)
-
Martin Kletzander -
Michal Privoznik