[dpdk-dev] [PATCH v5 1/4] net/mlx4: RSS parent queues new method maintenance

Adrien Mazarguil adrien.mazarguil at 6wind.com
Thu Jun 29 18:51:30 CEST 2017


Hi Vasily,

You should rearrange this series more logically, currently:

- 1/4 adds the ability to have multiple RSS parent QPs in mlx4.
- 2/4 adds rte_flow isolated mode support to mlx4 with the limitation that
      it must be enabled during init.
- 3/4 updates mlx4 to support the rte_flow RSS action with the limitation
      that it only works when isolated mode is also enabled.
- 4/4 adds a testpmd parameter to enable rte_flow isolated mode on all ports
      automatically during init.

Which makes it impossible to validate any of these commits with testpmd
before the last one. How about:

- 1/4 adds a testpmd parameter to enable rte_flow isolated mode on all ports
      automatically during init.
- 2/4 adds rte_flow isolated mode support to mlx4 with the limitation that
      it must be enabled during init.
- 3/4 adds the ability to have multiple RSS parent QPs in mlx4.
- 4/4 updates mlx4 to support the rte_flow RSS action with the limitation
      that it only works when isolated mode is also enabled.

Back to the current commit, please see below.

On Wed, Jun 28, 2017 at 05:03:54PM +0300, Vasily Philipov wrote:
> Insert just created parent queue in a list, keep the list in private
> structure.
> 
> Signed-off-by: Vasily Philipov <vasilyf at mellanox.com>

This log message is not very helpful to people not familiar with the mlx4
PMD, it doesn't explain why it's done (think of the other contributors!)
Here's an alternative suggestion:

 net/mlx4: refactor RSS parent queue allocation

 A special "parent" queue must be allocated in addition to a group of
 standard Rx queues for RSS to work. This is done automatically outside
 of isolated mode by the PMD when applications request several Rx queues.

 Since each configured flow rule with the RSS action may target a different
 set of queues, the PMD must have the ability to dynamically allocate
 several parent queues, one per RSS group.

 Refactor RSS parent queue allocations (currently limited to a single
 parent) in preparation for flow API RSS action support.

> ---
> The series depends on:
> 
> http://dpdk.org/ml/archives/dev/2017-April/064327.html
> http://dpdk.org/dev/patchwork/patch/23741/

You can drop this part from subsequent iterations as it's now applied.

> ---
>  drivers/net/mlx4/mlx4.c | 377 +++++++++++++++++++++++++++++++++---------------
>  drivers/net/mlx4/mlx4.h |  17 ++-
>  2 files changed, 274 insertions(+), 120 deletions(-)
> 
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index ec4419a..9f3c746 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -533,13 +533,94 @@ void priv_unlock(struct priv *priv)
>  
>  static int
>  rxq_setup(struct rte_eth_dev *dev, struct rxq *rxq, uint16_t desc,
> -	  unsigned int socket, int inactive, const struct rte_eth_rxconf *conf,
> -	  struct rte_mempool *mp);
> +	  unsigned int socket, int inactive,
> +	  const struct rte_eth_rxconf *conf,
> +	  struct rte_mempool *mp, int children_n,
> +	  struct rxq *rxq_parent);
>  
>  static void
>  rxq_cleanup(struct rxq *rxq);
>  
>  /**
> + * Create RSS parent queue.
> + *
> + * The new created strucutre will be on the head of priv parents list.

strucutre => structure

How about:

 The new parent is inserted in front of the list in the private
 structure.

> + *
> + * @param priv
> + *   Pointer to private structure.
> + * @param queues
> + *   queues indices array, if NULL use all Rx queues.

queues => Queues

(being pedantic and all)

> + * @param children_n
> + *   The number of entries in queues[].
> + *
> + * @return
> + *   0 on success, negative errno value on failure.
> + */
> +static int
> +priv_create_parent(struct priv *priv,
> +		   uint16_t queues[],
> +		   uint16_t children_n)

Please rename this function "priv_parent_create()" for consistency.

