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" <u9012...@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 ><diproiet...@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 <diproiet...@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 > > > > > _______________________________________________ dev mailing list dev@openvswitch.org http://openvswitch.org/mailman/listinfo/dev
