On 2/3/2017 10:02 AM, Iremonger, Bernard wrote:

Hi Konstantin,

quoted

-----Original Message-----
From: Ananyev, Konstantin
Sent: Friday, February 3, 2017 9:50 AM
To: Iremonger, Bernard <redacted>; Yigit, Ferruh
[off-list ref]; Lu, Wenzhuo [off-list ref];
dev@dpdk.org
Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: clean up rte_eth_dev_info_get

Hi Bernard,

quoted

-----Original Message-----
From: Iremonger, Bernard
Sent: Friday, February 3, 2017 9:21 AM
To: Ananyev, Konstantin <redacted>; Yigit, Ferruh
[off-list ref]; Lu, Wenzhuo [off-list ref];
dev@dpdk.org
Subject: RE: [dpdk-dev] [PATCH] net/ixgbe: clean up
rte_eth_dev_info_get

Hi Konstantin,

quoted

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ananyev,
Konstantin
Sent: Wednesday, February 1, 2017 6:10 PM
To: Yigit, Ferruh <redacted>; Lu, Wenzhuo
[off-list ref]; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: clean up
rte_eth_dev_info_get

quoted

-----Original Message-----
From: Yigit, Ferruh
Sent: Wednesday, February 1, 2017 5:40 PM
To: Ananyev, Konstantin <redacted>; Lu,
Wenzhuo [off-list ref]; dev@dpdk.org
Subject: Re: [dpdk-dev] [PATCH] net/ixgbe: clean up
rte_eth_dev_info_get

On 2/1/2017 4:24 PM, Ananyev, Konstantin wrote:

quoted

Hi Wenzhuo,

quoted

-----Original Message-----
From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Wenzhuo

Lu

quoted

quoted

quoted

quoted

quoted

Sent: Wednesday, January 25, 2017 2:39 AM
To: dev@dpdk.org
Cc: Lu, Wenzhuo <redacted>
Subject: [dpdk-dev] [PATCH] net/ixgbe: clean up
rte_eth_dev_info_get

It'not appropriate to call rte_eth_dev_info_get in PMD, as
rte_eth_dev_info_get need to get info from PMD.
Remove rte_eth_dev_info_get from PMD code and get the info
directly.

Signed-off-by: Wenzhuo Lu <redacted>
---
 drivers/net/ixgbe/ixgbe_ethdev.c | 144
++++++++++++++++++---------------------
 1 file changed, 68 insertions(+), 76 deletions(-)

diff --git a/drivers/net/ixgbe/ixgbe_ethdev.c

b/drivers/net/ixgbe/ixgbe_ethdev.c
index 64ce55a..f14a68b 100644

--- a/drivers/net/ixgbe/ixgbe_ethdev.c
+++ b/drivers/net/ixgbe/ixgbe_ethdev.c

@@ -4401,17 +4401,17 @@ static int

ixgbevf_dev_xstats_get_names(__rte_unused struct rte_eth_dev *dev,

quoted

quoted

quoted

 	int rar_entry;
 	uint8_t *new_mac = (uint8_t *)(mac_addr);
 	struct rte_eth_dev *dev;
-	struct rte_eth_dev_info dev_info;
+	struct rte_pci_device *pci_dev;

 	RTE_ETH_VALID_PORTID_OR_ERR_RET(port, -ENODEV);

 	dev = &rte_eth_devices[port];
-	rte_eth_dev_info_get(port, &dev_info);
+	pci_dev = IXGBE_DEV_TO_PCI(dev);

-	if (is_ixgbe_pmd(dev_info.driver_name) != 0)
+	if (is_ixgbe_pmd(dev->data->drv_name))
 		return -ENOTSUP;

I wonder why do we need now that it is really an ixgbe device
all over the

place?

quoted

This device specific API, so it is missing merits of abstraction
layer, application can these APIs with any port_id, API should be
protected

for it.

Ah ok, my bad - didn't realize from the patch that it affects only
device specific API :) Would It be too much hassle to move these
functions into a separate file (rte_ixgbe_pmd.c or so)?
Konstantin

quoted

quoted

Konstantin

All the device specific API functions are prefixed with rte_pmd_ixgbe

That's ok.

quoted

so I don't think a separate file is necessary.

So far I didn't say it is necessary.
Though I think it would be good to group these functions in a separate file to
help avoid confusion (as happened to me) and keep ixgbe_ethdev.c smaller
and cleaner.
Again would be easier to maintain things in future, when more folks will
come up with some extensions for it.
That's why I am asking:  would it be a lot of work to do?
It is probably worth doing it now, while we have this API relatively small.
Konstantin

I would need to investigate what is involved in moving the rte_pmd_ixgbe_* functions to a separate file.
They may be using some of the static functions and data in the ixgbe_ethdev.c  file which could be a problem.
The rte_pmd_ixgbe_* functions are considered an "interim solution" until a "generic solution" is agreed.
It might be best to postpone this work until the "generic solution" is agreed.

Still I believe it is good idea to move these functions into a separate
file, but not for this release J.

For PMD specific APIs, even we keep using direct API call method or
benefit from abstraction layer, means of using ioctl perhaps, these APIs
will be implemented in the PMD. And logically grouping them is good idea.
Briefly, I don't think we need to wait until "generic solution" agreed
for this.

But, of course an investigation needs to be done on required effort, and
decide based on that.

Thanks,
ferruh

Regards,

Bernard.