DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] net/ice: fix representor port segmentation fault
@ 2021-09-02  9:44 dapengx.yu
  2021-09-02 14:45 ` Wang, Haiyue
  2021-09-03  5:41 ` [dpdk-dev] [PATCH v2] " dapengx.yu
  0 siblings, 2 replies; 8+ messages in thread
From: dapengx.yu @ 2021-09-02  9:44 UTC (permalink / raw)
  To: Qiming Yang, Qi Zhang; +Cc: dev, haiyue.wang, Dapeng Yu, stable

From: Dapeng Yu <dapengx.yu@intel.com>

If DCF representor port is closed after DCF port is closed, there will
be segmentation fault because representor accesses the released resource
of DCF port.

This patch makes DCF representor port is always closed before DCF port is
closed.

Fixes: 5674465a32c8 ("net/ice: add DCF VLAN handling")
Cc: stable@dpdk.org

Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
---
 drivers/net/ice/ice_dcf_ethdev.c         |  1 +
 drivers/net/ice/ice_dcf_ethdev.h         |  1 +
 drivers/net/ice/ice_dcf_vf_representor.c | 26 ++++++++++++++++++++----
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ice/ice_dcf_ethdev.c b/drivers/net/ice/ice_dcf_ethdev.c
index cab7c4da87..b837f69fd4 100644
--- a/drivers/net/ice/ice_dcf_ethdev.c
+++ b/drivers/net/ice/ice_dcf_ethdev.c
@@ -874,6 +874,7 @@ ice_dcf_dev_close(struct rte_eth_dev *dev)
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
+	ice_dcf_vf_repr_close_all(adapter);
 	ice_dcf_free_repr_info(adapter);
 	ice_dcf_uninit_parent_adapter(dev);
 	ice_dcf_uninit_hw(dev, &adapter->real_hw);
diff --git a/drivers/net/ice/ice_dcf_ethdev.h b/drivers/net/ice/ice_dcf_ethdev.h
index 8510e37119..9e8d3ef0c5 100644
--- a/drivers/net/ice/ice_dcf_ethdev.h
+++ b/drivers/net/ice/ice_dcf_ethdev.h
@@ -64,5 +64,6 @@ int ice_dcf_vf_repr_init(struct rte_eth_dev *vf_rep_eth_dev, void *init_param);
 int ice_dcf_vf_repr_uninit(struct rte_eth_dev *vf_rep_eth_dev);
 int ice_dcf_vf_repr_init_vlan(struct rte_eth_dev *vf_rep_eth_dev);
 void ice_dcf_vf_repr_stop_all(struct ice_dcf_adapter *dcf_adapter);
+void ice_dcf_vf_repr_close_all(struct ice_dcf_adapter *dcf_adapter);
 
 #endif /* _ICE_DCF_ETHDEV_H_ */
diff --git a/drivers/net/ice/ice_dcf_vf_representor.c b/drivers/net/ice/ice_dcf_vf_representor.c
index 970461f3e9..121dea7483 100644
--- a/drivers/net/ice/ice_dcf_vf_representor.c
+++ b/drivers/net/ice/ice_dcf_vf_representor.c
@@ -46,6 +46,7 @@ static int
 ice_dcf_vf_repr_dev_stop(struct rte_eth_dev *dev)
 {
 	dev->data->dev_link.link_status = ETH_LINK_DOWN;
+	dev->data->dev_started = 0;
 
 	return 0;
 }
@@ -53,6 +54,7 @@ ice_dcf_vf_repr_dev_stop(struct rte_eth_dev *dev)
 static int
 ice_dcf_vf_repr_dev_close(struct rte_eth_dev *dev)
 {
+	(void)ice_dcf_vf_repr_dev_stop(dev);
 	return ice_dcf_vf_repr_uninit(dev);
 }
 
