Presently, the userpace connection tracker 'established' packet state diverges from the kernel and this patch brings them in line. The behavior is now that 'established' is only possible after a reply packet is seen. The previous behavior is hard to notice when rules are written to commit a connection in the trusted direction, which is recommended.
A test is added to verify this. The documentation is updated to describe the new behavior of 'established' and also clarify 'new'. Signed-off-by: Darrell Ball <[email protected]> --- lib/conntrack-private.h | 1 + lib/conntrack.c | 21 ++++++++++++++++----- lib/meta-flow.xml | 10 +++++++--- tests/system-traffic.at | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+), 8 deletions(-) diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index ac0198f..1f6a107 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -107,6 +107,7 @@ struct conn { uint8_t seq_skew_dir; /* True if alg data connection. */ uint8_t alg_related; + uint8_t reply_seen; }; enum ct_update_res { diff --git a/lib/conntrack.c b/lib/conntrack.c index dea2fed..323114a 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -928,6 +928,21 @@ nat_res_exhaustion: return NULL; } +/* This function is called with the bucket lock held. */ +static void +conn_handle_reply(struct dp_packet *pkt, const struct conn_lookup_ctx *ctx, + struct conn **conn) +{ + if (ctx->reply) { + pkt->md.ct_state |= CS_REPLY_DIR; + (*conn)->reply_seen = true; + } + if ((*conn)->reply_seen) { + pkt->md.ct_state |= CS_ESTABLISHED; + pkt->md.ct_state &= ~CS_NEW; + } +} + static bool conn_update_state(struct conntrack *ct, struct dp_packet *pkt, struct conn_lookup_ctx *ctx, struct conn **conn, @@ -950,11 +965,7 @@ conn_update_state(struct conntrack *ct, struct dp_packet *pkt, switch (res) { case CT_UPDATE_VALID: - pkt->md.ct_state |= CS_ESTABLISHED; - pkt->md.ct_state &= ~CS_NEW; - if (ctx->reply) { - pkt->md.ct_state |= CS_REPLY_DIR; - } + conn_handle_reply(pkt, ctx, conn); break; case CT_UPDATE_INVALID: pkt->md.ct_state = CS_INVALID; diff --git a/lib/meta-flow.xml b/lib/meta-flow.xml index 08ee0ec..33a2ad6 100644 --- a/lib/meta-flow.xml +++ b/lib/meta-flow.xml @@ -2493,13 +2493,17 @@ actions=clone(load:0->NXM_OF_IN_PORT[],output:123) <dl> <dt><code>new</code> (0x01)</dt> <dd> - A new connection. Set to 1 if this is an uncommitted connection. + A new connection. Set to 1 if this is an uncommitted connection + or a committed connection that has not seen a reply yet. </dd> <dt><code>est</code> (0x02)</dt> <dd> - Part of an existing connection. Set to 1 if this is a committed - connection. + There are two requirements for a packet to be established, namely, + the associated connection must be committed and a reply must have + been seen. The reply packet that creates this condition will be + marked as established as well as subsequent packets in either + direction that are associated with the same conntrack entry. </dd> <dt><code>rel</code> (0x04)</dt> diff --git a/tests/system-traffic.at b/tests/system-traffic.at index fd7b612..cd55406 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -871,6 +871,41 @@ NS_CHECK_EXEC([at_ns1], [ping -q -c 3 -i 0.3 -w 2 10.1.1.1 | FORMAT_PING], [0], OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([conntrack - IPv4 ping Check Est state]) +CHECK_CONNTRACK() +OVS_TRAFFIC_VSWITCHD_START() + +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") + +dnl Allow any traffic from ns0->ns1. Only allow nd, return traffic from ns1->ns0. +dnl Check that packet is not established before a reply. +AT_DATA([flows.txt], [dnl +priority=1,action=drop +priority=10,arp,action=normal +table=0,priority=10,in_port=2,icmp,ct_state=-trk,action=ct(table=0) +table=0,priority=10,in_port=2,icmp,ct_state=+trk+est actions=output:1 +table=0,priority=10,in_port=1,icmp actions=ct(commit,table=1) +table=1,priority=10,in_port=1,icmp,ct_state=+trk+new actions=ct(table=2) +table=2,priority=10,in_port=1,icmp,ct_state=+trk+new actions=output:2 +]) + +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) + +dnl Pings from ns0->ns1 should work fine. +NS_CHECK_EXEC([at_ns0], [ping -q -c 1 -i 0.3 -w 2 10.1.1.2 | FORMAT_PING], [0], [dnl +1 packets transmitted, 1 received, 0% packet loss, time 0ms +]) + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | FORMAT_CT(10.1.1.2)], [0], [dnl +icmp,orig=(src=10.1.1.1,dst=10.1.1.2,id=<cleared>,type=8,code=0),reply=(src=10.1.1.2,dst=10.1.1.1,id=<cleared>,type=0,code=0) +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([conntrack - IPv6 ping]) CHECK_CONNTRACK() OVS_TRAFFIC_VSWITCHD_START() -- 1.9.1 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
