On 2/7/2022 12:56 PM, Morten Brørup wrote:
From: Ferruh Yigit [mailto:ferruh.yi...@intel.com]
Sent: Monday, 7 February 2022 13.36

On 2/7/2022 12:18 PM, Morten Brørup wrote:
From: Ferruh Yigit [mailto:ferruh.yi...@intel.com]
Sent: Monday, 7 February 2022 12.46

On 2/7/2022 1:47 AM, Min Hu (Connor) wrote:
Added the ethdev dump API which provides functions for query
private
info

Isn't API and function are same thing in this contexts?

from device. There exists many private properties in different PMD
drivers,
such as adapter state, Rx/Tx func algorithm in hns3 PMD. The
information of
these properties is important for debug. As the information is
private,
the new API is introduced.>

In the patch title 'ethdev' is duplicated, can you fix it?


Signed-off-by: Min Hu (Connor) <humi...@huawei.com>
Acked-by: Morten Brørup <m...@smartsharesystems.com>
Acked-by: Ray Kinsella <m...@ashroe.eu>
Acked-by: Ajit Khaparde <ajit.khapa...@broadcom.com>

[...]

@@ -990,6 +990,20 @@ typedef int
(*eth_representor_info_get_t)(struct
rte_eth_dev *dev,
    typedef int (*eth_rx_metadata_negotiate_t)(struct rte_eth_dev
*dev,
                                       uint64_t *features);

+/**
+ * @internal
+ * Dump ethdev private info to a file.
+ *

It doesn't dump the 'ethdev' private info, it dumps the private info
from device.

It seems perfectly clear to me. How would you prefer it phrased
instead?


What described in the document is more accurate,
"query private info from device".

What we are dumping here is not ethdev private info, it is device
private info,
and we really don't know what that data may be in the ethdev layer.

Also there is a chance that 'ethdev private info' can be confused with
'ethdev->data->dev_private'

OK. Now I got your point! The difference is very subtle.


[...]


+ */
+__rte_experimental
+int rte_eth_dev_priv_dump(FILE *file, uint16_t port_id);
+

What do you think to have the 'port_id' as first argument to be
consistent
with the other APIs?

The _dump APIs in other libraries have the file pointer as the first
parameter, so let's follow that convention here too. No need to move
the port_id parameter here.


Yes, for most of the _dump() APIs, file pointer seems is the first
argument,
bu they are from various libraries.

Within the ethdev APIs, I think it makes sense that all APIs start with
'port_id' parameter for consistency, like done in:
rte_flow_dev_dump(uint16_t port_id, ...)

Only rte_dma_dump() has the file pointer last, and I didn't catch it
when the function was defined.


OK. Then I agree with you about following the convention like 
rte_flow_dev_dump() with the port_id first.

I even think Connor got it right the first time, and I proposed following the 
other convention.


Ahh, may bad I missed that, sorry for not commenting on time.


It's not easy when there are two opposite conventions. :-)


Yep, that is the main issue.

Reply via email to