Re: [ovs-dev] [PATCH v10 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats.
Thanks Kevin for reviewing and acknowledging the patch. I will correct the spelling and send the updated patch. Thanks & Regards, Sriram. -Original Message- From: Kevin Traynor Sent: 25 October 2019 20:36 To: Sriram Vatala ; ovs-dev@openvswitch.org; i.maxim...@ovn.org Cc: Ilya Maximets Subject: Re: [PATCH v10 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats. On 24/10/2019 19:21, Sriram Vatala wrote: > From: Ilya Maximets > > This is yet another refactoring for upcoming detailed drop stats. > It allowes to use single function for all the software calculated s/allowes/allows > 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 > Cc: Sriram Vatala > Signed-off-by: Ilya Maximets > Signed-off-by: Sriram Vatala Acked-by: Kevin Traynor > --- > 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, \ > -
Re: [ovs-dev] [PATCH v10 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats.
On 24/10/2019 19:21, Sriram Vatala wrote: > From: Ilya Maximets > > This is yet another refactoring for upcoming detailed drop stats. > It allowes to use single function for all the software calculated s/allowes/allows > 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 > Cc: Sriram Vatala > Signed-off-by: Ilya Maximets > Signed-off-by: Sriram Vatala Acked-by: Kevin Traynor > --- > 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
Re: [ovs-dev] [PATCH v10 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats.
Hi, Please review the patch set v10. Changes since v9 : Patch - 1: Fixed the issue with index inside the for loop for copying the dpdk extended stats inside 'netdev_dpdk_get_custom_stats' function. Patch - 2: Modified the commit message. Changed the custom stats prefix from 'netdev_dpdk_' to 'ovs_' Added note about the prefix 'ovs_' in dpdk documentation. Addressed other comments given by Kevin. Thanks & Regards, Sriram. -Original Message- From: Sriram Vatala Sent: 24 October 2019 23:51 To: ovs-dev@openvswitch.org; i.maxim...@ovn.org; ktray...@redhat.com Cc: Ilya Maximets ; Sriram Vatala Subject: [PATCH v10 1/3] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats. From: Ilya Maximets 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. Co-authored-by: Sriram Vatala Cc: Sriram Vatala Signed-off-by: Ilya Maximets Signed-off-by: Sriram Vatala --- 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, \ -