diff mbox

[ovs-dev] tests: Add force/commit test to system-traffic.at

Message ID 1500313463-15636-1-git-send-email-joe@ovn.org
State Superseded
Headers show

Commit Message

Gregory Rose July 17, 2017, 5:44 p.m. UTC 
From: Greg Rose <gvrose8192@gmail.com>

Add a new test for the kernel module datapath to test if the
conntrack force direction change and commit is working correctly.
This test was used to find and root cause BZ 18090854.

Signed-off-by: Joe Stringer <joe@ovn.org>
Signed-off-by: Greg Rose <gvrose8192@gmail.com>
---
 tests/system-traffic.at | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

Comments

Gregory Rose July 17, 2017, 5:54 p.m. UTC | #1 
On 07/17/2017 10:44 AM, gvrose8192@gmail.com wrote:
> From: Greg Rose <gvrose8192@gmail.com>
> 
> Add a new test for the kernel module datapath to test if the
> conntrack force direction change and commit is working correctly.
> This test was used to find and root cause BZ 18090854.
> 
> Signed-off-by: Joe Stringer <joe@ovn.org>
> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
> ---
>   tests/system-traffic.at | 12 +++++++++++-
>   1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> index b2393f5..e83ad3a 100644
> --- a/tests/system-traffic.at
> +++ b/tests/system-traffic.at
> @@ -789,7 +789,7 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
>   AT_DATA([flows.txt], [dnl
>   priority=1,action=drop
>   priority=10,arp,action=normal
> -priority=100,in_port=1,udp,action=ct(commit),controller
> +priority=100,in_port=1,udp,action=ct(force,commit),controller
>   priority=100,in_port=2,ct_state=-trk,udp,action=ct(table=0)
>   priority=100,in_port=2,ct_state=+trk+est,udp,action=ct(force,commit,table=1)
>   table=1,in_port=2,ct_state=+trk,udp,action=controller
> @@ -826,6 +826,16 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.2,"], [],
>   udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2)
>   ])
>   
> +ovs-appctl vlog/set dpif:dbg
> +
> +dnl OK, now send another packet from port 1 and see that it switches again
> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
> +AT_CHECK([ovs-appctl revalidator/purge], [0])
> +
> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.1,"], [], [dnl
> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
> +])
> +
>   OVS_TRAFFIC_VSWITCHD_STOP
>   AT_CLEANUP
>   
> 
git send-email --from="joe@ovn.org" didn't work I guess.

Joe Stringer is the author.  Sorry for the mistake.

- Greg
Joe Stringer July 17, 2017, 6:05 p.m. UTC | #2 
On 17 July 2017 at 10:54, Greg Rose <gvrose8192@gmail.com> wrote:
> On 07/17/2017 10:44 AM, gvrose8192@gmail.com wrote:
>>
>> From: Greg Rose <gvrose8192@gmail.com>
>>
>> Add a new test for the kernel module datapath to test if the
>> conntrack force direction change and commit is working correctly.
>> This test was used to find and root cause BZ 18090854.
>>
>> Signed-off-by: Joe Stringer <joe@ovn.org>
>> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
>> ---
>>   tests/system-traffic.at | 12 +++++++++++-
>>   1 file changed, 11 insertions(+), 1 deletion(-)
>>
>> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>> index b2393f5..e83ad3a 100644
>> --- a/tests/system-traffic.at
>> +++ b/tests/system-traffic.at
>> @@ -789,7 +789,7 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
>>   AT_DATA([flows.txt], [dnl
>>   priority=1,action=drop
>>   priority=10,arp,action=normal
>> -priority=100,in_port=1,udp,action=ct(commit),controller
>> +priority=100,in_port=1,udp,action=ct(force,commit),controller
>>   priority=100,in_port=2,ct_state=-trk,udp,action=ct(table=0)
>>
>> priority=100,in_port=2,ct_state=+trk+est,udp,action=ct(force,commit,table=1)
>>   table=1,in_port=2,ct_state=+trk,udp,action=controller
>> @@ -826,6 +826,16 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep
>> "orig=.src=10\.1\.1\.2,"], [],
>>
>> udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2)
>>   ])
>>   +ovs-appctl vlog/set dpif:dbg
>> +
>> +dnl OK, now send another packet from port 1 and see that it switches
>> again
>> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1
>> packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000
>> actions=resubmit(,0)"])
>> +AT_CHECK([ovs-appctl revalidator/purge], [0])
>> +
>> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep
>> "orig=.src=10\.1\.1\.1,"], [], [dnl
>>
>> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
>> +])
>> +
>>   OVS_TRAFFIC_VSWITCHD_STOP
>>   AT_CLEANUP
>>
>
> git send-email --from="joe@ovn.org" didn't work I guess.
>
> Joe Stringer is the author.  Sorry for the mistake.

