Re: [dpdk-dev] [PATCH v5 1/2] librte_ether: add protection against overwrite device data

["Kerlin, MarcinX" <marcinx.kerlin@intel.com>]

Hi Thomas,

> -----Original Message-----
> From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com]
> Sent: Thursday, October 06, 2016 11:41 AM
> To: Kerlin, MarcinX <marcinx.kerlin@intel.com>
> Cc: dev@dpdk.org; De Lara Guarch, Pablo <pablo.de.lara.guarch@intel.com>;
> Gonzalez Monroy, Sergio <sergio.gonzalez.monroy@intel.com>
> Subject: Re: [PATCH v5 1/2] librte_ether: add protection against overwrite
> device data
> 
> Hi Marcin,
> 
> 2016-09-30 16:00, Marcin Kerlin:
> > Added protection against overwrite device data in array
> > rte_eth_dev_data[] for the next secondary applications. Secondary
> > process appends in the first free place rather than at the beginning.
> > This behavior prevents overwriting devices data of primary process by
> secondary process.
> 
> I've just realized that you are trying to fix an useless code.
> Why not just remove the array rte_eth_dev_data[] at first?

because pointer to rte_eth_dev_data in rte_eth_devices[] is 
just to array rte_eth_dev_data[].

rte_ethdev.c:214 
eth_dev->data = &rte_eth_dev_data[port_id];

> We already have the array rte_eth_devices[] and there is a pointer to
> rte_eth_dev_data in rte_eth_dev.

As you write above there is a pointer, but after run secondary testpmd this pointer
will indicate data which hold from now data for secondary testpmd.

1) run testpmd [primary]

rte_eth_devices[0].data.name = 3:0.1

2) run testpmd [secondary]
This testpmd overwrite first index in rte_eth_dev_data[]
3:0.1 -> eth_pcap0

3) back to testpmd [primary]
rte_eth_devices[0].data.name = eth_pcap0

from now in primary testpmd our device name is eth_pcap0 but should be 3:0.1

> 
> Is it just a workaround to be able to lookup the rte_eth_dev_data memzone in
> the secondary process?

No it is not workaround, it is protection against overwrite device data.
I think that my cover letter good explain what is wrong. I did there
short debug log. 

> So wouldn't it be saner to have rte_eth_devices[] in a memzone?

So you mean that move rte_eth_devices[] to memzone + remove rte_eth_dev_data[].
What will indicate pointer inside rte_eth_dev  rte_eth_devices[]:
(struct rte_eth_dev_data *data;  /**< Pointer to device data */)

If I did not understand you please clarify more.

Regards,
Marcin
> 
> Sergio, as the multi-process maintainer, I guess you have an idea :)