Re: [dpdk-dev] net/iavf: fix invalid flow access

[Ye Xiaolong <xiaolong.ye@intel.com>]

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.