Re: [PATCH v1 3/3] app/testpmd: fix unnecessary change when set tunnel TSO

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

在 2023/11/11 11:37, Ferruh Yigit 写道:
> On 11/10/2023 8:19 AM, Huisong Li wrote:
>> Currently, there are two conditions to set tunnel TSO, like "parse tunnel"
>> and "outer IP checksum". If these conditions are not satisfied, testpmd
>> should not change their configuration, like tx_offloads on port and per
>> queue, and no need to request "reconfig device".
>>
>> Fixes: 597f9fafe13b ("app/testpmd: convert to new Tx offloads API")
>> Cc: stable@dpdk.org
>>
>> Signed-off-by: Huisong Li <lihuisong@huawei.com>
>> ---
>>   app/test-pmd/cmdline.c | 36 ++++++++++++++++++++----------------
>>   1 file changed, 20 insertions(+), 16 deletions(-)
>>
>> diff --git a/app/test-pmd/cmdline.c b/app/test-pmd/cmdline.c
>> index d3243d016b..33e66d1d93 100644
>> --- a/app/test-pmd/cmdline.c
>> +++ b/app/test-pmd/cmdline.c
>> @@ -5093,17 +5093,6 @@ cmd_tunnel_tso_set_parsed(void *parsed_result,
>>   				res->port_id);
>>   			return;
>>   		}
>> -		check_tunnel_tso_nic_support(res->port_id, dev_info.tx_offload_capa);
>> -	}
>> -
>> -	if (ports[res->port_id].tunnel_tso_segsz == 0) {
>> -		ports[res->port_id].dev_conf.txmode.offloads &= ~all_tunnel_tso;
>> -		printf("TSO for tunneled packets is disabled\n");
>> -	} else {
>> -		ports[res->port_id].dev_conf.txmode.offloads |=
>> -			(all_tunnel_tso & dev_info.tx_offload_capa);
>> -		printf("TSO segment size for tunneled packets is %d\n",
>> -			ports[res->port_id].tunnel_tso_segsz);
>>   
>>   		/* Below conditions are needed to make it work:
>>   		 * (1) tunnel TSO is supported by the NIC;
>> @@ -5116,14 +5105,29 @@ cmd_tunnel_tso_set_parsed(void *parsed_result,
>>   		 * is not necessary for IPv6 tunneled pkts because there's no
>>   		 * checksum in IP header anymore.
>>   		 */
>> -
>> -		if (!ports[res->port_id].parse_tunnel)
>> +		if (!ports[res->port_id].parse_tunnel) {
>>   			fprintf(stderr,
>> -				"Warning: csum parse_tunnel must be set so that tunneled packets are recognized\n");
>> +				"Error: csum parse_tunnel must be set so that tunneled packets are recognized\n");
>> +			return;
>> +		}
>>   		if (!(ports[res->port_id].dev_conf.txmode.offloads &
>> -		      RTE_ETH_TX_OFFLOAD_OUTER_IPV4_CKSUM))
>> +		      RTE_ETH_TX_OFFLOAD_OUTER_IPV4_CKSUM)) {
>>   			fprintf(stderr,
>> -				"Warning: csum set outer-ip must be set to hw if outer L3 is IPv4; not necessary for IPv6\n");
>> +				"Error: csum set outer-ip must be set to hw if outer L3 is IPv4; not necessary for IPv6\n");
>> +			return;
>> +		}
>> +
>> +		check_tunnel_tso_nic_support(res->port_id, dev_info.tx_offload_capa);
>> +	}
>> +
>> +	if (ports[res->port_id].tunnel_tso_segsz == 0) {
>> +		ports[res->port_id].dev_conf.txmode.offloads &= ~all_tunnel_tso;
>> +		printf("TSO for tunneled packets is disabled\n");
>> +	} else {
>> +		ports[res->port_id].dev_conf.txmode.offloads |=
>> +			(all_tunnel_tso & dev_info.tx_offload_capa);
>> +		printf("TSO segment size for tunneled packets is %d\n",
>> +			ports[res->port_id].tunnel_tso_segsz);
>>
> Similar comment with previous patch, why having separate if block for
> same check, above block can be merged to it.
Ack
>
> .