From mboxrd@z Thu Jan 1 00:00:00 1970 From: Thomas Monjalon Subject: Re: [PATCH 5/7] ethdev: unification of flow types Date: Mon, 02 Feb 2015 16:38:43 +0100 Message-ID: <5329819.johOmNX3Da@xps13> References: <1421650577-25969-1-git-send-email-helin.zhang@intel.com> <1421650577-25969-6-git-send-email-helin.zhang@intel.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7Bit Cc: dev-VfR2kkLFssw@public.gmane.org To: Helin Zhang Return-path: In-Reply-To: <1421650577-25969-6-git-send-email-helin.zhang-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> List-Id: patches and discussions about DPDK List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces-VfR2kkLFssw@public.gmane.org Sender: "dev" Hi Helin, 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 Why not using an enum? Nitpicking: numbers from 0 to 9 should be right aligned. > /** > * 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 = 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 = 64, > -}; 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_. [...] > 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 */ I think this comment is useless ;) -- Thomas