[dpdk-dev] [RFC PATCH 0/6] remove pci driver from vdevs

[Neil Horman]

On Mon, Aug 31, 2015 at 11:21:46AM +0000, Iremonger, Bernard wrote:
> Hi Keith, Neil,
> 
> <snip>
> 
> > >> > On Thu, Aug 27, 2015 at 04:40:35PM +0100, Bernard Iremonger wrote:
> > >> > > There is a dummy pci driver in the vdev PMD's at present.
> > >> > > This RFC proposes to remove the pci driver from the vdev PMD's.
> > >> > > Changes have been made to librte_ether to handle vdevs which do
> > >> > > not
> > >> > have a pci driver.
> > >> > >
> > >> > > The pdev PMD's should work as before with the changes to
> > >>librte_ether
> > >> > > The vdev PMD's which still have a pci driver should work as
> > >> > > before
> > >>with the
> > >> > librte_ether changes.
> > >> > >
> > >> > > The following vdev PMD's have had the  pci driver removed
> > >> > >
> > >> > > bonding PMD
> > >> > > null PMD
> > >> > > pcap PMD
> > >> > > ring PMD
> > >> >
> > >> > Any reason there is no patch for the af_packet driver?
> > >> >
> > >> > John
> > >>
> > >> I have just modified the Intel vdev PMD's.
> > >> It would be best if the owners of the non Intel vdev's submitted
> > >>patches for their drivers.
> > >>
> > >I disagree.  Its ok given that this is an RFC patch I suppose, but if
> > >you intend to actually propose this change for review, you need to
> > >modify all affected drivers in a single commit.  Asking individual
> > >driver maintainers to submit patches to not access a struct element
> > >that is removed in a separate patch will by definition cause FTBFS
> > >errors.  All references to the structure member being removed must also
> > >be eliminated in the same or a prior commit, preferably the former.
> > 
> > +1, if you introduce a chance that effects other places in the
> > code/drivers then you must also make the changes to those parts as well.
> > It really should not be an option IMO.
> > >
> > >Neil
> 
> < snip >
> 
> > Regards,
> > ++Keith
> > Intel Corporation
> > 
> > 
> Firstly no struct element has been removed from struct rte_eth_dev {} in the eth_dev patch.
> A dev_flags  element has been added to struct rte_eth_dev{}
> A numa_node element has been added to struct rte_eth_dev_data{}.
> Unmodified VDEV's and PDEV's will not be aware of these new elements and will be unaffected by the eth_dev changes.
> 

Then NAK to the entire series, as it doesn't address this:
http://dpdk.org/ml/archives/dev/2015-July/022107.html

If that was your intent.  The desire that Thomas had was to refactor the bus
code so that a drivers bus type was independent of the devices it registered.
This really just makes filling out the pci driver struct optional (which likely
becomes very messy down the road).  I think what's being sought is something
like this: 

1) remove the pci driver field from the ethdev struct, perhaps replacing it with
a union pointer to all the bus type structures supported (though I don't think
thats required)

2) modify the driver init routine so that each driver module can register with
whatever bus type it supports.

3) add a per-bus-instance init routine (akin to the probe routine in the linux
and bsd driver models) called once for each instance of that device type found
on a given bus.  This routine allocates and registers an ethernet device
(optionally associating the bus type with the device using the pointer suggested
in (1)

There are other ways to do this of course, this is just a suggestion.  The point
being, the goal is to remove the explicit relationship of a ethdev to a pci
device, as that relationship is clearly not guaranteed.  Doing so allows us to
remove the need to determine bus type at init time, which would be a big
benefit.

Regards
Neil

> Regards,
> 
> Bernard.
> 
> 
> 
> 
>