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

["Burakov, Anatoly" <anatoly.burakov@intel.com>]

On 9/25/2026 11:47 AM, Burakov, Anatoly wrote:
> A general comment: instead of reducing/bringing things back and storing 
> a flag noting whether we did, I would rather do the following:
> 
> 0) store default rx pb size at init
> 1) on enabling FDIR, recalculate using that value minus FDIR table size
> 2) on disabling FDIR[*], restore the default
> 3) similarly, on enable/disable VMDq, recalculate and/or reset
> 
> [*] there is no "disable FDIR" call, only fdir flush which just flushes 
> the FDIR tables but does not actually disable FDIR. arguably, we should 
> convert it to "disable FDIR" by flushing FDIR *and* writing 0 to 
> FDIRCTRL *and* restoring rx pb size to defaults. naturally, after 
> running fdir disable function, FDIR will need to be reconfigured for 
> next FDIR flow and get rx pb size recalculated again.
> 
> So, a bit of a refactor, but I think that would make way more sense.

I asked an AI to implement a fix based on this, and here's what it came 
up with, it is roughly what I would like to see instead (obviously, 
please review/rework as appropriate e.g. to properly support VMDq as well):

diff --git a/drivers/net/intel/ixgbe/ixgbe_fdir.c 
b/drivers/net/intel/ixgbe/ixgbe_fdir.c
index b32dc542874..9f48a27cb3a 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,20 @@ ixgbe_set_fdir_flex_conf(struct ixgbe_adapter *adapter,
         return 0;
  }

+static void
+ixgbe_fdir_disable(struct ixgbe_hw *hw)
+{
+       uint32_t rx_pb_size;
+       int i;
+
+       IXGBE_WRITE_REG(hw, IXGBE_FDIRCTRL, 0);
+       rx_pb_size = (uint32_t)hw->mac.rx_pb_size << IXGBE_RXPBSIZE_SHIFT;
+       IXGBE_WRITE_REG(hw, IXGBE_RXPBSIZE(0), rx_pb_size);
+       for (i = 1; i < 8; i++)
+               IXGBE_WRITE_REG(hw, IXGBE_RXPBSIZE(i), 0);
+       IXGBE_WRITE_FLUSH(hw);
+}
+
  int
  ixgbe_fdir_configure(struct ixgbe_adapter *adapter,
                 const struct rte_eth_fdir_conf *fdir_conf,
@@ -561,7 +574,7 @@ ixgbe_fdir_configure(struct ixgbe_adapter *adapter,
  {
         struct ixgbe_hw *hw = IXGBE_DEV_PRIVATE_TO_HW(adapter);
         int err;
-       uint32_t fdirctrl, pbsize;
+       uint32_t fdirctrl, pbsize, rx_pb_size;
         int i;
         enum rte_fdir_mode mode = fdir_conf->mode;

@@ -589,13 +602,14 @@ ixgbe_fdir_configure(struct ixgbe_adapter *adapter,
                 return err;

         /*
-        * Before enabling Flow Director, the Rx Packet Buffer size
-        * must be reduced.  The new value is the current size minus
-        * flow director memory usage size.
+        * Before enabling Flow Director, the Rx Packet Buffer size 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));
+       pbsize = 1 << (PBALLOC_SIZE_SHIFT +
+                       (fdirctrl & FDIRCTRL_PBALLOC_MASK));
+       rx_pb_size = (uint32_t)hw->mac.rx_pb_size << IXGBE_RXPBSIZE_SHIFT;
+       IXGBE_WRITE_REG(hw, IXGBE_RXPBSIZE(0), rx_pb_size - pbsize);

         /*
          * The defaults in the HW for RX PB 1-7 are not zero and so 
should be
@@ -609,21 +623,25 @@ ixgbe_fdir_configure(struct ixgbe_adapter *adapter,
         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;
  }

  /*
@@ -1180,28 +1198,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)
@@ -1360,13 +1356,26 @@ 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;
+       int ret;

-       had_flows = (fdir_info->n_flows != 0);
+       if (fdir_conf->mode != RTE_FDIR_MODE_NONE) {
+               ret = ixgbe_reinit_fdir_tables_82599(hw);
+               if (ret < 0) {
+                       PMD_INIT_LOG(ERR, "Failed to re-initialize FD 
table.");
+                       return ret;
+               }
+
+               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);
@@ -1386,8 +1395,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 6868893d46a..da05e61e8b4 100644
--- a/drivers/net/intel/ixgbe/ixgbe_flow.c
+++ b/drivers/net/intel/ixgbe/ixgbe_flow.c
@@ -3157,15 +3157,15 @@ ixgbe_flow_destroy(struct rte_eth_dev *dev,
                 memcpy(&fdir_rule,
                         &fdir_rule_ptr->filter_info,
                         sizeof(struct ixgbe_fdir_rule));
-               ret = ixgbe_fdir_filter_program(adapter, fdir_conf, 
&fdir_rule, TRUE, FALSE);
+               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) {
                         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)
+                               fdir_info->n_flows--;
                 }
                 break;
         case RTE_ETH_FILTER_L2_TUNNEL:

-- 
Thanks,
Anatoly