@@ -464,7 +466,6 @@ void
 ice_dcf_vf_repr_stop_all(struct ice_dcf_adapter *dcf_adapter)
 {
 	uint16_t vf_id;
-	int ret;
 
 	if (!dcf_adapter->repr_infos)
 		return;
@@ -475,8 +476,25 @@ ice_dcf_vf_repr_stop_all(struct ice_dcf_adapter *dcf_adapter)
 		if (!vf_rep_eth_dev || vf_rep_eth_dev->data->dev_started == 0)
 			continue;
 
-		ret = ice_dcf_vf_repr_dev_stop(vf_rep_eth_dev);
-		if (!ret)
-			vf_rep_eth_dev->data->dev_started = 0;
+		(void)ice_dcf_vf_repr_dev_stop(vf_rep_eth_dev);
+	}
+}
+
+void
+ice_dcf_vf_repr_close_all(struct ice_dcf_adapter *dcf_adapter)
+{
+	uint16_t vf_id;
+
+	if (!dcf_adapter->repr_infos)
+		return;
+
+	for (vf_id = 0; vf_id < dcf_adapter->real_hw.num_vfs; vf_id++) {
+		struct rte_eth_dev *vf_rep_eth_dev =
+				dcf_adapter->repr_infos[vf_id].vf_rep_eth_dev;
+		if (!vf_rep_eth_dev)
+			continue;
+
+		(void)ice_dcf_vf_repr_dev_close(vf_rep_eth_dev);
+		(void)rte_eth_dev_release_port(vf_rep_eth_dev);
 	}
 }
-- 
2.27.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ice: fix representor port segmentation fault
  2021-09-02  9:44 [dpdk-dev] [PATCH] net/ice: fix representor port segmentation fault dapengx.yu
@ 2021-09-02 14:45 ` Wang, Haiyue
  2021-09-03  3:08   ` Yu, DapengX
  2021-09-03  5:41 ` [dpdk-dev] [PATCH v2] " dapengx.yu
  1 sibling, 1 reply; 8+ messages in thread
From: Wang, Haiyue @ 2021-09-02 14:45 UTC (permalink / raw)
  To: Yu, DapengX, Yang, Qiming, Zhang, Qi Z; +Cc: dev, stable

> -----Original Message-----
> From: Yu, DapengX <dapengx.yu@intel.com>
> Sent: Thursday, September 2, 2021 17:44
> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Yu, DapengX <dapengx.yu@intel.com>;
> stable@dpdk.org
> Subject: [PATCH] net/ice: fix representor port segmentation fault
> 
> From: Dapeng Yu <dapengx.yu@intel.com>
> 
> If DCF representor port is closed after DCF port is closed, there will
> be segmentation fault because representor accesses the released resource
> of DCF port.
> 
> This patch makes DCF representor port is always closed before DCF port is
> closed.
> 
> Fixes: 5674465a32c8 ("net/ice: add DCF VLAN handling")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
> ---
>  drivers/net/ice/ice_dcf_ethdev.c         |  1 +
>  drivers/net/ice/ice_dcf_ethdev.h         |  1 +
>  drivers/net/ice/ice_dcf_vf_representor.c | 26 ++++++++++++++++++++----
>  3 files changed, 24 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/net/ice/ice_dcf_ethdev.c b/drivers/net/ice/ice_dcf_ethdev.c
> index cab7c4da87..b837f69fd4 100644
> --- a/drivers/net/ice/ice_dcf_ethdev.c
> +++ b/drivers/net/ice/ice_dcf_ethdev.c
> @@ -874,6 +874,7 @@ ice_dcf_dev_close(struct rte_eth_dev *dev)
>  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
>  		return 0;
> 
> +	ice_dcf_vf_repr_close_all(adapter);
>  	ice_dcf_free_repr_info(adapter);
>  	ice_dcf_uninit_parent_adapter(dev);
>  	ice_dcf_uninit_hw(dev, &adapter->real_hw);
> diff --git a/drivers/net/ice/ice_dcf_ethdev.h b/drivers/net/ice/ice_dcf_ethdev.h
> index 8510e37119..9e8d3ef0c5 100644
> --- a/drivers/net/ice/ice_dcf_ethdev.h
> +++ b/drivers/net/ice/ice_dcf_ethdev.h
> @@ -64,5 +64,6 @@ int ice_dcf_vf_repr_init(struct rte_eth_dev *vf_rep_eth_dev, void *init_param);
>  int ice_dcf_vf_repr_uninit(struct rte_eth_dev *vf_rep_eth_dev);
>  int ice_dcf_vf_repr_init_vlan(struct rte_eth_dev *vf_rep_eth_dev);
>  void ice_dcf_vf_repr_stop_all(struct ice_dcf_adapter *dcf_adapter);
> +void ice_dcf_vf_repr_close_all(struct ice_dcf_adapter *dcf_adapter);
> 
>  #endif /* _ICE_DCF_ETHDEV_H_ */
> diff --git a/drivers/net/ice/ice_dcf_vf_representor.c b/drivers/net/ice/ice_dcf_vf_representor.c
> index 970461f3e9..121dea7483 100644
> --- a/drivers/net/ice/ice_dcf_vf_representor.c
> +++ b/drivers/net/ice/ice_dcf_vf_representor.c
> @@ -46,6 +46,7 @@ static int
>  ice_dcf_vf_repr_dev_stop(struct rte_eth_dev *dev)
>  {
>  	dev->data->dev_link.link_status = ETH_LINK_DOWN;
> +	dev->data->dev_started = 0;
> 
>  	return 0;
>  }
> @@ -53,6 +54,7 @@ ice_dcf_vf_repr_dev_stop(struct rte_eth_dev *dev)
>  static int
>  ice_dcf_vf_repr_dev_close(struct rte_eth_dev *dev)
>  {
> +	(void)ice_dcf_vf_repr_dev_stop(dev);

Do we truly need the "(void)" ?

>  	return ice_dcf_vf_repr_uninit(dev);
>  }
> 
> @@ -464,7 +466,6 @@ void
>  ice_dcf_vf_repr_stop_all(struct ice_dcf_adapter *dcf_adapter)
>  {
>  	uint16_t vf_id;
> -	int ret;
> 
>  	if (!dcf_adapter->repr_infos)
>  		return;
> @@ -475,8 +476,25 @@ ice_dcf_vf_repr_stop_all(struct ice_dcf_adapter *dcf_adapter)
>  		if (!vf_rep_eth_dev || vf_rep_eth_dev->data->dev_started == 0)
>  			continue;
> 
> -		ret = ice_dcf_vf_repr_dev_stop(vf_rep_eth_dev);
> -		if (!ret)
> -			vf_rep_eth_dev->data->dev_started = 0;
> +		(void)ice_dcf_vf_repr_dev_stop(vf_rep_eth_dev);
> +	}
> +}
> +
> +void
> +ice_dcf_vf_repr_close_all(struct ice_dcf_adapter *dcf_adapter)
> +{
> +	uint16_t vf_id;
> +
> +	if (!dcf_adapter->repr_infos)
> +		return;
> +
> +	for (vf_id = 0; vf_id < dcf_adapter->real_hw.num_vfs; vf_id++) {
> +		struct rte_eth_dev *vf_rep_eth_dev =
> +				dcf_adapter->repr_infos[vf_id].vf_rep_eth_dev;
> +		if (!vf_rep_eth_dev)
> +			continue;
> +
> +		(void)ice_dcf_vf_repr_dev_close(vf_rep_eth_dev);
> +		(void)rte_eth_dev_release_port(vf_rep_eth_dev);
>  	}
>  }
> --
> 2.27.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ice: fix representor port segmentation fault
  2021-09-02 14:45 ` Wang, Haiyue