No worries, I can fix this up when I apply. Thanks for pointing it out.

If in doubt, you can always use "git format-patch -o myseries",
read/edit the files, then run "git send-email myseries/*"
Gregory Rose July 17, 2017, 6:07 p.m. UTC | #3 
On 07/17/2017 11:05 AM, Joe Stringer wrote:
> On 17 July 2017 at 10:54, Greg Rose <gvrose8192@gmail.com> wrote:
> > On 07/17/2017 10:44 AM, gvrose8192@gmail.com wrote:
> >>
> >> From: Greg Rose <gvrose8192@gmail.com>
> >>
> >> Add a new test for the kernel module datapath to test if the
> >> conntrack force direction change and commit is working correctly.
> >> This test was used to find and root cause BZ 18090854.
> >>
> >> Signed-off-by: Joe Stringer <joe@ovn.org>
> >> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
> >> ---
> >>    tests/system-traffic.at | 12 +++++++++++-
> >>    1 file changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> >> index b2393f5..e83ad3a 100644
> >> --- a/tests/system-traffic.at
> >> +++ b/tests/system-traffic.at
> >> @@ -789,7 +789,7 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
> >>    AT_DATA([flows.txt], [dnl
> >>    priority=1,action=drop
> >>    priority=10,arp,action=normal
> >> -priority=100,in_port=1,udp,action=ct(commit),controller
> >> +priority=100,in_port=1,udp,action=ct(force,commit),controller
> >>    priority=100,in_port=2,ct_state=-trk,udp,action=ct(table=0)
> >>
> >> priority=100,in_port=2,ct_state=+trk+est,udp,action=ct(force,commit,table=1)
> >>    table=1,in_port=2,ct_state=+trk,udp,action=controller
> >> @@ -826,6 +826,16 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep
> >> "orig=.src=10\.1\.1\.2,"], [],
> >>
> >> udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2)
> >>    ])
> >>    +ovs-appctl vlog/set dpif:dbg
> >> +
> >> +dnl OK, now send another packet from port 1 and see that it switches
> >> again
> >> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1
> >> packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000
> >> actions=resubmit(,0)"])
> >> +AT_CHECK([ovs-appctl revalidator/purge], [0])
> >> +
> >> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep
> >> "orig=.src=10\.1\.1\.1,"], [], [dnl
> >>
> >> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
> >> +])
> >> +
> >>    OVS_TRAFFIC_VSWITCHD_STOP
> >>    AT_CLEANUP
> >>
> >
> > git send-email --from="joe@ovn.org" didn't work I guess.
> >
> > Joe Stringer is the author.  Sorry for the mistake.
>
> No worries, I can fix this up when I apply. Thanks for pointing it out.
>
> If in doubt, you can always use "git format-patch -o myseries",
> read/edit the files, then run "git send-email myseries/*"
>
Sure - but it'd be nice if git worked right.  I probably did some minor thing wrong...

Thanks,

- Greg
Ben Pfaff July 17, 2017, 6:07 p.m. UTC | #4 
On Mon, Jul 17, 2017 at 10:54:31AM -0700, Greg Rose wrote:
> On 07/17/2017 10:44 AM, gvrose8192@gmail.com wrote:
> >From: Greg Rose <gvrose8192@gmail.com>
> >
> >Add a new test for the kernel module datapath to test if the
> >conntrack force direction change and commit is working correctly.
> >This test was used to find and root cause BZ 18090854.
> >
> >Signed-off-by: Joe Stringer <joe@ovn.org>
> >Signed-off-by: Greg Rose <gvrose8192@gmail.com>
> >---
> >  tests/system-traffic.at | 12 +++++++++++-
> >  1 file changed, 11 insertions(+), 1 deletion(-)
> >
> >diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> >index b2393f5..e83ad3a 100644
> >--- a/tests/system-traffic.at
> >+++ b/tests/system-traffic.at
> >@@ -789,7 +789,7 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
> >  AT_DATA([flows.txt], [dnl
> >  priority=1,action=drop
> >  priority=10,arp,action=normal
> >-priority=100,in_port=1,udp,action=ct(commit),controller
> >+priority=100,in_port=1,udp,action=ct(force,commit),controller
> >  priority=100,in_port=2,ct_state=-trk,udp,action=ct(table=0)
> >  priority=100,in_port=2,ct_state=+trk+est,udp,action=ct(force,commit,table=1)
> >  table=1,in_port=2,ct_state=+trk,udp,action=controller
> >@@ -826,6 +826,16 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.2,"], [],
> >  udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2)
> >  ])
> >+ovs-appctl vlog/set dpif:dbg
> >+
> >+dnl OK, now send another packet from port 1 and see that it switches again
> >+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
> >+AT_CHECK([ovs-appctl revalidator/purge], [0])
> >+
> >+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.1,"], [], [dnl
> >+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
> >+])
> >+
> >  OVS_TRAFFIC_VSWITCHD_STOP
> >  AT_CLEANUP
> >
> git send-email --from="joe@ovn.org" didn't work I guess.
> 
> Joe Stringer is the author.  Sorry for the mistake.

