When changing number of Rx or Tx queues, per queue basic stats can be
renumbered in DPDK ethdev layer [1].
OVS maintains an internal xstats IDs cache that was refreshed when a
cached id was not valid anymore (in netdev_dpdk_get_custom_stats) or if
a new DPDK port was created.
This did not handle changes of Rx/Tx queues count.
For example, with a mlx5 port:
$ ovs-vsctl set interface dpdk0 options:n_rxq=2
$ ovs-vsctl get interface dpdk0 statistics |
sed -e 's#[{}]##g' -e 's#, #\n#g' |
grep rx_q._errors
rx_q0_errors=0
Move the cache filling after reconfiguring and starting the port.
There is no need to flush the cache in netdev_dpdk_get_custom_stats.
While at it, the xstats code can be cleaned up:
- remove wrong or Lapalissade comments,
- don't check x*alloc return value,
- expect that consecutive calls to xstats API return the same number of
elements,
- only write to dev-> when all DPDK calls succeeded,
- add missing lock annotations to netdev_dpdk_clear_xstats and
netdev_dpdk_get_xstat_name,
1: https://git.dpdk.org/dpdk/tree/lib/librte_ethdev/rte_ethdev.c?h=v20.11#n2696
Reported-at:
https://mail.openvswitch.org/pipermail/ovs-dev/2021-November/389456.html
Signed-off-by: David Marchand <[email protected]>
---
lib/netdev-dpdk.c | 160 ++++++++++++++++++----------------------------
1 file changed, 62 insertions(+), 98 deletions(-)
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c
index ca92c947a2..51bb41551b 100644
--- a/lib/netdev-dpdk.c
+++ b/lib/netdev-dpdk.c
@@ -540,6 +540,7 @@ static void netdev_dpdk_vhost_destruct(struct netdev
*netdev);
static int netdev_dpdk_get_sw_custom_stats(const struct netdev *,
struct netdev_custom_stats *);
+static void netdev_dpdk_configure_xstats(struct netdev_dpdk *dev);
static void netdev_dpdk_clear_xstats(struct netdev_dpdk *dev);
int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);
@@ -1161,6 +1162,8 @@ dpdk_eth_dev_init(struct netdev_dpdk *dev)
}
dev->started = true;
+ netdev_dpdk_configure_xstats(dev);
+
rte_eth_promiscuous_enable(dev->port_id);
rte_eth_allmulticast_enable(dev->port_id);
@@ -1559,23 +1562,19 @@ netdev_dpdk_dealloc(struct netdev *netdev)
static void
netdev_dpdk_clear_xstats(struct netdev_dpdk *dev)
+ OVS_REQUIRES(dev->mutex)
{
- /* If statistics are already allocated, we have to
- * reconfigure, as port_id could have been changed. */
- if (dev->rte_xstats_names) {
- free(dev->rte_xstats_names);
- dev->rte_xstats_names = NULL;
- dev->rte_xstats_names_size = 0;
- }
- if (dev->rte_xstats_ids) {
- free(dev->rte_xstats_ids);
- dev->rte_xstats_ids = NULL;
- dev->rte_xstats_ids_size = 0;
- }
+ free(dev->rte_xstats_names);
+ dev->rte_xstats_names = NULL;
+ dev->rte_xstats_names_size = 0;
+ free(dev->rte_xstats_ids);
+ dev->rte_xstats_ids = NULL;
+ dev->rte_xstats_ids_size = 0;
}
-static const char*
+static const char *
netdev_dpdk_get_xstat_name(struct netdev_dpdk *dev, uint64_t id)
+ OVS_REQUIRES(dev->mutex)
{
if (id >= dev->rte_xstats_names_size) {
return "UNKNOWN";
@@ -1583,101 +1582,70 @@ netdev_dpdk_get_xstat_name(struct netdev_dpdk *dev,
uint64_t id)
return dev->rte_xstats_names[id].name;
}
-static bool
+static void
netdev_dpdk_configure_xstats(struct netdev_dpdk *dev)
OVS_REQUIRES(dev->mutex)
{
+ struct rte_eth_xstat_name *rte_xstats_names = NULL;
+ struct rte_eth_xstat *rte_xstats = NULL;
+ int rte_xstats_names_size;
int rte_xstats_len;
- bool ret;
- struct rte_eth_xstat *rte_xstats;
- uint64_t id;
- int xstats_no;
const char *name;
+ uint64_t id;
- /* Retrieving all XSTATS names. If something will go wrong
- * or amount of counters will be equal 0, rte_xstats_names
- * buffer will be marked as NULL, and any further xstats
- * query won't be performed (e.g. during netdev_dpdk_get_stats
- * execution). */
+ netdev_dpdk_clear_xstats(dev);
- ret = false;
- rte_xstats = NULL;
+ rte_xstats_names_size = rte_eth_xstats_get_names(dev->port_id, NULL, 0);
+ if (rte_xstats_names_size < 0) {
+ VLOG_WARN("Cannot get XSTATS names for port: "DPDK_PORT_ID_FMT,
+ dev->port_id);
+ goto out;
+ }
- if (dev->rte_xstats_names == NULL || dev->rte_xstats_ids == NULL) {
- dev->rte_xstats_names_size =
- rte_eth_xstats_get_names(dev->port_id, NULL, 0);
+ rte_xstats_names = xcalloc(rte_xstats_names_size,
+ sizeof *rte_xstats_names);
+ rte_xstats_len = rte_eth_xstats_get_names(dev->port_id,
+ rte_xstats_names,
+ rte_xstats_names_size);
+ if (rte_xstats_len < 0 || rte_xstats_len != rte_xstats_names_size) {
+ VLOG_WARN("Cannot get XSTATS names for port: "DPDK_PORT_ID_FMT,
+ dev->port_id);
+ goto out;
+ }
- if (dev->rte_xstats_names_size < 0) {
- VLOG_WARN("Cannot get XSTATS for port: "DPDK_PORT_ID_FMT,
- dev->port_id);
- dev->rte_xstats_names_size = 0;
- } else {
- /* Reserve memory for xstats names and values */
- dev->rte_xstats_names = xcalloc(dev->rte_xstats_names_size,
- sizeof *dev->rte_xstats_names);
-
- if (dev->rte_xstats_names) {
- /* Retreive xstats names */
- rte_xstats_len =
- rte_eth_xstats_get_names(dev->port_id,
- dev->rte_xstats_names,
- dev->rte_xstats_names_size);
-
- if (rte_xstats_len < 0) {
- VLOG_WARN("Cannot get XSTATS names for port: "
- DPDK_PORT_ID_FMT, dev->port_id);
- goto out;
- } else if (rte_xstats_len != dev->rte_xstats_names_size) {
- VLOG_WARN("XSTATS size doesn't match for port: "
- DPDK_PORT_ID_FMT, dev->port_id);
- goto out;
- }
+ rte_xstats = xcalloc(rte_xstats_names_size, sizeof *rte_xstats);
+ rte_xstats_len = rte_eth_xstats_get(dev->port_id, rte_xstats,
+ rte_xstats_names_size);
+ if (rte_xstats_len < 0 || rte_xstats_len != rte_xstats_names_size) {
+ VLOG_WARN("Cannot get XSTATS for port: "DPDK_PORT_ID_FMT,
+ dev->port_id);
+ goto out;
+ }
- dev->rte_xstats_ids = xcalloc(dev->rte_xstats_names_size,
- sizeof(uint64_t));
-
- /* We have to calculate number of counters */
- rte_xstats = xmalloc(rte_xstats_len * sizeof *rte_xstats);
- memset(rte_xstats, 0xff, sizeof *rte_xstats * rte_xstats_len);
-
- /* Retreive xstats values */
- if (rte_eth_xstats_get(dev->port_id, rte_xstats,
- rte_xstats_len) > 0) {
- dev->rte_xstats_ids_size = 0;
- xstats_no = 0;
- for (uint32_t i = 0; i < rte_xstats_len; i++) {
- id = rte_xstats[i].id;
- name = netdev_dpdk_get_xstat_name(dev, id);
- /* We need to filter out everything except
- * dropped, error and management counters */
- if (string_ends_with(name, "_errors") ||
- strstr(name, "_management_") ||
- string_ends_with(name, "_dropped")) {
-
- dev->rte_xstats_ids[xstats_no] = id;
- xstats_no++;
- }
- }
- dev->rte_xstats_ids_size = xstats_no;
- ret = true;
- } else {
- VLOG_WARN("Can't get XSTATS IDs for port: "
- DPDK_PORT_ID_FMT, dev->port_id);
- }
+ dev->rte_xstats_names = rte_xstats_names;
+ rte_xstats_names = NULL;
+ dev->rte_xstats_names_size = rte_xstats_names_size;
- free(rte_xstats);
- }
+ dev->rte_xstats_ids = xcalloc(rte_xstats_names_size,
+ sizeof *dev->rte_xstats_ids);
+ for (unsigned int i = 0; i < rte_xstats_names_size; i++) {
+ id = rte_xstats[i].id;
+ name = netdev_dpdk_get_xstat_name(dev, id);
+
+ /* We need to filter out everything except dropped, error and
+ * management counters. */
+ if (string_ends_with(name, "_errors") ||
+ strstr(name, "_management_") ||
+ string_ends_with(name, "_dropped")) {
+
+ dev->rte_xstats_ids[dev->rte_xstats_ids_size] = id;
+ dev->rte_xstats_ids_size++;
}
- } else {
- /* Already configured */
- ret = true;
}
out:
- if (!ret) {
- netdev_dpdk_clear_xstats(dev);
- }
- return ret;
+ free(rte_xstats);
+ free(rte_xstats_names);
}
static bool
@@ -1963,7 +1931,6 @@ netdev_dpdk_set_config(struct netdev *netdev, const
struct smap *args,
dev->devargs = xstrdup(new_devargs);
dev->port_id = new_port_id;
netdev_request_reconfigure(&dev->up);
- netdev_dpdk_clear_xstats(dev);
err = 0;
}
}
@@ -3196,7 +3163,7 @@ netdev_dpdk_get_custom_stats(const struct netdev *netdev,
ovs_mutex_lock(&dev->mutex);
- if (netdev_dpdk_configure_xstats(dev)) {
+ if (dev->rte_xstats_ids_size > 0) {
uint64_t *values = xcalloc(dev->rte_xstats_ids_size,
sizeof(uint64_t));
@@ -3223,9 +3190,6 @@ netdev_dpdk_get_custom_stats(const struct netdev *netdev,
} else {
VLOG_WARN("Cannot get XSTATS values for port: "DPDK_PORT_ID_FMT,
dev->port_id);
- /* Let's clear statistics cache, so it will be
- * reconfigured */
- netdev_dpdk_clear_xstats(dev);
}
free(values);
--
2.23.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
