Hmm, it was the first time I'd used the internal system tester so I must have 
invoked it improperly.  I'll fix these issues and send out a v2.

Thanks,

--Justin


> On Aug 8, 2017, at 6:02 PM, Darrell Ball <db...@vmware.com> wrote:
> 
> I just did a second round
> 
> One correction for first round 73 should have been 79
> 
> First round:
> Kernel: 53, 58, 69, 70
> Userspace: 57, 58, 63, 69, 70, 71, 72, 79
> 
> Second Round:
> Kernel: 53, 58, 69, 70
> Userspace: 58, 63, 69, 70, 71, 72, 79
> 
> Without patch, no failures on both.
> 
> 
> 
> -----Original Message-----
> From: Darrell Ball <db...@vmware.com>
> Date: Tuesday, August 8, 2017 at 5:51 PM
> To: Justin Pettit <jpettit....@gmail.com>
> Cc: Justin Pettit <jpet...@ovn.org>, "d...@openvswitch.org" 
> <d...@openvswitch.org>
> Subject: Re: [ovs-dev] [PATCH] ofproto-dpif: Mark packets as "untracked" 
> after call to ct().
> 
>    I just rebased about 15 minutes ago:
> 
>    Kernel: 53, 58, 69, 70
>    Userspace: 57, 58, 63, 69, 70, 71, 72, 73
> 
>    -----Original Message-----
>    From: Justin Pettit <jpettit....@gmail.com>
>    Date: Tuesday, August 8, 2017 at 5:48 PM
>    To: Darrell Ball <db...@vmware.com>
>    Cc: Justin Pettit <jpet...@ovn.org>, "d...@openvswitch.org" 
> <d...@openvswitch.org>
>    Subject: Re: [ovs-dev] [PATCH] ofproto-dpif: Mark packets as "untracked" 
> after call to ct().
> 
>        Can you share them? I don't see them on my system or our internal 
> system tester that runs the kernel and user space tests.  
> 
>        --Justin
> 
> 
>> On Aug 8, 2017, at 5:46 PM, Darrell Ball <db...@vmware.com> wrote:
>> 
>> With this patch applied:
>> 
>> I get 4 failures in kernel system tests and 8 failures in userspace system 
>> tests.
>> 
>> 
>> 
>> 
>> -----Original Message-----
>> From: <ovs-dev-boun...@openvswitch.org> on behalf of Justin Pettit 
>> <jpet...@ovn.org>
>> Date: Tuesday, August 8, 2017 at 4:12 PM
>> To: "d...@openvswitch.org" <d...@openvswitch.org>
>> Subject: [ovs-dev] [PATCH] ofproto-dpif: Mark packets as "untracked" after   
>>  call to ct().
>> 
>>   Packet and Connection state is only available to the processing path
>>   that follows the "recirc_table" argument of the ct() action.  The
>>   previous behavior made these states available until the end of the
>>   pipeline.  This commit changes the behavior so that the Packet and
>>   Connection state are cleared for the current processing path whenever
>>   ct() is called (in addition to reaching the end of the pipeline.)
>> 
>>   A future commit will remove the behavior that a "send to controller"
>>   action causes all packets for that flow to be handled via the slow-path.
>>   The current behavior of connection tracking state makes that difficult
>>   due to datapath actions containing multiple OpenFlow rules that may
>>   contain different connection tracking states.  This change will make
>>   that future commit possible.
>> 
>>   Signed-off-by: Justin Pettit <jpet...@ovn.org>
>>   ---
>>    lib/ofp-actions.c            | 27 +++++++++++++--------------
>>    ofproto/ofproto-dpif-xlate.c | 24 +++++++-----------------
>>    tests/ofproto-dpif.at        | 10 +++++-----
>>    utilities/ovs-ofctl.8.in     | 10 ++++++----
>>    4 files changed, 31 insertions(+), 40 deletions(-)
>> 
>>   diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c
>>   index bfc8a805ffd5..71eb70c3c239 100644
>>   --- a/lib/ofp-actions.c
>>   +++ b/lib/ofp-actions.c
>>   @@ -5858,20 +5858,19 @@ format_DEBUG_RECIRC(const struct ofpact_null *a 
>> OVS_UNUSED,
>>     *
>>     *   - Packet State:
>>     *
>>   - *      Untracked packets have not yet passed through the connection 
>> tracker,
>>   - *      and the connection state for such packets is unknown. In most 
>> cases,
>>   - *      packets entering the OpenFlow pipeline will initially be in the
>>   - *      untracked state. Untracked packets may become tracked by executing
>>   - *      NXAST_CT with a "recirc_table" specified. This makes various 
>> aspects
>>   - *      about the connection available, in particular the connection 
>> state.
>>   - *
>>   - *      Tracked packets have previously passed through the connection 
>> tracker.
>>   - *      These packets will remain tracked through until the end of the 
>> OpenFlow
>>   - *      pipeline. Tracked packets which have NXAST_CT executed with a
>>   - *      "recirc_table" specified will return to the tracked state.
>>   - *
>>   - *      The packet state is only significant for the duration of packet
>>   - *      processing within the OpenFlow pipeline.
>>   + *      Untracked packets have an unknown connection state.  In most
>>   + *      cases, packets entering the OpenFlow pipeline will initially be
>>   + *      in the untracked state. Untracked packets may become tracked by
>>   + *      executing NXAST_CT with a "recirc_table" specified. This makes
>>   + *      various aspects about the connection available, in particular
>>   + *      the connection state.
>>   + *
>>   + *      An NXAST_CT action always puts the packet into an untracked
>>   + *      state for the current processing path.  If "recirc_table" is
>>   + *      set, execution is forked and the packet passes through the
>>   + *      connection tracker.  The specified table's processing path is
>>   + *      able to match on Connection state until the end of the OpenFlow
>>   + *      pipeline or NXAST_CT is called again.
>>     *
>>     *   - Connection State:
>>     *
>>   diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c
>>   index 973e760547fa..333b1b595e81 100644
>>   --- a/ofproto/ofproto-dpif-xlate.c
>>   +++ b/ofproto/ofproto-dpif-xlate.c
>>   @@ -5721,10 +5721,6 @@ put_ct_nat(struct xlate_ctx *ctx)
>>    static void
>>    compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack 
>> *ofc)
>>    {
>>   -    ovs_u128 old_ct_label = ctx->xin->flow.ct_label;
>>   -    ovs_u128 old_ct_label_mask = ctx->wc->masks.ct_label;
>>   -    uint32_t old_ct_mark = ctx->xin->flow.ct_mark;
>>   -    uint32_t old_ct_mark_mask = ctx->wc->masks.ct_mark;
>>        size_t ct_offset;
>>        uint16_t zone;
>> 
>>   @@ -5761,23 +5757,17 @@ compose_conntrack_action(struct xlate_ctx *ctx, 
>> struct ofpact_conntrack *ofc)
>>        ctx->ct_nat_action = NULL;
>>        nl_msg_end_nested(ctx->odp_actions, ct_offset);
>> 
>>   -    /* Restore the original ct fields in the key. These should only be 
>> exposed
>>   -     * after recirculation to another table. */
>>   -    ctx->xin->flow.ct_mark = old_ct_mark;
>>   -    ctx->wc->masks.ct_mark = old_ct_mark_mask;
>>   -    ctx->xin->flow.ct_label = old_ct_label;
>>   -    ctx->wc->masks.ct_label = old_ct_label_mask;
>>   -
>>   -    if (ofc->recirc_table == NX_CT_RECIRC_NONE) {
>>   -        /* If we do not recirculate as part of this action, hide the 
>> results of
>>   -         * connection tracking from subsequent recirculations. */
>>   -        ctx->conntracked = false;
>>   -    } else {
>>   +    if (ofc->recirc_table != NX_CT_RECIRC_NONE) {
>>            /* Use ct_* fields from datapath during recirculation upcall. */
>>            ctx->conntracked = true;
>>            compose_recirculate_and_fork(ctx, ofc->recirc_table);
>>   -        ctx->conntracked = false;
>>        }
>>   +
>>   +    /* The ct_* fields are only available in the scope of the
>>   +     * 'recirc_table' call chain. */
>>   +    flow_clear_conntrack(&ctx->xin->flow);
>>   +    flow_clear_conntrack(&ctx->wc->masks);
>>   +    ctx->conntracked = false;
>>    }
>> 
>>    static void
>>   diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at
>>   index 284a65ec6524..28a7e827cac2 100644
>>   --- a/tests/ofproto-dpif.at
>>   +++ b/tests/ofproto-dpif.at
>>   @@ -8949,7 +8949,7 @@ OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
>> 
>>    dnl Check this output. We only see the latter two packets, not the first.
>>    AT_CHECK([cat ofctl_monitor.log], [0], [dnl
>>   -NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
>> ct_state=new|trk,ct_zone=1,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1
>>  (via action) data_len=42 (unbuffered)
>>   +NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
>> reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) 
>> data_len=42 (unbuffered)
>>    
>> udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=1,tp_dst=2
>>  udp_csum:e9d6
>>    dnl
>>    NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
>> ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2000000000000000000000000,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2
>>  (via action) data_len=42 (unbuffered)
>>   @@ -8970,7 +8970,7 @@ OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
>> 
>>    dnl Check this output. We should see both packets
>>    AT_CHECK([cat ofctl_monitor.log], [0], [dnl
>>   -NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
>> ct_state=new|trk,ct_zone=1,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1
>>  (via action) data_len=42 (unbuffered)
>>   +NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
>> reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x1,reg4=0x1,in_port=1 (via action) 
>> data_len=42 (unbuffered)
>>    
>> udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=3,tp_dst=2
>>  udp_csum:e9d4
>>    dnl
>>    NXT_PACKET_IN (xid=0x0): table_id=6 cookie=0x0 total_len=42 
>> ct_state=est|rpl|trk,ct_zone=1,ct_mark=0x1,ct_label=0x4d2000000000000000000000000,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=2,reg0=0x1,reg1=0x4d2,reg2=0x1,reg3=0x2,reg4=0x1,in_port=2
>>  (via action) data_len=42 (unbuffered)
>>   @@ -9025,7 +9025,7 @@ AT_CHECK([cat ofctl_monitor.log], [0], [dnl
>>    NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=42 in_port=1 (via action) 
>> data_len=42 (unbuffered)
>>    
>> udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=1,tp_dst=2
>>  udp_csum:e9d6
>>    dnl
>>   -NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 
>> ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=2
>>  (via action) data_len=42 (unbuffered)
>>   +NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 in_port=2 
>> (via action) data_len=42 (unbuffered)
>>    
>> udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=2,tp_dst=1
>>  udp_csum:e9d6
>>    ])
>> 
>>   @@ -9047,7 +9047,7 @@ AT_CHECK([cat ofctl_monitor.log], [0], [dnl
>>    NXT_PACKET_IN (xid=0x0): cookie=0x0 total_len=42 in_port=1 (via action) 
>> data_len=42 (unbuffered)
>>    
>> udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=3,tp_dst=4
>>  udp_csum:e9d2
>>    dnl
>>   -NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 
>> ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=3,ct_tp_dst=4,in_port=2
>>  (via action) data_len=42 (unbuffered)
>>   +NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 in_port=2 
>> (via action) data_len=42 (unbuffered)
>>    
>> udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10.1.1.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=4,tp_dst=3
>>  udp_csum:e9d2
>>    ])
>> 
>>   @@ -9362,7 +9362,7 @@ OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit])
>> 
>>    dnl Check this output. We only see the latter two packets, not the first.
>>    AT_CHECK([cat ofctl_monitor.log], [0], [dnl
>>   -NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 
>> ct_state=new|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=1
>>  (via action) data_len=42 (unbuffered)
>>   +NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 in_port=1 
>> (via action) data_len=42 (unbuffered)
>>    
>> udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10.1.1.1,nw_dst=10.1.1.2,nw_tos=0,nw_ecn=0,nw_ttl=64,tp_src=1,tp_dst=2
>>  udp_csum:e9d6
>>    dnl
>>    NXT_PACKET_IN (xid=0x0): table_id=1 cookie=0x0 total_len=42 
>> ct_state=est|rpl|trk,ct_nw_src=10.1.1.1,ct_nw_dst=10.1.1.2,ct_nw_proto=17,ct_tp_src=1,ct_tp_dst=2,in_port=2
>>  (via action) data_len=42 (unbuffered)
>>   diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in
>>   index f6bd90374a18..c65de97f5e2e 100644
>>   --- a/utilities/ovs-ofctl.8.in
>>   +++ b/utilities/ovs-ofctl.8.in
>>   @@ -1031,11 +1031,13 @@ Restores the queue to the value it was before any 
>> \fBset_queue\fR
>>    actions were applied.
>>    .
>>    .IP \fBct\fR
>>   -.IQ \fBct\fB(\fR[\fIargument\fR][\fB,\fIargument\fR...]\fB)
>>   +.IQ \fBct(\fR[\fIargument\fR][\fB,\fIargument\fR...]\fB)
>>    Send the packet through the connection tracker.  Refer to the 
>> \fBct_state\fR
>>   -documentation above for possible packet and connection states. The 
>> following
>>   -arguments are supported:
>>   -
>>   +documentation above for possible packet and connection states. A \fBct\fR
>>   +action always sets the packet to an untracked state and clears out the
>>   +\fBct_state\fR fields for the current processing path.  Those fields are
>>   +only available for the processing path pointed to by the \fBtable\fR
>>   +argument.  The following arguments are supported:
>>    .RS
>>    .IP \fBcommit\fR
>>    .RS
>>   -- 
>>   2.7.4
>> 
>>   _______________________________________________
>>   dev mailing list
>>   d...@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=0zUUtGTqA0rQPJshxslL-2FLBopCJJ8Fp6o61txtq4U&s=jTQyR94DiGGkyGn7myT8AHsq2kgY_XWC-w4nvIDGdTk&e=
>>  
>> 
>> 
> 
> 
> 
> 

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to