Re: [PATCH] ethdev: fix wrong memset

[Yuanhan Liu <yuanhan.liu@linux.intel.com>]

On Mon, Jan 23, 2017 at 11:32:23AM +0000, Ferruh Yigit wrote:
> On 1/23/2017 11:24 AM, Yuanhan Liu wrote:
> > On Mon, Jan 23, 2017 at 11:05:25AM +0000, Ferruh Yigit wrote:
> >>>>>>>>  lib/librte_ether/rte_ethdev.c | 2 +-
> >>>>>>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> >>>>>>>>
> >>>>>>>> diff --git a/lib/librte_ether/rte_ethdev.c b/lib/librte_ether/rte_ethdev.c
> >>>>>>>> index 4790faf..61f44e2 100644
> >>>>>>>> --- a/lib/librte_ether/rte_ethdev.c
> >>>>>>>> +++ b/lib/librte_ether/rte_ethdev.c
> >>>>>>>> @@ -225,7 +225,7 @@ struct rte_eth_dev *
> >>>>>>>>  		return NULL;
> >>>>>>>>  	}
> >>>>>>>>  
> >>>>>>>> -	memset(&rte_eth_devices[port_id], 0, sizeof(*eth_dev->data));
> >>>>>>>> +	memset(&rte_eth_dev_data[port_id], 0, sizeof(struct rte_eth_dev_data));
> >>>>>>>
> >>>>>>> Not directly related to the this issue, but, after fix, this may have
> >>>>>>> issues with secondary process.
> >>>>>>>
> >>>>>>> There were patches sent to fix this.
> >>>>>>
> >>>>>> I mean this one:
> >>>>>> http://dpdk.org/ml/archives/dev/2017-January/054422.html
> >>>>>
> >>>>> d948f596fee2 ("ethdev: fix port data mismatched in multiple process
> >>>>> model") should have fixed it.
> >>>>
> >>>> Think about case, where secondary process uses a virtual PMD, which does
> >>>> a rte_eth_dev_allocate() call, shouldn't this corrupt primary process
> >>>> device data?
> >>>
> >>> Yes, it may. However, I doubt that's the typical usage. 
> >>
> >> But this is a use case, and broken now,
> > 
> > I thought it was broken since the beginning?
> 
> No, memset(&rte_eth_dev_data[port_id], ...) breaks it.

Oh, you were talking about that particular case Remy's patch meant to
fix.

> >> and fix is known.
> > 
> > And there is already a fix?
> 
> http://dpdk.org/ml/archives/dev/2017-January/054422.html

Yes, it should fix that issue. One question: do Remy or you regularly
run some multiple process test cases (and with vdev both in primary
and secondary process)?

	--yliu