Re: [dpdk-dev] [PATCH v4 4/7] i40e: add hash filter control implementation

["Zhang, Helin" <helin.zhang@intel.com>]

Hi Andrey

Yes, you are right. Actually I tried to rename those macros to more readable. For now, I'd like to put those renaming into another patches after all current feature patches merged. Thank you very much!

Regards,
Helin

> -----Original Message-----
> From: Chilikin, Andrey
> Sent: Monday, October 13, 2014 6:23 PM
> To: Zhang, Helin; dev@dpdk.org
> Subject: RE: [dpdk-dev] [PATCH v4 4/7] i40e: add hash filter control
> implementation
> 
> Hi Helin,
> 
> Should we define packet classification types separately and do not reuse bit
> shifts for RSS register as pctypes?
> Packet classification is a global index table which used by RSS Hash Enable
> registers, not vice versa.
> For example, there is no Packet classification named
> "ETH_RSS_NONF_IPV4_UDP_SHIFT" in Table 7-15 of XL710 Datasheet,  it is
> "PCTYPE_NONF_IPV4_UDP", so
> 
> switch (info->pctype) {
> 	case PCTYPE_NONF_IPV4_UDP:
> 	case PCTYPE_NONF_IPV4_TCP:
> 	...
> Is more clean and readable for me than
> 
> switch (info->pctype) {
> 	case ETH_RSS_NONF_IPV4_UDP_SHIFT:
> 	case ETH_RSS_NONF_IPV4_TCP_SHIFT:
> 
> We can rerefine ETH_RSS_* using PCTYPE_* though:
> 
> #define ETH_RSS_NONF_IPV4_UDP_SHIFT PCTYPE_NONF_IPV4_UDP and so
> one
> 
> Regards,
> Andrey
> 
> -----Original Message-----
> From: dev [mailto:dev-bounces@dpdk.org] On Behalf Of Helin Zhang
> Sent: Monday, October 13, 2014 7:13 AM
> To: dev@dpdk.org
> Subject: [dpdk-dev] [PATCH v4 4/7] i40e: add hash filter control implementation
> 
> Hash filter control has been implemented for i40e. It includes getting/setting
> - hash function type
> - symmetric hash enable per pctype (packet classification type)
> - symmetric hash enable per port
> - filter swap configuration
> 
> Signed-off-by: Helin Zhang <helin.zhang@intel.com>
> ---
>  lib/librte_pmd_i40e/i40e_ethdev.c | 402
> ++++++++++++++++++++++++++++++++++++++
>  1 file changed, 402 insertions(+)
> 
> diff --git a/lib/librte_pmd_i40e/i40e_ethdev.c
> b/lib/librte_pmd_i40e/i40e_ethdev.c
> index 46c43a7..60b619b 100644
> --- a/lib/librte_pmd_i40e/i40e_ethdev.c
> +++ b/lib/librte_pmd_i40e/i40e_ethdev.c
> @@ -216,6 +216,10 @@ static int i40e_dev_rss_hash_update(struct
> rte_eth_dev *dev,
>  				    struct rte_eth_rss_conf *rss_conf);  static int
> i40e_dev_rss_hash_conf_get(struct rte_eth_dev *dev,
>  				      struct rte_eth_rss_conf *rss_conf);
> +static int i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
> +				enum rte_filter_type filter_type,
> +				enum rte_filter_op filter_op,
> +				void *arg);
> 
>  /* Default hash key buffer for RSS */
>  static uint32_t rss_key_default[I40E_PFQF_HKEY_MAX_INDEX + 1]; @@
> -267,6 +271,7 @@ static struct eth_dev_ops i40e_eth_dev_ops = {
>  	.reta_query                   = i40e_dev_rss_reta_query,
>  	.rss_hash_update              = i40e_dev_rss_hash_update,
>  	.rss_hash_conf_get            = i40e_dev_rss_hash_conf_get,
> +	.filter_ctrl                  = i40e_dev_filter_ctrl,
>  };
> 
>  static struct eth_driver rte_i40e_pmd = { @@ -4162,3 +4167,400 @@
> i40e_pf_config_mq_rx(struct i40e_pf *pf)
> 
>  	return 0;
>  }
> +
> +/* Get the symmetric hash enable configurations per PCTYPE */ static
> +int i40e_get_symmetric_hash_enable_per_pctype(struct i40e_hw *hw,
> +			struct rte_eth_sym_hash_ena_info *info) {
> +	uint32_t reg;
> +
> +	switch (info->pctype) {
> +	case ETH_RSS_NONF_IPV4_UDP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_TCP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_SCTP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_OTHER_SHIFT:
> +	case ETH_RSS_FRAG_IPV4_SHIFT:
> +	case ETH_RSS_NONF_IPV6_UDP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_TCP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_SCTP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_OTHER_SHIFT:
> +	case ETH_RSS_FRAG_IPV6_SHIFT:
> +	case ETH_RSS_L2_PAYLOAD_SHIFT:
> +		reg = I40E_READ_REG(hw, I40E_GLQF_HSYM(info->pctype));
> +		info->enable = reg & I40E_GLQF_HSYM_SYMH_ENA_MASK ? 1 : 0;
> +		break;
> +	default:
> +		PMD_DRV_LOG(ERR, "PCTYPE[%u] not supported", info->pctype);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Set the symmetric hash enable configurations per PCTYPE */ static
> +int i40e_set_symmetric_hash_enable_per_pctype(struct i40e_hw *hw,
> +		const struct rte_eth_sym_hash_ena_info *info) {
> +	uint32_t reg;
> +
> +	switch (info->pctype) {
> +	case ETH_RSS_NONF_IPV4_UDP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_TCP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_SCTP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_OTHER_SHIFT:
> +	case ETH_RSS_FRAG_IPV4_SHIFT:
> +	case ETH_RSS_NONF_IPV6_UDP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_TCP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_SCTP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_OTHER_SHIFT:
> +	case ETH_RSS_FRAG_IPV6_SHIFT:
> +	case ETH_RSS_L2_PAYLOAD_SHIFT:
> +		reg = info->enable ? I40E_GLQF_HSYM_SYMH_ENA_MASK : 0;
> +		I40E_WRITE_REG(hw, I40E_GLQF_HSYM(info->pctype), reg);
> +		I40E_WRITE_FLUSH(hw);
> +		break;
> +	default:
> +		PMD_DRV_LOG(ERR, "PCTYPE[%u] not supported", info->pctype);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Get the symmetric hash enable configurations per port */ static void
> +i40e_get_symmetric_hash_enable_per_port(struct i40e_hw *hw, uint8_t
> +*enable) {
> +	uint32_t reg = I40E_READ_REG(hw, I40E_PRTQF_CTL_0);
> +
> +	*enable = 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
> +enable) {
> +	uint32_t reg = 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 |= 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 &= ~I40E_PRTQF_CTL_0_HSYM_ENA_MASK;
> +	}
> +	I40E_WRITE_REG(hw, I40E_PRTQF_CTL_0, reg);
> +	I40E_WRITE_FLUSH(hw);
> +}
> +
> +/* Get filter swap configurations */
> +static int
> +i40e_get_filter_swap(struct i40e_hw *hw, struct
> +rte_eth_filter_swap_info *info) {
> +	uint32_t reg;
> +
> +	switch (info->pctype) {
> +	case ETH_RSS_NONF_IPV4_UDP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_TCP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_SCTP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_OTHER_SHIFT:
> +	case ETH_RSS_FRAG_IPV4_SHIFT:
> +	case ETH_RSS_NONF_IPV6_UDP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_TCP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_SCTP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_OTHER_SHIFT:
> +	case ETH_RSS_FRAG_IPV6_SHIFT:
> +	case ETH_RSS_L2_PAYLOAD_SHIFT:
> +		reg = I40E_READ_REG(hw, I40E_GLQF_SWAP(0, info->pctype));
> +		PMD_DRV_LOG(DEBUG, "Value read from I40E_GLQF_SWAP[0,%d]: "
> +						"0x%x", info->pctype, reg);
> +
> +		/**
> +		 * The offset and length read from register in word unit,
> +		 * which need to be converted in byte unit before being saved.
> +		 */
> +		info->off0_src0 =
> +			(uint8_t)((reg & I40E_GLQF_SWAP_OFF0_SRC0_MASK) >>
> +					I40E_GLQF_SWAP_OFF0_SRC0_SHIFT) << 1;
> +		info->off0_src1 =
> +			(uint8_t)((reg & I40E_GLQF_SWAP_OFF0_SRC1_MASK) >>
> +					I40E_GLQF_SWAP_OFF0_SRC1_SHIFT) << 1;
> +		info->len0 = (uint8_t)((reg & I40E_GLQF_SWAP_FLEN0_MASK) >>
> +					I40E_GLQF_SWAP_FLEN0_SHIFT) << 1;
> +		info->off1_src0 =
> +			(uint8_t)((reg & I40E_GLQF_SWAP_OFF1_SRC0_MASK) >>
> +					I40E_GLQF_SWAP_OFF1_SRC0_SHIFT) << 1;
> +		info->off1_src1 =
> +			(uint8_t)((reg & I40E_GLQF_SWAP_OFF1_SRC1_MASK) >>
> +					I40E_GLQF_SWAP_OFF1_SRC1_SHIFT) << 1;
> +		info->len1 = (uint8_t)((reg & I40E_GLQF_SWAP_FLEN1_MASK) >>
> +					I40E_GLQF_SWAP_FLEN1_SHIFT) << 1;
> +		break;
> +	default:
> +		PMD_DRV_LOG(ERR, "PCTYPE[%u] not supported", info->pctype);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Set filter swap configurations */
> +static int
> +i40e_set_filter_swap(struct i40e_hw *hw,
> +		     const struct rte_eth_filter_swap_info *info) { #define
> +I40E_FIELD_LEN_MAX 0x1f #define I40E_FIELD_OFFSET_MAX 0x7f
> +	uint32_t reg;
> +
> +	switch (info->pctype) {
> +	case ETH_RSS_NONF_IPV4_UDP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_TCP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_SCTP_SHIFT:
> +	case ETH_RSS_NONF_IPV4_OTHER_SHIFT:
> +	case ETH_RSS_FRAG_IPV4_SHIFT:
> +	case ETH_RSS_NONF_IPV6_UDP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_TCP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_SCTP_SHIFT:
> +	case ETH_RSS_NONF_IPV6_OTHER_SHIFT:
> +	case ETH_RSS_FRAG_IPV6_SHIFT:
> +	case ETH_RSS_L2_PAYLOAD_SHIFT:
> +		if (info->off0_src0 > I40E_FIELD_OFFSET_MAX) {
> +			PMD_DRV_LOG(ERR, "off0_src0 (0x%x) exceeds the "
> +				"maximum of 0x%x", info->off0_src0,
> +						I40E_FIELD_OFFSET_MAX);
> +			return I40E_ERR_PARAM;
> +		} else if (info->off0_src1 > I40E_FIELD_OFFSET_MAX) {
> +			PMD_DRV_LOG(ERR, "off0_src1 (0x%x) exceeds the "
> +				"maximum of 0x%x", info->off0_src1,
> +						I40E_FIELD_OFFSET_MAX);
> +			return I40E_ERR_PARAM;
> +		} else if (info->len0 > I40E_FIELD_LEN_MAX) {
> +			PMD_DRV_LOG(ERR, "len0 (0x%x) exceeds the maximum "
> +				"of 0x%x", info->len0, I40E_FIELD_LEN_MAX);
> +			return I40E_ERR_PARAM;
> +		} else if (info->off1_src0 > I40E_FIELD_OFFSET_MAX) {
> +			PMD_DRV_LOG(ERR, "off1_src0 (0x%x) exceeds the "
> +				"maximum of 0x%x", info->off1_src0,
> +						I40E_FIELD_OFFSET_MAX);
> +			return I40E_ERR_PARAM;
> +		} else if (info->off1_src1 > I40E_FIELD_OFFSET_MAX) {
> +			PMD_DRV_LOG(ERR, "off1_src1 (0x%x) exceeds the "
> +				"maximum of 0x%x", info->off1_src1,
> +						I40E_FIELD_OFFSET_MAX);
> +			return I40E_ERR_PARAM;
> +		} else if (info->len1 > I40E_FIELD_LEN_MAX) {
> +			PMD_DRV_LOG(ERR, "len1 (0x%x) exceeds the maximum "
> +				"of 0x%x", info->len1, I40E_FIELD_LEN_MAX);
> +			return I40E_ERR_PARAM;
> +		}
> +
> +		/**
> +		 * The offset and length given in byte unit, which need to be
> +		 * converted in word unit before being written to the register,
> +		 * as hardware requires it in word unit.
> +		 */
> +		reg = (info->off0_src0 >> 1) << I40E_GLQF_SWAP_OFF0_SRC0_SHIFT;
> +		reg |= (info->off0_src1 >> 1) <<
> +			I40E_GLQF_SWAP_OFF0_SRC1_SHIFT;
> +		reg |= (info->len0 >> 1) << I40E_GLQF_SWAP_FLEN0_SHIFT;
> +		reg |= (info->off1_src0 >> 1) <<
> +			I40E_GLQF_SWAP_OFF1_SRC0_SHIFT;
> +		reg |= (info->off1_src1 >> 1) <<
> +			I40E_GLQF_SWAP_OFF1_SRC1_SHIFT;
> +		reg |= (info->len1 >> 1) << I40E_GLQF_SWAP_FLEN1_SHIFT;
> +
> +		PMD_DRV_LOG(DEBUG, "Value to be written to "
> +			"I40E_GLQF_SWAP[0,%d]: 0x%x", info->pctype, reg);
> +		I40E_WRITE_REG(hw, I40E_GLQF_SWAP(0, info->pctype), reg);
> +		I40E_WRITE_FLUSH(hw);
> +		break;
> +	default:
> +		PMD_DRV_LOG(ERR, "PCTYPE[%u] not supported", info->pctype);
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
> +/* Get hash function type */
> +static void
> +i40e_get_hash_function(struct i40e_hw *hw, enum rte_eth_hash_function
> +*hf) {
> +	uint32_t reg = I40E_READ_REG(hw, I40E_GLQF_CTL);
> +
> +	if (reg & I40E_GLQF_CTL_HTOEP_MASK)
> +		*hf = RTE_ETH_HASH_FUNCTION_TOEPLITZ;
> +	else
> +		*hf = RTE_ETH_HASH_FUNCTION_SIMPLE_XOR;
> +
> +	PMD_DRV_LOG(INFO, "Hash function is %s",
> +		(reg & I40E_GLQF_CTL_HTOEP_MASK) ? "Toeplitz" : "Simple XOR"); }
> +
> +/* Set hash function type */
> +static int
> +i40e_set_hash_function(struct i40e_hw *hw, enum rte_eth_hash_function
> +hf) {
> +	uint32_t reg = I40E_READ_REG(hw, I40E_GLQF_CTL);
> +
> +	if (hf == RTE_ETH_HASH_FUNCTION_TOEPLITZ) {
> +		if (reg & I40E_GLQF_CTL_HTOEP_MASK) {
> +			PMD_DRV_LOG(DEBUG, "Hash function already set to "
> +								"Toeplitz");
> +			return 0;
> +		}
> +		reg |= I40E_GLQF_CTL_HTOEP_MASK;
> +	} else if (hf == RTE_ETH_HASH_FUNCTION_SIMPLE_XOR) {
> +		if (!(reg & I40E_GLQF_CTL_HTOEP_MASK)) {
> +			PMD_DRV_LOG(DEBUG, "Hash function already set to "
> +							"Simple XOR");
> +			return 0;
> +		}
> +		reg &= ~I40E_GLQF_CTL_HTOEP_MASK;
> +	} else {
> +		PMD_DRV_LOG(ERR, "Unknown hash function type");
> +		return -EINVAL;
> +	}
> +
> +	PMD_DRV_LOG(INFO, "Hash function set to %s",
> +		(reg & I40E_GLQF_CTL_HTOEP_MASK) ? "Toeplitz" : "Simple XOR");
> +	I40E_WRITE_REG(hw, I40E_GLQF_CTL, reg);
> +	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 = 0;
> +
> +	if (!hw || !info) {
> +		PMD_DRV_LOG(ERR, "Invalid pointer");
> +		return -EFAULT;
> +	}
> +
> +	switch (info->info_type) {
> +	case RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PCTYPE:
> +		ret = i40e_get_symmetric_hash_enable_per_pctype(hw,
> +					&(info->info.sym_hash_ena));
> +		break;
> +	case RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PORT:
> +		i40e_get_symmetric_hash_enable_per_port(hw,
> +					&(info->info.enable));
> +		break;
> +	case RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP:
> +		ret = i40e_get_filter_swap(hw, &(info->info.filter_swap));
> +		break;
> +	case RTE_ETH_HASH_FILTER_INFO_TYPE_HASH_FUNCTION:
> +		i40e_get_hash_function(hw, &(info->info.hash_function));
> +		break;
> +	default:
> +		PMD_DRV_LOG(ERR, "Hash filter info type (%d) not supported",
> +							info->info_type);
> +		ret = -EINVAL;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int
> +i40e_hash_filter_set(struct i40e_hw *hw, struct
> +rte_eth_hash_filter_info *info) {
> +	int ret = 0;
> +
> +	if (!hw || !info) {
> +		PMD_DRV_LOG(ERR, "Invalid pointer");
> +		return -EFAULT;
> +	}
> +
> +	switch (info->info_type) {
> +	case RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PCTYPE:
> +		ret = i40e_set_symmetric_hash_enable_per_pctype(hw,
> +					&(info->info.sym_hash_ena));
> +		break;
> +	case RTE_ETH_HASH_FILTER_INFO_TYPE_SYM_HASH_ENA_PER_PORT:
> +		i40e_set_symmetric_hash_enable_per_port(hw, info->info.enable);
> +		break;
> +	case RTE_ETH_HASH_FILTER_INFO_TYPE_FILTER_SWAP:
> +		ret = i40e_set_filter_swap(hw, &(info->info.filter_swap));
> +		break;
> +	case RTE_ETH_HASH_FILTER_INFO_TYPE_HASH_FUNCTION:
> +		ret = i40e_set_hash_function(hw, info->info.hash_function);
> +		break;
> +	default:
> +		PMD_DRV_LOG(ERR, "Hash filter info type (%d) not supported",
> +							info->info_type);
> +		ret = -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 =
> I40E_DEV_PRIVATE_TO_HW(dev->data->dev_private);
> +	int ret = 0;
> +
> +	switch (filter_op) {
> +	case RTE_ETH_FILTER_OP_NONE:
> +		break;
> +	case RTE_ETH_FILTER_OP_GET:
> +		ret = i40e_hash_filter_get(hw,
> +			(struct rte_eth_hash_filter_info *)arg);
> +		break;
> +	case RTE_ETH_FILTER_OP_SET:
> +		ret = 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 = -ENOTSUP;
> +		break;
> +	}
> +
> +	return ret;
> +}
> +
> +static int
> +i40e_dev_filter_ctrl(struct rte_eth_dev *dev,
> +		     enum rte_filter_type filter_type,
> +		     enum rte_filter_op filter_op,
> +		     void *arg)
> +{
> +	int ret = -ENOTSUP;
> +
> +	switch (filter_type) {
> +	case RTE_ETH_FILTER_HASH:
> +		ret = i40e_hash_filter_ctrl(dev, filter_op, arg);
> +		break;
> +	case RTE_ETH_FILTER_FDIR:
> +		break;
> +	case RTE_ETH_FILTER_TUNNEL:
> +		break;
> +	default:
> +		PMD_DRV_LOG(WARNING, "Filter type (%d) not supported",
> +							filter_type);
> +		break;
> +	}
> +
> +	return ret;
> +}
> --
> 1.8.1.4