diff mbox

[ovs-dev,RFC] Extremely crude conntrack resubmit test case

Message ID 1446775592-6973-1-git-send-email-rbryant@redhat.com
State Not Applicable
Headers show

Commit Message

Russell Bryant Nov. 6, 2015, 2:06 a.m. UTC 
This patch includes a really crude test case demonstrating the problem
I'm seeing with resubmitting to a table that does ct() multiple times.
It only seems to work on the first resubmit.

The test case is REALLY crude, but I think it shows the issue.  The test
case will always fail as written so I can look at the flows.  I run this
with:

  $ sudo make check-kmod TESTSUITEFLAGS="-k resubmit -v"

The interesting part is the flows output at the end.  Here's what I see:

>  cookie=0x0, duration=1.037s, table=0, n_packets=2, n_bytes=84, priority=150,arp actions=NORMAL
>  cookie=0x0, duration=1.037s, table=0, n_packets=3, n_bytes=266, priority=100,in_port=1 actions=drop
>  cookie=0x0, duration=1.037s, table=0, n_packets=2, n_bytes=168, priority=100,in_port=2 actions=drop
>  cookie=0x0, duration=1.036s, table=0, n_packets=1, n_bytes=98, priority=100,ip,in_port=LOCAL actions=load:0x1->NXM_NX_REG0[0..15],resubmit(,1),load:0x2->NXM_NX_REG0[0..15],resubmit(,1)
>  cookie=0x0, duration=1.036s, table=0, n_packets=1, n_bytes=78, priority=1 actions=drop
>  cookie=0x0, duration=1.035s, table=1, n_packets=1, n_bytes=98, priority=100,ip actions=ct(table=2,zone=NXM_NX_REG0[0..15])
>  cookie=0x0, duration=1.035s, table=2, n_packets=1, n_bytes=98, priority=100,ip,reg0=0x1 actions=output:1
>  cookie=0x0, duration=1.034s, table=2, n_packets=0, n_bytes=0, priority=100,ip,reg0=0x2 actions=output:2

There is a flow in table 0 that resubmits to table 1 twice.  It should
result in the packet being output to ports 1 and 2 in table 2.  It only
hits tables 1 and 2 once, which is the first resubmit from the flow in
table 0.

I'm running this in a new VM and the following message seen in my
OpenStack test environment does *not* show up at all in this
environment: "openvswitch: ovs-system: deferred action limit reached,
drop recirc action".
---
 tests/system-traffic.at | 40 ++++++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

Comments

Ben Pfaff Nov. 23, 2015, 6:27 p.m. UTC | #1 
On Thu, Nov 05, 2015 at 09:06:32PM -0500, Russell Bryant wrote:
> This patch includes a really crude test case demonstrating the problem
> I'm seeing with resubmitting to a table that does ct() multiple times.
> It only seems to work on the first resubmit.
> 
> The test case is REALLY crude, but I think it shows the issue.  The test
> case will always fail as written so I can look at the flows.  I run this
> with:
> 
>   $ sudo make check-kmod TESTSUITEFLAGS="-k resubmit -v"

Trying to catch up on email.  I think this was fixed by:

commit e37b8437e915a02a88116e5ea7af1a7e716bd597
Author: Joe Stringer <joestringer@nicira.com>
Date:   Fri Nov 6 16:16:47 2015 -0800

    ofproto-dpif-xlate: Don't stop processing after ct.
    
    If conntrack recirculates, it should not stop processing the current
    pipeline. The cloned packet will begin processing in the table specified
    with the current metadata and action set; The current copy of the packet
    will continue processing, including to return back to prior resubmit()
    calls.
    
    Reported-by: Russell Bryant <rbryant@redhat.com>
    Signed-off-by: Joe Stringer <joestringer@nicira.com>
    Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>

Let me know if I'm wrong and I'll investigate.
Russell Bryant Nov. 23, 2015, 6:36 p.m. UTC | #2 
On 11/23/2015 01:27 PM, Ben Pfaff wrote:
> On Thu, Nov 05, 2015 at 09:06:32PM -0500, Russell Bryant wrote:
>> This patch includes a really crude test case demonstrating the problem
>> I'm seeing with resubmitting to a table that does ct() multiple times.
>> It only seems to work on the first resubmit.
>>
>> The test case is REALLY crude, but I think it shows the issue.  The test
>> case will always fail as written so I can look at the flows.  I run this
>> with:
>>
>>   $ sudo make check-kmod TESTSUITEFLAGS="-k resubmit -v"
> 
> Trying to catch up on email.  I think this was fixed by:
> 
> commit e37b8437e915a02a88116e5ea7af1a7e716bd597
> Author: Joe Stringer <joestringer@nicira.com>
> Date:   Fri Nov 6 16:16:47 2015 -0800
> 
>     ofproto-dpif-xlate: Don't stop processing after ct.
>     
>     If conntrack recirculates, it should not stop processing the current
>     pipeline. The cloned packet will begin processing in the table specified
>     with the current metadata and action set; The current copy of the packet
>     will continue processing, including to return back to prior resubmit()
>     calls.
>     
>     Reported-by: Russell Bryant <rbryant@redhat.com>
>     Signed-off-by: Joe Stringer <joestringer@nicira.com>
>     Acked-by: Jarno Rajahalme <jrajahalme@nicira.com>
> 
> Let me know if I'm wrong and I'll investigate.

Correct, and a cleaned up version of the test case went in after it:

https://github.com/openvswitch/ovs/commit/c4e34c6114bcf4cf9248fe910ae8f202b6293f40
diff mbox

Patch

diff --git a/tests/system-traffic.at b/tests/system-traffic.at
index 3b2de83..80b96c0 100644
--- a/tests/system-traffic.at
+++ b/tests/system-traffic.at
@@ -1266,3 +1266,43 @@  NS_CHECK_EXEC([at_ns0], [ping -s 3200 -q -c 3 -i 0.3 -w 2 10.1.1.100 | FORMAT_PI
 
 OVS_TRAFFIC_VSWITCHD_STOP
 AT_CLEANUP
+
+AT_SETUP([conntrack - resubmit to ct multiple times])
+CHECK_CONNTRACK()
+
+OVS_TRAFFIC_VSWITCHD_START(
+   [set-fail-mode br0 secure -- ])
+
+ADD_NAMESPACES(at_ns0, at_ns1)
+
+AT_CHECK([ip addr add dev br0 "10.1.1.1/24"])
+AT_CHECK([ip link set dev br0 up])
+on_exit 'ip addr del dev br0 "10.1.1.1/24"'
+ADD_VETH(p0, at_ns0, br0, "10.1.1.2/24")
+ADD_VETH(p1, at_ns1, br0, "10.1.1.3/24")
+
+AT_DATA([flows.txt], [dnl
+table=0,priority=150,arp,action=normal
+table=0,priority=100,in_port=1,action=drop
+table=0,priority=100,in_port=2,action=drop
+table=0,priority=100,ip,in_port=LOCAL,action=load:1->NXM_NX_REG0[[0..15]],resubmit(,1),load:2->NXM_NX_REG0[[0..15]],resubmit(,1)
+table=0,priority=1,action=drop
+
+table=1,priority=100,ip,action=ct(table=2,zone=NXM_NX_REG0[[0..15]])
+
+table=2,priority=100,ip,reg0=1,action=output:1
+table=2,priority=100,ip,reg0=2,action=output:2
+])
+
+AT_CHECK([ovs-ofctl add-flows br0 flows.txt])
+
+AT_CHECK([ping -q -c 1 -w 1 -I 10.1.1.1 10.1.1.2 | FORMAT_PING], [0], [dnl
+1 packets transmitted, 0 received, 100% packet loss, time 0ms
+])
+
+AT_CHECK([ovs-ofctl -O OpenFlow13 show br0 ; ovs-ofctl -O OpenFlow13 dump-flows br0 | cut -f3- -d','], [0], [dnl
+always fail and show me the output
+])
+
+OVS_TRAFFIC_VSWITCHD_STOP
+AT_CLEANUP