@ 2021-09-03  3:08   ` Yu, DapengX
  2021-09-03  4:37     ` Wang, Haiyue
  0 siblings, 1 reply; 8+ messages in thread
From: Yu, DapengX @ 2021-09-03  3:08 UTC (permalink / raw)
  To: Wang, Haiyue, Yang, Qiming, Zhang, Qi Z; +Cc: dev, stable



> -----Original Message-----
> From: Wang, Haiyue <haiyue.wang@intel.com>
> Sent: Thursday, September 2, 2021 10:45 PM
> To: Yu, DapengX <dapengx.yu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH] net/ice: fix representor port segmentation fault
> 
> > -----Original Message-----
> > From: Yu, DapengX <dapengx.yu@intel.com>
> > Sent: Thursday, September 2, 2021 17:44
> > To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Yu, DapengX
> > <dapengx.yu@intel.com>; stable@dpdk.org
> > Subject: [PATCH] net/ice: fix representor port segmentation fault
> >
> > From: Dapeng Yu <dapengx.yu@intel.com>
> >
> > If DCF representor port is closed after DCF port is closed, there will
> > be segmentation fault because representor accesses the released
> > resource of DCF port.
> >
> > This patch makes DCF representor port is always closed before DCF port
> > is closed.
> >
> > Fixes: 5674465a32c8 ("net/ice: add DCF VLAN handling")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
> > ---
> >  drivers/net/ice/ice_dcf_ethdev.c         |  1 +
> >  drivers/net/ice/ice_dcf_ethdev.h         |  1 +
> >  drivers/net/ice/ice_dcf_vf_representor.c | 26
> > ++++++++++++++++++++----
> >  3 files changed, 24 insertions(+), 4 deletions(-)
> >
> > diff --git a/drivers/net/ice/ice_dcf_ethdev.c
> > b/drivers/net/ice/ice_dcf_ethdev.c
> > index cab7c4da87..b837f69fd4 100644
> > --- a/drivers/net/ice/ice_dcf_ethdev.c
> > +++ b/drivers/net/ice/ice_dcf_ethdev.c
> > @@ -874,6 +874,7 @@ ice_dcf_dev_close(struct rte_eth_dev *dev)
> >  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> >  		return 0;
> >
> > +	ice_dcf_vf_repr_close_all(adapter);
> >  	ice_dcf_free_repr_info(adapter);
> >  	ice_dcf_uninit_parent_adapter(dev);
> >  	ice_dcf_uninit_hw(dev, &adapter->real_hw); diff --git
> > a/drivers/net/ice/ice_dcf_ethdev.h b/drivers/net/ice/ice_dcf_ethdev.h
> > index 8510e37119..9e8d3ef0c5 100644
> > --- a/drivers/net/ice/ice_dcf_ethdev.h
> > +++ b/drivers/net/ice/ice_dcf_ethdev.h
> > @@ -64,5 +64,6 @@ int ice_dcf_vf_repr_init(struct rte_eth_dev
> > *vf_rep_eth_dev, void *init_param);  int ice_dcf_vf_repr_uninit(struct
> > rte_eth_dev *vf_rep_eth_dev);  int ice_dcf_vf_repr_init_vlan(struct
> > rte_eth_dev *vf_rep_eth_dev);  void ice_dcf_vf_repr_stop_all(struct
> > ice_dcf_adapter *dcf_adapter);
> > +void ice_dcf_vf_repr_close_all(struct ice_dcf_adapter *dcf_adapter);
> >
> >  #endif /* _ICE_DCF_ETHDEV_H_ */
> > diff --git a/drivers/net/ice/ice_dcf_vf_representor.c
> > b/drivers/net/ice/ice_dcf_vf_representor.c
> > index 970461f3e9..121dea7483 100644
> > --- a/drivers/net/ice/ice_dcf_vf_representor.c
> > +++ b/drivers/net/ice/ice_dcf_vf_representor.c
> > @@ -46,6 +46,7 @@ static int
> >  ice_dcf_vf_repr_dev_stop(struct rte_eth_dev *dev)  {
> >  	dev->data->dev_link.link_status = ETH_LINK_DOWN;
> > +	dev->data->dev_started = 0;
> >
> >  	return 0;
> >  }
> > @@ -53,6 +54,7 @@ ice_dcf_vf_repr_dev_stop(struct rte_eth_dev *dev)
> > static int  ice_dcf_vf_repr_dev_close(struct rte_eth_dev *dev)  {
> > +	(void)ice_dcf_vf_repr_dev_stop(dev);
> 
> Do we truly need the "(void)" ?

Here use (void) to make discarding of return value explicit.
There will be no compiler(clang, icc, and gcc) warning even if remove (void)
So it is fine without that.

> 
> >  	return ice_dcf_vf_repr_uninit(dev);
> >  }
> >
> > @@ -464,7 +466,6 @@ void
> >  ice_dcf_vf_repr_stop_all(struct ice_dcf_adapter *dcf_adapter)  {
> >  	uint16_t vf_id;
> > -	int ret;
> >
> >  	if (!dcf_adapter->repr_infos)
> >  		return;
> > @@ -475,8 +476,25 @@ ice_dcf_vf_repr_stop_all(struct ice_dcf_adapter
> *dcf_adapter)
> >  		if (!vf_rep_eth_dev || vf_rep_eth_dev->data->dev_started
> == 0)
> >  			continue;
> >
> > -		ret = ice_dcf_vf_repr_dev_stop(vf_rep_eth_dev);
> > -		if (!ret)
> > -			vf_rep_eth_dev->data->dev_started = 0;
> > +		(void)ice_dcf_vf_repr_dev_stop(vf_rep_eth_dev);
> > +	}
> > +}
> > +
> > +void
> > +ice_dcf_vf_repr_close_all(struct ice_dcf_adapter *dcf_adapter) {
> > +	uint16_t vf_id;
> > +
> > +	if (!dcf_adapter->repr_infos)
> > +		return;
> > +
> > +	for (vf_id = 0; vf_id < dcf_adapter->real_hw.num_vfs; vf_id++) {
> > +		struct rte_eth_dev *vf_rep_eth_dev =
> > +				dcf_adapter-
> >repr_infos[vf_id].vf_rep_eth_dev;
> > +		if (!vf_rep_eth_dev)
> > +			continue;
> > +
> > +		(void)ice_dcf_vf_repr_dev_close(vf_rep_eth_dev);
> > +		(void)rte_eth_dev_release_port(vf_rep_eth_dev);
> >  	}
> >  }
> > --
> > 2.27.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH] net/ice: fix representor port segmentation fault
  2021-09-03  3:08   ` Yu, DapengX
@ 2021-09-03  4:37     ` Wang, Haiyue
  0 siblings, 0 replies; 8+ messages in thread
