diff mbox series

[ovs-dev] system-tso: Skip encap tests when userspace TSO is enabled.

Message ID 20220105145738.758971-1-fbl@redhat.com
State Accepted
Commit 7ed60839d043777aba80c57a171d10e96077a989
Headers show
Series [ovs-dev] system-tso: Skip encap tests when userspace TSO is enabled. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed

Commit Message

Flavio Leitner Jan. 5, 2022, 2:57 p.m. UTC 
It seems Linux native tunnel configuration changed to enable
checksum by default and that causes the check-system-tso unit
test below to fail:
 10: datapath - ping over vxlan tunnel    FAILED (system-traffic.at:248)

That happens because userspace TSO doesn't support encapsulation
as mentioned in the current documentation. In this specific case,
udp_extract_tnl_md() checks if the checksum is correct, but since
TSO is enabled, the outer UDP header contains only the pseudo
checksum and not the full packet checksum.

Although the packet is marked correctly with UDP csum offload flag
and the code could use that to verify the pseudo csum, more work
is needed to properly translate the offloading flags from the outer
headers to the inner headers.  For example, if the payload is a
TCP packet, most probably the flag DP_PACKET_OL_TX_UDP_CKSUM doesn't
make sense after decapsulating that.

This patch skips the tunnel tests when the userspace TSO is enabled.
Fixes: 29bb3093eb8b ("userspace: Enable TSO support for non-DPDK.")
Signed-off-by: Flavio Leitner <fbl@redhat.com>
---
 tests/system-common-macros.at |  8 ++++++++
 tests/system-traffic.at       | 17 +++++++++++++++++
 tests/system-tso-macros.at    |  2 ++
 3 files changed, 27 insertions(+)

Comments

David Marchand Jan. 18, 2022, 2:41 p.m. UTC | #1 
On Wed, Jan 5, 2022 at 3:58 PM Flavio Leitner <fbl@redhat.com> wrote:
>
> It seems Linux native tunnel configuration changed to enable
> checksum by default and that causes the check-system-tso unit
> test below to fail:
>  10: datapath - ping over vxlan tunnel    FAILED (system-traffic.at:248)
>
> That happens because userspace TSO doesn't support encapsulation
> as mentioned in the current documentation. In this specific case,
> udp_extract_tnl_md() checks if the checksum is correct, but since
> TSO is enabled, the outer UDP header contains only the pseudo
> checksum and not the full packet checksum.
>
> Although the packet is marked correctly with UDP csum offload flag
> and the code could use that to verify the pseudo csum, more work
> is needed to properly translate the offloading flags from the outer
> headers to the inner headers.  For example, if the payload is a
> TCP packet, most probably the flag DP_PACKET_OL_TX_UDP_CKSUM doesn't
> make sense after decapsulating that.
>
> This patch skips the tunnel tests when the userspace TSO is enabled.

> Fixes: 29bb3093eb8b ("userspace: Enable TSO support for non-DPDK.")
> Signed-off-by: Flavio Leitner <fbl@redhat.com>

Reviewed-by: David Marchand <david.marchand@redhat.com>
Ilya Maximets March 4, 2022, 11:08 p.m. UTC | #2 
On 1/18/22 15:41, David Marchand wrote:
> On Wed, Jan 5, 2022 at 3:58 PM Flavio Leitner <fbl@redhat.com> wrote:
>>
>> It seems Linux native tunnel configuration changed to enable
>> checksum by default and that causes the check-system-tso unit
>> test below to fail:
>>  10: datapath - ping over vxlan tunnel    FAILED (system-traffic.at:248)
>>
>> That happens because userspace TSO doesn't support encapsulation
>> as mentioned in the current documentation. In this specific case,
>> udp_extract_tnl_md() checks if the checksum is correct, but since
>> TSO is enabled, the outer UDP header contains only the pseudo
>> checksum and not the full packet checksum.
>>
>> Although the packet is marked correctly with UDP csum offload flag
>> and the code could use that to verify the pseudo csum, more work
>> is needed to properly translate the offloading flags from the outer
>> headers to the inner headers.  For example, if the payload is a
>> TCP packet, most probably the flag DP_PACKET_OL_TX_UDP_CKSUM doesn't
>> make sense after decapsulating that.
>>
>> This patch skips the tunnel tests when the userspace TSO is enabled.
> 
>> Fixes: 29bb3093eb8b ("userspace: Enable TSO support for non-DPDK.")
>> Signed-off-by: Flavio Leitner <fbl@redhat.com>
> 
> Reviewed-by: David Marchand <david.marchand@redhat.com>

