Message ID | 1413861289-26662-3-git-send-email-helin.zhang@intel.com (mailing list archive) |
---|---|
State | Superseded, archived |
Headers |
Return-Path: <dev-bounces@dpdk.org> X-Original-To: patchwork@dpdk.org Delivered-To: patchwork@dpdk.org Received: from [92.243.14.124] (localhost [IPv6:::1]) by dpdk.org (Postfix) with ESMTP id 0C7397E95; Tue, 21 Oct 2014 05:09:59 +0200 (CEST) Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id 36D427E7A for <dev@dpdk.org>; Tue, 21 Oct 2014 05:09:54 +0200 (CEST) Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga101.jf.intel.com with ESMTP; 20 Oct 2014 20:15:07 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.04,759,1406617200"; d="scan'208";a="622306768" Received: from shvmail01.sh.intel.com ([10.239.29.42]) by orsmga002.jf.intel.com with ESMTP; 20 Oct 2014 20:15:06 -0700 Received: from shecgisg004.sh.intel.com (shecgisg004.sh.intel.com [10.239.29.89]) by shvmail01.sh.intel.com with ESMTP id s9L3F44J014806; Tue, 21 Oct 2014 11:15:04 +0800 Received: from shecgisg004.sh.intel.com (localhost [127.0.0.1]) by shecgisg004.sh.intel.com (8.13.6/8.13.6/SuSE Linux 0.8) with ESMTP id s9L3F2N7026755; Tue, 21 Oct 2014 11:15:04 +0800 Received: (from hzhan75@localhost) by shecgisg004.sh.intel.com (8.13.6/8.13.6/Submit) id s9L3F2Lf026751; Tue, 21 Oct 2014 11:15:02 +0800 From: Helin Zhang <helin.zhang@intel.com> To: dev@dpdk.org Date: Tue, 21 Oct 2014 11:14:46 +0800 Message-Id: <1413861289-26662-3-git-send-email-helin.zhang@intel.com> X-Mailer: git-send-email 1.7.4.1 In-Reply-To: <1413861289-26662-1-git-send-email-helin.zhang@intel.com> References: <1413180766-12211-1-git-send-email-helin.zhang@intel.com> <1413861289-26662-1-git-send-email-helin.zhang@intel.com> Subject: [dpdk-dev] [PATCH v5 2/5] ethdev: add enum type and relevant structures for hash filter control X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches and discussions about DPDK <dev.dpdk.org> List-Unsubscribe: <http://dpdk.org/ml/options/dev>, <mailto:dev-request@dpdk.org?subject=unsubscribe> List-Archive: <http://dpdk.org/ml/archives/dev/> List-Post: <mailto:dev@dpdk.org> List-Help: <mailto:dev-request@dpdk.org?subject=help> List-Subscribe: <http://dpdk.org/ml/listinfo/dev>, <mailto:dev-request@dpdk.org?subject=subscribe> Errors-To: dev-bounces@dpdk.org Sender: "dev" <dev-bounces@dpdk.org> |
Commit Message
Zhang, Helin
Oct. 21, 2014, 3:14 a.m. UTC
enum type and relevant structures are added in rte_eth_ctrl.h to
support hash filter control.
Signed-off-by: Helin Zhang <helin.zhang@intel.com>
---
lib/librte_ether/rte_eth_ctrl.h | 75 +++++++++++++++++++++++++++++++++++++++++
1 file changed, 75 insertions(+)
v5 changes:
* Integrated with filter API defined recently.
Comments
2014-10-21 11:14, Helin Zhang: > +enum rte_eth_hash_filter_info_type { > + RTE_ETH_HASH_FILTER_INFO_TYPE_UNKNOWN = 0, > + RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PCTYPE, PCTYPE is an unknown word in the API layer. Could you replace it by something more generic? > + RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PORT, > + RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP, > + RTE_ETH_HASH_FILTER_INFO_TYPE_HASH_FUNCTION, > + RTE_ETH_HASH_FILTER_INFO_TYPE_MAX, > +}; You should comment each constant. > +struct rte_eth_sym_hash_ena_info { > + /**< packet classification type, defined in rte_ethdev.h */ > + uint8_t pctype; No, PCTYPE is not anymore defined in ethdev. > +/** > + * A structure used to set or get filter swap information, to support > + * 'RTE_ETH_FILTER_HASH', 'RTE_ETH_FILTER_GET/RTE_ETH_FILTER_SET', > + * with information type 'RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP'. > + */ > +struct rte_eth_filter_swap_info { > + /**< Packet classification type, defined in rte_ethdev.h */ > + uint8_t pctype; > + /**< Offset of the 1st field of the 1st couple to be swapped. */ > + uint8_t off0_src0; > + /**< Offset of the 2nd field of the 1st couple to be swapped. */ > + uint8_t off0_src1; > + /**< Field length of the first couple. */ > + uint8_t len0; > + /**< Offset of the 1st field of the 2nd couple to be swapped. */ > + uint8_t off1_src0; > + /**< Offset of the 2nd field of the 2nd couple to be swapped. */ > + uint8_t off1_src1; > + /**< Field length of the second couple. */ > + uint8_t len1; > +}; I guess it would be easier to understand if RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP was defined previously.
Thanks for your good comments! > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Monday, November 3, 2014 3:57 PM > To: Zhang, Helin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 2/5] ethdev: add enum type and relevant > structures for hash filter control > > 2014-10-21 11:14, Helin Zhang: > > +enum rte_eth_hash_filter_info_type { > > + RTE_ETH_HASH_FILTER_INFO_TYPE_UNKNOWN = 0, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PCTYPE, > > PCTYPE is an unknown word in the API layer. > Could you replace it by something more generic? So you suggested to remove words of PCTYPE, pctype, or packet classification type? I am not trying to rename ETH_RSS_IPV4_SHIFT, ..., ETH_RSS_NONF_IPV4_UDP_SHIFT, ..., etc. They are actually pctype in i40e, and not only for RSS. I would like to rename them into generic names. Any good naming ideas from you or other guys? My idea is to rename them like RTE_ETH_FLOW_TYPE_XX. > > > + RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PORT, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_HASH_FUNCTION, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_MAX, > > +}; > > You should comment each constant. OK. Good to know that! > > > +struct rte_eth_sym_hash_ena_info { > > + /**< packet classification type, defined in rte_ethdev.h */ > > + uint8_t pctype; > > No, PCTYPE is not anymore defined in ethdev. We need a generic name for that, how about flow_type? Good comments are welcome! > > > +/** > > + * A structure used to set or get filter swap information, to support > > + * 'RTE_ETH_FILTER_HASH', 'RTE_ETH_FILTER_GET/RTE_ETH_FILTER_SET', > > + * with information type 'RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP'. > > + */ > > +struct rte_eth_filter_swap_info { > > + /**< Packet classification type, defined in rte_ethdev.h */ > > + uint8_t pctype; > > + /**< Offset of the 1st field of the 1st couple to be swapped. */ > > + uint8_t off0_src0; > > + /**< Offset of the 2nd field of the 1st couple to be swapped. */ > > + uint8_t off0_src1; > > + /**< Field length of the first couple. */ > > + uint8_t len0; > > + /**< Offset of the 1st field of the 2nd couple to be swapped. */ > > + uint8_t off1_src0; > > + /**< Offset of the 2nd field of the 2nd couple to be swapped. */ > > + uint8_t off1_src1; > > + /**< Field length of the second couple. */ > > + uint8_t len1; > > +}; > > I guess it would be easier to understand if > RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP was defined previously. It has already been defined before this structure definition. I don't think I have understood your idea. Could you help to explain more? Thanks! > > -- > Thomas Regards, Helin
2014-11-06 03:41, Zhang, Helin: > > > +/** > > > + * A structure used to set or get filter swap information, to support > > > + * 'RTE_ETH_FILTER_HASH', 'RTE_ETH_FILTER_GET/RTE_ETH_FILTER_SET', > > > + * with information type 'RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP'. > > > + */ > > > +struct rte_eth_filter_swap_info { > > > + /**< Packet classification type, defined in rte_ethdev.h */ > > > + uint8_t pctype; > > > + /**< Offset of the 1st field of the 1st couple to be swapped. */ > > > + uint8_t off0_src0; > > > + /**< Offset of the 2nd field of the 1st couple to be swapped. */ > > > + uint8_t off0_src1; > > > + /**< Field length of the first couple. */ > > > + uint8_t len0; > > > + /**< Offset of the 1st field of the 2nd couple to be swapped. */ > > > + uint8_t off1_src0; > > > + /**< Offset of the 2nd field of the 2nd couple to be swapped. */ > > > + uint8_t off1_src1; > > > + /**< Field length of the second couple. */ > > > + uint8_t len1; > > > +}; > > > > I guess it would be easier to understand if > > RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP was defined previously. > > It has already been defined before this structure definition. > I don't think I have understood your idea. Could you help to explain more? Thanks! By "defined", I mean "explained". What is the action of swap filter? You offer new features in API without explaining them. It's probably obvious for you but not for me.
> -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Thursday, November 6, 2014 4:43 PM > To: Zhang, Helin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 2/5] ethdev: add enum type and relevant > structures for hash filter control > > 2014-11-06 03:41, Zhang, Helin: > > > > +/** > > > > + * A structure used to set or get filter swap information, to > > > > +support > > > > + * 'RTE_ETH_FILTER_HASH', > > > > +'RTE_ETH_FILTER_GET/RTE_ETH_FILTER_SET', > > > > + * with information type > 'RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP'. > > > > + */ > > > > +struct rte_eth_filter_swap_info { > > > > + /**< Packet classification type, defined in rte_ethdev.h */ > > > > + uint8_t pctype; > > > > + /**< Offset of the 1st field of the 1st couple to be swapped. */ > > > > + uint8_t off0_src0; > > > > + /**< Offset of the 2nd field of the 1st couple to be swapped. */ > > > > + uint8_t off0_src1; > > > > + /**< Field length of the first couple. */ > > > > + uint8_t len0; > > > > + /**< Offset of the 1st field of the 2nd couple to be swapped. */ > > > > + uint8_t off1_src0; > > > > + /**< Offset of the 2nd field of the 2nd couple to be swapped. */ > > > > + uint8_t off1_src1; > > > > + /**< Field length of the second couple. */ > > > > + uint8_t len1; > > > > +}; > > > > > > I guess it would be easier to understand if > > > RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP was defined previously. > > > > It has already been defined before this structure definition. > > I don't think I have understood your idea. Could you help to explain more? > Thanks! > > By "defined", I mean "explained". What is the action of swap filter? > You offer new features in API without explaining them. It's probably obvious for > you but not for me. Yes, they should be explained. > > -- > Thomas Regards, Helin
> -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Monday, November 3, 2014 3:57 PM > To: Zhang, Helin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 2/5] ethdev: add enum type and relevant > structures for hash filter control > > 2014-10-21 11:14, Helin Zhang: > > +enum rte_eth_hash_filter_info_type { > > + RTE_ETH_HASH_FILTER_INFO_TYPE_UNKNOWN = 0, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PCTYPE, > > PCTYPE is an unknown word in the API layer. > Could you replace it by something more generic? In ethdev layer, the idea of 'flow type' will be used for generic purpose. > > > + RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PORT, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_HASH_FUNCTION, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_MAX, > > +}; > > You should comment each constant. Yes, agree. > > > +struct rte_eth_sym_hash_ena_info { > > + /**< packet classification type, defined in rte_ethdev.h */ > > + uint8_t pctype; > > No, PCTYPE is not anymore defined in ethdev. No pctype will be there on ethdev layer. > > > +/** > > + * A structure used to set or get filter swap information, to support > > + * 'RTE_ETH_FILTER_HASH', 'RTE_ETH_FILTER_GET/RTE_ETH_FILTER_SET', > > + * with information type 'RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP'. > > + */ > > +struct rte_eth_filter_swap_info { > > + /**< Packet classification type, defined in rte_ethdev.h */ > > + uint8_t pctype; > > + /**< Offset of the 1st field of the 1st couple to be swapped. */ > > + uint8_t off0_src0; > > + /**< Offset of the 2nd field of the 1st couple to be swapped. */ > > + uint8_t off0_src1; > > + /**< Field length of the first couple. */ > > + uint8_t len0; > > + /**< Offset of the 1st field of the 2nd couple to be swapped. */ > > + uint8_t off1_src0; > > + /**< Offset of the 2nd field of the 2nd couple to be swapped. */ > > + uint8_t off1_src1; > > + /**< Field length of the second couple. */ > > + uint8_t len1; > > +}; > > I guess it would be easier to understand if > RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP was defined previously. > > -- > Thomas Regards, Helin
Hi Thomas In order to get things more generic, and remove any mappings on specific NIC hardwares, I planned to change the macros in rte_ethdev.h from /* Supported RSS offloads */ /* for 1G & 10G */ #define ETH_RSS_IPV4_SHIFT 0 #define ETH_RSS_IPV4_TCP_SHIFT 1 #define ETH_RSS_IPV6_SHIFT 2 #define ETH_RSS_IPV6_EX_SHIFT 3 #define ETH_RSS_IPV6_TCP_SHIFT 4 #define ETH_RSS_IPV6_TCP_EX_SHIFT 5 #define ETH_RSS_IPV4_UDP_SHIFT 6 #define ETH_RSS_IPV6_UDP_SHIFT 7 #define ETH_RSS_IPV6_UDP_EX_SHIFT 8 /* for 40G only */ #define ETH_RSS_NONF_IPV4_UDP_SHIFT 31 #define ETH_RSS_NONF_IPV4_TCP_SHIFT 33 #define ETH_RSS_NONF_IPV4_SCTP_SHIFT 34 #define ETH_RSS_NONF_IPV4_OTHER_SHIFT 35 #define ETH_RSS_FRAG_IPV4_SHIFT 36 #define ETH_RSS_NONF_IPV6_UDP_SHIFT 41 #define ETH_RSS_NONF_IPV6_TCP_SHIFT 43 #define ETH_RSS_NONF_IPV6_SCTP_SHIFT 44 #define ETH_RSS_NONF_IPV6_OTHER_SHIFT 45 #define ETH_RSS_FRAG_IPV6_SHIFT 46 #define ETH_RSS_FCOE_OX_SHIFT 48 #define ETH_RSS_FCOE_RX_SHIFT 49 #define ETH_RSS_FCOE_OTHER_SHIFT 50 #define ETH_RSS_L2_PAYLOAD_SHIFT 63 to /* Supported RSS offloads */ /* for 1G & 10G */ #define ETH_FLOW_TYPE_IPV4 0 #define ETH_FLOW_TYPE_IPV4_TCP 1 #define ETH_FLOW_TYPE_IPV6 2 #define ETH_FLOW_TYPE_IPV6_EX 3 #define ETH_FLOW_TYPE_IPV6_TCP 4 #define ETH_FLOW_TYPE_IPV6_TCP_EX 5 #define ETH_FLOW_TYPE_IPV4_UDP 6 #define ETH_FLOW_TYPE_IPV6_UDP 7 #define ETH_FLOW_TYPE_IPV6_UDP_EX 8 /* for 40G only */ #define ETH_FLOW_TYPE_NONFRAG_IPV4_UDP 9 #define ETH_FLOW_TYPE_NONFRAG_IPV4_TCP 10 #define ETH_FLOW_TYPE_NONFRAG_IPV4_SCTP 11 #define ETH_FLOW_TYPE_NONFRAG_IPV4_OTHER 12 #define ETH_FLOW_TYPE_FRAG_IPV4 13 #define ETH_FLOW_TYPE_NONFRAG_IPV6_UDP 14 #define ETH_FLOW_TYPE_NONFRAG_IPV6_TCP 15 #define ETH_FLOW_TYPE_NONFRAG_IPV6_SCTP 16 #define ETH_FLOW_TYPE_NONFRAG_IPV6_OTHER 17 #define ETH_FLOW_TYPE_FRAG_IPV6 18 #define ETH_FLOW_TYPE_L2_PAYLOAD 19 Any comments or better ideas on that? Thanks! Regards, Helin > -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Monday, November 3, 2014 3:57 PM > To: Zhang, Helin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 2/5] ethdev: add enum type and relevant > structures for hash filter control > > 2014-10-21 11:14, Helin Zhang: > > +enum rte_eth_hash_filter_info_type { > > + RTE_ETH_HASH_FILTER_INFO_TYPE_UNKNOWN = 0, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PCTYPE, > > PCTYPE is an unknown word in the API layer. > Could you replace it by something more generic? > > > + RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PORT, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_HASH_FUNCTION, > > + RTE_ETH_HASH_FILTER_INFO_TYPE_MAX, > > +}; > > You should comment each constant. > > > +struct rte_eth_sym_hash_ena_info { > > + /**< packet classification type, defined in rte_ethdev.h */ > > + uint8_t pctype; > > No, PCTYPE is not anymore defined in ethdev. > > > +/** > > + * A structure used to set or get filter swap information, to support > > + * 'RTE_ETH_FILTER_HASH', 'RTE_ETH_FILTER_GET/RTE_ETH_FILTER_SET', > > + * with information type 'RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP'. > > + */ > > +struct rte_eth_filter_swap_info { > > + /**< Packet classification type, defined in rte_ethdev.h */ > > + uint8_t pctype; > > + /**< Offset of the 1st field of the 1st couple to be swapped. */ > > + uint8_t off0_src0; > > + /**< Offset of the 2nd field of the 1st couple to be swapped. */ > > + uint8_t off0_src1; > > + /**< Field length of the first couple. */ > > + uint8_t len0; > > + /**< Offset of the 1st field of the 2nd couple to be swapped. */ > > + uint8_t off1_src0; > > + /**< Offset of the 2nd field of the 2nd couple to be swapped. */ > > + uint8_t off1_src1; > > + /**< Field length of the second couple. */ > > + uint8_t len1; > > +}; > > I guess it would be easier to understand if > RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP was defined previously. > > -- > Thomas
2014-11-11 06:46, Zhang, Helin: > In order to get things more generic, and remove any mappings on specific NIC hardwares, I planned to change the macros in rte_ethdev.h from > > /* Supported RSS offloads */ > /* for 1G & 10G */ > #define ETH_RSS_IPV4_SHIFT 0 > #define ETH_RSS_IPV4_TCP_SHIFT 1 > #define ETH_RSS_IPV6_SHIFT 2 > #define ETH_RSS_IPV6_EX_SHIFT 3 > #define ETH_RSS_IPV6_TCP_SHIFT 4 > #define ETH_RSS_IPV6_TCP_EX_SHIFT 5 > #define ETH_RSS_IPV4_UDP_SHIFT 6 > #define ETH_RSS_IPV6_UDP_SHIFT 7 > #define ETH_RSS_IPV6_UDP_EX_SHIFT 8 > /* for 40G only */ > #define ETH_RSS_NONF_IPV4_UDP_SHIFT 31 > #define ETH_RSS_NONF_IPV4_TCP_SHIFT 33 > #define ETH_RSS_NONF_IPV4_SCTP_SHIFT 34 > #define ETH_RSS_NONF_IPV4_OTHER_SHIFT 35 > #define ETH_RSS_FRAG_IPV4_SHIFT 36 > #define ETH_RSS_NONF_IPV6_UDP_SHIFT 41 > #define ETH_RSS_NONF_IPV6_TCP_SHIFT 43 > #define ETH_RSS_NONF_IPV6_SCTP_SHIFT 44 > #define ETH_RSS_NONF_IPV6_OTHER_SHIFT 45 > #define ETH_RSS_FRAG_IPV6_SHIFT 46 > #define ETH_RSS_FCOE_OX_SHIFT 48 > #define ETH_RSS_FCOE_RX_SHIFT 49 > #define ETH_RSS_FCOE_OTHER_SHIFT 50 > #define ETH_RSS_L2_PAYLOAD_SHIFT 63 > > to > > /* Supported RSS offloads */ > /* for 1G & 10G */ > #define ETH_FLOW_TYPE_IPV4 0 > #define ETH_FLOW_TYPE_IPV4_TCP 1 > #define ETH_FLOW_TYPE_IPV6 2 > #define ETH_FLOW_TYPE_IPV6_EX 3 > #define ETH_FLOW_TYPE_IPV6_TCP 4 > #define ETH_FLOW_TYPE_IPV6_TCP_EX 5 > #define ETH_FLOW_TYPE_IPV4_UDP 6 > #define ETH_FLOW_TYPE_IPV6_UDP 7 > #define ETH_FLOW_TYPE_IPV6_UDP_EX 8 > /* for 40G only */ > #define ETH_FLOW_TYPE_NONFRAG_IPV4_UDP 9 > #define ETH_FLOW_TYPE_NONFRAG_IPV4_TCP 10 > #define ETH_FLOW_TYPE_NONFRAG_IPV4_SCTP 11 > #define ETH_FLOW_TYPE_NONFRAG_IPV4_OTHER 12 > #define ETH_FLOW_TYPE_FRAG_IPV4 13 > #define ETH_FLOW_TYPE_NONFRAG_IPV6_UDP 14 > #define ETH_FLOW_TYPE_NONFRAG_IPV6_TCP 15 > #define ETH_FLOW_TYPE_NONFRAG_IPV6_SCTP 16 > #define ETH_FLOW_TYPE_NONFRAG_IPV6_OTHER 17 > #define ETH_FLOW_TYPE_FRAG_IPV6 18 > #define ETH_FLOW_TYPE_L2_PAYLOAD 19 > > Any comments or better ideas on that? Thanks! About the renaming RSS -> FLOW_TYPE, I have no objection. It seems a bit better. Some comments are needed to explain what means the value. I think the comments "1G & 10G" or "40G only" are possibly wrong. Actually you use ETH_FLOW_TYPE_IPV4 for ixgbe and ETH_FLOW_TYPE_FRAG_IPV4 or ETH_FLOW_TYPE_NONFRAG_IPV4_* for i40e. It's not consistent and clearly shows that you stick to the hardware definitions. Something really generic could be a set of flags like this: IPV4 IPV6 NONFRAG UDP TCP SCTP
> -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Wednesday, November 12, 2014 5:09 AM > To: Zhang, Helin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 2/5] ethdev: add enum type and relevant > structures for hash filter control > > 2014-11-11 06:46, Zhang, Helin: > > In order to get things more generic, and remove any mappings on > > specific NIC hardwares, I planned to change the macros in rte_ethdev.h > > from > > > > /* Supported RSS offloads */ > > /* for 1G & 10G */ > > #define ETH_RSS_IPV4_SHIFT 0 > > #define ETH_RSS_IPV4_TCP_SHIFT 1 > > #define ETH_RSS_IPV6_SHIFT 2 > > #define ETH_RSS_IPV6_EX_SHIFT 3 > > #define ETH_RSS_IPV6_TCP_SHIFT 4 > > #define ETH_RSS_IPV6_TCP_EX_SHIFT 5 > > #define ETH_RSS_IPV4_UDP_SHIFT 6 > > #define ETH_RSS_IPV6_UDP_SHIFT 7 > > #define ETH_RSS_IPV6_UDP_EX_SHIFT 8 > > /* for 40G only */ > > #define ETH_RSS_NONF_IPV4_UDP_SHIFT 31 > > #define ETH_RSS_NONF_IPV4_TCP_SHIFT 33 > > #define ETH_RSS_NONF_IPV4_SCTP_SHIFT 34 > > #define ETH_RSS_NONF_IPV4_OTHER_SHIFT 35 > > #define ETH_RSS_FRAG_IPV4_SHIFT 36 > > #define ETH_RSS_NONF_IPV6_UDP_SHIFT 41 > > #define ETH_RSS_NONF_IPV6_TCP_SHIFT 43 > > #define ETH_RSS_NONF_IPV6_SCTP_SHIFT 44 > > #define ETH_RSS_NONF_IPV6_OTHER_SHIFT 45 > > #define ETH_RSS_FRAG_IPV6_SHIFT 46 > > #define ETH_RSS_FCOE_OX_SHIFT 48 > > #define ETH_RSS_FCOE_RX_SHIFT 49 > > #define ETH_RSS_FCOE_OTHER_SHIFT 50 > > #define ETH_RSS_L2_PAYLOAD_SHIFT 63 > > > > to > > > > /* Supported RSS offloads */ > > /* for 1G & 10G */ > > #define ETH_FLOW_TYPE_IPV4 0 > > #define ETH_FLOW_TYPE_IPV4_TCP 1 > > #define ETH_FLOW_TYPE_IPV6 2 > > #define ETH_FLOW_TYPE_IPV6_EX 3 > > #define ETH_FLOW_TYPE_IPV6_TCP 4 > > #define ETH_FLOW_TYPE_IPV6_TCP_EX 5 > > #define ETH_FLOW_TYPE_IPV4_UDP 6 > > #define ETH_FLOW_TYPE_IPV6_UDP 7 > > #define ETH_FLOW_TYPE_IPV6_UDP_EX 8 > > /* for 40G only */ > > #define ETH_FLOW_TYPE_NONFRAG_IPV4_UDP 9 > > #define ETH_FLOW_TYPE_NONFRAG_IPV4_TCP 10 > > #define ETH_FLOW_TYPE_NONFRAG_IPV4_SCTP 11 > > #define ETH_FLOW_TYPE_NONFRAG_IPV4_OTHER 12 > > #define ETH_FLOW_TYPE_FRAG_IPV4 13 > > #define ETH_FLOW_TYPE_NONFRAG_IPV6_UDP 14 > > #define ETH_FLOW_TYPE_NONFRAG_IPV6_TCP 15 > > #define ETH_FLOW_TYPE_NONFRAG_IPV6_SCTP 16 > > #define ETH_FLOW_TYPE_NONFRAG_IPV6_OTHER 17 > > #define ETH_FLOW_TYPE_FRAG_IPV6 18 > > #define ETH_FLOW_TYPE_L2_PAYLOAD 19 > > > > Any comments or better ideas on that? Thanks! > > About the renaming RSS -> FLOW_TYPE, I have no objection. > It seems a bit better. Nice to hear that! > Some comments are needed to explain what means the value. > I think the comments "1G & 10G" or "40G only" are possibly wrong. Yes, I agree with you. > Actually you use ETH_FLOW_TYPE_IPV4 for ixgbe and > ETH_FLOW_TYPE_FRAG_IPV4 or ETH_FLOW_TYPE_NONFRAG_IPV4_* for i40e. > It's not consistent and clearly shows that you stick to the hardware definitions. > > Something really generic could be a set of flags like this: > IPV4 > IPV6 > NONFRAG > UDP > TCP > SCTP Good conclusion! We could think of it in a new patch set. I don't want to put everything into this patch. :) > > -- > Thomas Regards, Helin
2014-11-12 05:52, Zhang, Helin: > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > Actually you use ETH_FLOW_TYPE_IPV4 for ixgbe and > > ETH_FLOW_TYPE_FRAG_IPV4 or ETH_FLOW_TYPE_NONFRAG_IPV4_* for i40e. > > It's not consistent and clearly shows that you stick to the hardware definitions. > > > > Something really generic could be a set of flags like this: > > IPV4 > > IPV6 > > NONFRAG > > UDP > > TCP > > SCTP > > Good conclusion! We could think of it in a new patch set. > I don't want to put everything into this patch. :) If you agree flags must be used, the old defines must be removed. So no need to rename the defines. I think you should directly change to flags (in this patchset or another). Thanks
> -----Original Message----- > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > Sent: Wednesday, November 12, 2014 5:31 PM > To: Zhang, Helin > Cc: dev@dpdk.org > Subject: Re: [dpdk-dev] [PATCH v5 2/5] ethdev: add enum type and relevant > structures for hash filter control > > 2014-11-12 05:52, Zhang, Helin: > > From: Thomas Monjalon [mailto:thomas.monjalon@6wind.com] > > > Actually you use ETH_FLOW_TYPE_IPV4 for ixgbe and > > > ETH_FLOW_TYPE_FRAG_IPV4 or ETH_FLOW_TYPE_NONFRAG_IPV4_* for > i40e. > > > It's not consistent and clearly shows that you stick to the hardware > definitions. > > > > > > Something really generic could be a set of flags like this: > > > IPV4 > > > IPV6 > > > NONFRAG > > > UDP > > > TCP > > > SCTP > > > > Good conclusion! We could think of it in a new patch set. > > I don't want to put everything into this patch. :) > > If you agree flags must be used, the old defines must be removed. > So no need to rename the defines. > I think you should directly change to flags (in this patchset or another). I will keep it as it is in this patch for configuring hash functions. But I really agree with your idea of using flags instead. I will rework it in another patch set and send out a RFC to see if there is any objections. For this patch set, I will focus on feature itself. > > Thanks > -- > Thomas Regards, Helin
diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_ctrl.h index df21ac6..c469b57 100644 --- a/lib/librte_ether/rte_eth_ctrl.h +++ b/lib/librte_ether/rte_eth_ctrl.h @@ -51,6 +51,7 @@ extern "C" { */ enum rte_filter_type { RTE_ETH_FILTER_NONE = 0, + RTE_ETH_FILTER_HASH, RTE_ETH_FILTER_MAX }; @@ -71,6 +72,80 @@ enum rte_filter_op { RTE_ETH_FILTER_OP_MAX }; +/** + * Hash filter information types. + */ +enum rte_eth_hash_filter_info_type { + RTE_ETH_HASH_FILTER_INFO_TYPE_UNKNOWN = 0, + RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PCTYPE, + RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PORT, + RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP, + RTE_ETH_HASH_FILTER_INFO_TYPE_HASH_FUNCTION, + RTE_ETH_HASH_FILTER_INFO_TYPE_MAX, +}; + +/** + * Hash function types. + */ +enum rte_eth_hash_function { + RTE_ETH_HASH_FUNCTION_UNKNOWN = 0, + RTE_ETH_HASH_FUNCTION_TOEPLITZ, + RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, + RTE_ETH_HASH_FUNCTION_MAX, +}; + +/** + * A structure used to set or get symmetric hash enable information, to support + * 'RTE_ETH_FILTER_HASH', 'RTE_ETH_FILTER_GET/RTE_ETH_FILTER_SET', with + * information type 'RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PCTYPE'. + */ +struct rte_eth_sym_hash_ena_info { + /**< packet classification type, defined in rte_ethdev.h */ + uint8_t pctype; + uint8_t enable; /**< enable or disable flag */ +}; + +/** + * A structure used to set or get filter swap information, to support + * 'RTE_ETH_FILTER_HASH', 'RTE_ETH_FILTER_GET/RTE_ETH_FILTER_SET', + * with information type 'RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP'. + */ +struct rte_eth_filter_swap_info { + /**< Packet classification type, defined in rte_ethdev.h */ + uint8_t pctype; + /**< Offset of the 1st field of the 1st couple to be swapped. */ + uint8_t off0_src0; + /**< Offset of the 2nd field of the 1st couple to be swapped. */ + uint8_t off0_src1; + /**< Field length of the first couple. */ + uint8_t len0; + /**< Offset of the 1st field of the 2nd couple to be swapped. */ + uint8_t off1_src0; + /**< Offset of the 2nd field of the 2nd couple to be swapped. */ + uint8_t off1_src1; + /**< Field length of the second couple. */ + uint8_t len1; +}; + +/** + * A structure used to set or get hash filter information, to support filter + * type of 'RTE_ETH_FILTER_HASH' and its operations. + */ +struct rte_eth_hash_filter_info { + enum rte_eth_hash_filter_info_type info_type; /**< Information type. */ + /**< Details of hash filter infomation */ + union { + /* For RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PCTYPE */ + struct rte_eth_sym_hash_ena_info sym_hash_ena; + /* For RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP */ + struct rte_eth_filter_swap_info filter_swap; + /* For RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PORT */ + uint8_t enable; + /* For RTE_ETH_HASH_FILTER_INFO_TYPE_HASH_FUNCTION */ + enum rte_eth_hash_function hash_function; + } info; +}; + #ifdef __cplusplus } #endif