> +{
> +	int ret;
> +	uint16_t i;
> +	struct rxq *parent;
> +
> +	parent = rte_zmalloc("parent queue",
> +			     sizeof(*parent),
> +			     RTE_CACHE_LINE_SIZE);
> +	if (!parent)
> +		return -ENOMEM;
> +	ret = rxq_setup(priv->dev, parent, 0, 0, 0,
> +			NULL, NULL, children_n, NULL);
> +	if (ret) {
> +		rte_free(parent);
> +		return -ret;
> +	}
> +	parent->rss.queues_n = children_n;
> +	if (queues) {
> +		for (i = 0; i < children_n; ++i)
> +			parent->rss.queues[i] = queues[i];
> +	} else {
> +		/* the default RSS ring case */
> +		assert(priv->rxqs_n == children_n);
> +		for (i = 0; i < priv->rxqs_n; ++i)
> +			parent->rss.queues[i] = i;
> +	}
> +	LIST_INSERT_HEAD(&priv->parents, parent, next);
> +	return 0;
> +}

You should make this function return the new parent directly.

> +
> +/**
> + * Cleanup RX queue parent structure.

Cleanup => Clean up

> + *
> + * @param parexnt

parexnt => parent

> + *   RX queue parent structure.
> + */
> +void
> +rxq_parent_cleanup(struct rxq *parent)
> +{
> +	LIST_REMOVE(parent, next);
> +	rxq_cleanup(parent);
> +	rte_free(parent);
> +}
> +
> +/**
> + * Clean up parent structures from the parents list.
> + *
> + * @param priv
> + *   Pointer to private structure.
> + */
> +static void
> +priv_parents_list_cleanup(struct priv *priv)

Please keep it under the same name space:

 priv_parent_list_cleanup()

