[ovs-dev] [PATCH 1/4] tests: Add a tunnel packet-out test.

Fri May 20 18:44:54 UTC 2016

Hi William,

thanks for your feedback, responses inline.

I've sent a v2 of the test (the rest of the series has been acked and pushed)

http://openvswitch.org/pipermail/dev/2016-May/071433.html

I'm not sure it's worth merging anymore, since we fixed the bug, I'll leave the decision to you :-)

Thanks,

Daniele

On 18/05/2016 11:25, "William Tu" <u9012063 at gmail.com> wrote:

>Hi Daniele,
>
>
>Thanks! this saves a lot of time to reproduce the error. 
>The new testcase can exercise the code path correctly to reproduce the error, and after applying the 4 patches the error at " 2032:
>ovn.at:1230 <http://ovn.at:1230> ovn -- 3 HVs, 3 LS, 3 lports/LS, 1 LR" is resolved.
>
>
>
>The ukey issue also disappears.
>some comments inline. 
>
>
>On Tue, May 17, 2016 at 10:31 PM, Daniele Di Proietto 
><diproiettod at vmware.com> wrote:
>
>We only stress the same code path in testcase "ovn -- 3 HVs, 3 LS,
>3 lports/LS, 1 LR", which is slow to execute under valgrind.
>
>This makes it easier to reproduce a valgrind error, which is fixed by a
>later commit.
>
>Signed-off-by: Daniele Di Proietto <diproiettod at vmware.com>
>---
> tests/tunnel-push-pop.at <http://tunnel-push-pop.at> | 43 +++++++++++++++++++++++++++++++++++++++++++
> 1 file changed, 43 insertions(+)
>
>diff --git a/tests/tunnel-push-pop.at <http://tunnel-push-pop.at> b/tests/tunnel-push-pop.at <http://tunnel-push-pop.at>
>index 64a0cbe..6d55110 100644
>--- a/tests/tunnel-push-pop.at <http://tunnel-push-pop.at>
>+++ b/tests/tunnel-push-pop.at <http://tunnel-push-pop.at>
>@@ -43,6 +43,11 @@ 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 Wait for the two ARP requests to be sent. Sometime the system
>+dnl can be slow (e.g. under valgrind)
>+OVS_WAIT_UNTIL(test `ovs-pcap p0.pcap | sort | uniq | wc -l` -ge 2)
>+
>
>
>
>
>Is the above code related?

You're right, I should move this to a separate commit.

> 
> 
>
>
> AT_CHECK([ovs-pcap p0.pcap > p0.pcap.txt 2>&1])
>
> AT_CHECK([cat p0.pcap.txt | grep 101025c | uniq], [0], [dnl
>@@ -163,3 +168,41 @@ tunnel(tun_id=0x7b,src=1.1.2.92,dst=1.1.2.88,geneve({class=0xffff,type=0x80,len=
>
> OVS_VSWITCHD_STOP
> AT_CLEANUP
>+
>+AT_SETUP([tunnel_push_pop - packet_out])
>+
>+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-appctl vlog/set dpif_netdev:dbg])
>+AT_CHECK([ovs-vsctl add-br int-br -- set bridge int-br datapath_type=dummy])
>+AT_CHECK([ovs-vsctl add-port int-br t2 -- set Interface t2 type=geneve \
>+                       options:remote_ip=1.1.2.92 options:key=123 ofport_request=2 \
>+                       ])
>+
>+dnl First setup dummy interface IP address, then add the route
>+dnl so that tnl-port table can get valid IP address for the device.
>+AT_CHECK([ovs-appctl netdev-dummy/ip4addr br0 
>1.1.2.88/24 <http://1.1.2.88/24>], [0], [OK
>+])
>+
>+AT_CHECK([ovs-appctl ovs/route/add 
>1.1.2.92/24 <http://1.1.2.92/24> br0], [0], [OK
>+])
>+
>+AT_CHECK([ovs-ofctl add-flow br0 action=normal])
>+
>+dnl This ARP reply from p0 has two effects:
>+dnl 1. The ARP cache will learn that 1.1.2.92 is at f8:bc:12:44:34:b6.
>+dnl 2. The br0 mac learning will learn that f8:bc:12:44:34:b6 is on p0.
>+AT_CHECK([ovs-appctl netdev-dummy/receive p0 'recirc_id(0),in_port(2),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-vsctl -- set Interface p0 options:tx_pcap=p0.pcap])
>+
>+dnl Output to tunnel from a int-br internal port
>+AT_CHECK([ovs-ofctl add-flow int-br "in_port=LOCAL,actions=output:2"])
>+AT_CHECK([ovs-appctl netdev-dummy/receive int-br '50540000000a5054000000091234'])
>+OVS_WAIT_UNTIL(test `ovs-pcap p0.pcap | grep 50540000000a5054000000091234 | wc -l` -ge 1)
>+
>
>
>I'm not sure the above is necessary, but there is no harm to have more test cases.

I guess it's not really necessary, it's there to compare packet-out and the port input.

I decided to keep it.

>
> 
>
>+dnl Output to tunnel from the controller
>+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out int-br CONTROLLER "output:2" '50540000000a5054000000091235'])
>+OVS_WAIT_UNTIL(test `ovs-pcap p0.pcap | grep 50540000000a5054000000091235 | wc -l` -ge 1)
>+
>
>
>The above case will exercise the code path. 
>
> 
>
>Regards,
>
>William
>
>
>
>
>