To send someone else's patch, ensure that the author is correct in the
commit itself.  Sometimes this takes "git commit --amend
--author='...'".  Then send it the usual way with "git send-email",
which will leave you as the envelope sender of the email and put the
real author as the first line "From: ...author...".
Gregory Rose July 17, 2017, 6:19 p.m. UTC | #5 
On 07/17/2017 11:07 AM, Ben Pfaff wrote:
> On Mon, Jul 17, 2017 at 10:54:31AM -0700, Greg Rose wrote:
> > On 07/17/2017 10:44 AM, gvrose8192@gmail.com wrote:
> >> From: Greg Rose <gvrose8192@gmail.com>
> >>
> >> Add a new test for the kernel module datapath to test if the
> >> conntrack force direction change and commit is working correctly.
> >> This test was used to find and root cause BZ 18090854.
> >>
> >> Signed-off-by: Joe Stringer <joe@ovn.org>
> >> Signed-off-by: Greg Rose <gvrose8192@gmail.com>
> >> ---
> >>   tests/system-traffic.at | 12 +++++++++++-
> >>   1 file changed, 11 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/tests/system-traffic.at b/tests/system-traffic.at
> >> index b2393f5..e83ad3a 100644
> >> --- a/tests/system-traffic.at
> >> +++ b/tests/system-traffic.at
> >> @@ -789,7 +789,7 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
> >>   AT_DATA([flows.txt], [dnl
> >>   priority=1,action=drop
> >>   priority=10,arp,action=normal
> >> -priority=100,in_port=1,udp,action=ct(commit),controller
> >> +priority=100,in_port=1,udp,action=ct(force,commit),controller
> >>   priority=100,in_port=2,ct_state=-trk,udp,action=ct(table=0)
> >>   priority=100,in_port=2,ct_state=+trk+est,udp,action=ct(force,commit,table=1)
> >>   table=1,in_port=2,ct_state=+trk,udp,action=controller
> >> @@ -826,6 +826,16 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.2,"], [],
> >>   udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2)
> >>   ])
> >> +ovs-appctl vlog/set dpif:dbg
> >> +
> >> +dnl OK, now send another packet from port 1 and see that it switches again
> >> +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
> >> +AT_CHECK([ovs-appctl revalidator/purge], [0])
> >> +
> >> +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.1,"], [], [dnl
> >> +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
> >> +])
> >> +
> >>   OVS_TRAFFIC_VSWITCHD_STOP
> >>   AT_CLEANUP
> >>
> > git send-email --from="joe@ovn.org" didn't work I guess.
> >
> > Joe Stringer is the author.  Sorry for the mistake.
>
> To send someone else's patch, ensure that the author is correct in the
> commit itself.  Sometimes this takes "git commit --amend
> --author='...'".  Then send it the usual way with "git send-email",
> which will leave you as the envelope sender of the email and put the
> real author as the first line "From: ...author...".
>
Right, next time no short cut attempt...

Thanks Ben.
Darrell Ball July 17, 2017, 6:23 p.m. UTC | #6 
On 7/17/17, 10:44 AM, "ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com" <ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com> wrote:

    From: Greg Rose <gvrose8192@gmail.com>
    
    Add a new test for the kernel module datapath to test if the
    conntrack force direction change and commit is working correctly.

Although the kind of problem that the associated code fix
https://patchwork.ozlabs.org/patch/789646/
addresses is specifically related to kernel conntrack, the test added covers both kernel and
userspace conntrack and runs on both.
You may want to update the above text accordingly.

