From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mga02.intel.com (mga02.intel.com [134.134.136.20]) by dpdk.org (Postfix) with ESMTP id EAF6858E8 for ; Fri, 28 Nov 2014 13:52:04 +0100 (CET) Received: from orsmga001.jf.intel.com ([10.7.209.18]) by orsmga101.jf.intel.com with ESMTP; 28 Nov 2014 04:52:03 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.07,476,1413270000"; d="scan'208";a="615469206" Received: from irsmsx154.ger.corp.intel.com ([163.33.192.96]) by orsmga001.jf.intel.com with ESMTP; 28 Nov 2014 04:52:02 -0800 Received: from irsmsx105.ger.corp.intel.com ([169.254.7.144]) by IRSMSX154.ger.corp.intel.com ([169.254.12.18]) with mapi id 14.03.0195.001; Fri, 28 Nov 2014 12:52:01 +0000 From: "Ananyev, Konstantin" To: "Zhang, Helin" , "dev@dpdk.org" Thread-Topic: [PATCH v7 3/4] i40e: support of controlling hash functions Thread-Index: AQHQCwThfe8p/c/bNkyTn6CgOWgM6Zx1/Q5w Date: Fri, 28 Nov 2014 12:52:01 +0000 Message-ID: <2601191342CEEE43887BDE71AB977258213BB322@IRSMSX105.ger.corp.intel.com> References: <1416409096-1340-1-git-send-email-helin.zhang@intel.com> <1417176852-12345-1-git-send-email-helin.zhang@intel.com> <1417176852-12345-4-git-send-email-helin.zhang@intel.com> In-Reply-To: <1417176852-12345-4-git-send-email-helin.zhang@intel.com> Accept-Language: en-IE, en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-originating-ip: [163.33.239.180] Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Subject: Re: [dpdk-dev] [PATCH v7 3/4] i40e: support of controlling hash functions 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: Fri, 28 Nov 2014 12:52:06 -0000 Hi Helin, Few nits from me. Konstantin > -----Original Message----- > From: Zhang, Helin > Sent: Friday, November 28, 2014 12:14 PM > To: dev@dpdk.org > Cc: Cao, Waterman; Cao, Min; Ananyev, Konstantin; Zhang, Helin > Subject: [PATCH v7 3/4] i40e: support of controlling hash functions >=20 > Hash filter control has been implemented for i40e. It includes > getting/setting, > - global hash configurations (hash function type, and symmetric > hash enable per flow type) > - symmetric hash enable per port >=20 > Signed-off-by: Helin Zhang > --- > lib/librte_ether/rte_eth_ctrl.h | 63 ++++++++ > lib/librte_pmd_i40e/i40e_ethdev.c | 298 ++++++++++++++++++++++++++++++++= +++++- > 2 files changed, 359 insertions(+), 2 deletions(-) >=20 > v5 changes: > * Integrated with filter API defined recently. >=20 > v6 changes: > * Implemented the mapping function to convert RSS offload types to > Packet Classification Types, to isolate the real hardware > specific things. > * Removed initialization of global registers in i40e PMD, as global > registers shouldn't be initialized per port. > * Added more annotations to get code more understandable. > * Corrected annotation format for documenation. >=20 > v7 changes: > * Remove swap configurations, as it is not allowed by hardware design. > * Put symmetric hash per flow type and hash function type into > 'RTE_ETH_HASH_FILTER_GLOBAL_CONFIG', as they are controlling global > registers which will affects all the ports of the same NIC. >=20 > diff --git a/lib/librte_ether/rte_eth_ctrl.h b/lib/librte_ether/rte_eth_c= trl.h > index 6ab16c2..827d7ba 100644 > --- a/lib/librte_ether/rte_eth_ctrl.h > +++ b/lib/librte_ether/rte_eth_ctrl.h > @@ -55,6 +55,7 @@ enum rte_filter_type { > RTE_ETH_FILTER_ETHERTYPE, > RTE_ETH_FILTER_TUNNEL, > RTE_ETH_FILTER_FDIR, > + RTE_ETH_FILTER_HASH, > RTE_ETH_FILTER_MAX > }; >=20 > @@ -449,6 +450,68 @@ struct rte_eth_fdir_stats { > uint32_t best_cnt; /**< Number of filters in best effort spaces. */ > }; >=20 > +/** > + * Hash filter information types. > + * - RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT is for getting/setting th= e > + * information/configuration of 'symmetric hash enable' per port. > + * - RTE_ETH_HASH_FILTER_GLOBAL_CONFIG is for getting/setting the global > + * configurations of hash filters. Those global configurations are val= id > + * for all ports of the same NIC. > + */ > +enum rte_eth_hash_filter_info_type { > + RTE_ETH_HASH_FILTER_INFO_TYPE_UNKNOWN =3D 0, > + /** Symmetric hash enable per port */ > + RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT, > + /** Configure globally for hash filter */ > + RTE_ETH_HASH_FILTER_GLOBAL_CONFIG, > + RTE_ETH_HASH_FILTER_INFO_TYPE_MAX, > +}; > + > +/** > + * Hash function types. > + */ > +enum rte_eth_hash_function { > + RTE_ETH_HASH_FUNCTION_DEFAULT =3D 0, > + RTE_ETH_HASH_FUNCTION_TOEPLITZ, /**< Toeplitz */ > + RTE_ETH_HASH_FUNCTION_SIMPLE_XOR, /**< Simple XOR */ > + RTE_ETH_HASH_FUNCTION_MAX, > +}; > + > +#define UINT32_BIT (CHAR_BIT * sizeof(uint32_t)) > +#define RTE_SYM_HASH_MASK_ARRAY_SIZE \ > + (RTE_ALIGN(RTE_ETH_FLOW_TYPE_MAX, UINT32_BIT)/UINT32_BIT) > +/** > + * A structure used to set or get global hash function configurations wh= ich > + * include symmetric hash enable per flow type and hash function type. > + * Each bit in sym_hash_enable_mask[] indicates if the symmetric hash of= the > + * coresponding flow type is enabled or not. > + * Each bit in valid_bit_mask[] indicates if the coresponding bit in > + * sym_hash_enable_mask[] is valid or not. For the configurations gotten= , it > + * also means if the flow type is supported by hardware or not. > + */ > +struct rte_eth_hash_global_conf { > + enum rte_eth_hash_function hash_func; /**< Hash function type */ > + /** Bit mask for symmetric hash enable per flow type */ > + uint32_t sym_hash_enable_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE]; > + /** Bit mask indicates if the coresponding bit is valid */ > + uint32_t valid_bit_mask[RTE_SYM_HASH_MASK_ARRAY_SIZE]; > +}; > + > +/** > + * A structure used to set or get hash filter information, to support fi= lter > + * 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_SYM_HASH_ENA_PER_PORT */ > + uint8_t enable; > + /* Global configurations of hash filter */ > + struct rte_eth_hash_global_conf global_conf; > + } info; > +}; > + > #ifdef __cplusplus > } > #endif > diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c b/lib/librte_pmd_i40e/i40e= _ethdev.c > index 8fbe25f..ef8edd4 100644 > --- a/lib/librte_pmd_i40e/i40e_ethdev.c > +++ b/lib/librte_pmd_i40e/i40e_ethdev.c > @@ -93,6 +93,18 @@ > I40E_PFINT_ICR0_ENA_VFLR_MASK | \ > I40E_PFINT_ICR0_ENA_ADMINQ_MASK) >=20 > +#define I40E_FLOW_TYPES ( \ > + (1UL << RTE_ETH_FLOW_TYPE_UDPV4) | \ > + (1UL << RTE_ETH_FLOW_TYPE_TCPV4) | \ > + (1UL << RTE_ETH_FLOW_TYPE_SCTPV4) | \ > + (1UL << RTE_ETH_FLOW_TYPE_IPV4_OTHER) | \ > + (1UL << RTE_ETH_FLOW_TYPE_FRAG_IPV4) | \ > + (1UL << RTE_ETH_FLOW_TYPE_UDPV6) | \ > + (1UL << RTE_ETH_FLOW_TYPE_TCPV6) | \ > + (1UL << RTE_ETH_FLOW_TYPE_SCTPV6) | \ > + (1UL << RTE_ETH_FLOW_TYPE_IPV6_OTHER) | \ > + (1UL << RTE_ETH_FLOW_TYPE_FRAG_IPV6)) > + > static int eth_i40e_dev_init(\ > __attribute__((unused)) struct eth_driver *eth_drv, > struct rte_eth_dev *eth_dev); > @@ -198,6 +210,7 @@ static int i40e_dev_filter_ctrl(struct rte_eth_dev *d= ev, > enum rte_filter_type filter_type, > enum rte_filter_op filter_op, > void *arg); > +static void i40e_hw_init(struct i40e_hw *hw); >=20 > static struct rte_pci_id pci_id_i40e_map[] =3D { > #define RTE_PCI_DEV_ID_DECL_I40E(vend, dev) {RTE_PCI_DEVICE(vend, dev)}, > @@ -397,6 +410,9 @@ eth_i40e_dev_init(__rte_unused struct eth_driver *eth= _drv, > /* Make sure all is clean before doing PF reset */ > i40e_clear_hw(hw); >=20 > + /* Initialize the hardware */ > + i40e_hw_init(hw); > + > /* Reset here to make sure all is clean for each PF */ > ret =3D i40e_pf_reset(hw); > if (ret) { > @@ -5120,6 +5136,264 @@ i40e_pf_config_mq_rx(struct i40e_pf *pf) > return ret; > } >=20 > +/* Get the symmetric hash enable configurations per port */ > +static void > +i40e_get_symmetric_hash_enable_per_port(struct i40e_hw *hw, uint8_t *ena= ble) > +{ > + uint32_t reg =3D I40E_READ_REG(hw, I40E_PRTQF_CTL_0); > + > + *enable =3D reg & I40E_PRTQF_CTL_0_HSYM_ENA_MASK ? 1 : 0; > +} > + > +/* Set the symmetric hash enable configurations per port */ > +static void > +i40e_set_symmetric_hash_enable_per_port(struct i40e_hw *hw, uint8_t enab= le) > +{ > + uint32_t reg =3D I40E_READ_REG(hw, I40E_PRTQF_CTL_0); > + > + if (enable > 0) { > + if (reg & I40E_PRTQF_CTL_0_HSYM_ENA_MASK) { > + PMD_DRV_LOG(INFO, "Symmetric hash has already " > + "been enabled"); > + return; > + } > + reg |=3D I40E_PRTQF_CTL_0_HSYM_ENA_MASK; > + } else { > + if (!(reg & I40E_PRTQF_CTL_0_HSYM_ENA_MASK)) { > + PMD_DRV_LOG(INFO, "Symmetric hash has already " > + "been disabled"); > + return; > + } > + reg &=3D ~I40E_PRTQF_CTL_0_HSYM_ENA_MASK; > + } > + I40E_WRITE_REG(hw, I40E_PRTQF_CTL_0, reg); > + I40E_WRITE_FLUSH(hw); > +} > + > +/* > + * Get global configurations of hash function type and symmetric hash en= able > + * per flow type (pctype). Note that global configuration means it affec= ts all > + * the ports on the same NIC. > + */ > +static int > +i40e_get_hash_filter_global_config(struct i40e_hw *hw, > + struct rte_eth_hash_global_conf *g_cfg) > +{ > + uint32_t reg, mask =3D I40E_FLOW_TYPES; > + enum rte_eth_flow_type i; > + enum i40e_filter_pctype pctype; > + > + memset(g_cfg, 0, sizeof(*g_cfg)); > + reg =3D I40E_READ_REG(hw, I40E_GLQF_CTL); > + if (reg & I40E_GLQF_CTL_HTOEP_MASK) > + g_cfg->hash_func =3D RTE_ETH_HASH_FUNCTION_TOEPLITZ; > + else > + g_cfg->hash_func =3D RTE_ETH_HASH_FUNCTION_SIMPLE_XOR; > + PMD_DRV_LOG(DEBUG, "Hash function is %s", > + (reg & I40E_GLQF_CTL_HTOEP_MASK) ? "Toeplitz" : "Simple XOR"); > + > + for (i =3D 0; mask && i < RTE_ETH_FLOW_TYPE_MAX; i++) { > + if (!(mask & (1UL << i))) > + continue; > + mask &=3D ~(1UL << i); > + /* Bit set indicats the coresponding flow type is supported */ > + g_cfg->valid_bit_mask[0] |=3D (1UL << i); > + pctype =3D i40e_flowtype_to_pctype(i); > + if (!pctype) > + continue; As I said in offline discussion, we don't need these 2 lines above: If i40e supports that flow type (bit in a mask is set), then we surely shou= ld be able to convert it to pctype. > + reg =3D I40E_READ_REG(hw, I40E_GLQF_HSYM(pctype)); > + if (reg & I40E_GLQF_HSYM_SYMH_ENA_MASK) > + g_cfg->sym_hash_enable_mask[0] |=3D (1UL << i); > + } > + > + return 0; > +} > + > +static int > +i40e_hash_global_config_check(struct rte_eth_hash_global_conf *g_cfg) > +{ > + uint32_t i; > + uint32_t mask0, i40e_mask =3D I40E_FLOW_TYPES; > + > + if (g_cfg->hash_func !=3D RTE_ETH_HASH_FUNCTION_TOEPLITZ && > + g_cfg->hash_func !=3D RTE_ETH_HASH_FUNCTION_SIMPLE_XOR && > + g_cfg->hash_func !=3D RTE_ETH_HASH_FUNCTION_DEFAULT) { > + PMD_DRV_LOG(ERR, "Unsupported hash function type %d", > + g_cfg->hash_func); > + return -EINVAL; > + } > + > + /* > + * As i40e supports less than 32 flow types, only first 32 bits need to > + * be checked. > + */ > + mask0 =3D g_cfg->valid_bit_mask[0]; > + for (i =3D 0; i < RTE_SYM_HASH_MASK_ARRAY_SIZE; i++) { > + if (i =3D=3D 0) { > + /* Check if any unsupported flow type configured */ > + if ((mask0 | i40e_mask) ^ i40e_mask) > + goto mask_err; > + } else { > + if (g_cfg->valid_bit_mask[i]) > + goto mask_err; > + } > + } > + > + return 0; > + > +mask_err: > + PMD_DRV_LOG(ERR, "i40e unsupported flow type bit(s) configured"); > + > + return -EINVAL; > +} > + > +/* > + * Set global configurations of hash function type and symmetric hash en= able > + * per flow type (pctype). Note any modifying global configuration will = affect > + * all the ports on the same NIC. > + */ > +static int > +i40e_set_hash_filter_global_config(struct i40e_hw *hw, > + struct rte_eth_hash_global_conf *g_cfg) > +{ > + int ret; > + uint32_t i, reg; > + uint32_t mask0 =3D g_cfg->valid_bit_mask[0]; > + enum i40e_filter_pctype pctype; > + > + /* Check the input parameters */ > + ret =3D i40e_hash_global_config_check(g_cfg); > + if (ret < 0) > + return ret; > + > + for (i =3D 0; mask0 && i < UINT32_BIT; i++) { > + if (!(mask0 & (1UL << i))) > + continue; > + mask0 &=3D ~(1UL << i); > + pctype =3D i40e_flowtype_to_pctype(i); > + if (!pctype) > + continue; As I said in offline discussion, we don't need these 2 lines above: If i40e supports that flow type (bit in a mask is set), then we surely shou= ld be able to convert it to pctype. > + reg =3D (g_cfg->sym_hash_enable_mask[0] & (1UL << i)) ? > + I40E_GLQF_HSYM_SYMH_ENA_MASK : 0; > + I40E_WRITE_REG(hw, I40E_GLQF_HSYM(pctype), reg); > + } > + > + reg =3D I40E_READ_REG(hw, I40E_GLQF_CTL); > + if (g_cfg->hash_func =3D=3D RTE_ETH_HASH_FUNCTION_TOEPLITZ) { > + /* Toeplitz */ > + if (reg & I40E_GLQF_CTL_HTOEP_MASK) { > + PMD_DRV_LOG(DEBUG, "Hash function already set to " > + "Toeplitz"); > + goto out; > + } > + reg |=3D I40E_GLQF_CTL_HTOEP_MASK; > + } else if (g_cfg->hash_func =3D=3D RTE_ETH_HASH_FUNCTION_SIMPLE_XOR) { > + /* Simple XOR */ > + if (!(reg & I40E_GLQF_CTL_HTOEP_MASK)) { > + PMD_DRV_LOG(DEBUG, "Hash function already set to " > + "Simple XOR"); > + goto out; > + } > + reg &=3D ~I40E_GLQF_CTL_HTOEP_MASK; > + } else > + /* Use the default, and keep it as it is */ > + goto out; > + > + I40E_WRITE_REG(hw, I40E_GLQF_CTL, reg); > + > +out: > + I40E_WRITE_FLUSH(hw); > + > + return 0; > +} > + > +static int > +i40e_hash_filter_get(struct i40e_hw *hw, struct rte_eth_hash_filter_info= *info) > +{ > + int ret =3D 0; > + > + if (!hw || !info) { > + PMD_DRV_LOG(ERR, "Invalid pointer"); > + return -EFAULT; > + } > + > + switch (info->info_type) { > + case RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT: > + i40e_get_symmetric_hash_enable_per_port(hw, > + &(info->info.enable)); > + break; > + case RTE_ETH_HASH_FILTER_GLOBAL_CONFIG: > + ret =3D i40e_get_hash_filter_global_config(hw, > + &(info->info.global_conf)); > + break; > + default: > + PMD_DRV_LOG(ERR, "Hash filter info type (%d) not supported", > + info->info_type); > + ret =3D -EINVAL; > + break; > + } > + > + return ret; > +} > + > +static int > +i40e_hash_filter_set(struct i40e_hw *hw, struct rte_eth_hash_filter_info= *info) > +{ > + int ret =3D 0; > + > + if (!hw || !info) { > + PMD_DRV_LOG(ERR, "Invalid pointer"); > + return -EFAULT; > + } > + > + switch (info->info_type) { > + case RTE_ETH_HASH_FILTER_SYM_HASH_ENA_PER_PORT: > + i40e_set_symmetric_hash_enable_per_port(hw, info->info.enable); > + break; > + case RTE_ETH_HASH_FILTER_GLOBAL_CONFIG: > + ret =3D i40e_set_hash_filter_global_config(hw, > + &(info->info.global_conf)); > + break; > + default: > + PMD_DRV_LOG(ERR, "Hash filter info type (%d) not supported", > + info->info_type); > + ret =3D -EINVAL; > + break; > + } > + > + return ret; > +} > + > +/* Operations for hash function */ > +static int > +i40e_hash_filter_ctrl(struct rte_eth_dev *dev, > + enum rte_filter_op filter_op, > + void *arg) > +{ > + struct i40e_hw *hw =3D I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private); > + int ret =3D 0; > + > + switch (filter_op) { > + case RTE_ETH_FILTER_NOP: > + break; > + case RTE_ETH_FILTER_GET: > + ret =3D i40e_hash_filter_get(hw, > + (struct rte_eth_hash_filter_info *)arg); > + break; > + case RTE_ETH_FILTER_SET: > + ret =3D i40e_hash_filter_set(hw, > + (struct rte_eth_hash_filter_info *)arg); > + break; > + default: > + PMD_DRV_LOG(WARNING, "Filter operation (%d) not supported", > + filter_op); > + ret =3D -ENOTSUP; > + break; > + } > + > + return ret; > +} > + > /* > * Configure ethertype filter, which can director packet by filtering > * with mac address and ether_type or only ether_type > @@ -5222,6 +5496,9 @@ i40e_dev_filter_ctrl(struct rte_eth_dev *dev, > return -EINVAL; >=20 > switch (filter_type) { > + case RTE_ETH_FILTER_HASH: > + ret =3D i40e_hash_filter_ctrl(dev, filter_op, arg); > + break; > case RTE_ETH_FILTER_MACVLAN: > ret =3D i40e_mac_filter_handle(dev, filter_op, arg); > break; > @@ -5244,10 +5521,26 @@ i40e_dev_filter_ctrl(struct rte_eth_dev *dev, > return ret; > } >=20 > +/* > + * As some registers wouldn't be reset unless a global hardware reset, > + * hardware initialization is needed to put those registers into an > + * expected initial state. > + */ > +static void > +i40e_hw_init(struct i40e_hw *hw) > +{ > + /* clear the PF Queue Filter control register */ > + I40E_WRITE_REG(hw, I40E_PFQF_CTL_0, 0); > + > + /* Disable symmetric hash per port */ > + i40e_set_symmetric_hash_enable_per_port(hw, 0); > +} > + > enum i40e_filter_pctype > i40e_flowtype_to_pctype(enum rte_eth_flow_type flow_type) > { > - static const enum i40e_filter_pctype pctype_table[] =3D { > + static const enum i40e_filter_pctype > + pctype_table[RTE_ETH_FLOW_TYPE_MAX] =3D { > [RTE_ETH_FLOW_TYPE_UDPV4] =3D I40E_FILTER_PCTYPE_NONF_IPV4_UDP, > [RTE_ETH_FLOW_TYPE_TCPV4] =3D I40E_FILTER_PCTYPE_NONF_IPV4_TCP, > [RTE_ETH_FLOW_TYPE_SCTPV4] =3D I40E_FILTER_PCTYPE_NONF_IPV4_SCTP, > @@ -5270,7 +5563,8 @@ i40e_flowtype_to_pctype(enum rte_eth_flow_type flow= _type) > enum rte_eth_flow_type > i40e_pctype_to_flowtype(enum i40e_filter_pctype pctype) > { > - static const enum rte_eth_flow_type flowtype_table[] =3D { > + static const enum rte_eth_flow_type > + flowtype_table[RTE_ETH_FLOW_TYPE_MAX] =3D { > [I40E_FILTER_PCTYPE_NONF_IPV4_UDP] =3D RTE_ETH_FLOW_TYPE_UDPV4, > [I40E_FILTER_PCTYPE_NONF_IPV4_TCP] =3D RTE_ETH_FLOW_TYPE_TCPV4, > [I40E_FILTER_PCTYPE_NONF_IPV4_SCTP] =3D RTE_ETH_FLOW_TYPE_SCTPV4, > -- > 1.8.1.4