DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev]  net/iavf: fix invalid flow access
@ 2020-05-22  2:11 Jeff Guo
  2020-05-22  3:43 ` Peng, Yuan
  2020-05-22  9:12 ` Ye Xiaolong
  0 siblings, 2 replies; 4+ messages in thread
From: Jeff Guo @ 2020-05-22  2:11 UTC (permalink / raw)
  To: beilei.xing, qi.z.zhang, jingjing.wu, qiming.yang
  Cc: xiaolong.ye, dev, jia.guo

When hash init, the default rss rules would be added, while hash uninit,
the default rss rules should be deleted. Add the missing part in the hash
uninit process. Also add invalid flow checking func in iavf generic flow
to avoid the error of "Cannot access memory at address 0xXXXXXX" occur.

Fixes: 5ea614254332 ("net/iavf: fix VF reset for RSS")
Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
 drivers/net/iavf/iavf_generic_flow.c | 24 ++++++++++++++++++++----
 drivers/net/iavf/iavf_hash.c         | 12 ++++++++----
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/net/iavf/iavf_generic_flow.c b/drivers/net/iavf/iavf_generic_flow.c
index c0c67d0c7..b6c26c4fd 100644
--- a/drivers/net/iavf/iavf_generic_flow.c
+++ b/drivers/net/iavf/iavf_generic_flow.c
@@ -935,6 +935,22 @@ iavf_flow_create(struct rte_eth_dev *dev,
 	return flow;
 }
 
