diff mbox

[ovs-dev,v5] tunneling: Avoid recirculation on datapath by computing the recirculate actions at translate time.

Message ID CALDO+SZ8F8xN=CU+3Hx8P2gf40eYxnnOJm=GeeVj4oZUVvKw6g@mail.gmail.com
State Not Applicable
Headers show

Commit Message

William Tu May 4, 2017, 3:08 p.m. UTC
On Thu, May 4, 2017 at 4:39 AM, Zoltán Balogh
<zoltan.balogh@ericsson.com> wrote:
>> I think that what's happening is that when build_tunnel_send()
>> serializes the ODP action for push_tunnel, it doesn't update the
>> 'flow' to reflect the new encapsulated state of the packet. Then, when
>> calling apply_nested_clone_actions() it performs lookup in the second
>> bridge using the unmodified flow, ie as though it hasn't been
>> encapsulated at all. If, for instance, on the underlay bridge you
>> match on the tunnel protocol type or port and attempt to forward such
>> packets directly to the external interface then have a default drop
>> for packets that don't match, you'll see the traffic get dropped. The
>> resulting datapath flows can end up generating a set of actions with
>> "clone(drop),push_tnl(...)", which also seems wrong.
>>
>> William and I have been looking at this a bit, but it'd be good if you
>> had a chance to look too. I think that William is working on a simpler
>> test case to reproduce. For reference if you are not familiar with
>> "make check-system-userspace" tests, there is some documentation
>> available below---the system-traffic.at tests are the same for 'make
>> check-kernel' and 'make check-system-userspace'.
>>
>> http://docs.openvswitch.org/en/latest/topics/testing/#datapath-testing
>>
>> Cheers,
>> Joe
>
> Hi Joe,
>
> Thank you for the notification! I've observed the faulty behavior when the packet is sent out on a patch port after a tunnel header was pushed. I have a script to setup a simple tunneling scenario on a single host for testing.
> The attached script creates and connects two namespaces (ns1, ns2) over a userspace tunnel like below.
>
> # GRE tunneling test setup
>
>  192.168.10.10
>       |      +-------------+                  +-------------+
>       |      |             |                  |             |   192.168.10.20
>      ns1 <-->o    br-in1   |                  |    br-in2   |      |
>              |             |                  |             o<--> ns2
>              +------o------+                  +------o------+
>                    gre1                             gre2
>   10.0.0.1              LOCAL       20.0.0.2             LOCAL
>  (10.0.0.2)  +-----------o-+       (20.0.0.1) +-----------o-+
>              |             |                  |             |
>              |    br-p1    |                  |    br-p2    |
>              |             |                  |             |
>              +------o------+                  +------o------+
>        patch1/veth1 |                                | patch2/veth2
>                     +--------------------------------+
>
>
>
> If I use veth ports for the tunnel, then ping between ns1 and ns2 does work fine.
>
>  > ./setup_tunnel.sh
>  > ip netns exec ns1 ping 192.168.10.20 -c 1
>  PING 192.168.10.20 (192.168.10.20) 56(84) bytes of data.
>  64 bytes from 192.168.10.20: icmp_seq=1 ttl=64 time=1.31 ms
>
>  --- 192.168.10.20 ping statistics ---
>  1 packets transmitted, 1 received, 0% packet loss, time 0ms
>  rtt min/avg/max/mdev = 1.312/1.312/1.312/0.000 ms
>
>
> If I create patch ports instead of the veth ones (passing 'patch-port' as arg to the script), then ping does not work anymore. I can see the "clone(drop),push_tnl(...)" datapath flow as you mentioned before.
>
>  > ./setup_tunnel.sh patch-port
>  > ip netns exec ns1 ping 192.168.10.20 -c 1
>  PING 192.168.10.20 (192.168.10.20) 56(84) bytes of data.
>
>  --- 192.168.10.20 ping statistics ---
>  1 packets transmitted, 0 received, 100% packet loss, time 0ms
>
> I've been working on a fix, but have not found a proper solution yet.
>
> Best regards,
> Zoltan

Hi Zoltan,

Thanks for working on this! I've also created a simple test case to
reproduce the issue, please see patch below. When running
~/ovs# make check TESTSUITEFLAGS='783'
~/ovs# cat tests/testsuite.dir/0783/testsuite.log

bridge("int-br")
----------------
 0. priority 32768
    output:3
     -> output to native tunnel
     -> tunneling to 1.1.2.92 via br0
     -> tunneling from aa:55:aa:55:00:00 1.1.2.88 to f8:bc:12:44:34:b6 1.1.2.92

bridge("br0")
-------------
 0. udp, priority 99   ----> here the packet is GRE and shouldn't match udp,
    NORMAL
     -> no learned MAC for destination, flooding

