[v2] app/testpmd: fix error detection in MTU command
Checks
Commit Message
MTU is used in testpmd to set the maximum payload size for packets.
According to testpmd, the setting influnce RX only.
In rte_ethdev there's no relation between MTU setting and JUMBO offload or
rx_max_pkt_len.
The previous fix in patch referenced below was meant to update the
correlated variables of max_pkt_len and JUMBO offload, but by doing
so it assumes that MTU setting can only exist when JUMBO offload
supported in the device. For example fail-safe device does supports set MTU
and doesn't support JUMBO offload, and in this case, though set MTU
succeeds, an error message is still printed since the JUMBO packet
offload is disabled.
The fix separates the two conditions to make sure the error
triggers only in case the set_mtu action actually failed.
Fixes: 150c9ac2df13 ("app/testpmd: update Rx offload after setting MTU")
Cc: xavier.huwei@huawei.com
Signed-off-by: Shy Shyman <shys@mellanox.com>
---
v2 - removed warning message.
---
app/test-pmd/config.c | 8 +++-----
1 file changed, 3 insertions(+), 5 deletions(-)
Comments
On 6/8/2020 3:17 PM, Shy Shyman wrote:
> MTU is used in testpmd to set the maximum payload size for packets.
> According to testpmd, the setting influnce RX only.
> In rte_ethdev there's no relation between MTU setting and JUMBO offload or
> rx_max_pkt_len.
>
> The previous fix in patch referenced below was meant to update the
> correlated variables of max_pkt_len and JUMBO offload, but by doing
> so it assumes that MTU setting can only exist when JUMBO offload
> supported in the device. For example fail-safe device does supports set MTU
> and doesn't support JUMBO offload, and in this case, though set MTU
> succeeds, an error message is still printed since the JUMBO packet
> offload is disabled.
>
> The fix separates the two conditions to make sure the error
> triggers only in case the set_mtu action actually failed.
>
> Fixes: 150c9ac2df13 ("app/testpmd: update Rx offload after setting MTU")
> Cc: xavier.huwei@huawei.com
>
> Signed-off-by: Shy Shyman <shys@mellanox.com>
Reviewed-by: Ferruh Yigit <ferruh.yigit@intel.com>
> -----Original Message-----
> From: dev <dev-bounces@dpdk.org> On Behalf Of Shy Shyman
> Sent: Monday, June 8, 2020 10:18 PM
> To: dev@dpdk.org
> Cc: wenzhuo.lu@intel.com; beilei.xing@intel.com;
> bernard.iremonger@intel.com; xavier.huwei@huawei.com
> Subject: [dpdk-dev] [PATCH v2] app/testpmd: fix error detection in MTU
> command
>
> MTU is used in testpmd to set the maximum payload size for packets.
> According to testpmd, the setting influnce RX only.
> In rte_ethdev there's no relation between MTU setting and JUMBO offload
> or
> rx_max_pkt_len.
>
> The previous fix in patch referenced below was meant to update the
> correlated variables of max_pkt_len and JUMBO offload, but by doing
> so it assumes that MTU setting can only exist when JUMBO offload
> supported in the device. For example fail-safe device does supports set MTU
> and doesn't support JUMBO offload, and in this case, though set MTU
> succeeds, an error message is still printed since the JUMBO packet
> offload is disabled.
>
> The fix separates the two conditions to make sure the error
> triggers only in case the set_mtu action actually failed.
>
> Fixes: 150c9ac2df13 ("app/testpmd: update Rx offload after setting MTU")
> Cc: xavier.huwei@huawei.com
>
> Signed-off-by: Shy Shyman <shys@mellanox.com>
Reviewed-by: Phil Yang <phil.yang@arm.com>
On 6/9/2020 3:44 PM, Phil Yang wrote:
>> -----Original Message-----
>> From: dev <dev-bounces@dpdk.org> On Behalf Of Shy Shyman
>> Sent: Monday, June 8, 2020 10:18 PM
>> To: dev@dpdk.org
>> Cc: wenzhuo.lu@intel.com; beilei.xing@intel.com;
>> bernard.iremonger@intel.com; xavier.huwei@huawei.com
>> Subject: [dpdk-dev] [PATCH v2] app/testpmd: fix error detection in MTU
>> command
>>
>> MTU is used in testpmd to set the maximum payload size for packets.
>> According to testpmd, the setting influnce RX only.
>> In rte_ethdev there's no relation between MTU setting and JUMBO offload
>> or
>> rx_max_pkt_len.
>>
>> The previous fix in patch referenced below was meant to update the
>> correlated variables of max_pkt_len and JUMBO offload, but by doing
>> so it assumes that MTU setting can only exist when JUMBO offload
>> supported in the device. For example fail-safe device does supports set MTU
>> and doesn't support JUMBO offload, and in this case, though set MTU
>> succeeds, an error message is still printed since the JUMBO packet
>> offload is disabled.
>>
>> The fix separates the two conditions to make sure the error
>> triggers only in case the set_mtu action actually failed.
>>
>> Fixes: 150c9ac2df13 ("app/testpmd: update Rx offload after setting MTU")
>> Cc: xavier.huwei@huawei.com
>>
>> Signed-off-by: Shy Shyman <shys@mellanox.com>
>
> Reviewed-by: Phil Yang <phil.yang@arm.com>
>
Applied to dpdk-next-net/master, thanks.
@@ -1277,8 +1277,9 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
return;
}
diag = rte_eth_dev_set_mtu(port_id, mtu);
- if (diag == 0 &&
- dev_info.rx_offload_capa & DEV_RX_OFFLOAD_JUMBO_FRAME) {
+ if (diag)
+ printf("Set MTU failed. diag=%d\n", diag);
+ else if (dev_info.rx_offload_capa & DEV_RX_OFFLOAD_JUMBO_FRAME) {
/*
* Ether overhead in driver is equal to the difference of
* max_rx_pktlen and max_mtu in rte_eth_dev_info when the
@@ -1293,10 +1294,7 @@ port_mtu_set(portid_t port_id, uint16_t mtu)
} else
rte_port->dev_conf.rxmode.offloads &=
~DEV_RX_OFFLOAD_JUMBO_FRAME;
-
- return;
}
- printf("Set MTU failed. diag=%d\n", diag);
}
/* Generic flow management functions. */