[v2] net/ifc: do not notify before HW ready

  If the device is not clearly reset by the previous driver and holds
some invalid ring addr, and the relay thread kicks it before HW is
properly re-configured, a bad DMA request may happen.

Besides, the notify_addr which is used by the relay thread is set in
the vdpa_ifcvf_start function, if a kick relay happens before
vdpa_ifcvf_start finishes, a null addr is accessed.

Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")

Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
---
v2:
* Add description for the fix in the commit log.
---
 drivers/net/ifc/ifcvf_vdpa.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)
  

Looks good to me.

Reviewed-by: Ye Xiaolong <xiaolong.ye@intel.com>

Thanks,
Xiaolong

On 09/14, Xiao Wang wrote:
>If the device is not clearly reset by the previous driver and holds
>some invalid ring addr, and the relay thread kicks it before HW is
>properly re-configured, a bad DMA request may happen.
>
>Besides, the notify_addr which is used by the relay thread is set in
>the vdpa_ifcvf_start function, if a kick relay happens before
>vdpa_ifcvf_start finishes, a null addr is accessed.
>
>Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
>
>Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
>---
>v2:
>* Add description for the fix in the commit log.
>---
> drivers/net/ifc/ifcvf_vdpa.c | 8 ++++----
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
>diff --git a/drivers/net/ifc/ifcvf_vdpa.c b/drivers/net/ifc/ifcvf_vdpa.c
>index 3c5430dc0..7d3085d8d 100644
>--- a/drivers/net/ifc/ifcvf_vdpa.c
>+++ b/drivers/net/ifc/ifcvf_vdpa.c
>@@ -503,11 +503,11 @@ update_datapath(struct ifcvf_internal *internal)
> 		if (ret)
> 			goto err;
> 
>-		ret = setup_notify_relay(internal);
>+		ret = vdpa_ifcvf_start(internal);
> 		if (ret)
> 			goto err;
> 
>-		ret = vdpa_ifcvf_start(internal);
>+		ret = setup_notify_relay(internal);
> 		if (ret)
> 			goto err;
> 
>@@ -515,12 +515,12 @@ update_datapath(struct ifcvf_internal *internal)
> 	} else if (rte_atomic32_read(&internal->running) &&
> 		   (!rte_atomic32_read(&internal->started) ||
> 		    !rte_atomic32_read(&internal->dev_attached))) {
>-		vdpa_ifcvf_stop(internal);
>-
> 		ret = unset_notify_relay(internal);
> 		if (ret)
> 			goto err;
> 
>+		vdpa_ifcvf_stop(internal);
>+
> 		ret = vdpa_disable_vfio_intr(internal);
> 		if (ret)
> 			goto err;
>-- 
>2.15.1
>
  

> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Ye Xiaolong
> Sent: Friday, September 14, 2018 4:42 PM
> To: Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: Bie, Tiwei <tiwei.bie@intel.com>; dev@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/ifc: do not notify before HW ready
> 
> Looks good to me.
> 
> Reviewed-by: Ye Xiaolong <xiaolong.ye@intel.com>
> 
> Thanks,
> Xiaolong
> 
> On 09/14, Xiao Wang wrote:
> >If the device is not clearly reset by the previous driver and holds
> >some invalid ring addr, and the relay thread kicks it before HW is
> >properly re-configured, a bad DMA request may happen.
> >
> >Besides, the notify_addr which is used by the relay thread is set in
> >the vdpa_ifcvf_start function, if a kick relay happens before
> >vdpa_ifcvf_start finishes, a null addr is accessed.
> >
> >Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
> >
> >Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi
  

On 09/14/2018 02:25 AM, Xiao Wang wrote:
> If the device is not clearly reset by the previous driver and holds
> some invalid ring addr, and the relay thread kicks it before HW is
> properly re-configured, a bad DMA request may happen.
> 
> Besides, the notify_addr which is used by the relay thread is set in
> the vdpa_ifcvf_start function, if a kick relay happens before
> vdpa_ifcvf_start finishes, a null addr is accessed.
> 
> Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
> 

Looks like this should be in stable branch too. Can you confirm?	

> Signed-off-by: Xiao Wang <xiao.w.wang@intel.com>
> ---
> v2:
> * Add description for the fix in the commit log.
> ---
>  drivers/net/ifc/ifcvf_vdpa.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ifc/ifcvf_vdpa.c b/drivers/net/ifc/ifcvf_vdpa.c
> index 3c5430dc0..7d3085d8d 100644
> --- a/drivers/net/ifc/ifcvf_vdpa.c
> +++ b/drivers/net/ifc/ifcvf_vdpa.c
> @@ -503,11 +503,11 @@ update_datapath(struct ifcvf_internal *internal)
>  		if (ret)
>  			goto err;
>  
> -		ret = setup_notify_relay(internal);
> +		ret = vdpa_ifcvf_start(internal);
>  		if (ret)
>  			goto err;
>  
> -		ret = vdpa_ifcvf_start(internal);
> +		ret = setup_notify_relay(internal);
>  		if (ret)
>  			goto err;
>  
> @@ -515,12 +515,12 @@ update_datapath(struct ifcvf_internal *internal)
>  	} else if (rte_atomic32_read(&internal->running) &&
>  		   (!rte_atomic32_read(&internal->started) ||
>  		    !rte_atomic32_read(&internal->dev_attached))) {
> -		vdpa_ifcvf_stop(internal);
> -
>  		ret = unset_notify_relay(internal);
>  		if (ret)
>  			goto err;
>  
> +		vdpa_ifcvf_stop(internal);
> +
>  		ret = vdpa_disable_vfio_intr(internal);
>  		if (ret)
>  			goto err;
>
  

Hi,

> -----Original Message-----
> From: Kevin Traynor [mailto:ktraynor@redhat.com]
> Sent: Wednesday, September 26, 2018 1:16 AM
> To: Wang, Xiao W <xiao.w.wang@intel.com>
> Cc: Ye, Xiaolong <xiaolong.ye@intel.com>; Bie, Tiwei <tiwei.bie@intel.com>;
> dev@dpdk.org; stable@dpdk.org
> Subject: Re: [dpdk-dev] [PATCH v2] net/ifc: do not notify before HW ready
> 
> On 09/14/2018 02:25 AM, Xiao Wang wrote:
> > If the device is not clearly reset by the previous driver and holds
> > some invalid ring addr, and the relay thread kicks it before HW is
> > properly re-configured, a bad DMA request may happen.
> >
> > Besides, the notify_addr which is used by the relay thread is set in
> > the vdpa_ifcvf_start function, if a kick relay happens before
> > vdpa_ifcvf_start finishes, a null addr is accessed.
> >
> > Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
> >
> 
> Looks like this should be in stable branch too. Can you confirm?	

Yes, they should go also into stable branch, thanks for the notice.

BRs,
Xiao
  

On 9/26/2018 1:12 AM, Wang, Xiao W wrote:
> Hi,
> 
>> -----Original Message-----
>> From: Kevin Traynor [mailto:ktraynor@redhat.com]
>> Sent: Wednesday, September 26, 2018 1:16 AM
>> To: Wang, Xiao W <xiao.w.wang@intel.com>
>> Cc: Ye, Xiaolong <xiaolong.ye@intel.com>; Bie, Tiwei <tiwei.bie@intel.com>;
>> dev@dpdk.org; stable@dpdk.org
>> Subject: Re: [dpdk-dev] [PATCH v2] net/ifc: do not notify before HW ready
>>
>> On 09/14/2018 02:25 AM, Xiao Wang wrote:
>>> If the device is not clearly reset by the previous driver and holds
>>> some invalid ring addr, and the relay thread kicks it before HW is
>>> properly re-configured, a bad DMA request may happen.
>>>
>>> Besides, the notify_addr which is used by the relay thread is set in
>>> the vdpa_ifcvf_start function, if a kick relay happens before
>>> vdpa_ifcvf_start finishes, a null addr is accessed.
>>>
>>> Fixes: a3f8150eac6d ("net/ifcvf: add ifcvf vDPA driver")
>>>
>>
>> Looks like this should be in stable branch too. Can you confirm?	
> 
> Yes, they should go also into stable branch, thanks for the notice.

"Cc: stable@dpdk.org" already has been added while merging next-net
Commit c5ea24905585 ("net/ifc: do not notify before HW ready")

Thanks for checking.