[dpdk-dev] [RFC PATCH 1/6] librte_ether: add fields from rte_pci_driver to rte_eth_dev and rte_eth_dev_data.

Ananyev, Konstantin konstantin.ananyev at intel.com
Tue Sep 1 14:37:29 CEST 2015



> -----Original Message-----
> From: dev [mailto:dev-bounces at dpdk.org] On Behalf Of Iremonger, Bernard
> Sent: Tuesday, September 01, 2015 12:39 PM
> To: Thomas Monjalon
> Cc: dev at dpdk.org
> Subject: Re: [dpdk-dev] [RFC PATCH 1/6] librte_ether: add fields from rte_pci_driver to rte_eth_dev and rte_eth_dev_data.
> 
> Hi THomas,
> 
> <snip>
> 
> > > @@ -424,7 +425,10 @@ rte_eth_dev_socket_id(uint8_t port_id)  {
> > >  	if (!rte_eth_dev_is_valid_port(port_id))
> > >  		return -1;
> > > -	return rte_eth_devices[port_id].pci_dev->numa_node;
> > > +	if (rte_eth_devices[port_id].dev_type == RTE_ETH_DEV_PCI)
> > > +		return rte_eth_devices[port_id].pci_dev->numa_node;
> > > +	else
> > > +		return rte_eth_devices[port_id].data->numa_node;
> >
> > Clearly not the way to go.
> > We should remove the special cases (PCI, PDEV, VDEV) instead of adding
> > more checks.
> 
> The dev_type field is not new, just using it now to distinguish between PCI and non PCI devices.
> 
> I have moved some of the RTE_PCI_DRV flags to a new dev_flags field in struct rte_eth_dev{},
> These flags are independent of the driver type (PCI or non PCI).
> Do you have view on this new dev_flags field and macros?

What looks strange here to me, I that we now we duplicate some fields here?
Let say for PCI devices numa_node would be precent in pci_dev and in data, right?
If there are some fields that are common for all device types (PCI, VDEV, etc) why not to create
some unified structure for them that would be used by all device types and remove subsequent fields from pci_dev?
Or alternatively create a union of structs (one struct per device type)?
Then, as was pointed before, we wouldn't these branches by device_type at all.
Konstantin


> 
> Regards,
> 
> Bernard.
> 
> 


More information about the dev mailing list