Otherwise 
Acked-by: Darrell Ball <dlu998@gmail.com>


    This test was used to find and root cause BZ 18090854.
    
    Signed-off-by: Joe Stringer <joe@ovn.org>
    Signed-off-by: Greg Rose <gvrose8192@gmail.com>
    ---
     tests/system-traffic.at | 12 +++++++++++-
     1 file changed, 11 insertions(+), 1 deletion(-)
    
    diff --git a/tests/system-traffic.at b/tests/system-traffic.at
    index b2393f5..e83ad3a 100644
    --- a/tests/system-traffic.at
    +++ b/tests/system-traffic.at
    @@ -789,7 +789,7 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
     AT_DATA([flows.txt], [dnl
     priority=1,action=drop
     priority=10,arp,action=normal
    -priority=100,in_port=1,udp,action=ct(commit),controller
    +priority=100,in_port=1,udp,action=ct(force,commit),controller
     priority=100,in_port=2,ct_state=-trk,udp,action=ct(table=0)
     priority=100,in_port=2,ct_state=+trk+est,udp,action=ct(force,commit,table=1)
     table=1,in_port=2,ct_state=+trk,udp,action=controller
    @@ -826,6 +826,16 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.2,"], [],
     udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2)
     ])
     
    +ovs-appctl vlog/set dpif:dbg
    +
    +dnl OK, now send another packet from port 1 and see that it switches again
    +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
    +AT_CHECK([ovs-appctl revalidator/purge], [0])
    +
    +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.1,"], [], [dnl
    +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
    +])
    +
     OVS_TRAFFIC_VSWITCHD_STOP
     AT_CLEANUP
     
    -- 
    1.8.3.1
    
    _______________________________________________
    dev mailing list
    dev@openvswitch.org
    https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=OmRby7ktw0_CALnQ5BN7FdkzgJLCpKz0O35RfqlLF4c&s=EjvudY4kKjqWHrvab87oT8u-YWKYhqmjH82JBcAK8gE&e=
Gregory Rose July 17, 2017, 6:26 p.m. UTC | #7 
On 07/17/2017 11:23 AM, Darrell Ball wrote:
> 
> 
> On 7/17/17, 10:44 AM, "ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com" <ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com> wrote:
> 
>      From: Greg Rose <gvrose8192@gmail.com>
>      
>      Add a new test for the kernel module datapath to test if the
>      conntrack force direction change and commit is working correctly.
> 
> Although the kind of problem that the associated code fix
> https://patchwork.ozlabs.org/patch/789646/
> addresses is specifically related to kernel conntrack, the test added covers both kernel and
> userspace conntrack and runs on both.
> You may want to update the above text accordingly.

I did not know that.

I can send a V2 or Joe can fix before pushing.  Either way is fine by me.

Thanks,

- Greg

> 
> Otherwise
> Acked-by: Darrell Ball <dlu998@gmail.com>
> 
> 
>      This test was used to find and root cause BZ 18090854.
>      
>      Signed-off-by: Joe Stringer <joe@ovn.org>
>      Signed-off-by: Greg Rose <gvrose8192@gmail.com>
>      ---
>       tests/system-traffic.at | 12 +++++++++++-
>       1 file changed, 11 insertions(+), 1 deletion(-)
>      
>      diff --git a/tests/system-traffic.at b/tests/system-traffic.at
>      index b2393f5..e83ad3a 100644
>      --- a/tests/system-traffic.at
>      +++ b/tests/system-traffic.at
>      @@ -789,7 +789,7 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
>       AT_DATA([flows.txt], [dnl
>       priority=1,action=drop
>       priority=10,arp,action=normal
>      -priority=100,in_port=1,udp,action=ct(commit),controller
>      +priority=100,in_port=1,udp,action=ct(force,commit),controller
>       priority=100,in_port=2,ct_state=-trk,udp,action=ct(table=0)
>       priority=100,in_port=2,ct_state=+trk+est,udp,action=ct(force,commit,table=1)
>       table=1,in_port=2,ct_state=+trk,udp,action=controller
>      @@ -826,6 +826,16 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.2,"], [],
>       udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2)
>       ])
>       
>      +ovs-appctl vlog/set dpif:dbg
>      +
>      +dnl OK, now send another packet from port 1 and see that it switches again
>      +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
>      +AT_CHECK([ovs-appctl revalidator/purge], [0])
>      +
>      +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.1,"], [], [dnl
>      +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
>      +])
>      +
>       OVS_TRAFFIC_VSWITCHD_STOP
>       AT_CLEANUP
>       
>      --
>      1.8.3.1
>      
>      _______________________________________________
>      dev mailing list
>      dev@openvswitch.org
>      https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=OmRby7ktw0_CALnQ5BN7FdkzgJLCpKz0O35RfqlLF4c&s=EjvudY4kKjqWHrvab87oT8u-YWKYhqmjH82JBcAK8gE&e=
>      
>
Darrell Ball July 17, 2017, 7:27 p.m. UTC | #8 
On 7/17/17, 11:26 AM, "Greg Rose" <gvrose8192@gmail.com> wrote:

    On 07/17/2017 11:23 AM, Darrell Ball wrote:
    > 
    > 
    > On 7/17/17, 10:44 AM, "ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com" <ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com> wrote:
    > 
    >      From: Greg Rose <gvrose8192@gmail.com>
    >      
    >      Add a new test for the kernel module datapath to test if the
    >      conntrack force direction change and commit is working correctly.
    > 
    > Although the kind of problem that the associated code fix
    > https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_789646_&d=DwICaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=n3KZl3lnE-vdWgXO9q3zBE7yv11YOv51N2FWOarYEbk&s=0eYyRkq_xYLqcz617OlfxQ_vR4MDvowvLfPUwLf0C5U&e= 
    > addresses is specifically related to kernel conntrack, the test added covers both kernel and
    > userspace conntrack and runs on both.
    > You may want to update the above text accordingly.
    
    I did not know that.
    
    I can send a V2 or Joe can fix before pushing.  Either way is fine by me.
    
    Thanks,
    
    - Greg