> +{
> +	while (!LIST_EMPTY(&priv->parents)) {
> +		struct rxq *parent = LIST_FIRST(&priv->parents);
> +
> +		rxq_parent_cleanup(parent);

Relatively minor, this could be shortened as:

 rxq_parent_cleanup(LIST_FIRST(priv->parents));

> +	}
> +}
> +
> +/**
>   * Ethernet device configuration.
>   *
>   * Prepare the driver for a given number of TX and RX queues.
> @@ -588,7 +669,7 @@ void priv_unlock(struct priv *priv)
>  		for (i = 0; (i != priv->rxqs_n); ++i)
>  			if ((*priv->rxqs)[i] != NULL)
>  				return EINVAL;
> -		rxq_cleanup(&priv->rxq_parent);
> +		priv_parents_list_cleanup(priv);
>  		priv->rss = 0;
>  		priv->rxqs_n = 0;
>  	}
> @@ -613,7 +694,7 @@ void priv_unlock(struct priv *priv)
>  	priv->rss = 1;
>  	tmp = priv->rxqs_n;
>  	priv->rxqs_n = rxqs_n;
> -	ret = rxq_setup(dev, &priv->rxq_parent, 0, 0, 0, NULL, NULL);
> +	ret = priv_create_parent(priv, NULL, priv->rxqs_n);
>  	if (!ret)
>  		return 0;
>  	/* Failure, rollback. */
> @@ -2503,7 +2584,7 @@ struct txq_mp2mr_mbuf_check_data {
>  	if (!BITFIELD_ISSET(priv->mac_configured, mac_index))
>  		return;
>  	if (priv->rss) {
> -		rxq_mac_addr_del(&priv->rxq_parent, mac_index);
> +		rxq_mac_addr_del(LIST_FIRST(&priv->parents), mac_index);
>  		goto end;
>  	}
>  	for (i = 0; (i != priv->dev->data->nb_rx_queues); ++i)
> @@ -2570,7 +2651,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		goto end;
>  	}
>  	if (priv->rss) {
> -		ret = rxq_mac_addr_add(&priv->rxq_parent, mac_index);
> +		ret = rxq_mac_addr_add(LIST_FIRST(&priv->parents), mac_index);
>  		if (ret)
>  			return ret;
>  		goto end;
> @@ -2752,8 +2833,9 @@ struct txq_mp2mr_mbuf_check_data {
>  		rxq_promiscuous_disable(rxq);
>  		rxq_allmulticast_disable(rxq);
>  		rxq_mac_addrs_del(rxq);
> -		claim_zero(ibv_destroy_qp(rxq->qp));
>  	}
> +	if (rxq->qp != NULL)
> +		claim_zero(ibv_destroy_qp(rxq->qp));
>  	if (rxq->cq != NULL)
>  		claim_zero(ibv_destroy_cq(rxq->cq));
>  	if (rxq->rd != NULL) {
> @@ -3330,15 +3412,18 @@ struct txq_mp2mr_mbuf_check_data {
>   *   Completion queue to associate with QP.
>   * @param desc
>   *   Number of descriptors in QP (hint only).
> - * @param parent
> - *   If nonzero, create a parent QP, otherwise a child.
> + * @param children_n
> + *   If nonzero, a number of children for parent QP and zero for a child.
> + * @param rxq_parent
> + *   Pointer for a parent in a child case, NULL otherwise.
>   *
>   * @return
>   *   QP pointer or NULL in case of error.
>   */
>  static struct ibv_qp *
>  rxq_setup_qp_rss(struct priv *priv, struct ibv_cq *cq, uint16_t desc,
> -		 int parent, struct ibv_exp_res_domain *rd)
> +		 int children_n, struct ibv_exp_res_domain *rd,
> +		 struct rxq *rxq_parent)
>  {
>  	struct ibv_exp_qp_init_attr attr = {
>  		/* CQ to be associated with the send queue. */
> @@ -3368,16 +3453,16 @@ struct txq_mp2mr_mbuf_check_data {
>  	attr.max_inl_recv = priv->inl_recv_size,
>  	attr.comp_mask |= IBV_EXP_QP_INIT_ATTR_INL_RECV;
>  #endif
> -	if (parent) {
> +	if (children_n > 0) {
>  		attr.qpg.qpg_type = IBV_EXP_QPG_PARENT;
>  		/* TSS isn't necessary. */
>  		attr.qpg.parent_attrib.tss_child_count = 0;
>  		attr.qpg.parent_attrib.rss_child_count =
> -			rte_align32pow2(priv->rxqs_n + 1) >> 1;
> +			rte_align32pow2(children_n + 1) >> 1;
>  		DEBUG("initializing parent RSS queue");
>  	} else {
>  		attr.qpg.qpg_type = IBV_EXP_QPG_CHILD_RX;
> -		attr.qpg.qpg_parent = priv->rxq_parent.qp;
> +		attr.qpg.qpg_parent = rxq_parent->qp;
>  		DEBUG("initializing child RSS queue");
>  	}
>  	return ibv_exp_create_qp(priv->ctx, &attr);
> @@ -3413,13 +3498,7 @@ struct txq_mp2mr_mbuf_check_data {
>  	struct ibv_recv_wr *bad_wr;
>  	unsigned int mb_len;
>  	int err;
> -	int parent = (rxq == &priv->rxq_parent);
>  
> -	if (parent) {
> -		ERROR("%p: cannot rehash parent queue %p",
> -		      (void *)dev, (void *)rxq);
> -		return EINVAL;
> -	}
>  	mb_len = rte_pktmbuf_data_room_size(rxq->mp);
>  	DEBUG("%p: rehashing queue %p", (void *)dev, (void *)rxq);
>  	/* Number of descriptors and mbufs currently allocated. */
> @@ -3464,6 +3543,8 @@ struct txq_mp2mr_mbuf_check_data {
>  	}
>  	/* From now on, any failure will render the queue unusable.
>  	 * Reinitialize QP. */
> +	if (!tmpl.qp)
> +		goto skip_init;
>  	mod = (struct ibv_exp_qp_attr){ .qp_state = IBV_QPS_RESET };
>  	err = ibv_exp_modify_qp(tmpl.qp, &mod, IBV_EXP_QP_STATE);
>  	if (err) {
> @@ -3471,12 +3552,6 @@ struct txq_mp2mr_mbuf_check_data {
>  		assert(err > 0);
>  		return err;
>  	}
> -	err = ibv_resize_cq(tmpl.cq, desc_n);
> -	if (err) {
> -		ERROR("%p: cannot resize CQ: %s", (void *)dev, strerror(err));
> -		assert(err > 0);
> -		return err;
> -	}
>  	mod = (struct ibv_exp_qp_attr){
>  		/* Move the QP to this state. */
>  		.qp_state = IBV_QPS_INIT,
> @@ -3485,9 +3560,6 @@ struct txq_mp2mr_mbuf_check_data {
>  	};
>  	err = ibv_exp_modify_qp(tmpl.qp, &mod,
>  				(IBV_EXP_QP_STATE |
> -#ifdef RSS_SUPPORT
> -				 (parent ? IBV_EXP_QP_GROUP_RSS : 0) |
> -#endif /* RSS_SUPPORT */
>  				 IBV_EXP_QP_PORT));
>  	if (err) {
>  		ERROR("%p: QP state to IBV_QPS_INIT failed: %s",
> @@ -3495,6 +3567,13 @@ struct txq_mp2mr_mbuf_check_data {
>  		assert(err > 0);
>  		return err;
>  	};
> +skip_init:
> +	err = ibv_resize_cq(tmpl.cq, desc_n);
> +	if (err) {
> +		ERROR("%p: cannot resize CQ: %s", (void *)dev, strerror(err));
> +		assert(err > 0);
> +		return err;
> +	}
>  	/* Reconfigure flows. Do not care for errors. */
>  	if (!priv->rss) {
>  		rxq_mac_addrs_add(&tmpl);
> @@ -3562,6 +3641,8 @@ struct txq_mp2mr_mbuf_check_data {
>  	rxq->elts_n = 0;
>  	rte_free(rxq->elts.sp);
>  	rxq->elts.sp = NULL;
> +	if (!tmpl.qp)
> +		goto skip_rtr;
>  	/* Post WRs. */
>  	err = ibv_post_recv(tmpl.qp,
>  			    (tmpl.sp ?
> @@ -3589,6 +3670,116 @@ struct txq_mp2mr_mbuf_check_data {
>  }
>  
>  /**
> + * Create verbs QP resources associated with a rxq.
> + *
> + * @param rxq
> + *   Pointer to RX queue structure.
> + * @param desc
> + *   Number of descriptors to configure in queue.
> + * @param inactive
> + *   If true, the queue is disabled because its index is higher or
> + *   equal to the real number of queues, which must be a power of 2.
> + * @param children_n
> + *   The number of children in a parent case, zero for a child.
> + * @param rxq_parent
> + *   The pointer to a parent RX structure for a child in RSS case,
> + *   NULL for parent.
> + *
> + * @return
> + *   0 on success, errno value on failure.
> + */
> +int
> +rxq_create_qp(struct rxq *rxq,
> +	      uint16_t desc,
> +	      int inactive,
> +	      int children_n,
> +	      struct rxq *rxq_parent)
> +{
> +	int ret;
> +	struct ibv_exp_qp_attr mod;
> +	struct ibv_exp_query_intf_params params;
> +	enum ibv_exp_query_intf_status status;
> +	struct ibv_recv_wr *bad_wr;
> +	int parent = (children_n > 0);
> +	struct priv *priv = rxq->priv;
> +
> +#ifdef RSS_SUPPORT
> +	if (priv->rss && !inactive && (rxq_parent || parent))
> +		rxq->qp = rxq_setup_qp_rss(priv, rxq->cq, desc,
> +					   children_n, rxq->rd,
> +					   rxq_parent);
> +	else
> +#endif /* RSS_SUPPORT */
> +		rxq->qp = rxq_setup_qp(priv, rxq->cq, desc, rxq->rd);
> +	if (rxq->qp == NULL) {
> +		ret = (errno ? errno : EINVAL);
> +		ERROR("QP creation failure: %s",
> +		      strerror(ret));
> +		return ret;
> +	}
> +	mod = (struct ibv_exp_qp_attr){
> +		/* Move the QP to this state. */
> +		.qp_state = IBV_QPS_INIT,
> +		/* Primary port number. */
> +		.port_num = priv->port
> +	};
> +	ret = ibv_exp_modify_qp(rxq->qp, &mod,
> +				(IBV_EXP_QP_STATE |
> +#ifdef RSS_SUPPORT
> +				 (parent ? IBV_EXP_QP_GROUP_RSS : 0) |
> +#endif /* RSS_SUPPORT */
> +				 IBV_EXP_QP_PORT));
> +	if (ret) {
> +		ERROR("QP state to IBV_QPS_INIT failed: %s",
> +		      strerror(ret));
> +		return ret;
> +	}
> +	if (parent || !priv->rss) {
> +		/* Configure MAC and broadcast addresses. */
> +		ret = rxq_mac_addrs_add(rxq);
> +		if (ret) {
> +			ERROR("QP flow attachment failed: %s",
> +			      strerror(ret));
> +			return ret;
> +		}
> +	}
> +	if (!parent) {
> +		ret = ibv_post_recv(rxq->qp,
> +				    (rxq->sp ?
> +				     &(*rxq->elts.sp)[0].wr :
> +				     &(*rxq->elts.no_sp)[0].wr),
> +				    &bad_wr);
> +		if (ret) {
> +			ERROR("ibv_post_recv() failed for WR %p: %s",
> +			      (void *)bad_wr,
> +			      strerror(ret));
> +			return ret;
> +		}
> +	}
> +	mod = (struct ibv_exp_qp_attr){
> +		.qp_state = IBV_QPS_RTR
> +	};
> +	ret = ibv_exp_modify_qp(rxq->qp, &mod, IBV_EXP_QP_STATE);
> +	if (ret) {
> +		ERROR("QP state to IBV_QPS_RTR failed: %s",
> +		      strerror(ret));
> +		return ret;
> +	}
> +	params = (struct ibv_exp_query_intf_params){
> +		.intf_scope = IBV_EXP_INTF_GLOBAL,
> +		.intf = IBV_EXP_INTF_QP_BURST,
> +		.obj = rxq->qp,
> +	};
> +	rxq->if_qp = ibv_exp_query_intf(priv->ctx, &params, &status);
> +	if (rxq->if_qp == NULL) {
> +		ERROR("QP interface family query failed with status %d",
> +		      status);
> +		return errno;
> +	}
> +	return 0;
> +}
> +
> +/**
>   * Configure a RX queue.
>   *
>   * @param dev
> @@ -3606,14 +3797,21 @@ struct txq_mp2mr_mbuf_check_data {
>   *   Thresholds parameters.
>   * @param mp
>   *   Memory pool for buffer allocations.
> + * @param children_n
> + *   The number of children in a parent case, zero for a child.
> + * @param rxq_parent
> + *   The pointer to a parent RX structure (or NULL) in a child case,
> + *   NULL for parent.
>   *
>   * @return
>   *   0 on success, errno value on failure.
>   */
>  static int
>  rxq_setup(struct rte_eth_dev *dev, struct rxq *rxq, uint16_t desc,
> -	  unsigned int socket, int inactive, const struct rte_eth_rxconf *conf,
> -	  struct rte_mempool *mp)
> +	  unsigned int socket, int inactive,
> +	  const struct rte_eth_rxconf *conf,
> +	  struct rte_mempool *mp, int children_n,
> +	  struct rxq *rxq_parent)
>  {
>  	struct priv *priv = dev->data->dev_private;
>  	struct rxq tmpl = {
> @@ -3621,17 +3819,15 @@ struct txq_mp2mr_mbuf_check_data {
>  		.mp = mp,
>  		.socket = socket
>  	};
> -	struct ibv_exp_qp_attr mod;
>  	union {
>  		struct ibv_exp_query_intf_params params;
>  		struct ibv_exp_cq_init_attr cq;
>  		struct ibv_exp_res_domain_init_attr rd;
>  	} attr;
>  	enum ibv_exp_query_intf_status status;
> -	struct ibv_recv_wr *bad_wr;
>  	unsigned int mb_len;
>  	int ret = 0;
> -	int parent = (rxq == &priv->rxq_parent);
> +	int parent = (children_n > 0);
>  
>  	(void)conf; /* Thresholds configuration (ignored). */
>  	/*
> @@ -3711,45 +3907,6 @@ struct txq_mp2mr_mbuf_check_data {
>  	      priv->device_attr.max_qp_wr);
>  	DEBUG("priv->device_attr.max_sge is %d",
>  	      priv->device_attr.max_sge);
> -#ifdef RSS_SUPPORT
> -	if (priv->rss && !inactive)
> -		tmpl.qp = rxq_setup_qp_rss(priv, tmpl.cq, desc, parent,
> -					   tmpl.rd);
> -	else
> -#endif /* RSS_SUPPORT */
> -		tmpl.qp = rxq_setup_qp(priv, tmpl.cq, desc, tmpl.rd);
> -	if (tmpl.qp == NULL) {
> -		ret = (errno ? errno : EINVAL);
> -		ERROR("%p: QP creation failure: %s",
> -		      (void *)dev, strerror(ret));
> -		goto error;
> -	}
> -	mod = (struct ibv_exp_qp_attr){
> -		/* Move the QP to this state. */
> -		.qp_state = IBV_QPS_INIT,
> -		/* Primary port number. */
> -		.port_num = priv->port
> -	};
> -	ret = ibv_exp_modify_qp(tmpl.qp, &mod,
> -				(IBV_EXP_QP_STATE |
> -#ifdef RSS_SUPPORT
> -				 (parent ? IBV_EXP_QP_GROUP_RSS : 0) |
> -#endif /* RSS_SUPPORT */
> -				 IBV_EXP_QP_PORT));
> -	if (ret) {
> -		ERROR("%p: QP state to IBV_QPS_INIT failed: %s",
> -		      (void *)dev, strerror(ret));
> -		goto error;
> -	}
> -	if ((parent) || (!priv->rss))  {
> -		/* Configure MAC and broadcast addresses. */
> -		ret = rxq_mac_addrs_add(&tmpl);
> -		if (ret) {
> -			ERROR("%p: QP flow attachment failed: %s",
> -			      (void *)dev, strerror(ret));
> -			goto error;
> -		}
> -	}
>  	/* Allocate descriptors for RX queues, except for the RSS parent. */
>  	if (parent)
>  		goto skip_alloc;
> @@ -3760,29 +3917,14 @@ struct txq_mp2mr_mbuf_check_data {
>  	if (ret) {
>  		ERROR("%p: RXQ allocation failed: %s",
>  		      (void *)dev, strerror(ret));
> -		goto error;
> -	}
> -	ret = ibv_post_recv(tmpl.qp,
> -			    (tmpl.sp ?
> -			     &(*tmpl.elts.sp)[0].wr :
> -			     &(*tmpl.elts.no_sp)[0].wr),
> -			    &bad_wr);
> -	if (ret) {
> -		ERROR("%p: ibv_post_recv() failed for WR %p: %s",
> -		      (void *)dev,
> -		      (void *)bad_wr,
> -		      strerror(ret));
> -		goto error;
> +		return ret;
>  	}
>  skip_alloc:
> -	mod = (struct ibv_exp_qp_attr){
> -		.qp_state = IBV_QPS_RTR
> -	};
> -	ret = ibv_exp_modify_qp(tmpl.qp, &mod, IBV_EXP_QP_STATE);
> -	if (ret) {
> -		ERROR("%p: QP state to IBV_QPS_RTR failed: %s",
> -		      (void *)dev, strerror(ret));
> -		goto error;
> +	if (parent || rxq_parent || !priv->rss) {
> +		ret = rxq_create_qp(&tmpl, desc, inactive,
> +				    children_n, rxq_parent);
> +		if (ret)
> +			goto error;
>  	}
>  	/* Save port ID. */
>  	tmpl.port_id = dev->data->port_id;
> @@ -3794,21 +3936,11 @@ struct txq_mp2mr_mbuf_check_data {
>  	};
>  	tmpl.if_cq = ibv_exp_query_intf(priv->ctx, &attr.params, &status);
>  	if (tmpl.if_cq == NULL) {
> +		ret = EINVAL;
>  		ERROR("%p: CQ interface family query failed with status %d",
>  		      (void *)dev, status);
>  		goto error;
>  	}
> -	attr.params = (struct ibv_exp_query_intf_params){
> -		.intf_scope = IBV_EXP_INTF_GLOBAL,
> -		.intf = IBV_EXP_INTF_QP_BURST,
> -		.obj = tmpl.qp,
> -	};
> -	tmpl.if_qp = ibv_exp_query_intf(priv->ctx, &attr.params, &status);
> -	if (tmpl.if_qp == NULL) {
> -		ERROR("%p: QP interface family query failed with status %d",
> -		      (void *)dev, status);
> -		goto error;
> -	}
>  	/* Clean up rxq in case we're reinitializing it. */
>  	DEBUG("%p: cleaning-up old rxq just in case", (void *)rxq);
>  	rxq_cleanup(rxq);
> @@ -3846,6 +3978,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		    unsigned int socket, const struct rte_eth_rxconf *conf,
>  		    struct rte_mempool *mp)
>  {
> +	struct rxq *parent;
>  	struct priv *priv = dev->data->dev_private;
>  	struct rxq *rxq = (*priv->rxqs)[idx];
>  	int inactive = 0;
> @@ -3880,9 +4013,16 @@ struct txq_mp2mr_mbuf_check_data {
>  			return -ENOMEM;
>  		}
>  	}
> -	if (idx >= rte_align32pow2(priv->rxqs_n + 1) >> 1)
> -		inactive = 1;
> -	ret = rxq_setup(dev, rxq, desc, socket, inactive, conf, mp);
> +	if (priv->rss) {
> +		/* The list consists of the single default one. */
> +		parent = LIST_FIRST(&priv->parents);
> +		if (idx >= rte_align32pow2(priv->rxqs_n + 1) >> 1)
> +			inactive = 1;
> +	} else {
> +		parent = NULL;
> +	}
> +	ret = rxq_setup(dev, rxq, desc, socket,
> +			inactive, conf, mp, 0, parent);
>  	if (ret)
>  		rte_free(rxq);
>  	else {
> @@ -3919,7 +4059,6 @@ struct txq_mp2mr_mbuf_check_data {
>  		return;
>  	priv = rxq->priv;
>  	priv_lock(priv);
> -	assert(rxq != &priv->rxq_parent);
>  	for (i = 0; (i != priv->rxqs_n); ++i)
>  		if ((*priv->rxqs)[i] == rxq) {
>  			DEBUG("%p: removing RX queue %p from list",
> @@ -3971,7 +4110,7 @@ struct txq_mp2mr_mbuf_check_data {
>  	DEBUG("%p: attaching configured flows to all RX queues", (void *)dev);
>  	priv->started = 1;
>  	if (priv->rss) {
> -		rxq = &priv->rxq_parent;
> +		rxq = LIST_FIRST(&priv->parents);
>  		r = 1;
>  	} else {
>  		rxq = (*priv->rxqs)[0];
> @@ -4054,7 +4193,7 @@ struct txq_mp2mr_mbuf_check_data {
>  	DEBUG("%p: detaching flows from all RX queues", (void *)dev);
>  	priv->started = 0;
>  	if (priv->rss) {
> -		rxq = &priv->rxq_parent;
> +		rxq = LIST_FIRST(&priv->parents);
>  		r = 1;
>  	} else {
>  		rxq = (*priv->rxqs)[0];
> @@ -4188,7 +4327,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		priv->txqs = NULL;
>  	}
>  	if (priv->rss)
> -		rxq_cleanup(&priv->rxq_parent);
> +		priv_parents_list_cleanup(priv);
>  	if (priv->pd != NULL) {
>  		assert(priv->ctx != NULL);
>  		claim_zero(ibv_dealloc_pd(priv->pd));
> @@ -4569,7 +4708,7 @@ struct txq_mp2mr_mbuf_check_data {
>  	if (!priv->started)
>  		goto end;
>  	if (priv->rss) {
> -		ret = rxq_promiscuous_enable(&priv->rxq_parent);
> +		ret = rxq_promiscuous_enable(LIST_FIRST(&priv->parents));
>  		if (ret) {
>  			priv_unlock(priv);
>  			return;
> @@ -4614,7 +4753,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		return;
>  	}
>  	if (priv->rss) {
> -		rxq_promiscuous_disable(&priv->rxq_parent);
> +		rxq_promiscuous_disable(LIST_FIRST(&priv->parents));
>  		goto end;
>  	}
>  	for (i = 0; (i != priv->rxqs_n); ++i)
> @@ -4649,7 +4788,7 @@ struct txq_mp2mr_mbuf_check_data {
>  	if (!priv->started)
>  		goto end;
>  	if (priv->rss) {
> -		ret = rxq_allmulticast_enable(&priv->rxq_parent);
> +		ret = rxq_allmulticast_enable(LIST_FIRST(&priv->parents));
>  		if (ret) {
>  			priv_unlock(priv);
>  			return;
> @@ -4694,7 +4833,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		return;
>  	}
>  	if (priv->rss) {
> -		rxq_allmulticast_disable(&priv->rxq_parent);
> +		rxq_allmulticast_disable(LIST_FIRST(&priv->parents));
>  		goto end;
>  	}
>  	for (i = 0; (i != priv->rxqs_n); ++i)
> @@ -5003,7 +5142,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		 * Rehashing flows in all RX queues is necessary.
>  		 */
>  		if (priv->rss)
> -			rxq_mac_addrs_del(&priv->rxq_parent);
> +			rxq_mac_addrs_del(LIST_FIRST(&priv->parents));
>  		else
>  			for (i = 0; (i != priv->rxqs_n); ++i)
>  				if ((*priv->rxqs)[i] != NULL)
> @@ -5011,7 +5150,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		priv->vlan_filter[j].enabled = 1;
>  		if (priv->started) {
>  			if (priv->rss)
> -				rxq_mac_addrs_add(&priv->rxq_parent);
> +				rxq_mac_addrs_add(LIST_FIRST(&priv->parents));
>  			else
>  				for (i = 0; (i != priv->rxqs_n); ++i) {
>  					if ((*priv->rxqs)[i] == NULL)
> @@ -5025,7 +5164,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		 * Rehashing flows in all RX queues is necessary.
>  		 */
>  		if (priv->rss)
> -			rxq_mac_addrs_del(&priv->rxq_parent);
> +			rxq_mac_addrs_del(LIST_FIRST(&priv->parents));
>  		else
>  			for (i = 0; (i != priv->rxqs_n); ++i)
>  				if ((*priv->rxqs)[i] != NULL)
> @@ -5033,7 +5172,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		priv->vlan_filter[j].enabled = 0;
>  		if (priv->started) {
>  			if (priv->rss)
> -				rxq_mac_addrs_add(&priv->rxq_parent);
> +				rxq_mac_addrs_add(LIST_FIRST(&priv->parents));
>  			else
>  				for (i = 0; (i != priv->rxqs_n); ++i) {
>  					if ((*priv->rxqs)[i] == NULL)
> diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
> index 9a3bae9..fd24888 100644
> --- a/drivers/net/mlx4/mlx4.h
> +++ b/drivers/net/mlx4/mlx4.h
> @@ -219,6 +219,7 @@ struct rxq_elt {
>  
>  /* RX queue descriptor. */
>  struct rxq {
> +	LIST_ENTRY(rxq) next; /* Used by parent queue only */
>  	struct priv *priv; /* Back pointer to private data. */
>  	struct rte_mempool *mp; /* Memory Pool for allocations. */
>  	struct ibv_mr *mr; /* Memory Region (for mp). */
> @@ -246,6 +247,10 @@ struct rxq {
>  	struct mlx4_rxq_stats stats; /* RX queue counters. */
>  	unsigned int socket; /* CPU socket ID for allocations. */
>  	struct ibv_exp_res_domain *rd; /* Resource Domain. */
> +	struct {
> +		uint16_t queues_n;
> +		uint16_t queues[RTE_MAX_QUEUES_PER_PORT];
> +	} rss;
>  };
>  
>  /* TX element. */
> @@ -339,7 +344,6 @@ struct priv {
>  #endif
>  	unsigned int max_rss_tbl_sz; /* Maximum number of RSS queues. */
>  	/* RX/TX queues. */
> -	struct rxq rxq_parent; /* Parent queue when RSS is enabled. */
>  	unsigned int rxqs_n; /* RX queues array size. */
>  	unsigned int txqs_n; /* TX queues array size. */
>  	struct rxq *(*rxqs)[]; /* RX queues. */
> @@ -348,10 +352,21 @@ struct priv {
>  	struct rte_flow_drop *flow_drop_queue; /* Flow drop queue. */
>  	LIST_HEAD(mlx4_flows, rte_flow) flows;
>  	struct rte_intr_conf intr_conf; /* Active interrupt configuration. */
> +	LIST_HEAD(mlx4_parents, rxq) parents;
>  	rte_spinlock_t lock; /* Lock for control functions. */
>  };
>  
>  void priv_lock(struct priv *priv);
>  void priv_unlock(struct priv *priv);
>  
> +int
> +rxq_create_qp(struct rxq *rxq,
> +	      uint16_t desc,
> +	      int inactive,
> +	      int children_n,
> +	      struct rxq *rxq_parent);
> +
> +void
> +rxq_parent_cleanup(struct rxq *parent);
> +
>  #endif /* RTE_PMD_MLX4_H_ */
> -- 
> 1.8.3.1
> 

-- 
Adrien Mazarguil
6WIND


More information about the dev mailing list