[ovs-dev] [PATCH v9 1/2] netdev-dpdk: Reuse vhost function for dpdk ETH custom stats.
Ilya Maximets
i.maximets at ovn.org
Tue Oct 15 13:43:45 UTC 2019
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 <i.maximets at samsung.com>
>>>
>>> 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 <sriram.v at altencalsoftlabs.com>
>>> Signed-off-by: Ilya Maximets <i.maximets at samsung.com>
>>> Signed-off-by: Sriram Vatala <sriram.v at altencalsoftlabs.com>
>>> ---
>>> 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_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;
>>> }
>>>
>>> @@ -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,
>>>
>>
>
More information about the dev
mailing list