[dpdk-dev] [PATCH v2] Change rte_eal_vdev_init to update port_id
rkerur at gmail.com
Tue Aug 25 19:59:59 CEST 2015
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.
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
> +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?
More information about the dev