[v3,3/5] app/testpmd: fix port status of slave device
Checks
Commit Message
From: Huisong Li <lihuisong@huawei.com>
Starting or stopping a bonded port also starts or stops all active slaves
under the bonded port. If this port is a bonded device, we need to modify
the port status of all slaves.
Fixes: 0e545d3047fe ("app/testpmd: check stopping port is not in bonding")
Cc: stable@dpdk.org
Signed-off-by: Huisong Li <lihuisong@huawei.com>
Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
Acked-by: Aman Singh <aman.deep.singh@intel.com>
---
app/test-pmd/cmdline.c | 1 +
app/test-pmd/testpmd.c | 74 +++++++++++++++++++++++++++++++++++++++---
app/test-pmd/testpmd.h | 3 +-
3 files changed, 73 insertions(+), 5 deletions(-)
Comments
03/05/2022 11:02, Min Hu (Connor) пишет:
> From: Huisong Li <lihuisong@huawei.com>
>
> Starting or stopping a bonded port also starts or stops all active slaves
> under the bonded port. If this port is a bonded device, we need to modify
> the port status of all slaves.
>
> Fixes: 0e545d3047fe ("app/testpmd: check stopping port is not in bonding")
> Cc: stable@dpdk.org
>
> Signed-off-by: Huisong Li <lihuisong@huawei.com>
> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
> Acked-by: Aman Singh <aman.deep.singh@intel.com>
> ---
> app/test-pmd/cmdline.c | 1 +
> app/test-pmd/testpmd.c | 74 +++++++++++++++++++++++++++++++++++++++---
> app/test-pmd/testpmd.h | 3 +-
> 3 files changed, 73 insertions(+), 5 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 6ffea8e21a..d9fc7a88bd 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -6671,6 +6671,7 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
> "Failed to enable promiscuous mode for port %u: %s - ignore\n",
> port_id, rte_strerror(-ret));
>
> + ports[port_id].bond_flag = 1;
> ports[port_id].need_setup = 0;
> ports[port_id].port_status = RTE_PORT_STOPPED;
> }
> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
> index fe2ce19f99..dc90600787 100644
> --- a/app/test-pmd/testpmd.c
> +++ b/app/test-pmd/testpmd.c
> @@ -66,6 +66,9 @@
> #ifdef RTE_EXEC_ENV_WINDOWS
> #include <process.h>
> #endif
> +#ifdef RTE_NET_BOND
> +#include <rte_eth_bond.h>
> +#endif
>
> #include "testpmd.h"
>
> @@ -597,11 +600,57 @@ eth_dev_configure_mp(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
> return 0;
> }
>
> +#ifdef RTE_NET_BOND
> +static int
> +change_bonding_slave_port_status(portid_t bond_pid, bool is_stop)
> +{
> + portid_t slave_pids[RTE_MAX_ETHPORTS];
> + struct rte_port *port;
> + int num_slaves;
> + portid_t slave_pid;
> + int i;
> +
> + num_slaves = rte_eth_bond_slaves_get(bond_pid, slave_pids,
> + RTE_MAX_ETHPORTS);
> + if (num_slaves < 0) {
> + fprintf(stderr, "Failed to get slave list for port = %u\n",
> + bond_pid);
> + return num_slaves;
> + }
> +
> + for (i = 0; i < num_slaves; i++) {
> + slave_pid = slave_pids[i];
> + port = &ports[slave_pid];
> + port->port_status =
> + is_stop ? RTE_PORT_STOPPED : RTE_PORT_STARTED;
> + }
> +
> + return 0;
> +}
> +#endif
> +
> static int
> eth_dev_start_mp(uint16_t port_id)
> {
> - if (is_proc_primary())
> - return rte_eth_dev_start(port_id);
> + int ret;
> +
> + if (is_proc_primary()) {
> + ret = rte_eth_dev_start(port_id);
> + if (ret != 0)
> + return ret;
> +
> +#ifdef RTE_NET_BOND
> + struct rte_port *port = &ports[port_id];
> +
> + /*
> + * Starting a bonded port also starts all slaves under the bonded
> + * device. So if this port is bond device, we need to modify the
> + * port status of these slaves.
> + */
> + if (port->bond_flag == 1)
> + return change_bonding_slave_port_status(port_id, false);
> +#endif
> + }
>
> return 0;
> }
> @@ -609,8 +658,25 @@ eth_dev_start_mp(uint16_t port_id)
> static int
> eth_dev_stop_mp(uint16_t port_id)
> {
> - if (is_proc_primary())
> - return rte_eth_dev_stop(port_id);
> + int ret;
> +
> + if (is_proc_primary()) {
> + ret = rte_eth_dev_stop(port_id);
> + if (ret != 0)
> + return ret;
> +
> +#ifdef RTE_NET_BOND
Here and in other places - probably no need to pollute the code
with all these 'ifdef RTE_NET_BOND'.
I suppose this logic (for checking is bonding API present or not)
can be hidden inside change_bonding_slave_port_status() itself.
> + struct rte_port *port = &ports[port_id];
> +
> + /*
> + * Stopping a bonded port also stops all slaves under the bonded
> + * device. So if this port is bond device, we need to modify the
> + * port status of these slaves.
> + */
> + if (port->bond_flag == 1)
> + return change_bonding_slave_port_status(port_id, true);
> +#endif
> + }
>
> return 0;
> }
> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
> index 31f766c965..67f253b30e 100644
> --- a/app/test-pmd/testpmd.h
> +++ b/app/test-pmd/testpmd.h
> @@ -266,7 +266,8 @@ struct rte_port {
> uint32_t mc_addr_nb; /**< nb. of addr. in mc_addr_pool */
> queueid_t queue_nb; /**< nb. of queues for flow rules */
> uint32_t queue_sz; /**< size of a queue for flow rules */
> - uint8_t slave_flag; /**< bonding slave port */
> + uint8_t slave_flag : 1, /**< bonding slave port */
> + bond_flag : 1; /**< port is bond device */
> struct port_template *pattern_templ_list; /**< Pattern templates. */
> struct port_template *actions_templ_list; /**< Actions templates. */
> struct port_table *table_list; /**< Flow tables. */
Hi, Konstantin,
在 2022/5/4 7:39, Konstantin Ananyev 写道:
> 03/05/2022 11:02, Min Hu (Connor) пишет:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> Starting or stopping a bonded port also starts or stops all active slaves
>> under the bonded port. If this port is a bonded device, we need to modify
>> the port status of all slaves.
>>
>> Fixes: 0e545d3047fe ("app/testpmd: check stopping port is not in
>> bonding")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>> Acked-by: Aman Singh <aman.deep.singh@intel.com>
>> ---
>> app/test-pmd/cmdline.c | 1 +
>> app/test-pmd/testpmd.c | 74 +++++++++++++++++++++++++++++++++++++++---
>> app/test-pmd/testpmd.h | 3 +-
>> 3 files changed, 73 insertions(+), 5 deletions(-)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index 6ffea8e21a..d9fc7a88bd 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -6671,6 +6671,7 @@ static void cmd_create_bonded_device_parsed(void
>> *parsed_result,
>> "Failed to enable promiscuous mode for port %u: %s -
>> ignore\n",
>> port_id, rte_strerror(-ret));
>> + ports[port_id].bond_flag = 1;
>> ports[port_id].need_setup = 0;
>> ports[port_id].port_status = RTE_PORT_STOPPED;
>> }
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index fe2ce19f99..dc90600787 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -66,6 +66,9 @@
>> #ifdef RTE_EXEC_ENV_WINDOWS
>> #include <process.h>
>> #endif
>> +#ifdef RTE_NET_BOND
>> +#include <rte_eth_bond.h>
>> +#endif
>> #include "testpmd.h"
>> @@ -597,11 +600,57 @@ eth_dev_configure_mp(uint16_t port_id, uint16_t
>> nb_rx_q, uint16_t nb_tx_q,
>> return 0;
>> }
>> +#ifdef RTE_NET_BOND
>> +static int
>> +change_bonding_slave_port_status(portid_t bond_pid, bool is_stop)
>> +{
>> + portid_t slave_pids[RTE_MAX_ETHPORTS];
>> + struct rte_port *port;
>> + int num_slaves;
>> + portid_t slave_pid;
>> + int i;
>> +
>> + num_slaves = rte_eth_bond_slaves_get(bond_pid, slave_pids,
>> + RTE_MAX_ETHPORTS);
>> + if (num_slaves < 0) {
>> + fprintf(stderr, "Failed to get slave list for port = %u\n",
>> + bond_pid);
>> + return num_slaves;
>> + }
>> +
>> + for (i = 0; i < num_slaves; i++) {
>> + slave_pid = slave_pids[i];
>> + port = &ports[slave_pid];
>> + port->port_status =
>> + is_stop ? RTE_PORT_STOPPED : RTE_PORT_STARTED;
>> + }
>> +
>> + return 0;
>> +}
>> +#endif
>> +
>> static int
>> eth_dev_start_mp(uint16_t port_id)
>> {
>> - if (is_proc_primary())
>> - return rte_eth_dev_start(port_id);
>> + int ret;
>> +
>> + if (is_proc_primary()) {
>> + ret = rte_eth_dev_start(port_id);
>> + if (ret != 0)
>> + return ret;
>> +
>> +#ifdef RTE_NET_BOND
>> + struct rte_port *port = &ports[port_id];
>> +
>> + /*
>> + * Starting a bonded port also starts all slaves under the
>> bonded
>> + * device. So if this port is bond device, we need to modify the
>> + * port status of these slaves.
>> + */
>> + if (port->bond_flag == 1)
>> + return change_bonding_slave_port_status(port_id, false);
>> +#endif
>> + }
>> return 0;
>> }
>> @@ -609,8 +658,25 @@ eth_dev_start_mp(uint16_t port_id)
>> static int
>> eth_dev_stop_mp(uint16_t port_id)
>> {
>> - if (is_proc_primary())
>> - return rte_eth_dev_stop(port_id);
>> + int ret;
>> +
>> + if (is_proc_primary()) {
>> + ret = rte_eth_dev_stop(port_id);
>> + if (ret != 0)
>> + return ret;
>> +
>> +#ifdef RTE_NET_BOND
>
> Here and in other places - probably no need to pollute the code
> with all these 'ifdef RTE_NET_BOND'.
> I suppose this logic (for checking is bonding API present or not)
> can be hidden inside change_bonding_slave_port_status() itself.
>
I think it does not pollute the code. anyone can tell according to
the flag 'ifdef RTE_NET_BOND'.
if hiddle inside 'change_bonding_slave_port_status', it will be weird.
For example, if the port is not bonding port, It will also invoke
'change_bonding_slave_port_status'. That is unreasonable.
>
>> + struct rte_port *port = &ports[port_id];
>> +
>> + /*
>> + * Stopping a bonded port also stops all slaves under the bonded
>> + * device. So if this port is bond device, we need to modify the
>> + * port status of these slaves.
>> + */
>> + if (port->bond_flag == 1)
>> + return change_bonding_slave_port_status(port_id, true);
>> +#endif
>> + }
>> return 0;
>> }
>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>> index 31f766c965..67f253b30e 100644
>> --- a/app/test-pmd/testpmd.h
>> +++ b/app/test-pmd/testpmd.h
>> @@ -266,7 +266,8 @@ struct rte_port {
>> uint32_t mc_addr_nb; /**< nb. of addr. in
>> mc_addr_pool */
>> queueid_t queue_nb; /**< nb. of queues for flow
>> rules */
>> uint32_t queue_sz; /**< size of a queue for flow
>> rules */
>> - uint8_t slave_flag; /**< bonding slave port */
>> + uint8_t slave_flag : 1, /**< bonding slave port */
>> + bond_flag : 1; /**< port is bond device */
>> struct port_template *pattern_templ_list; /**< Pattern
>> templates. */
>> struct port_template *actions_templ_list; /**< Actions
>> templates. */
>> struct port_table *table_list; /**< Flow tables. */
>
> .
Hi Conor,
> 在 2022/5/4 7:39, Konstantin Ananyev 写道:
>> 03/05/2022 11:02, Min Hu (Connor) пишет:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> Starting or stopping a bonded port also starts or stops all active
>>> slaves
>>> under the bonded port. If this port is a bonded device, we need to
>>> modify
>>> the port status of all slaves.
>>>
>>> Fixes: 0e545d3047fe ("app/testpmd: check stopping port is not in
>>> bonding")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>> Acked-by: Aman Singh <aman.deep.singh@intel.com>
>>> ---
>>> app/test-pmd/cmdline.c | 1 +
>>> app/test-pmd/testpmd.c | 74 +++++++++++++++++++++++++++++++++++++++---
>>> app/test-pmd/testpmd.h | 3 +-
>>> 3 files changed, 73 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>> index 6ffea8e21a..d9fc7a88bd 100644
>>> --- a/app/test-pmd/cmdline.c
>>> +++ b/app/test-pmd/cmdline.c
>>> @@ -6671,6 +6671,7 @@ static void
>>> cmd_create_bonded_device_parsed(void *parsed_result,
>>> "Failed to enable promiscuous mode for port %u: %s
>>> - ignore\n",
>>> port_id, rte_strerror(-ret));
>>> + ports[port_id].bond_flag = 1;
>>> ports[port_id].need_setup = 0;
>>> ports[port_id].port_status = RTE_PORT_STOPPED;
>>> }
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>> index fe2ce19f99..dc90600787 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -66,6 +66,9 @@
>>> #ifdef RTE_EXEC_ENV_WINDOWS
>>> #include <process.h>
>>> #endif
>>> +#ifdef RTE_NET_BOND
>>> +#include <rte_eth_bond.h>
>>> +#endif
>>> #include "testpmd.h"
>>> @@ -597,11 +600,57 @@ eth_dev_configure_mp(uint16_t port_id, uint16_t
>>> nb_rx_q, uint16_t nb_tx_q,
>>> return 0;
>>> }
>>> +#ifdef RTE_NET_BOND
>>> +static int
>>> +change_bonding_slave_port_status(portid_t bond_pid, bool is_stop)
>>> +{
>>> + portid_t slave_pids[RTE_MAX_ETHPORTS];
>>> + struct rte_port *port;
>>> + int num_slaves;
>>> + portid_t slave_pid;
>>> + int i;
>>> +
>>> + num_slaves = rte_eth_bond_slaves_get(bond_pid, slave_pids,
>>> + RTE_MAX_ETHPORTS);
>>> + if (num_slaves < 0) {
>>> + fprintf(stderr, "Failed to get slave list for port = %u\n",
>>> + bond_pid);
>>> + return num_slaves;
>>> + }
>>> +
>>> + for (i = 0; i < num_slaves; i++) {
>>> + slave_pid = slave_pids[i];
>>> + port = &ports[slave_pid];
>>> + port->port_status =
>>> + is_stop ? RTE_PORT_STOPPED : RTE_PORT_STARTED;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +#endif
>>> +
>>> static int
>>> eth_dev_start_mp(uint16_t port_id)
>>> {
>>> - if (is_proc_primary())
>>> - return rte_eth_dev_start(port_id);
>>> + int ret;
>>> +
>>> + if (is_proc_primary()) {
>>> + ret = rte_eth_dev_start(port_id);
>>> + if (ret != 0)
>>> + return ret;
>>> +
>>> +#ifdef RTE_NET_BOND
>>> + struct rte_port *port = &ports[port_id];
>>> +
>>> + /*
>>> + * Starting a bonded port also starts all slaves under the
>>> bonded
>>> + * device. So if this port is bond device, we need to modify
>>> the
>>> + * port status of these slaves.
>>> + */
>>> + if (port->bond_flag == 1)
>>> + return change_bonding_slave_port_status(port_id, false);
>>> +#endif
>>> + }
>>> return 0;
>>> }
>>> @@ -609,8 +658,25 @@ eth_dev_start_mp(uint16_t port_id)
>>> static int
>>> eth_dev_stop_mp(uint16_t port_id)
>>> {
>>> - if (is_proc_primary())
>>> - return rte_eth_dev_stop(port_id);
>>> + int ret;
>>> +
>>> + if (is_proc_primary()) {
>>> + ret = rte_eth_dev_stop(port_id);
>>> + if (ret != 0)
>>> + return ret;
>>> +
>>> +#ifdef RTE_NET_BOND
>>
>> Here and in other places - probably no need to pollute the code
>> with all these 'ifdef RTE_NET_BOND'.
>> I suppose this logic (for checking is bonding API present or not)
>> can be hidden inside change_bonding_slave_port_status() itself.
>>
> I think it does not pollute the code. anyone can tell according to
> the flag 'ifdef RTE_NET_BOND'.
That what I am talking about.
Spreading ifdefed code all around the palce is not a good thing.
It makes it harder to read, understand and maintain.
Much more plausible is to hide that logic in one place whenever possible.
> if hiddle inside 'change_bonding_slave_port_status', it will be weird.
I don't see why is that.
Let say if bonding is disabled that function can do nothing or even
return an error to avoid it's misuse.
> For example, if the port is not bonding port, It will also invoke
> 'change_bonding_slave_port_status'. That is unreasonable.
As I can read the code, hange_bonding_slave_port_status() will be
invoked only when port->bond_flag is set. Which implies that port is a
bonded one, no?
>
>
>>
>>> + struct rte_port *port = &ports[port_id];
>>> +
>>> + /*
>>> + * Stopping a bonded port also stops all slaves under the
>>> bonded
>>> + * device. So if this port is bond device, we need to modify
>>> the
>>> + * port status of these slaves.
>>> + */
>>> + if (port->bond_flag == 1)
>>> + return change_bonding_slave_port_status(port_id, true);
>>> +#endif
>>> + }
>>> return 0;
>>> }
>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>>> index 31f766c965..67f253b30e 100644
>>> --- a/app/test-pmd/testpmd.h
>>> +++ b/app/test-pmd/testpmd.h
>>> @@ -266,7 +266,8 @@ struct rte_port {
>>> uint32_t mc_addr_nb; /**< nb. of addr. in
>>> mc_addr_pool */
>>> queueid_t queue_nb; /**< nb. of queues for flow
>>> rules */
>>> uint32_t queue_sz; /**< size of a queue for flow
>>> rules */
>>> - uint8_t slave_flag; /**< bonding slave port */
>>> + uint8_t slave_flag : 1, /**< bonding slave port */
>>> + bond_flag : 1; /**< port is bond device */
>>> struct port_template *pattern_templ_list; /**< Pattern
>>> templates. */
>>> struct port_template *actions_templ_list; /**< Actions
>>> templates. */
>>> struct port_table *table_list; /**< Flow tables. */
>>
>> .
On 5/6/2022 9:16 AM, Min Hu (Connor) wrote:
> Hi, Konstantin,
>
> 在 2022/5/4 7:39, Konstantin Ananyev 写道:
>> 03/05/2022 11:02, Min Hu (Connor) пишет:
>>> From: Huisong Li <lihuisong@huawei.com>
>>>
>>> Starting or stopping a bonded port also starts or stops all active
>>> slaves
>>> under the bonded port. If this port is a bonded device, we need to
>>> modify
>>> the port status of all slaves.
>>>
>>> Fixes: 0e545d3047fe ("app/testpmd: check stopping port is not in
>>> bonding")
>>> Cc: stable@dpdk.org
>>>
>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>> Acked-by: Aman Singh <aman.deep.singh@intel.com>
>>> ---
>>> app/test-pmd/cmdline.c | 1 +
>>> app/test-pmd/testpmd.c | 74 +++++++++++++++++++++++++++++++++++++++---
>>> app/test-pmd/testpmd.h | 3 +-
>>> 3 files changed, 73 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>> index 6ffea8e21a..d9fc7a88bd 100644
>>> --- a/app/test-pmd/cmdline.c
>>> +++ b/app/test-pmd/cmdline.c
>>> @@ -6671,6 +6671,7 @@ static void
>>> cmd_create_bonded_device_parsed(void *parsed_result,
>>> "Failed to enable promiscuous mode for port %u: %s
>>> - ignore\n",
>>> port_id, rte_strerror(-ret));
>>> + ports[port_id].bond_flag = 1;
>>> ports[port_id].need_setup = 0;
>>> ports[port_id].port_status = RTE_PORT_STOPPED;
>>> }
>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>> index fe2ce19f99..dc90600787 100644
>>> --- a/app/test-pmd/testpmd.c
>>> +++ b/app/test-pmd/testpmd.c
>>> @@ -66,6 +66,9 @@
>>> #ifdef RTE_EXEC_ENV_WINDOWS
>>> #include <process.h>
>>> #endif
>>> +#ifdef RTE_NET_BOND
>>> +#include <rte_eth_bond.h>
>>> +#endif
>>> #include "testpmd.h"
>>> @@ -597,11 +600,57 @@ eth_dev_configure_mp(uint16_t port_id, uint16_t
>>> nb_rx_q, uint16_t nb_tx_q,
>>> return 0;
>>> }
>>> +#ifdef RTE_NET_BOND
>>> +static int
>>> +change_bonding_slave_port_status(portid_t bond_pid, bool is_stop)
>>> +{
>>> + portid_t slave_pids[RTE_MAX_ETHPORTS];
>>> + struct rte_port *port;
>>> + int num_slaves;
>>> + portid_t slave_pid;
>>> + int i;
>>> +
>>> + num_slaves = rte_eth_bond_slaves_get(bond_pid, slave_pids,
>>> + RTE_MAX_ETHPORTS);
>>> + if (num_slaves < 0) {
>>> + fprintf(stderr, "Failed to get slave list for port = %u\n",
>>> + bond_pid);
>>> + return num_slaves;
>>> + }
>>> +
>>> + for (i = 0; i < num_slaves; i++) {
>>> + slave_pid = slave_pids[i];
>>> + port = &ports[slave_pid];
>>> + port->port_status =
>>> + is_stop ? RTE_PORT_STOPPED : RTE_PORT_STARTED;
>>> + }
>>> +
>>> + return 0;
>>> +}
>>> +#endif
>>> +
>>> static int
>>> eth_dev_start_mp(uint16_t port_id)
>>> {
>>> - if (is_proc_primary())
>>> - return rte_eth_dev_start(port_id);
>>> + int ret;
>>> +
>>> + if (is_proc_primary()) {
>>> + ret = rte_eth_dev_start(port_id);
>>> + if (ret != 0)
>>> + return ret;
>>> +
>>> +#ifdef RTE_NET_BOND
>>> + struct rte_port *port = &ports[port_id];
>>> +
>>> + /*
>>> + * Starting a bonded port also starts all slaves under the
>>> bonded
>>> + * device. So if this port is bond device, we need to modify
>>> the
>>> + * port status of these slaves.
>>> + */
>>> + if (port->bond_flag == 1)
>>> + return change_bonding_slave_port_status(port_id, false);
>>> +#endif
>>> + }
>>> return 0;
>>> }
>>> @@ -609,8 +658,25 @@ eth_dev_start_mp(uint16_t port_id)
>>> static int
>>> eth_dev_stop_mp(uint16_t port_id)
>>> {
>>> - if (is_proc_primary())
>>> - return rte_eth_dev_stop(port_id);
>>> + int ret;
>>> +
>>> + if (is_proc_primary()) {
>>> + ret = rte_eth_dev_stop(port_id);
>>> + if (ret != 0)
>>> + return ret;
>>> +
>>> +#ifdef RTE_NET_BOND
>>
>> Here and in other places - probably no need to pollute the code
>> with all these 'ifdef RTE_NET_BOND'.
>> I suppose this logic (for checking is bonding API present or not)
>> can be hidden inside change_bonding_slave_port_status() itself.
>>
> I think it does not pollute the code. anyone can tell according to
> the flag 'ifdef RTE_NET_BOND'.
> if hiddle inside 'change_bonding_slave_port_status', it will be weird.
> For example, if the port is not bonding port, It will also invoke
> 'change_bonding_slave_port_status'. That is unreasonable.
>
Hi Konstantin,
I also was not happy to have bonding (or any PMD) ifdef in the generic
(start()/stop()) functions, but the ifdef blocks updates testpmd
(application) level status. So that can't be handled in the PMD and need
to be in the application level.
Which is enforcing to have same PMD specific code in the testpmd level,
if you have any suggestion to prevent this, I am for it.
I will proceed with first two patch of this set, which fixes bonding PMD
issues, I will hold the testpmd ones for more comments.
Thanks,
ferruh
>
>>
>>> + struct rte_port *port = &ports[port_id];
>>> +
>>> + /*
>>> + * Stopping a bonded port also stops all slaves under the
>>> bonded
>>> + * device. So if this port is bond device, we need to modify
>>> the
>>> + * port status of these slaves.
>>> + */
>>> + if (port->bond_flag == 1)
>>> + return change_bonding_slave_port_status(port_id, true);
>>> +#endif
>>> + }
>>> return 0;
>>> }
>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>>> index 31f766c965..67f253b30e 100644
>>> --- a/app/test-pmd/testpmd.h
>>> +++ b/app/test-pmd/testpmd.h
>>> @@ -266,7 +266,8 @@ struct rte_port {
>>> uint32_t mc_addr_nb; /**< nb. of addr. in
>>> mc_addr_pool */
>>> queueid_t queue_nb; /**< nb. of queues for flow
>>> rules */
>>> uint32_t queue_sz; /**< size of a queue for flow
>>> rules */
>>> - uint8_t slave_flag; /**< bonding slave port */
>>> + uint8_t slave_flag : 1, /**< bonding slave port */
>>> + bond_flag : 1; /**< port is bond device */
>>> struct port_template *pattern_templ_list; /**< Pattern
>>> templates. */
>>> struct port_template *actions_templ_list; /**< Actions
>>> templates. */
>>> struct port_table *table_list; /**< Flow tables. */
>>
>> .
Hi Ferruh,
> On 5/6/2022 9:16 AM, Min Hu (Connor) wrote:
>> Hi, Konstantin,
>>
>> 在 2022/5/4 7:39, Konstantin Ananyev 写道:
>>> 03/05/2022 11:02, Min Hu (Connor) пишет:
>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>
>>>> Starting or stopping a bonded port also starts or stops all active
>>>> slaves
>>>> under the bonded port. If this port is a bonded device, we need to
>>>> modify
>>>> the port status of all slaves.
>>>>
>>>> Fixes: 0e545d3047fe ("app/testpmd: check stopping port is not in
>>>> bonding")
>>>> Cc: stable@dpdk.org
>>>>
>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>> Acked-by: Aman Singh <aman.deep.singh@intel.com>
>>>> ---
>>>> app/test-pmd/cmdline.c | 1 +
>>>> app/test-pmd/testpmd.c | 74
>>>> +++++++++++++++++++++++++++++++++++++++---
>>>> app/test-pmd/testpmd.h | 3 +-
>>>> 3 files changed, 73 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>>> index 6ffea8e21a..d9fc7a88bd 100644
>>>> --- a/app/test-pmd/cmdline.c
>>>> +++ b/app/test-pmd/cmdline.c
>>>> @@ -6671,6 +6671,7 @@ static void
>>>> cmd_create_bonded_device_parsed(void *parsed_result,
>>>> "Failed to enable promiscuous mode for port %u: %s
>>>> - ignore\n",
>>>> port_id, rte_strerror(-ret));
>>>> + ports[port_id].bond_flag = 1;
>>>> ports[port_id].need_setup = 0;
>>>> ports[port_id].port_status = RTE_PORT_STOPPED;
>>>> }
>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>>> index fe2ce19f99..dc90600787 100644
>>>> --- a/app/test-pmd/testpmd.c
>>>> +++ b/app/test-pmd/testpmd.c
>>>> @@ -66,6 +66,9 @@
>>>> #ifdef RTE_EXEC_ENV_WINDOWS
>>>> #include <process.h>
>>>> #endif
>>>> +#ifdef RTE_NET_BOND
>>>> +#include <rte_eth_bond.h>
>>>> +#endif
>>>> #include "testpmd.h"
>>>> @@ -597,11 +600,57 @@ eth_dev_configure_mp(uint16_t port_id,
>>>> uint16_t nb_rx_q, uint16_t nb_tx_q,
>>>> return 0;
>>>> }
>>>> +#ifdef RTE_NET_BOND
>>>> +static int
>>>> +change_bonding_slave_port_status(portid_t bond_pid, bool is_stop)
>>>> +{
>>>> + portid_t slave_pids[RTE_MAX_ETHPORTS];
>>>> + struct rte_port *port;
>>>> + int num_slaves;
>>>> + portid_t slave_pid;
>>>> + int i;
>>>> +
>>>> + num_slaves = rte_eth_bond_slaves_get(bond_pid, slave_pids,
>>>> + RTE_MAX_ETHPORTS);
>>>> + if (num_slaves < 0) {
>>>> + fprintf(stderr, "Failed to get slave list for port = %u\n",
>>>> + bond_pid);
>>>> + return num_slaves;
>>>> + }
>>>> +
>>>> + for (i = 0; i < num_slaves; i++) {
>>>> + slave_pid = slave_pids[i];
>>>> + port = &ports[slave_pid];
>>>> + port->port_status =
>>>> + is_stop ? RTE_PORT_STOPPED : RTE_PORT_STARTED;
>>>> + }
>>>> +
>>>> + return 0;
>>>> +}
>>>> +#endif
>>>> +
>>>> static int
>>>> eth_dev_start_mp(uint16_t port_id)
>>>> {
>>>> - if (is_proc_primary())
>>>> - return rte_eth_dev_start(port_id);
>>>> + int ret;
>>>> +
>>>> + if (is_proc_primary()) {
>>>> + ret = rte_eth_dev_start(port_id);
>>>> + if (ret != 0)
>>>> + return ret;
>>>> +
>>>> +#ifdef RTE_NET_BOND
>>>> + struct rte_port *port = &ports[port_id];
>>>> +
>>>> + /*
>>>> + * Starting a bonded port also starts all slaves under the
>>>> bonded
>>>> + * device. So if this port is bond device, we need to
>>>> modify the
>>>> + * port status of these slaves.
>>>> + */
>>>> + if (port->bond_flag == 1)
>>>> + return change_bonding_slave_port_status(port_id, false);
>>>> +#endif
>>>> + }
>>>> return 0;
>>>> }
>>>> @@ -609,8 +658,25 @@ eth_dev_start_mp(uint16_t port_id)
>>>> static int
>>>> eth_dev_stop_mp(uint16_t port_id)
>>>> {
>>>> - if (is_proc_primary())
>>>> - return rte_eth_dev_stop(port_id);
>>>> + int ret;
>>>> +
>>>> + if (is_proc_primary()) {
>>>> + ret = rte_eth_dev_stop(port_id);
>>>> + if (ret != 0)
>>>> + return ret;
>>>> +
>>>> +#ifdef RTE_NET_BOND
>>>
>>> Here and in other places - probably no need to pollute the code
>>> with all these 'ifdef RTE_NET_BOND'.
>>> I suppose this logic (for checking is bonding API present or not)
>>> can be hidden inside change_bonding_slave_port_status() itself.
>>>
>> I think it does not pollute the code. anyone can tell according to
>> the flag 'ifdef RTE_NET_BOND'.
>> if hiddle inside 'change_bonding_slave_port_status', it will be weird.
>> For example, if the port is not bonding port, It will also invoke
>> 'change_bonding_slave_port_status'. That is unreasonable.
>>
>
> Hi Konstantin,
>
> I also was not happy to have bonding (or any PMD) ifdef in the generic
> (start()/stop()) functions, but the ifdef blocks updates testpmd
> (application) level status. So that can't be handled in the PMD and need
> to be in the application level.
> Which is enforcing to have same PMD specific code in the testpmd level,
> if you have any suggestion to prevent this, I am for it.
I am not aking to move it to PMD.
What I am saying that this ifdef logic could be grouped in one place
(inside change_bonding_slave_port_status()) instead of spreading it
around multiple places.
>
> I will proceed with first two patch of this set, which fixes bonding PMD
> issues, I will hold the testpmd ones for more comments.
>
> Thanks,
> ferruh
>
>>
>>>
>>>> + struct rte_port *port = &ports[port_id];
>>>> +
>>>> + /*
>>>> + * Stopping a bonded port also stops all slaves under the
>>>> bonded
>>>> + * device. So if this port is bond device, we need to
>>>> modify the
>>>> + * port status of these slaves.
>>>> + */
>>>> + if (port->bond_flag == 1)
>>>> + return change_bonding_slave_port_status(port_id, true);
>>>> +#endif
>>>> + }
>>>> return 0;
>>>> }
>>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>>>> index 31f766c965..67f253b30e 100644
>>>> --- a/app/test-pmd/testpmd.h
>>>> +++ b/app/test-pmd/testpmd.h
>>>> @@ -266,7 +266,8 @@ struct rte_port {
>>>> uint32_t mc_addr_nb; /**< nb. of addr. in
>>>> mc_addr_pool */
>>>> queueid_t queue_nb; /**< nb. of queues for flow
>>>> rules */
>>>> uint32_t queue_sz; /**< size of a queue for
>>>> flow rules */
>>>> - uint8_t slave_flag; /**< bonding slave port */
>>>> + uint8_t slave_flag : 1, /**< bonding slave port */
>>>> + bond_flag : 1; /**< port is bond device */
>>>> struct port_template *pattern_templ_list; /**< Pattern
>>>> templates. */
>>>> struct port_template *actions_templ_list; /**< Actions
>>>> templates. */
>>>> struct port_table *table_list; /**< Flow tables. */
>>>
>>> .
>
Hi, Konstantin,
fixed in v4, thanks.
在 2022/5/11 5:48, Konstantin Ananyev 写道:
>
> Hi Ferruh,
>
>> On 5/6/2022 9:16 AM, Min Hu (Connor) wrote:
>>> Hi, Konstantin,
>>>
>>> 在 2022/5/4 7:39, Konstantin Ananyev 写道:
>>>> 03/05/2022 11:02, Min Hu (Connor) пишет:
>>>>> From: Huisong Li <lihuisong@huawei.com>
>>>>>
>>>>> Starting or stopping a bonded port also starts or stops all active
>>>>> slaves
>>>>> under the bonded port. If this port is a bonded device, we need to
>>>>> modify
>>>>> the port status of all slaves.
>>>>>
>>>>> Fixes: 0e545d3047fe ("app/testpmd: check stopping port is not in
>>>>> bonding")
>>>>> Cc: stable@dpdk.org
>>>>>
>>>>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>>>>> Signed-off-by: Min Hu (Connor) <humin29@huawei.com>
>>>>> Acked-by: Aman Singh <aman.deep.singh@intel.com>
>>>>> ---
>>>>> app/test-pmd/cmdline.c | 1 +
>>>>> app/test-pmd/testpmd.c | 74
>>>>> +++++++++++++++++++++++++++++++++++++++---
>>>>> app/test-pmd/testpmd.h | 3 +-
>>>>> 3 files changed, 73 insertions(+), 5 deletions(-)
>>>>>
>>>>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>>>>> index 6ffea8e21a..d9fc7a88bd 100644
>>>>> --- a/app/test-pmd/cmdline.c
>>>>> +++ b/app/test-pmd/cmdline.c
>>>>> @@ -6671,6 +6671,7 @@ static void
>>>>> cmd_create_bonded_device_parsed(void *parsed_result,
>>>>> "Failed to enable promiscuous mode for port %u:
>>>>> %s - ignore\n",
>>>>> port_id, rte_strerror(-ret));
>>>>> + ports[port_id].bond_flag = 1;
>>>>> ports[port_id].need_setup = 0;
>>>>> ports[port_id].port_status = RTE_PORT_STOPPED;
>>>>> }
>>>>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>>>>> index fe2ce19f99..dc90600787 100644
>>>>> --- a/app/test-pmd/testpmd.c
>>>>> +++ b/app/test-pmd/testpmd.c
>>>>> @@ -66,6 +66,9 @@
>>>>> #ifdef RTE_EXEC_ENV_WINDOWS
>>>>> #include <process.h>
>>>>> #endif
>>>>> +#ifdef RTE_NET_BOND
>>>>> +#include <rte_eth_bond.h>
>>>>> +#endif
>>>>> #include "testpmd.h"
>>>>> @@ -597,11 +600,57 @@ eth_dev_configure_mp(uint16_t port_id,
>>>>> uint16_t nb_rx_q, uint16_t nb_tx_q,
>>>>> return 0;
>>>>> }
>>>>> +#ifdef RTE_NET_BOND
>>>>> +static int
>>>>> +change_bonding_slave_port_status(portid_t bond_pid, bool is_stop)
>>>>> +{
>>>>> + portid_t slave_pids[RTE_MAX_ETHPORTS];
>>>>> + struct rte_port *port;
>>>>> + int num_slaves;
>>>>> + portid_t slave_pid;
>>>>> + int i;
>>>>> +
>>>>> + num_slaves = rte_eth_bond_slaves_get(bond_pid, slave_pids,
>>>>> + RTE_MAX_ETHPORTS);
>>>>> + if (num_slaves < 0) {
>>>>> + fprintf(stderr, "Failed to get slave list for port = %u\n",
>>>>> + bond_pid);
>>>>> + return num_slaves;
>>>>> + }
>>>>> +
>>>>> + for (i = 0; i < num_slaves; i++) {
>>>>> + slave_pid = slave_pids[i];
>>>>> + port = &ports[slave_pid];
>>>>> + port->port_status =
>>>>> + is_stop ? RTE_PORT_STOPPED : RTE_PORT_STARTED;
>>>>> + }
>>>>> +
>>>>> + return 0;
>>>>> +}
>>>>> +#endif
>>>>> +
>>>>> static int
>>>>> eth_dev_start_mp(uint16_t port_id)
>>>>> {
>>>>> - if (is_proc_primary())
>>>>> - return rte_eth_dev_start(port_id);
>>>>> + int ret;
>>>>> +
>>>>> + if (is_proc_primary()) {
>>>>> + ret = rte_eth_dev_start(port_id);
>>>>> + if (ret != 0)
>>>>> + return ret;
>>>>> +
>>>>> +#ifdef RTE_NET_BOND
>>>>> + struct rte_port *port = &ports[port_id];
>>>>> +
>>>>> + /*
>>>>> + * Starting a bonded port also starts all slaves under the
>>>>> bonded
>>>>> + * device. So if this port is bond device, we need to
>>>>> modify the
>>>>> + * port status of these slaves.
>>>>> + */
>>>>> + if (port->bond_flag == 1)
>>>>> + return change_bonding_slave_port_status(port_id, false);
>>>>> +#endif
>>>>> + }
>>>>> return 0;
>>>>> }
>>>>> @@ -609,8 +658,25 @@ eth_dev_start_mp(uint16_t port_id)
>>>>> static int
>>>>> eth_dev_stop_mp(uint16_t port_id)
>>>>> {
>>>>> - if (is_proc_primary())
>>>>> - return rte_eth_dev_stop(port_id);
>>>>> + int ret;
>>>>> +
>>>>> + if (is_proc_primary()) {
>>>>> + ret = rte_eth_dev_stop(port_id);
>>>>> + if (ret != 0)
>>>>> + return ret;
>>>>> +
>>>>> +#ifdef RTE_NET_BOND
>>>>
>>>> Here and in other places - probably no need to pollute the code
>>>> with all these 'ifdef RTE_NET_BOND'.
>>>> I suppose this logic (for checking is bonding API present or not)
>>>> can be hidden inside change_bonding_slave_port_status() itself.
>>>>
>>> I think it does not pollute the code. anyone can tell according to
>>> the flag 'ifdef RTE_NET_BOND'.
>>> if hiddle inside 'change_bonding_slave_port_status', it will be weird.
>>> For example, if the port is not bonding port, It will also invoke
>>> 'change_bonding_slave_port_status'. That is unreasonable.
>>>
>>
>> Hi Konstantin,
>>
>> I also was not happy to have bonding (or any PMD) ifdef in the generic
>> (start()/stop()) functions, but the ifdef blocks updates testpmd
>> (application) level status. So that can't be handled in the PMD and
>> need to be in the application level.
>> Which is enforcing to have same PMD specific code in the testpmd level,
>> if you have any suggestion to prevent this, I am for it.
>
>
> I am not aking to move it to PMD.
> What I am saying that this ifdef logic could be grouped in one place
> (inside change_bonding_slave_port_status()) instead of spreading it
> around multiple places.
>
>>
>> I will proceed with first two patch of this set, which fixes bonding
>> PMD issues, I will hold the testpmd ones for more comments.
>>
>> Thanks,
>> ferruh
>>
>>>
>>>>
>>>>> + struct rte_port *port = &ports[port_id];
>>>>> +
>>>>> + /*
>>>>> + * Stopping a bonded port also stops all slaves under the
>>>>> bonded
>>>>> + * device. So if this port is bond device, we need to
>>>>> modify the
>>>>> + * port status of these slaves.
>>>>> + */
>>>>> + if (port->bond_flag == 1)
>>>>> + return change_bonding_slave_port_status(port_id, true);
>>>>> +#endif
>>>>> + }
>>>>> return 0;
>>>>> }
>>>>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>>>>> index 31f766c965..67f253b30e 100644
>>>>> --- a/app/test-pmd/testpmd.h
>>>>> +++ b/app/test-pmd/testpmd.h
>>>>> @@ -266,7 +266,8 @@ struct rte_port {
>>>>> uint32_t mc_addr_nb; /**< nb. of addr. in
>>>>> mc_addr_pool */
>>>>> queueid_t queue_nb; /**< nb. of queues for flow
>>>>> rules */
>>>>> uint32_t queue_sz; /**< size of a queue for
>>>>> flow rules */
>>>>> - uint8_t slave_flag; /**< bonding slave port */
>>>>> + uint8_t slave_flag : 1, /**< bonding slave
>>>>> port */
>>>>> + bond_flag : 1; /**< port is bond device */
>>>>> struct port_template *pattern_templ_list; /**< Pattern
>>>>> templates. */
>>>>> struct port_template *actions_templ_list; /**< Actions
>>>>> templates. */
>>>>> struct port_table *table_list; /**< Flow tables. */
>>>>
>>>> .
>>
>
> .
On 5/11/2022 3:16 AM, Min Hu (Connor) wrote:
<...>
>>>>>> @@ -609,8 +658,25 @@ eth_dev_start_mp(uint16_t port_id)
>>>>>> static int
>>>>>> eth_dev_stop_mp(uint16_t port_id)
>>>>>> {
>>>>>> - if (is_proc_primary())
>>>>>> - return rte_eth_dev_stop(port_id);
>>>>>> + int ret;
>>>>>> +
>>>>>> + if (is_proc_primary()) {
>>>>>> + ret = rte_eth_dev_stop(port_id);
>>>>>> + if (ret != 0)
>>>>>> + return ret;
>>>>>> +
>>>>>> +#ifdef RTE_NET_BOND
>>>>>
>>>>> Here and in other places - probably no need to pollute the code
>>>>> with all these 'ifdef RTE_NET_BOND'.
>>>>> I suppose this logic (for checking is bonding API present or not)
>>>>> can be hidden inside change_bonding_slave_port_status() itself.
>>>>>
>>>> I think it does not pollute the code. anyone can tell according to
>>>> the flag 'ifdef RTE_NET_BOND'.
>>>> if hiddle inside 'change_bonding_slave_port_status', it will be weird.
>>>> For example, if the port is not bonding port, It will also invoke
>>>> 'change_bonding_slave_port_status'. That is unreasonable.
>>>>
>>>
>>> Hi Konstantin,
>>>
>>> I also was not happy to have bonding (or any PMD) ifdef in the
>>> generic (start()/stop()) functions, but the ifdef blocks updates
>>> testpmd (application) level status. So that can't be handled in the
>>> PMD and need to be in the application level.
>>> Which is enforcing to have same PMD specific code in the testpmd level,
>>> if you have any suggestion to prevent this, I am for it.
>>
>>
>> I am not aking to move it to PMD.
>> What I am saying that this ifdef logic could be grouped in one place
>> (inside change_bonding_slave_port_status()) instead of spreading it
>> around multiple places.
>>
>
> Hi, Konstantin,
> fixed in v4, thanks.
Hi Conor,
What do you think to apply same on patch 4/5?
@@ -6671,6 +6671,7 @@ static void cmd_create_bonded_device_parsed(void *parsed_result,
"Failed to enable promiscuous mode for port %u: %s - ignore\n",
port_id, rte_strerror(-ret));
+ ports[port_id].bond_flag = 1;
ports[port_id].need_setup = 0;
ports[port_id].port_status = RTE_PORT_STOPPED;
}
@@ -66,6 +66,9 @@
#ifdef RTE_EXEC_ENV_WINDOWS
#include <process.h>
#endif
+#ifdef RTE_NET_BOND
+#include <rte_eth_bond.h>
+#endif
#include "testpmd.h"
@@ -597,11 +600,57 @@ eth_dev_configure_mp(uint16_t port_id, uint16_t nb_rx_q, uint16_t nb_tx_q,
return 0;
}
+#ifdef RTE_NET_BOND
+static int
+change_bonding_slave_port_status(portid_t bond_pid, bool is_stop)
+{
+ portid_t slave_pids[RTE_MAX_ETHPORTS];
+ struct rte_port *port;
+ int num_slaves;
+ portid_t slave_pid;
+ int i;
+
+ num_slaves = rte_eth_bond_slaves_get(bond_pid, slave_pids,
+ RTE_MAX_ETHPORTS);
+ if (num_slaves < 0) {
+ fprintf(stderr, "Failed to get slave list for port = %u\n",
+ bond_pid);
+ return num_slaves;
+ }
+
+ for (i = 0; i < num_slaves; i++) {
+ slave_pid = slave_pids[i];
+ port = &ports[slave_pid];
+ port->port_status =
+ is_stop ? RTE_PORT_STOPPED : RTE_PORT_STARTED;
+ }
+
+ return 0;
+}
+#endif
+
static int
eth_dev_start_mp(uint16_t port_id)
{
- if (is_proc_primary())
- return rte_eth_dev_start(port_id);
+ int ret;
+
+ if (is_proc_primary()) {
+ ret = rte_eth_dev_start(port_id);
+ if (ret != 0)
+ return ret;
+
+#ifdef RTE_NET_BOND
+ struct rte_port *port = &ports[port_id];
+
+ /*
+ * Starting a bonded port also starts all slaves under the bonded
+ * device. So if this port is bond device, we need to modify the
+ * port status of these slaves.
+ */
+ if (port->bond_flag == 1)
+ return change_bonding_slave_port_status(port_id, false);
+#endif
+ }
return 0;
}
@@ -609,8 +658,25 @@ eth_dev_start_mp(uint16_t port_id)
static int
eth_dev_stop_mp(uint16_t port_id)
{
- if (is_proc_primary())
- return rte_eth_dev_stop(port_id);
+ int ret;
+
+ if (is_proc_primary()) {
+ ret = rte_eth_dev_stop(port_id);
+ if (ret != 0)
+ return ret;
+
+#ifdef RTE_NET_BOND
+ struct rte_port *port = &ports[port_id];
+
+ /*
+ * Stopping a bonded port also stops all slaves under the bonded
+ * device. So if this port is bond device, we need to modify the
+ * port status of these slaves.
+ */
+ if (port->bond_flag == 1)
+ return change_bonding_slave_port_status(port_id, true);
+#endif
+ }
return 0;
}
@@ -266,7 +266,8 @@ struct rte_port {
uint32_t mc_addr_nb; /**< nb. of addr. in mc_addr_pool */
queueid_t queue_nb; /**< nb. of queues for flow rules */
uint32_t queue_sz; /**< size of a queue for flow rules */
- uint8_t slave_flag; /**< bonding slave port */
+ uint8_t slave_flag : 1, /**< bonding slave port */
+ bond_flag : 1; /**< port is bond device */
struct port_template *pattern_templ_list; /**< Pattern templates. */
struct port_template *actions_templ_list; /**< Actions templates. */
struct port_table *table_list; /**< Flow tables. */