[dpdk-dev,v5,3/4] net/mlx4: support for the RSS flow action

  The isolated mode should be enabled.
The number of queues in RSS ring must be power of 2.
The sharing a queue between several RSS rings is impossible.

Signed-off-by: Vasily Philipov <vasilyf@mellanox.com>
---
 drivers/net/mlx4/mlx4.c      |  21 +++--
 drivers/net/mlx4/mlx4.h      |   5 ++
 drivers/net/mlx4/mlx4_flow.c | 197 ++++++++++++++++++++++++++++++++++++++++++-
 drivers/net/mlx4/mlx4_flow.h |   3 +-
 4 files changed, 211 insertions(+), 15 deletions(-)
  

On Wed, Jun 28, 2017 at 05:03:56PM +0300, Vasily Philipov wrote:
> The isolated mode should be enabled.
> The number of queues in RSS ring must be power of 2.
> The sharing a queue between several RSS rings is impossible.
> 
> Signed-off-by: Vasily Philipov <vasilyf@mellanox.com>

Alternative suggestion for commit log:

 net/mlx4: support flow API RSS action

 This commit adds support for the flow API RSS action with the following
 limitations:

 - Only supported when isolated mode is enabled.
 - The number of queues specified by the action (rte_flow_action_rss.num)
   must be a power of two.
 - Each queue index can be specified at most once in the configuration
   (rte_flow_action_rss.queue[]).
 - Because a queue can be associated with a single RSS context, it cannot be
   targeted by multiple RSS actions simultaneously. 

A few more comments about this patch, see below.

> ---
>  drivers/net/mlx4/mlx4.c      |  21 +++--
>  drivers/net/mlx4/mlx4.h      |   5 ++
>  drivers/net/mlx4/mlx4_flow.c | 197 ++++++++++++++++++++++++++++++++++++++++++-
>  drivers/net/mlx4/mlx4_flow.h |   3 +-
>  4 files changed, 211 insertions(+), 15 deletions(-)
> 
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index 22fa7c6..6ab7241 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -554,9 +554,9 @@ void priv_unlock(struct priv *priv)
>   *   The number of entries in queues[].
>   *
>   * @return
> - *   0 on success, negative errno value on failure.
> + *   Pointer to a parent rxq structure, NULL on failure.
>   */
> -static int
> +struct rxq *
>  priv_create_parent(struct priv *priv,
>  		   uint16_t queues[],
>  		   uint16_t children_n)
> @@ -568,13 +568,15 @@ void priv_unlock(struct priv *priv)
>  	parent = rte_zmalloc("parent queue",
>  			     sizeof(*parent),
>  			     RTE_CACHE_LINE_SIZE);
> -	if (!parent)
> -		return -ENOMEM;
> +	if (!parent) {
> +		ERROR("cannot allocate memory for RSS parent queue");
> +		return NULL;
> +	}
>  	ret = rxq_setup(priv->dev, parent, 0, 0, 0,
>  			NULL, NULL, children_n, NULL);
>  	if (ret) {
>  		rte_free(parent);
> -		return -ret;
> +		return NULL;
>  	}
>  	parent->rss.queues_n = children_n;
>  	if (queues) {
> @@ -587,7 +589,7 @@ void priv_unlock(struct priv *priv)
>  			parent->rss.queues[i] = i;
>  	}
>  	LIST_INSERT_HEAD(&priv->parents, parent, next);
> -	return 0;
> +	return parent;
>  }
> 
>  /**
> @@ -639,7 +641,6 @@ void priv_unlock(struct priv *priv)
>  	unsigned int rxqs_n = dev->data->nb_rx_queues;
>  	unsigned int txqs_n = dev->data->nb_tx_queues;
>  	unsigned int tmp;
> -	int ret;
>  
>  	priv->rxqs = (void *)dev->data->rx_queues;
>  	priv->txqs = (void *)dev->data->tx_queues;
> @@ -696,14 +697,12 @@ void priv_unlock(struct priv *priv)
>  	priv->rxqs_n = rxqs_n;
>  	if (priv->isolated)
>  		return 0;
> -	ret = priv_create_parent(priv, NULL, priv->rxqs_n);
> -	if (!ret)
> +	if (priv_create_parent(priv, NULL, priv->rxqs_n))
>  		return 0;
>  	/* Failure, rollback. */
>  	priv->rss = 0;
>  	priv->rxqs_n = tmp;
> -	assert(ret > 0);
> -	return ret;
> +	return ENOMEM;
>  }

