[dpdk-dev] [PATCH v2 22/30] net/mlx5: fully convert a flow to verbs in validate

[Nelio Laranjeiro]

Validation of flows is only making few verifications on the pattern, in
some situation the validate action could end by with success whereas the
pattern could not be converted correctly.

This brings this conversion verification part also to the validate.

Signed-off-by: Nelio Laranjeiro <nelio.laranjeiro at 6wind.com>
---
 drivers/net/mlx5/mlx5_flow.c | 194 +++++++++++++++++++++++++------------------
 1 file changed, 114 insertions(+), 80 deletions(-)

diff --git a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c
index 88d02c9..090a298 100644
--- a/drivers/net/mlx5/mlx5_flow.c
+++ b/drivers/net/mlx5/mlx5_flow.c
@@ -307,6 +307,7 @@ struct mlx5_flow_parse {
 	struct ibv_flow_attr *ibv_attr; /**< Verbs attribute. */
 	unsigned int offset; /**< Offset in bytes in the ibv_attr buffer. */
 	uint32_t inner; /**< Set once VXLAN is encountered. */
+	uint32_t create:1; /**< Leave allocated resources on exit. */
 	uint64_t hash_fields; /**< Fields that participate in the hash. */
 	struct mlx5_flow_action actions; /**< Parsed action result. */
 };
@@ -418,7 +419,7 @@ mlx5_flow_item_validate(const struct rte_flow_item *item,
 }
 
 /**
- * Validate a flow supported by the NIC.
+ * Validate and convert a flow supported by the NIC.
  *
  * @param priv
  *   Pointer to private structure.
@@ -437,16 +438,24 @@ mlx5_flow_item_validate(const struct rte_flow_item *item,
  *   0 on success, a negative errno value otherwise and rte_errno is set.
  */
 static int
