Re: [PATCH v3 01/10] rte: change xstats to use integer ids
From: Remy Horton <hidden>
Date: 2016-06-08 11:16:39
'noon, On 08/06/2016 10:37, Thomas Monjalon wrote:
2016-05-30 11:48, Remy Horton:quoted
struct rte_eth_xstats { + /* FIXME: Remove name[] once remaining drivers converted */ char name[RTE_ETH_XSTATS_NAME_SIZE];What is the plan? This field must be deprecated with an attribute. We cannot have 2 different APIs depending of the driver.
This is where it gets logistically tricky.. Since there's an API/ABI breakage notice in place on this, my own preference would be to have the entire patchset quashed into a single patch. Problem is that rte/app changes (patches 1 & 7-9) are normally applied via master whereas driver changes (patches 2-6) go in via dpdk-next-net - it is not clear to me how patches should be submitted for this case..
What are the remaining drivers to convert?
Opps, none. All relevant drivers are converted..
This structure and the other one (rte_eth_xstats) are badly named. There is only one stat in each. So they should not have the plural form. rte_eth_xstat and rte_eth_xstat_name would be better.
I kept rte_eth_xstats as it was the name already in use within DPDK. Will change the other.
quoted
+int rte_eth_xstats_count(uint8_t port_id);This function is useless because we can have the count with rte_eth_xstats_get(p, NULL, 0) By the way it would be more consistent to have the same behaviour in rte_eth_xstats_names().
Feedback I got with earlier patches was that a seperate count function was preferable to overloading the fetch function using *data==NULL - is the use of the latter specifically preferred? Other comments noted. ..Remy