[dpdk-dev] [PATCH v5 2/4] net/mlx4: implement isolated mode from flow API

[Adrien Mazarguil]

On Wed, Jun 28, 2017 at 05:03:55PM +0300, Vasily Philipov wrote:
> The user must request isolated mode before device configuration,
> the default RSS ring isn't created in this case.
> 
> Signed-off-by: Vasily Philipov <vasilyf at mellanox.com>

Minor nit, please see below.

> ---
>  drivers/net/mlx4/mlx4.c      | 58 +++++++++++++++++++++++++++++++++++---------
>  drivers/net/mlx4/mlx4.h      |  1 +
>  drivers/net/mlx4/mlx4_flow.c | 39 +++++++++++++++++++++++++++++
>  drivers/net/mlx4/mlx4_flow.h |  5 ++++
>  4 files changed, 92 insertions(+), 11 deletions(-)
> 
> diff --git a/drivers/net/mlx4/mlx4.c b/drivers/net/mlx4/mlx4.c
> index 9f3c746..22fa7c6 100644
> --- a/drivers/net/mlx4/mlx4.c
> +++ b/drivers/net/mlx4/mlx4.c
> @@ -650,7 +650,7 @@ void priv_unlock(struct priv *priv)
>  	}
>  	if (rxqs_n == priv->rxqs_n)
>  		return 0;
> -	if (!rte_is_power_of_2(rxqs_n)) {
> +	if (!rte_is_power_of_2(rxqs_n) && !priv->isolated) {
>  		unsigned n_active;
>  
>  		n_active = rte_align32pow2(rxqs_n + 1) >> 1;
> @@ -694,6 +694,8 @@ void priv_unlock(struct priv *priv)
>  	priv->rss = 1;
>  	tmp = priv->rxqs_n;
>  	priv->rxqs_n = rxqs_n;
> +	if (priv->isolated)
> +		return 0;
>  	ret = priv_create_parent(priv, NULL, priv->rxqs_n);
>  	if (!ret)
>  		return 0;
> @@ -2580,6 +2582,7 @@ struct txq_mp2mr_mbuf_check_data {
>  {
>  	unsigned int i;
>  
> +	assert(!priv->isolated);
>  	assert(mac_index < elemof(priv->mac));
>  	if (!BITFIELD_ISSET(priv->mac_configured, mac_index))
>  		return;
> @@ -2829,7 +2832,7 @@ struct txq_mp2mr_mbuf_check_data {
>  						rxq->if_cq,
>  						&params));
>  	}
> -	if (rxq->qp != NULL) {
> +	if (rxq->qp != NULL && !rxq->priv->isolated) {
>  		rxq_promiscuous_disable(rxq);
>  		rxq_allmulticast_disable(rxq);
>  		rxq_mac_addrs_del(rxq);
> @@ -3530,7 +3533,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		return 0;
>  	}
>  	/* Remove attached flows if RSS is disabled (no parent queue). */
> -	if (!priv->rss) {
> +	if (!priv->rss && !priv->isolated) {
>  		rxq_allmulticast_disable(&tmpl);
>  		rxq_promiscuous_disable(&tmpl);
>  		rxq_mac_addrs_del(&tmpl);
> @@ -3575,7 +3578,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		return err;
>  	}
>  	/* Reconfigure flows. Do not care for errors. */
> -	if (!priv->rss) {
> +	if (!priv->rss && !priv->isolated) {
>  		rxq_mac_addrs_add(&tmpl);
>  		if (priv->promisc)
>  			rxq_promiscuous_enable(&tmpl);
> @@ -3734,7 +3737,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		      strerror(ret));
>  		return ret;
>  	}
> -	if (parent || !priv->rss) {
> +	if (!priv->isolated && (parent || !priv->rss)) {
>  		/* Configure MAC and broadcast addresses. */
>  		ret = rxq_mac_addrs_add(rxq);
>  		if (ret) {
> @@ -4013,7 +4016,7 @@ struct txq_mp2mr_mbuf_check_data {
>  			return -ENOMEM;
>  		}
>  	}
> -	if (priv->rss) {
> +	if (priv->rss && !priv->isolated) {
>  		/* The list consists of the single default one. */
>  		parent = LIST_FIRST(&priv->parents);
>  		if (idx >= rte_align32pow2(priv->rxqs_n + 1) >> 1)
> @@ -4109,7 +4112,10 @@ struct txq_mp2mr_mbuf_check_data {
>  	}
>  	DEBUG("%p: attaching configured flows to all RX queues", (void *)dev);
>  	priv->started = 1;
> -	if (priv->rss) {
> +	if (priv->isolated) {
> +		rxq = NULL;
> +		r = 1;
> +	} else if (priv->rss) {
>  		rxq = LIST_FIRST(&priv->parents);
>  		r = 1;
>  	} else {
> @@ -4192,7 +4198,10 @@ struct txq_mp2mr_mbuf_check_data {
>  	}
>  	DEBUG("%p: detaching flows from all RX queues", (void *)dev);
>  	priv->started = 0;
> -	if (priv->rss) {
> +	if (priv->isolated) {
> +		rxq = NULL;
> +		r = 1;
> +	} else if (priv->rss) {
>  		rxq = LIST_FIRST(&priv->parents);
>  		r = 1;
>  	} else {
> @@ -4620,6 +4629,8 @@ struct txq_mp2mr_mbuf_check_data {
>  	if (mlx4_is_secondary())
>  		return;
>  	priv_lock(priv);
> +	if (priv->isolated)
> +		goto end;
>  	DEBUG("%p: removing MAC address from index %" PRIu32,
>  	      (void *)dev, index);
>  	/* Last array entry is reserved for broadcast. */
> @@ -4653,6 +4664,12 @@ struct txq_mp2mr_mbuf_check_data {
>  		return -ENOTSUP;
>  	(void)vmdq;
>  	priv_lock(priv);
> +	if (priv->isolated) {
> +		DEBUG("%p: cannot add MAC address, "
> +		      "device is in isolated mode", (void *)dev);
> +		re = EPERM;
> +		goto end;
> +	}
>  	DEBUG("%p: adding MAC address at index %" PRIu32,
>  	      (void *)dev, index);
>  	/* Last array entry is reserved for broadcast. */
> @@ -4700,6 +4717,12 @@ struct txq_mp2mr_mbuf_check_data {
>  	if (mlx4_is_secondary())
>  		return;
>  	priv_lock(priv);
> +	if (priv->isolated) {
> +		DEBUG("%p: cannot enable promiscuous, "
> +		      "device is in isolated mode", (void *)dev);
> +		priv_unlock(priv);
> +		return;
> +	}
>  	if (priv->promisc) {
>  		priv_unlock(priv);
>  		return;
> @@ -4748,7 +4771,7 @@ struct txq_mp2mr_mbuf_check_data {
>  	if (mlx4_is_secondary())
>  		return;
>  	priv_lock(priv);
> -	if (!priv->promisc) {
> +	if (!priv->promisc || priv->isolated) {
>  		priv_unlock(priv);
>  		return;
>  	}
> @@ -4780,6 +4803,12 @@ struct txq_mp2mr_mbuf_check_data {
>  	if (mlx4_is_secondary())
>  		return;
>  	priv_lock(priv);
> +	if (priv->isolated) {
> +		DEBUG("%p: cannot enable allmulticast, "
> +		      "device is in isolated mode", (void *)dev);
> +		priv_unlock(priv);
> +		return;
> +	}
>  	if (priv->allmulti) {
>  		priv_unlock(priv);
>  		return;
> @@ -4832,7 +4861,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		priv_unlock(priv);
>  		return;
>  	}
> -	if (priv->rss) {
> +	if (priv->rss && !priv->isolated) {
>  		rxq_allmulticast_disable(LIST_FIRST(&priv->parents));
>  		goto end;
>  	}
> @@ -4971,7 +5000,7 @@ struct txq_mp2mr_mbuf_check_data {
>  		}
>  		/* Reenable non-RSS queue attributes. No need to check
>  		 * for errors at this stage. */
> -		if (!priv->rss) {
> +		if (!priv->rss && !priv->isolated) {
>  			rxq_mac_addrs_add(rxq);
>  			if (priv->promisc)
>  				rxq_promiscuous_enable(rxq);
> @@ -5206,6 +5235,12 @@ struct txq_mp2mr_mbuf_check_data {
>  	if (mlx4_is_secondary())
>  		return -E_RTE_SECONDARY;
>  	priv_lock(priv);
> +	if (priv->isolated) {
> +		DEBUG("%p: cannot set vlan filter, "
> +		      "device is in isolated mode", (void *)dev);
> +		priv_unlock(priv);
> +		return -EINVAL;
> +	}
>  	ret = vlan_filter_set(dev, vlan_id, on);
>  	priv_unlock(priv);
>  	assert(ret >= 0);
> @@ -5218,6 +5253,7 @@ struct txq_mp2mr_mbuf_check_data {
>  	.destroy = mlx4_flow_destroy,
>  	.flush = mlx4_flow_flush,
>  	.query = NULL,
> +	.isolate = mlx4_flow_isolate,
>  };
>  
>  /**
> diff --git a/drivers/net/mlx4/mlx4.h b/drivers/net/mlx4/mlx4.h
> index fd24888..b5fe1b4 100644
> --- a/drivers/net/mlx4/mlx4.h
> +++ b/drivers/net/mlx4/mlx4.h
> @@ -339,6 +339,7 @@ struct priv {
>  	unsigned int rss:1; /* RSS is enabled. */
>  	unsigned int vf:1; /* This is a VF device. */
>  	unsigned int pending_alarm:1; /* An alarm is pending. */
> +	unsigned int isolated:1; /* Toggle isolated mode. */
>  #ifdef INLINE_RECV
>  	unsigned int inl_recv_size; /* Inline recv size */
>  #endif
> diff --git a/drivers/net/mlx4/mlx4_flow.c b/drivers/net/mlx4/mlx4_flow.c
> index edfac03..3fd2716 100644
> --- a/drivers/net/mlx4/mlx4_flow.c
> +++ b/drivers/net/mlx4/mlx4_flow.c
> @@ -957,6 +957,45 @@ struct rte_flow *
>  }
>  
>  /**
> + * @see rte_flow_isolate()
> + *
> + * Must be done before calling dev_configure().
> + *
> + * RSS action is possible only if this mode was requested.

This shouldn't be documented here but in the code that parses the RSS
action, where it's actually the case thanks to the associated check and
error message "RSS cannot be used without isolated mode".

You should just remove this line.

> + *
> + * @param dev
> + *   Pointer to the ethernet device structure.
> + * @param enable
> + *   Nonzero to enter isolated mode, attempt to leave it otherwise.
> + * @param[out] error
> + *   Perform verbose error reporting if not NULL. PMDs initialize this
> + *   structure in case of error only.
> + *
> + * @return
> + *   0 on success, a negative value on error.
> + */
> +int
> +mlx4_flow_isolate(struct rte_eth_dev *dev,
> +		  int enable,
> +		  struct rte_flow_error *error)
> +{
> +	struct priv *priv = dev->data->dev_private;
> +
> +	priv_lock(priv);
> +	if (priv->rxqs) {
> +		rte_flow_error_set(error, ENOTSUP,
> +				   RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
> +				   NULL, "isolated mode must be set"
> +				   " before configuring the device");
> +		priv_unlock(priv);
> +		return -rte_errno;
> +	}
> +	priv->isolated = !!enable;
> +	priv_unlock(priv);
> +	return 0;
> +}
> +
> +/**
>   * Destroy a flow.
>   *
>   * @param priv
> diff --git a/drivers/net/mlx4/mlx4_flow.h b/drivers/net/mlx4/mlx4_flow.h
> index 12a293e..4d007da 100644
> --- a/drivers/net/mlx4/mlx4_flow.h
> +++ b/drivers/net/mlx4/mlx4_flow.h
> @@ -90,6 +90,11 @@ struct mlx4_flow {
>  	unsigned int offset; /**< Offset in bytes in the ibv_attr buffer. */
>  };
>  
> +int
> +mlx4_flow_isolate(struct rte_eth_dev *dev,
> +		  int enable,
> +		  struct rte_flow_error *error);
> +
>  struct mlx4_flow_action {
>  	uint32_t drop:1; /**< Target is a drop queue. */
>  	uint32_t queue:1; /**< Target is a receive queue. */
> -- 
> 1.8.3.1
> 

-- 
Adrien Mazarguil
6WIND