same here

The test command for the userspace side is:

dball@ubuntu:~/openvswitch/ovs$ sudo make check-system-userspace TESTSUITEFLAGS='19' -C _gcc
[sudo] password for dball: 
make: Entering directory `/home/dball/openvswitch/ovs/_gcc'
make  all-recursive
make[1]: Entering directory `/home/dball/openvswitch/ovs/_gcc'
Making all in datapath
make[2]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
Making all in linux
make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath/linux'
make -C /lib/modules/3.19.0-25-generic/build M=/home/dball/openvswitch/ovs/_gcc/datapath/linux modules
make[4]: Entering directory `/usr/src/linux-headers-3.19.0-25-generic'
  Building modules, stage 2.
  MODPOST 6 modules
make[4]: Leaving directory `/usr/src/linux-headers-3.19.0-25-generic'
make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath/linux'
make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
make[2]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
make[2]: Entering directory `/home/dball/openvswitch/ovs/_gcc'
make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
make[3]: `distfiles' is up to date.
make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
make[2]: Leaving directory `/home/dball/openvswitch/ovs/_gcc'
make[1]: Leaving directory `/home/dball/openvswitch/ovs/_gcc'
set /bin/bash '../tests/system-userspace-testsuite' -C tests  AUTOTEST_PATH='utilities:vswitchd:ovsdb:vtep:tests::ovn/controller-vtep:ovn/northd:ovn/utilities:ovn/controller' 19 -j1; \
	"$@" || (test X'' = Xyes && "$@" --recheck)
## ------------------------------ ##
## openvswitch 2.7.90 test suite. ##
## ------------------------------ ##
 19: conntrack - force commit                        ok

## ------------- ##
## Test results. ##
## ------------- ##

