diff mbox series

[ovs-dev] tests: Fix hash function dependencies in "tunnel - ERSPAN v1/v2 metadata".

Message ID 20180821162203.20808-1-blp@ovn.org
State Accepted
Headers show
Series [ovs-dev] tests: Fix hash function dependencies in "tunnel - ERSPAN v1/v2 metadata". | expand

Commit Message

Ben Pfaff Aug. 21, 2018, 4:22 p.m. UTC 
This test only worked if each OpenFlow port was assigned a particular
datapath port number: p1 to port 3, p2 to port 2, p3 and p4 to port 1.
This happened consistently on little-endian architectures because of the
use of a particular hash function, but on big-endian architectures it
failed because the hash function was different.

This commit fixes the problem by adding the non-dummy ports separately.
(Dummy ports try to take the datapath port number corresponding to their
name, when it is available.)  This does result in swapping a couple of
datapaths port numbers, so that p1 has port 1, p2 has port 2, and the
erspan ports have port 3, hence the size of the patch.

Reported-by: James Page <james.page@canonical.com>
Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/351382.html
Signed-off-by: Ben Pfaff <blp@ovn.org>
---
 tests/tunnel.at | 53 +++++++++++++++++++++++++++++++----------------------
 1 file changed, 31 insertions(+), 22 deletions(-)

Comments

William Tu Aug. 21, 2018, 10:49 p.m. UTC | #1 
On Tue, Aug 21, 2018 at 9:22 AM Ben Pfaff <blp@ovn.org> wrote:
>
> This test only worked if each OpenFlow port was assigned a particular
> datapath port number: p1 to port 3, p2 to port 2, p3 and p4 to port 1.
> This happened consistently on little-endian architectures because of the
> use of a particular hash function, but on big-endian architectures it
> failed because the hash function was different.
>
> This commit fixes the problem by adding the non-dummy ports separately.
> (Dummy ports try to take the datapath port number corresponding to their
> name, when it is available.)  This does result in swapping a couple of
> datapaths port numbers, so that p1 has port 1, p2 has port 2, and the
> erspan ports have port 3, hence the size of the patch.
>
> Reported-by: James Page <james.page@canonical.com>
> Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/351382.html
> Signed-off-by: Ben Pfaff <blp@ovn.org>

Thanks for the fix!
Acked-by: William Tu <u9012063@gmail.com>
Ben Pfaff Aug. 21, 2018, 11:07 p.m. UTC | #2 
On Tue, Aug 21, 2018 at 03:49:25PM -0700, William Tu wrote:
> On Tue, Aug 21, 2018 at 9:22 AM Ben Pfaff <blp@ovn.org> wrote:
> >
> > This test only worked if each OpenFlow port was assigned a particular
> > datapath port number: p1 to port 3, p2 to port 2, p3 and p4 to port 1.
> > This happened consistently on little-endian architectures because of the
> > use of a particular hash function, but on big-endian architectures it
> > failed because the hash function was different.
> >
> > This commit fixes the problem by adding the non-dummy ports separately.
> > (Dummy ports try to take the datapath port number corresponding to their
> > name, when it is available.)  This does result in swapping a couple of
> > datapaths port numbers, so that p1 has port 1, p2 has port 2, and the
> > erspan ports have port 3, hence the size of the patch.
> >
> > Reported-by: James Page <james.page@canonical.com>
> > Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2018-August/351382.html
> > Signed-off-by: Ben Pfaff <blp@ovn.org>
> 
> Thanks for the fix!
> Acked-by: William Tu <u9012063@gmail.com>

Thanks, applied and backported.
diff mbox series

Patch

diff --git a/tests/tunnel.at b/tests/tunnel.at
index ae379db120fb..8040bb81e7ba 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -513,22 +513,31 @@  AT_SETUP([tunnel - ERSPAN v1/v2 metadata])
 OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=dummy \
                         ofport_request=1 \
                     -- add-port br0 p2 -- set Interface p2 type=dummy \
