On 4 May 2017 at 04:39, Zoltán Balogh <[email protected]> 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.

Thanks for taking a look. Andy and I have been throwing some thoughts
around about this, but I'm not sure we came to a concrete solution yet
either. My main thought is that I think that the 'flow' needs to be
modified in the similar way that the 'push_tnl' would work in the
datapath - updating the IP/UDP fields in a protocol-specific manner.
Then the shared code between patch ports and this tunneling
translation needs close attention to check everything is lined up
correctly, restored after output translation, etc.

Given that master is broken, it would be nice to restore it to a good
state. The quickest way to do so would be to revert this patch on
master. Then you could re-propose the patches to achieve this 'direct
translation of tunneling' logic. That said, if you expect this to be
fixed shortly then perhaps we could just wait for a fix. The main
worry I have is that the translation code tends to be pretty
elaborate/subtle so getting a solid fix in this area may take some
time (and my regular tester box has already been complaining at me for
a while now).

What do you think? I'm happy to give you a bit more time if you think
that's the best approach.

Cheers,
Joe
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to