On Thursday 10 November 2016 01:21 PM, Jianbo Liu wrote: > On 10 November 2016 at 15:26, Shreyansh Jain <shreyansh.jain at nxp.com> > wrote: >> Hello David, list, >> >> I need some help and clarification regarding some changes I am doing to >> cleanup the EAL code. >> >> There are some changes which should be done for eth_driver/rte_eth_device >> structures: >> >> 1. most obvious, eth_driver should be renamed to rte_eth_driver. >> 2. eth_driver currently has rte_pci_driver embedded in it >> - there can be ethernet devices which are _not_ PCI >> - in which case, this structure should be removed. >> 3. Similarly, rte_eth_dev has rte_pci_device which should be replaced with >> rte_device. >> >> This is what the current outline of eth_driver is: >> >> +------------------------+ >> | eth_driver | >> | +---------------------+| >> | | rte_pci_driver || >> | | +------------------+|| >> | | | rte_driver ||| >> | | | name[] ||| >> | | | ... ||| >> | | +------------------+|| >> | | .probe || >> | | .remove || >> | | ... || >> | +---------------------+| >> | .eth_dev_init | >> | .eth_dev_uninit | >> +------------------------+ >> >> This is what I was thinking: >> >> +---------------------+ +----------------------+ >> | rte_pci_driver | |eth_driver | >> | +------------------+| _|_struct rte_driver *p | >> | | rte_driver <-------/ | .eth_dev_init | >> | | ... || | .eth_dev_uninit | >> | | name || +----------------------+ >> | | <more> || >> | +------------------+| >> | <PCI specific info>| >> +---------------------+ >> >> ::Impact:: >> Various drivers use the rte_pci_driver embedded in the eth_driver object for >> device initialization. >> == They assume that rte_pci_driver is directly embedded and hence simply >> dereference. >> == e.g. eth_igb_dev_init() in drivers/net/e1000/igb_ethdev.c file >> >> With the above change, such drivers would have to access rte_driver and then >> perform container_of to obtain their respective rte_xxx_driver. >> == this would be useful in case there is a non-PCI driver >> >> ::Problem:: >> I am not sure of reason as to why eth_driver embedded rte_pci_driver in >> first place - other than a convenient way to define it before PCI driver >> registration. >> >> As all the existing PMDs are impacted - am I missing something here in >> making the above change? >> > > How do you know eth_driver->p is pointing to a rte_pci_driver or > rte_soc_driver? > Maybe you need to add a type/flag in rte_driver.
My take: PMD implementation would specify this - similar to how it is done now. A PCI PMD would perform a container_of(rte_pci_driver,...). I don't think we need a differentiation here - primarily because generic doesn't handle the eth_driver. > >> Probably, similar is the case for rte_eth_dev. >> >> - >> Shreyansh > -- - Shreyansh