[PATCH] app/testpmd: fix out-of-bound reference in offload config

[lihuisong (C)]

LGTM,
Acked-by: Huisong Li <lihuisong at huawei.com>
在 2025/2/21 4:44, Stephen Hemminger 写道:
> When configuring offloads, need to check the port id before
> indexing into the ports[] array. This can easily be done
> by moving the call to oh_dev_conf_get_print_err() to before
> the checks for port stopped.
>
> Fixes: c73a9071877a ("app/testpmd: add commands to test new offload API")
> Cc: stable at dpdk.org
> Signed-off-by: Stephen Hemminger <stephen at networkplumber.org>
> ---
>   app/test-pmd/cmdline.c | 57 +++++++++++++++++++++++-------------------
>   1 file changed, 31 insertions(+), 26 deletions(-)
>
> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
> index 86d763b66a..d059b65003 100644
> --- a/app/test-pmd/cmdline.c
> +++ b/app/test-pmd/cmdline.c
> @@ -11627,7 +11627,7 @@ cmd_rx_offload_get_configuration_parsed(
>   	struct cmd_rx_offload_get_configuration_result *res = parsed_result;
>   	struct rte_eth_dev_info dev_info;
>   	portid_t port_id = res->port_id;
> -	struct rte_port *port = &ports[port_id];
> +	struct rte_port *port;
>   	struct rte_eth_conf dev_conf;
>   	uint64_t port_offloads;
>   	uint64_t queue_offloads;
> @@ -11635,12 +11635,13 @@ cmd_rx_offload_get_configuration_parsed(
>   	int q;
>   	int ret;
>   
> -	printf("Rx Offloading Configuration of port %d :\n", port_id);
> -
>   	ret = eth_dev_conf_get_print_err(port_id, &dev_conf);
>   	if (ret != 0)
>   		return;
>   
> +	port = &ports[port_id];
> +	printf("Rx Offloading Configuration of port %d :\n", port_id);
> +
>   	port_offloads = dev_conf.rxmode.offloads;
>   	printf("  Port :");
>   	print_rx_offloads(port_offloads);
> @@ -11741,12 +11742,17 @@ static void
>   config_port_rx_offload(portid_t port_id, char *name, bool on)
>   {
>   	struct rte_eth_dev_info dev_info;
> -	struct rte_port *port = &ports[port_id];
> +	struct rte_port *port;
>   	uint16_t nb_rx_queues;
>   	uint64_t offload;
>   	int q;
>   	int ret;
>   
> +	ret = eth_dev_info_get_print_err(port_id, &dev_info);
> +	if (ret != 0)
> +		return;
> +
> +	port = &ports[port_id];
>   	if (port->port_status != RTE_PORT_STOPPED) {
>   		fprintf(stderr,
>   			"Error: Can't config offload when Port %d is not stopped\n",
> @@ -11754,10 +11760,6 @@ config_port_rx_offload(portid_t port_id, char *name, bool on)
>   		return;
>   	}
>   
> -	ret = eth_dev_info_get_print_err(port_id, &dev_info);
> -	if (ret != 0)
> -		return;
> -
>   	if (!strcmp(name, "all")) {
>   		offload = dev_info.rx_offload_capa;
>   	} else {
> @@ -11943,10 +11945,15 @@ cmd_config_per_queue_rx_offload_parsed(void *parsed_result,
>   	struct rte_eth_dev_info dev_info;
>   	portid_t port_id = res->port_id;
>   	uint16_t queue_id = res->queue_id;
> -	struct rte_port *port = &ports[port_id];
> +	struct rte_port *port;
>   	uint64_t offload;
>   	int ret;
>   
> +	ret = eth_dev_info_get_print_err(port_id, &dev_info);
> +	if (ret != 0)
> +		return;
> +
> +	port = &ports[port_id];
>   	if (port->port_status != RTE_PORT_STOPPED) {
>   		fprintf(stderr,
>   			"Error: Can't config offload when Port %d is not stopped\n",
> @@ -11954,10 +11961,6 @@ cmd_config_per_queue_rx_offload_parsed(void *parsed_result,
>   		return;
>   	}
>   
> -	ret = eth_dev_info_get_print_err(port_id, &dev_info);
> -	if (ret != 0)
> -		return;
> -
>   	if (queue_id >= dev_info.nb_rx_queues) {
>   		fprintf(stderr,
>   			"Error: input queue_id should be 0 ... %d\n",
> @@ -12145,7 +12148,7 @@ cmd_tx_offload_get_configuration_parsed(
>   	struct cmd_tx_offload_get_configuration_result *res = parsed_result;
>   	struct rte_eth_dev_info dev_info;
>   	portid_t port_id = res->port_id;
> -	struct rte_port *port = &ports[port_id];
> +	struct rte_port *port;
>   	struct rte_eth_conf dev_conf;
>   	uint64_t port_offloads;
>   	uint64_t queue_offloads;
> @@ -12153,12 +12156,12 @@ cmd_tx_offload_get_configuration_parsed(
>   	int q;
>   	int ret;
>   
> -	printf("Tx Offloading Configuration of port %d :\n", port_id);
> -
>   	ret = eth_dev_conf_get_print_err(port_id, &dev_conf);
>   	if (ret != 0)
>   		return;
>   
> +	printf("Tx Offloading Configuration of port %d :\n", port_id);
> +	port = &ports[port_id];
>   	port_offloads = dev_conf.txmode.offloads;
>   	printf("  Port :");
>   	print_tx_offloads(port_offloads);
> @@ -12263,12 +12266,17 @@ static void
>   config_port_tx_offload(portid_t port_id, char *name, bool on)
>   {
>   	struct rte_eth_dev_info dev_info;
> -	struct rte_port *port = &ports[port_id];
> +	struct rte_port *port;
>   	uint16_t nb_tx_queues;
>   	uint64_t offload;
>   	int q;
>   	int ret;
>   
> +	ret = eth_dev_info_get_print_err(port_id, &dev_info);
> +	if (ret != 0)
> +		return;
> +
> +	port = &ports[port_id];
>   	if (port->port_status != RTE_PORT_STOPPED) {
>   		fprintf(stderr,
>   			"Error: Can't config offload when Port %d is not stopped\n",
> @@ -12276,10 +12284,6 @@ config_port_tx_offload(portid_t port_id, char *name, bool on)
>   		return;
>   	}
>   
> -	ret = eth_dev_info_get_print_err(port_id, &dev_info);
> -	if (ret != 0)
> -		return;
> -
>   	if (!strcmp(name, "all")) {
>   		offload = dev_info.tx_offload_capa;
>   	} else {
> @@ -12469,10 +12473,15 @@ cmd_config_per_queue_tx_offload_parsed(void *parsed_result,
>   	struct rte_eth_dev_info dev_info;
>   	portid_t port_id = res->port_id;
>   	uint16_t queue_id = res->queue_id;
> -	struct rte_port *port = &ports[port_id];
> +	struct rte_port *port;
>   	uint64_t offload;
>   	int ret;
>   
> +	ret = eth_dev_info_get_print_err(port_id, &dev_info);
> +	if (ret != 0)
> +		return;
> +
> +	port = &ports[port_id];
>   	if (port->port_status != RTE_PORT_STOPPED) {
>   		fprintf(stderr,
>   			"Error: Can't config offload when Port %d is not stopped\n",
> @@ -12480,10 +12489,6 @@ cmd_config_per_queue_tx_offload_parsed(void *parsed_result,
>   		return;
>   	}
>   
> -	ret = eth_dev_info_get_print_err(port_id, &dev_info);
> -	if (ret != 0)
> -		return;
> -
>   	if (queue_id >= dev_info.nb_tx_queues) {
>   		fprintf(stderr,
>   			"Error: input queue_id should be 0 ... %d\n",