+static bool
+iavf_flow_is_valid(struct rte_flow *flow)
+{
+	struct iavf_flow_engine *engine;
+	void *temp;
+
+	if (flow && flow->engine) {
+		TAILQ_FOREACH_SAFE(engine, &engine_list, node, temp) {
+			if (engine == flow->engine)
+				return true;
+		}
+	}
+
+	return false;
+}
+
 static int
 iavf_flow_destroy(struct rte_eth_dev *dev,
 		  struct rte_flow *flow,
@@ -945,10 +961,10 @@ iavf_flow_destroy(struct rte_eth_dev *dev,
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
 	int ret = 0;
 
-	if (!flow || !flow->engine || !flow->engine->destroy) {
+	if (!iavf_flow_is_valid(flow) || !flow->engine->destroy) {
 		rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_HANDLE,
-				   NULL, "Invalid flow");
+				   NULL, "Invalid flow destroy");
 		return -rte_errno;
 	}
 
@@ -1002,10 +1018,10 @@ iavf_flow_query(struct rte_eth_dev *dev,
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct rte_flow_query_count *count = data;
 
-	if (!flow || !flow->engine || !flow->engine->query_count) {
+	if (!iavf_flow_is_valid(flow) || !flow->engine->query_count) {
 		rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_HANDLE,
-				   NULL, "Invalid flow");
+				   NULL, "Invalid flow query");
 		return -rte_errno;
 	}
 
diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
index 56ab170d8..af528863b 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -887,7 +887,7 @@ static struct iavf_flow_parser iavf_hash_parser = {
 };
 
 static int
-iavf_hash_default_set(struct iavf_adapter *ad)
+iavf_hash_default_set(struct iavf_adapter *ad, bool add)
 {
 	struct virtchnl_rss_cfg *rss_cfg;
 	uint16_t i;
@@ -902,9 +902,10 @@ iavf_hash_default_set(struct iavf_adapter *ad)
 		rss_cfg->proto_hdrs = *iavf_hash_default_hdrs[i];
 		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
 
-		ret = iavf_add_del_rss_cfg(ad, rss_cfg, true);
+		ret = iavf_add_del_rss_cfg(ad, rss_cfg, add);
 		if (ret) {
-			PMD_DRV_LOG(ERR, "fail to add RSS configure");
+			PMD_DRV_LOG(ERR, "fail to %s RSS configure",
+				    add ? "add" : "delete");
 			rte_free(rss_cfg);
 			return ret;
 		}
@@ -941,7 +942,7 @@ iavf_hash_init(struct iavf_adapter *ad)
 		return ret;
 	}
 
-	ret = iavf_hash_default_set(ad);
+	ret = iavf_hash_default_set(ad, true);
 	if (ret) {
 		PMD_DRV_LOG(ERR, "fail to set default RSS");
 		iavf_unregister_parser(parser, ad);
@@ -1222,6 +1223,9 @@ iavf_hash_destroy(__rte_unused struct iavf_adapter *ad,
 static void
 iavf_hash_uninit(struct iavf_adapter *ad)
 {
+	if (iavf_hash_default_set(ad, false))
+		PMD_DRV_LOG(ERR, "fail to delete default RSS");
+
 	iavf_unregister_parser(&iavf_hash_parser, ad);
 }
 
-- 
2.20.1


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

* Re: [dpdk-dev] net/iavf: fix invalid flow access
  2020-05-22  2:11 [dpdk-dev] net/iavf: fix invalid flow access Jeff Guo
@ 2020-05-22  3:43 ` Peng, Yuan
  2020-05-22  7:43   ` Zhang, Qi Z
  2020-05-22  9:12 ` Ye Xiaolong
  1 sibling, 1 reply; 4+ messages in thread
From: Peng, Yuan @ 2020-05-22  3:43 UTC (permalink / raw)
  To: Guo, Jia, Xing, Beilei, Zhang, Qi Z, Wu, Jingjing, Yang, Qiming
  Cc: Ye, Xiaolong, dev, Guo, Jia

Test-by Peng, Yuan <yuan.peng@intel.com>

-----Original Message-----
From: dev <dev-bounces@dpdk.org> On Behalf Of Jeff Guo
Sent: Friday, May 22, 2020 10:12 AM
To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Yang, Qiming <qiming.yang@intel.com>
Cc: Ye, Xiaolong <xiaolong.ye@intel.com>; dev@dpdk.org; Guo, Jia <jia.guo@intel.com>
Subject: [dpdk-dev] net/iavf: fix invalid flow access

When hash init, the default rss rules would be added, while hash uninit, the default rss rules should be deleted. Add the missing part in the hash uninit process. Also add invalid flow checking func in iavf generic flow to avoid the error of "Cannot access memory at address 0xXXXXXX" occur.

Fixes: 5ea614254332 ("net/iavf: fix VF reset for RSS")
Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")

Signed-off-by: Jeff Guo <jia.guo@intel.com>
---
 drivers/net/iavf/iavf_generic_flow.c | 24 ++++++++++++++++++++----
 drivers/net/iavf/iavf_hash.c         | 12 ++++++++----
 2 files changed, 28 insertions(+), 8 deletions(-)

diff --git a/drivers/net/iavf/iavf_generic_flow.c b/drivers/net/iavf/iavf_generic_flow.c
index c0c67d0c7..b6c26c4fd 100644
--- a/drivers/net/iavf/iavf_generic_flow.c
+++ b/drivers/net/iavf/iavf_generic_flow.c
@@ -935,6 +935,22 @@ iavf_flow_create(struct rte_eth_dev *dev,
 	return flow;
 }
 
+static bool
+iavf_flow_is_valid(struct rte_flow *flow) {
+	struct iavf_flow_engine *engine;
+	void *temp;
+
+	if (flow && flow->engine) {
+		TAILQ_FOREACH_SAFE(engine, &engine_list, node, temp) {
+			if (engine == flow->engine)
+				return true;
+		}
+	}
+
+	return false;
+}
+
 static int
 iavf_flow_destroy(struct rte_eth_dev *dev,
 		  struct rte_flow *flow,
@@ -945,10 +961,10 @@ iavf_flow_destroy(struct rte_eth_dev *dev,
 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
 	int ret = 0;
 
-	if (!flow || !flow->engine || !flow->engine->destroy) {
+	if (!iavf_flow_is_valid(flow) || !flow->engine->destroy) {
 		rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_HANDLE,
-				   NULL, "Invalid flow");
+				   NULL, "Invalid flow destroy");
 		return -rte_errno;
 	}
 
@@ -1002,10 +1018,10 @@ iavf_flow_query(struct rte_eth_dev *dev,
 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
 	struct rte_flow_query_count *count = data;
 
-	if (!flow || !flow->engine || !flow->engine->query_count) {
+	if (!iavf_flow_is_valid(flow) || !flow->engine->query_count) {
 		rte_flow_error_set(error, EINVAL,
 				   RTE_FLOW_ERROR_TYPE_HANDLE,
-				   NULL, "Invalid flow");
+				   NULL, "Invalid flow query");
 		return -rte_errno;
 	}
 
diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c index 56ab170d8..af528863b 100644
--- a/drivers/net/iavf/iavf_hash.c
+++ b/drivers/net/iavf/iavf_hash.c
@@ -887,7 +887,7 @@ static struct iavf_flow_parser iavf_hash_parser = {  };
 
 static int
-iavf_hash_default_set(struct iavf_adapter *ad)
+iavf_hash_default_set(struct iavf_adapter *ad, bool add)
 {
 	struct virtchnl_rss_cfg *rss_cfg;
 	uint16_t i;
@@ -902,9 +902,10 @@ iavf_hash_default_set(struct iavf_adapter *ad)
 		rss_cfg->proto_hdrs = *iavf_hash_default_hdrs[i];
 		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
 
-		ret = iavf_add_del_rss_cfg(ad, rss_cfg, true);
+		ret = iavf_add_del_rss_cfg(ad, rss_cfg, add);
 		if (ret) {
-			PMD_DRV_LOG(ERR, "fail to add RSS configure");
+			PMD_DRV_LOG(ERR, "fail to %s RSS configure",
+				    add ? "add" : "delete");
 			rte_free(rss_cfg);
 			return ret;
 		}
@@ -941,7 +942,7 @@ iavf_hash_init(struct iavf_adapter *ad)
 		return ret;
 	}
 
-	ret = iavf_hash_default_set(ad);
+	ret = iavf_hash_default_set(ad, true);
 	if (ret) {
 		PMD_DRV_LOG(ERR, "fail to set default RSS");
 		iavf_unregister_parser(parser, ad);
@@ -1222,6 +1223,9 @@ iavf_hash_destroy(__rte_unused struct iavf_adapter *ad,  static void  iavf_hash_uninit(struct iavf_adapter *ad)  {
+	if (iavf_hash_default_set(ad, false))
+		PMD_DRV_LOG(ERR, "fail to delete default RSS");
+
 	iavf_unregister_parser(&iavf_hash_parser, ad);  }
 
--
2.20.1


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

* Re: [dpdk-dev] net/iavf: fix invalid flow access
  2020-05-22  3:43 ` Peng, Yuan
@ 2020-05-22  7:43   ` Zhang, Qi Z
  0 siblings, 0 replies; 4+ messages in thread
From: Zhang, Qi Z @ 2020-05-22  7:43 UTC (permalink / raw)
  To: Peng, Yuan, Guo, Jia, Xing, Beilei, Wu, Jingjing, Yang, Qiming
  Cc: Ye, Xiaolong, dev, Guo, Jia



> -----Original Message-----
> From: Peng, Yuan <yuan.peng@intel.com>
> Sent: Friday, May 22, 2020 11:44 AM
> To: Guo, Jia <jia.guo@intel.com>; Xing, Beilei <beilei.xing@intel.com>; Zhang,
> Qi Z <qi.z.zhang@intel.com>; Wu, Jingjing <jingjing.wu@intel.com>; Yang,
> Qiming <qiming.yang@intel.com>
> Cc: Ye, Xiaolong <xiaolong.ye@intel.com>; dev@dpdk.org; Guo, Jia
> <jia.guo@intel.com>
> Subject: RE: [dpdk-dev] net/iavf: fix invalid flow access
> 
> Test-by Peng, Yuan <yuan.peng@intel.com>
> 
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Jeff Guo
> Sent: Friday, May 22, 2020 10:12 AM
> To: Xing, Beilei <beilei.xing@intel.com>; Zhang, Qi Z <qi.z.zhang@intel.com>;
> Wu, Jingjing <jingjing.wu@intel.com>; Yang, Qiming
> <qiming.yang@intel.com>
> Cc: Ye, Xiaolong <xiaolong.ye@intel.com>; dev@dpdk.org; Guo, Jia
> <jia.guo@intel.com>
> Subject: [dpdk-dev] net/iavf: fix invalid flow access
> 
> When hash init, the default rss rules would be added, while hash uninit, the
> default rss rules should be deleted. Add the missing part in the hash uninit
> process. Also add invalid flow checking func in iavf generic flow to avoid the
> error of "Cannot access memory at address 0xXXXXXX" occur.
> 
> Fixes: 5ea614254332 ("net/iavf: fix VF reset for RSS")
> Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
> 
> Signed-off-by: Jeff Guo <jia.guo@intel.com>

Acked-by: Qi Zhang <qi.z.zhang@intel.com>



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

* Re: [dpdk-dev] net/iavf: fix invalid flow access
  2020-05-22  2:11 [dpdk-dev] net/iavf: fix invalid flow access Jeff Guo
  2020-05-22  3:43 ` Peng, Yuan
@ 2020-05-22  9:12 ` Ye Xiaolong
  1 sibling, 0 replies; 4+ messages in thread
From: Ye Xiaolong @ 2020-05-22  9:12 UTC (permalink / raw)
  To: Jeff Guo; +Cc: beilei.xing, qi.z.zhang, jingjing.wu, qiming.yang, dev

On 05/21, Jeff Guo wrote:
>When hash init, the default rss rules would be added, while hash uninit,
>the default rss rules should be deleted. Add the missing part in the hash
>uninit process. Also add invalid flow checking func in iavf generic flow
>to avoid the error of "Cannot access memory at address 0xXXXXXX" occur.
>
>Fixes: 5ea614254332 ("net/iavf: fix VF reset for RSS")
>Fixes: ff2d0c345c3b ("net/iavf: support generic flow API")
>
>Signed-off-by: Jeff Guo <jia.guo@intel.com>
>---
> drivers/net/iavf/iavf_generic_flow.c | 24 ++++++++++++++++++++----
> drivers/net/iavf/iavf_hash.c         | 12 ++++++++----
> 2 files changed, 28 insertions(+), 8 deletions(-)
>
>diff --git a/drivers/net/iavf/iavf_generic_flow.c b/drivers/net/iavf/iavf_generic_flow.c
>index c0c67d0c7..b6c26c4fd 100644
>--- a/drivers/net/iavf/iavf_generic_flow.c
>+++ b/drivers/net/iavf/iavf_generic_flow.c
>@@ -935,6 +935,22 @@ iavf_flow_create(struct rte_eth_dev *dev,
> 	return flow;
> }
> 
>+static bool
>+iavf_flow_is_valid(struct rte_flow *flow)
>+{
>+	struct iavf_flow_engine *engine;
>+	void *temp;
>+
>+	if (flow && flow->engine) {
>+		TAILQ_FOREACH_SAFE(engine, &engine_list, node, temp) {
>+			if (engine == flow->engine)
>+				return true;
>+		}
>+	}
>+
>+	return false;
>+}
>+
> static int
> iavf_flow_destroy(struct rte_eth_dev *dev,
> 		  struct rte_flow *flow,
>@@ -945,10 +961,10 @@ iavf_flow_destroy(struct rte_eth_dev *dev,
> 	struct iavf_info *vf = IAVF_DEV_PRIVATE_TO_VF(ad);
> 	int ret = 0;
> 
>-	if (!flow || !flow->engine || !flow->engine->destroy) {
>+	if (!iavf_flow_is_valid(flow) || !flow->engine->destroy) {
> 		rte_flow_error_set(error, EINVAL,
> 				   RTE_FLOW_ERROR_TYPE_HANDLE,
>-				   NULL, "Invalid flow");
>+				   NULL, "Invalid flow destroy");
> 		return -rte_errno;
> 	}
> 
>@@ -1002,10 +1018,10 @@ iavf_flow_query(struct rte_eth_dev *dev,
> 		IAVF_DEV_PRIVATE_TO_ADAPTER(dev->data->dev_private);
> 	struct rte_flow_query_count *count = data;
> 
>-	if (!flow || !flow->engine || !flow->engine->query_count) {
>+	if (!iavf_flow_is_valid(flow) || !flow->engine->query_count) {
> 		rte_flow_error_set(error, EINVAL,
> 				   RTE_FLOW_ERROR_TYPE_HANDLE,
>-				   NULL, "Invalid flow");
>+				   NULL, "Invalid flow query");
> 		return -rte_errno;
> 	}
> 
>diff --git a/drivers/net/iavf/iavf_hash.c b/drivers/net/iavf/iavf_hash.c
>index 56ab170d8..af528863b 100644
>--- a/drivers/net/iavf/iavf_hash.c
>+++ b/drivers/net/iavf/iavf_hash.c
>@@ -887,7 +887,7 @@ static struct iavf_flow_parser iavf_hash_parser = {
> };
> 
> static int
>-iavf_hash_default_set(struct iavf_adapter *ad)
>+iavf_hash_default_set(struct iavf_adapter *ad, bool add)
> {
> 	struct virtchnl_rss_cfg *rss_cfg;
> 	uint16_t i;
>@@ -902,9 +902,10 @@ iavf_hash_default_set(struct iavf_adapter *ad)
> 		rss_cfg->proto_hdrs = *iavf_hash_default_hdrs[i];
> 		rss_cfg->rss_algorithm = VIRTCHNL_RSS_ALG_TOEPLITZ_ASYMMETRIC;
> 
>-		ret = iavf_add_del_rss_cfg(ad, rss_cfg, true);
>+		ret = iavf_add_del_rss_cfg(ad, rss_cfg, add);
> 		if (ret) {
>-			PMD_DRV_LOG(ERR, "fail to add RSS configure");
>+			PMD_DRV_LOG(ERR, "fail to %s RSS configure",
>+				    add ? "add" : "delete");
> 			rte_free(rss_cfg);
> 			return ret;
> 		}
>@@ -941,7 +942,7 @@ iavf_hash_init(struct iavf_adapter *ad)
> 		return ret;
> 	}
> 
>-	ret = iavf_hash_default_set(ad);
>+	ret = iavf_hash_default_set(ad, true);
> 	if (ret) {
> 		PMD_DRV_LOG(ERR, "fail to set default RSS");
> 		iavf_unregister_parser(parser, ad);
>@@ -1222,6 +1223,9 @@ iavf_hash_destroy(__rte_unused struct iavf_adapter *ad,
> static void
> iavf_hash_uninit(struct iavf_adapter *ad)
> {
>+	if (iavf_hash_default_set(ad, false))
>+		PMD_DRV_LOG(ERR, "fail to delete default RSS");
>+
> 	iavf_unregister_parser(&iavf_hash_parser, ad);
> }
> 
>-- 
>2.20.1
>

Applied to dpdk-next-net-intel, Thanks.

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

end of thread, other threads:[~2020-05-22  9:21 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-22  2:11 [dpdk-dev] net/iavf: fix invalid flow access Jeff Guo
2020-05-22  3:43 ` Peng, Yuan
2020-05-22  7:43   ` Zhang, Qi Z
2020-05-22  9:12 ` Ye Xiaolong

DPDK patches and discussions

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://inbox.dpdk.org/dev/0 dev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 dev dev/ https://inbox.dpdk.org/dev \
		dev@dpdk.org
	public-inbox-index dev

Example config snippet for mirrors.
Newsgroup available over NNTP:
	nntp://inbox.dpdk.org/inbox.dpdk.dev


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git