[dpdk-dev,2/4] i40e: split function for input set change of hash and fdir

Message ID 1451032200-24973-3-git-send-email-jingjing.wu@intel.com (mailing list archive)
State Superseded, archived
Headers

Commit Message

Jingjing Wu Dec. 25, 2015, 8:29 a.m. UTC
  This patch splited function for input set change of hash and fdir,
and added a new function to set the input set to default when
initialization.

Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
---
 drivers/net/i40e/i40e_ethdev.c | 330 +++++++++++++++++++++--------------------
 drivers/net/i40e/i40e_ethdev.h |  11 +-
 drivers/net/i40e/i40e_fdir.c   |   5 +-
 3 files changed, 180 insertions(+), 166 deletions(-)
  

Comments

Chilikin, Andrey Jan. 20, 2016, 8:04 p.m. UTC | #1
Hi Jingjing,

As I can see this patch not only splits fdir functionality from common fdir/hash code but also removes compatibility with DPDK 2.2 as it deletes I40E_INSET_FLEX_PAYLOAD from valid fdir input set values. Yes, flexible payload configuration can be set for fdir separately at the port initialization, but this is more legacy from the previous generations of NICs which did not support dynamic input set configuration. I believe it would better to have I40E_INSET_FLEX_PAYLOAD valid for fdir input set same as in DPDK 2.2. So in legacy mode, when application has to run on an old NIC and on a new one, only legacy configuration would be used, but for applications targeting new HW single point of configuration would be used instead of mix of two.

