[dpdk-dev] [PATCH v5 1/2] librte_ether: add protection against overwrite device data
Thomas Monjalon
thomas.monjalon at 6wind.com
Thu Oct 6 16:20:47 CEST 2016
2016-10-06 13:57, Kerlin, MarcinX:
> Hi Thomas,
>
> From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com]
> >
> > 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];
This line indicates that the pointer data is to the struct rte_eth_dev_data
of the port_id, not the array of every devices.
> > 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.
[...]
I think I've understood the bug.
I'm just saying you are fixing a weird design (rte_eth_dev_data[]).
> > 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.
I'm talking about the initial introduction of rte_eth_dev_data[]
which seems to be a workaround for multi-process without touching
rte_eth_devices[] allocated as a global variable (not a memzone).
> > 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[].
Yes
> What will indicate pointer inside rte_eth_dev rte_eth_devices[]:
> (struct rte_eth_dev_data *data; /**< Pointer to device data */)
After thinking more about it, I realize that rte_eth_devices cannot be
in a shared memzone because of its local pointers.
Sorry for the noise, I'll reconsider your patch.
More information about the dev
mailing list