From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga01.intel.com (mga01.intel.com [192.55.52.88]) by dpdk.org (Postfix) with ESMTP id 82CFE9AA0 for ; Tue, 3 Feb 2015 06:40:17 +0100 (CET) Received: from fmsmga002.fm.intel.com ([10.253.24.26]) by fmsmga101.fm.intel.com with ESMTP; 02 Feb 2015 21:40:15 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.09,510,1418112000"; d="scan'208";a="671861428" Received: from pgsmsx101.gar.corp.intel.com ([10.221.44.78]) by fmsmga002.fm.intel.com with ESMTP; 02 Feb 2015 21:40:13 -0800 Received: from shsmsx103.ccr.corp.intel.com (10.239.4.69) by PGSMSX101.gar.corp.intel.com (10.221.44.78) with Microsoft SMTP Server (TLS) id 14.3.195.1; Tue, 3 Feb 2015 13:40:12 +0800 Received: from shsmsx104.ccr.corp.intel.com ([169.254.5.161]) by SHSMSX103.ccr.corp.intel.com ([169.254.4.91]) with mapi id 14.03.0195.001; Tue, 3 Feb 2015 13:40:06 +0800 From: "Zhang, Helin" To: Thomas Monjalon Thread-Topic: [dpdk-dev] [PATCH 5/7] ethdev: unification of flow types Thread-Index: AQHQPv5nwbxm6CQwM02wdRp3zUjDtJzeZ60w Date: Tue, 3 Feb 2015 05:40:04 +0000 Message-ID: References: <1421650577-25969-1-git-send-email-helin.zhang@intel.com> <1421650577-25969-6-git-send-email-helin.zhang@intel.com> <5329819.johOmNX3Da@xps13> In-Reply-To: <5329819.johOmNX3Da@xps13> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [10.239.127.40] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Cc: "dev@dpdk.org" Subject: Re: [dpdk-dev] [PATCH 5/7] ethdev: unification of flow types X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 03 Feb 2015 05:40:18 -0000 > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Monday, February 2, 2015 11:39 PM > To: Zhang, Helin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH 5/7] ethdev: unification of flow types >=20 > Hi Helin, >=20 > 2015-01-19 14:56, Helin Zhang: > > Flow types was defined actually for i40e hardware specifically, and > > wasn't able to be used for defining RSS offload types of all PMDs. It > > removed the enum flow types, and uses macros instead with new names. > > The new macros can be used for defining RSS offload types later. Also > > modifications are made in i40e and testpmd accordingly. > > > > Signed-off-by: Helin Zhang > [...] > > --- a/lib/librte_ether/rte_eth_ctrl.h > > +++ b/lib/librte_ether/rte_eth_ctrl.h > > @@ -46,6 +46,35 @@ > > extern "C" { > > #endif > > > > +/* > > + * A packet can be identified by hardware as different flow types. > > +Different > > + * NIC hardwares may support different flow types. > > + * Basically, the NIC hardware identifies the flow type as deep > > +protocol as > > + * possible, and exclusively. For example, if a packet is identified > > +as > > + * 'ETH_FLOW_TYPE_NONFRAG_IPV4_TCP', it will not be any of other flow > > +types, > > + * though it is an actual IPV4 packet. > > + * Note that the flow types are used to define RSS offload types in > > + * rte_ethdev.h. > > + */ > > +#define ETH_FLOW_TYPE_UNKNOWN 0 > > +#define ETH_FLOW_TYPE_IPV4 1 > > +#define ETH_FLOW_TYPE_FRAG_IPV4 2 > > +#define ETH_FLOW_TYPE_NONFRAG_IPV4_TCP 3 > > +#define ETH_FLOW_TYPE_NONFRAG_IPV4_UDP 4 > > +#define ETH_FLOW_TYPE_NONFRAG_IPV4_SCTP 5 #define > > +ETH_FLOW_TYPE_NONFRAG_IPV4_OTHER 6 > > +#define ETH_FLOW_TYPE_IPV6 7 > > +#define ETH_FLOW_TYPE_FRAG_IPV6 8 > > +#define ETH_FLOW_TYPE_NONFRAG_IPV6_TCP 9 > > +#define ETH_FLOW_TYPE_NONFRAG_IPV6_UDP 10 > > +#define ETH_FLOW_TYPE_NONFRAG_IPV6_SCTP 11 #define > > +ETH_FLOW_TYPE_NONFRAG_IPV6_OTHER 12 > > +#define ETH_FLOW_TYPE_L2_PAYLOAD 13 > > +#define ETH_FLOW_TYPE_IPV6_EX 14 > > +#define ETH_FLOW_TYPE_IPV6_TCP_EX 15 > > +#define ETH_FLOW_TYPE_IPV6_UDP_EX 16 > > +#define ETH_FLOW_TYPE_MAX 17 >=20 > Why not using an enum? Enum is 'int' which needs 32 bits, while flow_type is just 16 bits. The old one which is not enum was in rte_ethdev.h, the enum one was added recently in rte_eth_ctrl.h. > Nitpicking: numbers from 0 to 9 should be right aligned. In my source file, they are right aligned. >=20 > > /** > > * Feature filter types > > */ > > @@ -179,24 +208,6 @@ struct rte_eth_tunnel_filter_conf { > > #define RTE_ETH_FDIR_MAX_FLEXLEN 16 /** < Max length of > flexbytes. */ > > > > /** > > - * Flow type > > - */ > > -enum rte_eth_flow_type { > > - RTE_ETH_FLOW_TYPE_NONE =3D 0, > > - RTE_ETH_FLOW_TYPE_UDPV4, > > - RTE_ETH_FLOW_TYPE_TCPV4, > > - RTE_ETH_FLOW_TYPE_SCTPV4, > > - RTE_ETH_FLOW_TYPE_IPV4_OTHER, > > - RTE_ETH_FLOW_TYPE_FRAG_IPV4, > > - RTE_ETH_FLOW_TYPE_UDPV6, > > - RTE_ETH_FLOW_TYPE_TCPV6, > > - RTE_ETH_FLOW_TYPE_SCTPV6, > > - RTE_ETH_FLOW_TYPE_IPV6_OTHER, > > - RTE_ETH_FLOW_TYPE_FRAG_IPV6, > > - RTE_ETH_FLOW_TYPE_MAX =3D 64, > > -}; >=20 > You are renaming the prefix RTE_ETH_FLOW_TYPE_ to ETH_FLOW_TYPE. > As this is an exported enum (in the API), we should keep RTE_ prefix. > If you are trying to shorten the names, I suggest RTE_ETH_FLOW_. OK. Started with RTE_ETH_FLOW_ is good for me. I will modified it in v2. >=20 > [...] > > struct rte_eth_fdir_input { > > - enum rte_eth_flow_type flow_type; /**< Type of flow */ > > + uint16_t flow_type; /**< Type of flow */ > [...] > > struct rte_eth_fdir_flex_mask { > > - enum rte_eth_flow_type flow_type; /**< Flow type */ > > + uint16_t flow_type; /**< Flow type */ >=20 > I think this comment is useless ;) OK. I will remove it. Thanks! Regards, Helin >=20 > -- > Thomas