Regards,
Andrey

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Friday, December 25, 2015 8:30 AM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Zhang, Helin; Chilikin, Andrey; Pei, Yulong
> Subject: [PATCH 2/4] i40e: split function for input set change of hash and fdir
> 
> This patch splited function for input set change of hash and fdir, and added a
> new function to set the input set to default when initialization.
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 330 +++++++++++++++++++++--------------------
>  drivers/net/i40e/i40e_ethdev.h |  11 +-
>  drivers/net/i40e/i40e_fdir.c   |   5 +-
>  3 files changed, 180 insertions(+), 166 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index bf6220d..b919aac 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -262,7 +262,8 @@
>  #define I40E_REG_INSET_FLEX_PAYLOAD_WORD7
> 0x0000000000000080ULL
>  /* 8th word of flex payload */
>  #define I40E_REG_INSET_FLEX_PAYLOAD_WORD8
> 0x0000000000000040ULL
> -
> +/* all 8 words flex payload */
> +#define I40E_REG_INSET_FLEX_PAYLOAD_WORDS
> 0x0000000000003FC0ULL
>  #define I40E_REG_INSET_MASK_DEFAULT              0x0000000000000000ULL
> 
>  #define I40E_TRANSLATE_INSET 0
> @@ -373,6 +374,7 @@ static int i40e_dev_udp_tunnel_add(struct rte_eth_dev
> *dev,
>  				struct rte_eth_udp_tunnel *udp_tunnel);  static
> int i40e_dev_udp_tunnel_del(struct rte_eth_dev *dev,
>  				struct rte_eth_udp_tunnel *udp_tunnel);
> +static void i40e_filter_input_set_init(struct i40e_pf *pf);
>  static int i40e_ethertype_filter_set(struct i40e_pf *pf,
>  			struct rte_eth_ethertype_filter *filter,
>  			bool add);
> @@ -787,6 +789,8 @@ eth_i40e_dev_init(struct rte_eth_dev *dev)
>  	 * It should be removed once issues are fixed in NVM.
>  	 */
>  	i40e_flex_payload_reg_init(hw);
> +	/* Initialize the input set for filters (hash and fd) to default value */
> +	i40e_filter_input_set_init(pf);
> 
>  	/* Initialize the parameters for adminq */
>  	i40e_init_adminq_parameter(hw);
> @@ -6545,43 +6549,32 @@ i40e_get_valid_input_set(enum i40e_filter_pctype
> pctype,
>  	 */
>  	static const uint64_t valid_fdir_inset_table[] = {
>  		[I40E_FILTER_PCTYPE_FRAG_IPV4] =
> -		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST |
> -		I40E_INSET_FLEX_PAYLOAD,
> +		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST,
>  		[I40E_FILTER_PCTYPE_NONF_IPV4_UDP] =
>  		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST |
> -		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
> -		I40E_INSET_FLEX_PAYLOAD,
> +		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT,
>  		[I40E_FILTER_PCTYPE_NONF_IPV4_TCP] =
> -		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST |
> -		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
> -		I40E_INSET_FLEX_PAYLOAD,
> +		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST,
>  		[I40E_FILTER_PCTYPE_NONF_IPV4_SCTP] =
>  		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST |
>  		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
> -		I40E_INSET_SCTP_VT | I40E_INSET_FLEX_PAYLOAD,
> +		I40E_INSET_SCTP_VT,
>  		[I40E_FILTER_PCTYPE_NONF_IPV4_OTHER] =
> -		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST |
> -		I40E_INSET_FLEX_PAYLOAD,
> +		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST,
>  		[I40E_FILTER_PCTYPE_FRAG_IPV6] =
> -		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST |
> -		I40E_INSET_FLEX_PAYLOAD,
> +		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST,
>  		[I40E_FILTER_PCTYPE_NONF_IPV6_UDP] =
> -		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST |
> -		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
> -		I40E_INSET_FLEX_PAYLOAD,
> +		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST,
>  		[I40E_FILTER_PCTYPE_NONF_IPV6_TCP] =
> -		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST |
> -		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
> -		I40E_INSET_FLEX_PAYLOAD,
> +		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST,
>  		[I40E_FILTER_PCTYPE_NONF_IPV6_SCTP] =
>  		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST |
>  		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
> -		I40E_INSET_SCTP_VT | I40E_INSET_FLEX_PAYLOAD,
> +		I40E_INSET_SCTP_VT,
>  		[I40E_FILTER_PCTYPE_NONF_IPV6_OTHER] =
> -		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST |
> -		I40E_INSET_FLEX_PAYLOAD,
> +		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST,
>  		[I40E_FILTER_PCTYPE_L2_PAYLOAD] =
> -		I40E_INSET_LAST_ETHER_TYPE | I40E_INSET_FLEX_PAYLOAD,
> +		I40E_INSET_LAST_ETHER_TYPE,
>  	};
> 
>  	if (pctype > I40E_FILTER_PCTYPE_L2_PAYLOAD) @@ -6809,7 +6802,7
> @@ i40e_translate_input_set_reg(uint64_t input)
>  	return val;
>  }
> 
> -static uint8_t
> +static int
>  i40e_generate_inset_mask_reg(uint64_t inset, uint32_t *mask, uint8_t
> nb_elem)  {
>  	uint8_t i, idx = 0;
> @@ -6827,16 +6820,13 @@ i40e_generate_inset_mask_reg(uint64_t inset,
> uint32_t *mask, uint8_t nb_elem)
>  	if (!inset || !mask || !nb_elem)
>  		return 0;
> 
> -	if (!inset && nb_elem >= I40E_INSET_MASK_NUM_REG) {
> -		for (i = 0; i < I40E_INSET_MASK_NUM_REG; i++)
> -			mask[i] = 0;
> -		return I40E_INSET_MASK_NUM_REG;
> -	}
> 
>  	for (i = 0, idx = 0; i < RTE_DIM(inset_mask_map); i++) {
> -		if (idx >= nb_elem)
> -			break;
> -		if (inset & inset_mask_map[i].inset) {
> +		if ((inset & inset_mask_map[i].inset) ==
> inset_mask_map[i].inset) {
> +			if (idx >= nb_elem) {
> +				PMD_DRV_LOG(ERR, "exceed maximal
> number of bitmasks");
> +				return -EINVAL;
> +			}
>  			mask[idx] = inset_mask_map[i].mask;
>  			idx++;
>  		}
> @@ -6845,25 +6835,6 @@ i40e_generate_inset_mask_reg(uint64_t inset,
> uint32_t *mask, uint8_t nb_elem)
>  	return idx;
>  }
> 
> -static uint64_t
> -i40e_get_reg_inset(struct i40e_hw *hw, enum rte_filter_type filter,
> -			    enum i40e_filter_pctype pctype)
> -{
> -	uint64_t reg = 0;
> -
> -	if (filter == RTE_ETH_FILTER_HASH) {
> -		reg = I40E_READ_REG(hw, I40E_GLQF_HASH_INSET(1,
> pctype));
> -		reg <<= I40E_32_BIT_WIDTH;
> -		reg |= I40E_READ_REG(hw, I40E_GLQF_HASH_INSET(0,
> pctype));
> -	} else if (filter == RTE_ETH_FILTER_FDIR) {
> -		reg = I40E_READ_REG(hw, I40E_PRTQF_FD_INSET(pctype, 1));
> -		reg <<= I40E_32_BIT_WIDTH;
> -		reg |= I40E_READ_REG(hw, I40E_PRTQF_FD_INSET(pctype, 0));
> -	}
> -
> -	return reg;
> -}
> -
>  static void
>  i40e_check_write_reg(struct i40e_hw *hw, uint32_t addr, uint32_t val)  { @@
> -6876,103 +6847,149 @@ i40e_check_write_reg(struct i40e_hw *hw, uint32_t
> addr, uint32_t val)
>  		    (uint32_t)I40E_READ_REG(hw, addr));  }
> 
> -static int
> -i40e_set_hash_inset_mask(struct i40e_hw *hw,
> -			 enum i40e_filter_pctype pctype,
> -			 enum rte_filter_input_set_op op,
> -			 uint32_t *mask_reg,
> -			 uint8_t num)
> +static void
> +i40e_filter_input_set_init(struct i40e_pf *pf)
>  {
> -	uint32_t reg;
> -	uint8_t i;
> +	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
> +	enum i40e_filter_pctype pctype;
> +	uint64_t input_set, inset_reg;
> +	uint32_t mask_reg[I40E_INSET_MASK_NUM_REG] = {0};
> +	int num, i;
> 
> -	if (!mask_reg || num > RTE_ETH_INPUT_SET_SELECT)
> -		return -EINVAL;
> +	for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
> +	     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++) {
> +		if (!I40E_VALID_PCTYPE(pctype))
> +			continue;
> +		input_set = i40e_get_default_input_set(pctype);
> 
> -	if (op == RTE_ETH_INPUT_SET_SELECT) {
> -		for (i = 0; i < I40E_INSET_MASK_NUM_REG; i++) {
> -			i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i,
> pctype),
> -					     0);
> -			if (i >= num)
> -				continue;
> +		num = i40e_generate_inset_mask_reg(input_set, mask_reg,
> +
> I40E_INSET_MASK_NUM_REG);
> +		if (num < 0)
> +			return;
> +		inset_reg = i40e_translate_input_set_reg(input_set);
> +
> +		i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 0),
> +				      (uint32_t)(inset_reg & UINT32_MAX));
> +		i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 1),
> +				     (uint32_t)((inset_reg >>
> +				     I40E_32_BIT_WIDTH) & UINT32_MAX));
> +		i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(0, pctype),
> +				      (uint32_t)(inset_reg & UINT32_MAX));
> +		i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(1, pctype),
> +				     (uint32_t)((inset_reg >>
> +				     I40E_32_BIT_WIDTH) & UINT32_MAX));
> +
> +		for (i = 0; i < num; i++) {
> +			i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i,
> pctype),
> +					     mask_reg[i]);
>  			i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i,
> pctype),
>  					     mask_reg[i]);
>  		}
> -	} else if (op == RTE_ETH_INPUT_SET_ADD) {
> -		uint8_t j, count = 0;
> -
> -		for (i = 0; i < I40E_INSET_MASK_NUM_REG; i++) {
> -			reg = I40E_READ_REG(hw, I40E_GLQF_HASH_MSK(i,
> pctype));
> -			if (reg & I40E_GLQF_HASH_MSK_FIELD)
> -				count++;
> +		/*clear unused mask registers of the pctype */
> +		for (i = num; i < I40E_INSET_MASK_NUM_REG; i++) {
> +			i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i,
> pctype),
> +					     0);
> +			i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i,
> pctype),
> +					     0);
>  		}
> -		if (count + num > I40E_INSET_MASK_NUM_REG)
> -			return -EINVAL;
> +		I40E_WRITE_FLUSH(hw);
> 
> -		for (i = count, j = 0; i < I40E_INSET_MASK_NUM_REG; i++, j++)
> -			i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i,
> pctype),
> -					     mask_reg[j]);
> +		/* store the default input set */
> +		pf->hash_input_set[pctype] = input_set;
> +		pf->fdir.input_set[pctype] = input_set;
>  	}
> -
> -	return 0;
>  }
> 
> -static int
> -i40e_set_fd_inset_mask(struct i40e_hw *hw,
> -		       enum i40e_filter_pctype pctype,
> -		       enum rte_filter_input_set_op op,
> -		       uint32_t *mask_reg,
> -		       uint8_t num)
> +int
> +i40e_hash_filter_inset_select(struct i40e_hw *hw,
> +			 struct rte_eth_input_set_conf *conf)
>  {
> -	uint32_t reg;
> -	uint8_t i;
> +	struct i40e_pf *pf = &((struct i40e_adapter *)hw->back)->pf;
> +	enum i40e_filter_pctype pctype;
> +	uint64_t input_set, inset_reg = 0;
> +	uint32_t mask_reg[I40E_INSET_MASK_NUM_REG] = {0};
> +	int ret, i, num;
> 
> -	if (!mask_reg || num > RTE_ETH_INPUT_SET_SELECT)
> +	if (!hw || !conf) {
> +		PMD_DRV_LOG(ERR, "Invalid pointer");
> +		return -EFAULT;
> +	}
> +	if (conf->op != RTE_ETH_INPUT_SET_SELECT &&
> +	    conf->op != RTE_ETH_INPUT_SET_ADD) {
> +		PMD_DRV_LOG(ERR, "Unsupported input set operation");
>  		return -EINVAL;
> +	}
> 
> -	if (op == RTE_ETH_INPUT_SET_SELECT) {
> -		for (i = 0; i < I40E_INSET_MASK_NUM_REG; i++) {
> -			i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i,
> pctype),
> -					     0);
> -			if (i >= num)
> -				continue;
> -			i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i,
> pctype),
> -					     mask_reg[i]);
> -		}
> -	} else if (op == RTE_ETH_INPUT_SET_ADD) {
> -		uint8_t j, count = 0;
> -
> -		for (i = 0; i < I40E_INSET_MASK_NUM_REG; i++) {
> -			reg = I40E_READ_REG(hw, I40E_GLQF_FD_MSK(i,
> pctype));
> -			if (reg & I40E_GLQF_FD_MSK_FIELD)
> -				count++;
> -		}
> -		if (count + num > I40E_INSET_MASK_NUM_REG)
> -			return -EINVAL;
> +	pctype = i40e_flowtype_to_pctype(conf->flow_type);
> +	if (pctype == 0 || pctype > I40E_FILTER_PCTYPE_L2_PAYLOAD) {
> +		PMD_DRV_LOG(ERR, "Not supported flow type (%u)",
> +			    conf->flow_type);
> +		return -EINVAL;
> +	}
> 
> -		for (i = count, j = 0; i < I40E_INSET_MASK_NUM_REG; i++, j++)
> -			i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i,
> pctype),
> -					     mask_reg[j]);
> +	ret = i40e_parse_input_set(&input_set, pctype, conf->field,
> +				   conf->inset_size);
> +	if (ret) {
> +		PMD_DRV_LOG(ERR, "Failed to parse input set");
> +		return -EINVAL;
>  	}
> +	if (i40e_validate_input_set(pctype, RTE_ETH_FILTER_HASH,
> +				    input_set) != 0) {
> +		PMD_DRV_LOG(ERR, "Invalid input set");
> +		return -EINVAL;
> +	}
> +	if (conf->op == RTE_ETH_INPUT_SET_ADD) {
> +		/* get inset value in register */
> +		inset_reg = I40E_READ_REG(hw, I40E_GLQF_HASH_INSET(1,
> pctype));
> +		inset_reg <<= I40E_32_BIT_WIDTH;
> +		inset_reg |= I40E_READ_REG(hw, I40E_GLQF_HASH_INSET(0,
> pctype));
> +		input_set |= pf->hash_input_set[pctype];
> +	}
> +	num = i40e_generate_inset_mask_reg(input_set, mask_reg,
> +					   I40E_INSET_MASK_NUM_REG);
> +	if (num < 0)
> +		return -EINVAL;
> +
> +	inset_reg |= i40e_translate_input_set_reg(input_set);
> 
> +	i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(0, pctype),
> +			      (uint32_t)(inset_reg & UINT32_MAX));
> +	i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(1, pctype),
> +			     (uint32_t)((inset_reg >>
> +			     I40E_32_BIT_WIDTH) & UINT32_MAX));
> +
> +	for (i = 0; i < num; i++)
> +		i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i, pctype),
> +				     mask_reg[i]);
> +	/*clear unused mask registers of the pctype */
> +	for (i = num; i < I40E_INSET_MASK_NUM_REG; i++)
> +		i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i, pctype),
> +				     0);
> +	I40E_WRITE_FLUSH(hw);
> +
> +	pf->hash_input_set[pctype] = input_set;
>  	return 0;
>  }
> 
>  int
> -i40e_filter_inset_select(struct i40e_hw *hw,
> -			 struct rte_eth_input_set_conf *conf,
> -			 enum rte_filter_type filter)
> +i40e_fdir_filter_inset_select(struct i40e_pf *pf,
> +			 struct rte_eth_input_set_conf *conf)
>  {
> +	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
>  	enum i40e_filter_pctype pctype;
> -	uint64_t inset_reg = 0, input_set;
> -	uint32_t mask_reg[I40E_INSET_MASK_NUM_REG];
> -	uint8_t num;
> -	int ret;
> +	uint64_t input_set, inset_reg = 0;
> +	uint32_t mask_reg[I40E_INSET_MASK_NUM_REG] = {0};
> +	int ret, i, num;
> 
>  	if (!hw || !conf) {
>  		PMD_DRV_LOG(ERR, "Invalid pointer");
>  		return -EFAULT;
>  	}
> +	if (conf->op != RTE_ETH_INPUT_SET_SELECT &&
> +	    conf->op != RTE_ETH_INPUT_SET_ADD) {
> +		PMD_DRV_LOG(ERR, "Unsupported input set operation");
> +		return -EINVAL;
> +	}
> 
>  	pctype = i40e_flowtype_to_pctype(conf->flow_type);
>  	if (pctype == 0 || pctype > I40E_FILTER_PCTYPE_L2_PAYLOAD) { @@ -
> 6980,60 +6997,54 @@ i40e_filter_inset_select(struct i40e_hw *hw,
>  			    conf->flow_type);
>  		return -EINVAL;
>  	}
> -	if (filter != RTE_ETH_FILTER_HASH && filter != RTE_ETH_FILTER_FDIR)
> {
> -		PMD_DRV_LOG(ERR, "Not supported filter type (%u)", filter);
> -		return -EINVAL;
> -	}
> -
>  	ret = i40e_parse_input_set(&input_set, pctype, conf->field,
>  				   conf->inset_size);
>  	if (ret) {
>  		PMD_DRV_LOG(ERR, "Failed to parse input set");
>  		return -EINVAL;
>  	}
> -	if (i40e_validate_input_set(pctype, filter, input_set) != 0) {
> +	if (i40e_validate_input_set(pctype, RTE_ETH_FILTER_FDIR,
> +				    input_set) != 0) {
>  		PMD_DRV_LOG(ERR, "Invalid input set");
>  		return -EINVAL;
>  	}
> 
> -	if (conf->op == RTE_ETH_INPUT_SET_ADD) {
> -		inset_reg |= i40e_get_reg_inset(hw, filter, pctype);
> -	} else if (conf->op != RTE_ETH_INPUT_SET_SELECT) {
> -		PMD_DRV_LOG(ERR, "Unsupported input set operation");
> -		return -EINVAL;
> -	}
> +	/* get inset value in register */
> +	inset_reg = I40E_READ_REG(hw, I40E_PRTQF_FD_INSET(pctype, 1));
> +	inset_reg <<= I40E_32_BIT_WIDTH;
> +	inset_reg |= I40E_READ_REG(hw, I40E_PRTQF_FD_INSET(pctype, 0));
> +
> +	/*Can not change the inset reg for flex payload for fdir,
> +	 * it is done by writing I40E_PRTQF_FD_FLXINSET
> +	 * in i40e_set_flex_mask_on_pctype.
> +	 */
> +	if (conf->op == RTE_ETH_INPUT_SET_SELECT)
> +		inset_reg &= I40E_REG_INSET_FLEX_PAYLOAD_WORDS;
> +	else
> +		input_set |= pf->fdir.input_set[pctype];
>  	num = i40e_generate_inset_mask_reg(input_set, mask_reg,
>  					   I40E_INSET_MASK_NUM_REG);
> -	inset_reg |= i40e_translate_input_set_reg(input_set);
> -
> -	if (filter == RTE_ETH_FILTER_HASH) {
> -		ret = i40e_set_hash_inset_mask(hw, pctype, conf->op,
> mask_reg,
> -					       num);
> -		if (ret)
> -			return -EINVAL;
> +	if (num < 0)
> +		return -EINVAL;
> 
> -		i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(0, pctype),
> -				      (uint32_t)(inset_reg & UINT32_MAX));
> -		i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(1, pctype),
> -				     (uint32_t)((inset_reg >>
> -				     I40E_32_BIT_WIDTH) & UINT32_MAX));
> -	} else if (filter == RTE_ETH_FILTER_FDIR) {
> -		ret = i40e_set_fd_inset_mask(hw, pctype, conf->op, mask_reg,
> -					     num);
> -		if (ret)
> -			return -EINVAL;
> +	inset_reg |= i40e_translate_input_set_reg(input_set);
> 
> -		i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 0),
> -				      (uint32_t)(inset_reg & UINT32_MAX));
> -		i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 1),
> -				     (uint32_t)((inset_reg >>
> -				     I40E_32_BIT_WIDTH) & UINT32_MAX));
> -	} else {
> -		PMD_DRV_LOG(ERR, "Not supported filter type (%u)", filter);
> -		return -EINVAL;
> -	}
> +	i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 0),
> +			      (uint32_t)(inset_reg & UINT32_MAX));
> +	i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 1),
> +			     (uint32_t)((inset_reg >>
> +			     I40E_32_BIT_WIDTH) & UINT32_MAX));
> +
> +	for (i = 0; i < num; i++)
> +		i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i, pctype),
> +				     mask_reg[i]);
> +	/*clear unused mask registers of the pctype */
> +	for (i = num; i < I40E_INSET_MASK_NUM_REG; i++)
> +		i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i, pctype),
> +				     0);
>  	I40E_WRITE_FLUSH(hw);
> 
> +	pf->fdir.input_set[pctype] = input_set;
>  	return 0;
>  }
> 
> @@ -7085,9 +7096,8 @@ i40e_hash_filter_set(struct i40e_hw *hw, struct
> rte_eth_hash_filter_info *info)
>  				&(info->info.global_conf));
>  		break;
>  	case RTE_ETH_HASH_FILTER_INPUT_SET_SELECT:
> -		ret = i40e_filter_inset_select(hw,
> -					       &(info->info.input_set_conf),
> -					       RTE_ETH_FILTER_HASH);
> +		ret = i40e_hash_filter_inset_select(hw,
> +					       &(info->info.input_set_conf));
>  		break;
> 
>  	default:
> diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
> index 1f9792b..4dc7e1e 100644
> --- a/drivers/net/i40e/i40e_ethdev.h
> +++ b/drivers/net/i40e/i40e_ethdev.h
> @@ -361,6 +361,8 @@ struct i40e_fdir_info {
>  	struct i40e_rx_queue *rxq;
>  	void *prg_pkt;                 /* memory for fdir program packet */
>  	uint64_t dma_addr;             /* physic address of packet memory*/
> +	/* input set bits for each pctype */
> +	uint64_t input_set[I40E_FILTER_PCTYPE_MAX];
>  	/*
>  	 * the rule how bytes stream is extracted as flexible payload
>  	 * for each payload layer, the setting can up to three elements @@ -
> 427,6 +429,8 @@ struct i40e_pf {
>  	uint16_t fdir_qp_offset;
> 
>  	uint16_t hash_lut_size; /* The size of hash lookup table */
> +	/* input set bits for each pctype */
> +	uint64_t hash_input_set[I40E_FILTER_PCTYPE_MAX];
>  	/* store VXLAN UDP ports */
>  	uint16_t vxlan_ports[I40E_MAX_PF_UDP_OFFLOAD_PORTS];
>  	uint16_t vxlan_bitmap; /* Vxlan bit mask */ @@ -569,9 +573,10 @@
> int i40e_fdir_ctrl_func(struct rte_eth_dev *dev,  int
> i40e_select_filter_input_set(struct i40e_hw *hw,
>  				 struct rte_eth_input_set_conf *conf,
>  				 enum rte_filter_type filter);
> -int i40e_filter_inset_select(struct i40e_hw *hw,
> -			     struct rte_eth_input_set_conf *conf,
> -			     enum rte_filter_type filter);
> +int i40e_hash_filter_inset_select(struct i40e_hw *hw,
> +			     struct rte_eth_input_set_conf *conf); int
> +i40e_fdir_filter_inset_select(struct i40e_pf *pf,
> +			     struct rte_eth_input_set_conf *conf);
> 
>  void i40e_rxq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
>  	struct rte_eth_rxq_info *qinfo);
> diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c index
> 9ad6981..155a34a 100644
> --- a/drivers/net/i40e/i40e_fdir.c
> +++ b/drivers/net/i40e/i40e_fdir.c
> @@ -1361,7 +1361,6 @@ i40e_fdir_filter_set(struct rte_eth_dev *dev,
>  		     struct rte_eth_fdir_filter_info *info)  {
>  	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data-
> >dev_private);
> -	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
>  	int ret = 0;
> 
>  	if (!info) {
> @@ -1371,8 +1370,8 @@ i40e_fdir_filter_set(struct rte_eth_dev *dev,
> 
>  	switch (info->info_type) {
>  	case RTE_ETH_FDIR_FILTER_INPUT_SET_SELECT:
> -		ret = i40e_filter_inset_select(hw,
> -			&(info->info.input_set_conf), RTE_ETH_FILTER_FDIR);
> +		ret = i40e_fdir_filter_inset_select(pf,
> +				&(info->info.input_set_conf));
>  		break;
>  	default:
>  		PMD_DRV_LOG(ERR, "FD filter info type (%d) not supported",
> --
> 2.4.0
  
Jingjing Wu Jan. 21, 2016, 1:28 a.m. UTC | #2
Hi, Andrey

Thanks for your comments. You are correct, I removed the I40E_INSET_FLEX_PAYLOAD
from valid fdir input set values, and this is one reason why I splited function for input set
change of hash and and it is because all flex payload configuration can be
set in struct rte_fdir_conf during device configure phase. And it is a more flexible configuration
including flexpayload's selection, input set selection by word and mask setting in bits.

If I enable it in the input set change API, it will be duplicate. And the input set change on
flexible payload only on word, just some ability compared with rte_fdir_conf.
If flexible selection isn't done in  struct rte_fdir_conf, the input set 
selection in input set change API doesn't make sense. If flexible selection is done in
struct rte_fdir_conf, why not selection input set in struct rte_fdir_conf at the same time?

And about you concern, "when application has to run on an old NIC and on a new one",
The rte_fdir_conf is for each eth_dev, so it will be fine.

Thanks
Jingjing  


> -----Original Message-----
> From: Chilikin, Andrey
> Sent: Thursday, January 21, 2016 4:05 AM
> To: Wu, Jingjing; dev@dpdk.org
> Cc: Zhang, Helin; Pei, Yulong; Ananyev, Konstantin
> Subject: RE: [PATCH 2/4] i40e: split function for input set change of hash and
> fdir
> 
> Hi Jingjing,
> 
> As I can see this patch not only splits fdir functionality from common
> fdir/hash code but also removes compatibility with DPDK 2.2 as it deletes
> I40E_INSET_FLEX_PAYLOAD from valid fdir input set values. Yes, flexible
> payload configuration can be set for fdir separately at the port initialization,
> but this is more legacy from the previous generations of NICs which did not
> support dynamic input set configuration. I believe it would better to have
> I40E_INSET_FLEX_PAYLOAD valid for fdir input set same as in DPDK 2.2. So in
> legacy mode, when application has to run on an old NIC and on a new one,
> only legacy configuration would be used, but for applications targeting new
> HW single point of configuration would be used instead of mix of two.
> 
> Regards,
> Andrey
>
  
Chilikin, Andrey Jan. 21, 2016, 8:06 p.m. UTC | #3
Hi Jingjing,

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Thursday, January 21, 2016 1:29 AM
> To: Chilikin, Andrey; dev@dpdk.org
> Cc: Zhang, Helin; Pei, Yulong; Ananyev, Konstantin
> Subject: RE: [PATCH 2/4] i40e: split function for input set change of hash and
> fdir
> 
> Hi, Andrey
> 
> Thanks for your comments. You are correct, I removed the
> I40E_INSET_FLEX_PAYLOAD from valid fdir input set values, and this is one
> reason why I splited function for input set change of hash and and it is because
> all flex payload configuration can be set in struct rte_fdir_conf during device
> configure phase. And it is a more flexible configuration including flexpayload's
> selection, input set selection by word and mask setting in bits.

Should it be then two patches? First patch to split fdir and hash input set configuration and then second one to remove existing functionality? At the moment it is not obvious that this patch not just splits fdir input set configuration but removes some features in a way that fdir it is not compatible with DPDK 2.2 anymore.

> 
> If I enable it in the input set change API, it will be duplicate. And the input set
> change on flexible payload only on word, just some ability compared with
> rte_fdir_conf.
> If flexible selection isn't done in  struct rte_fdir_conf, the input set selection in
> input set change API doesn't make sense. If flexible selection is done in struct
> rte_fdir_conf, why not selection input set in struct rte_fdir_conf at the same
> time?

I do not have a problem with selecting it at the same time - it always was this way with the legacy systems. But now new NIC supports new way of configuring input set with flexible payload as a part of this input set. So why not to have new way of configuration available as well and change input set using one API call instead of splitting single configuration in to two parts?

> And about you concern, "when application has to run on an old NIC and on a
> new one", The rte_fdir_conf is for each eth_dev, so it will be fine.
> 
> Thanks
> Jingjing

Regards,
Andrey

> 
> 
> > -----Original Message-----
> > From: Chilikin, Andrey
> > Sent: Thursday, January 21, 2016 4:05 AM
> > To: Wu, Jingjing; dev@dpdk.org
> > Cc: Zhang, Helin; Pei, Yulong; Ananyev, Konstantin
> > Subject: RE: [PATCH 2/4] i40e: split function for input set change of
> > hash and fdir
> >
> > Hi Jingjing,
> >
> > As I can see this patch not only splits fdir functionality from common
> > fdir/hash code but also removes compatibility with DPDK 2.2 as it
> > deletes I40E_INSET_FLEX_PAYLOAD from valid fdir input set values. Yes,
> > flexible payload configuration can be set for fdir separately at the
> > port initialization, but this is more legacy from the previous
> > generations of NICs which did not support dynamic input set
> > configuration. I believe it would better to have
> > I40E_INSET_FLEX_PAYLOAD valid for fdir input set same as in DPDK 2.2.
> > So in legacy mode, when application has to run on an old NIC and on a
> > new one, only legacy configuration would be used, but for applications
> targeting new HW single point of configuration would be used instead of mix of
> two.
> >
> > Regards,
> > Andrey
> >
  
Jingjing Wu Jan. 26, 2016, 1:12 a.m. UTC | #4
> >
> > Thanks for your comments. You are correct, I removed the
> > I40E_INSET_FLEX_PAYLOAD from valid fdir input set values, and this is
> > one reason why I splited function for input set change of hash and and
> > it is because all flex payload configuration can be set in struct
> > rte_fdir_conf during device configure phase. And it is a more flexible
> > configuration including flexpayload's selection, input set selection by word
> and mask setting in bits.
> 
> Should it be then two patches? First patch to split fdir and hash input set
> configuration and then second one to remove existing functionality? At the
> moment it is not obvious that this patch not just splits fdir input set
> configuration but removes some features in a way that fdir it is not
> compatible with DPDK 2.2 anymore.
> 
OK. I will try to split it to two patches.

> >
> > If I enable it in the input set change API, it will be duplicate. And
> > the input set change on flexible payload only on word, just some
> > ability compared with rte_fdir_conf.
> > If flexible selection isn't done in  struct rte_fdir_conf, the input
> > set selection in input set change API doesn't make sense. If flexible
> > selection is done in struct rte_fdir_conf, why not selection input set
> > in struct rte_fdir_conf at the same time?
> 
> I do not have a problem with selecting it at the same time - it always was this
> way with the legacy systems. But now new NIC supports new way of
> configuring input set with flexible payload as a part of this input set. So why
> not to have new way of configuration available as well and change input set
> using one API call instead of splitting single configuration in to two parts?
> 
Yes, if we support two way, at least we need to make sure the consistency.
In this patch set, I didn't add new way to configure flexible selection and mask
Setting. So to make sure consistency, just remove the flexible payload in this
Patch.

Thanks
Jingjing
> > And about you concern, "when application has to run on an old NIC and
> > on a new one", The rte_fdir_conf is for each eth_dev, so it will be fine.
> >
  

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index bf6220d..b919aac 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -262,7 +262,8 @@ 
 #define I40E_REG_INSET_FLEX_PAYLOAD_WORD7        0x0000000000000080ULL
 /* 8th word of flex payload */
 #define I40E_REG_INSET_FLEX_PAYLOAD_WORD8        0x0000000000000040ULL
-
+/* all 8 words flex payload */
+#define I40E_REG_INSET_FLEX_PAYLOAD_WORDS        0x0000000000003FC0ULL
 #define I40E_REG_INSET_MASK_DEFAULT              0x0000000000000000ULL
 
 #define I40E_TRANSLATE_INSET 0
@@ -373,6 +374,7 @@  static int i40e_dev_udp_tunnel_add(struct rte_eth_dev *dev,
 				struct rte_eth_udp_tunnel *udp_tunnel);
 static int i40e_dev_udp_tunnel_del(struct rte_eth_dev *dev,
 				struct rte_eth_udp_tunnel *udp_tunnel);
+static void i40e_filter_input_set_init(struct i40e_pf *pf);
 static int i40e_ethertype_filter_set(struct i40e_pf *pf,
 			struct rte_eth_ethertype_filter *filter,
 			bool add);
@@ -787,6 +789,8 @@  eth_i40e_dev_init(struct rte_eth_dev *dev)
 	 * It should be removed once issues are fixed in NVM.
 	 */
 	i40e_flex_payload_reg_init(hw);
+	/* Initialize the input set for filters (hash and fd) to default value */
+	i40e_filter_input_set_init(pf);
 
 	/* Initialize the parameters for adminq */
 	i40e_init_adminq_parameter(hw);
@@ -6545,43 +6549,32 @@  i40e_get_valid_input_set(enum i40e_filter_pctype pctype,
 	 */
 	static const uint64_t valid_fdir_inset_table[] = {
 		[I40E_FILTER_PCTYPE_FRAG_IPV4] =
-		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST |
-		I40E_INSET_FLEX_PAYLOAD,
+		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST,
 		[I40E_FILTER_PCTYPE_NONF_IPV4_UDP] =
 		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST |
-		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
-		I40E_INSET_FLEX_PAYLOAD,
+		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT,
 		[I40E_FILTER_PCTYPE_NONF_IPV4_TCP] =
-		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST |
-		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
-		I40E_INSET_FLEX_PAYLOAD,
+		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST,
 		[I40E_FILTER_PCTYPE_NONF_IPV4_SCTP] =
 		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST |
 		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
-		I40E_INSET_SCTP_VT | I40E_INSET_FLEX_PAYLOAD,
+		I40E_INSET_SCTP_VT,
 		[I40E_FILTER_PCTYPE_NONF_IPV4_OTHER] =
-		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST |
-		I40E_INSET_FLEX_PAYLOAD,
+		I40E_INSET_IPV4_SRC | I40E_INSET_IPV4_DST,
 		[I40E_FILTER_PCTYPE_FRAG_IPV6] =
-		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST |
-		I40E_INSET_FLEX_PAYLOAD,
+		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST,
 		[I40E_FILTER_PCTYPE_NONF_IPV6_UDP] =
-		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST |
-		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
-		I40E_INSET_FLEX_PAYLOAD,
+		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST,
 		[I40E_FILTER_PCTYPE_NONF_IPV6_TCP] =
-		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST |
-		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
-		I40E_INSET_FLEX_PAYLOAD,
+		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST,
 		[I40E_FILTER_PCTYPE_NONF_IPV6_SCTP] =
 		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST |
 		I40E_INSET_SRC_PORT | I40E_INSET_DST_PORT |
-		I40E_INSET_SCTP_VT | I40E_INSET_FLEX_PAYLOAD,
+		I40E_INSET_SCTP_VT,
 		[I40E_FILTER_PCTYPE_NONF_IPV6_OTHER] =
-		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST |
-		I40E_INSET_FLEX_PAYLOAD,
+		I40E_INSET_IPV6_SRC | I40E_INSET_IPV6_DST,
 		[I40E_FILTER_PCTYPE_L2_PAYLOAD] =
-		I40E_INSET_LAST_ETHER_TYPE | I40E_INSET_FLEX_PAYLOAD,
+		I40E_INSET_LAST_ETHER_TYPE,
 	};
 
 	if (pctype > I40E_FILTER_PCTYPE_L2_PAYLOAD)
@@ -6809,7 +6802,7 @@  i40e_translate_input_set_reg(uint64_t input)
 	return val;
 }
 
-static uint8_t
+static int
 i40e_generate_inset_mask_reg(uint64_t inset, uint32_t *mask, uint8_t nb_elem)
 {
 	uint8_t i, idx = 0;
@@ -6827,16 +6820,13 @@  i40e_generate_inset_mask_reg(uint64_t inset, uint32_t *mask, uint8_t nb_elem)
 	if (!inset || !mask || !nb_elem)
 		return 0;
 
-	if (!inset && nb_elem >= I40E_INSET_MASK_NUM_REG) {
-		for (i = 0; i < I40E_INSET_MASK_NUM_REG; i++)
-			mask[i] = 0;
-		return I40E_INSET_MASK_NUM_REG;
-	}
 
 	for (i = 0, idx = 0; i < RTE_DIM(inset_mask_map); i++) {
-		if (idx >= nb_elem)
-			break;
-		if (inset & inset_mask_map[i].inset) {
+		if ((inset & inset_mask_map[i].inset) == inset_mask_map[i].inset) {
+			if (idx >= nb_elem) {
+				PMD_DRV_LOG(ERR, "exceed maximal number of bitmasks");
+				return -EINVAL;
+			}
 			mask[idx] = inset_mask_map[i].mask;
 			idx++;
 		}
@@ -6845,25 +6835,6 @@  i40e_generate_inset_mask_reg(uint64_t inset, uint32_t *mask, uint8_t nb_elem)
 	return idx;
 }
 
-static uint64_t
-i40e_get_reg_inset(struct i40e_hw *hw, enum rte_filter_type filter,
-			    enum i40e_filter_pctype pctype)
-{
-	uint64_t reg = 0;
-
-	if (filter == RTE_ETH_FILTER_HASH) {
-		reg = I40E_READ_REG(hw, I40E_GLQF_HASH_INSET(1, pctype));
-		reg <<= I40E_32_BIT_WIDTH;
-		reg |= I40E_READ_REG(hw, I40E_GLQF_HASH_INSET(0, pctype));
-	} else if (filter == RTE_ETH_FILTER_FDIR) {
-		reg = I40E_READ_REG(hw, I40E_PRTQF_FD_INSET(pctype, 1));
-		reg <<= I40E_32_BIT_WIDTH;
-		reg |= I40E_READ_REG(hw, I40E_PRTQF_FD_INSET(pctype, 0));
-	}
-
-	return reg;
-}
-
 static void
 i40e_check_write_reg(struct i40e_hw *hw, uint32_t addr, uint32_t val)
 {
@@ -6876,103 +6847,149 @@  i40e_check_write_reg(struct i40e_hw *hw, uint32_t addr, uint32_t val)
 		    (uint32_t)I40E_READ_REG(hw, addr));
 }
 
-static int
-i40e_set_hash_inset_mask(struct i40e_hw *hw,
-			 enum i40e_filter_pctype pctype,
-			 enum rte_filter_input_set_op op,
-			 uint32_t *mask_reg,
-			 uint8_t num)
+static void
+i40e_filter_input_set_init(struct i40e_pf *pf)
 {
-	uint32_t reg;
-	uint8_t i;
+	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
+	enum i40e_filter_pctype pctype;
+	uint64_t input_set, inset_reg;
+	uint32_t mask_reg[I40E_INSET_MASK_NUM_REG] = {0};
+	int num, i;
 
-	if (!mask_reg || num > RTE_ETH_INPUT_SET_SELECT)
-		return -EINVAL;
+	for (pctype = I40E_FILTER_PCTYPE_NONF_IPV4_UDP;
+	     pctype <= I40E_FILTER_PCTYPE_L2_PAYLOAD; pctype++) {
+		if (!I40E_VALID_PCTYPE(pctype))
+			continue;
+		input_set = i40e_get_default_input_set(pctype);
 
-	if (op == RTE_ETH_INPUT_SET_SELECT) {
-		for (i = 0; i < I40E_INSET_MASK_NUM_REG; i++) {
-			i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i, pctype),
-					     0);
-			if (i >= num)
-				continue;
+		num = i40e_generate_inset_mask_reg(input_set, mask_reg,
+						   I40E_INSET_MASK_NUM_REG);
+		if (num < 0)
+			return;
+		inset_reg = i40e_translate_input_set_reg(input_set);
+
+		i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 0),
+				      (uint32_t)(inset_reg & UINT32_MAX));
+		i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 1),
+				     (uint32_t)((inset_reg >>
+				     I40E_32_BIT_WIDTH) & UINT32_MAX));
+		i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(0, pctype),
+				      (uint32_t)(inset_reg & UINT32_MAX));
+		i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(1, pctype),
+				     (uint32_t)((inset_reg >>
+				     I40E_32_BIT_WIDTH) & UINT32_MAX));
+
+		for (i = 0; i < num; i++) {
+			i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i, pctype),
+					     mask_reg[i]);
 			i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i, pctype),
 					     mask_reg[i]);
 		}
