Re: [dpdk-dev] [PATCH 1/3] app/testpmd: fix port status of active slave device

["lihuisong (C)" <lihuisong@huawei.com>]

在 2021/11/15 21:01, Singh, Aman Deep 写道:
> On 10/25/2021 12:09 PM, Min Hu (Connor) wrote:
>> From: Huisong Li <lihuisong@huawei.com>
>>
>> Stopping a bond device also stops all active slaves under the bond 
>> device.
>> If this port is bond device, we need to modify the port status of all
>> slaves from RTE_PORT_STARTED to RTE_PORT_STOPPED.
>>
>> 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>
>> ---
>
> Hi Huisong,
>
> Just wanted to check, if this behaviour can be taken care-off in 
> bonding PMD itself.
> Or each application will need to in-corporate this change.
>
> Thanks

If an application maintains each port status, the application is 
responsible for it.

The bonding PMD driver does not know what needs to be changed in the 
application.

That's what I think.

>
>>   app/test-pmd/cmdline.c |  1 +
>>   app/test-pmd/testpmd.c | 49 +++++++++++++++++++++++++++++++++++++++---
>>   app/test-pmd/testpmd.h |  3 ++-
>>   3 files changed, 49 insertions(+), 4 deletions(-)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index 722f4fb9d9..5bfb4b509b 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -6639,6 +6639,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 af0e79fe6d..d6b9ebc4dd 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -65,6 +65,9 @@
>>   #ifdef RTE_EXEC_ENV_WINDOWS
>>   #include <process.h>
>>   #endif
>> +#ifdef RTE_NET_BOND
>> +#include <rte_eth_bond.h>
>> +#endif
>>     #include "testpmd.h"
>>   @@ -2986,6 +2989,35 @@ start_port(portid_t pid)
>>       return 0;
>>   }
>>   +#ifdef RTE_NET_BOND
>> +static void
>> +change_bonding_active_slave_port_status(portid_t bond_pid)
>> +{
>> +    portid_t slave_pids[RTE_MAX_ETHPORTS];
>> +    struct rte_port *port;
>> +    int num_active_slaves;
>> +    portid_t slave_pid;
>> +    int i;
>> +
>> +    num_active_slaves = rte_eth_bond_active_slaves_get(bond_pid, 
>> slave_pids,
>> +                               RTE_MAX_ETHPORTS);
>> +    if (num_active_slaves < 0) {
>> +        fprintf(stderr, "Failed to get slave list for port = %u\n",
>> +            bond_pid);
>> +        return;
>> +    }
>> +
>> +    for (i = 0; i < num_active_slaves; i++) {
>> +        slave_pid = slave_pids[i];
>> +        port = &ports[slave_pid];
>> +        if (rte_atomic16_cmpset(&(port->port_status),
>> +            RTE_PORT_STARTED, RTE_PORT_STOPPED) == 0)
>> +            fprintf(stderr, "Port %u can not be set into stopped\n",
>> +                slave_pid);
>> +    }
>> +}
>> +#endif
>> +
>>   void
>>   stop_port(portid_t pid)
>>   {
>> @@ -3042,9 +3074,20 @@ stop_port(portid_t pid)
>>           if (port->flow_list)
>>               port_flow_flush(pi);
>>   -        if (eth_dev_stop_mp(pi) != 0)
>> -            RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port %u\n",
>> -                pi);
>> +        if (is_proc_primary()) {
>> +#ifdef RTE_NET_BOND
>> +            /*
>> +             * Stopping a bond device also stops all active slaves
>> +             * under the bond device. If this port is bond device,
>> +             * we need to modify the port status of all slaves.
>> +             */
>> +            if (port->bond_flag == 1)
>> +                change_bonding_active_slave_port_status(pi);
>> +#endif
>> +            if (rte_eth_dev_stop(pi) != 0)
>> +                RTE_LOG(ERR, EAL, "rte_eth_dev_stop failed for port 
>> %u\n",
>> +                    pi);
>> +        }
>>             if (rte_atomic16_cmpset(&(port->port_status),
>>               RTE_PORT_HANDLING, RTE_PORT_STOPPED) == 0)
>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>> index e3995d24ab..ad3b4f875c 100644
>> --- a/app/test-pmd/testpmd.h
>> +++ b/app/test-pmd/testpmd.h
>> @@ -237,7 +237,8 @@ struct rte_port {
>>       struct rte_eth_txconf tx_conf[RTE_MAX_QUEUES_PER_PORT+1]; /**< 
>> per queue tx configuration */
>>       struct rte_ether_addr   *mc_addr_pool; /**< pool of multicast 
>> addrs */
>>       uint32_t                mc_addr_nb; /**< nb. of addr. in 
>> mc_addr_pool */
>> -    uint8_t                 slave_flag; /**< bonding slave port */
>> +    uint8_t                 slave_flag : 1, /**< bonding slave port */
>> +                bond_flag : 1; /**< port is bond device */
>>       struct port_flow        *flow_list; /**< Associated flows. */
>>       struct port_indirect_action *actions_list;
>>       /**< Associated indirect actions. */
> .