1 test was successful.
make: Leaving directory `/home/dball/openvswitch/ovs/_gcc'


    
    > 
    > Otherwise
    > Acked-by: Darrell Ball <dlu998@gmail.com>
    > 
    > 
    >      This test was used to find and root cause BZ 18090854.
    >      
    >      Signed-off-by: Joe Stringer <joe@ovn.org>
    >      Signed-off-by: Greg Rose <gvrose8192@gmail.com>
    >      ---
    >       tests/system-traffic.at | 12 +++++++++++-
    >       1 file changed, 11 insertions(+), 1 deletion(-)
    >      
    >      diff --git a/tests/system-traffic.at b/tests/system-traffic.at
    >      index b2393f5..e83ad3a 100644
    >      --- a/tests/system-traffic.at
    >      +++ b/tests/system-traffic.at
    >      @@ -789,7 +789,7 @@ ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
    >       AT_DATA([flows.txt], [dnl
    >       priority=1,action=drop
    >       priority=10,arp,action=normal
    >      -priority=100,in_port=1,udp,action=ct(commit),controller
    >      +priority=100,in_port=1,udp,action=ct(force,commit),controller
    >       priority=100,in_port=2,ct_state=-trk,udp,action=ct(table=0)
    >       priority=100,in_port=2,ct_state=+trk+est,udp,action=ct(force,commit,table=1)
    >       table=1,in_port=2,ct_state=+trk,udp,action=controller
    >      @@ -826,6 +826,16 @@ AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.2,"], [],
    >       udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2)
    >       ])
    >       
    >      +ovs-appctl vlog/set dpif:dbg
    >      +
    >      +dnl OK, now send another packet from port 1 and see that it switches again
    >      +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
    >      +AT_CHECK([ovs-appctl revalidator/purge], [0])
    >      +
    >      +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.1,"], [], [dnl
    >      +udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
    >      +])
    >      +
    >       OVS_TRAFFIC_VSWITCHD_STOP
    >       AT_CLEANUP
    >       
    >      --
    >      1.8.3.1
    >      
    >      _______________________________________________
    >      dev mailing list
    >      dev@openvswitch.org
    >      https://urldefense.proofpoint.com/v2/url?u=https-3A__mail.openvswitch.org_mailman_listinfo_ovs-2Ddev&d=DwICAg&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=OmRby7ktw0_CALnQ5BN7FdkzgJLCpKz0O35RfqlLF4c&s=EjvudY4kKjqWHrvab87oT8u-YWKYhqmjH82JBcAK8gE&e=
    >      
    >
Joe Stringer July 17, 2017, 7:35 p.m. UTC | #9 
On 17 July 2017 at 12:27, Darrell Ball <dball@vmware.com> wrote:
>
>
> On 7/17/17, 11:26 AM, "Greg Rose" <gvrose8192@gmail.com> wrote:
>
>     On 07/17/2017 11:23 AM, Darrell Ball wrote:
>     >
>     >
>     > On 7/17/17, 10:44 AM, "ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com" <ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com> wrote:
>     >
>     >      From: Greg Rose <gvrose8192@gmail.com>
>     >
>     >      Add a new test for the kernel module datapath to test if the
>     >      conntrack force direction change and commit is working correctly.
>     >
>     > Although the kind of problem that the associated code fix
>     > https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_789646_&d=DwICaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=n3KZl3lnE-vdWgXO9q3zBE7yv11YOv51N2FWOarYEbk&s=0eYyRkq_xYLqcz617OlfxQ_vR4MDvowvLfPUwLf0C5U&e=
>     > addresses is specifically related to kernel conntrack, the test added covers both kernel and
>     > userspace conntrack and runs on both.
>     > You may want to update the above text accordingly.
>
>     I did not know that.
>
>     I can send a V2 or Joe can fix before pushing.  Either way is fine by me.
>
>     Thanks,
>
>     - Greg
>
> same here
>
> The test command for the userspace side is:
>
> dball@ubuntu:~/openvswitch/ovs$ sudo make check-system-userspace TESTSUITEFLAGS='19' -C _gcc
> [sudo] password for dball:
> make: Entering directory `/home/dball/openvswitch/ovs/_gcc'
> make  all-recursive
> make[1]: Entering directory `/home/dball/openvswitch/ovs/_gcc'
> Making all in datapath
> make[2]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> Making all in linux
> make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath/linux'
> make -C /lib/modules/3.19.0-25-generic/build M=/home/dball/openvswitch/ovs/_gcc/datapath/linux modules
> make[4]: Entering directory `/usr/src/linux-headers-3.19.0-25-generic'
>   Building modules, stage 2.
>   MODPOST 6 modules
> make[4]: Leaving directory `/usr/src/linux-headers-3.19.0-25-generic'
> make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath/linux'
> make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> make[2]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> make[2]: Entering directory `/home/dball/openvswitch/ovs/_gcc'
> make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> make[3]: `distfiles' is up to date.
> make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> make[2]: Leaving directory `/home/dball/openvswitch/ovs/_gcc'
> make[1]: Leaving directory `/home/dball/openvswitch/ovs/_gcc'
> set /bin/bash '../tests/system-userspace-testsuite' -C tests  AUTOTEST_PATH='utilities:vswitchd:ovsdb:vtep:tests::ovn/controller-vtep:ovn/northd:ovn/utilities:ovn/controller' 19 -j1; \
>         "$@" || (test X'' = Xyes && "$@" --recheck)
> ## ------------------------------ ##
> ## openvswitch 2.7.90 test suite. ##
> ## ------------------------------ ##
>  19: conntrack - force commit                        ok
>
> ## ------------- ##
> ## Test results. ##
> ## ------------- ##
>
> 1 test was successful.
> make: Leaving directory `/home/dball/openvswitch/ovs/_gcc'