-	} else if (op == RTE_ETH_INPUT_SET_ADD) {
-		uint8_t j, count = 0;
-
-		for (i = 0; i < I40E_INSET_MASK_NUM_REG; i++) {
-			reg = I40E_READ_REG(hw, I40E_GLQF_HASH_MSK(i, pctype));
-			if (reg & I40E_GLQF_HASH_MSK_FIELD)
-				count++;
+		/*clear unused mask registers of the pctype */
+		for (i = num; i < I40E_INSET_MASK_NUM_REG; i++) {
+			i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i, pctype),
+					     0);
+			i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i, pctype),
+					     0);
 		}
-		if (count + num > I40E_INSET_MASK_NUM_REG)
-			return -EINVAL;
+		I40E_WRITE_FLUSH(hw);
 
-		for (i = count, j = 0; i < I40E_INSET_MASK_NUM_REG; i++, j++)
-			i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i, pctype),
-					     mask_reg[j]);
+		/* store the default input set */
+		pf->hash_input_set[pctype] = input_set;
+		pf->fdir.input_set[pctype] = input_set;
 	}
-
-	return 0;
 }
 
-static int
-i40e_set_fd_inset_mask(struct i40e_hw *hw,
-		       enum i40e_filter_pctype pctype,
-		       enum rte_filter_input_set_op op,
-		       uint32_t *mask_reg,
-		       uint8_t num)
+int
+i40e_hash_filter_inset_select(struct i40e_hw *hw,
+			 struct rte_eth_input_set_conf *conf)
 {
-	uint32_t reg;
-	uint8_t i;
+	struct i40e_pf *pf = &((struct i40e_adapter *)hw->back)->pf;
+	enum i40e_filter_pctype pctype;
+	uint64_t input_set, inset_reg = 0;
+	uint32_t mask_reg[I40E_INSET_MASK_NUM_REG] = {0};
+	int ret, i, num;
 
-	if (!mask_reg || num > RTE_ETH_INPUT_SET_SELECT)
+	if (!hw || !conf) {
+		PMD_DRV_LOG(ERR, "Invalid pointer");
+		return -EFAULT;
+	}
+	if (conf->op != RTE_ETH_INPUT_SET_SELECT &&
+	    conf->op != RTE_ETH_INPUT_SET_ADD) {
+		PMD_DRV_LOG(ERR, "Unsupported input set operation");
 		return -EINVAL;
+	}
 
-	if (op == RTE_ETH_INPUT_SET_SELECT) {
-		for (i = 0; i < I40E_INSET_MASK_NUM_REG; i++) {
-			i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i, pctype),
-					     0);
-			if (i >= num)
-				continue;
-			i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i, pctype),
-					     mask_reg[i]);
-		}
-	} else if (op == RTE_ETH_INPUT_SET_ADD) {
-		uint8_t j, count = 0;
-
-		for (i = 0; i < I40E_INSET_MASK_NUM_REG; i++) {
-			reg = I40E_READ_REG(hw, I40E_GLQF_FD_MSK(i, pctype));
-			if (reg & I40E_GLQF_FD_MSK_FIELD)
-				count++;
-		}
-		if (count + num > I40E_INSET_MASK_NUM_REG)
-			return -EINVAL;
+	pctype = i40e_flowtype_to_pctype(conf->flow_type);
+	if (pctype == 0 || pctype > I40E_FILTER_PCTYPE_L2_PAYLOAD) {
+		PMD_DRV_LOG(ERR, "Not supported flow type (%u)",
+			    conf->flow_type);
+		return -EINVAL;
+	}
 
-		for (i = count, j = 0; i < I40E_INSET_MASK_NUM_REG; i++, j++)
-			i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i, pctype),
-					     mask_reg[j]);
+	ret = i40e_parse_input_set(&input_set, pctype, conf->field,
+				   conf->inset_size);
+	if (ret) {
+		PMD_DRV_LOG(ERR, "Failed to parse input set");
+		return -EINVAL;
 	}