From: Wang, Haiyue @ 2021-09-03  4:37 UTC (permalink / raw)
  To: Yu, DapengX, Yang, Qiming, Zhang, Qi Z; +Cc: dev, stable

> -----Original Message-----
> From: Yu, DapengX <dapengx.yu@intel.com>
> Sent: Friday, September 3, 2021 11:09
> To: Wang, Haiyue <haiyue.wang@intel.com>; Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH] net/ice: fix representor port segmentation fault
> 
> 
> 
> > -----Original Message-----
> > From: Wang, Haiyue <haiyue.wang@intel.com>
> > Sent: Thursday, September 2, 2021 10:45 PM
> > To: Yu, DapengX <dapengx.yu@intel.com>; Yang, Qiming
> > <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; stable@dpdk.org
> > Subject: RE: [PATCH] net/ice: fix representor port segmentation fault
> >
> > > -----Original Message-----
> > > From: Yu, DapengX <dapengx.yu@intel.com>
> > > Sent: Thursday, September 2, 2021 17:44
> > > To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> > > <qi.z.zhang@intel.com>
> > > Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Yu, DapengX
> > > <dapengx.yu@intel.com>; stable@dpdk.org
> > > Subject: [PATCH] net/ice: fix representor port segmentation fault
> > >
> > > From: Dapeng Yu <dapengx.yu@intel.com>
> > >
> > > If DCF representor port is closed after DCF port is closed, there will
> > > be segmentation fault because representor accesses the released
> > > resource of DCF port.
> > >
> > > This patch makes DCF representor port is always closed before DCF port
> > > is closed.
> > >
> > > Fixes: 5674465a32c8 ("net/ice: add DCF VLAN handling")
> > > Cc: stable@dpdk.org
> > >
> > > Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
> > > ---
> > >  drivers/net/ice/ice_dcf_ethdev.c         |  1 +
> > >  drivers/net/ice/ice_dcf_ethdev.h         |  1 +
> > >  drivers/net/ice/ice_dcf_vf_representor.c | 26
> > > ++++++++++++++++++++----
> > >  3 files changed, 24 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/drivers/net/ice/ice_dcf_ethdev.c
> > > b/drivers/net/ice/ice_dcf_ethdev.c
> > > index cab7c4da87..b837f69fd4 100644
> > > --- a/drivers/net/ice/ice_dcf_ethdev.c
> > > +++ b/drivers/net/ice/ice_dcf_ethdev.c
> > > @@ -874,6 +874,7 @@ ice_dcf_dev_close(struct rte_eth_dev *dev)
> > >  	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> > >  		return 0;
> > >
> > > +	ice_dcf_vf_repr_close_all(adapter);
> > >  	ice_dcf_free_repr_info(adapter);
> > >  	ice_dcf_uninit_parent_adapter(dev);
> > >  	ice_dcf_uninit_hw(dev, &adapter->real_hw); diff --git
> > > a/drivers/net/ice/ice_dcf_ethdev.h b/drivers/net/ice/ice_dcf_ethdev.h
> > > index 8510e37119..9e8d3ef0c5 100644
> > > --- a/drivers/net/ice/ice_dcf_ethdev.h
> > > +++ b/drivers/net/ice/ice_dcf_ethdev.h
> > > @@ -64,5 +64,6 @@ int ice_dcf_vf_repr_init(struct rte_eth_dev
> > > *vf_rep_eth_dev, void *init_param);  int ice_dcf_vf_repr_uninit(struct
> > > rte_eth_dev *vf_rep_eth_dev);  int ice_dcf_vf_repr_init_vlan(struct
> > > rte_eth_dev *vf_rep_eth_dev);  void ice_dcf_vf_repr_stop_all(struct
> > > ice_dcf_adapter *dcf_adapter);
> > > +void ice_dcf_vf_repr_close_all(struct ice_dcf_adapter *dcf_adapter);
> > >
> > >  #endif /* _ICE_DCF_ETHDEV_H_ */
> > > diff --git a/drivers/net/ice/ice_dcf_vf_representor.c
> > > b/drivers/net/ice/ice_dcf_vf_representor.c
> > > index 970461f3e9..121dea7483 100644
> > > --- a/drivers/net/ice/ice_dcf_vf_representor.c
> > > +++ b/drivers/net/ice/ice_dcf_vf_representor.c
> > > @@ -46,6 +46,7 @@ static int
> > >  ice_dcf_vf_repr_dev_stop(struct rte_eth_dev *dev)  {
> > >  	dev->data->dev_link.link_status = ETH_LINK_DOWN;
> > > +	dev->data->dev_started = 0;
> > >
> > >  	return 0;
> > >  }
> > > @@ -53,6 +54,7 @@ ice_dcf_vf_repr_dev_stop(struct rte_eth_dev *dev)
> > > static int  ice_dcf_vf_repr_dev_close(struct rte_eth_dev *dev)  {
> > > +	(void)ice_dcf_vf_repr_dev_stop(dev);
> >
> > Do we truly need the "(void)" ?
> 
> Here use (void) to make discarding of return value explicit.

No need so paranoid ;-)

