| Message ID | 20191029145006.29048-1-sriram.v@altencalsoftlabs.com |
|---|---|
| State | Accepted |
| Delegated to: | Ilya Maximets |
| Headers | show |
| Series | [ovs-dev,v11,1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats. | expand |
Hi All, Please review the update patch. Changes since v10 : Corrected the spelling mistake in the commit message. Thanks & Regards, Sriram. -----Original Message----- From: Sriram Vatala <sriram.v@altencalsoftlabs.com> Sent: 29 October 2019 20:20 To: ovs-dev@openvswitch.org; ktraynor@redhat.com; i.maximets@ovn.org Cc: Ilya Maximets <i.maximets@samsung.com>; Sriram Vatala <sriram.v@altencalsoftlabs.com> Subject: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats. From: Ilya Maximets <i.maximets@samsung.com> This is yet another refactoring for upcoming detailed drop stats. It allows 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. Co-authored-by: Sriram Vatala <sriram.v@altencalsoftlabs.com> Cc: Sriram Vatala <sriram.v@altencalsoftlabs.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Sriram Vatala <sriram.v@altencalsoftlabs.com> --- lib/netdev-dpdk.c | 69 +++++++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 04e1a2d1b..2cc2516a9 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -474,6 +474,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); @@ -1174,7 +1176,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; } @@ -2775,7 +2777,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); @@ -2790,23 +2794,22 @@ 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++) { - ovs_strlcpy(custom_stats->counters[i].name, + 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[i].value = values[i]; + custom_stats->counters[sw_stats_size + i].value = + values[i]; } } 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); @@ -2821,39 +2824,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_CSTAT ovs_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; } @@ -4437,7 +4448,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, @@ -4453,7 +4464,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, -- 2.20.1
Hi, Please consider this as a gentle remainder. Thanks & Regards, Sriram. -----Original Message----- From: Sriram Vatala <sriram.v@altencalsoftlabs.com> Sent: 30 October 2019 11:57 To: 'ovs-dev@openvswitch.org' <ovs-dev@openvswitch.org>; 'ktraynor@redhat.com' <ktraynor@redhat.com>; 'i.maximets@ovn.org' <i.maximets@ovn.org> Cc: 'Ilya Maximets' <i.maximets@samsung.com> Subject: RE: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats. Hi All, Please review the update patch. Changes since v10 : Corrected the spelling mistake in the commit message. Thanks & Regards, Sriram. -----Original Message----- From: Sriram Vatala <sriram.v@altencalsoftlabs.com> Sent: 29 October 2019 20:20 To: ovs-dev@openvswitch.org; ktraynor@redhat.com; i.maximets@ovn.org Cc: Ilya Maximets <i.maximets@samsung.com>; Sriram Vatala <sriram.v@altencalsoftlabs.com> Subject: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats. From: Ilya Maximets <i.maximets@samsung.com> This is yet another refactoring for upcoming detailed drop stats. It allows 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. Co-authored-by: Sriram Vatala <sriram.v@altencalsoftlabs.com> Cc: Sriram Vatala <sriram.v@altencalsoftlabs.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Sriram Vatala <sriram.v@altencalsoftlabs.com> --- lib/netdev-dpdk.c | 69 +++++++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 04e1a2d1b..2cc2516a9 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -474,6 +474,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); @@ -1174,7 +1176,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; } @@ -2775,7 +2777,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); @@ -2790,23 +2794,22 @@ 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++) { - ovs_strlcpy(custom_stats->counters[i].name, + 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[i].value = values[i]; + custom_stats->counters[sw_stats_size + i].value = + values[i]; } } 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); @@ -2821,39 +2824,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_CSTAT ovs_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; } @@ -4437,7 +4448,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, @@ -4453,7 +4464,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, -- 2.20.1
Hi All, Please consider this as a gentle remainder. Thanks & Regards, Sriram. -----Original Message----- From: Sriram Vatala <sriram.v@altencalsoftlabs.com> Sent: 30 October 2019 11:57 To: 'ovs-dev@openvswitch.org' <ovs-dev@openvswitch.org>; 'ktraynor@redhat.com' <ktraynor@redhat.com>; 'i.maximets@ovn.org' <i.maximets@ovn.org> Cc: 'Ilya Maximets' <i.maximets@samsung.com> Subject: RE: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats. Hi All, Please review the update patch. Changes since v10 : Corrected the spelling mistake in the commit message. Thanks & Regards, Sriram. -----Original Message----- From: Sriram Vatala <sriram.v@altencalsoftlabs.com> Sent: 29 October 2019 20:20 To: ovs-dev@openvswitch.org; ktraynor@redhat.com; i.maximets@ovn.org Cc: Ilya Maximets <i.maximets@samsung.com>; Sriram Vatala <sriram.v@altencalsoftlabs.com> Subject: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats. From: Ilya Maximets <i.maximets@samsung.com> This is yet another refactoring for upcoming detailed drop stats. It allows 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. Co-authored-by: Sriram Vatala <sriram.v@altencalsoftlabs.com> Cc: Sriram Vatala <sriram.v@altencalsoftlabs.com> Signed-off-by: Ilya Maximets <i.maximets@samsung.com> Signed-off-by: Sriram Vatala <sriram.v@altencalsoftlabs.com> --- lib/netdev-dpdk.c | 69 +++++++++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 29 deletions(-) diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 04e1a2d1b..2cc2516a9 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -474,6 +474,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); @@ -1174,7 +1176,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; } @@ -2775,7 +2777,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); @@ -2790,23 +2794,22 @@ 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++) { - ovs_strlcpy(custom_stats->counters[i].name, + 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[i].value = values[i]; + custom_stats->counters[sw_stats_size + i].value = + values[i]; } } 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); @@ -2821,39 +2824,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_CSTAT ovs_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; } @@ -4437,7 +4448,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, @@ -4453,7 +4464,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, -- 2.20.1
On 05/11/2019 04:52, Sriram Vatala wrote: > Hi All, > Please consider this as a gentle remainder. > > Thanks & Regards, > Sriram. > > -----Original Message----- > From: Sriram Vatala <sriram.v@altencalsoftlabs.com> > Sent: 30 October 2019 11:57 > To: 'ovs-dev@openvswitch.org' <ovs-dev@openvswitch.org>; > 'ktraynor@redhat.com' <ktraynor@redhat.com>; 'i.maximets@ovn.org' > <i.maximets@ovn.org> > Cc: 'Ilya Maximets' <i.maximets@samsung.com> > Subject: RE: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH > custom stats. > > Hi All, > Please review the update patch. > > Changes since v10 : > Corrected the spelling mistake in the commit message. > Hi Sriram, I've acked v10, so not planning to re-review unless there are some other changes. thanks, Kevin. p.s. In general if it is only minor changes it is usually ok to keep acks from a previous version. If it is doubtful you can check with the reviewer. > Thanks & Regards, > Sriram. > > -----Original Message----- > From: Sriram Vatala <sriram.v@altencalsoftlabs.com> > Sent: 29 October 2019 20:20 > To: ovs-dev@openvswitch.org; ktraynor@redhat.com; i.maximets@ovn.org > Cc: Ilya Maximets <i.maximets@samsung.com>; Sriram Vatala > <sriram.v@altencalsoftlabs.com> > Subject: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH > custom stats. > > From: Ilya Maximets <i.maximets@samsung.com> > > This is yet another refactoring for upcoming detailed drop stats. > It allows 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. > > Co-authored-by: Sriram Vatala <sriram.v@altencalsoftlabs.com> > Cc: Sriram Vatala <sriram.v@altencalsoftlabs.com> > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > Signed-off-by: Sriram Vatala <sriram.v@altencalsoftlabs.com> > --- > lib/netdev-dpdk.c | 69 +++++++++++++++++++++++++++-------------------- > 1 file changed, 40 insertions(+), 29 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > 04e1a2d1b..2cc2516a9 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -474,6 +474,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); @@ -1174,7 +1176,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; > } > @@ -2775,7 +2777,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); > > @@ -2790,23 +2794,22 @@ 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++) { > - ovs_strlcpy(custom_stats->counters[i].name, > + 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[i].value = values[i]; > + custom_stats->counters[sw_stats_size + i].value = > + values[i]; > } > } 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); @@ -2821,39 +2824,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_CSTAT > > ovs_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; > } > > @@ -4437,7 +4448,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, @@ -4453,7 +4464,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, > -- > 2.20.1 >
Thanks Kevin for your response. @i.maximets@ovn.org : Can you please review the patch set. Thanks & Regards, Sriram. -----Original Message----- From: Kevin Traynor <ktraynor@redhat.com> Sent: 05 November 2019 16:07 To: Sriram Vatala <sriram.v@altencalsoftlabs.com>; ovs-dev@openvswitch.org; i.maximets@ovn.org Cc: 'Ilya Maximets' <i.maximets@samsung.com> Subject: Re: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats. On 05/11/2019 04:52, Sriram Vatala wrote: > Hi All, > Please consider this as a gentle remainder. > > Thanks & Regards, > Sriram. > > -----Original Message----- > From: Sriram Vatala <sriram.v@altencalsoftlabs.com> > Sent: 30 October 2019 11:57 > To: 'ovs-dev@openvswitch.org' <ovs-dev@openvswitch.org>; > 'ktraynor@redhat.com' <ktraynor@redhat.com>; 'i.maximets@ovn.org' > <i.maximets@ovn.org> > Cc: 'Ilya Maximets' <i.maximets@samsung.com> > Subject: RE: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for > dpdk ETH custom stats. > > Hi All, > Please review the update patch. > > Changes since v10 : > Corrected the spelling mistake in the commit message. > Hi Sriram, I've acked v10, so not planning to re-review unless there are some other changes. thanks, Kevin. p.s. In general if it is only minor changes it is usually ok to keep acks from a previous version. If it is doubtful you can check with the reviewer. > Thanks & Regards, > Sriram. > > -----Original Message----- > From: Sriram Vatala <sriram.v@altencalsoftlabs.com> > Sent: 29 October 2019 20:20 > To: ovs-dev@openvswitch.org; ktraynor@redhat.com; i.maximets@ovn.org > Cc: Ilya Maximets <i.maximets@samsung.com>; Sriram Vatala > <sriram.v@altencalsoftlabs.com> > Subject: [PATCH v11 1/3] netdev-dpdk: Reuse vhost function for dpdk > ETH custom stats. > > From: Ilya Maximets <i.maximets@samsung.com> > > This is yet another refactoring for upcoming detailed drop stats. > It allows 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. > > Co-authored-by: Sriram Vatala <sriram.v@altencalsoftlabs.com> > Cc: Sriram Vatala <sriram.v@altencalsoftlabs.com> > Signed-off-by: Ilya Maximets <i.maximets@samsung.com> > Signed-off-by: Sriram Vatala <sriram.v@altencalsoftlabs.com> > --- > lib/netdev-dpdk.c | 69 > +++++++++++++++++++++++++++-------------------- > 1 file changed, 40 insertions(+), 29 deletions(-) > > diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index > 04e1a2d1b..2cc2516a9 100644 > --- a/lib/netdev-dpdk.c > +++ b/lib/netdev-dpdk.c > @@ -474,6 +474,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); @@ -1174,7 > +1176,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; > } > @@ -2775,7 +2777,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); > > @@ -2790,23 +2794,22 @@ 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++) { > - ovs_strlcpy(custom_stats->counters[i].name, > + 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[i].value = values[i]; > + custom_stats->counters[sw_stats_size + i].value = > + values[i]; > } > } 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); @@ -2821,39 +2824,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_CSTAT > > ovs_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; > } > > @@ -4437,7 +4448,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, @@ -4453,7 +4464,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, > -- > 2.20.1 >
On 05.11.2019 13:54, Sriram Vatala wrote: > Thanks Kevin for your response. > > @i.maximets@ovn.org : Can you please review the patch set. I have it on my ToDo list for this week. Best regards, Ilya Maximets.
diff --git a/lib/netdev-dpdk.c b/lib/netdev-dpdk.c index 04e1a2d1b..2cc2516a9 100644 --- a/lib/netdev-dpdk.c +++ b/lib/netdev-dpdk.c @@ -474,6 +474,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); @@ -1174,7 +1176,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; } @@ -2775,7 +2777,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); @@ -2790,23 +2794,22 @@ 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++) { - ovs_strlcpy(custom_stats->counters[i].name, + 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[i].value = values[i]; + custom_stats->counters[sw_stats_size + i].value = values[i]; } } 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); @@ -2821,39 +2824,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_CSTAT ovs_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; } @@ -4437,7 +4448,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, @@ -4453,7 +4464,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,