From mboxrd@z Thu Jan 1 00:00:00 1970 From: =?iso-8859-1?Q?N=E9lio?= Laranjeiro Subject: Re: [PATCH v2 18/20] net/mlx5: add flow GRE item Date: Mon, 9 Jul 2018 15:58:08 +0200 Message-ID: <20180709135808.nwaopl6mk2ya4xjx@laranjeiro-vm.dev.6wind.com> References: <6190b2d787d3a9d2095e2eed17db1c20b2e3c4e1.1530111623.git.nelio.laranjeiro@6wind.com> <20180706234610.GC53779@yongseok-MBP.local> Mime-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Transfer-Encoding: 8bit Cc: dev@dpdk.org, Adrien Mazarguil To: Yongseok Koh Return-path: Received: from mail-wr1-f65.google.com (mail-wr1-f65.google.com [209.85.221.65]) by dpdk.org (Postfix) with ESMTP id 152CB1B42F for ; Mon, 9 Jul 2018 15:58:25 +0200 (CEST) Received: by mail-wr1-f65.google.com with SMTP id r16-v6so11114083wrt.11 for ; Mon, 09 Jul 2018 06:58:25 -0700 (PDT) Content-Disposition: inline In-Reply-To: <20180706234610.GC53779@yongseok-MBP.local> List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" 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