[v1,3/3] app/testpmd: fix unnecessary change when set tunnel TSO
Checks
Commit Message
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(-)
Comments
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.
在 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
>
> .
@@ -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);
}
cmd_config_queue_tx_offloads(&ports[res->port_id]);