Re: [PATCH V2] app/testpmd: update bond port configurations when add slave

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

在 2022/10/28 18:59, humin (Q) 写道:
>
> 在 2022/10/28 14:20, Huisong Li 写道:
>> Some capabilities (like, rx_offload_capa and tx_offload_capa) of bonding
>> device in dev_info is zero when no slave is added. And its capability 
>> will
>> be updated when add a new slave device.
>>
>> The capability to update dynamically may introduce some problems if not
>> handled properly. For example, the reconfig() is called to initialize
>> bonding port configurations when create a bonding device. The global
>> tx_mode is assigned to dev_conf.txmode. The 
>> DEV_TX_OFFLOAD_MBUF_FAST_FREE
>> which is the default value of global tx_mode.offloads in testpmd is 
>> removed
>> from bonding device configuration because of zero rx_offload_capa.
>> As a result, this offload isn't set to bonding device.
>>
>> Generally, port configurations of bonding device must be within the
>> intersection of the capability of all slave devices. If use original 
>> port
>> configurations, the removed capabilities because of adding a new 
>> slave may
>> cause failure when re-initialize bonding device.
>>
>> So port configurations of bonding device also need to be updated 
>> because of
>> the added and removed capabilities. In addition, this also helps to 
>> ensure
>> consistency between testpmd and bonding device.
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   - v2: fix a spelling error in commit log
>>
>> ---
>>   app/test-pmd/testpmd.c                | 40 +++++++++++++++++++++++++++
>>   app/test-pmd/testpmd.h                |  3 +-
>>   drivers/net/bonding/bonding_testpmd.c |  2 ++
>>   3 files changed, 44 insertions(+), 1 deletion(-)
>>
>> diff --git a/app/test-pmd/testpmd.c b/app/test-pmd/testpmd.c
>> index 97adafacd0..7324b8865c 100644
>> --- a/app/test-pmd/testpmd.c
>> +++ b/app/test-pmd/testpmd.c
>> @@ -2805,6 +2805,41 @@ fill_xstats_display_info(void)
>>           fill_xstats_display_info_for_port(pi);
>>   }
>>   +/*
>> + * Some capabilities (like, rx_offload_capa and tx_offload_capa) of 
>> bonding
>> + * device in dev_info is zero when no slave is added. And its 
>> capability of
>
> “And its capability of will be ”, what does this mean ?
will fix in next version.
>
>> + * will be updated when add a new slave device. So adding a device 
>> slave need
>> + * to update the port configurations of bonding device.
>> + */
>> +static void
>> +update_bonding_port_dev_conf(portid_t bond_pid)
>> +{
>> +#ifdef RTE_NET_BOND
>> +    struct rte_port *port = &ports[bond_pid];
>> +    uint16_t i;
>> +    int ret;
>> +
>> +    ret = eth_dev_info_get_print_err(bond_pid, &port->dev_info);
>> +    if (ret != 0) {
>> +        fprintf(stderr, "Failed to get dev info for port = %u\n",
>> +            bond_pid);
>> +        return;
>> +    }
>> +
>> +    if (port->dev_info.tx_offload_capa & 
>> RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE)
>> +        port->dev_conf.txmode.offloads |=
>> +                RTE_ETH_TX_OFFLOAD_MBUF_FAST_FREE;
>> +    /* Apply Tx offloads configuration */
>> +    for (i = 0; i < port->dev_info.max_tx_queues; i++)
>> +        port->txq[i].conf.offloads = port->dev_conf.txmode.offloads;
>> +
>> +    port->dev_conf.rx_adv_conf.rss_conf.rss_hf &=
>> +                port->dev_info.flow_type_rss_offloads;
>> +#else
>> +    RTE_SET_USED(bond_pid);
>> +#endif
>> +}
>> +
>>   int
>>   start_port(portid_t pid)
>>   {
>> @@ -2869,6 +2904,11 @@ start_port(portid_t pid)
>>                   return -1;
>>               }
>>   +            if (port->bond_flag == 1 && port->update_conf == 1) {
>> +                update_bonding_port_dev_conf(pi);
>> +                port->update_conf = 0;
>> +            }
>> +
>>               /* configure port */
>>               diag = eth_dev_configure_mp(pi, nb_rxq + nb_hairpinq,
>>                                nb_txq + nb_hairpinq,
>> diff --git a/app/test-pmd/testpmd.h b/app/test-pmd/testpmd.h
>> index 7fef96f9b1..82714119e8 100644
>> --- a/app/test-pmd/testpmd.h
>> +++ b/app/test-pmd/testpmd.h
>> @@ -316,7 +316,8 @@ struct rte_port {
>>       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 : 1, /**< bonding slave port */
>> -                bond_flag : 1; /**< port is bond device */
>> +                bond_flag : 1, /**< port is bond device */
>> +                update_conf : 1; /**< need to update bonding device 
>> configuration */
>>       struct port_template    *pattern_templ_list; /**< Pattern 
>> templates. */
>>       struct port_template    *actions_templ_list; /**< Actions 
>> templates. */
>>       struct port_table       *table_list; /**< Flow tables. */
>> diff --git a/drivers/net/bonding/bonding_testpmd.c 
>> b/drivers/net/bonding/bonding_testpmd.c
>> index 3941f4cf23..9529e16fb6 100644
>> --- a/drivers/net/bonding/bonding_testpmd.c
>> +++ b/drivers/net/bonding/bonding_testpmd.c
>> @@ -625,6 +625,7 @@ static void cmd_add_bonding_slave_parsed(void 
>> *parsed_result,
>>               slave_port_id, master_port_id);
>>           return;
>>       }
>> +    ports[master_port_id].update_conf = 1;
>>       init_port_config();
>>       set_port_slave_flag(slave_port_id);
>>   }
>> @@ -762,6 +763,7 @@ static void cmd_create_bonded_device_parsed(void 
>> *parsed_result,
>>           fprintf(stderr, "Failed to enable promiscuous mode for port 
>> %u: %s - ignore\n",
>>               port_id, rte_strerror(-ret));
>>   +    ports[port_id].update_conf = 1;
>>       ports[port_id].bond_flag = 1;
>>       ports[port_id].need_setup = 0;
>>       ports[port_id].port_status = RTE_PORT_STOPPED;
> .