Thanks for trying it out. I'll hold off on applying this until we get
the kernel side in.
Gregory Rose July 17, 2017, 7:50 p.m. UTC | #10 
On 07/17/2017 12:35 PM, Joe Stringer wrote:
> On 17 July 2017 at 12:27, Darrell Ball <dball@vmware.com> wrote:
> >
> >
> > On 7/17/17, 11:26 AM, "Greg Rose" <gvrose8192@gmail.com> wrote:
> >
> >      On 07/17/2017 11:23 AM, Darrell Ball wrote:
> >      >
> >      >
> >      > On 7/17/17, 10:44 AM, "ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com" <ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com> wrote:
> >      >
> >      >      From: Greg Rose <gvrose8192@gmail.com>
> >      >
> >      >      Add a new test for the kernel module datapath to test if the
> >      >      conntrack force direction change and commit is working correctly.
> >      >
> >      > Although the kind of problem that the associated code fix
> >      > https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_789646_&d=DwICaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=n3KZl3lnE-vdWgXO9q3zBE7yv11YOv51N2FWOarYEbk&s=0eYyRkq_xYLqcz617OlfxQ_vR4MDvowvLfPUwLf0C5U&e=
> >      > addresses is specifically related to kernel conntrack, the test added covers both kernel and
> >      > userspace conntrack and runs on both.
> >      > You may want to update the above text accordingly.
> >
> >      I did not know that.
> >
> >      I can send a V2 or Joe can fix before pushing.  Either way is fine by me.
> >
> >      Thanks,
> >
> >      - Greg
> >
> > same here
> >
> > The test command for the userspace side is:
> >
> > dball@ubuntu:~/openvswitch/ovs$ sudo make check-system-userspace TESTSUITEFLAGS='19' -C _gcc
> > [sudo] password for dball:
> > make: Entering directory `/home/dball/openvswitch/ovs/_gcc'
> > make  all-recursive
> > make[1]: Entering directory `/home/dball/openvswitch/ovs/_gcc'
> > Making all in datapath
> > make[2]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > Making all in linux
> > make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath/linux'
> > make -C /lib/modules/3.19.0-25-generic/build M=/home/dball/openvswitch/ovs/_gcc/datapath/linux modules
> > make[4]: Entering directory `/usr/src/linux-headers-3.19.0-25-generic'
> >    Building modules, stage 2.
> >    MODPOST 6 modules
> > make[4]: Leaving directory `/usr/src/linux-headers-3.19.0-25-generic'
> > make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath/linux'
> > make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > make[2]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > make[2]: Entering directory `/home/dball/openvswitch/ovs/_gcc'
> > make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > make[3]: `distfiles' is up to date.
> > make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > make[2]: Leaving directory `/home/dball/openvswitch/ovs/_gcc'
> > make[1]: Leaving directory `/home/dball/openvswitch/ovs/_gcc'
> > set /bin/bash '../tests/system-userspace-testsuite' -C tests  AUTOTEST_PATH='utilities:vswitchd:ovsdb:vtep:tests::ovn/controller-vtep:ovn/northd:ovn/utilities:ovn/controller' 19 -j1; \
> >          "$@" || (test X'' = Xyes && "$@" --recheck)
> > ## ------------------------------ ##
> > ## openvswitch 2.7.90 test suite. ##
> > ## ------------------------------ ##
> >   19: conntrack - force commit                        ok
> >
> > ## ------------- ##
> > ## Test results. ##
> > ## ------------- ##
> >
> > 1 test was successful.
> > make: Leaving directory `/home/dball/openvswitch/ovs/_gcc'
>
> Thanks for trying it out. I'll hold off on applying this until we get
> the kernel side in.
>
Dave Miller applied the upstream kernel patch.

See attachment.

Just FYI as I'm not sure who follows netdev.

Thanks,

- Greg
Gregory Rose July 18, 2017, 3:08 p.m. UTC | #11 
On 07/17/2017 12:35 PM, Joe Stringer wrote:
> On 17 July 2017 at 12:27, Darrell Ball <dball@vmware.com> wrote:
> >
> >
> > On 7/17/17, 11:26 AM, "Greg Rose" <gvrose8192@gmail.com> wrote:
> >
> >      On 07/17/2017 11:23 AM, Darrell Ball wrote:
> >      >
> >      >
> >      > On 7/17/17, 10:44 AM, "ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com" <ovs-dev-bounces@openvswitch.org on behalf of gvrose8192@gmail.com> wrote:
> >      >
> >      >      From: Greg Rose <gvrose8192@gmail.com>
> >      >
> >      >      Add a new test for the kernel module datapath to test if the
> >      >      conntrack force direction change and commit is working correctly.
> >      >
> >      > Although the kind of problem that the associated code fix
> >      > https://urldefense.proofpoint.com/v2/url?u=https-3A__patchwork.ozlabs.org_patch_789646_&d=DwICaQ&c=uilaK90D4TOVoH58JNXRgQ&r=BVhFA09CGX7JQ5Ih-uZnsw&m=n3KZl3lnE-vdWgXO9q3zBE7yv11YOv51N2FWOarYEbk&s=0eYyRkq_xYLqcz617OlfxQ_vR4MDvowvLfPUwLf0C5U&e=
> >      > addresses is specifically related to kernel conntrack, the test added covers both kernel and
> >      > userspace conntrack and runs on both.
> >      > You may want to update the above text accordingly.
> >
> >      I did not know that.
> >
> >      I can send a V2 or Joe can fix before pushing.  Either way is fine by me.
> >
> >      Thanks,
> >
> >      - Greg
> >
> > same here
> >
> > The test command for the userspace side is:
> >
> > dball@ubuntu:~/openvswitch/ovs$ sudo make check-system-userspace TESTSUITEFLAGS='19' -C _gcc
> > [sudo] password for dball:
> > make: Entering directory `/home/dball/openvswitch/ovs/_gcc'
> > make  all-recursive
> > make[1]: Entering directory `/home/dball/openvswitch/ovs/_gcc'
> > Making all in datapath
> > make[2]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > Making all in linux
> > make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath/linux'
> > make -C /lib/modules/3.19.0-25-generic/build M=/home/dball/openvswitch/ovs/_gcc/datapath/linux modules
> > make[4]: Entering directory `/usr/src/linux-headers-3.19.0-25-generic'
> >    Building modules, stage 2.
> >    MODPOST 6 modules
> > make[4]: Leaving directory `/usr/src/linux-headers-3.19.0-25-generic'
> > make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath/linux'
> > make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > make[2]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > make[2]: Entering directory `/home/dball/openvswitch/ovs/_gcc'
> > make[3]: Entering directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > make[3]: `distfiles' is up to date.
> > make[3]: Leaving directory `/home/dball/openvswitch/ovs/_gcc/datapath'
> > make[2]: Leaving directory `/home/dball/openvswitch/ovs/_gcc'
> > make[1]: Leaving directory `/home/dball/openvswitch/ovs/_gcc'
> > set /bin/bash '../tests/system-userspace-testsuite' -C tests  AUTOTEST_PATH='utilities:vswitchd:ovsdb:vtep:tests::ovn/controller-vtep:ovn/northd:ovn/utilities:ovn/controller' 19 -j1; \
> >          "$@" || (test X'' = Xyes && "$@" --recheck)
> > ## ------------------------------ ##
> > ## openvswitch 2.7.90 test suite. ##
> > ## ------------------------------ ##
> >   19: conntrack - force commit                        ok
> >
> > ## ------------- ##
> > ## Test results. ##
> > ## ------------- ##
> >
> > 1 test was successful.
> > make: Leaving directory `/home/dball/openvswitch/ovs/_gcc'
>
> Thanks for trying it out. I'll hold off on applying this until we get
> the kernel side in.
>
I'll send a V2 shortly.

Thanks,

- Greg
diff mbox

Patch

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index b2393f5..e83ad3a 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -789,7 +789,7 @@  ADD_VETH(p1, at_ns1, br0, "10.1.1.2/24")
 AT_DATA([flows.txt], [dnl
 priority=1,action=drop
 priority=10,arp,action=normal
-priority=100,in_port=1,udp,action=ct(commit),controller
+priority=100,in_port=1,udp,action=ct(force,commit),controller
 priority=100,in_port=2,ct_state=-trk,udp,action=ct(table=0)
 priority=100,in_port=2,ct_state=+trk+est,udp,action=ct(force,commit,table=1)
 table=1,in_port=2,ct_state=+trk,udp,action=controller
@@ -826,6 +826,16 @@  AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.2,"], [],
 udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1),reply=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2)
 ])
 
+ovs-appctl vlog/set dpif:dbg
+
+dnl OK, now send another packet from port 1 and see that it switches again
+AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"])
+AT_CHECK([ovs-appctl revalidator/purge], [0])
+
+AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.1,"], [], [dnl
+udp,orig=(src=10.1.1.1,dst=10.1.1.2,sport=1,dport=2),reply=(src=10.1.1.2,dst=10.1.1.1,sport=2,dport=1)
+])
+
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP