From a27d70a8984879bc0a66afc2d7c35149659be24d Mon Sep 17 00:00:00 2001 From: Eelco Chaudron Date: Mon, 12 Oct 2020 07:38:38 -0400 Subject: [PATCH] conntrack: add generic IP protocol support Currently, userspace conntrack only tracks TCP, UDP, and ICMP, and all other IP protocols are discarded, and the +inv state is returned. This is not in line with the kernel conntrack. Where if no L4 information can be extracted it's treated as generic L3. The change below mimics the behavior of the kernel. Signed-off-by: Eelco Chaudron Acked-by: Flavio Leitner Signed-off-by: Ilya Maximets --- NEWS | 3 +++ lib/conntrack-private.h | 3 +++ lib/conntrack.c | 29 +++++++++++++++++++---------- tests/system-traffic.at | 29 +++++++++++++++++++++++++++++ 4 files changed, 54 insertions(+), 10 deletions(-) diff --git a/NEWS b/NEWS index 1a39cc661..e62d815fa 100644 --- a/NEWS +++ b/NEWS @@ -17,6 +17,9 @@ Post-v2.14.0 restricts a flow dump to a single PMD thread if set. * New 'options:dpdk-vf-mac' field for DPDK interface of VF ports, that allows configuring the MAC address of a VF representor. + * Add generic IP protocol support to conntrack. With this change, all + none UDP, TCP, and ICMP traffic will be treated as general L3 + traffic, i.e. using 3 tupples. - The environment variable OVS_UNBOUND_CONF, if set, is now used as the DNS resolver's (unbound) configuration file. - Linux datapath: diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index 789af82ff..3895bc688 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -59,6 +59,9 @@ struct conn_key { uint8_t nw_proto; }; +/* Verify that nw_proto stays uint8_t as it's used to index into l4_protos[] */ +BUILD_ASSERT_DECL(MEMBER_SIZEOF(struct conn_key, nw_proto) == sizeof(uint8_t)); + /* This is used for alg expectations; an expectation is a * context created in preparation for establishing a data * connection. The expectation is created by the control diff --git a/lib/conntrack.c b/lib/conntrack.c index 930ed0be6..bba38f9f5 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -146,12 +146,7 @@ detect_ftp_ctl_type(const struct conn_lookup_ctx *ctx, static void expectation_clean(struct conntrack *ct, const struct conn_key *parent_key); -static struct ct_l4_proto *l4_protos[] = { - [IPPROTO_TCP] = &ct_proto_tcp, - [IPPROTO_UDP] = &ct_proto_other, - [IPPROTO_ICMP] = &ct_proto_icmp4, - [IPPROTO_ICMPV6] = &ct_proto_icmp6, -}; +static struct ct_l4_proto *l4_protos[UINT8_MAX + 1]; static void handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, @@ -293,6 +288,7 @@ ct_print_conn_info(const struct conn *c, const char *log_msg, struct conntrack * conntrack_init(void) { + static struct ovsthread_once setup_l4_once = OVSTHREAD_ONCE_INITIALIZER; struct conntrack *ct = xzalloc(sizeof *ct); ovs_rwlock_init(&ct->resources_lock); @@ -320,6 +316,18 @@ conntrack_init(void) ct->clean_thread = ovs_thread_create("ct_clean", clean_thread_main, ct); ct->ipf = ipf_init(); + /* Initialize the l4 protocols. */ + if (ovsthread_once_start(&setup_l4_once)) { + for (int i = 0; i < ARRAY_SIZE(l4_protos); i++) { + l4_protos[i] = &ct_proto_other; + } + /* IPPROTO_UDP uses ct_proto_other, so no need to initialize it. */ + l4_protos[IPPROTO_TCP] = &ct_proto_tcp; + l4_protos[IPPROTO_ICMP] = &ct_proto_icmp4; + l4_protos[IPPROTO_ICMPV6] = &ct_proto_icmp6; + + ovsthread_once_done(&setup_l4_once); + } return ct; } @@ -1982,9 +1990,10 @@ extract_l4(struct conn_key *key, const void *data, size_t size, bool *related, return (!related || check_l4_icmp6(key, data, size, l3, validate_checksum)) && extract_l4_icmp6(key, data, size, related); - } else { - return false; } + + /* For all other protocols we do not have L4 keys, so keep them zero. */ + return true; } static bool @@ -2267,8 +2276,8 @@ nat_select_range_tuple(struct conntrack *ct, const struct conn *conn, conn->nat_info->nat_action & NAT_ACTION_SRC_PORT ? true : false; union ct_addr first_addr = ct_addr; - bool pat_enabled = conn->key.nw_proto != IPPROTO_ICMP && - conn->key.nw_proto != IPPROTO_ICMPV6; + bool pat_enabled = conn->key.nw_proto == IPPROTO_TCP || + conn->key.nw_proto == IPPROTO_UDP; while (true) { if (conn->nat_info->nat_action & NAT_ACTION_SRC) { diff --git a/tests/system-traffic.at b/tests/system-traffic.at index 14f349b5b..d2a4dbffe 100644 --- a/tests/system-traffic.at +++ b/tests/system-traffic.at @@ -2333,6 +2333,35 @@ NXST_FLOW reply: OVS_TRAFFIC_VSWITCHD_STOP AT_CLEANUP +AT_SETUP([conntrack - generic IP protocol]) +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 +table=0, priority=1,action=drop +table=0, priority=10,arp,action=normal +table=0, priority=100,ip,action=ct(table=1) +table=1, priority=100,in_port=1,ip,ct_state=+trk+new,action=ct(commit) +table=1, priority=100,in_port=1,ct_state=+trk+est,action=normal +]) + +AT_CHECK([ovs-ofctl --bundle add-flows br0 flows.txt]) + +AT_CHECK([ovs-ofctl -O OpenFlow13 packet-out br0 "in_port=1 packet=01005e00001200005e000101080045c0002800000000ff7019cdc0a8001ee0000012210164010001ba52c0a800010000000000000000000000000000 actions=resubmit(,0)"]) + +AT_CHECK([ovs-appctl dpctl/dump-conntrack | grep "orig=.src=192\.168\.0\.30,"], [], [dnl +112,orig=(src=192.168.0.30,dst=224.0.0.18,sport=0,dport=0),reply=(src=224.0.0.18,dst=192.168.0.30,sport=0,dport=0) +]) + +OVS_TRAFFIC_VSWITCHD_STOP +AT_CLEANUP + AT_SETUP([conntrack - ICMP related]) AT_SKIP_IF([test $HAVE_NC = no]) CHECK_CONNTRACK()