-                        ofport_request=2 \
-                    -- add-port br0 p3 -- set Interface p3 type=erspan \
-                        options:remote_ip=1.1.1.1 ofport_request=3 \
-                        options:key=1 options:erspan_ver=1 options:erspan_idx=7 \
-                    -- add-port br0 p4 -- set Interface p4 type=erspan \
-                        options:remote_ip=1.1.1.2 ofport_request=4 \
-                        options:key=2 options:erspan_ver=2 options:erspan_dir=1 options:erspan_hwid=7 \
-                    ])
+                        ofport_request=2])
+
+# Add these ports separately to ensure that they get the datapath port
+# number expected below.
+ovs-vsctl -- add-port br0 p3 \
+          -- set Interface p3 type=erspan \
+                              ofport_request=3 \
+                              options:remote_ip=1.1.1.1 \
+                              options:key=1 options:erspan_ver=1 \
+                              options:erspan_idx=7 \
+          -- add-port br0 p4 \
+          -- set Interface p4 type=erspan \
+                              ofport_request=4 \
+                              options:remote_ip=1.1.1.2 \
+                              options:key=2 options:erspan_ver=2 \
+                              options:erspan_dir=1 \
+                              options:erspan_hwid=7
 OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP
 
 AT_CHECK([ovs-appctl dpif/show | tail -n +3], [0], [dnl
     br0 65534/100: (dummy-internal)
-    p1 1/3: (dummy)
+    p1 1/1: (dummy)
     p2 2/2: (dummy)
-    p3 3/1: (erspan: erspan_idx=0x7, erspan_ver=1, key=1, remote_ip=1.1.1.1)
-    p4 4/1: (erspan: erspan_dir=1, erspan_hwid=0x7, erspan_ver=2, key=2, remote_ip=1.1.1.2)
+    p3 3/3: (erspan: erspan_idx=0x7, erspan_ver=1, key=1, remote_ip=1.1.1.1)
+    p4 4/3: (erspan: erspan_dir=1, erspan_hwid=0x7, erspan_ver=2, key=2, remote_ip=1.1.1.2)
 ])
 
 AT_DATA([flows.txt], [dnl
@@ -540,40 +549,40 @@  in_port=4,tun_erspan_ver=2,tun_erspan_dir=1,tun_erspan_hwid=0xf/0x1,actions=2
 AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 
 dnl test encap: in_port=1,actions=3 (erspan v1 port)
-AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(3),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
-  [Datapath actions: set(tunnel(tun_id=0x1,dst=1.1.1.1,ttl=64,erspan(ver=1,idx=0x7),flags(df|key))),1
+  [Datapath actions: set(tunnel(tun_id=0x1,dst=1.1.1.1,ttl=64,erspan(ver=1,idx=0x7),flags(df|key))),3
 ])
 
 dnl test encap: in_port=2,actions=4 (erspan v2 port)
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(2),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
-  [Datapath actions: set(tunnel(tun_id=0x2,dst=1.1.1.2,ttl=64,erspan(ver=2,dir=1,hwid=0x7),flags(df|key))),1
+  [Datapath actions: set(tunnel(tun_id=0x2,dst=1.1.1.2,ttl=64,erspan(ver=2,dir=1,hwid=0x7),flags(df|key))),3
 ])
 
 dnl receive packet from ERSPAN port with v1 metadata
-AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x1,src=1.1.1.1,dst=2.2.2.2,ttl=64,erspan(ver=1,idx=0x7),flags(df|key)),in_port(1),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x1,src=1.1.1.1,dst=2.2.2.2,ttl=64,erspan(ver=1,idx=0x7),flags(df|key)),in_port(3),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
   [Megaflow: recirc_id=0,eth,ip,tun_id=0x1,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=0,tun_erspan_ver=1,tun_erspan_idx=0x7,tun_flags=+df-csum+key,in_port=3,nw_frag=no
-Datapath actions: 3
+Datapath actions: 1
 ])
 
 dnl receive packet from ERSPAN port with wrong v1 metadata
-AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x1,src=1.1.1.1,dst=2.2.2.2,ttl=64,erspan(ver=1,idx=0xabcd),flags(df|key)),in_port(1),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x1,src=1.1.1.1,dst=2.2.2.2,ttl=64,erspan(ver=1,idx=0xabcd),flags(df|key)),in_port(3),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
   [Megaflow: recirc_id=0,eth,ip,tun_id=0x1,tun_src=1.1.1.1,tun_dst=2.2.2.2,tun_tos=0,tun_erspan_ver=1,tun_erspan_idx=0xabcd,tun_flags=+df-csum+key,in_port=3,nw_frag=no
 Datapath actions: drop
 ])
 
 dnl receive packet from ERSPAN port with v2 metadata
-AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x2,src=1.1.1.2,dst=2.2.2.2,ttl=64,erspan(ver=2,dir=1,hwid=0x7),flags(df|key)),in_port(1),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x2,src=1.1.1.2,dst=2.2.2.2,ttl=64,erspan(ver=2,dir=1,hwid=0x7),flags(df|key)),in_port(3),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
   [Megaflow: recirc_id=0,eth,ip,tun_id=0x2,tun_src=1.1.1.2,tun_dst=2.2.2.2,tun_tos=0,tun_erspan_ver=2,tun_erspan_dir=1,tun_erspan_hwid=0x1,tun_flags=+df-csum+key,in_port=4,nw_frag=no
 Datapath actions: 2
 ])
 
 dnl receive packet from ERSPAN port with wrong v2 metadata
-AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x2,src=1.1.1.2,dst=2.2.2.2,ttl=64,erspan(ver=2,dir=0,hwid=0x17),flags(df|key)),in_port(1),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x2,src=1.1.1.2,dst=2.2.2.2,ttl=64,erspan(ver=2,dir=0,hwid=0x17),flags(df|key)),in_port(3),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
   [Megaflow: recirc_id=0,eth,ip,tun_id=0x2,tun_src=1.1.1.2,tun_dst=2.2.2.2,tun_tos=0,tun_erspan_ver=2,tun_erspan_dir=0,tun_erspan_hwid=0x1,tun_flags=+df-csum+key,in_port=4,nw_frag=no
 Datapath actions: drop
@@ -593,7 +602,7 @@  NXST_FLOW reply:
 ])
 
 dnl this time it won't drop
-AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x2,src=1.1.1.2,dst=2.2.2.2,ttl=64,erspan(ver=2,dir=0,hwid=0x17),flags(df|key)),in_port(1),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x2,src=1.1.1.2,dst=2.2.2.2,ttl=64,erspan(ver=2,dir=0,hwid=0x17),flags(df|key)),in_port(3),skb_mark(0),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(frag=no)'], [0], [stdout])
 AT_CHECK([tail -2 stdout], [0],
   [Megaflow: recirc_id=0,eth,ip,tun_id=0x2,tun_src=1.1.1.2,tun_dst=2.2.2.2,tun_tos=0,tun_erspan_ver=2,tun_flags=+df-csum+key,in_port=4,nw_frag=no
 Datapath actions: 2
@@ -613,9 +622,9 @@  NXST_FLOW reply:
  in_port=1 actions=set_tunnel:0xb,set_field:0x7->tun_erspan_idx,output:5
 ])
 
-AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(3),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'in_port(1),eth(src=50:54:00:00:00:05,dst=50:54:00:00:00:07),eth_type(0x0800),ipv4(src=192.168.0.1,dst=192.168.0.2,proto=1,tos=0,ttl=128,frag=no),icmp(type=8,code=0)'], [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
-  [Datapath actions: set(tunnel(tun_id=0xb,dst=1.1.1.2,ttl=64,erspan(ver=1,idx=0x7),flags(df|key))),1
+  [Datapath actions: set(tunnel(tun_id=0xb,dst=1.1.1.2,ttl=64,erspan(ver=1,idx=0x7),flags(df|key))),3
 ])
 
 OVS_VSWITCHD_STOP