+	if (i40e_validate_input_set(pctype, RTE_ETH_FILTER_HASH,
+				    input_set) != 0) {
+		PMD_DRV_LOG(ERR, "Invalid input set");
+		return -EINVAL;
+	}
+	if (conf->op == RTE_ETH_INPUT_SET_ADD) {
+		/* get inset value in register */
+		inset_reg = I40E_READ_REG(hw, I40E_GLQF_HASH_INSET(1, pctype));
+		inset_reg <<= I40E_32_BIT_WIDTH;
+		inset_reg |= I40E_READ_REG(hw, I40E_GLQF_HASH_INSET(0, pctype));
+		input_set |= pf->hash_input_set[pctype];
+	}
+	num = i40e_generate_inset_mask_reg(input_set, mask_reg,
+					   I40E_INSET_MASK_NUM_REG);
+	if (num < 0)
+		return -EINVAL;
+
+	inset_reg |= i40e_translate_input_set_reg(input_set);
 
+	i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(0, pctype),
+			      (uint32_t)(inset_reg & UINT32_MAX));
+	i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(1, pctype),
+			     (uint32_t)((inset_reg >>
+			     I40E_32_BIT_WIDTH) & UINT32_MAX));
+
+	for (i = 0; i < num; i++)
+		i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i, pctype),
+				     mask_reg[i]);
+	/*clear unused mask registers of the pctype */
+	for (i = num; i < I40E_INSET_MASK_NUM_REG; i++)
+		i40e_check_write_reg(hw, I40E_GLQF_HASH_MSK(i, pctype),
+				     0);
+	I40E_WRITE_FLUSH(hw);
+
+	pf->hash_input_set[pctype] = input_set;
 	return 0;
 }
 
 int