I think the above changes should be merged in the previous commit
("net/mlx4: RSS parent queues new method maintenance"). priv_create_parent()
can return the rxq pointer (or NULL in case of error) from the start.

>  /**
> diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
> index b5fe1b4..f45e017 100644
> --- a/drivers/net/mlx4/mlx4.h
> +++ b/drivers/net/mlx4/mlx4.h
> @@ -370,4 +370,9 @@ struct priv {
>  void
>  rxq_parent_cleanup(struct rxq *parent);
>  
> +struct rxq *
> +priv_create_parent(struct priv *priv,
> +		   uint16_t queues[],
> +		   uint16_t children_n);
> +
>  #endif /* RTE_PMD_MLX4_H_ */
> diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
> index 3fd2716..9c0fba1 100644
> --- a/drivers/net/mlx4/mlx4_flow.c
> +++ b/drivers/net/mlx4/mlx4_flow.c
> @@ -112,6 +112,7 @@ struct rte_flow_drop {
>  static const enum rte_flow_action_type valid_actions[] = {
>  	RTE_FLOW_ACTION_TYPE_DROP,
>  	RTE_FLOW_ACTION_TYPE_QUEUE,
> +	RTE_FLOW_ACTION_TYPE_RSS,
>  	RTE_FLOW_ACTION_TYPE_END,
>  };
>  
> @@ -672,6 +673,76 @@ struct rte_flow_drop {
>  			if (!queue || (queue->index > (priv->rxqs_n - 1)))
>  				goto exit_action_not_supported;
>  			action.queue = 1;
> +			action.queues_n = 1;
> +			action.queues[0] = queue->index;
> +		} else if (actions->type == RTE_FLOW_ACTION_TYPE_RSS) {
> +			int i;
> +			int ierr;
> +			const struct rte_flow_action_rss *rss =
> +				(const struct rte_flow_action_rss *)
> +				actions->conf;
> +
> +			if (!priv->hw_rss) {
> +				rte_flow_error_set(error, ENOTSUP,
> +					   RTE_FLOW_ERROR_TYPE_ACTION,
> +					   actions,
> +					   "RSS cannot be used with "
> +					   "the current configuration");
> +				return -rte_errno;
> +			}
> +			if (!priv->isolated) {
> +				rte_flow_error_set(error, ENOTSUP,
> +					   RTE_FLOW_ERROR_TYPE_ACTION,
> +					   actions,
> +					   "RSS cannot be used without "
> +					   "isolated mode");
> +				return -rte_errno;
> +			}
> +			if (!rte_is_power_of_2(rss->num)) {
> +				rte_flow_error_set(error, ENOTSUP,
> +					   RTE_FLOW_ERROR_TYPE_ACTION,
> +					   actions,
> +					   "the number of queues "
> +					   "should be power of two");
> +				return -rte_errno;
> +			}
> +			if (priv->max_rss_tbl_sz < rss->num) {
> +				rte_flow_error_set(error, ENOTSUP,
> +					   RTE_FLOW_ERROR_TYPE_ACTION,
> +					   actions,
> +					   "the number of queues "
> +					   "is too large");
> +				return -rte_errno;
> +			}
> +			/* checking indexes array */
> +			ierr = 0;
> +			for (i = 0; i < rss->num; ++i) {
> +				int j;
> +				if (rss->queue[i] >= priv->rxqs_n)
> +					ierr = 1;
> +				/*
> +				 * Prevent the user from specifying
> +				 * the same queue twice in the RSS array.
> +				 */
> +				for (j = i + 1; j < rss->num && !ierr; ++j)
> +					if (rss->queue[j] == rss->queue[i])
> +						ierr = 1;
> +				if (ierr) {
> +					rte_flow_error_set(
> +						error,
> +						ENOTSUP,
> +						RTE_FLOW_ERROR_TYPE_HANDLE,
> +						NULL,
> +						"RSS action only supports "
> +						"unique queue indices "
> +						"in a list");
> +					return -rte_errno;
> +				}
> +			}
> +			action.queue = 1;
> +			action.queues_n = rss->num;
> +			for (i = 0; i < rss->num; ++i)
> +				action.queues[i] = rss->queue[i];
>  		} else {
>  			goto exit_action_not_supported;
>  		}
> @@ -797,6 +868,82 @@ struct rte_flow_drop {
>  }
>  
>  /**
> + * Get RSS parent rxq structure for given queues.
> + *
> + * Creates a new or returns an existed one.
> + *
> + * @param priv
> + *   Pointer to private structure.
> + * @param queues
> + *   queues indices array, NULL in default RSS case.
> + * @param children_n
> + *   the size of queues array.
> + *
> + * @return
> + *   Pointer to a parent rxq structure, NULL on failure.
> + */
> +static struct rxq *
> +priv_get_parent(struct priv *priv,

Please use a common prefix for all these functions for consistency, you
should rename it "priv_parent_get()".

> +		uint16_t queues[],
> +		uint16_t children_n,
> +		struct rte_flow_error *error)
> +{
> +	unsigned int i;
> +	struct rxq *parent;
> +
> +	for (parent = LIST_FIRST(&priv->parents);
> +	     parent;
> +	     parent = LIST_NEXT(parent, next)) {
> +		unsigned int same = 0;
> +		unsigned int overlap = 0;
> +
> +		/*
> +		 * Find out whether an appropriate parent queue already exists
> +		 * and can be reused, otherwise make sure there are no overlaps.
> +		 */
> +		for (i = 0; i < children_n; ++i) {
> +			unsigned int j;
> +
> +			for (j = 0; j < parent->rss.queues_n; ++j) {
> +				if (parent->rss.queues[j] != queues[i])
> +					continue;
> +				++overlap;
> +				if (i == j)
> +					++same;
> +			}
> +		}
> +		if (same == children_n &&
> +			children_n == parent->rss.queues_n)
> +			return parent;
> +		else if (overlap)
> +			goto error;
> +	}
> +	/* Exclude the cases when some QPs were created without RSS */
> +	for (i = 0; i < children_n; ++i) {
> +		struct rxq *rxq = (*priv->rxqs)[queues[i]];
> +		if (rxq->qp)
> +			goto error;
> +	}
> +	parent = priv_create_parent(priv, queues, children_n);
> +	if (!parent) {
> +		rte_flow_error_set(error,
> +				   ENOMEM, RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +				   NULL, "flow rule creation failure");
> +		return NULL;
> +	}
> +	return parent;
> +
> +error:
> +	rte_flow_error_set(error,
> +			   EEXIST,
> +			   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +			   NULL,
> +			   "sharing a queue between several"
> +			   " RSS groups is not supported");
> +	return NULL;
> +}
> +
> +/**
>   * Complete flow rule creation.
>   *
>   * @param priv
> @@ -819,6 +966,7 @@ struct rte_flow_drop {
>  {
>  	struct ibv_qp *qp;
>  	struct rte_flow *rte_flow;
> +	struct rxq *rxq_parent = NULL;
>  
>  	assert(priv->pd);
>  	assert(priv->ctx);
> @@ -831,9 +979,39 @@ struct rte_flow_drop {
>  	if (action->drop) {
>  		qp = priv->flow_drop_queue->qp;
>  	} else {
> -		struct rxq *rxq = (*priv->rxqs)[action->queue_id];
> +		int ret;
> +		unsigned int i;
> +		struct rxq *rxq = NULL;
>  
> -		qp = rxq->qp;
> +		if (action->queues_n > 1) {
> +			rxq_parent = priv_get_parent(priv, action->queues,
> +						     action->queues_n, error);
> +			if (!rxq_parent)
> +				goto error;
> +		}
> +		for (i = 0; i < action->queues_n; ++i) {
> +			rxq = (*priv->rxqs)[action->queues[i]];
> +			/*
> +			 * In case of isolated mode we postpone
> +			 * ibv receive queue creation till the first
> +			 * rte_flow rule will be applied on that queue.
> +			 */
> +			if (!rxq->qp) {
> +				assert(priv->isolated);
> +				ret = rxq_create_qp(rxq, rxq->elts_n,
> +						    0, 0, rxq_parent);
> +				if (ret) {
> +					rte_flow_error_set(
> +						error,
> +						ENOMEM,
> +						RTE_FLOW_ERROR_TYPE_HANDLE,
> +						NULL,
> +						"flow rule creation failure");
> +					goto error;
> +				}
> +			}
> +		}
> +		qp = action->queues_n > 1 ? rxq_parent->qp : rxq->qp;
>  		rte_flow->qp = qp;
>  	}
>  	rte_flow->ibv_attr = ibv_attr;
> @@ -846,6 +1024,8 @@ struct rte_flow_drop {
>  	return rte_flow;
>  
>  error:
> +	if (rxq_parent)
> +		rxq_parent_cleanup(rxq_parent);
>  	rte_free(rte_flow);
>  	return NULL;
>  }
> @@ -909,11 +1089,22 @@ struct rte_flow_drop {
>  			continue;
>  		} else if (actions->type == RTE_FLOW_ACTION_TYPE_QUEUE) {
>  			action.queue = 1;
> -			action.queue_id =
> +			action.queues_n = 1;
> +			action.queues[0] =
>  				((const struct rte_flow_action_queue *)
>  				 actions->conf)->index;
>  		} else if (actions->type == RTE_FLOW_ACTION_TYPE_DROP) {
>  			action.drop = 1;
> +		} else if (actions->type == RTE_FLOW_ACTION_TYPE_RSS) {
> +			unsigned int i;
> +			const struct rte_flow_action_rss *rss =
> +				(const struct rte_flow_action_rss *)
> +				 actions->conf;
> +
> +			action.queue = 1;
> +			action.queues_n = rss->num;
> +			for (i = 0; i < rss->num; ++i)
> +				action.queues[i] = rss->queue[i];
>  		} else {
>  			rte_flow_error_set(error, ENOTSUP,
>  					   RTE_FLOW_ERROR_TYPE_ACTION,
> diff --git a/drivers/net/mlx4/mlx4_flow.h b/drivers/net/mlx4/mlx4_flow.h
> index 4d007da..beabcf2 100644
> --- a/drivers/net/mlx4/mlx4_flow.h
> +++ b/drivers/net/mlx4/mlx4_flow.h
> @@ -98,7 +98,8 @@ struct mlx4_flow {
>  struct mlx4_flow_action {
>  	uint32_t drop:1; /**< Target is a drop queue. */
>  	uint32_t queue:1; /**< Target is a receive queue. */
> -	uint32_t queue_id; /**< Identifier of the queue. */
> +	uint16_t queues[RTE_MAX_QUEUES_PER_PORT]; /**< Queue indices to use. */
> +	uint16_t queues_n; /**< Number of entries in queue[] */
>  };
>  
>  int mlx4_priv_flow_start(struct priv *priv);
> -- 
> 1.8.3.1