From 5ca75531afd1244898ee7c8d2307857ba8260cba Mon Sep 17 00:00:00 2001
Message-Id: <5ca75531afd1244898ee7c8d2307857ba8260cba@dist-git>
From: Michal Privoznik <mprivozn@redhat.com>
Date: Wed, 9 Oct 2019 14:27:41 +0200
Subject: [PATCH] virNetDevOpenvswitchInterfaceStats: Optimize for speed
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
We run 'ovs-vsctl' nine times (first to find if interface is
there and then eight times = for each stats member separately).
This is very inefficient. I've found a way to run it once and
with a bit of help from virJSON module we can parse out stats
we need.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
(cherry picked from commit c297eab52599c91a4cb26b66dbdfe9d07c3142d3)
https://bugzilla.redhat.com/show_bug.cgi?id=1759904
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Message-Id: <9904d3bee93a9e103012d088b77999f023acaa2b.1570623892.git.mprivozn@redhat.com>
Reviewed-by: Jiri Denemark <jdenemar@redhat.com>
---
src/util/virnetdevopenvswitch.c | 119 +++++++++++++++++++++-----------
1 file changed, 78 insertions(+), 41 deletions(-)
diff --git a/src/util/virnetdevopenvswitch.c b/src/util/virnetdevopenvswitch.c
index f36916a300..fc92ab2e7f 100644
--- a/src/util/virnetdevopenvswitch.c
+++ b/src/util/virnetdevopenvswitch.c
@@ -35,6 +35,7 @@
#include "virmacaddr.h"
#include "virstring.h"
#include "virlog.h"
+#include "virjson.h"
#define VIR_FROM_THIS VIR_FROM_NONE
@@ -339,58 +340,94 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname,
virDomainInterfaceStatsPtr stats)
{
virCommandPtr cmd = NULL;
- char *output;
- char *tmp;
- bool gotStats = false;
+ VIR_AUTOFREE(char *) output = NULL;
+ virJSONValuePtr jsonStats = NULL;
+ virJSONValuePtr jsonMap = NULL;
+ size_t i;
int ret = -1;
- /* Just ensure the interface exists in ovs */
cmd = virCommandNew(OVSVSCTL);
virNetDevOpenvswitchAddTimeout(cmd);
- virCommandAddArgList(cmd, "get", "Interface", ifname, "name", NULL);
+ virCommandAddArgList(cmd, "--if-exists", "--format=list", "--data=json",
+ "--no-headings", "--columns=statistics", "list",
+ "Interface", ifname, NULL);
virCommandSetOutputBuffer(cmd, &output);
- if (virCommandRun(cmd, NULL) < 0) {
+ /* The above command returns either:
+ * 1) empty string if @ifname doesn't exist, or
+ * 2) a JSON array, for instance:
+ * ["map",[["collisions",0],["rx_bytes",0],["rx_crc_err",0],["rx_dropped",0],
+ * ["rx_errors",0],["rx_frame_err",0],["rx_over_err",0],["rx_packets",0],
+ * ["tx_bytes",12406],["tx_dropped",0],["tx_errors",0],["tx_packets",173]]]
+ */
+
+ if (virCommandRun(cmd, NULL) < 0 ||
+ STREQ_NULLABLE(output, "")) {
/* no ovs-vsctl or interface 'ifname' doesn't exists in ovs */
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Interface not found"));
goto cleanup;
}
-#define GET_STAT(name, member) \
- do { \
- VIR_FREE(output); \
- virCommandFree(cmd); \
- cmd = virCommandNew(OVSVSCTL); \
- virNetDevOpenvswitchAddTimeout(cmd); \
- virCommandAddArgList(cmd, "--if-exists", "get", "Interface", \
- ifname, "statistics:" name, NULL); \
- virCommandSetOutputBuffer(cmd, &output); \
- if (virCommandRun(cmd, NULL) < 0 || !output || !*output || *output == '\n') { \
- stats->member = -1; \
- } else { \
- if (virStrToLong_ll(output, &tmp, 10, &stats->member) < 0 || \
- *tmp != '\n') { \
- virReportError(VIR_ERR_INTERNAL_ERROR, "%s", \
- _("Fail to parse ovs-vsctl output")); \
- goto cleanup; \
- } \
- gotStats = true; \
- } \
- } while (0)
-
- /* The TX/RX fields appear to be swapped here
- * because this is the host view. */
- GET_STAT("rx_bytes", tx_bytes);
- GET_STAT("rx_packets", tx_packets);
- GET_STAT("rx_errors", tx_errs);
- GET_STAT("rx_dropped", tx_drop);
- GET_STAT("tx_bytes", rx_bytes);
- GET_STAT("tx_packets", rx_packets);
- GET_STAT("tx_errors", rx_errs);
- GET_STAT("tx_dropped", rx_drop);
-
- if (!gotStats) {
+ if (!(jsonStats = virJSONValueFromString(output)) ||
+ !virJSONValueIsArray(jsonStats) ||
+ !(jsonMap = virJSONValueArrayGet(jsonStats, 1))) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Unable to parse ovs-vsctl output"));
+ goto cleanup;
+ }
+
+ stats->rx_bytes = stats->rx_packets = stats->rx_errs = stats->rx_drop = -1;
+ stats->tx_bytes = stats->tx_packets = stats->tx_errs = stats->tx_drop = -1;
+
+ for (i = 0; i < virJSONValueArraySize(jsonMap); i++) {
+ virJSONValuePtr item = virJSONValueArrayGet(jsonMap, i);
+ virJSONValuePtr jsonKey;
+ virJSONValuePtr jsonVal;
+ const char *key;
+ long long val;
+
+ if (!item ||
+ (!(jsonKey = virJSONValueArrayGet(item, 0))) ||
+ (!(jsonVal = virJSONValueArrayGet(item, 1))) ||
+ (!(key = virJSONValueGetString(jsonKey))) ||
+ (virJSONValueGetNumberLong(jsonVal, &val) < 0)) {
+ virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
+ _("Malformed ovs-vsctl output"));
+ goto cleanup;
+ }
+
+ /* The TX/RX fields appear to be swapped here
+ * because this is the host view. */
+ if (STREQ(key, "rx_bytes")) {
+ stats->tx_bytes = val;
+ } else if (STREQ(key, "rx_packets")) {
+ stats->tx_packets = val;
+ } else if (STREQ(key, "rx_errors")) {
+ stats->tx_errs = val;
+ } else if (STREQ(key, "rx_dropped")) {
+ stats->tx_drop = val;
+ } else if (STREQ(key, "tx_bytes")) {
+ stats->rx_bytes = val;
+ } else if (STREQ(key, "tx_packets")) {
+ stats->rx_packets = val;
+ } else if (STREQ(key, "tx_errors")) {
+ stats->rx_errs = val;
+ } else if (STREQ(key, "tx_dropped")) {
+ stats->rx_drop = val;
+ } else {
+ VIR_DEBUG("Unused ovs-vsctl stat key=%s val=%lld", key, val);
+ }
+ }
+
+ if (stats->rx_bytes == -1 &&
+ stats->rx_packets == -1 &&
+ stats->rx_errs == -1 &&
+ stats->rx_drop == -1 &&
+ stats->tx_bytes == -1 &&
+ stats->tx_packets == -1 &&
+ stats->tx_errs == -1 &&
+ stats->tx_drop == -1) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("Interface doesn't have any statistics"));
goto cleanup;
@@ -399,7 +436,7 @@ virNetDevOpenvswitchInterfaceStats(const char *ifname,
ret = 0;
cleanup:
- VIR_FREE(output);
+ virJSONValueFree(jsonStats);
virCommandFree(cmd);
return ret;
}
--
2.23.0