RE: [PATCH] net/mlx5: support matching optional fields of GRE

["Sean Zhang (Networking SW)" <xiazhang@nvidia.com>]

Hi Raslan,

Rebased and new version sent out.

Thanks,
Sean

> -----Original Message-----
> From: Raslan Darawsheh <rasland@nvidia.com>
> Sent: Thursday, February 24, 2022 9:18 PM
> To: Sean Zhang (Networking SW) <xiazhang@nvidia.com>; NBU-Contact-
> Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>; Matan Azrad
> <matan@nvidia.com>; Slava Ovsiienko <viacheslavo@nvidia.com>
> Cc: dev@dpdk.org
> Subject: RE: [PATCH] net/mlx5: support matching optional fields of GRE
> 
> Hi Sean,
> 
> Can you kindly rebase this patch on top of the latest next-net-mlx/main?
> And send a new version?
> 
> Kindest regards,
> Raslan Darawsheh
> 
> > -----Original Message-----
> > From: Sean Zhang <xiazhang@nvidia.com>
> > Sent: Thursday, February 17, 2022 8:27 AM
> > To: NBU-Contact-Thomas Monjalon (EXTERNAL) <thomas@monjalon.net>;
> > Matan Azrad <matan@nvidia.com>; Slava Ovsiienko
> > <viacheslavo@nvidia.com>
> > Cc: dev@dpdk.org
> > Subject: [PATCH] net/mlx5: support matching optional fields of GRE
> >
> > This patch adds matching on the optional fields
> > (checksum/key/sequence) of GRE header. The matching on checksum and
> > sequence fields requests support from rdma-core with the capability of misc5
> and tunnel_header 0-3.
> >
> > For patterns without checksum and sequence specified, keep using misc
> > for matching as before, but for patterns with checksum or sequence,
> > validate capability first and then use misc5 for the matching.
> >
> > Signed-off-by: Sean Zhang <xiazhang@nvidia.com>
> > Acked-by: Slava Ovsiienko <viacheslavo@nvidia.com>
> >
> > ---
> > v3 - some code style change
> > v2 - replace default mask with nic mask
> > ---
> >  doc/guides/nics/mlx5.rst             |  10 +++
> >  drivers/common/mlx5/mlx5_devx_cmds.c |   3 +
> >  drivers/net/mlx5/linux/mlx5_os.c     |   2 +
> >  drivers/net/mlx5/mlx5.h              |   1 +
> >  drivers/net/mlx5/mlx5_flow.c         | 101
> > +++++++++++++++++++++++++++++
> >  drivers/net/mlx5/mlx5_flow.h         |   6 ++
> >  drivers/net/mlx5/mlx5_flow_dv.c      | 121
> > +++++++++++++++++++++++++++++++++++
> >  7 files changed, 244 insertions(+)
> >
> > diff --git a/doc/guides/nics/mlx5.rst b/doc/guides/nics/mlx5.rst index
> > c3cc0c0..4611f0f 100644
> > --- a/doc/guides/nics/mlx5.rst
> > +++ b/doc/guides/nics/mlx5.rst
> > @@ -511,6 +511,16 @@ Limitations
> >      from the reference "Clock Queue" completions,
> >      the scheduled send timestamps should not be specified with
> > non-zero MSB.
> >
> > +- Match on GRE header supports the following fields:
> > +
> > +  - c_rsvd0_v: C bit, K bit, S bit
> > +  - protocol type
> > +  - Checksum
> > +  - Key
> > +  - Sequence
> > +
> > +  Matching on checksum and sequence needs OFED 5.6+.
> > +
> >  Statistics
> >  ----------
> >
> > diff --git a/drivers/common/mlx5/mlx5_devx_cmds.c
> > b/drivers/common/mlx5/mlx5_devx_cmds.c
> > index 2e807a0..77436ed 100644
> > --- a/drivers/common/mlx5/mlx5_devx_cmds.c
> > +++ b/drivers/common/mlx5/mlx5_devx_cmds.c
> > @@ -1043,6 +1043,9 @@ struct mlx5_devx_obj *
> >  	attr->flow.tunnel_header_0_1 = MLX5_GET
> >  		(flow_table_nic_cap, hcattr,
> >  		 ft_field_support_2_nic_receive.tunnel_header_0_1);
> > +	attr->flow.tunnel_header_2_3 = MLX5_GET
> > +		(flow_table_nic_cap, hcattr,
> > +		 ft_field_support_2_nic_receive.tunnel_header_2_3);
> >  	attr->pkt_integrity_match =
> > mlx5_devx_query_pkt_integrity_match(hcattr);
> >  	attr->inner_ipv4_ihl = MLX5_GET
> >  		(flow_table_nic_cap, hcattr,
> > diff --git a/drivers/net/mlx5/linux/mlx5_os.c
> > b/drivers/net/mlx5/linux/mlx5_os.c
> > index bbe05bb..b8a9dea 100644
> > --- a/drivers/net/mlx5/linux/mlx5_os.c
> > +++ b/drivers/net/mlx5/linux/mlx5_os.c
> > @@ -1383,6 +1383,8 @@
> >  		}
> >  		if (config->hca_attr.flow.tunnel_header_0_1)
> >  			sh->tunnel_header_0_1 = 1;
> > +		if (config->hca_attr.flow.tunnel_header_2_3)
> > +			sh->tunnel_header_2_3 = 1;
> >  #endif
> >  #ifdef HAVE_MLX5_DR_CREATE_ACTION_ASO
> >  		if (config->hca_attr.flow_hit_aso && diff --git
> > a/drivers/net/mlx5/mlx5.h b/drivers/net/mlx5/mlx5.h index
> > 737ad68..5faca7a 100644
> > --- a/drivers/net/mlx5/mlx5.h
> > +++ b/drivers/net/mlx5/mlx5.h
> > @@ -1154,6 +1154,7 @@ struct mlx5_dev_ctx_shared {
> >  	uint32_t meter_aso_en:1; /* Flow Meter ASO is supported. */
> >  	uint32_t ct_aso_en:1; /* Connection Tracking ASO is supported. */
> >  	uint32_t tunnel_header_0_1:1; /* tunnel_header_0_1 is supported.
> > */
> > +	uint32_t tunnel_header_2_3:1; /* tunnel_header_2_3 is supported.
> > */
> >  	uint32_t misc5_cap:1; /* misc5 matcher parameter is supported. */
> >  	uint32_t reclaim_mode:1; /* Reclaim memory. */
> >  	uint32_t dr_drop_action_en:1; /* Use DR drop action. */ diff --git
> > a/drivers/net/mlx5/mlx5_flow.c b/drivers/net/mlx5/mlx5_flow.c index
> > 179cc3b..fc1249d 100644
> > --- a/drivers/net/mlx5/mlx5_flow.c
> > +++ b/drivers/net/mlx5/mlx5_flow.c
> > @@ -2815,6 +2815,107 @@ struct mlx5_flow_tunnel_info {  }
> >
> >  /**
> > + * Validate GRE optional item.
> > + *
> > + * @param[in] dev
> > + *   Pointer to the Ethernet device structure.
> > + * @param[in] item
> > + *   Item specification.
> > + * @param[in] item_flags
> > + *   Bit flags to mark detected items.
> > + * @param[in] attr
> > + *   Flow rule attributes.
> > + * @param[in] gre_item
> > + *   Pointer to gre_item
> > + * @param[out] error
> > + *   Pointer to error structure.
> > + *
> > + * @return
> > + *   0 on success, a negative errno value otherwise and rte_errno is set.
> > + */
> > +int
> > +mlx5_flow_validate_item_gre_option(struct rte_eth_dev *dev,
> > +				   const struct rte_flow_item *item,
> > +				   uint64_t item_flags,
> > +				   const struct rte_flow_attr *attr,
> > +				   const struct rte_flow_item *gre_item,
> > +				   struct rte_flow_error *error)
> > +{
> > +	const struct rte_flow_item_gre *gre_spec = gre_item->spec;
> > +	const struct rte_flow_item_gre *gre_mask = gre_item->mask;
> > +	const struct rte_flow_item_gre_opt *spec = item->spec;
> > +	const struct rte_flow_item_gre_opt *mask = item->mask;
> > +	struct mlx5_priv *priv = dev->data->dev_private;
> > +	int ret = 0;
> > +	struct rte_flow_item_gre_opt nic_mask = {
> > +		.checksum_rsvd = {
> > +			.checksum = RTE_BE16(UINT16_MAX),
> > +			.reserved1 = 0x0,
> > +		},
> > +		.key = {
> > +			.key = RTE_BE32(UINT32_MAX),
> > +		},
> > +		.sequence = {
> > +			.sequence = RTE_BE32(UINT32_MAX),
> > +		},
> > +	};
> > +
> > +	if (!(item_flags & MLX5_FLOW_LAYER_GRE))
> > +		return rte_flow_error_set(error, ENOTSUP,
> > +					  RTE_FLOW_ERROR_TYPE_ITEM,
> > item,
> > +					  "No preceding GRE header");
> > +	if (item_flags & MLX5_FLOW_LAYER_INNER)
> > +		return rte_flow_error_set(error, ENOTSUP,
> > +					  RTE_FLOW_ERROR_TYPE_ITEM,
> > item,
> > +					  "GRE option following a wrong
> > item");
> > +	if (!spec || !mask)
> > +		return rte_flow_error_set(error, EINVAL,
> > +					  RTE_FLOW_ERROR_TYPE_ITEM,
> > item,
> > +					  "At least one field
> > gre_option(checksum/key/sequence) must be specified");
> > +	if (!gre_mask)
> > +		gre_mask = &rte_flow_item_gre_mask;
> > +	if (mask->checksum_rsvd.checksum)
> > +		if (gre_spec && (gre_mask->c_rsvd0_ver &
> > RTE_BE16(0x8000)) &&
> > +				 !(gre_spec->c_rsvd0_ver &
> > RTE_BE16(0x8000)))
> > +			return rte_flow_error_set(error, EINVAL,
> > +
> > RTE_FLOW_ERROR_TYPE_ITEM,
> > +						  item,
> > +						  "Checksum bit must be on");
> > +	if (mask->key.key)
> > +		if (gre_spec && (gre_mask->c_rsvd0_ver &
> > RTE_BE16(0x2000)) &&
> > +				 !(gre_spec->c_rsvd0_ver &
> > RTE_BE16(0x2000)))
> > +			return rte_flow_error_set(error, EINVAL,
> > +
> > RTE_FLOW_ERROR_TYPE_ITEM,
> > +						  item, "Key bit must be on");
> > +	if (mask->sequence.sequence)
> > +		if (gre_spec && (gre_mask->c_rsvd0_ver &
> > RTE_BE16(0x1000)) &&
> > +				 !(gre_spec->c_rsvd0_ver &
> > RTE_BE16(0x1000)))
> > +			return rte_flow_error_set(error, EINVAL,
> > +
> > RTE_FLOW_ERROR_TYPE_ITEM,
> > +						  item,
> > +						  "Sequence bit must be on");
> > +	if (mask->checksum_rsvd.checksum || mask->sequence.sequence)
> > {
> > +		if (priv->sh->steering_format_version ==
> > +		    MLX5_STEERING_LOGIC_FORMAT_CONNECTX_5 ||
> > +		    ((attr->group || attr->transfer) &&
> > +		     !priv->sh->misc5_cap) ||
> > +		    (!(priv->sh->tunnel_header_0_1 &&
> > +		       priv->sh->tunnel_header_2_3) &&
> > +		    !attr->group && !attr->transfer))
> > +			return rte_flow_error_set(error, EINVAL,
> > +
> > RTE_FLOW_ERROR_TYPE_ITEM,
> > +						  item,
> > +						  "Checksum/Sequence not
> > supported");
> > +	}
> > +	ret = mlx5_flow_item_acceptable
> > +		(item, (const uint8_t *)mask,
> > +		 (const uint8_t *)&nic_mask,
> > +		 sizeof(struct rte_flow_item_gre_opt),
> > +		 MLX5_ITEM_RANGE_NOT_ACCEPTED, error);
> > +	return ret;
> > +}
> > +
> > +/**
> >   * Validate GRE item.
> >   *
> >   * @param[in] item
> > diff --git a/drivers/net/mlx5/mlx5_flow.h
> > b/drivers/net/mlx5/mlx5_flow.h index 7fec79a..667b241 100644
> > --- a/drivers/net/mlx5/mlx5_flow.h
> > +++ b/drivers/net/mlx5/mlx5_flow.h
> > @@ -1537,6 +1537,12 @@ int mlx5_flow_validate_item_gre_key(const
> > struct rte_flow_item *item,
> >  				    uint64_t item_flags,
> >  				    const struct rte_flow_item *gre_item,
> >  				    struct rte_flow_error *error);
> > +int mlx5_flow_validate_item_gre_option(struct rte_eth_dev *dev,
> > +				const struct rte_flow_item *item,
> > +				uint64_t item_flags,
> > +				const struct rte_flow_attr *attr,
> > +				const struct rte_flow_item *gre_item,
> > +				struct rte_flow_error *error);
> >  int mlx5_flow_validate_item_ipv4(const struct rte_flow_item *item,
> >  				 uint64_t item_flags,
> >  				 uint64_t last_item,
> > diff --git a/drivers/net/mlx5/mlx5_flow_dv.c
> > b/drivers/net/mlx5/mlx5_flow_dv.c index ef9c66e..ba61a33 100644
> > --- a/drivers/net/mlx5/mlx5_flow_dv.c
> > +++ b/drivers/net/mlx5/mlx5_flow_dv.c
> > @@ -7101,6 +7101,13 @@ struct mlx5_list_entry *
> >  			gre_item = items;
> >  			last_item = MLX5_FLOW_LAYER_GRE;
> >  			break;
> > +		case RTE_FLOW_ITEM_TYPE_GRE_OPTION:
> > +			ret = mlx5_flow_validate_item_gre_option(dev,
> > items, item_flags,
> > +							  attr, gre_item,
> > error);
> > +			if (ret < 0)
> > +				return ret;
> > +			last_item = MLX5_FLOW_LAYER_GRE;
> > +			break;
> >  		case RTE_FLOW_ITEM_TYPE_NVGRE:
> >  			ret = mlx5_flow_validate_item_nvgre(items,
> > item_flags,
> >  							    next_protocol,
> > @@ -8822,6 +8829,110 @@ struct mlx5_list_entry *  }
> >
> >  /**
> > + * Add GRE optional items to matcher and to the value.
> > + *
> > + * @param[in, out] matcher
> > + *   Flow matcher.
> > + * @param[in, out] key
> > + *   Flow matcher value.
> > + * @param[in] item
> > + *   Flow pattern to translate.
> > + * @param[in] gre_item
> > + *   Pointer to gre_item.
> > + * @param[in] pattern_flags
> > + *   Accumulated pattern flags.
> > + */
> > +static void
> > +flow_dv_translate_item_gre_option(void *matcher, void *key,
> > +				  const struct rte_flow_item *item,
> > +				  const struct rte_flow_item *gre_item,
> > +				  uint64_t pattern_flags)
> > +{
> > +	const struct rte_flow_item_gre_opt *option_m = item->mask;
> > +	const struct rte_flow_item_gre_opt *option_v = item->spec;
> > +	const struct rte_flow_item_gre *gre_m = gre_item->mask;
> > +	const struct rte_flow_item_gre *gre_v = gre_item->spec;
> > +	static const struct rte_flow_item_gre empty_gre = {0};
> > +	struct rte_flow_item gre_key_item;
> > +	uint16_t c_rsvd0_ver_m, c_rsvd0_ver_v;
> > +	uint16_t protocol_m, protocol_v;
> > +	void *misc5_m;
> > +	void *misc5_v;
> > +
> > +	/*
> > +	 * If only match key field, keep using misc for matching.
> > +	 * If need to match checksum or sequence, using misc5 and do
> > +	 * not need using misc.
> > +	 */
> > +	if (!(option_m->sequence.sequence ||
> > +	      option_m->checksum_rsvd.checksum)) {
> > +		flow_dv_translate_item_gre(matcher, key, gre_item,
> > +					   pattern_flags);
> > +		gre_key_item.spec = &option_v->key.key;
> > +		gre_key_item.mask = &option_m->key.key;
> > +		flow_dv_translate_item_gre_key(matcher, key,
> > &gre_key_item);
> > +		return;
> > +	}
> > +	if (!gre_v) {
> > +		gre_v = &empty_gre;
> > +		gre_m = &empty_gre;
> > +	} else {
> > +		if (!gre_m)
> > +			gre_m = &rte_flow_item_gre_mask;
> > +	}
> > +	protocol_v = gre_v->protocol;
> > +	protocol_m = gre_m->protocol;
> > +	if (!protocol_m) {
> > +		/* Force next protocol to prevent matchers duplication */
> > +		uint16_t ether_type =
> > +			mlx5_translate_tunnel_etypes(pattern_flags);
> > +		if (ether_type) {
> > +			protocol_v = rte_be_to_cpu_16(ether_type);
> > +			protocol_m = UINT16_MAX;
> > +		}
> > +	}
> > +	c_rsvd0_ver_v = gre_v->c_rsvd0_ver;
> > +	c_rsvd0_ver_m = gre_m->c_rsvd0_ver;
> > +	if (option_m->sequence.sequence) {
> > +		c_rsvd0_ver_v |= RTE_BE16(0x1000);
> > +		c_rsvd0_ver_m |= RTE_BE16(0x1000);
> > +	}
> > +	if (option_m->key.key) {
> > +		c_rsvd0_ver_v |= RTE_BE16(0x2000);
> > +		c_rsvd0_ver_m |= RTE_BE16(0x2000);
> > +	}
> > +	if (option_m->checksum_rsvd.checksum) {
> > +		c_rsvd0_ver_v |= RTE_BE16(0x8000);
> > +		c_rsvd0_ver_m |= RTE_BE16(0x8000);
> > +	}
> > +	/*
> > +	 * Hardware parses GRE optional field into the fixed location,
> > +	 * do not need to adjust the tunnel dword indices.
> > +	 */
> > +	misc5_v = MLX5_ADDR_OF(fte_match_param, key,
> > misc_parameters_5);
> > +	misc5_m = MLX5_ADDR_OF(fte_match_param, matcher,
> > misc_parameters_5);
> > +	MLX5_SET(fte_match_set_misc5, misc5_v, tunnel_header_0,
> > +		 rte_be_to_cpu_32((c_rsvd0_ver_v | protocol_v << 16) &
> > +				  (c_rsvd0_ver_m | protocol_m << 16)));
> > +	MLX5_SET(fte_match_set_misc5, misc5_m, tunnel_header_0,
> > +		 rte_be_to_cpu_32(c_rsvd0_ver_m | protocol_m << 16));
> > +	MLX5_SET(fte_match_set_misc5, misc5_v, tunnel_header_1,
> > +		 rte_be_to_cpu_32(option_v->checksum_rsvd.checksum &
> > +				  option_m->checksum_rsvd.checksum));
> > +	MLX5_SET(fte_match_set_misc5, misc5_m, tunnel_header_1,
> > +		 rte_be_to_cpu_32(option_m->checksum_rsvd.checksum));
> > +	MLX5_SET(fte_match_set_misc5, misc5_v, tunnel_header_2,
> > +		 rte_be_to_cpu_32(option_v->key.key & option_m-
> > >key.key));
> > +	MLX5_SET(fte_match_set_misc5, misc5_m, tunnel_header_2,
> > +		 rte_be_to_cpu_32(option_m->key.key));
> > +	MLX5_SET(fte_match_set_misc5, misc5_v, tunnel_header_3,
> > +		 rte_be_to_cpu_32(option_v->sequence.sequence &
> > +				  option_m->sequence.sequence));
> > +	MLX5_SET(fte_match_set_misc5, misc5_m, tunnel_header_3,
> > +		 rte_be_to_cpu_32(option_m->sequence.sequence));
> > +}
> > +
> > +/**
> >   * Add NVGRE item to matcher and to the value.
> >   *
> >   * @param[in, out] matcher
> > @@ -12697,6 +12808,7 @@ struct mlx5_list_entry *
> >  	};
> >  	const struct rte_flow_item *integrity_items[2] = {NULL, NULL};
> >  	const struct rte_flow_item *tunnel_item = NULL;
> > +	const struct rte_flow_item *gre_item = NULL;
> >
> >  	if (!wks)
> >  		return rte_flow_error_set(error, ENOMEM, @@ -13469,12
> > +13581,18 @@ struct mlx5_list_entry *
> >  			matcher.priority =
> > MLX5_TUNNEL_PRIO_GET(rss_desc);
> >  			last_item = MLX5_FLOW_LAYER_GRE;
> >  			tunnel_item = items;
> > +			gre_item = items;
> >  			break;
> >  		case RTE_FLOW_ITEM_TYPE_GRE_KEY:
> >  			flow_dv_translate_item_gre_key(match_mask,
> >  						       match_value, items);
> >  			last_item = MLX5_FLOW_LAYER_GRE_KEY;
> >  			break;
> > +		case RTE_FLOW_ITEM_TYPE_GRE_OPTION:
> > +			matcher.priority =
> > MLX5_TUNNEL_PRIO_GET(rss_desc);
> > +			last_item = MLX5_FLOW_LAYER_GRE;
> > +			tunnel_item = items;
> > +			break;
> >  		case RTE_FLOW_ITEM_TYPE_NVGRE:
> >  			matcher.priority =
> > MLX5_TUNNEL_PRIO_GET(rss_desc);
> >  			last_item = MLX5_FLOW_LAYER_GRE;
> > @@ -13634,6 +13752,9 @@ struct mlx5_list_entry *
> >  		else if (tunnel_item->type ==
> > RTE_FLOW_ITEM_TYPE_NVGRE)
> >  			flow_dv_translate_item_nvgre(match_mask,
> > match_value,
> >  						     tunnel_item, item_flags);
> > +		else if (tunnel_item->type ==
> > RTE_FLOW_ITEM_TYPE_GRE_OPTION)
> > +			flow_dv_translate_item_gre_option(match_mask,
> > match_value,
> > +					tunnel_item, gre_item, item_flags);
> >  		else
> >  			MLX5_ASSERT(false);
> >  	}
> > --
> > 1.8.3.1