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

["humin (Q)" <humin29@huawei.com>]

在 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 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;