diff mbox series

[ovs-dev,v7,01/11] ofproto-dpif: Fix incorrect checksums in input packets

Message ID 20220614115743.1143341-2-emma.finn@intel.com
State Changes Requested
Headers show
Series [ovs-dev,v7,01/11] ofproto-dpif: Fix incorrect checksums in input packets | expand

Checks

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

Commit Message

Emma Finn June 14, 2022, 11:57 a.m. UTC 
The IP checksum field was invalid in the input packets
for some unit tests. The unit tests will still pass without
a valid checksum, however we should still fix these.

Signed-off-by: Emma Finn <emma.finn@intel.com>
Acked-by: Eelco Chaudron <echaudro@redhat.com>

---
This was found using the autovalidator introduced later in
this series.
---
---
 tests/ofproto-dpif.at | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

Comments

Eelco Chaudron June 23, 2022, 3:37 p.m. UTC | #1 
On 14 Jun 2022, at 13:57, Emma Finn wrote:

> The IP checksum field was invalid in the input packets
> for some unit tests. The unit tests will still pass without
> a valid checksum, however we should still fix these.
>
> Signed-off-by: Emma Finn <emma.finn@intel.com>
> Acked-by: Eelco Chaudron <echaudro@redhat.com>
>
> ---
> This was found using the autovalidator introduced later in
> this series.
> ---

Although I've ACKed this patch in the previous series, I now concluded that these checksum failures are ok to keep in. So I think this patch can be removed from the series.

This has to do with the general way OVS as a layer two switch is supposed to deal with checksums (and failures). If the checksum was already bad to begin with, it should not try to correct the checksum, as now an invalid packet has become valid. It should only change the checksum according to the data changed.

Also, see comments on patch 11.
diff mbox series

Patch

diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
index dbb3b6dda..935ae80e0 100644
--- a/tests/ofproto-dpif.at
+++ b/tests/ofproto-dpif.at
@@ -2009,7 +2009,7 @@  dnl Checksum UDP.
 AT_CHECK([ovs-ofctl monitor br0 65534 -P nxt_packet_in --detach --no-chdir --pidfile 2> ofctl_monitor.log])
 
 for i in 1 ; do
-    ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 20 22 22 22 22 22 08 00 45 00 00 1C 00 00 00 00 00 11 00 00 C0 A8 00 01 C0 A8 00 02 00 08 00 0B 00 00 12 34 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00'
+    ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 20 22 22 22 22 22 08 00 45 00 00 1C 00 00 00 00 00 11 39 7E C0 A8 00 01 C0 A8 00 02 00 08 00 0B 00 00 12 34 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00'
 done
 OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 18])
 OVS_APP_EXIT_AND_WAIT([ovs-ofctl])
@@ -2079,7 +2079,7 @@  dnl Checksum SCTP.
 AT_CHECK([ovs-ofctl monitor br0 65534 -P nxt_packet_in --detach --no-chdir --pidfile 2> ofctl_monitor.log])
 
 for i in 1 ; do
-    ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 20 22 22 22 22 22 08 00 45 00 00 24 00 00 00 00 00 84 00 00 C0 A8 00 01 C0 A8 00 02 04 58 08 af 00 00 00 00 d9 d7 91 57 01 00 00 34 cf 28 ec 4e 00 01 40 00 00 0a ff ff b7 53 24 19 00 05 00 08 7f 00 00 01 00 05 00 08 c0 a8 02 07 00 0c 00 06 00 05 00 00 80 00 00 04 c0 00 00 04'
+    ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 20 22 22 22 22 22 08 00 45 00 00 24 00 00 00 00 00 84 39 03 C0 A8 00 01 C0 A8 00 02 04 58 08 af 00 00 00 00 d9 d7 91 57 01 00 00 34 cf 28 ec 4e 00 01 40 00 00 0a ff ff b7 53 24 19 00 05 00 08 7f 00 00 01 00 05 00 08 c0 a8 02 07 00 0c 00 06 00 05 00 00 80 00 00 04 c0 00 00 04'
 done
 
 AT_CHECK([ovs-appctl time/warp 1000], [0], [ignore])
@@ -2951,7 +2951,7 @@  dnl        192.168.0.1.80 > 192.168.0.2.0: Flags [none], cksum 0x7744 (correct),
 AT_CHECK([ovs-ofctl monitor br0 65534 -P nxt_packet_in --detach --no-chdir --pidfile 2> ofctl_monitor.log])
 
 for i in 1 2 3; do
-    ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 60 66 66 66 02 01 88 48 00 01 40 20 00 01 41 1f 45 00 00 2c 00 00 00 00 ff 06 3b 78 c0 a8 00 01 c0 a8 00 02 00 50 00 00 00 00 00 2a 00 00 00 2a 50 00 27 10 77 44 00 00 48 4f 47 45'
+    ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 60 66 66 66 02 01 88 48 00 01 40 20 00 01 41 1f 45 00 00 2c 00 00 00 00 ff 06 3a 78 c0 a8 00 01 c0 a8 00 02 00 50 00 00 00 00 00 2a 00 00 00 2a 50 00 27 10 77 44 00 00 48 4f 47 45'
 done
 
 OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
@@ -2979,7 +2979,7 @@  dnl        192.168.0.1.80 > 192.168.0.2.0: Flags [none], cksum 0x7744 (correct),
 AT_CHECK([ovs-ofctl monitor br0 65534 -P nxt_packet_in --detach --no-chdir --pidfile 2> ofctl_monitor.log])
 
 for i in 1 2 3; do
-    ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 60 66 66 66 02 10 88 47 00 01 40 20 00 01 41 1f 45 00 00 2c 00 00 00 00 ff 06 3b 78 c0 a8 00 01 c0 a8 00 02 00 50 00 00 00 00 00 2a 00 00 00 2a 50 00 27 10 77 44 00 00 48 4f 47 45'
+    ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 07 60 66 66 66 02 10 88 47 00 01 40 20 00 01 41 1f 45 00 00 2c 00 00 00 00 ff 06 3a 78 c0 a8 00 01 c0 a8 00 02 00 50 00 00 00 00 00 2a 00 00 00 2a 50 00 27 10 77 44 00 00 48 4f 47 45'
 done
 OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
 OVS_APP_EXIT_AND_WAIT(ovs-ofctl)
@@ -3356,7 +3356,7 @@  dnl         192.168.0.1.80 > 192.168.0.2.0: Flags [none], cksum 0x77ec (correct)
 AT_CHECK([ovs-ofctl -O OpenFlow12 monitor br0 65534 -P standard --detach --no-chdir --pidfile 2> ofctl_monitor.log])
 
 for i in 1 2 3; do
-    ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 01 60 66 66 66 00 08 88 48 00 01 41 20 45 20 00 2c 00 00 00 00 ff 06 3a 78 c0 a8 00 01 c0 a8 00 02 00 50 00 00 00 00 00 2a 00 00 00 2a 50 00 27 10 77 44 00 00 48 4f 47 45'
+    ovs-appctl netdev-dummy/receive p1 '50 54 00 00 00 01 60 66 66 66 00 08 88 48 00 01 41 20 45 20 00 2c 00 00 00 00 ff 06 3a 58 c0 a8 00 01 c0 a8 00 02 00 50 00 00 00 00 00 2a 00 00 00 2a 50 00 27 10 77 44 00 00 48 4f 47 45'
 done
 OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 6])
 OVS_APP_EXIT_AND_WAIT(ovs-ofctl)