> There will be no compiler(clang, icc, and gcc) warning even if remove (void)
> So it is fine without that.

Then code is clean.

> > > --
> > > 2.27.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH v2] net/ice: fix representor port segmentation fault
  2021-09-02  9:44 [dpdk-dev] [PATCH] net/ice: fix representor port segmentation fault dapengx.yu
  2021-09-02 14:45 ` Wang, Haiyue
@ 2021-09-03  5:41 ` dapengx.yu
  2021-09-07  2:00   ` [dpdk-dev] [PATCH v3] " dapengx.yu
  1 sibling, 1 reply; 8+ messages in thread
From: dapengx.yu @ 2021-09-03  5:41 UTC (permalink / raw)
  To: Qiming Yang, Qi Zhang; +Cc: dev, haiyue.wang, Dapeng Yu, stable

From: Dapeng Yu <dapengx.yu@intel.com>

If DCF representor port is closed after DCF port is closed, there will
be segmentation fault because representor accesses the released resource
of DCF port.

This patch makes DCF representor port is always closed before DCF port is
closed.

Fixes: 5674465a32c8 ("net/ice: add DCF VLAN handling")
Cc: stable@dpdk.org

Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
---
V2:
 * Remove (void) to make code clean
---
 drivers/net/ice/ice_dcf_ethdev.c         |  1 +
 drivers/net/ice/ice_dcf_ethdev.h         |  1 +
 drivers/net/ice/ice_dcf_vf_representor.c | 26 ++++++++++++++++++++----
 3 files changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/net/ice/ice_dcf_ethdev.c b/drivers/net/ice/ice_dcf_ethdev.c
index cab7c4da87..b837f69fd4 100644
--- a/drivers/net/ice/ice_dcf_ethdev.c
+++ b/drivers/net/ice/ice_dcf_ethdev.c
@@ -874,6 +874,7 @@ ice_dcf_dev_close(struct rte_eth_dev *dev)
 	if (rte_eal_process_type() != RTE_PROC_PRIMARY)
 		return 0;
 
+	ice_dcf_vf_repr_close_all(adapter);
 	ice_dcf_free_repr_info(adapter);
 	ice_dcf_uninit_parent_adapter(dev);
 	ice_dcf_uninit_hw(dev, &adapter->real_hw);
diff --git a/drivers/net/ice/ice_dcf_ethdev.h b/drivers/net/ice/ice_dcf_ethdev.h
index 8510e37119..9e8d3ef0c5 100644
--- a/drivers/net/ice/ice_dcf_ethdev.h
+++ b/drivers/net/ice/ice_dcf_ethdev.h
@@ -64,5 +64,6 @@ int ice_dcf_vf_repr_init(struct rte_eth_dev *vf_rep_eth_dev, void *init_param);
 int ice_dcf_vf_repr_uninit(struct rte_eth_dev *vf_rep_eth_dev);
 int ice_dcf_vf_repr_init_vlan(struct rte_eth_dev *vf_rep_eth_dev);
 void ice_dcf_vf_repr_stop_all(struct ice_dcf_adapter *dcf_adapter);
+void ice_dcf_vf_repr_close_all(struct ice_dcf_adapter *dcf_adapter);
 
 #endif /* _ICE_DCF_ETHDEV_H_ */
diff --git a/drivers/net/ice/ice_dcf_vf_representor.c b/drivers/net/ice/ice_dcf_vf_representor.c
index 970461f3e9..fd9af06c82 100644
--- a/drivers/net/ice/ice_dcf_vf_representor.c
+++ b/drivers/net/ice/ice_dcf_vf_representor.c
@@ -46,6 +46,7 @@ static int
 ice_dcf_vf_repr_dev_stop(struct rte_eth_dev *dev)
 {
 	dev->data->dev_link.link_status = ETH_LINK_DOWN;
+	dev->data->dev_started = 0;
 
 	return 0;
 }
@@ -53,6 +54,7 @@ ice_dcf_vf_repr_dev_stop(struct rte_eth_dev *dev)
 static int
 ice_dcf_vf_repr_dev_close(struct rte_eth_dev *dev)
 {
+	ice_dcf_vf_repr_dev_stop(dev);
 	return ice_dcf_vf_repr_uninit(dev);
 }
 
@@ -464,7 +466,6 @@ void
 ice_dcf_vf_repr_stop_all(struct ice_dcf_adapter *dcf_adapter)
 {
 	uint16_t vf_id;
-	int ret;
 
 	if (!dcf_adapter->repr_infos)
 		return;
@@ -475,8 +476,25 @@ ice_dcf_vf_repr_stop_all(struct ice_dcf_adapter *dcf_adapter)
 		if (!vf_rep_eth_dev || vf_rep_eth_dev->data->dev_started == 0)
 			continue;
 
-		ret = ice_dcf_vf_repr_dev_stop(vf_rep_eth_dev);
-		if (!ret)
-			vf_rep_eth_dev->data->dev_started = 0;
+		ice_dcf_vf_repr_dev_stop(vf_rep_eth_dev);
+	}
+}
+
+void
+ice_dcf_vf_repr_close_all(struct ice_dcf_adapter *dcf_adapter)
+{
+	uint16_t vf_id;
+
+	if (!dcf_adapter->repr_infos)
+		return;
+
+	for (vf_id = 0; vf_id < dcf_adapter->real_hw.num_vfs; vf_id++) {
+		struct rte_eth_dev *vf_rep_eth_dev =
+				dcf_adapter->repr_infos[vf_id].vf_rep_eth_dev;
+		if (!vf_rep_eth_dev)
+			continue;
+
+		ice_dcf_vf_repr_dev_close(vf_rep_eth_dev);
+		rte_eth_dev_release_port(vf_rep_eth_dev);
 	}
 }
-- 
2.27.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [dpdk-dev] [PATCH v3] net/ice: fix representor port segmentation fault
  2021-09-03  5:41 ` [dpdk-dev] [PATCH v2] " dapengx.yu
