I've reached out to 3 other users of OVS connection tracking for feedback and will report back to this thread once I hear anything.
On Fri, Aug 11, 2017 at 2:58 AM, Justin Pettit <[email protected]> wrote: > I just wanted to specially call out this patch for testing. My hope is that > we can merge this into 2.8, but wanted to give people a heads up that they > may want to try it with their applications. The most visible change is that > a call to ct() will clear the ct_state for any actions that follow it. As > before, the ct_state will be accessible to flows rooted in the table > specified by the "table" argument to ct(), but now another ct() action will > clear the state for the current processing path (but the new ct_state will be > available if the "table" argument is given). > > My hope is that this won't affect people's pipelines, but it's worth checking > if it does. I would recommend that anyone that uses the conntrack action in > their applications give this patch a try (on top of branch-2.8). If there > are any problems, I can try to help you reconstruct your pipeline so it's not > affected. (The changes should be minimal, and there are a couple of examples > in the patch itself.) I've done some light testing with OVN, and it didn't > require any changes, and I hope it's the same for everyone else. > > The reason I'd like to get this change into 2.8 is that it's a requirement > for a performance optimization I'd like to get into 2.9. Currently, if a > controller action is specified, all traffic is slow-pathed to userspace. As > we start making heavier use of local controllers, this will have a serious > negative effect on forwarding performance and increase in CPU usage. I've > made a few other (more minor, corner-casey) changes to the OpenFlow > processing in 2.8, and I'd like to get all potentially breaking issues into a > single release. > > If you have an application that uses conntrack, please give it a try and let > me know if you run into any issues. > > Thanks! > > --Justin > > >> On Aug 10, 2017, at 11:38 PM, Justin Pettit <[email protected]> wrote: >> >> 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 <[email protected]> >> --- >> v1->v2: Fix system tests. >> --- >> lib/ofp-actions.c | 27 +++++++++++++-------------- >> ofproto/ofproto-dpif-xlate.c | 21 +++++++-------------- >> tests/ofproto-dpif.at | 10 +++++----- >> tests/system-traffic.at | 4 ++-- >> utilities/ovs-ofctl.8.in | 10 ++++++---- >> 5 files changed, 33 insertions(+), 39 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..9e1f837cb23e 100644 >> --- a/ofproto/ofproto-dpif-xlate.c >> +++ b/ofproto/ofproto-dpif-xlate.c >> @@ -5721,9 +5721,7 @@ 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; >> @@ -5735,7 +5733,7 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct >> ofpact_conntrack *ofc) >> /* Process nested actions first, to populate the key. */ >> ctx->ct_nat_action = NULL; >> ctx->wc->masks.ct_mark = 0; >> - ctx->wc->masks.ct_label.u64.hi = ctx->wc->masks.ct_label.u64.lo = 0; >> + ctx->wc->masks.ct_label = OVS_U128_ZERO; >> do_xlate_actions(ofc->actions, ofpact_ct_get_action_len(ofc), ctx); >> >> if (ofc->zone_src.field) { >> @@ -5761,23 +5759,18 @@ 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 { >> - /* Use ct_* fields from datapath during recirculation upcall. */ >> + if (ofc->recirc_table != NX_CT_RECIRC_NONE) { >> 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); >> + 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/tests/system-traffic.at b/tests/system-traffic.at >> index 798dd2cbd2c2..522eaa615834 100644 >> --- a/tests/system-traffic.at >> +++ b/tests/system-traffic.at >> @@ -2287,7 +2287,7 @@ dnl Ingress pipeline >> dnl - Allow all connections from LOCAL port (commit and proceed to egress) >> dnl - All other connections go through conntracker using the input port as >> dnl a connection tracking zone. >> -table=1,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,zone=OXM_OF_IN_PORT[[0..15]]),goto_table:2 >> +table=1,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,table=2,zone=OXM_OF_IN_PORT[[0..15]]) >> table=1,priority=100,ip,action=ct(table=2,zone=OXM_OF_IN_PORT[[0..15]]) >> table=1,priority=1,action=drop >> >> @@ -2295,7 +2295,7 @@ dnl Egress pipeline >> dnl - Allow all connections from LOCAL port (commit and skip to output) >> dnl - Allow other established connections to go through conntracker using >> dnl output port as a connection tracking zone. >> -table=2,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,zone=NXM_NX_REG0[[0..15]]),goto_table:4 >> +table=2,priority=150,in_port=LOCAL,ip,ct_state=+trk+new,action=ct(commit,table=4,zone=NXM_NX_REG0[[0..15]]) >> table=2,priority=100,ip,ct_state=+trk+est,action=ct(table=3,zone=NXM_NX_REG0[[0..15]]) >> table=2,priority=1,action=drop >> >> 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 >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev > -- Russell Bryant _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
