From 5ca75531afd1244898ee7c8d2307857ba8260cba Mon Sep 17 00:00:00 2001 Message-Id: <5ca75531afd1244898ee7c8d2307857ba8260cba@dist-git> From: Michal Privoznik 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 Reviewed-by: Ján Tomko (cherry picked from commit c297eab52599c91a4cb26b66dbdfe9d07c3142d3) https://bugzilla.redhat.com/show_bug.cgi?id=1759904 Signed-off-by: Michal Privoznik Message-Id: <9904d3bee93a9e103012d088b77999f023acaa2b.1570623892.git.mprivozn@redhat.com> Reviewed-by: Jiri Denemark --- 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