-priv_flow_validate(struct priv *priv,
-		   const struct rte_flow_attr *attr,
-		   const struct rte_flow_item items[],
-		   const struct rte_flow_action actions[],
-		   struct rte_flow_error *error,
-		   struct mlx5_flow_parse *flow)
+priv_flow_convert(struct priv *priv,
+		  const struct rte_flow_attr *attr,
+		  const struct rte_flow_item items[],
+		  const struct rte_flow_action actions[],
+		  struct rte_flow_error *error,
+		  struct mlx5_flow_parse *flow)
 {
 	const struct mlx5_flow_items *cur_item = mlx5_flow_items;
 
 	(void)priv;
+	*flow = (struct mlx5_flow_parse){
+		.ibv_attr = flow->ibv_attr,
+		.create = flow->create,
+		.offset = sizeof(struct ibv_flow_attr),
+		.actions = {
+			.mark_id = MLX5_FLOW_MARK_DEFAULT,
+		},
+	};
 	if (attr->group) {
 		rte_flow_error_set(error, ENOTSUP,
 				   RTE_FLOW_ERROR_TYPE_ATTR_GROUP,
@@ -647,35 +656,6 @@ priv_flow_validate(struct priv *priv,
 }
 
 /**
- * Validate a flow supported by the NIC.
- *
- * @see rte_flow_validate()
- * @see rte_flow_ops
- */
-int
-mlx5_flow_validate(struct rte_eth_dev *dev,
-		   const struct rte_flow_attr *attr,
-		   const struct rte_flow_item items[],
-		   const struct rte_flow_action actions[],
-		   struct rte_flow_error *error)
-{
-	struct priv *priv = dev->data->dev_private;
-	int ret;
-	struct mlx5_flow_parse flow = {
-		.offset = sizeof(struct ibv_flow_attr),
-		.actions = {
-			.mark_id = MLX5_FLOW_MARK_DEFAULT,
-			.queues_n = 0,
-		},
-	};
-
-	priv_lock(priv);
-	ret = priv_flow_validate(priv, attr, items, actions, error, &flow);
-	priv_unlock(priv);
-	return ret;
-}
-
-/**
  * Convert Ethernet item to Verbs specification.
  *
  * @param item[in]
@@ -1016,6 +996,7 @@ mlx5_flow_create_flag_mark(struct mlx5_flow_parse *flow, uint32_t mark_id)
 	struct ibv_flow_spec_action_tag *tag;
 	unsigned int size = sizeof(struct ibv_flow_spec_action_tag);
 
+	assert(flow->actions.mark);
 	tag = (void *)((uintptr_t)flow->ibv_attr + flow->offset);
 	*tag = (struct ibv_flow_spec_action_tag){
 		.type = IBV_FLOW_SPEC_ACTION_TAG,
@@ -1023,6 +1004,7 @@ mlx5_flow_create_flag_mark(struct mlx5_flow_parse *flow, uint32_t mark_id)
 		.tag_id = mlx5_flow_mark_set(mark_id),
 	};
 	++flow->ibv_attr->num_of_specs;
+	flow->offset += size;
 	return 0;
 }
 
@@ -1167,12 +1149,10 @@ priv_flow_create_action_queue(struct priv *priv,
 }
 
 /**
- * Convert a flow.
+ * Validate a flow.
  *
  * @param priv
  *   Pointer to private structure.
- * @param list
- *   Pointer to a TAILQ flow list.
  * @param[in] attr
  *   Flow rule attributes.
  * @param[in] pattern
@@ -1181,40 +1161,35 @@ priv_flow_create_action_queue(struct priv *priv,
  *   Associated actions (list terminated by the END action).
  * @param[out] error
  *   Perform verbose error reporting if not NULL.
+ * @param[in,out] parser
+ *   MLX5 parser structure.
  *
  * @return
- *   A flow on success, NULL otherwise.
+ *   0 on success, negative errno value on failure.
  */
-static struct rte_flow *
-priv_flow_create(struct priv *priv,
-		 struct mlx5_flows *list,
-		 const struct rte_flow_attr *attr,
-		 const struct rte_flow_item items[],
-		 const struct rte_flow_action actions[],
-		 struct rte_flow_error *error)
+static int
+priv_flow_validate(struct priv *priv,
+		   const struct rte_flow_attr *attr,
+		   const struct rte_flow_item items[],
+		   const struct rte_flow_action actions[],
+		   struct rte_flow_error *error,
+		   struct mlx5_flow_parse *parser)
 {
-	struct rte_flow *rte_flow;
-	struct mlx5_flow_parse flow = {
-		.offset = sizeof(struct ibv_flow_attr),
-		.actions = {
-			.mark_id = MLX5_FLOW_MARK_DEFAULT,
-			.queues = { 0 },
-			.queues_n = 0,
-		},
-	};
 	int err;
 
-	err = priv_flow_validate(priv, attr, items, actions, error, &flow);
+	err = priv_flow_convert(priv, attr, items, actions, error, parser);
 	if (err)
 		goto exit;
-	flow.ibv_attr = rte_malloc(__func__, flow.offset, 0);
-	flow.offset = sizeof(struct ibv_flow_attr);
-	if (!flow.ibv_attr) {
+	if (parser->actions.mark)
+		parser->offset += sizeof(struct ibv_flow_spec_action_tag);
+	parser->ibv_attr = rte_malloc(__func__, parser->offset, 0);
+	if (!parser->ibv_attr) {
 		rte_flow_error_set(error, ENOMEM, RTE_FLOW_ERROR_TYPE_HANDLE,
 				   NULL, "cannot allocate ibv_attr memory");
+		err = rte_errno;
 		goto exit;
 	}
-	*flow.ibv_attr = (struct ibv_flow_attr){
+	*parser->ibv_attr = (struct ibv_flow_attr){
 		.type = IBV_FLOW_ATTR_NORMAL,
 		.size = sizeof(struct ibv_flow_attr),
 		.priority = attr->priority,
@@ -1222,32 +1197,91 @@ priv_flow_create(struct priv *priv,
 		.port = 0,
 		.flags = 0,
 	};
-	flow.inner = 0;
-	flow.hash_fields = 0;
-	claim_zero(priv_flow_validate(priv, attr, items, actions,
-				      error, &flow));
-	if (flow.actions.mark && !flow.actions.drop) {
-		mlx5_flow_create_flag_mark(&flow, flow.actions.mark_id);
-		flow.offset += sizeof(struct ibv_flow_spec_action_tag);
-	}
-	if (flow.actions.drop)
-		rte_flow =
-			priv_flow_create_action_queue_drop(priv, &flow, error);
+	err = priv_flow_convert(priv, attr, items, actions, error, parser);
+	if (err || parser->create)
+		goto exit;
+	if (parser->actions.mark)
+		mlx5_flow_create_flag_mark(parser, parser->actions.mark_id);
+	return 0;
+exit:
+	if (parser->ibv_attr)
+		rte_free(parser->ibv_attr);
+	return err;
+}
+
+/**
+ * Convert a flow.
+ *
+ * @param priv
+ *   Pointer to private structure.
+ * @param list
+ *   Pointer to a TAILQ flow list.
+ * @param[in] attr
+ *   Flow rule attributes.
+ * @param[in] pattern
+ *   Pattern specification (list terminated by the END pattern item).
+ * @param[in] actions
+ *   Associated actions (list terminated by the END action).
+ * @param[out] error
+ *   Perform verbose error reporting if not NULL.
+ *
+ * @return
+ *   A flow on success, NULL otherwise.
+ */
+static struct rte_flow *
+priv_flow_create(struct priv *priv,
+		 struct mlx5_flows *list,
+		 const struct rte_flow_attr *attr,
+		 const struct rte_flow_item items[],
+		 const struct rte_flow_action actions[],
+		 struct rte_flow_error *error)
+{
+	struct mlx5_flow_parse parser = { .create = 1, };
+	struct rte_flow *flow;
+	int err;
+
+	err = priv_flow_validate(priv, attr, items, actions, error, &parser);
+	if (err)
+		goto exit;
+	if (parser.actions.drop)
+		flow = priv_flow_create_action_queue_drop(priv, &parser, error);
 	else
-		rte_flow = priv_flow_create_action_queue(priv, &flow, error);
-	if (!rte_flow)
+		flow = priv_flow_create_action_queue(priv, &parser, error);
+	if (!flow)
 		goto exit;
-	if (rte_flow) {
-		TAILQ_INSERT_TAIL(list, rte_flow, next);
-		DEBUG("Flow created %p", (void *)rte_flow);
-	}
-	return rte_flow;
+	TAILQ_INSERT_TAIL(list, flow, next);
+	DEBUG("Flow created %p", (void *)flow);
+	return flow;
 exit:
-	rte_free(flow.ibv_attr);
+	if (parser.ibv_attr)
+		rte_free(parser.ibv_attr);
 	return NULL;
 }
 
 /**
+ * Validate a flow supported by the NIC.
+ *
+ * @see rte_flow_validate()
+ * @see rte_flow_ops
+ */
+int
+mlx5_flow_validate(struct rte_eth_dev *dev,
+		   const struct rte_flow_attr *attr,
+		   const struct rte_flow_item items[],
+		   const struct rte_flow_action actions[],
+		   struct rte_flow_error *error)
+{
+	struct priv *priv = dev->data->dev_private;
+	int ret;
+	struct mlx5_flow_parse parser = { .create = 0, };
+
+	priv_lock(priv);
+	ret = priv_flow_validate(priv, attr, items, actions, error, &parser);
+	priv_unlock(priv);
+	return ret;
+}
+
+/**
  * Create a flow.
  *
  * @see rte_flow_create()
-- 
2.1.4