> -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon at 6wind.com] > Sent: Monday, February 2, 2015 11:39 PM > To: Zhang, Helin > Cc: dev at dpdk.org > Subject: Re: [dpdk-dev] [PATCH 5/7] ethdev: unification of flow types > > 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 <helin.zhang at intel.com> > [...] > > --- 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? 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. > > > /** > > * 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_. OK. Started with RTE_ETH_FLOW_ is good for me. I will modified it in v2. > > [...] > > 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 ;) OK. I will remove it. Thanks! Regards, Helin > > -- > Thomas