@ 2021-09-07  2:00   ` dapengx.yu
  2021-09-13  5:46     ` Wang, Haiyue
  0 siblings, 1 reply; 8+ messages in thread
From: dapengx.yu @ 2021-09-07  2:00 UTC (permalink / raw)
  To: Qiming Yang, Qi Zhang; +Cc: dev, haiyue.wang, Dapeng Yu, stable

From: Dapeng Yu <dapengx.yu@intel.com>

If DCF representor port is closed after DCF port is closed, there will
be segmentation fault because representor accesses the released resource
of DCF port.

This patch checks if the resource is present before accessing.

Fixes: 5674465a32c8 ("net/ice: add DCF VLAN handling")
Cc: stable@dpdk.org

Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
---
V2:
 * Remove (void) to make code clean.
V3:
 * Only check resource accessibility but not release it.
---
 drivers/net/ice/ice_dcf_vf_representor.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/drivers/net/ice/ice_dcf_vf_representor.c b/drivers/net/ice/ice_dcf_vf_representor.c
index 970461f3e9..b547c42f91 100644
--- a/drivers/net/ice/ice_dcf_vf_representor.c
+++ b/drivers/net/ice/ice_dcf_vf_representor.c
@@ -114,6 +114,11 @@ ice_dcf_vf_repr_hw(struct ice_dcf_vf_repr *repr)
 	struct ice_dcf_adapter *dcf_adapter =
 			repr->dcf_eth_dev->data->dev_private;
 
