From: Adrien Mazarguil <adrien.mazarguil@6wind.com>
To: Vasily Philipov <vasilyf@mellanox.com>
Cc: dev@dpdk.org, Nelio Laranjeiro <nelio.laranjeiro@6wind.com>
Subject: Re: [dpdk-dev] [PATCH v5 3/4] net/mlx4: support for the RSS flow action
Date: Thu, 29 Jun 2017 18:53:01 +0200 [thread overview]
Message-ID: <20170629165301.GQ19852@6wind.com> (raw)
In-Reply-To: <baae15c251341c379eadbcc0997f05c5d377af26.1498658407.git.vasilyf@mellanox.com>
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
--
Adrien Mazarguil
6WIND
next prev parent reply other threads:[~2017-06-29 16:53 UTC|newest]
Thread overview: 55+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-05-25 13:02 [dpdk-dev] [PATCH 1/3] net/mlx4: implement isolated mode from flow API Vasily Philipov
2017-05-25 13:02 ` [dpdk-dev] [PATCH 2/3] net/mlx4: support for the RSS flow action Vasily Philipov
2017-05-25 13:02 ` [dpdk-dev] [PATCH 3/3] app/testpmd: add isolated mode parameter Vasily Philipov
2017-05-25 14:05 ` [dpdk-dev] [PATCH v2 1/3] net/mlx4: implement isolated mode from flow API Vasily Philipov
2017-05-25 14:05 ` [dpdk-dev] [PATCH v2 2/3] net/mlx4: support for the RSS flow action Vasily Philipov
2017-05-25 14:05 ` [dpdk-dev] [PATCH v2 3/3] app/testpmd: add isolated mode parameter Vasily Philipov
2017-05-25 14:10 ` [dpdk-dev] [PATCH v3 1/3] net/mlx4: implement isolated mode from flow API Vasily Philipov
2017-05-25 14:10 ` [dpdk-dev] [PATCH v3 2/3] net/mlx4: support for the RSS flow action Vasily Philipov
2017-05-25 14:10 ` [dpdk-dev] [PATCH v3 3/3] app/testpmd: add isolated mode parameter Vasily Philipov
2017-06-04 13:34 ` [dpdk-dev] [PATCH v4 1/4] net/mlx4: RSS parent queues new method maintenance Vasily Philipov
2017-06-04 13:35 ` [dpdk-dev] [PATCH v4 2/4] net/mlx4: implement isolated mode from flow API Vasily Philipov
2017-06-04 13:35 ` [dpdk-dev] [PATCH v4 3/4] net/mlx4: support for the RSS flow action Vasily Philipov
2017-06-04 13:35 ` [dpdk-dev] [PATCH v4 4/4] app/testpmd: add isolated mode parameter Vasily Philipov
2017-06-20 1:26 ` Wu, Jingjing
2017-06-21 9:43 ` Vasily Philipov
2017-06-22 1:13 ` Wu, Jingjing
2017-06-26 5:53 ` Vasily Philipov
2017-06-27 8:28 ` Thomas Monjalon
2017-06-29 5:52 ` Wu, Jingjing
2017-06-28 14:03 ` [dpdk-dev] [PATCH v5 1/4] net/mlx4: RSS parent queues new method maintenance Vasily Philipov
2017-06-29 16:51 ` Adrien Mazarguil
2017-06-28 14:03 ` [dpdk-dev] [PATCH v5 2/4] net/mlx4: implement isolated mode from flow API Vasily Philipov
2017-06-29 16:52 ` Adrien Mazarguil
2017-06-28 14:03 ` [dpdk-dev] [PATCH v5 3/4] net/mlx4: support for the RSS flow action Vasily Philipov
2017-06-29 16:53 ` Adrien Mazarguil [this message]
2017-06-28 14:03 ` [dpdk-dev] [PATCH v5 4/4] app/testpmd: add isolated mode parameter Vasily Philipov
2017-06-29 16:53 ` Adrien Mazarguil
2017-07-02 12:32 ` [dpdk-dev] [PATCH v6 1/4] " Vasily Philipov
2017-07-02 12:32 ` [dpdk-dev] [PATCH v6 2/4] net/mlx4: refactor RSS parent queue allocation Vasily Philipov
2017-07-02 12:32 ` [dpdk-dev] [PATCH v6 3/4] net/mlx4: implement isolated mode from flow API Vasily Philipov
2017-07-02 12:32 ` [dpdk-dev] [PATCH v6 4/4] net/mlx4: support flow API RSS action Vasily Philipov
2017-07-04 11:14 ` [dpdk-dev] [PATCH v7 1/4] app/testpmd: add isolated mode parameter Vasily Philipov
2017-07-04 11:22 ` Vasily Philipov
2017-07-04 15:20 ` Adrien Mazarguil
2017-07-04 11:14 ` [dpdk-dev] [PATCH v7 2/4] net/mlx4: implement isolated mode from flow API Vasily Philipov
2017-07-04 11:22 ` Vasily Philipov
2017-07-04 15:20 ` Adrien Mazarguil
2017-07-04 11:14 ` [dpdk-dev] [PATCH v7 3/4] net/mlx4: refactor RSS parent queue allocation Vasily Philipov
2017-07-04 11:22 ` Vasily Philipov
2017-07-04 15:20 ` Adrien Mazarguil
2017-07-04 11:14 ` [dpdk-dev] [PATCH v7 4/4] net/mlx4: support flow API RSS action Vasily Philipov
2017-07-04 11:22 ` Vasily Philipov
2017-07-04 15:21 ` Adrien Mazarguil
2017-07-05 8:14 ` [dpdk-dev] [PATCH v8 1/4] app/testpmd: add isolated mode parameter Vasily Philipov
2017-07-05 14:49 ` Adrien Mazarguil
2017-07-05 15:18 ` Ferruh Yigit
2017-07-05 15:31 ` Ferruh Yigit
2017-07-06 6:03 ` Vasily Philipov
2017-07-05 15:46 ` Ferruh Yigit
2017-07-05 8:14 ` [dpdk-dev] [PATCH v8 2/4] net/mlx4: implement isolated mode from flow API Vasily Philipov
2017-07-05 14:49 ` Adrien Mazarguil
2017-07-05 8:14 ` [dpdk-dev] [PATCH v8 3/4] net/mlx4: refactor RSS parent queue allocation Vasily Philipov
2017-07-05 14:49 ` Adrien Mazarguil
2017-07-05 8:14 ` [dpdk-dev] [PATCH v8 4/4] net/mlx4: support flow API RSS action Vasily Philipov
2017-07-05 14:49 ` Adrien Mazarguil
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20170629165301.GQ19852@6wind.com \
--to=adrien.mazarguil@6wind.com \
--cc=dev@dpdk.org \
--cc=nelio.laranjeiro@6wind.com \
--cc=vasilyf@mellanox.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
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).