Thanks!  Applied and backported down to 2.14.

Best regards, Ilya Maximets.
diff mbox series

Patch

diff --git a/tests/system-common-macros.at b/tests/system-common-macros.at
index 19a0b125b..8b9f5c752 100644
--- a/tests/system-common-macros.at
+++ b/tests/system-common-macros.at
@@ -281,6 +281,14 @@  m4_define([OVS_START_L7],
 #
 m4_define([OFPROTO_CLEAR_DURATION_IDLE], [[sed -e 's/duration=.*s,/duration=<cleared>,/g' -e 's/idle_age=[0-9]*,/idle_age=<cleared>,/g']])
 
+# OVS_CHECK_TUNNEL_TSO()
+#
+# Macro to be used in general tunneling tests that could be also
+# used by system-tso. In that case, tunneling is not supported and
+# the test should be skipped.
+m4_define([OVS_CHECK_TUNNEL_TSO],
+    [m4_ifdef([CHECK_SYSTEM_TSO], [AT_SKIP_IF(:)])])
+
 # OVS_CHECK_VXLAN()
 #
 # Do basic check for vxlan functionality, skip the test if it's not there.
diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index d79753a99..6b01f8655 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -218,6 +218,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over vxlan tunnel])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_VXLAN()
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -259,6 +260,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over vxlan6 tunnel])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_VXLAN_UDP6ZEROCSUM()
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -302,6 +304,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over gre tunnel])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_KERNEL_EXCL(3, 10, 4, 15)
 OVS_CHECK_GRE()
 
@@ -343,6 +346,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over ip6gre L2 tunnel])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_KERNEL_EXCL(3, 10, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
@@ -383,6 +387,7 @@  AT_CLEANUP
 
 
 AT_SETUP([datapath - ping over erspan v1 tunnel])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_KERNEL_EXCL(3, 10, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
@@ -419,6 +424,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over erspan v2 tunnel])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_KERNEL_EXCL(3, 10, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
@@ -455,6 +461,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over ip6erspan v1 tunnel])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_KERNEL_EXCL(3, 10, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
@@ -494,6 +501,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over ip6erspan v2 tunnel])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_KERNEL_EXCL(3, 10, 4, 15)
 OVS_CHECK_GRE()
 OVS_CHECK_ERSPAN()
@@ -534,6 +542,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over geneve tunnel])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_GENEVE()
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -575,6 +584,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over geneve tunnel, delete flow regression])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_GENEVE()
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -629,6 +639,7 @@  OVS_TRAFFIC_VSWITCHD_STOP(["/|ERR|/d
 AT_CLEANUP
 
 AT_SETUP([datapath - flow resume with geneve tun_metadata])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_GENEVE()
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -680,6 +691,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over geneve6 tunnel])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_GENEVE_UDP6ZEROCSUM()
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -723,6 +735,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over gre tunnel by simulated packets])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_MIN_KERNEL(3, 10)
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -769,6 +782,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over erspan v1 tunnel by simulated packets])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_MIN_KERNEL(3, 10)
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -817,6 +831,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over erspan v2 tunnel by simulated packets])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_MIN_KERNEL(3, 10)
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -870,6 +885,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over ip6erspan v1 tunnel by simulated packets])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_MIN_KERNEL(3, 10)
 
 OVS_TRAFFIC_VSWITCHD_START()
@@ -925,6 +941,7 @@  OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
 
 AT_SETUP([datapath - ping over ip6erspan v2 tunnel by simulated packets])
+OVS_CHECK_TUNNEL_TSO()
 OVS_CHECK_MIN_KERNEL(3, 10)
 
 OVS_TRAFFIC_VSWITCHD_START()
diff --git a/tests/system-tso-macros.at b/tests/system-tso-macros.at
index 406334f3e..1a8004761 100644
--- a/tests/system-tso-macros.at
+++ b/tests/system-tso-macros.at
@@ -29,3 +29,5 @@  m4_define([CONFIGURE_VETH_OFFLOADS],
     [AT_CHECK([ethtool -K $1 sg on], [0], [ignore], [ignore])]
     [AT_CHECK([ethtool -K $1 tso on], [0], [ignore], [ignore])]
 )
+
+m4_define([CHECK_SYSTEM_TSO], [])