[dpdk-dev] [PATCH v2 18/20] net/mlx5: add flow GRE item

[Nélio Laranjeiro]

Hi Yongseok,

Only discussing here question, other comments are address, as I don't
have any objection I'll make the modification for them.

On Fri, Jul 06, 2018 at 04:46:11PM -0700, Yongseok Koh wrote:
>[...] 
> > +
> >  /** Handles information leading to a drop fate. */
> >  struct mlx5_flow_verbs {
> >  	LIST_ENTRY(mlx5_flow_verbs) next;
> > @@ -1005,12 +1010,23 @@ mlx5_flow_item_ipv6(const struct rte_flow_item *item, struct rte_flow *flow,
> >  						  item,
> >  						  "L3 cannot follow an L4"
> >  						  " layer");
> > +		/*
> > +		 * IPv6 is not recognised by the NIC inside a GRE tunnel.
> > +		 * Such support has to be disabled as the rule will be
> > +		 * accepted.  Tested with Mellanox OFED 4.3-3.0.2.1
> > +		 */
> 
> This comment doesn't look appropriate. Do you think it is a bug of OFED/FW,
> which can be fixed? Or, is it a HW erratum? Let's talk offline.

By the time it was as this Mellanox OFED was the latest GA 4.3-3.0.2.1,
this is no more the case today as it cannot be downloaded anymore. A
verification is still necessary.  If the issue is not present anymore
I'll remove the comment with the test.

>[...] 
> > +{
> > +	unsigned int i;
> > +	const enum ibv_flow_spec_type search = IBV_FLOW_SPEC_IPV6;
> > +	struct ibv_spec_header *hdr = (struct ibv_spec_header *)
> > +		((uint8_t *)attr + sizeof(struct ibv_flow_attr));
> > +
> > +	if (!attr)
> > +		return;
> > +	for (i = 0; i != attr->num_of_specs; ++i) {
> > +		if (hdr->type == search) {
> > +			struct ibv_flow_spec_ipv6 *ip =
> > +				(struct ibv_flow_spec_ipv6 *)hdr;
> > +
> > +			if (!ip->val.next_hdr) {
> 
> What if protocol in IP header does have wrong value other than 47 (IPPROTO_GRE)?
> Shouldn't we have a validation check for it in mlx5_flow_item_gre()?
>[...]

Already added, the same issue occurs also with UDP/TCP.  If the user
uses some protocol it must match the following layer, otherwise its
request won't be respected which is a bug.

Thanks,

-- 
Nélio Laranjeiro
6WIND