On 15.10.2019 15:35, Kevin Traynor wrote:
With corrected email for Ilya.On 15/10/2019 14:33, Kevin Traynor wrote:On 21/09/2019 03:40, Sriram Vatala wrote:From: Ilya Maximets <[email protected]> This is yet another refactoring for upcoming detailed drop stats. It allowes to use single function for all the software calculated statistics in netdev-dpdk for both vhost and ETH ports. UINT64_MAX used as a marker for non-supported statistics in a same way as it's done in bridge.c for common netdev stats.Hi Ilya, one comment below, thanks, Kevin.Cc: Sriram Vatala <[email protected]> Signed-off-by: Ilya Maximets <[email protected]> Signed-off-by: Sriram Vatala <[email protected]> --- lib/netdev-dpdk.c | 67 +++++++++++++++++++++++++++-------------------- 1 file changed, 39 insertions(+), 28 deletions(-) diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index bc20d6843..652b57e3b 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -471,6 +471,8 @@ struct netdev_rxq_dpdk { static void netdev_dpdk_destruct(struct netdev *netdev); 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_clear_xstats(struct netdev_dpdk *dev);int netdev_dpdk_get_vid(const struct netdev_dpdk *dev);@@ -1171,7 +1173,7 @@ common_construct(struct netdev *netdev, dpdk_port_t port_no, dev->rte_xstats_ids = NULL; dev->rte_xstats_ids_size = 0;- dev->tx_retries = 0;+ dev->tx_retries = (dev->type == DPDK_DEV_VHOST) ? 0 : UINT64_MAX;return 0;} @@ -2771,7 +2773,9 @@ netdev_dpdk_get_custom_stats(const struct netdev *netdev,uint32_t i;struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); - int rte_xstats_ret; + int rte_xstats_ret, sw_stats_size; + + netdev_dpdk_get_sw_custom_stats(netdev, custom_stats);ovs_mutex_lock(&dev->mutex); @@ -2786,12 +2790,13 @@ netdev_dpdk_get_custom_stats(const struct netdev *netdev,if (rte_xstats_ret > 0 && rte_xstats_ret <= dev->rte_xstats_ids_size) {- custom_stats->size = rte_xstats_ret;- custom_stats->counters = - (struct netdev_custom_counter *) xcalloc(rte_xstats_ret, - sizeof(struct netdev_custom_counter)); + sw_stats_size = custom_stats->size; + custom_stats->size += rte_xstats_ret; + custom_stats->counters = xrealloc(custom_stats->counters, + custom_stats->size * + sizeof *custom_stats->counters);- for (i = 0; i < rte_xstats_ret; i++) {+ for (i = sw_stats_size; i < sw_stats_size + rte_xstats_ret; i++) { ovs_strlcpy(custom_stats->counters[i].name, netdev_dpdk_get_xstat_name(dev, dev->rte_xstats_ids[i]),I think you need to add another array index counter for ret_xstats_ids[] and values[] as they are still using i, but i is now starting with sw_stats_size and not 0 anymore.
Good catch.
I didn't actually test this code hoping that it'll be tested along
with the second patch.
For this part we could just move the 'sw_stats_size' from
the loop counter to the counters[i]. Like this:
for (i = 0; i < rte_xstats_ret; i++) {
ovs_strlcpy(custom_stats->counters[sw_stats_size + i].name,
netdev_dpdk_get_xstat_name(dev,
dev->rte_xstats_ids[i]),
NETDEV_CUSTOM_STATS_NAME_SIZE);
custom_stats->counters[sw_stats_size + i].value = values[i];
}
@@ -2801,8 +2806,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); - custom_stats->counters = NULL; - custom_stats->size = 0; /* Let's clear statistics cache, so it will be * reconfigured */ netdev_dpdk_clear_xstats(dev); @@ -2817,39 +2820,47 @@ netdev_dpdk_get_custom_stats(const struct netdev *netdev, }static int-netdev_dpdk_vhost_get_custom_stats(const struct netdev *netdev, - struct netdev_custom_stats *custom_stats) +netdev_dpdk_get_sw_custom_stats(const struct netdev *netdev, + struct netdev_custom_stats *custom_stats) { struct netdev_dpdk *dev = netdev_dpdk_cast(netdev); - int i; + int i, n;-#define VHOST_CSTATS \- VHOST_CSTAT(tx_retries) +#define SW_CSTATS \ + SW_CSTAT(tx_retries)-#define VHOST_CSTAT(NAME) + 1- custom_stats->size = VHOST_CSTATS; -#undef VHOST_CSTAT +#define SW_CSTAT(NAME) + 1 + custom_stats->size = SW_CSTATS; +#undef SW_CSTAT custom_stats->counters = xcalloc(custom_stats->size, sizeof *custom_stats->counters); - i = 0; -#define VHOST_CSTAT(NAME) \ - ovs_strlcpy(custom_stats->counters[i++].name, #NAME, \ - NETDEV_CUSTOM_STATS_NAME_SIZE); - VHOST_CSTATS; -#undef VHOST_CSTATovs_mutex_lock(&dev->mutex); rte_spinlock_lock(&dev->stats_lock);i = 0; -#define VHOST_CSTAT(NAME) \ +#define SW_CSTAT(NAME) \ custom_stats->counters[i++].value = dev->NAME; - VHOST_CSTATS; -#undef VHOST_CSTAT + SW_CSTATS; +#undef SW_CSTAT rte_spinlock_unlock(&dev->stats_lock);ovs_mutex_unlock(&dev->mutex); + i = 0;+ n = 0; +#define SW_CSTAT(NAME) \ + if (custom_stats->counters[i].value != UINT64_MAX) { \ + ovs_strlcpy(custom_stats->counters[n].name, #NAME, \ + NETDEV_CUSTOM_STATS_NAME_SIZE); \ + custom_stats->counters[n].value = custom_stats->counters[i].value; \ + n++; \ + } \ + i++; + SW_CSTATS; +#undef SW_CSTAT + + custom_stats->size = n; return 0; }@@ -4433,7 +4444,7 @@ static const struct netdev_class dpdk_vhost_class = {.send = netdev_dpdk_vhost_send, .get_carrier = netdev_dpdk_vhost_get_carrier, .get_stats = netdev_dpdk_vhost_get_stats, - .get_custom_stats = netdev_dpdk_vhost_get_custom_stats, + .get_custom_stats = netdev_dpdk_get_sw_custom_stats, .get_status = netdev_dpdk_vhost_user_get_status, .reconfigure = netdev_dpdk_vhost_reconfigure, .rxq_recv = netdev_dpdk_vhost_rxq_recv, @@ -4449,7 +4460,7 @@ static const struct netdev_class dpdk_vhost_client_class = { .send = netdev_dpdk_vhost_send, .get_carrier = netdev_dpdk_vhost_get_carrier, .get_stats = netdev_dpdk_vhost_get_stats, - .get_custom_stats = netdev_dpdk_vhost_get_custom_stats, + .get_custom_stats = netdev_dpdk_get_sw_custom_stats, .get_status = netdev_dpdk_vhost_user_get_status, .reconfigure = netdev_dpdk_vhost_client_reconfigure, .rxq_recv = netdev_dpdk_vhost_rxq_recv,
_______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
