From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) by dpdk.org (Postfix) with ESMTP id A9F921B1BC for ; Thu, 5 Oct 2017 10:39:19 +0200 (CEST) Received: from fmsmga001.fm.intel.com ([10.253.24.23]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 05 Oct 2017 01:39:18 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.42,480,1500966000"; d="scan'208";a="1202487602" Received: from fmsmsx108.amr.corp.intel.com ([10.18.124.206]) by fmsmga001.fm.intel.com with ESMTP; 05 Oct 2017 01:39:18 -0700 Received: from fmsmsx157.amr.corp.intel.com (10.18.116.73) by FMSMSX108.amr.corp.intel.com (10.18.124.206) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 5 Oct 2017 01:39:19 -0700 Received: from shsmsx152.ccr.corp.intel.com (10.239.6.52) by FMSMSX157.amr.corp.intel.com (10.18.116.73) with Microsoft SMTP Server (TLS) id 14.3.319.2; Thu, 5 Oct 2017 01:39:18 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.213]) by SHSMSX152.ccr.corp.intel.com ([169.254.6.93]) with mapi id 14.03.0319.002; Thu, 5 Oct 2017 16:39:16 +0800 From: "Wu, Jingjing" To: Adrien Mazarguil CC: Sean Harte , "Xing, Beilei" , "Chilikin, Andrey" , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v6 4/8] ethdev: add GTP items to support flow API Thread-Index: AQHTOOKYJb06OzQaOUm0dqevQjPtgqLK/jwAgAAK7YCAAAn6gIAE6KWAgAFXeQCAA5wxMP//gQeAgACIFGA= Date: Thu, 5 Oct 2017 08:39:16 +0000 Message-ID: <9BB6961774997848B5B42BEC655768F810E8A09C@SHSMSX103.ccr.corp.intel.com> References: <1506565054-67690-1-git-send-email-beilei.xing@intel.com> <1506662342-18966-1-git-send-email-beilei.xing@intel.com> <1506662342-18966-5-git-send-email-beilei.xing@intel.com> <94479800C636CB44BD422CB454846E0132038E26@SHSMSX101.ccr.corp.intel.com> <20171002122737.GK3871@6wind.com> <9BB6961774997848B5B42BEC655768F810E89F3C@SHSMSX103.ccr.corp.intel.com> <20171005083019.GY3871@6wind.com> In-Reply-To: <20171005083019.GY3871@6wind.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiYjQ1YTU2MzgtMTViMS00ODljLThlZjEtNTg2ZTQ0ZDkzMGM4IiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX0lDIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE2LjUuOS4zIiwiVHJ1c3RlZExhYmVsSGFzaCI6ImVhcWxVOVJJWGJ0RUpReHdEZUF1QzhjcWs4XC80VVwvUDlvM1NWREdWZVozUT0ifQ== x-ctpclassification: CTP_IC dlp-product: dlpe-windows dlp-version: 11.0.0.116 dlp-reaction: no-action x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v6 4/8] ethdev: add GTP items to support flow API 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, 05 Oct 2017 08:39:20 -0000 > -----Original Message----- > From: Adrien Mazarguil [mailto:adrien.mazarguil@6wind.com] > Sent: Thursday, October 5, 2017 4:30 PM > To: Wu, Jingjing > Cc: Sean Harte ; Xing, Beilei ; = Chilikin, > Andrey ; dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v6 4/8] ethdev: add GTP items to support f= low API >=20 > On Thu, Oct 05, 2017 at 08:06:38AM +0000, Wu, Jingjing wrote: > > > > > > > -----Original Message----- > > > From: Sean Harte [mailto:seanbh@gmail.com] > > > Sent: Tuesday, October 3, 2017 4:57 PM > > > To: Adrien Mazarguil > > > Cc: Xing, Beilei ; Wu, Jingjing ; > Chilikin, > > > Andrey ; dev@dpdk.org > > > Subject: Re: [dpdk-dev] [PATCH v6 4/8] ethdev: add GTP items to suppo= rt flow API > > > > > > On 2 October 2017 at 13:27, Adrien Mazarguil > wrote: > > > > On Fri, Sep 29, 2017 at 10:29:55AM +0100, Sean Harte wrote: > > > >> On 29 September 2017 at 09:54, Xing, Beilei wrote: > > > > > > > >> >> > /** > > > >> >> > + * RTE_FLOW_ITEM_TYPE_GTP. > > > >> >> > + * > > > >> >> > + * Matches a GTPv1 header. > > > >> >> > + */ > > > >> >> > +struct rte_flow_item_gtp { > > > >> >> > + /** > > > >> >> > + * Version (3b), protocol type (1b), reserved (1b), > > > >> >> > + * Extension header flag (1b), > > > >> >> > + * Sequence number flag (1b), > > > >> >> > + * N-PDU number flag (1b). > > > >> >> > + */ > > > >> >> > + uint8_t v_pt_rsv_flags; > > > >> >> > + uint8_t msg_type; /**< Message type. */ > > > >> >> > + rte_be16_t msg_len; /**< Message length. */ > > > >> >> > + rte_be32_t teid; /**< Tunnel endpoint identifier. */ = }; > > > >> >> > > > >> >> In future, you might add support for GTPv2 (which is used since= LTE). > > > >> >> Maybe this structure should have v1 in its name to avoid confus= ion? > > > >> > > > > >> > I considered it before. But I think we can modify it when we sup= port GTPv2 in > future, > > > and keep concise 'GTP' currently:) since I have described it matches= v1 header. > > > >> > > > > >> > > > >> You could rename v_pt_rsv_flags to version_flags to avoid some fut= ure > > > >> code changes to support GTPv2. There's still the issue that not al= l > > > >> GTPv2 messages have a TEID though. > > > > > > > > Although they have the same size, the header of these two protocols > > > > obviously differs. My suggestion would be to go with a separate GTP= v2 > > > > pattern item using its own dedicated structure instead. > > > > > > > > -- > > > > Adrien Mazarguil > > > > 6WIND > > > > > > The 1st four bytes are the same (flags in first byte have different > > > meanings, but the bits indicating the version are in the same > > > location). After that, different fields in each version are optional, > > > and the headers have variable size. A single structure could be used > > > if the first field is renamed to something like "version_flags", and > > > then check that the teid field in item->mask is not set if > > > ((version_flags >> 5 =3D=3D 2) && ((version_flags >> 4) & 1) =3D=3D 1= ). If > > > there's going to be two structures, it would be good to put v1 and v2 > > > in the names, in my opinion. > > > > I think the name GTP is OK for now. Due to v1 and v2 are different, why= not rename > them > > when the v2 supporting are introduced? >=20 > In any case I'd rather avoid renaming and modifying existing items and > structure contents once part of the API to avoid API/ABI breakage that > require deprecation notices, user applications updates and so on; rte_flo= w > has been created as a kind of append-only API for this reason (of course > there are exceptions, such as a bad design choice for the VLAN item I int= end > to fix at some point). >=20 > I'm fine with the name "GTP" as defined now and documented as matching > GTPv1. We can add "GTPv2"-themed definitions later when some implementati= on > provides the ability to match this version. If you want to append the "v1= " > suffix right now to be more explicit, I'm also fine with that. Your call. >=20 Got your point, I'm also fine with the name now for GTPv1, and add "GTPv2" = when It is supported. Thanks Jingjing