-i40e_filter_inset_select(struct i40e_hw *hw,
-			 struct rte_eth_input_set_conf *conf,
-			 enum rte_filter_type filter)
+i40e_fdir_filter_inset_select(struct i40e_pf *pf,
+			 struct rte_eth_input_set_conf *conf)
 {
+	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	enum i40e_filter_pctype pctype;
-	uint64_t inset_reg = 0, input_set;
-	uint32_t mask_reg[I40E_INSET_MASK_NUM_REG];
-	uint8_t num;
-	int ret;
+	uint64_t input_set, inset_reg = 0;
+	uint32_t mask_reg[I40E_INSET_MASK_NUM_REG] = {0};
+	int ret, i, num;
 
 	if (!hw || !conf) {
 		PMD_DRV_LOG(ERR, "Invalid pointer");
 		return -EFAULT;
 	}
+	if (conf->op != RTE_ETH_INPUT_SET_SELECT &&
+	    conf->op != RTE_ETH_INPUT_SET_ADD) {
+		PMD_DRV_LOG(ERR, "Unsupported input set operation");
+		return -EINVAL;
+	}
 
 	pctype = i40e_flowtype_to_pctype(conf->flow_type);
 	if (pctype == 0 || pctype > I40E_FILTER_PCTYPE_L2_PAYLOAD) {
@@ -6980,60 +6997,54 @@  i40e_filter_inset_select(struct i40e_hw *hw,
 			    conf->flow_type);
 		return -EINVAL;
 	}
-	if (filter != RTE_ETH_FILTER_HASH && filter != RTE_ETH_FILTER_FDIR) {
-		PMD_DRV_LOG(ERR, "Not supported filter type (%u)", filter);
-		return -EINVAL;
-	}
-
 	ret = i40e_parse_input_set(&input_set, pctype, conf->field,
 				   conf->inset_size);
 	if (ret) {
 		PMD_DRV_LOG(ERR, "Failed to parse input set");
 		return -EINVAL;
 	}
-	if (i40e_validate_input_set(pctype, filter, input_set) != 0) {
+	if (i40e_validate_input_set(pctype, RTE_ETH_FILTER_FDIR,
+				    input_set) != 0) {
 		PMD_DRV_LOG(ERR, "Invalid input set");
 		return -EINVAL;
 	}
 
-	if (conf->op == RTE_ETH_INPUT_SET_ADD) {
-		inset_reg |= i40e_get_reg_inset(hw, filter, pctype);
-	} else if (conf->op != RTE_ETH_INPUT_SET_SELECT) {
-		PMD_DRV_LOG(ERR, "Unsupported input set operation");
-		return -EINVAL;
-	}
+	/* get inset value in register */
+	inset_reg = I40E_READ_REG(hw, I40E_PRTQF_FD_INSET(pctype, 1));
+	inset_reg <<= I40E_32_BIT_WIDTH;
+	inset_reg |= I40E_READ_REG(hw, I40E_PRTQF_FD_INSET(pctype, 0));
+
+	/*Can not change the inset reg for flex payload for fdir,
+	 * it is done by writing I40E_PRTQF_FD_FLXINSET
+	 * in i40e_set_flex_mask_on_pctype.
+	 */
+	if (conf->op == RTE_ETH_INPUT_SET_SELECT)
+		inset_reg &= I40E_REG_INSET_FLEX_PAYLOAD_WORDS;
+	else
+		input_set |= pf->fdir.input_set[pctype];
 	num = i40e_generate_inset_mask_reg(input_set, mask_reg,
 					   I40E_INSET_MASK_NUM_REG);
-	inset_reg |= i40e_translate_input_set_reg(input_set);
-
-	if (filter == RTE_ETH_FILTER_HASH) {
-		ret = i40e_set_hash_inset_mask(hw, pctype, conf->op, mask_reg,
-					       num);
-		if (ret)
-			return -EINVAL;
+	if (num < 0)
+		return -EINVAL;
 
-		i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(0, pctype),
-				      (uint32_t)(inset_reg & UINT32_MAX));
-		i40e_check_write_reg(hw, I40E_GLQF_HASH_INSET(1, pctype),
-				     (uint32_t)((inset_reg >>
-				     I40E_32_BIT_WIDTH) & UINT32_MAX));
-	} else if (filter == RTE_ETH_FILTER_FDIR) {
-		ret = i40e_set_fd_inset_mask(hw, pctype, conf->op, mask_reg,
-					     num);
-		if (ret)
-			return -EINVAL;
+	inset_reg |= i40e_translate_input_set_reg(input_set);
 
-		i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 0),
-				      (uint32_t)(inset_reg & UINT32_MAX));
-		i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 1),
-				     (uint32_t)((inset_reg >>
-				     I40E_32_BIT_WIDTH) & UINT32_MAX));
-	} else {
-		PMD_DRV_LOG(ERR, "Not supported filter type (%u)", filter);
-		return -EINVAL;
-	}
+	i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 0),
+			      (uint32_t)(inset_reg & UINT32_MAX));
+	i40e_check_write_reg(hw, I40E_PRTQF_FD_INSET(pctype, 1),
+			     (uint32_t)((inset_reg >>
+			     I40E_32_BIT_WIDTH) & UINT32_MAX));
+
+	for (i = 0; i < num; i++)
+		i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i, pctype),
+				     mask_reg[i]);
+	/*clear unused mask registers of the pctype */
+	for (i = num; i < I40E_INSET_MASK_NUM_REG; i++)
+		i40e_check_write_reg(hw, I40E_GLQF_FD_MSK(i, pctype),
+				     0);
 	I40E_WRITE_FLUSH(hw);
 
