diff mbox series

[ovs-dev,PATCHv2] tunnel: fix tunnel flags set/clear.

Message ID 1515618791-564-1-git-send-email-u9012063@gmail.com
State Accepted
Headers show
Series [ovs-dev,PATCHv2] tunnel: fix tunnel flags set/clear. | expand

Commit Message

William Tu Jan. 10, 2018, 9:13 p.m. UTC 
Existing code only set these tunnel flags (df, csum, and key) when the
flag is set in the output tunnel port, but did not clear when the flag
is unset.  The patch fixes it by setting and clearing it accordingly.

Two existing testcases need to fix:
'tunnel - Geneve option present' has no key set up, so we should match
'flags(df)' instead of 'flags(df|key)'.  The second case
'tunnel - concomitant IPv6 and IPv4 tunnels' follows the same pattern.
One additional test case 'tunnel - Mix Geneve/GRE options' is added.

Signed-off-by: William Tu <u9012063@gmail.com>
VMWare-BZ: #2019012
---
v1->v2:
  refactor the tunnel flag set/unset code.
---
 ofproto/tunnel.c |  1 +
 tests/tunnel.at  | 37 ++++++++++++++++++++++++++++++++++---
 2 files changed, 35 insertions(+), 3 deletions(-)

Comments

Ben Pfaff Jan. 10, 2018, 10:24 p.m. UTC | #1 
On Wed, Jan 10, 2018 at 01:13:11PM -0800, William Tu wrote:
> Existing code only set these tunnel flags (df, csum, and key) when the
> flag is set in the output tunnel port, but did not clear when the flag
> is unset.  The patch fixes it by setting and clearing it accordingly.
> 
> Two existing testcases need to fix:
> 'tunnel - Geneve option present' has no key set up, so we should match
> 'flags(df)' instead of 'flags(df|key)'.  The second case
> 'tunnel - concomitant IPv6 and IPv4 tunnels' follows the same pattern.
> One additional test case 'tunnel - Mix Geneve/GRE options' is added.
> 
> Signed-off-by: William Tu <u9012063@gmail.com>
> VMWare-BZ: #2019012
> ---
> v1->v2:
>   refactor the tunnel flag set/unset code.

Thank you.

I applied this to master and branch-2.8.  It did not apply to branch-2.7
because of conflicts in the test case.  I don't know whether it is
important to backport to that and earlier branches.
diff mbox series

Patch

diff --git a/ofproto/tunnel.c b/ofproto/tunnel.c
index 770e0103476a..e0214ced69e2 100644
--- a/ofproto/tunnel.c
+++ b/ofproto/tunnel.c
@@ -464,6 +464,7 @@  tnl_port_send(const struct ofport_dpif *ofport, struct flow *flow,
         }
     }
 
+    flow->tunnel.flags &= ~(FLOW_TNL_F_MASK & ~FLOW_TNL_PUB_F_MASK);
     flow->tunnel.flags |= (cfg->dont_fragment ? FLOW_TNL_F_DONT_FRAGMENT : 0)
         | (cfg->csum ? FLOW_TNL_F_CSUM : 0)
         | (cfg->out_key_present ? FLOW_TNL_F_KEY : 0);
diff --git a/tests/tunnel.at b/tests/tunnel.at
index 767af99bfe9c..3c217b344f9b 100644
--- a/tests/tunnel.at
+++ b/tests/tunnel.at
@@ -647,7 +647,7 @@  Datapath actions: 2
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0xffff,type=1,len=0}),flags(df|key)),in_port(6081),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=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=+df-csum+key,tun_metadata1,tun_metadata2=NP,in_port=1,nw_ecn=0,nw_frag=no
-Datapath actions: set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,tp_dst=6081,geneve({class=0xffff,type=0x1,len=0}),flags(df|key))),6081
+Datapath actions: set(tunnel(dst=1.1.1.1,ttl=64,tp_dst=6081,geneve({class=0xffff,type=0x1,len=0}),flags(df))),6081
 ])
 
 OVS_VSWITCHD_STOP
@@ -668,14 +668,45 @@  AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
 
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'tunnel(tun_id=0,src=1.1.1.1,dst=1.1.1.2,ttl=64),in_port(4789)'], [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
-  [Datapath actions: set(tunnel(tun_id=0x0,ipv6_dst=2001:cafe::1,ttl=64,tp_dst=4789,flags(df|key))),4789
+  [Datapath actions: set(tunnel(ipv6_dst=2001:cafe::1,ttl=64,tp_dst=4789,flags(df))),4789
 ])
 
 AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'tunnel(tun_id=0x0,ipv6_src=2001:cafe::1,ipv6_dst=2001:cafe::2,ttl=64),in_port(4789)'], [0], [stdout])
 AT_CHECK([tail -1 stdout], [0],
-  [Datapath actions: set(tunnel(tun_id=0x0,dst=1.1.1.1,ttl=64,tp_dst=4789,flags(df|key))),4789
+  [Datapath actions: set(tunnel(dst=1.1.1.1,ttl=64,tp_dst=4789,flags(df))),4789
 ])
 
 
 OVS_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([tunnel - Mix Geneve/GRE options])
+OVS_VSWITCHD_START([add-port br0 p1 -- set Interface p1 type=geneve \
+                    options:remote_ip=1.1.1.1 options:csum=true ofport_request=1 \
+                    -- add-port br0 p2 -- set Interface p2 type=dummy \
+                    ofport_request=2 ofport_request=2 \
+                    -- add-port br0 p3 -- set Interface p3 type=gre \
+                    options:remote_ip=2.2.2.2 options:csum=false options:key=123 ofport_request=3])
+OVS_VSWITCHD_DISABLE_TUNNEL_PUSH_POP
+
+AT_DATA([flows.txt], [dnl
+priority=1,in_port=1,actions=3
+])
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+AT_CHECK([ovs-ofctl dump-flows br0 | ofctl_strip | sort],
+[0], [dnl
+ priority=1,in_port=1 actions=output:3
+NXST_FLOW reply:
+])
+
+dnl the input packet from geneve tunnel has flags(-df+csum+key) flags, making
+dnl sure that the output gre tunnel has (+df-csum+key).
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy 'recirc_id(0),tunnel(tun_id=0x0,src=1.1.1.1,dst=1.1.1.2,ttl=64,geneve({class=0xffff,type=0,len=4,0x12345678}),flags(csum|key)),in_port(6081),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=0,tun_src=1.1.1.1,tun_dst=1.1.1.2,tun_tos=0,tun_flags=-df+csum+key,in_port=1,nw_ecn=0,nw_frag=no
+Datapath actions: set(tunnel(tun_id=0x7b,dst=2.2.2.2,ttl=64,flags(df|key))),1
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP