diff mbox

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

Message ID 1453789575-6297-3-git-send-email-jingjing.wu@intel.com (mailing list archive)
State Superseded, archived
Delegated to: Bruce Richardson
Headers

Commit Message

Jingjing Wu Jan. 26, 2016, 6:26 a.m. UTC 
  This patch split function for input set changing of hash
and fdir to avoid multiple check on different situation.

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

Comments

Zhang, Helin Feb. 25, 2016, 8:51 a.m. UTC | #1 
Jingjing

Your patches have depencies on my i40e base driver udpate patch set.
Some registers should be read/written by AQ commands, with using interfaces
of i40e_read_rx_ctl() and i40e_write_rx_ctl().

Please check below link and see the list of those registers.
http://www.dpdk.org/dev/patchwork/patch/10654/

Regards,
Helin

> -----Original Message-----
> From: Wu, Jingjing
> Sent: Tuesday, January 26, 2016 2:26 PM
> To: dev@dpdk.org
> Cc: Wu, Jingjing; Zhang, Helin; Chilikin, Andrey; Lu, Wenzhuo; Pei, Yulong
> Subject: [PATCH 02/12] i40e: split function for input set change of hash and
> fdir
> 
> This patch split function for input set changing of hash and fdir to avoid
> multiple check on different situation.
> 
> Signed-off-by: Jingjing Wu <jingjing.wu@intel.com>
> ---
>  drivers/net/i40e/i40e_ethdev.c | 233 +++++++++++++++++--------------------
> ----
>  drivers/net/i40e/i40e_ethdev.h |  11 +-
>  drivers/net/i40e/i40e_fdir.c   |   5 +-
>  3 files changed, 107 insertions(+), 142 deletions(-)
> 
> diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
> index bf6220d..004e206 100644
> --- a/drivers/net/i40e/i40e_ethdev.c
> +++ b/drivers/net/i40e/i40e_ethdev.c
> @@ -6845,25 +6845,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 +6857,96 @@ 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)
> +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;
If you have check of 'if (!hw || !conf)', above line is not good.

> +	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) {
Check hw might not be needed at all.
Jingjing Wu Feb. 26, 2016, 12:32 a.m. UTC | #2 
> -----Original Message-----
> From: Zhang, Helin
> Sent: Thursday, February 25, 2016 4:51 PM
> To: Wu, Jingjing <jingjing.wu@intel.com>; dev@dpdk.org
> Cc: Chilikin, Andrey <andrey.chilikin@intel.com>; Lu, Wenzhuo <wenzhuo.lu@intel.com>;
> Pei, Yulong <yulong.pei@intel.com>
> Subject: RE: [PATCH 02/12] i40e: split function for input set change of hash and fdir
> 
> Jingjing
> 
> Your patches have depencies on my i40e base driver udpate patch set.
> Some registers should be read/written by AQ commands, with using interfaces
> of i40e_read_rx_ctl() and i40e_write_rx_ctl().
> 
> Please check below link and see the list of those registers.
> http://www.dpdk.org/dev/patchwork/patch/10654/
> 
Yes, I know that. I was planning to rebase this or send a fix after "i40e base driver update" is merged.
>
diff mbox

Patch

diff --git a/drivers/net/i40e/i40e_ethdev.c b/drivers/net/i40e/i40e_ethdev.c
index bf6220d..004e206 100644
--- a/drivers/net/i40e/i40e_ethdev.c
+++ b/drivers/net/i40e/i40e_ethdev.c
@@ -6845,25 +6845,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 +6857,96 @@  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)
+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_HASH_MSK(i, pctype),
-					     0);
-			if (i >= num)
-				continue;
-			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++;
-		}
-		if (count + num > I40E_INSET_MASK_NUM_REG)
-			return -EINVAL;
-
-		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]);
 	}
 
-	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)
-{
-	uint32_t reg;
-	uint8_t i;
-
-	if (!mask_reg || num > RTE_ETH_INPUT_SET_SELECT)
+	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;
+	}
 
-	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;
+	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;
 
-		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;
+	inset_reg |= i40e_translate_input_set_reg(input_set);
 
-		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]);
-	}
+	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 +6954,48 @@  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));
+
+	if (conf->op == RTE_ETH_INPUT_SET_ADD)
+		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 +7047,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",