From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr0-f181.google.com (mail-wr0-f181.google.com [209.85.128.181]) by dpdk.org (Postfix) with ESMTP id BC28237A0 for ; Thu, 29 Jun 2017 18:53:09 +0200 (CEST) Received: by mail-wr0-f181.google.com with SMTP id 77so192606560wrb.1 for ; Thu, 29 Jun 2017 09:53:09 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=89v/g4H4fw972Dr7NhuDLnVfUaKYDCGnWl0sxg4gjNI=; b=YRbT3vs5bNdz/N26JCWCaiyM6UMBiMWMjeUUnLSjWbC8cGuUTd5NTDrzvXenUDBQEs dpiZ0MscmdCE7wjLMt80MMzTcEIgJBNPJwjaYMFvYFJJRxZ71AfHLsILjFHg6jPzZPp4 ZU1/qHa1xAcY7bFXUF3LfyYaGqPgErypTPhDbmCm8cDdEu9av9kCOHmjBI04jWvSiWH6 dv3anN76YL+iKuO6ps0Or3ZSa6dxVVt9B/gOkfd2/ErDwEqf65kyxgbeugmlkoM6X4bC 2ujBIWyMewzKB2zGiuG/x9Cu44fP92a3/Vc6xYENOJ+sJpRlxu5taz2KVIk5uCfNN8TT KVRA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=89v/g4H4fw972Dr7NhuDLnVfUaKYDCGnWl0sxg4gjNI=; b=fwXRIzaQNI90hv32dy8wR4iHEwR4TMSrhArjgeJxRQc6zMgqFUKX9dD95enVztVJUj KzyJKRyQwL9XHpUMTfwNdtV1xf2FGdZyw3wt+KVzhLeqlCqufnU557IIWZyETZ6f6yR6 l4MmrtqAzZYnRVduqpRMiv/26UqyMhY8n9tP5GLrz/M0B4ps8rgbPlDM+ubqjXQbP82B CkL2dNxT2lrd1Ry2Qg7qVvjuLzMOVORnoSbvNW132vruQDP6niwTiS15Fq1ITbgAHmHY hyFD1JP6uWdClnU63NuH7sR4QBdrbyJuAnDULcRe3cxlbYi0RfSnN8Ts5kygwHL2Hgwg xZWw== X-Gm-Message-State: AKS2vOwHC1gFZ3suxtPlGNoeO0yZ2eZw4rp2avXelxNZbMuvCrwOPRr/ G6cfrONEeD/R/FKC X-Received: by 10.223.176.218 with SMTP id j26mr23959458wra.192.1498755189268; Thu, 29 Jun 2017 09:53:09 -0700 (PDT) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id t75sm5976915wmd.10.2017.06.29.09.53.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 29 Jun 2017 09:53:08 -0700 (PDT) Date: Thu, 29 Jun 2017 18:53:01 +0200 From: Adrien Mazarguil To: Vasily Philipov Cc: dev@dpdk.org, Nelio Laranjeiro Message-ID: <20170629165301.GQ19852@6wind.com> References: <0dca86aa1372d6ff09d0aff01d522c580e0e24ab.1495717153.git.vasilyf@mellanox.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: Subject: Re: [dpdk-dev] [PATCH v5 3/4] net/mlx4: support for the RSS flow action X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 29 Jun 2017 16:53:10 -0000 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 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