From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wm0-f66.google.com (mail-wm0-f66.google.com [74.125.82.66]) by dpdk.org (Postfix) with ESMTP id 9943E29D6 for ; Thu, 23 Nov 2017 10:40:10 +0100 (CET) Received: by mail-wm0-f66.google.com with SMTP id b189so15451647wmd.0 for ; Thu, 23 Nov 2017 01:40:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=6wind-com.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=yk1dczrd4Fo9VT/RA2hppFQrZ+gDIVwDp5s4/osYHvk=; b=OOmYGrkgptZ4jZj9I2MkasBY0GATsIlB4rsYTp5g+Rm6uTIKXe2DSOdGPz8878bEuT xVy3l4cpkJkTRSTKH63shTGYgOvy2JtjvjxNOTMeUpRdk5vv7gZ57ribxlidVsgJTBxq r5YSHIQcGinfoamVBkUAVy0TWLGm+QBgz0JShD64PRb0YvGrzn+RSsDAvJz84T5kPDMz kQxc+0ANx+FZJsL1Y5M5q5T3rt7L1M9j9I5JRX70K4DZDQoohObAg68NSLx+/tgGpce8 GHb3dWkfuey//ky5Ho2zg/sMX0n5N8xKYzJCVsFOnxMUvizYa7rK54vtTayXKCYFAwIP r6eg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=yk1dczrd4Fo9VT/RA2hppFQrZ+gDIVwDp5s4/osYHvk=; b=qmI2G9ALYfgXbu6HmyTW/P+pwOLQFZZqTUyo1ZrOmR7cQ6Ay2n6Z+DDs15GYafgtRK VZo0v4mp8+R54142sUTmEmwMGRFKIvMo40IHcHyDSovjefitKN4lDfLIbQh7dMOZNHfG UdAGKL/K3tS9T3YcMqTs/y2K2jiXio/AtM7vrHnDNy0bXgBjFLEQNx7chfBA383vOGOD yYZbHVST5NqndkVvih6ZusjZgElGd3u2UZ5GCmrgNtvgQVVQ//3LkLtLlvDytsGeRAJk 4XFaYBYdVWN4P8zXDOF1sngOw5wCdtJIQUWPbb23z3oIOw8O2DEjDWuiWwRbGmJMpi0V RpuQ== X-Gm-Message-State: AJaThX7TXwKiB80sdGgATf2vJPbNQNeX99X5/AX3UuCOU5ye4i2ukIQP 4oTNQc6rELBlGUarT4+3ibq4Tg== X-Google-Smtp-Source: AGs4zMaykEb4221eTT3tJPJkJ9rYE12hgar13LYG0xq2oECN5VC8KSWqdXhoKZGGdT5A+f5grrpU2w== X-Received: by 10.80.215.218 with SMTP id m26mr31998904edj.27.1511430010282; Thu, 23 Nov 2017 01:40:10 -0800 (PST) Received: from 6wind.com (host.78.145.23.62.rev.coltfrance.com. [62.23.145.78]) by smtp.gmail.com with ESMTPSA id g3sm10510112edi.66.2017.11.23.01.40.09 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Thu, 23 Nov 2017 01:40:09 -0800 (PST) Date: Thu, 23 Nov 2017 10:39:57 +0100 From: Adrien Mazarguil To: Andrew Rybchenko Cc: dev@dpdk.org, Jingjing Wu , Roman Zhukov Message-ID: <20171123093957.GH4062@6wind.com> References: <1511166121-743-1-git-send-email-arybchenko@solarflare.com> <1511166121-743-2-git-send-email-arybchenko@solarflare.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1511166121-743-2-git-send-email-arybchenko@solarflare.com> Subject: Re: [dpdk-dev] [PATCH 1/2] ethdev: add GENEVE flow pattern item X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 23 Nov 2017 09:40:10 -0000 On Mon, Nov 20, 2017 at 08:22:00AM +0000, Andrew Rybchenko wrote: > From: Roman Zhukov > > Add new pattern item RTE_FLOW_ITEM_TYPE_GENEVE in flow API. > This commit also adds default mask for these item. > > Signed-off-by: Roman Zhukov > Signed-off-by: Andrew Rybchenko OK, the main issue in this patch is you're inserting GENEVE in the middle of everything (enums, documentation, etc). Just append it to the enum to avoid ABI breakage as described in my previous message and use the same position everywhere else for consistency. It must appear after RTE_FLOW_ITEM_TYPE_ESP. More comments below. > --- > doc/guides/prog_guide/rte_flow.rst | 12 ++++++++++++ > lib/librte_ether/rte_flow.c | 1 + > lib/librte_ether/rte_flow.h | 31 +++++++++++++++++++++++++++++++ > 3 files changed, 44 insertions(+) > > diff --git a/doc/guides/prog_guide/rte_flow.rst b/doc/guides/prog_guide/rte_flow.rst > index d158be5..2f96623 100644 > --- a/doc/guides/prog_guide/rte_flow.rst > +++ b/doc/guides/prog_guide/rte_flow.rst > @@ -863,6 +863,18 @@ Matches a VXLAN header (RFC 7348). > - ``rsvd1``: reserved, normally 0x00. > - Default ``mask`` matches VNI only. > > +Item: ``GENEVE`` > +^^^^^^^^^^^^^^^ > + > +Matches a GENEVE header. > + > +- ``ver_opt_len_o_c_rsvd0``: version (2b), length of the options fields (6b), > + OAM packet (1b), critical options present (1b), reserved 0 (6b). > +- ``protocol``: protocol type. > +- ``vni``: virtual network identifier. > +- ``rsvd1``: reserved, normally 0x00. > +- Default ``mask`` matches protocol type and VNI. > + I'm not sure about the default mask. It should be the least common denominator, not necessarily what the first implementation supports. How about making it like VXLAN, i.e. VNI only? Does that make sense at all? > Item: ``E_TAG`` > ^^^^^^^^^^^^^^^ > > diff --git a/lib/librte_ether/rte_flow.c b/lib/librte_ether/rte_flow.c > index 6659063..bf1b253 100644 > --- a/lib/librte_ether/rte_flow.c > +++ b/lib/librte_ether/rte_flow.c > @@ -77,6 +77,7 @@ static const struct rte_flow_desc_data rte_flow_desc_item[] = { > MK_FLOW_ITEM(TCP, sizeof(struct rte_flow_item_tcp)), > MK_FLOW_ITEM(SCTP, sizeof(struct rte_flow_item_sctp)), > MK_FLOW_ITEM(VXLAN, sizeof(struct rte_flow_item_vxlan)), > + MK_FLOW_ITEM(GENEVE, sizeof(struct rte_flow_item_geneve)), You should add it at the end but some items are already missing from that list. Since I plan to send an overhaul for this function, you can leave this change out for the time being. > MK_FLOW_ITEM(MPLS, sizeof(struct rte_flow_item_mpls)), > MK_FLOW_ITEM(GRE, sizeof(struct rte_flow_item_gre)), > MK_FLOW_ITEM(E_TAG, sizeof(struct rte_flow_item_e_tag)), > diff --git a/lib/librte_ether/rte_flow.h b/lib/librte_ether/rte_flow.h > index 47c88ea..29d81d4 100644 > --- a/lib/librte_ether/rte_flow.h > +++ b/lib/librte_ether/rte_flow.h > @@ -272,6 +272,13 @@ enum rte_flow_item_type { > RTE_FLOW_ITEM_TYPE_VXLAN, > > /** > + * Matches a GENEVE header. > + * > + * See struct rte_flow_item_geneve. > + */ > + RTE_FLOW_ITEM_TYPE_GENEVE, > + > + /** > * Matches a E_TAG header. > * > * See struct rte_flow_item_e_tag. > @@ -651,6 +658,30 @@ static const struct rte_flow_item_vxlan rte_flow_item_vxlan_mask = { > #endif > > /** > + * RTE_FLOW_ITEM_TYPE_GENEVE. > + * > + * Matches a GENEVE header. > + */ > +struct rte_flow_item_geneve { > + /** > + * Version (2b), length of the options fields (6b), OAM packet (1b), > + * critical options present (1b), reserved 0 (6b). > + */ > + rte_be16_t ver_opt_len_o_c_rsvd0; > + rte_be16_t protocol; /**< Protocol type. */ > + uint8_t vni[3]; /**< Virtual Network Identifier. */ > + uint8_t rsvd1; /**< Reserved, normally 0x00. */ > +}; > + > +/** Default mask for RTE_FLOW_ITEM_TYPE_GENEVE. */ > +#ifndef __cplusplus > +static const struct rte_flow_item_geneve rte_flow_item_geneve_mask = { > + .protocol = RTE_BE16(0xffff), > + .vni = "\xff\xff\xff", > +}; > +#endif So how about removing .protocol from the default mask? > + > +/** > * RTE_FLOW_ITEM_TYPE_E_TAG. > * > * Matches a E-tag header. > -- > 2.7.4 > -- Adrien Mazarguil 6WIND