From patchwork Tue Mar 7 00:22:48 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jarno Rajahalme X-Patchwork-Id: 736015 X-Patchwork-Delegate: joestringer@nicira.com Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Received: from mail.linuxfoundation.org (mail.linuxfoundation.org [140.211.169.12]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ozlabs.org (Postfix) with ESMTPS id 3vcczy41cdz9sNd for ; Tue, 7 Mar 2017 11:32:14 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 2B1EBC64; Tue, 7 Mar 2017 00:23:36 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp2.linuxfoundation.org (smtp2.linux-foundation.org [172.17.192.36]) by mail.linuxfoundation.org (Postfix) with ESMTPS id EDB40C60 for ; Tue, 7 Mar 2017 00:23:32 +0000 (UTC) X-Greylist: domain auto-whitelisted by SQLgrey-1.7.6 Received: from relay2-d.mail.gandi.net (relay2-d.mail.gandi.net [217.70.183.194]) by smtp2.linuxfoundation.org (Postfix) with ESMTPS id 47D3C1DABD for ; Tue, 7 Mar 2017 00:23:29 +0000 (UTC) Received: from mfilter19-d.gandi.net (mfilter19-d.gandi.net [217.70.178.147]) by relay2-d.mail.gandi.net (Postfix) with ESMTP id E677EC5A68; Tue, 7 Mar 2017 01:23:27 +0100 (CET) X-Virus-Scanned: Debian amavisd-new at mfilter19-d.gandi.net Received: from relay2-d.mail.gandi.net ([IPv6:::ffff:217.70.183.194]) by mfilter19-d.gandi.net (mfilter19-d.gandi.net [::ffff:10.0.15.180]) (amavisd-new, port 10024) with ESMTP id RqqhKCM4Vq1f; Tue, 7 Mar 2017 01:23:25 +0100 (CET) X-Originating-IP: 208.91.1.34 Received: from sc9-mailhost3.vmware.com (unknown [208.91.1.34]) (Authenticated sender: jarno@ovn.org) by relay2-d.mail.gandi.net (Postfix) with ESMTPSA id AF11CC5A55; Tue, 7 Mar 2017 01:23:24 +0100 (CET) From: Jarno Rajahalme To: dev@openvswitch.org Date: Mon, 6 Mar 2017 16:22:48 -0800 Message-Id: <1488846170-23012-21-git-send-email-jarno@ovn.org> X-Mailer: git-send-email 2.1.4 In-Reply-To: <1488846170-23012-1-git-send-email-jarno@ovn.org> References: <1488846170-23012-1-git-send-email-jarno@ovn.org> X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00, RCVD_IN_DNSWL_LOW autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp2.linux-foundation.org Subject: [ovs-dev] [PATCH v3 20/22] conntrack: Force commit. X-BeenThere: ovs-dev@openvswitch.org X-Mailman-Version: 2.1.12 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Sender: ovs-dev-bounces@openvswitch.org Errors-To: ovs-dev-bounces@openvswitch.org Userspace support for force commit. Signed-off-by: Jarno Rajahalme Acked-by: Joe Stringer --- include/openvswitch/ofp-actions.h | 7 +++- lib/conntrack.c | 16 ++++++-- lib/conntrack.h | 2 +- lib/dpif-netdev.c | 8 ++-- lib/odp-util.c | 20 ++++++++- lib/ofp-actions.c | 29 +++++++++++-- ofproto/ofproto-dpif-xlate.c | 3 +- tests/odp.at | 16 ++++++++ tests/ofp-actions.at | 10 +++++ tests/ofproto-dpif.at | 85 +++++++++++++++++++++++++++++++++++++++ tests/system-traffic.at | 53 ++++++++++++++++++++++++ tests/test-conntrack.c | 9 +++-- utilities/ovs-ofctl.8.in | 12 ++++++ 13 files changed, 252 insertions(+), 18 deletions(-) diff --git a/include/openvswitch/ofp-actions.h b/include/openvswitch/ofp-actions.h index 5ea0763..622dd7a 100644 --- a/include/openvswitch/ofp-actions.h +++ b/include/openvswitch/ofp-actions.h @@ -556,9 +556,14 @@ ofpact_nest_get_action_len(const struct ofpact_nest *on) /* Bits for 'flags' in struct nx_action_conntrack. * * If NX_CT_F_COMMIT is set, then the connection entry is moved from the - * unconfirmed to confirmed list in the tracker. */ + * unconfirmed to confirmed list in the tracker. + * If NX_CT_F_FORCE is set, in addition to NX_CT_F_COMMIT, then the conntrack + * entry is replaced with a new one in case the original direction of the + * existing entry is opposite of the current packet direction. + */ enum nx_conntrack_flags { NX_CT_F_COMMIT = 1 << 0, + NX_CT_F_FORCE = 1 << 1, }; /* Magic value for struct nx_action_conntrack 'recirc_table' field, to specify diff --git a/lib/conntrack.c b/lib/conntrack.c index 8ae501b..60a3972 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -239,12 +239,21 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt, static struct conn * process_one(struct conntrack *ct, struct dp_packet *pkt, struct conn_lookup_ctx *ctx, uint16_t zone, - bool commit, long long now) + bool force, bool commit, long long now) { unsigned bucket = hash_to_bucket(ctx->hash); struct conn *conn = ctx->conn; uint16_t state = 0; + /* Delete found entry if in wrong direction. 'force' implies commit. */ + if (conn && force && ctx->reply) { + ovs_list_remove(&conn->exp_node); + hmap_remove(&ct->buckets[bucket].connections, &conn->node); + atomic_count_dec(&ct->n_conn); + delete_conn(conn); + conn = NULL; + } + if (conn) { if (ctx->related) { state |= CS_RELATED; @@ -301,7 +310,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt, * 'setlabel' behaves similarly for the connection label.*/ int conntrack_execute(struct conntrack *ct, struct dp_packet_batch *pkt_batch, - ovs_be16 dl_type, bool commit, uint16_t zone, + ovs_be16 dl_type, bool force, bool commit, uint16_t zone, const uint32_t *setmark, const struct ovs_key_ct_labels *setlabel, const char *helper) @@ -364,7 +373,8 @@ conntrack_execute(struct conntrack *ct, struct dp_packet_batch *pkt_batch, conn_key_lookup(ctb, &ctxs[j], now); - conn = process_one(ct, pkts[j], &ctxs[j], zone, commit, now); + conn = process_one(ct, pkts[j], &ctxs[j], zone, force, commit, + now); if (conn && setmark) { set_mark(pkts[j], conn, setmark[0], setmark[1]); diff --git a/lib/conntrack.h b/lib/conntrack.h index 254f61c..0437cd3 100644 --- a/lib/conntrack.h +++ b/lib/conntrack.h @@ -65,7 +65,7 @@ void conntrack_init(struct conntrack *); void conntrack_destroy(struct conntrack *); int conntrack_execute(struct conntrack *, struct dp_packet_batch *, - ovs_be16 dl_type, bool commit, + ovs_be16 dl_type, bool force, bool commit, uint16_t zone, const uint32_t *setmark, const struct ovs_key_ct_labels *setlabel, const char *helper); diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 42196b2..ee2b691 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -4734,6 +4734,7 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, case OVS_ACTION_ATTR_CT: { const struct nlattr *b; + bool force = false; bool commit = false; unsigned int left; uint16_t zone = 0; @@ -4747,7 +4748,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, switch(sub_type) { case OVS_CT_ATTR_FORCE_COMMIT: - /* Not implemented yet. */ + force = true; + /* fall through. */ case OVS_CT_ATTR_COMMIT: commit = true; break; @@ -4770,8 +4772,8 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, } } - conntrack_execute(&dp->conntrack, packets_, aux->flow->dl_type, commit, - zone, setmark, setlabel, helper); + conntrack_execute(&dp->conntrack, packets_, aux->flow->dl_type, force, + commit, zone, setmark, setlabel, helper); break; } diff --git a/lib/odp-util.c b/lib/odp-util.c index 939e378..4a3afd0 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -722,6 +722,7 @@ format_odp_ct_nat(struct ds *ds, const struct nlattr *attr) static const struct nl_policy ovs_conntrack_policy[] = { [OVS_CT_ATTR_COMMIT] = { .type = NL_A_FLAG, .optional = true, }, + [OVS_CT_ATTR_FORCE_COMMIT] = { .type = NL_A_FLAG, .optional = true, }, [OVS_CT_ATTR_ZONE] = { .type = NL_A_U16, .optional = true, }, [OVS_CT_ATTR_MARK] = { .type = NL_A_UNSPEC, .optional = true, .min_len = sizeof(uint32_t) * 2 }, @@ -740,7 +741,7 @@ format_odp_conntrack_action(struct ds *ds, const struct nlattr *attr) const uint32_t *mark; const char *helper; uint16_t zone; - bool commit; + bool commit, force; const struct nlattr *nat; if (!nl_parse_nested(attr, ovs_conntrack_policy, a, ARRAY_SIZE(a))) { @@ -749,6 +750,7 @@ format_odp_conntrack_action(struct ds *ds, const struct nlattr *attr) } commit = a[OVS_CT_ATTR_COMMIT] ? true : false; + force = a[OVS_CT_ATTR_FORCE_COMMIT] ? true : false; zone = a[OVS_CT_ATTR_ZONE] ? nl_attr_get_u16(a[OVS_CT_ATTR_ZONE]) : 0; mark = a[OVS_CT_ATTR_MARK] ? nl_attr_get(a[OVS_CT_ATTR_MARK]) : NULL; label = a[OVS_CT_ATTR_LABELS] ? nl_attr_get(a[OVS_CT_ATTR_LABELS]): NULL; @@ -756,11 +758,14 @@ format_odp_conntrack_action(struct ds *ds, const struct nlattr *attr) nat = a[OVS_CT_ATTR_NAT]; ds_put_format(ds, "ct"); - if (commit || zone || mark || label || helper || nat) { + if (commit || force || zone || mark || label || helper || nat) { ds_put_cstr(ds, "("); if (commit) { ds_put_format(ds, "commit,"); } + if (force) { + ds_put_format(ds, "force_commit,"); + } if (zone) { ds_put_format(ds, "zone=%"PRIu16",", zone); } @@ -1464,6 +1469,7 @@ parse_conntrack_action(const char *s_, struct ofpbuf *actions) const char *helper = NULL; size_t helper_len = 0; bool commit = false; + bool force_commit = false; uint16_t zone = 0; struct { uint32_t value; @@ -1498,6 +1504,11 @@ find_end: s += n; continue; } + if (ovs_scan(s, "force_commit%n", &n)) { + force_commit = true; + s += n; + continue; + } if (ovs_scan(s, "zone=%"SCNu16"%n", &zone, &n)) { s += n; continue; @@ -1548,10 +1559,15 @@ find_end: } s++; } + if (commit && force_commit) { + return -EINVAL; + } start = nl_msg_start_nested(actions, OVS_ACTION_ATTR_CT); if (commit) { nl_msg_put_flag(actions, OVS_CT_ATTR_COMMIT); + } else if (force_commit) { + nl_msg_put_flag(actions, OVS_CT_ATTR_FORCE_COMMIT); } if (zone) { nl_msg_put_u16(actions, OVS_CT_ATTR_ZONE, zone); diff --git a/lib/ofp-actions.c b/lib/ofp-actions.c index 489fdb0..5f391d1 100644 --- a/lib/ofp-actions.c +++ b/lib/ofp-actions.c @@ -5265,6 +5265,15 @@ format_DEBUG_RECIRC(const struct ofpact_null *a OVS_UNUSED, struct ds *s) * tracker, and the state of the connection will be stored beyond the * lifetime of packet processing. * + * A committed connection always has the directionality of the packet that + * caused the connection to be committed in the first place. This is the + * "original direction" of the connection, and the opposite direction is + * the "reply direction". If a connection is already committed, but it is + * then decided that the original direction should be the opposite of the + * existing connection, NX_CT_F_FORCE flag may be used in addition to + * NX_CT_F_COMMIT flag to in effect terminate the existing connection and + * start a new one in the current direction. + * * Connections may transition back into the uncommitted state due to * external timers, or due to the contents of packets that are sent to the * connection tracker. This behaviour is outside of the scope of the @@ -5326,7 +5335,7 @@ format_DEBUG_RECIRC(const struct ofpact_null *a OVS_UNUSED, struct ds *s) * * Zero or more actions may immediately follow this action. These actions will * be executed within the context of the connection tracker, and they require - * the NX_CT_F_COMMIT flag to be set. + * NX_CT_F_COMMIT flag be set. */ struct nx_action_conntrack { ovs_be16 type; /* OFPAT_VENDOR. */ @@ -5391,9 +5400,16 @@ decode_NXAST_RAW_CT(const struct nx_action_conntrack *nac, { const size_t ct_offset = ofpacts_pull(out); struct ofpact_conntrack *conntrack = ofpact_put_CT(out); + int error; + conntrack->flags = ntohs(nac->flags); + if (conntrack->flags & NX_CT_F_FORCE && + !(conntrack->flags & NX_CT_F_COMMIT)) { + error = OFPERR_OFPBAC_BAD_ARGUMENT; + goto out; + } - int error = decode_ct_zone(nac, conntrack, vl_mff_map); + error = decode_ct_zone(nac, conntrack, vl_mff_map); if (error) { goto out; } @@ -5489,6 +5505,8 @@ parse_CT(char *arg, struct ofpbuf *ofpacts, while (ofputil_parse_key_value(&arg, &key, &value)) { if (!strcmp(key, "commit")) { oc->flags |= NX_CT_F_COMMIT; + } else if (!strcmp(key, "force")) { + oc->flags |= NX_CT_F_FORCE; } else if (!strcmp(key, "table")) { error = str_to_u8(value, "recirc_table", &oc->recirc_table); if (!error && oc->recirc_table == NX_CT_RECIRC_NONE) { @@ -5534,7 +5552,9 @@ parse_CT(char *arg, struct ofpbuf *ofpacts, break; } } - + if (oc->flags & NX_CT_F_FORCE && !(oc->flags & NX_CT_F_COMMIT)) { + error = xasprintf("\"force\" flag requires \"commit\" flag."); + } ofpact_finish_CT(ofpacts, &oc); ofpbuf_push_uninit(ofpacts, ct_offset); return error; @@ -5568,6 +5588,9 @@ format_CT(const struct ofpact_conntrack *a, struct ds *s) if (a->flags & NX_CT_F_COMMIT) { ds_put_format(s, "%scommit%s,", colors.value, colors.end); } + if (a->flags & NX_CT_F_FORCE) { + ds_put_format(s, "%sforce%s,", colors.value, colors.end); + } if (a->recirc_table != NX_CT_RECIRC_NONE) { ds_put_format(s, "%stable=%s%"PRIu8",", colors.special, colors.end, a->recirc_table); diff --git a/ofproto/ofproto-dpif-xlate.c b/ofproto/ofproto-dpif-xlate.c index 9328b55..b79e011 100644 --- a/ofproto/ofproto-dpif-xlate.c +++ b/ofproto/ofproto-dpif-xlate.c @@ -5138,7 +5138,8 @@ compose_conntrack_action(struct xlate_ctx *ctx, struct ofpact_conntrack *ofc) ct_offset = nl_msg_start_nested(ctx->odp_actions, OVS_ACTION_ATTR_CT); if (ofc->flags & NX_CT_F_COMMIT) { - nl_msg_put_flag(ctx->odp_actions, OVS_CT_ATTR_COMMIT); + nl_msg_put_flag(ctx->odp_actions, ofc->flags & NX_CT_F_FORCE ? + OVS_CT_ATTR_FORCE_COMMIT : OVS_CT_ATTR_COMMIT); } nl_msg_put_u16(ctx->odp_actions, OVS_CT_ATTR_ZONE, zone); put_ct_mark(&ctx->xin->flow, ctx->odp_actions, ctx->wc); diff --git a/tests/odp.at b/tests/odp.at index 459ff35..e408c9f 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -332,6 +332,22 @@ ct(commit,nat(src=fe80::20c:29ff:fe88:a18b,random)) ct(commit,nat(src=fe80::20c:29ff:fe88:1-fe80::20c:29ff:fe88:a18b,random)) ct(commit,nat(src=[[fe80::20c:29ff:fe88:1]]-[[fe80::20c:29ff:fe88:a18b]]:255-4096,random)) ct(commit,helper=ftp,nat(src=10.1.1.240-10.1.1.255)) +ct(force_commit) +ct(force_commit,zone=5) +ct(force_commit,mark=0xa0a0a0a0/0xfefefefe) +ct(force_commit,label=0x1234567890abcdef1234567890abcdef/0xf1f2f3f4f5f6f7f8f9f0fafbfcfdfeff) +ct(force_commit,helper=ftp) +ct(nat) +ct(force_commit,nat(src)) +ct(force_commit,nat(dst)) +ct(force_commit,nat(src=10.0.0.240,random)) +ct(force_commit,nat(src=10.0.0.240:32768-65535,random)) +ct(force_commit,nat(dst=10.0.0.128-10.0.0.254,hash)) +ct(force_commit,nat(src=10.0.0.240-10.0.0.254:32768-65535,persistent)) +ct(force_commit,nat(src=fe80::20c:29ff:fe88:a18b,random)) +ct(force_commit,nat(src=fe80::20c:29ff:fe88:1-fe80::20c:29ff:fe88:a18b,random)) +ct(force_commit,nat(src=[[fe80::20c:29ff:fe88:1]]-[[fe80::20c:29ff:fe88:a18b]]:255-4096,random)) +ct(force_commit,helper=ftp,nat(src=10.1.1.240-10.1.1.255)) trunc(100) clone(1) clone(clone(push_vlan(vid=12,pcp=0),2),1) diff --git a/tests/ofp-actions.at b/tests/ofp-actions.at index c52d217..7e8bec1 100644 --- a/tests/ofp-actions.at +++ b/tests/ofp-actions.at @@ -161,6 +161,12 @@ ffff 0018 00002320 0023 0000 00000000 0000 FF 000000 0000 # actions=ct(commit) ffff 0018 00002320 0023 0001 00000000 0000 FF 000000 0000 +# actions=ct(commit,force) +ffff 0018 00002320 0023 0003 00000000 0000 FF 000000 0000 + +# bad OpenFlow10 actions: OFPBAC_BAD_ARGUMENT +ffff 0018 00002320 0023 0002 00000000 0000 FF 000000 0000 + # actions=ct(table=10) ffff 0018 00002320 0023 0000 00000000 0000 0A 000000 0000 @@ -182,6 +188,10 @@ ffff 0018 00002320 0023 0000 00010004 001F FF 000000 0000 ffff 0030 00002320 0023 0001 00000000 0000 FF 000000 0000 dnl ffff 0018 00002320 0007 001f 0001d604 000000000000f009 +# actions=ct(commit,force,exec(load:0xf009->NXM_NX_CT_MARK[])) +ffff 0030 00002320 0023 0003 00000000 0000 FF 000000 0000 dnl +ffff 0018 00002320 0007 001f 0001d604 000000000000f009 + # bad OpenFlow10 actions: OFPBAC_BAD_SET_ARGUMENT & ofp_actions|WARN|cannot set CT fields outside of ct action ffff 0018 00002320 0007 001f 0001d604 000000000000f009 diff --git a/tests/ofproto-dpif.at b/tests/ofproto-dpif.at index 82c863c..a6aad1b 100644 --- a/tests/ofproto-dpif.at +++ b/tests/ofproto-dpif.at @@ -8528,6 +8528,88 @@ udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:0a,dl_dst=50:54:00:00:00:09,nw_src=10. OVS_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([ofproto-dpif - conntrack - force commit]) +OVS_VSWITCHD_START + +add_of_ports br0 1 2 + +AT_CHECK([ovs-appctl vlog/set dpif_netdev:dbg vconn:info ofproto_dpif:info]) + +dnl Allow new connections on p1->p2, but not on p2->p1. +AT_DATA([flows.txt], [dnl +dnl Table 0 +dnl +table=0,priority=100,arp,action=normal +table=0,priority=10,in_port=1,udp,action=ct(commit),controller +table=0,priority=10,in_port=2,udp,action=ct(table=1) +table=0,priority=1,action=drop +dnl +dnl Table 1 +dnl +table=1,priority=10,in_port=2,ct_state=+est,udp,action=ct(force,commit),controller +table=1,priority=1,action=drop +]) + +AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) + +AT_CAPTURE_FILE([ofctl_monitor.log]) +AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl -P nxt_packet_in --detach --no-chdir --pidfile 2> ofctl_monitor.log]) + +AT_CHECK([ovs-appctl netdev-dummy/receive p2 'in_port(2),eth(src=50:54:00:00:00:0a,dst=50:54:00:00:00:09),eth_type(0x0800),ipv4(src=10.1.1.2,dst=10.1.1.1,proto=17,tos=0,ttl=64,frag=no),udp(src=2,dst=1)']) + +dnl OK, now start a new connection from port 1. +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.1.1.1,dst=10.1.1.2,proto=17,tos=0,ttl=64,frag=no),udp(src=1,dst=2)']) + +dnl Now try a reply from port 2. +AT_CHECK([ovs-appctl netdev-dummy/receive p2 'in_port(2),eth(src=50:54:00:00:00:0a,dst=50:54:00:00:00:09),eth_type(0x0800),ipv4(src=10.1.1.2,dst=10.1.1.1,proto=17,tos=0,ttl=64,frag=no),udp(src=2,dst=1)']) + +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 4]) +OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit]) + +dnl Check this output. We only see the latter two packets, not the first. +dnl Note that the first packet doesn't have the ct_state bits set. This +dnl happens because the ct_state field is available only after recirc. +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) +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 +]) + +AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl -P nxt_packet_in --detach --no-chdir --pidfile 2> ofctl_monitor.log]) + +dnl OK, now start a second connection from port 1 +AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.1.1.1,dst=10.1.1.2,proto=17,tos=0,ttl=64,frag=no),udp(src=3,dst=4)']) + +dnl Now try a reply from port 2. +AT_CHECK([ovs-appctl netdev-dummy/receive p2 'in_port(2),eth(src=50:54:00:00:00:0a,dst=50:54:00:00:00:09),eth_type(0x0800),ipv4(src=10.1.1.2,dst=10.1.1.1,proto=17,tos=0,ttl=64,frag=no),udp(src=4,dst=3)']) + +OVS_WAIT_UNTIL([test `wc -l < ofctl_monitor.log` -ge 4]) +OVS_WAIT_UNTIL([ovs-appctl -t ovs-ofctl exit]) + +dnl Check this output. We should see both packets +dnl Note that the first packet doesn't have the ct_state bits set. This +dnl happens because the ct_state field is available only after recirc. +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) +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 +]) + +dnl +dnl Check that the directionality has been changed by force commit. +dnl +AT_CHECK([ovs-appctl dpctl/dump-conntrack | sort], [], [dnl +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) +udp,orig=(src=10.1.1.2,dst=10.1.1.1,sport=4,dport=3),reply=(src=10.1.1.1,dst=10.1.1.2,sport=3,dport=4) +]) + +OVS_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([ofproto-dpif - conntrack - ipv6]) OVS_VSWITCHD_START @@ -9033,6 +9115,9 @@ AT_CHECK([ovs-ofctl add-flows br0 flows.txt]) AT_CHECK([ovs-appctl netdev-dummy/receive p1 'in_port(1),eth(src=50:54:00:00:00:09,dst=50:54:00:00:00:0a),eth_type(0x0800),ipv4(src=10.1.1.1,dst=10.1.1.2,proto=17,tos=0,ttl=64,frag=no),udp(src=1,dst=2)']) AT_CHECK([ovs-appctl netdev-dummy/receive p2 'in_port(2),eth(src=50:54:00:00:00:0a,dst=50:54:00:00:00:09),eth_type(0x0800),ipv4(src=10.1.1.2,dst=10.1.1.1,proto=17,tos=0,ttl=64,frag=no),udp(src=2,dst=1)']) +# Give time for logs to appear. +ovs-appctl revalidator/wait + AT_CHECK([cat ovs-vswitchd.log | strip_ufid | filter_flow_install], [0], [dnl ct_state(+rpl+trk),ct_label(0x1),recirc_id(0x1),in_port(2),eth_type(0x0800),ipv4(frag=no), actions:1 recirc_id(0),in_port(1),eth_type(0x0800),ipv4(proto=17,frag=no),udp(src=1), actions:ct(commit,label=0x1),2 diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 4fdd27e..cad2677 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -653,6 +653,59 @@ udp,vlan_tci=0x0000,dl_src=50:54:00:00:00:09,dl_dst=50:54:00:00:00:0a,nw_src=10. OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([conntrack - force commit]) +CHECK_CONNTRACK() +OVS_TRAFFIC_VSWITCHD_START() +AT_CHECK([ovs-appctl vlog/set dpif:dbg dpif_netdev:dbg ofproto_dpif_upcall:dbg]) + +ADD_NAMESPACES(at_ns0, at_ns1) + +ADD_VETH(p0, at_ns0, br0, "10.1.1.1/24") +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=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 +]) + +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) + +AT_CAPTURE_FILE([ofctl_monitor.log]) +AT_CHECK([ovs-ofctl monitor br0 65534 invalid_ttl --detach --no-chdir --pidfile 2> ofctl_monitor.log]) + +dnl Send an unsolicited reply from port 2. This should be dropped. +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101020a0101010002000100080000 actions=resubmit(,0)"]) + +dnl OK, now start a new connection from port 1. +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101010a0101020001000200080000 actions=resubmit(,0)"]) + +dnl Now try a reply from port 2. +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=2 packet=50540000000a50540000000908004500001c000000000011a4cd0a0101020a0101010002000100080000 actions=resubmit(,0)"]) + +AT_CHECK([ovs-appctl revalidator/purge], [0]) + +dnl Check this output. We only see the latter two packets, not the first. +AT_CHECK([cat ofctl_monitor.log], [0], [dnl +NXT_PACKET_IN2 (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=0,tp_src=1,tp_dst=2 udp_csum:0 +NXT_PACKET_IN2 (xid=0x0): table_id=1 cookie=0x0 total_len=42 ct_state=new|trk,ct_nw_src=10.1.1.2,ct_nw_dst=10.1.1.1,ct_nw_proto=17,ct_tp_src=2,ct_tp_dst=1,in_port=2 (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.2,nw_dst=10.1.1.1,nw_tos=0,nw_ecn=0,nw_ttl=0,tp_src=2,tp_dst=1 udp_csum:0 +]) + +dnl +dnl Check that the directionality has been changed by force commit. +dnl +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=10\.1\.1\.2,"], [], [dnl +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_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([conntrack - IPv4 ping]) CHECK_CONNTRACK() OVS_TRAFFIC_VSWITCHD_START() diff --git a/tests/test-conntrack.c b/tests/test-conntrack.c index d259325..4213f5c 100644 --- a/tests/test-conntrack.c +++ b/tests/test-conntrack.c @@ -88,7 +88,8 @@ ct_thread_main(void *aux_) pkt_batch = prepare_packets(batch_size, change_conn, aux->tid, &dl_type); ovs_barrier_block(&barrier); for (i = 0; i < n_pkts; i += batch_size) { - conntrack_execute(&ct, pkt_batch, dl_type, true, 0, NULL, NULL, NULL); + conntrack_execute(&ct, pkt_batch, dl_type, false, true, 0, NULL, NULL, + NULL); } ovs_barrier_block(&barrier); destroy_packets(pkt_batch); @@ -170,15 +171,15 @@ pcap_batch_execute_conntrack(struct conntrack *ct, } if (flow.dl_type != dl_type) { - conntrack_execute(ct, &new_batch, dl_type, true, 0, NULL, NULL, - NULL); + conntrack_execute(ct, &new_batch, dl_type, false, true, 0, NULL, + NULL, NULL); dp_packet_batch_init(&new_batch); } new_batch.packets[new_batch.count++] = packet;; } if (!dp_packet_batch_is_empty(&new_batch)) { - conntrack_execute(ct, &new_batch, dl_type, true, 0, NULL, NULL, NULL); + conntrack_execute(ct, &new_batch, dl_type, false, true, 0, NULL, NULL, NULL); } } diff --git a/utilities/ovs-ofctl.8.in b/utilities/ovs-ofctl.8.in index d783f85..14db85e 100644 --- a/utilities/ovs-ofctl.8.in +++ b/utilities/ovs-ofctl.8.in @@ -1037,6 +1037,18 @@ Commit the connection to the connection tracking module. Information about the connection will be stored beyond the lifetime of the packet in the pipeline. Some \fBct_state\fR flags are only available for committed connections. .RE +.IP \fBforce\fR +.RS +A committed connection always has the directionality of the packet +that caused the connection to be committed in the first place. This +is the ``original direction'' of the connection, and the opposite +direction is the ``reply direction''. If a connection is already +committed, but it is in the wrong direction, \fBforce\fR flag may be +used in addition to \fBcommit\fR flag to effectively terminate the +existing connection and start a new one in the current direction. +This flag has no effect if the original direction of the connection is +already the same as that of the current packet. +.RE .IP \fBtable=\fInumber\fR Fork pipeline processing in two. The original instance of the packet will continue processing the current actions list as an untracked packet. An