+	pf->fdir.input_set[pctype] = input_set;
 	return 0;
 }
 
@@ -7085,9 +7096,8 @@  i40e_hash_filter_set(struct i40e_hw *hw, struct rte_eth_hash_filter_info *info)
 				&(info->info.global_conf));
 		break;
 	case RTE_ETH_HASH_FILTER_INPUT_SET_SELECT:
-		ret = i40e_filter_inset_select(hw,
-					       &(info->info.input_set_conf),
-					       RTE_ETH_FILTER_HASH);
+		ret = i40e_hash_filter_inset_select(hw,
+					       &(info->info.input_set_conf));
 		break;
 
 	default:
diff --git a/drivers/net/i40e/i40e_ethdev.h b/drivers/net/i40e/i40e_ethdev.h
index 1f9792b..4dc7e1e 100644
--- a/drivers/net/i40e/i40e_ethdev.h
+++ b/drivers/net/i40e/i40e_ethdev.h
@@ -361,6 +361,8 @@  struct i40e_fdir_info {
 	struct i40e_rx_queue *rxq;
 	void *prg_pkt;                 /* memory for fdir program packet */
 	uint64_t dma_addr;             /* physic address of packet memory*/
+	/* input set bits for each pctype */
+	uint64_t input_set[I40E_FILTER_PCTYPE_MAX];
 	/*
 	 * the rule how bytes stream is extracted as flexible payload
 	 * for each payload layer, the setting can up to three elements
@@ -427,6 +429,8 @@  struct i40e_pf {
 	uint16_t fdir_qp_offset;
 
 	uint16_t hash_lut_size; /* The size of hash lookup table */
+	/* input set bits for each pctype */
+	uint64_t hash_input_set[I40E_FILTER_PCTYPE_MAX];
 	/* store VXLAN UDP ports */
 	uint16_t vxlan_ports[I40E_MAX_PF_UDP_OFFLOAD_PORTS];
 	uint16_t vxlan_bitmap; /* Vxlan bit mask */
@@ -569,9 +573,10 @@  int i40e_fdir_ctrl_func(struct rte_eth_dev *dev,
 int i40e_select_filter_input_set(struct i40e_hw *hw,
 				 struct rte_eth_input_set_conf *conf,
 				 enum rte_filter_type filter);
-int i40e_filter_inset_select(struct i40e_hw *hw,
-			     struct rte_eth_input_set_conf *conf,
-			     enum rte_filter_type filter);
+int i40e_hash_filter_inset_select(struct i40e_hw *hw,
+			     struct rte_eth_input_set_conf *conf);
+int i40e_fdir_filter_inset_select(struct i40e_pf *pf,
+			     struct rte_eth_input_set_conf *conf);
 
 void i40e_rxq_info_get(struct rte_eth_dev *dev, uint16_t queue_id,
 	struct rte_eth_rxq_info *qinfo);
diff --git a/drivers/net/i40e/i40e_fdir.c b/drivers/net/i40e/i40e_fdir.c
index 9ad6981..155a34a 100644
--- a/drivers/net/i40e/i40e_fdir.c
+++ b/drivers/net/i40e/i40e_fdir.c
@@ -1361,7 +1361,6 @@  i40e_fdir_filter_set(struct rte_eth_dev *dev,
 		     struct rte_eth_fdir_filter_info *info)
 {
 	struct i40e_pf *pf = I40E_DEV_PRIVATE_TO_PF(dev->data->dev_private);
-	struct i40e_hw *hw = I40E_PF_TO_HW(pf);
 	int ret = 0;
 
 	if (!info) {
@@ -1371,8 +1370,8 @@  i40e_fdir_filter_set(struct rte_eth_dev *dev,
 
 	switch (info->info_type) {
 	case RTE_ETH_FDIR_FILTER_INPUT_SET_SELECT:
-		ret = i40e_filter_inset_select(hw,
-			&(info->info.input_set_conf), RTE_ETH_FILTER_FDIR);
+		ret = i40e_fdir_filter_inset_select(pf,
+				&(info->info.input_set_conf));
 		break;
 	default:
 		PMD_DRV_LOG(ERR, "FD filter info type (%d) not supported",