Thread (71 messages) 71 messages, 6 authors, 2016-06-20

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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help