On 10/17/22 15:32, Chas Williams wrote:
This appears to be correct. A minor comment inline.

On 10/17/22 04:42, Andrew Rybchenko wrote:
Chas, Cornor, could you review the patch, please.

Thanks,
Andrew.

On 9/11/22 15:24, Ivan Malov wrote:
According to the documentation, rte_eth_dev_configure()
can be invoked repeatedly while in stopped state.
The current implementation in the bonding driver
allows for that (technically), but the user sees
warnings which say that back-end devices have
already been harnessed. Re-factor the code
to have cleanup before each (re-)configure.

Signed-off-by: Ivan Malov <ivan.ma...@oktetlabs.ru>
Reviewed-by: Andrew Rybchenko <andrew.rybche...@oktetlabs.ru>
---
  drivers/net/bonding/rte_eth_bond_pmd.c | 27 ++++++++++++++++++--------
  1 file changed, 19 insertions(+), 8 deletions(-)

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index d01c954296..92a33b45bd 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2143,18 +2143,13 @@ bond_ethdev_stop(struct rte_eth_dev *eth_dev)
      return 0;
  }
-int
-bond_ethdev_close(struct rte_eth_dev *dev)
+static void
+bond_ethdev_cfg_cleanup(struct rte_eth_dev *dev)
  {
      struct bond_dev_private *internals = dev->data->dev_private;
      uint16_t bond_port_id = internals->port_id;
-    int skipped = 0;
      struct rte_flow_error ferror;
-
-    if (rte_eal_process_type() != RTE_PROC_PRIMARY)
-        return 0;
-
-    RTE_BOND_LOG(INFO, "Closing bonded device %s", dev->device->name);
+    int skipped = 0;
      /* Flush flows in all back-end devices before removing them */
      bond_flow_ops.flush(dev, &ferror);
@@ -2176,6 +2171,20 @@ bond_ethdev_close(struct rte_eth_dev *dev)
              skipped++;
          }
      }
+}
+
+int
+bond_ethdev_close(struct rte_eth_dev *dev)
+{
+    struct bond_dev_private *internals = dev->data->dev_private;
+
+    if (rte_eal_process_type() != RTE_PROC_PRIMARY)
+        return 0;
+
+    RTE_BOND_LOG(INFO, "Closing bonded device %s", dev->device->name);
+
+    bond_ethdev_cfg_cleanup(dev);
+
      bond_ethdev_free_queues(dev);
      rte_bitmap_reset(internals->vlan_filter_bmp);
      rte_bitmap_free(internals->vlan_filter_bmp);
@@ -3606,6 +3615,8 @@ bond_ethdev_configure(struct rte_eth_dev *dev)
      unsigned i, j;
+    bond_ethdev_cfg_cleanup(dev);
+

You might want a space after the variable declaration section. It makes
it easier to read.

I can fix it on applying. May I add your ack?


      /*
       * If RSS is enabled, fill table with default values and
       * set key to the value specified in port RSS configuration.


Reply via email to