Re: [dpdk-dev] [PATCH] net/mlx5: make FDB default rule optional

[Slava Ovsiienko <viacheslavo@mellanox.com>]

> -----Original Message-----
> From: Matan Azrad <matan@mellanox.com>
> Sent: Tuesday, January 28, 2020 19:07
> To: Slava Ovsiienko <viacheslavo@mellanox.com>
> Cc: dev@dpdk.org; stable@dpdk.org
> Subject: [PATCH] net/mlx5: make FDB default rule optional
> 
> There are RDMA-CORE versions which are not supported multi-table for some
> Mellanox mlx5 devices.
> 
> Hence, the optimization added in commit [1] which forwards all the FDB
> traffic to table 1 cannot be configured.
> 
> Make the above optimization optional:
> Do not fail when either table 1 cannot be created or the jump rule (all =>jump
> to table 1) is not configured successfully.
> In this case, all the flows will be configured to table 0.
> 
> [1] commit b67b4ecbde22 ("net/mlx5: skip table zero to improve insertion
> rate")
> 
> Cc: stable@dpdk.org
> 
> Signed-off-by: Matan Azrad <matan@mellanox.com>
Acked-by: Viacheslav Ovsiienko <viacheslavo@mellanox.com>

> ---
>  drivers/net/mlx5/mlx5.h         |  1 +
>  drivers/net/mlx5/mlx5_flow.c    |  6 ++++--
>  drivers/net/mlx5/mlx5_flow.h    |  4 ++--
>  drivers/net/mlx5/mlx5_flow_dv.c | 11 ++++++-----
> drivers/net/mlx5/mlx5_trigger.c | 11 ++++++++---
>  5 files changed, 21 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> 5818349..1fc2063 100644
> --- a/drivers/net/mlx5/mlx5.h
> +++ b/drivers/net/mlx5/mlx5.h
> @@ -792,6 +792,7 @@ struct mlx5_priv {
>  	/* UAR same-page access control required in 32bit implementations.
> */  #endif
>  	uint8_t skip_default_rss_reta; /* Skip configuration of default reta. */
> +	uint8_t fdb_def_rule; /* Whether fdb jump to table 1 is configured. */
>  };
> 
>  #define PORT_ID(priv) ((priv)->dev_data->port_id) diff --git
> a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index
> 5c9fea6..ffaf8a2 100644
> --- a/drivers/net/mlx5/mlx5_flow.c
> +++ b/drivers/net/mlx5/mlx5_flow.c
> @@ -5575,6 +5575,8 @@ struct mlx5_flow_counter *
>   *   Value is part of flow rule created by request external to PMD.
>   * @param[in] group
>   *   rte_flow group index value.
> + * @param[out] fdb_def_rule
> + *   Whether fdb jump to table 1 is configured.
>   * @param[out] table
>   *   HW table value.
>   * @param[out] error
> @@ -5585,10 +5587,10 @@ struct mlx5_flow_counter *
>   */
>  int
>  mlx5_flow_group_to_table(const struct rte_flow_attr *attributes, bool
> external,
> -			 uint32_t group, uint32_t *table,
> +			 uint32_t group, bool fdb_def_rule, uint32_t *table,
>  			 struct rte_flow_error *error)
>  {
> -	if (attributes->transfer && external) {
> +	if (attributes->transfer && external && fdb_def_rule) {
>  		if (group == UINT32_MAX)
>  			return rte_flow_error_set
>  						(error, EINVAL,
> diff --git a/drivers/net/mlx5/mlx5_flow.h b/drivers/net/mlx5/mlx5_flow.h
> index 39be5ba..82b4292 100644
> --- a/drivers/net/mlx5/mlx5_flow.h
> +++ b/drivers/net/mlx5/mlx5_flow.h
> @@ -753,8 +753,8 @@ struct mlx5_flow_driver_ops {  uint32_t
> mlx5_flow_id_release(struct mlx5_flow_id_pool *pool,
>  			      uint32_t id);
>  int mlx5_flow_group_to_table(const struct rte_flow_attr *attributes,
> -			     bool external, uint32_t group, uint32_t *table,
> -			     struct rte_flow_error *error);
> +			     bool external, uint32_t group, bool fdb_def_rule,
> +			     uint32_t *table, struct rte_flow_error *error);
>  uint64_t mlx5_flow_hashfields_adjust(struct mlx5_flow *dev_flow, int
> tunnel,
>  				     uint64_t layer_types,
>  				     uint64_t hash_fields);
> diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> b/drivers/net/mlx5/mlx5_flow_dv.c index b90734e..d57d360 100644
> --- a/drivers/net/mlx5/mlx5_flow_dv.c
> +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> @@ -3357,7 +3357,7 @@ struct field_modify_info modify_tcp[] = {
>  	target_group =
>  		((const struct rte_flow_action_jump *)action->conf)->group;
>  	ret = mlx5_flow_group_to_table(attributes, external, target_group,
> -				       &table, error);
> +				       true, &table, error);
>  	if (ret)
>  		return ret;
>  	if (attributes->group == target_group) @@ -4334,7 +4334,7 @@ struct
> field_modify_info modify_tcp[] = {
>  	int ret;
> 
>  	ret = mlx5_flow_group_to_table(attributes, external,
> -				       attributes->group,
> +				       attributes->group, !!priv->fdb_def_rule,
>  				       &table, error);
>  	if (ret)
>  		return ret;
> @@ -7011,7 +7011,7 @@ struct field_modify_info modify_tcp[] = {
>  	mhdr_res->ft_type = attr->egress ?
> MLX5DV_FLOW_TABLE_TYPE_NIC_TX :
> 
> MLX5DV_FLOW_TABLE_TYPE_NIC_RX;
>  	ret = mlx5_flow_group_to_table(attr, dev_flow->external, attr->group,
> -				       &table, error);
> +				       !!priv->fdb_def_rule, &table, error);
>  	if (ret)
>  		return ret;
>  	dev_flow->group = table;
> @@ -7279,8 +7279,9 @@ struct field_modify_info modify_tcp[] = {
>  		case RTE_FLOW_ACTION_TYPE_JUMP:
>  			jump_data = action->conf;
>  			ret = mlx5_flow_group_to_table(attr, dev_flow-
> >external,
> -						       jump_data->group,
> &table,
> -						       error);
> +						       jump_data->group,
> +						       !!priv->fdb_def_rule,
> +						       &table, error);
>  			if (ret)
>  				return ret;
>  			tbl = flow_dv_tbl_resource_get(dev, table, diff --git
> a/drivers/net/mlx5/mlx5_trigger.c b/drivers/net/mlx5/mlx5_trigger.c index
> ab6937a..7e12cd5 100644
> --- a/drivers/net/mlx5/mlx5_trigger.c
> +++ b/drivers/net/mlx5/mlx5_trigger.c
> @@ -423,9 +423,14 @@
>  		}
>  		mlx5_txq_release(dev, i);
>  	}
> -	if (priv->config.dv_esw_en && !priv->config.vf)
> -		if (!mlx5_flow_create_esw_table_zero_flow(dev))
> -			goto error;
> +	if (priv->config.dv_esw_en && !priv->config.vf) {
> +		if (mlx5_flow_create_esw_table_zero_flow(dev))
> +			priv->fdb_def_rule = 1;
> +		else
> +			DRV_LOG(INFO, "port %u FDB default rule cannot be"
> +				" configured - only Eswitch group 0 flows are"
> +				" supported.", dev->data->port_id);
> +	}
>  	if (priv->isolated)
>  		return 0;
>  	if (dev->data->promiscuous) {
> --
> 1.8.3.1