diff mbox series

[v2] net/bonding: make bonded device configure method re-entrant

Message ID 20221018194549.635113-1-ivan.malov@oktetlabs.ru (mailing list archive)
State Accepted, archived
Delegated to: Andrew Rybchenko
Headers
Series [v2] net/bonding: make bonded device configure method re-entrant |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation success Compilation OK
ci/intel-Testing success Testing PASS
ci/github-robot: build success github build: passed
ci/iol-mellanox-Performance success Performance Testing PASS
ci/iol-aarch64-unit-testing success Testing PASS
ci/iol-aarch64-compile-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS

Commit Message

Ivan Malov Oct. 18, 2022, 7:45 p.m. UTC 
  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.malov@oktetlabs.ru>
Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
Acked-by: Chas Williams <3chas3@gmail.com>
---
 drivers/net/bonding/rte_eth_bond_pmd.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

Comments

Andrew Rybchenko Oct. 19, 2022, 10:26 a.m. UTC | #1 
On 10/18/22 22:45, 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.malov@oktetlabs.ru>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Chas Williams <3chas3@gmail.com>

Applied to dpdk-next-net/main, thanks.
Yu Jiang Nov. 1, 2022, 10:56 a.m. UTC | #2 
Hi Ivan,

This patch introduces a bug https://bugs.dpdk.org/show_bug.cgi?id=1119, can you pls have a look quickly?

Best regards,
Yu Jiang

> -----Original Message-----
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> Sent: Wednesday, October 19, 2022 3:46 AM
> To: dev@dpdk.org
> Cc: Chas Williams <3chas3@gmail.com>; Min Hu (Connor)
> <humin29@huawei.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Chas Williams <chas3@att.com>; Burakov,
> Anatoly <anatoly.burakov@intel.com>
> Subject: [PATCH v2] net/bonding: make bonded device configure method re-
> entrant
> 
> 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.malov@oktetlabs.ru>
> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> Acked-by: Chas Williams <3chas3@gmail.com>
> ---
Ivan Malov Nov. 1, 2022, 4:33 p.m. UTC | #3 
Hi,

Thanks for noticing the bug. A fix has been proposed already:
https://patches.dpdk.org/project/dpdk/patch/20221101161853.2702425-1-ivan.malov@oktetlabs.ru/

With the fix applied, does the problem still manifest itself?
Please let me know.

Thank you.

On Tue, 1 Nov 2022, Jiang, YuX wrote:

> Hi Ivan,
>
> This patch introduces a bug https://bugs.dpdk.org/show_bug.cgi?id=1119, can you pls have a look quickly?
>
> Best regards,
> Yu Jiang
>
>> -----Original Message-----
>> From: Ivan Malov <ivan.malov@oktetlabs.ru>
>> Sent: Wednesday, October 19, 2022 3:46 AM
>> To: dev@dpdk.org
>> Cc: Chas Williams <3chas3@gmail.com>; Min Hu (Connor)
>> <humin29@huawei.com>; Andrew Rybchenko
>> <andrew.rybchenko@oktetlabs.ru>; Chas Williams <chas3@att.com>; Burakov,
>> Anatoly <anatoly.burakov@intel.com>
>> Subject: [PATCH v2] net/bonding: make bonded device configure method re-
>> entrant
>>
>> 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.malov@oktetlabs.ru>
>> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
>> Acked-by: Chas Williams <3chas3@gmail.com>
>> ---
>
>
Yu Jiang Nov. 2, 2022, 5:20 a.m. UTC | #4 
Hi Ivan,

https://bugs.dpdk.org/show_bug.cgi?id=1119
https://bugs.dpdk.org/show_bug.cgi?id=1118
Can be fixed by patches: 
	https://patches.dpdk.org/project/dpdk/patch/20221101161853.2702425-1-ivan.malov@oktetlabs.ru/
	https://patches.dpdk.org/project/dpdk/patch/20221031131744.2340150-1-ivan.malov@oktetlabs.ru/

Best regards,
Yu Jiang

> -----Original Message-----
> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> Sent: Wednesday, November 2, 2022 12:34 AM
> To: Jiang, YuX <yux.jiang@intel.com>
> Cc: dev@dpdk.org; Chas Williams <3chas3@gmail.com>; Min Hu (Connor)
> <humin29@huawei.com>; Andrew Rybchenko
> <andrew.rybchenko@oktetlabs.ru>; Chas Williams <chas3@att.com>; Burakov,
> Anatoly <anatoly.burakov@intel.com>
> Subject: RE: [PATCH v2] net/bonding: make bonded device configure method
> re-entrant
> 
> Hi,
> 
> Thanks for noticing the bug. A fix has been proposed already:
> https://patches.dpdk.org/project/dpdk/patch/20221101161853.2702425-1-
> ivan.malov@oktetlabs.ru/
> 
> With the fix applied, does the problem still manifest itself?
> Please let me know.
> 
> Thank you.
> 
> On Tue, 1 Nov 2022, Jiang, YuX wrote:
> 
> > Hi Ivan,
> >
> > This patch introduces a bug https://bugs.dpdk.org/show_bug.cgi?id=1119, can
> you pls have a look quickly?
> >
> > Best regards,
> > Yu Jiang
> >
> >> -----Original Message-----
> >> From: Ivan Malov <ivan.malov@oktetlabs.ru>
> >> Sent: Wednesday, October 19, 2022 3:46 AM
> >> To: dev@dpdk.org
> >> Cc: Chas Williams <3chas3@gmail.com>; Min Hu (Connor)
> >> <humin29@huawei.com>; Andrew Rybchenko
> >> <andrew.rybchenko@oktetlabs.ru>; Chas Williams <chas3@att.com>;
> >> Burakov, Anatoly <anatoly.burakov@intel.com>
> >> Subject: [PATCH v2] net/bonding: make bonded device configure method
> >> re- entrant
> >>
> >> 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.malov@oktetlabs.ru>
> >> Reviewed-by: Andrew Rybchenko <andrew.rybchenko@oktetlabs.ru>
> >> Acked-by: Chas Williams <3chas3@gmail.com>
> >> ---
> >
> >
diff mbox series

Patch

diff --git a/drivers/net/bonding/rte_eth_bond_pmd.c b/drivers/net/bonding/rte_eth_bond_pmd.c
index 27005c747c..486b7fc9f7 100644
--- a/drivers/net/bonding/rte_eth_bond_pmd.c
+++ b/drivers/net/bonding/rte_eth_bond_pmd.c
@@ -2148,18 +2148,14 @@  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);
 	while (internals->slave_count != skipped) {
 		uint16_t port_id = internals->slaves[skipped].port_id;
 
@@ -2178,6 +2174,20 @@  bond_ethdev_close(struct rte_eth_dev *dev)
 		}
 	}
 	bond_flow_ops.flush(dev, &ferror);
+}
+
+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);
@@ -3610,6 +3620,9 @@  bond_ethdev_configure(struct rte_eth_dev *dev)
 
 	unsigned i, j;
 
+
+	bond_ethdev_cfg_cleanup(dev);
+
 	/*
 	 * If RSS is enabled, fill table with default values and
 	 * set key to the value specified in port RSS configuration.