Final flow: unchanged
Megaflow: recirc_id=0,udp,in_port=LOCAL,vlan_tci=0x0000/0x1fff,dl_src=00:00:00:00:00:00,dl_dst=00:00:00:00:00:00,nw_ecn=0,nw_frag=no
Datapath actions:
clone(tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0x1c8)),out_port(100)),1)

---
At br0 we should have the outer GRE header applies to the flow so the
br0 can match on the GRE header. But the failed case is still matching
on the inner header.

--- patch for the testcase ---
diff --git a/tests/tunnel-push-pop.at b/tests/tunnel-push-pop.at
index 294d28a2416d..71d14ba587f0 100644
+NXST_FLOW reply:
+])
+
+OVS_VSWITCHD_STOP
+AT_CLEANUP
+

Regards,
William
diff mbox

Patch

--- a/tests/tunnel-push-pop.at
+++ b/tests/tunnel-push-pop.at
@@ -1,5 +1,71 @@ 
 AT_BANNER([tunnel_push_pop])

+AT_SETUP([tunnel_push_pop - faild after apply nested clone])
+
+OVS_VSWITCHD_START([add-port br0 p0 -- set Interface p0 type=dummy
ofport_request=1 other-config:hwaddr=aa:55:aa:
55:00:00])
+AT_CHECK([ovs-vsctl add-br int-br -- set bridge int-br
datapath_type=dummy], [0])
+AT_CHECK([ovs-vsctl add-port int-br t1 -- set Interface t1 type=gre \
+                       options:remote_ip=1.1.2.92 options:key=456
ofport_request=3], [0])
+
+AT_CHECK([ovs-appctl dpif/show], [0], [dnl
+dummy@ovs-dummy: hit:0 missed:0
+       br0:
+               br0 65534/100: (dummy-internal)
+               p0 1/1: (dummy)
+       int-br:
+               int-br 65534/2: (dummy-internal)
+               t1 3/3: (gre: key=456, remote_ip=1.1.2.92)
+])
+
+AT_CHECK([ovs-appctl dpif/show], [0], [dnl
+dummy@ovs-dummy: hit:0 missed:0
+       br0:
+               br0 65534/100: (dummy-internal)
+               p0 1/1: (dummy)
+       int-br:
+               int-br 65534/2: (dummy-internal)
+               t1 3/3: (gre: key=456, remote_ip=1.1.2.92)
+])
+
+AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 1.1.2.88/24], [0], [OK
+])
+AT_CHECK([ovs-appctl ovs/route/add 1.1.2.92/24 br0], [0], [OK
+])
+AT_CHECK([ovs-ofctl add-flow br0 'priority=1,action=normal'])
+
+dnl Check ARP request
+AT_CHECK([ovs-vsctl -- set Interface p0 options:pcap=p0.pcap])
+AT_CHECK([ovs-appctl netdev-dummy/receive int-br
'in_port(2),eth(src=aa:55:aa:55:00:00,dst=f8:bc:12:ff:ff:ff),eth_type(0x0800),ipv4(src=1.1.3.92,dst=1.1.3.88,proto=1,tos=0,ttl=64,frag=no),icmp(type=0,code=0)'])
+
+dnl Check ARP Snoop
+AT_CHECK([ovs-appctl netdev-dummy/receive br0
'recirc_id(0),in_port(100),eth(src=f8:bc:12:44:34:b6,dst=ff:ff:ff:ff:ff:ff),eth_type(0x0806),arp(sip=1.1.2.92,tip=1.1.2.88,op=2,sha=f8:bc:12:44:34:b6,tha=00:00:00:00:00:00)'])
+
+AT_CHECK([ovs-appctl tnl/neigh/show | tail -n+3 | sort], [0], [dnl
+1.1.2.92                                      f8:bc:12:44:34:b6   br0
+])
+
+AT_CHECK([ovs-appctl tnl/ports/show |sort], [0], [dnl
+Listening ports:
+gre_sys (3)
+])
+
+AT_CHECK([ovs-ofctl add-flow br0 'priority=99,udp,action=normal'])
+
+dnl Check GRE tunnel push
+AT_CHECK([ovs-ofctl add-flow int-br action=3])
+AT_CHECK([ovs-appctl ofproto/trace ovs-dummy
'in_port(2),eth_type(0x0800),ipv4(src=1.1.3.88,dst=1.1.3.112,proto=17,tos=0,ttl=64,frag=no),udp(src=51283,dst=4789)'],
[0], [stdout])
+
+AT_CHECK([tail -1 stdout], [0],
+  [Datapath actions:
clone(tnl_push(tnl_port(3),header(size=42,type=3,eth(dst=f8:bc:12:44:34:b6,src=aa:55:aa:55:00:00,dl_type=0x0800),ipv4(src=1.1.2.88,dst=1.1.2.92,proto=47,tos=0,ttl=64,frag=0x4000),gre((flags=0x2000,proto=0x6558),key=0x1c8)),out_port(100)),1)
+])
+
+AT_CHECK([ovs-ofctl dump-flows br0], [0], [dnl)