[PATCH v2] net/ixgbe: fix repeated Rx packet buffer shrink for FDIR

[Zhang Tengfei]

Enabling Flow Director reduces RXPBSIZE(0) to reserve packet buffer
memory for the filter table. The new size was computed from the current
register value, and nothing restored it when FDIR state was reset. After
the last FDIR rule was removed, or when the first rule failed after FDIR
had been enabled, the next rule created enabled FDIR again and reduced
the buffer a second time.

Compute RXPBSIZE(0) from the default packet buffer size, and disable
FDIR and restore the buffer when the last rule is removed, on flush, and
on the enable error paths.

Fixes: 301abd26b118 ("net/ixgbe: reset flow state on clear paths")
Fixes: 5007ac13189d ("ethdev: remove deprecated Flow Director configuration")
Cc: stable at dpdk.org

Signed-off-by: Zhang Tengfei <zhtfdev at gmail.com>
---
v2:
- Drop the rx_pb_reduced flag and keep resetting the FDIR mode to NONE
  when the last rule is removed (Bruce).
- Compute RXPBSIZE(0) from hw->mac.rx_pb_size, and disable FDIR and
  restore the buffer when the last rule is removed or on flush, based
  on Anatoly's draft.
- Disable FDIR again if the first rule fails after enabling it.
- Do not fail flush if re-initializing the FDIR tables fails, as FDIR
  is disabled right after.
- Add Fixes tag for the first-rule failure path.
- Compile tested only.
- Author name follows the pending .mailmap update: <https://patches.dpdk.org/project/dpdk/patch/20260927035949.36887-1-zhtfdev@gmail.com/>

 drivers/net/intel/ixgbe/ixgbe_ethdev.h |  1 +
 drivers/net/intel/ixgbe/ixgbe_fdir.c   | 70 +++++++++++++-------------
 drivers/net/intel/ixgbe/ixgbe_flow.c   | 34 ++++++++-----
 3 files changed, 58 insertions(+), 47 deletions(-)

diff --git a/drivers/net/intel/ixgbe/ixgbe_ethdev.h b/drivers/net/intel/ixgbe/ixgbe_ethdev.h
index 2e9d166e29..dfc8fce1a8 100644
--- a/drivers/net/intel/ixgbe/ixgbe_ethdev.h
+++ b/drivers/net/intel/ixgbe/ixgbe_ethdev.h
@@ -700,6 +700,7 @@ void ixgbe_filterlist_flush(struct rte_eth_dev *dev);
 int ixgbe_fdir_configure(struct rte_eth_dev *dev,
 			 const struct rte_eth_fdir_conf *fdir_conf,
 			 const struct ixgbe_hw_fdir_mask *fdir_mask);
+void ixgbe_fdir_disable(struct ixgbe_hw *hw);
 int ixgbe_fdir_set_input_mask(struct ixgbe_adapter *adapter,
 			      const struct ixgbe_hw_fdir_mask *mask,
 			      enum rte_fdir_mode mode);
diff --git a/drivers/net/intel/ixgbe/ixgbe_fdir.c b/drivers/net/intel/ixgbe/ixgbe_fdir.c
index 0159c5b3b7..ffa9f5cc4d 100644
--- a/drivers/net/intel/ixgbe/ixgbe_fdir.c
+++ b/drivers/net/intel/ixgbe/ixgbe_fdir.c
@@ -101,7 +101,6 @@ static int fdir_write_perfect_filter_82599(struct ixgbe_hw *hw,
 static int fdir_add_signature_filter_82599(struct ixgbe_hw *hw,
 		union ixgbe_atr_input *input, u8 queue, uint32_t fdircmd,
 		uint32_t fdirhash);
-static int ixgbe_fdir_flush(struct rte_eth_dev *dev);
 
 /**
  * This function is based on ixgbe_fdir_enable_82599() in base/ixgbe_82599.c.
@@ -554,6 +553,19 @@ ixgbe_set_fdir_flex_conf(struct ixgbe_adapter *adapter,
 	return 0;
 }
 
+void
+ixgbe_fdir_disable(struct ixgbe_hw *hw)
+{
+	int i;
+
+	IXGBE_WRITE_REG(hw, IXGBE_FDIRCTRL, 0);
+	IXGBE_WRITE_REG(hw, IXGBE_RXPBSIZE(0),
+			hw->mac.rx_pb_size << IXGBE_RXPBSIZE_SHIFT);
+	for (i = 1; i < 8; i++)
+		IXGBE_WRITE_REG(hw, IXGBE_RXPBSIZE(i), 0);
+	IXGBE_WRITE_FLUSH(hw);
+}
+
 int
 ixgbe_fdir_configure(struct rte_eth_dev *dev,
 		const struct rte_eth_fdir_conf *fdir_conf,
@@ -601,12 +613,12 @@ ixgbe_fdir_configure(struct rte_eth_dev *dev,
 
 	/*
 	 * Before enabling Flow Director, the Rx Packet Buffer size
-	 * must be reduced.  The new value is the current size minus
+	 * must be reduced.  The new value is the default size minus
 	 * flow director memory usage size.
 	 */
 	pbsize = (1 << (PBALLOC_SIZE_SHIFT + (fdirctrl & FDIRCTRL_PBALLOC_MASK)));
 	IXGBE_WRITE_REG(hw, IXGBE_RXPBSIZE(0),
-	    (IXGBE_READ_REG(hw, IXGBE_RXPBSIZE(0)) - pbsize));
+	    (hw->mac.rx_pb_size << IXGBE_RXPBSIZE_SHIFT) - pbsize);
 
 	/*
 	 * The defaults in the HW for RX PB 1-7 are not zero and so should be
@@ -620,21 +632,25 @@ ixgbe_fdir_configure(struct rte_eth_dev *dev,
 	err = ixgbe_fdir_set_input_mask(adapter, fdir_mask, mode);
 	if (err < 0) {
 		PMD_INIT_LOG(ERR, " Error on setting FD mask");
-		return err;
+		goto error;
 	}
 	err = ixgbe_set_fdir_flex_conf(adapter, &fdir_conf->flex_conf,
 				       &fdirctrl);
 	if (err < 0) {
 		PMD_INIT_LOG(ERR, " Error on setting FD flexible arguments.");
-		return err;
+		goto error;
 	}
 
 	err = fdir_enable_82599(hw, fdirctrl);
 	if (err < 0) {
 		PMD_INIT_LOG(ERR, " Error on enabling FD.");
-		return err;
+		goto error;
 	}
 	return 0;
+
+error:
+	ixgbe_fdir_disable(hw);
+	return err;
 }
 
 /*
@@ -1191,28 +1207,6 @@ ixgbe_fdir_filter_program(struct ixgbe_adapter *adapter,
 	return err;
 }
 
-static int
-ixgbe_fdir_flush(struct rte_eth_dev *dev)
-{
-	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
-	struct ixgbe_hw_fdir_info *info =
-			IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
-	int ret;
-
-	ret = ixgbe_reinit_fdir_tables_82599(hw);
-	if (ret < 0) {
-		PMD_INIT_LOG(ERR, "Failed to re-initialize FD table.");
-		return ret;
-	}
-
-	info->f_add = 0;
-	info->f_remove = 0;
-	info->add = 0;
-	info->remove = 0;
-
-	return ret;
-}
-
 #define FDIRENTRIES_NUM_SHIFT 10
 void
 ixgbe_fdir_info_get(struct rte_eth_dev *dev, struct rte_eth_fdir_info *fdir_info)
@@ -1371,13 +1365,22 @@ int
 ixgbe_clear_all_fdir_filter(struct rte_eth_dev *dev)
 {
 	struct rte_eth_fdir_conf *fdir_conf = IXGBE_DEV_FDIR_CONF(dev);
+	struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(dev->data->dev_private);
 	struct ixgbe_hw_fdir_info *fdir_info =
 		IXGBE_DEV_PRIVATE_TO_FDIR_INFO(dev->data->dev_private);
 	struct ixgbe_fdir_filter *fdir_filter;
-	bool had_flows;
-	int ret = 0;
 
-	had_flows = (fdir_info->n_flows != 0);
+	if (fdir_conf->mode != RTE_FDIR_MODE_NONE) {
+		if (ixgbe_reinit_fdir_tables_82599(hw) < 0)
+			PMD_DRV_LOG(WARNING, "Failed to re-initialize FD table");
+
+		fdir_info->f_add = 0;
+		fdir_info->f_remove = 0;
+		fdir_info->add = 0;
+		fdir_info->remove = 0;
+
+		ixgbe_fdir_disable(hw);
+	}
 
 	/* flush flow director */
 	rte_hash_reset(fdir_info->hash_handle);
