diff mbox series

app/testpmd: fix bonding mode set

Message ID 20220128023519.10027-1-humin29@huawei.com (mailing list archive)
State Accepted, archived
Delegated to: Ferruh Yigit
Headers
Series app/testpmd: fix bonding mode set |

Checks

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

Commit Message

humin (Q) Jan. 28, 2022, 2:35 a.m. UTC 
  when start testpmd, and type command like this, it will lead to
Segmentation fault, like:

testpmd> create bonded device 4 0
testpmd> add bonding slave 0 2
testpmd> add bonding slave 1 2
testpmd> port start 2
testpmd> set bonding mode 0 2
testpmd> quit
Stopping port 0...
Stopping ports...
...
Bye...
Segmentation fault

The reason to the bug is that rte timer do not be cancelled when quit.
That is, in 'bond_ethdev_start', resources are allocated according to
different bonding mode. In 'bond_ethdev_stop', resources are free by
the corresponding mode.

For example, 'bond_ethdev_start' start bond_mode_8023ad_ext_periodic_cb
timer for bonding mode 4. and 'bond_ethdev_stop' cancel the timer only
when the current bonding mode is 4. If the bonding mode is changed,
and directly quit the process, the timer will still on, and freed memory
will be accessed, then segmentation fault.

'bonding mode'changed means resources changed, reallocate resources for
different mode should be done, that is, device should be restarted.

Fixes: 2950a769315e ("bond: testpmd support")
Cc: stable@dpdk.org

Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
---
 app/test-pmd/cmdline.c | 13 +++++++++++++
 1 file changed, 13 insertions(+)

Comments

Ferruh Yigit Jan. 31, 2022, 4:13 p.m. UTC | #1 
On 1/28/2022 2:35 AM, Min Hu (Connor) wrote:
> when start testpmd, and type command like this, it will lead to
> Segmentation fault, like:
> 
> testpmd> create bonded device 4 0
> testpmd> add bonding slave 0 2
> testpmd> add bonding slave 1 2
> testpmd> port start 2
> testpmd> set bonding mode 0 2
> testpmd> quit
> Stopping port 0...
> Stopping ports...
> ...
> Bye...
> Segmentation fault
> 
> The reason to the bug is that rte timer do not be cancelled when quit.
> That is, in 'bond_ethdev_start', resources are allocated according to
> different bonding mode. In 'bond_ethdev_stop', resources are free by
> the corresponding mode.
> 
> For example, 'bond_ethdev_start' start bond_mode_8023ad_ext_periodic_cb
> timer for bonding mode 4. and 'bond_ethdev_stop' cancel the timer only
> when the current bonding mode is 4. If the bonding mode is changed,
> and directly quit the process, the timer will still on, and freed memory
> will be accessed, then segmentation fault.
> 
> 'bonding mode'changed means resources changed, reallocate resources for

'bonding mode' changed ...

> different mode should be done, that is, device should be restarted.
> 
> Fixes: 2950a769315e ("bond: testpmd support")
> Cc: stable@dpdk.org
> 
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>

Tested-by: Ferruh Yigit <ferruh.yigit@intel.com>

Applied to dpdk-next-net/main, thanks.

> ---
>   app/test-pmd/cmdline.c | 13 +++++++++++++
>   1 file changed, 13 insertions(+)
> 
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index e626b1c7d9..2c47ab0f18 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -5915,6 +5915,19 @@ static void cmd_set_bonding_mode_parsed(void *parsed_result,
>   {
>   	struct cmd_set_bonding_mode_result *res = parsed_result;
>   	portid_t port_id = res->port_id;
> +	struct rte_port *port = &ports[port_id];
> +
> +	/*
> +	 * Bonding mode changed means resources of device changed, like whether
> +	 * started rte timer or not. Device should be restarted when resources
> +	 * of device changed.
> +	 */
> +	if (port->port_status != RTE_PORT_STOPPED) {
> +		fprintf(stderr,
> +			"\t Error: Can't config bonding mode when port %d is not stopped\n",

Updated as "... Can't set bonding mode ..."

> +			port_id);
> +		return;
> +	}
>   
>   	/* Set the bonding mode for the relevant port. */
>   	if (0 != rte_eth_bond_mode_set(port_id, res->value))
diff mbox series

Patch

diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
index e626b1c7d9..2c47ab0f18 100644
--- a/app/test-pmd/cmdline.c
+++ b/app/test-pmd/cmdline.c
@@ -5915,6 +5915,19 @@  static void cmd_set_bonding_mode_parsed(void *parsed_result,
 {
 	struct cmd_set_bonding_mode_result *res = parsed_result;
 	portid_t port_id = res->port_id;
+	struct rte_port *port = &ports[port_id];
+
+	/*
+	 * Bonding mode changed means resources of device changed, like whether
+	 * started rte timer or not. Device should be restarted when resources
+	 * of device changed.
+	 */
+	if (port->port_status != RTE_PORT_STOPPED) {
+		fprintf(stderr,
+			"\t Error: Can't config bonding mode when port %d is not stopped\n",
+			port_id);
+		return;
+	}
 
 	/* Set the bonding mode for the relevant port. */
 	if (0 != rte_eth_bond_mode_set(port_id, res->value))