Hi Thomas, David Let us know how you want us to fix this? To fix rte_eal_vdev_init and rte_eal_pci_probe_one to return allocated port_id we had 2 approaches mentioned in earlier discussion. In addition to those we have another approach with changes isolated only to rte_ether component. I am attaching diffs (preliminary) with this email. Please let us know your inputs since it involves EAL component.
Thanks, Ravi On Thu, Aug 20, 2015 at 8:33 PM, Tetsuya Mukawa <mukawa at igel.co.jp> wrote: > On 2015/08/21 4:16, Ravi Kerur wrote: > > > > > /** > > > * Uninitalize a driver specified by name. > > > @@ -125,6 +127,38 @@ int rte_eal_vdev_init(const char *name, > > const char *args); > > > */ > > > int rte_eal_vdev_uninit(const char *name); > > > > > > +/** > > > + * Given name, return port_id associated with the device. > > > + * > > > + * @param name > > > + * Name associated with device. > > > + * @param port_id > > > + * The port identifier of the device. > > > + * > > > + * @return > > > + * - 0: Success. > > > + * - -EINVAL: NULL string (name) > > > + * - -ENODEV failure > > > > Please define above in 'rte_ethdev.h.' > > > > > > Hi Tetsuya, > > > > I would like to take a step back and explain why function declarations > > are in rte_dev.h and not in rte_ethdev.h > > > > Approach 1: > > Initially I thought of modifying driver init routine to return/update > > port_id as the init routine is the place port_id gets allocated and it > > would have been clean approach. However, it required changes to all > > PMD_VDEV driver init routine to modify function signature for the > > changes which I thought may be an overkill. > > > > Approach 2: > > Instead I chose to define 2 functions in librte_ether/rte_ethdev.c and > > make use of it. In this approach new functions are invoked from > > librte_eal/common/.c to get port_id. If I had new function > > declarations in rte_ethdev.h and included that file in > > librte_eal/common/.c files it creates circular dependancy and > > compilation fails, hence I took hybrid approach of definitions in > > librte_ether and declarations in librte_eal. > > > > Please let me know if there is a better approach to take care of your > > comments. As it stands declarations cannot be moved to rte_ethdev.h > > for compilation reasons. > > > > Thanks, > > Ravi > > > > Hi Ravi, > (Adding David) > > I appreciate your description. I understand why you define the functions > in rte_dev.h. > > About Approach2, I don't know a way to implement cleanly. > I guess if we define the functions in rte_dev.h, the developers who want > to use the functions will be confused because the functions are > implemented in ethdev.c, but it is needed to include rte_dev.h. > > To avoid such a confusion, following implementation might be worked, but > I am not sure this cording style is allowed in eal library. > > ---------------------------- > Define the functions in rte_ethdev.h, then fix librte_eal/common/.c > files like below > > ex) lib/librte_eal/common/eal_common_dev.c > ---------------------------- > +#include <rte_pci.h> > #include <rte_dev.h> > #include <rte_devargs.h> > #include <rte_debug.h> > > #include "eal_private.h" > > +extern int rte_eth_dev_get_port_by_name(const char *name, uint8_t > *port_id); > +extern int rte_eth_dev_get_port_by_addr(const struct rte_pci_addr > *addr, uint8_t *port_id); > ---------------------------- > > In this case, the developer might be able to notice that above usage in > eal library is some kind of exception. But I guess the DPDK code won't > be clean if we start having a exception. > So it might be good to choose Approach1, because apparently it is > straight forward. > Anyone won't be confused and complained about coding style. > > > Hi David, > > Could you please let us know what you think? > Do you have a good approach for this? > > Thanks, > Tetsuya > >
