Re: Proposal for a big eal / ethdev cleanup

[Jan Viktorin <viktorin@rehivetech.com>]

On Mon, 18 Jan 2016 22:11:56 +0100
David Marchand <david.marchand@6wind.com> wrote:

> Jan,
> 
> I was waiting for some others feedbacks before going into the code.
> Glad to see you already tried this.

Of course... I think, it's better to have a particular code (if
possible) to talk about ;). It is quite difficult to see all the
impacts. I hope to see more people to join this discussion.

> 
> 
> On Mon, Jan 18, 2016 at 3:58 PM, Jan Viktorin <viktorin@rehivetech.com> wrote:
> > On Thu, 14 Jan 2016 11:38:16 +0100
> > David Marchand <david.marchand@6wind.com> wrote:  
> >> - no need for a rte_pci_driver reference in rte_pci_device, since we
> >> have the rte_device driver  
> >
> > This is an issue, see below.
> >  
> >>
> >> - rte_pci_driver is modified to embed a rte_driver  
> >
> > The rte_driver and rte_pci_driver are related in a much different way
> > at the moment. The meaning of rte_driver is more like an rte_module in
> > the current DPDK.
> >
> > In fact, we don't have any generic rte_driver suitable for this purpose.
> > Thus, the transition to this model needs to rename rte_driver to
> > rte_module and to introduce a new data structure named rte_driver.
> >
> > Quite confusing... but this is how I understand it.  
> 
> Hum, yes.
> Well, looking at current rte_driver, this code has been first thought
> as a way to load pmd through dso, so yes, this is more like module
> init.
> Then the hotplug has been hooked on this, adding to the confusion.
> 
> 
> > (What is the current relation between rte_pci_device and rte_pci_driver?
> > Is the rte_pci_driver a singleton? I doubt. Well, it cannot be, as it
> > is embedded in each eth_driver.)  
> 
> Not sure I understand the question.

I was just thinking loudly. This note was not very important. It was a
part of my confusion. Result: rte_driver is semantically very different
from rte_pci_driver.

> 
> At the moment, a rte_pci_device references a rte_pci_driver.
> Associating those happens at pci "probe" time
> lib/librte_eal/common/eal_common_pci.c +202
> 
> I agree there is a pci_driver embedded in eth_driver, but that does
> not mean pci drivers must be eth drivers.
> 
> 
> > Another way, not that beautiful... Introduce rte_generic_driver and
> > rte_generic_device. (Or rte_gen_driver/rte_gen_device or
> > rte_bus_driver/rte_bus_device if you want). This enables to let the
> > rte_driver as it is and it avoids a lot of quite terrible transition
> > patches that can break everything.
> >  
> >> - no more devinit and devuninit functions in rte_pci_driver, they can
> >> be moved as init / uninit functions in rte_driver  
> >
> > The rte_driver has init/uninit already and its semantics seem to be
> > module_init and module_uninit.  
> 
> Ok, so what you propose is something like this ?

I've expressed my basic understanding of this topic in the RFC patch set
yesterday (as you know).

> 
> - keep rte_driver as it is (init and uninit), I would say the name can
> be changed later.

Agreed.

> - add rte_bus_driver (idem, not sure it is a good name) in place of
> the rte_driver I mentioned in my initial mail.

I don't like the name either. I have no other idea at the moment.

> Rather than have init / uninit, how about attach / detach methods ?

You mean attach a driver to a device? Yes, much better. And what about
probe? I was quite confused when writing a PMD as I couldn't understand
clearly where should I start touching the hardware.


Regards
Jan

> 
> 
> Regards,



-- 
   Jan Viktorin                  E-mail: Viktorin@RehiveTech.com
   System Architect              Web:    www.RehiveTech.com
   RehiveTech
   Brno, Czech Republic