+	if (!dcf_adapter) {
+		PMD_DRV_LOG(ERR, "DCF for VF representor has been released\n");
+		return NULL;
+	}
+
 	return &dcf_adapter->real_hw;
 }
 
@@ -124,6 +129,9 @@ ice_dcf_vf_repr_dev_info_get(struct rte_eth_dev *dev,
 	struct ice_dcf_vf_repr *repr = dev->data->dev_private;
 	struct ice_dcf_hw *dcf_hw = ice_dcf_vf_repr_hw(repr);
 
+	if (!dcf_hw)
+		return -EIO;
+
 	dev_info->device = dev->device;
 	dev_info->max_mac_addrs = 1;
 	dev_info->max_rx_queues = dcf_hw->vsi_res->num_queue_pairs;
-- 
2.27.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/ice: fix representor port segmentation fault
  2021-09-07  2:00   ` [dpdk-dev] [PATCH v3] " dapengx.yu
@ 2021-09-13  5:46     ` Wang, Haiyue
  2021-09-13  5:50       ` Zhang, Qi Z
  0 siblings, 1 reply; 8+ messages in thread
From: Wang, Haiyue @ 2021-09-13  5:46 UTC (permalink / raw)
  To: Yu, DapengX, Yang, Qiming, Zhang, Qi Z; +Cc: dev, stable

> -----Original Message-----
> From: Yu, DapengX <dapengx.yu@intel.com>
> Sent: Tuesday, September 7, 2021 10:01
> To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Yu, DapengX <dapengx.yu@intel.com>;
> stable@dpdk.org
> Subject: [PATCH v3] net/ice: fix representor port segmentation fault
> 
> From: Dapeng Yu <dapengx.yu@intel.com>
> 
> If DCF representor port is closed after DCF port is closed, there will
> be segmentation fault because representor accesses the released resource
> of DCF port.
> 
> This patch checks if the resource is present before accessing.
> 
> Fixes: 5674465a32c8 ("net/ice: add DCF VLAN handling")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
> ---
> V2:
>  * Remove (void) to make code clean.
> V3:
>  * Only check resource accessibility but not release it.
> ---
>  drivers/net/ice/ice_dcf_vf_representor.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 

Acked-by: Haiyue Wang <haiyue.wang@intel.com>

> --
> 2.27.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [dpdk-dev] [PATCH v3] net/ice: fix representor port segmentation fault
  2021-09-13  5:46     ` Wang, Haiyue
@ 2021-09-13  5:50       ` Zhang, Qi Z
  0 siblings, 0 replies; 8+ messages in thread
From: Zhang, Qi Z @ 2021-09-13  5:50 UTC (permalink / raw)
  To: Wang, Haiyue, Yu, DapengX, Yang, Qiming; +Cc: dev, stable



> -----Original Message-----
> From: Wang, Haiyue <haiyue.wang@intel.com>
> Sent: Monday, September 13, 2021 1:47 PM
> To: Yu, DapengX <dapengx.yu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: RE: [PATCH v3] net/ice: fix representor port segmentation fault
> 
> > -----Original Message-----
> > From: Yu, DapengX <dapengx.yu@intel.com>
> > Sent: Tuesday, September 7, 2021 10:01
> > To: Yang, Qiming <qiming.yang@intel.com>; Zhang, Qi Z
> > <qi.z.zhang@intel.com>
> > Cc: dev@dpdk.org; Wang, Haiyue <haiyue.wang@intel.com>; Yu, DapengX
> > <dapengx.yu@intel.com>; stable@dpdk.org
> > Subject: [PATCH v3] net/ice: fix representor port segmentation fault
> >
> > From: Dapeng Yu <dapengx.yu@intel.com>
> >
> > If DCF representor port is closed after DCF port is closed, there will
> > be segmentation fault because representor accesses the released
> > resource of DCF port.
> >
> > This patch checks if the resource is present before accessing.
> >
> > Fixes: 5674465a32c8 ("net/ice: add DCF VLAN handling")
> > Cc: stable@dpdk.org
> >
> > Signed-off-by: Dapeng Yu <dapengx.yu@intel.com>
> > ---
> > V2:
> >  * Remove (void) to make code clean.
> > V3:
> >  * Only check resource accessibility but not release it.
> > ---
> >  drivers/net/ice/ice_dcf_vf_representor.c | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> 
> Acked-by: Haiyue Wang <haiyue.wang@intel.com>

Applied to dpdk-next-net-intel.

Thanks
Qi

> 
> > --
> > 2.27.0
> 


^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-09-13  5:51 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-09-02  9:44 [dpdk-dev] [PATCH] net/ice: fix representor port segmentation fault dapengx.yu
2021-09-02 14:45 ` Wang, Haiyue
2021-09-03  3:08   ` Yu, DapengX
2021-09-03  4:37     ` Wang, Haiyue
2021-09-03  5:41 ` [dpdk-dev] [PATCH v2] " dapengx.yu
2021-09-07  2:00   ` [dpdk-dev] [PATCH v3] " dapengx.yu
2021-09-13  5:46     ` Wang, Haiyue
2021-09-13  5:50       ` Zhang, Qi Z

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).