@@ -1397,8 +1400,5 @@ ixgbe_clear_all_fdir_filter(struct rte_eth_dev *dev)
 	fdir_info->mask_added = FALSE;
 	fdir_conf->mode = RTE_FDIR_MODE_NONE;
 
-	if (had_flows)
-		ret = ixgbe_fdir_flush(dev);
-
-	return ret;
+	return 0;
 }
diff --git a/drivers/net/intel/ixgbe/ixgbe_flow.c b/drivers/net/intel/ixgbe/ixgbe_flow.c
index ff56d7b279..a8d1b269c7 100644
--- a/drivers/net/intel/ixgbe/ixgbe_flow.c
+++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
@@ -2625,6 +2625,7 @@ ixgbe_fdir_flow_program(struct rte_eth_dev *dev,
 	struct rte_eth_fdir_conf local_fdir_conf = *fdir_conf;
 	struct ixgbe_hw_fdir_info *fdir_info =
 			IXGBE_DEV_PRIVATE_TO_FDIR_INFO(adapter);
+	bool fdir_enabled = false;
 	int ret;
 
 	if (fdir_rule->queue >= dev->data->nb_rx_queues) {
@@ -2647,6 +2648,7 @@ ixgbe_fdir_flow_program(struct rte_eth_dev *dev,
 				RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 				NULL, "Failed to configure fdir mode");
 		}
+		fdir_enabled = true;
 	} else if (fdir_conf->mode != fdir_rule->mode) {
 		return rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
@@ -2657,17 +2659,25 @@ ixgbe_fdir_flow_program(struct rte_eth_dev *dev,
 	ret = ixgbe_fdir_process_rule(adapter, fdir_info, fdir_rule,
 		first_mask, error);
 	if (ret)
-		return ret;
+		goto error;
 
 	/* Program the filter */
 	ret = ixgbe_fdir_filter_program(adapter, &local_fdir_conf,
 			fdir_rule, FALSE, FALSE);
-	if (ret)
-		return rte_flow_error_set(error, EINVAL,
+	if (ret) {
+		ret = rte_flow_error_set(error, EINVAL,
 			RTE_FLOW_ERROR_TYPE_UNSPECIFIED,
 			NULL, "Failed to add fdir filter");
+		goto error;
+	}
 
 	return 0;
+
+error:
+	/* FDIR mode is only recorded on success, so undo the enable */
+	if (fdir_enabled)
+		ixgbe_fdir_disable(IXGBE_DEV_PRIVATE_TO_HW(adapter));
+	return ret;
 }
 
 /* Flow actions check specific to RSS filter */
@@ -3159,16 +3169,16 @@ ixgbe_flow_destroy(struct rte_eth_dev *dev,
 	case RTE_ETH_FILTER_FDIR:
 		fdir_rule_ptr = (struct ixgbe_fdir_rule_ele *)pmd_flow->rule;
 		fdir_rule = fdir_rule_ptr->filter_info;
-		ret = ixgbe_fdir_filter_program(adapter, fdir_conf, &fdir_rule, TRUE, FALSE);
-		if (!ret) {
-			rte_free(fdir_rule_ptr);
-			if (fdir_info->n_flows > 0 && --(fdir_info->n_flows) == 0) {
-				fdir_info->mask_added = false;
-				fdir_info->mask = (struct ixgbe_hw_fdir_mask){0};
-				fdir_info->flex_bytes_offset = 0;
-				fdir_conf->mode = RTE_FDIR_MODE_NONE;
-			}
+		if (fdir_info->n_flows == 1) {
+			ret = ixgbe_clear_all_fdir_filter(dev);
+		} else {
+			ret = ixgbe_fdir_filter_program(adapter, fdir_conf,
+					&fdir_rule, TRUE, FALSE);
+			if (!ret && fdir_info->n_flows > 0)
+				fdir_info->n_flows--;
 		}
+		if (!ret)
+			rte_free(fdir_rule_ptr);
 		break;
 	case RTE_ETH_FILTER_L2_TUNNEL:
 		l2_tn_filter_ptr = (struct ixgbe_eth_l2_tunnel_conf_ele *)
-- 
2.53.0