From patchwork Mon Jan 8 20:54:25 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Darrell Ball X-Patchwork-Id: 857121 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="VWsnr/9/"; dkim-atps=neutral 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 3zFnbP5WPdz9s71 for ; Tue, 9 Jan 2018 07:55:13 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id A39BAE6D; Mon, 8 Jan 2018 20:55:11 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id CE66FE5A for ; Mon, 8 Jan 2018 20:55:10 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pf0-f194.google.com (mail-pf0-f194.google.com [209.85.192.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 93FAEE3 for ; Mon, 8 Jan 2018 20:55:10 +0000 (UTC) Received: by mail-pf0-f194.google.com with SMTP id m26so6833572pfj.11 for ; Mon, 08 Jan 2018 12:55:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id; bh=jLviHjA1gX58NKYjKZkt8mxSQaTJYmH/zuYJLM8kEXo=; b=VWsnr/9/HK/vgNUrAVhxkO8RMXeyDI9k1KpkMVxHvm91W/g1Ee//UXo0w+FqxcTuBG TAHx1RY80bHm5fYNXPhAxizP0PlVLlHic/R4TUDYVCmwl7Ngmqx2vH7bpr0raqMAsWXa b6oJF8EQpP2D/audZxTB3+pM5seizFoQwKhuEpwGLg9fGl5Zbhwm4+c3gD5bX6+RSiMN MIuoC2GxHOfVeyy0eTyoGnn6welx5KpYx3Cdtj6AHOcu3HJRM/XA0O/MmkCnqciiz6TO OJpXOP7GtW3z125P87Oqo0NPanSwZRWVz/7X2T0CL9l+QpuAGpUmJ6YA+qL3nn9TI5lR lDGQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id; bh=jLviHjA1gX58NKYjKZkt8mxSQaTJYmH/zuYJLM8kEXo=; b=EfT23hI6rXZ47M6CG/OTiXrY7Zbl7+j76OZBQv9ickuDOv4++JC9Dpe0ce3/XEBsaY 6ScgQF0ujw59CwZmHwdDIV/yvQQtPOWgN67LgpMkyMDZ/w7CzRBgH21RKJqdADtHwF1C V9rDmDDANPywh1EHMCnX9t4aOIwB4WhuqiqOL+iKb59XnNaOzN+bnMnjdjmwSJwHxNbL ZIAkAse2+YGkfZ7LVLX+iSi4ANAJwNrZS/CTbs3F2k3lzNb4YXgBPSRdqt2QPJ04804H AZj1yHrsVD7QQ0c24UeDPFy71ZYpC3gftriiCjygkoAXMHrn6bg4zVDQMyQQb7hBjpWD tV/Q== X-Gm-Message-State: AKGB3mKpuwOsGKpAfwTLPefPIDIM3w9di80cuLTVErGT0IJDdq1C77fO RX/WmOT55HxZDncwL4xDoWE= X-Google-Smtp-Source: ACJfBovIbYF95g/Vz6Uhu9V3w86WKebJhnZ8IPS75Xbppan0p3Oienv2UqLNIhNloK81xeWNlAQi+A== X-Received: by 10.84.246.135 with SMTP id m7mr13603079pll.384.1515444910191; Mon, 08 Jan 2018 12:55:10 -0800 (PST) Received: from sc9-mailhost1.vmware.com (c-73-162-236-45.hsd1.ca.comcast.net. [73.162.236.45]) by smtp.gmail.com with ESMTPSA id i85sm28379519pfi.54.2018.01.08.12.55.08 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 08 Jan 2018 12:55:09 -0800 (PST) From: Darrell Ball To: dlu998@gmail.com, dev@openvswitch.org Date: Mon, 8 Jan 2018 12:54:25 -0800 Message-Id: <1515444869-21655-1-git-send-email-dlu998@gmail.com> X-Mailer: git-send-email 1.9.1 X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [patch v3 1/5] hindex: Add hindex_next_node_with_hash. 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 Add hindex_next_node_with_hash() api which gets a next hindex node with the same hash as the parameter node or null if there is no such next node. This api will be used is a subsequent patch. Signed-off-by: Darrell Ball --- lib/hindex.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/lib/hindex.h b/lib/hindex.h index 876c5a9..f70d086 100644 --- a/lib/hindex.h +++ b/lib/hindex.h @@ -154,6 +154,15 @@ hindex_node_with_hash(const struct hindex *hindex, size_t hash) return node; } +/* Returns the next node in 'hindex' with the same 'hash' of node, or a null + * pointer if no more nodes have that hash value. Node must be a valid + * non-null node. */ +static inline struct hindex_node * +hindex_next_node_with_hash(const struct hindex_node *node) +{ + return node->s; +} + /* Iteration. */ /* Iterates through every node in HINDEX. */ From patchwork Mon Jan 8 20:54:26 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Darrell Ball X-Patchwork-Id: 857122 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="OMFOPUf6"; dkim-atps=neutral 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 3zFnc01KMbz9s71 for ; Tue, 9 Jan 2018 07:55:44 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 91F2DED0; Mon, 8 Jan 2018 20:55:14 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 987FAE6E for ; Mon, 8 Jan 2018 20:55:12 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg0-f52.google.com (mail-pg0-f52.google.com [74.125.83.52]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id B936AE3 for ; Mon, 8 Jan 2018 20:55:11 +0000 (UTC) Received: by mail-pg0-f52.google.com with SMTP id q67so6365818pga.9 for ; Mon, 08 Jan 2018 12:55:11 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references; bh=zl09kNsIOK/yNl66UEAu6yDLMH15la5zCtGxwrhuWtA=; b=OMFOPUf6pgC/KvDQGe/K4o4q/B8dbiRpzdTnjlRztiOnMhIgFRylGPYdRgzxiw9mm1 0HwOnGKgZlIRoAsYmEfOWVmal16/UokLBN9xloyq86GneEggHnNoZFmgc+/mgm5ElQXb VdnNZAj7M2eS2osufBsK5MoomQhdz0N8WOemCHKAXNiQYO2KzCnTujS7+n4NP6L303N0 t7uNgMh45K5l6jWA6UqKe3fyk1MiSasBFlKePH8ih83jpEauXqN0H2HxQT/ov2fASzHK KjHTBPUr4ptnlsL+Styz/haDPHTGQ591od5LTUmSgztTjIrd90dUU1AN+60mDdwTxYMf JPLQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=zl09kNsIOK/yNl66UEAu6yDLMH15la5zCtGxwrhuWtA=; b=aj8vd8mrlc91RU2PuvgUjkRwofBPE5XsHLY/D0fw34T3aQ5cH2aAqMnlD0muRiTNq2 YoU6CmW/7g7VeY3Pwr3QVypqUbPbcRHOJCLjs609Qfc4uThdHs58OOzn1fsy6uWsF8Qv Zv6ZGNN9GqHnxbDCzWwCkiUFGH0UTNh0N4plqdZgELhS8bXPWe382KjuiobVkpxNa0qs XRsJId7EiYUyjQH9cvLHAEJmVwqzVukucti4oG3lIj1Ob07qUz63k8GPwJ/32tYaKWud o/cJzDWG1lXjit9gOKj5ChDj0rW5AopcZjVZAK17Bd76y/3oicArBK38z4IjQRZVsLHO dYEQ== X-Gm-Message-State: AKGB3mJ9okGf4HH2cFzaTpMlrHo4a7iG3/UmT1OmgkWWxIQsjHDtGW8T wzcVtoSteLdINuJbnI5mDKMdPg== X-Google-Smtp-Source: ACJfBosvBX9HkxF5n+4axck5EDhJBPFGumuj5bldy4npZF9OF++ZA87zdkJhG+RuwwufphmDWh63Bw== X-Received: by 10.159.249.65 with SMTP id h1mr3013703pls.261.1515444911234; Mon, 08 Jan 2018 12:55:11 -0800 (PST) Received: from sc9-mailhost1.vmware.com (c-73-162-236-45.hsd1.ca.comcast.net. [73.162.236.45]) by smtp.gmail.com with ESMTPSA id i85sm28379519pfi.54.2018.01.08.12.55.10 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 08 Jan 2018 12:55:10 -0800 (PST) From: Darrell Ball To: dlu998@gmail.com, dev@openvswitch.org Date: Mon, 8 Jan 2018 12:54:26 -0800 Message-Id: <1515444869-21655-2-git-send-email-dlu998@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1515444869-21655-1-git-send-email-dlu998@gmail.com> References: <1515444869-21655-1-git-send-email-dlu998@gmail.com> X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [patch v3 2/5] conntrack: Fix alg expectation cleanup. 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 Presently, alg expectations are removed by being time expired. This was intended to happen before the control connections and was intended to minimize the extra work involved for tracking and removing the expectations. This is not the best option since it should be possible to remove expectations when a control connection is removed and a new api is in the works to do this. Also, conceptually an expectation should not exist without a control connection context and it can be argued that this should be a strict requirement. The approach is changed to remove the expectations when the control connections are removed. The previous code to expire the expectations is removed at the same time. Fixes: bd5e81a0e ("Userspace Datapath: Add ALG infra and FTP.") Reported-at: https://mail.openvswitch.org/pipermail/ovs-dev/2017-December/341683.html Signed-off-by: Darrell Ball --- v2->v3: Use hindex map in lieu of hmap for efficiency: Ben P. lib/conntrack-private.h | 7 +- lib/conntrack.c | 191 ++++++++++++++++++++++++++++++++---------------- lib/conntrack.h | 8 +- 3 files changed, 136 insertions(+), 70 deletions(-) diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index ac0198f..60e2902 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -68,11 +68,10 @@ struct nat_conn_key_node { * connection. The expectation is created by the control * connection. */ struct alg_exp_node { + /* Node in alg_expectations. */ struct hmap_node node; - /* Expiry list node for an expectation. */ - struct ovs_list exp_node; - /* The time when this expectation will expire. */ - long long expiration; + /* Node in alg_expectation_refs. */ + struct hindex_node node_ref; /* Key of data connection to be created. */ struct conn_key key; /* Corresponding key of the control connection. */ diff --git a/lib/conntrack.c b/lib/conntrack.c index 6d078f5..6bb30ce 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -140,7 +140,7 @@ static enum ftp_ctl_pkt process_ftp_ctl_v4(struct conntrack *ct, struct dp_packet *pkt, const struct conn *conn_for_expectation, - long long now, ovs_be32 *v4_addr_rep, + ovs_be32 *v4_addr_rep, char **ftp_data_v4_start, size_t *addr_offset_from_ftp_data_start); @@ -148,6 +148,10 @@ static enum ftp_ctl_pkt detect_ftp_ctl_type(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt); +static void +expectation_clean(struct conntrack *ct, const struct conn_key *master_key, + uint32_t basis); + static struct ct_l4_proto *l4_protos[] = { [IPPROTO_TCP] = &ct_proto_tcp, [IPPROTO_UDP] = &ct_proto_other, @@ -166,8 +170,8 @@ handle_tftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx OVS_UNUSED, struct dp_packet *pkt OVS_UNUSED, const struct conn *conn_for_expectation, - long long now, enum ftp_ctl_pkt ftp_ctl OVS_UNUSED, - bool nat OVS_UNUSED); + long long now OVS_UNUSED, + enum ftp_ctl_pkt ftp_ctl OVS_UNUSED, bool nat OVS_UNUSED); typedef void (*alg_helper)(struct conntrack *ct, const struct conn_lookup_ctx *ctx, @@ -190,8 +194,6 @@ long long ct_timeout_val[] = { /* The maximum TCP or UDP port number. */ #define CT_MAX_L4_PORT 65535 -/* Alg expectation timeout. */ -#define CT_ALG_EXP_TIMEOUT (30 * 1000) /* String buffer used for parsing FTP string messages. * This is sized about twice what is needed to leave some * margin of error. */ @@ -312,6 +314,7 @@ conntrack_init(struct conntrack *ct) ct_rwlock_wrlock(&ct->resources_lock); hmap_init(&ct->nat_conn_keys); hmap_init(&ct->alg_expectations); + hindex_init(&ct->alg_expectation_refs); ovs_list_init(&ct->alg_exp_list); ct_rwlock_unlock(&ct->resources_lock); @@ -373,8 +376,10 @@ conntrack_destroy(struct conntrack *ct) HMAP_FOR_EACH_POP (alg_exp_node, node, &ct->alg_expectations) { free(alg_exp_node); } + ovs_list_poison(&ct->alg_exp_list); hmap_destroy(&ct->alg_expectations); + hindex_destroy(&ct->alg_expectation_refs); ct_rwlock_unlock(&ct->resources_lock); ct_rwlock_destroy(&ct->resources_lock); } @@ -512,16 +517,6 @@ handle_alg_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, } static void -alg_exp_init_expiration(struct conntrack *ct, - struct alg_exp_node *alg_exp_node, - long long now) - OVS_REQ_WRLOCK(ct->resources_lock) -{ - alg_exp_node->expiration = now + CT_ALG_EXP_TIMEOUT; - ovs_list_push_back(&ct->alg_exp_list, &alg_exp_node->exp_node); -} - -static void pat_packet(struct dp_packet *pkt, const struct conn *conn) { if (conn->nat_info->nat_action & NAT_ACTION_SRC) { @@ -809,6 +804,9 @@ conn_clean(struct conntrack *ct, struct conn *conn, struct conntrack_bucket *ctb) OVS_REQUIRES(ctb->lock) { + if (conn->alg) { + expectation_clean(ct, &conn->key, ct->hash_basis); + } ovs_list_remove(&conn->exp_node); hmap_remove(&ctb->connections, &conn->node); atomic_count_dec(&ct->n_conn); @@ -1386,27 +1384,6 @@ sweep_bucket(struct conntrack *ct, struct conntrack_bucket *ctb, } } } - - enum { MAX_ALG_EXP_TO_EXPIRE = 1000 }; - size_t alg_exp_count = hmap_count(&ct->alg_expectations); - /* XXX: revisit this. */ - size_t max_to_expire = MAX(alg_exp_count/10, MAX_ALG_EXP_TO_EXPIRE); - count = 0; - ct_rwlock_wrlock(&ct->resources_lock); - struct alg_exp_node *alg_exp_node, *alg_exp_node_next; - LIST_FOR_EACH_SAFE (alg_exp_node, alg_exp_node_next, - exp_node, &ct->alg_exp_list) { - if (now < alg_exp_node->expiration || count >= max_to_expire) { - min_expiration = MIN(min_expiration, alg_exp_node->expiration); - break; - } - ovs_list_remove(&alg_exp_node->exp_node); - hmap_remove(&ct->alg_expectations, &alg_exp_node->node); - free(alg_exp_node); - count++; - } - ct_rwlock_unlock(&ct->resources_lock); - return min_expiration; } @@ -2548,17 +2525,6 @@ conntrack_flush(struct conntrack *ct, const uint16_t *zone) ct_lock_unlock(&ct->buckets[i].lock); } - ct_rwlock_wrlock(&ct->resources_lock); - struct alg_exp_node *alg_exp_node, *alg_exp_node_next; - HMAP_FOR_EACH_SAFE (alg_exp_node, alg_exp_node_next, - node, &ct->alg_expectations) { - if (!zone || *zone == alg_exp_node->key.zone) { - ovs_list_remove(&alg_exp_node->exp_node); - hmap_remove(&ct->alg_expectations, &alg_exp_node->node); - free(alg_exp_node); - } - } - ct_rwlock_unlock(&ct->resources_lock); return 0; } @@ -2582,10 +2548,110 @@ expectation_lookup(struct hmap *alg_expectations, return NULL; } +/* This function must be called with the ct->resources write lock taken. */ +static void +expectation_remove(struct hmap *alg_expectations, + const struct conn_key *key, uint32_t basis) +{ + struct alg_exp_node *alg_exp_node; + uint32_t alg_exp_conn_key_hash = conn_key_hash(key, basis); + + HMAP_FOR_EACH_WITH_HASH (alg_exp_node, node, alg_exp_conn_key_hash, + alg_expectations) { + if (!conn_key_cmp(&alg_exp_node->key, key)) { + hmap_remove(alg_expectations, &alg_exp_node->node); + break; + } + } +} + +/* This function must be called with the ct->resources read lock taken. */ +static struct alg_exp_node * +expectation_ref_lookup_unique(const struct hindex *alg_expectation_refs, + const struct conn_key *master_key, + const struct conn_key *alg_exp_key, + uint32_t basis) +{ + struct alg_exp_node *alg_exp_node; + uint32_t alg_exp_conn_key_hash = conn_key_hash(master_key, basis); + + HINDEX_FOR_EACH_WITH_HASH (alg_exp_node, node_ref, alg_exp_conn_key_hash, + alg_expectation_refs) { + if (!conn_key_cmp(&alg_exp_node->master_key, master_key) && + !conn_key_cmp(&alg_exp_node->key, alg_exp_key)) { + return alg_exp_node; + } + } + return NULL; +} + +/* This function must be called with the ct->resources write lock taken. */ +static void +expectation_ref_create(struct hindex *alg_expectation_refs, + struct alg_exp_node *alg_exp_node, + uint32_t basis) +{ + if (!expectation_ref_lookup_unique(alg_expectation_refs, + &alg_exp_node->master_key, + &alg_exp_node->key, basis)) { + uint32_t alg_exp_conn_key_hash = + conn_key_hash(&alg_exp_node->master_key, basis); + hindex_insert(alg_expectation_refs, &alg_exp_node->node_ref, + alg_exp_conn_key_hash); + } +} + +/* This function must be called with the ct->resources read lock taken. */ +static struct alg_exp_node * +expectation_ref_lookup(const struct hindex *alg_expectation_refs, + const struct conn_key *master_key, + uint32_t basis) +{ + uint32_t alg_exp_conn_key_hash = conn_key_hash(master_key, basis); + struct hindex_node *hindex_node = + hindex_node_with_hash(alg_expectation_refs, alg_exp_conn_key_hash); + if (hindex_node) { + struct alg_exp_node *alg_exp_node = + CONTAINER_OF(hindex_node, struct alg_exp_node, node_ref); + return alg_exp_node; + } + + return NULL; +} + +static void +expectation_clean(struct conntrack *ct, const struct conn_key *master_key, + uint32_t basis) +{ + ct_rwlock_wrlock(&ct->resources_lock); + struct alg_exp_node *alg_exp_node = + expectation_ref_lookup(&ct->alg_expectation_refs, master_key, basis); + + if (alg_exp_node) { + expectation_remove(&ct->alg_expectations, &alg_exp_node->key, basis); + hindex_remove(&ct->alg_expectation_refs, &alg_exp_node->node_ref); + struct hindex_node *next_hindex_node = + hindex_next_node_with_hash(&alg_exp_node->node_ref); + free(alg_exp_node); + struct hindex_node *hindex_node = next_hindex_node; + + while (hindex_node) { + next_hindex_node = hindex_next_node_with_hash(hindex_node); + struct alg_exp_node *alg_exp_node = + CONTAINER_OF(hindex_node, struct alg_exp_node, node_ref); + expectation_remove(&ct->alg_expectations, &alg_exp_node->key, + basis); + hindex_remove(&ct->alg_expectation_refs, hindex_node); + free(alg_exp_node); + hindex_node = next_hindex_node; + } + } + ct_rwlock_unlock(&ct->resources_lock); +} + static void expectation_create(struct conntrack *ct, ovs_be16 dst_port, - const long long now, enum ct_alg_mode mode, const struct conn *master_conn) { @@ -2636,13 +2702,11 @@ expectation_create(struct conntrack *ct, alg_exp_node->alg_nat_repl_addr = alg_nat_repl_addr; uint32_t alg_exp_conn_key_hash = - conn_key_hash(&alg_exp_node->key, - ct->hash_basis); - hmap_insert(&ct->alg_expectations, - &alg_exp_node->node, + conn_key_hash(&alg_exp_node->key, ct->hash_basis); + hmap_insert(&ct->alg_expectations, &alg_exp_node->node, alg_exp_conn_key_hash); - - alg_exp_init_expiration(ct, alg_exp_node, now); + expectation_ref_create(&ct->alg_expectation_refs, alg_exp_node, + ct->hash_basis); ct_rwlock_unlock(&ct->resources_lock); } @@ -2775,7 +2839,7 @@ static enum ftp_ctl_pkt process_ftp_ctl_v4(struct conntrack *ct, struct dp_packet *pkt, const struct conn *conn_for_expectation, - long long now, ovs_be32 *v4_addr_rep, + ovs_be32 *v4_addr_rep, char **ftp_data_v4_start, size_t *addr_offset_from_ftp_data_start) { @@ -2901,7 +2965,7 @@ process_ftp_ctl_v4(struct conntrack *ct, return CT_FTP_CTL_INVALID; } - expectation_create(ct, port, now, mode, conn_for_expectation); + expectation_create(ct, port, mode, conn_for_expectation); return CT_FTP_CTL_INTEREST; } @@ -2918,7 +2982,6 @@ static enum ftp_ctl_pkt process_ftp_ctl_v6(struct conntrack *ct, struct dp_packet *pkt, const struct conn *conn_for_expectation, - long long now, struct ct_addr *v6_addr_rep, char **ftp_data_start, size_t *addr_offset_from_ftp_data_start, @@ -3004,7 +3067,7 @@ process_ftp_ctl_v6(struct conntrack *ct, OVS_NOT_REACHED(); } - expectation_create(ct, port, now, *mode, conn_for_expectation); + expectation_create(ct, port, *mode, conn_for_expectation); return CT_FTP_CTL_INTEREST; } @@ -3083,12 +3146,12 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, enum ftp_ctl_pkt rc; if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) { rc = process_ftp_ctl_v6(ct, pkt, conn_for_expectation, - now, &v6_addr_rep, &ftp_data_start, + &v6_addr_rep, &ftp_data_start, &addr_offset_from_ftp_data_start, &addr_size, &mode); } else { rc = process_ftp_ctl_v4(ct, pkt, conn_for_expectation, - now, &v4_addr_rep, &ftp_data_start, + &v4_addr_rep, &ftp_data_start, &addr_offset_from_ftp_data_start); } if (rc == CT_FTP_CTL_INVALID) { @@ -3179,10 +3242,10 @@ handle_tftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx OVS_UNUSED, struct dp_packet *pkt OVS_UNUSED, const struct conn *conn_for_expectation, - long long now, enum ftp_ctl_pkt ftp_ctl OVS_UNUSED, - bool nat OVS_UNUSED) + long long now OVS_UNUSED, + enum ftp_ctl_pkt ftp_ctl OVS_UNUSED, bool nat OVS_UNUSED) { - expectation_create(ct, conn_for_expectation->key.src.port, now, - CT_TFTP_MODE, conn_for_expectation); + expectation_create(ct, conn_for_expectation->key.src.port, CT_TFTP_MODE, + conn_for_expectation); return; } diff --git a/lib/conntrack.h b/lib/conntrack.h index 990f6c2..e41b0ce 100644 --- a/lib/conntrack.h +++ b/lib/conntrack.h @@ -28,6 +28,7 @@ #include "ovs-atomic.h" #include "ovs-thread.h" #include "packets.h" +#include "hindex.h" /* Userspace connection tracker * ============================ @@ -271,14 +272,17 @@ struct conntrack { /* Hash table for alg expectations. Expectations are created * by control connections to help create data connections. */ struct hmap alg_expectations OVS_GUARDED; + /* Used to lookup alg expectations from the control context. */ + struct hindex alg_expectation_refs OVS_GUARDED; /* Expiry list for alg expectations. */ struct ovs_list alg_exp_list OVS_GUARDED; /* This lock is used during NAT connection creation and deletion; * it is taken after a bucket lock and given back before that * bucket unlock. * This lock is similarly used to guard alg_expectations and - * alg_exp_list. If a bucket lock is also held during the normal - * code flow, then is must be taken first first and released last. + * alg_expectation_refs. If a bucket lock is also held during + * the normal code flow, then is must be taken first and released + * last. */ struct ct_rwlock resources_lock; From patchwork Mon Jan 8 20:54:27 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Darrell Ball X-Patchwork-Id: 857123 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="NwWavCuM"; dkim-atps=neutral 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 3zFncb2Rhyz9s71 for ; Tue, 9 Jan 2018 07:56:15 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id AF8A8FDB; Mon, 8 Jan 2018 20:55:15 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 1642CE6E for ; Mon, 8 Jan 2018 20:55:13 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg0-f67.google.com (mail-pg0-f67.google.com [74.125.83.67]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 850FD1E1 for ; Mon, 8 Jan 2018 20:55:12 +0000 (UTC) Received: by mail-pg0-f67.google.com with SMTP id r2so6363732pgq.13 for ; Mon, 08 Jan 2018 12:55:12 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references; bh=Tkoo38/anNtmsVYY0bhL+PoO6fl2bnZdUbycHuSWXL4=; b=NwWavCuMQ1s8MdFKuC8MJTaVnNQ7bB5yohtB3venoQkV0wIx5a077MNrkTiURX5jrC wbmn5wfhBpfOTBWApVTZVobz5L8G23baJfigOsCDfQHFs71/VzWXJY0b9UpLGoRaqXo+ D4tVqLcJ17Jjz/EUv+5Mhmcix4cHuXlEoCv1ZkjsrVb3MWzsSvm0L8moLXM+q16V6wWl CeNAQPtjoXxAw6s5Khhz7aKi1CRit6MDmEjaTcYGLKVb2iSppqnFWul4FytlY+nDhULY hFBuvoEM/GgRpGpQkcNzVvA3XGqNWpIkNjbWaY8gknIh3yKATw6PWFou/M3vlKAwalj0 m+Jw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=Tkoo38/anNtmsVYY0bhL+PoO6fl2bnZdUbycHuSWXL4=; b=ifPeD14e2q/iTkKTzQhlKdUQizt0puu6LNjhhkDgYtEyehPvqVl6m8xIPxsmq7orrw aoXQ1V57ljQU+rAhdemgt6iO3oNniTuf9mSeZeGdZalurW4PkY/8ixsV1ztnyXur6dUz lpD9t+s1acWps47b28AwYU2DCHJoL0yvN534E94D1kY66jy6ckn47rlVTUwxEw12mQAV A954sjBx7Ly9b0P9OAS4Twf9NIh3ehM4upaLz1lUS/6BDJkFUqw9g0R3hGpxEFaZGFir 4eRNq7hehyh70iCQPNP820bEZTlUj2bi+7e9FaTWnHXiSRRv2LXMupYle55f3QT7i3fX wbUw== X-Gm-Message-State: AKGB3mKR3a/zcVYiLXuKdVeLvtsSobUfc5aboTdDt3UKmq48OfC5H3FW KvPmzK2Dv2rRAjfue+0TvXk= X-Google-Smtp-Source: ACJfBotaLXrGkb6DMhASbbLNosqRnDOJvSXTuNHL/J5CSIXuZtKhp4RB0bbJNZOitRBXjdXh1LCpRQ== X-Received: by 10.99.117.18 with SMTP id q18mr6183179pgc.71.1515444912176; Mon, 08 Jan 2018 12:55:12 -0800 (PST) Received: from sc9-mailhost1.vmware.com (c-73-162-236-45.hsd1.ca.comcast.net. [73.162.236.45]) by smtp.gmail.com with ESMTPSA id i85sm28379519pfi.54.2018.01.08.12.55.11 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 08 Jan 2018 12:55:11 -0800 (PST) From: Darrell Ball To: dlu998@gmail.com, dev@openvswitch.org Date: Mon, 8 Jan 2018 12:54:27 -0800 Message-Id: <1515444869-21655-3-git-send-email-dlu998@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1515444869-21655-1-git-send-email-dlu998@gmail.com> References: <1515444869-21655-1-git-send-email-dlu998@gmail.com> X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [patch v3 3/5] conntrack: Add additional alg support. 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 In order to support more algs with different requirements, expectation handling is allowed to handle more cases, such as a wildcard source ip as in the case of SIP. NAT can also be skipped in some alg cases. Expectation_create() was otherwise simplified in the process. Some renaming was done to support the above changes. Signed-off-by: Darrell Ball --- lib/conntrack-private.h | 6 ++-- lib/conntrack.c | 89 ++++++++++++++++++++++++++++++------------------- 2 files changed, 58 insertions(+), 37 deletions(-) diff --git a/lib/conntrack-private.h b/lib/conntrack-private.h index 60e2902..a344801 100644 --- a/lib/conntrack-private.h +++ b/lib/conntrack-private.h @@ -82,9 +82,9 @@ struct alg_exp_node { * connection label and mark. */ ovs_u128 master_label; uint32_t master_mark; - /* True if the expectation is for passive mode, as is - * one option for FTP. */ - bool passive_mode; + /* True if for NAT application, the alg replaces the dest address; + * otherwise, the source address is replaced. */ + bool nat_rpl_dst; }; struct conn { diff --git a/lib/conntrack.c b/lib/conntrack.c index 6bb30ce..a036a12 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -71,6 +71,7 @@ enum ct_alg_ctl_type { CT_ALG_CTL_NONE, CT_ALG_CTL_FTP, CT_ALG_CTL_TFTP, + CT_ALG_CTL_SIP, }; static bool conn_key_extract(struct conntrack *, struct dp_packet *, @@ -128,8 +129,8 @@ extract_l3_ipv6(struct conn_key *key, const void *data, size_t size, const char **new_data); static struct alg_exp_node * -expectation_lookup(struct hmap *alg_expectations, - const struct conn_key *key, uint32_t basis); +expectation_lookup(struct hmap *alg_expectations, const struct conn_key *key, + uint32_t basis, bool src_ip_wc); static int repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 v4_addr_rep, @@ -168,7 +169,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, static void handle_tftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx OVS_UNUSED, - struct dp_packet *pkt OVS_UNUSED, + struct dp_packet *pkt, const struct conn *conn_for_expectation, long long now OVS_UNUSED, enum ftp_ctl_pkt ftp_ctl OVS_UNUSED, bool nat OVS_UNUSED); @@ -503,6 +504,15 @@ get_alg_ctl_type(const struct dp_packet *pkt, ovs_be16 tp_src, ovs_be16 tp_dst, return CT_ALG_CTL_NONE; } +static bool +alg_src_ip_wc(enum ct_alg_ctl_type alg_ctl_type) +{ + if (alg_ctl_type == CT_ALG_CTL_SIP) { + return true; + } + return false; +} + static void handle_alg_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, struct dp_packet *pkt, enum ct_alg_ctl_type ct_alg_ctl, @@ -889,7 +899,7 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt, nc->nat_info = xmemdup(nat_action_info, sizeof *nc->nat_info); if (alg_exp) { - if (alg_exp->passive_mode) { + if (alg_exp->nat_rpl_dst) { nc->rev_key.dst.addr = alg_exp->alg_nat_repl_addr; nc->nat_info->nat_action = NAT_ACTION_SRC; } else { @@ -1249,7 +1259,8 @@ process_one(struct conntrack *ct, struct dp_packet *pkt, ct_rwlock_rdlock(&ct->resources_lock); alg_exp = expectation_lookup(&ct->alg_expectations, &ctx->key, - ct->hash_basis); + ct->hash_basis, + alg_src_ip_wc(ct_alg_ctl)); if (alg_exp) { alg_exp_entry = *alg_exp; alg_exp = &alg_exp_entry; @@ -2530,11 +2541,14 @@ conntrack_flush(struct conntrack *ct, const uint16_t *zone) /* This function must be called with the ct->resources read lock taken. */ static struct alg_exp_node * -expectation_lookup(struct hmap *alg_expectations, - const struct conn_key *key, uint32_t basis) +expectation_lookup(struct hmap *alg_expectations, const struct conn_key *key, + uint32_t basis, bool src_ip_wc) { struct conn_key check_key = *key; check_key.src.port = ALG_WC_SRC_PORT; + if (src_ip_wc) { + memset(&check_key.src.addr, 0, sizeof check_key.src.addr); + } struct alg_exp_node *alg_exp_node; uint32_t alg_exp_conn_key_hash = conn_key_hash(&check_key, basis); @@ -2650,33 +2664,38 @@ expectation_clean(struct conntrack *ct, const struct conn_key *master_key, } static void -expectation_create(struct conntrack *ct, - ovs_be16 dst_port, - enum ct_alg_mode mode, - const struct conn *master_conn) +expectation_create(struct conntrack *ct, ovs_be16 dst_port, + const struct conn *master_conn, bool reply, bool src_ip_wc, + bool skip_nat) { struct ct_addr src_addr; struct ct_addr dst_addr; struct ct_addr alg_nat_repl_addr; + struct alg_exp_node *alg_exp_node = xzalloc(sizeof *alg_exp_node); - switch (mode) { - case CT_FTP_MODE_ACTIVE: - case CT_TFTP_MODE: - src_addr = master_conn->rev_key.src.addr; - dst_addr = master_conn->rev_key.dst.addr; - alg_nat_repl_addr = master_conn->key.src.addr; - break; - case CT_FTP_MODE_PASSIVE: + if (reply) { src_addr = master_conn->key.src.addr; dst_addr = master_conn->key.dst.addr; - alg_nat_repl_addr = master_conn->rev_key.dst.addr; - break; - default: - OVS_NOT_REACHED(); + if (skip_nat) { + alg_nat_repl_addr = dst_addr; + } else { + alg_nat_repl_addr = master_conn->rev_key.dst.addr; + } + alg_exp_node->nat_rpl_dst = true; + } else { + src_addr = master_conn->rev_key.src.addr; + dst_addr = master_conn->rev_key.dst.addr; + if (skip_nat) { + alg_nat_repl_addr = src_addr; + } else { + alg_nat_repl_addr = master_conn->key.src.addr; + } + alg_exp_node->nat_rpl_dst = false; + } + if (src_ip_wc) { + memset(&src_addr, 0, sizeof src_addr); } - struct alg_exp_node *alg_exp_node = - xzalloc(sizeof *alg_exp_node); alg_exp_node->key.dl_type = master_conn->key.dl_type; alg_exp_node->key.nw_proto = master_conn->key.nw_proto; alg_exp_node->key.zone = master_conn->key.zone; @@ -2687,13 +2706,12 @@ expectation_create(struct conntrack *ct, alg_exp_node->master_mark = master_conn->mark; alg_exp_node->master_label = master_conn->label; alg_exp_node->master_key = master_conn->key; - alg_exp_node->passive_mode = mode == CT_FTP_MODE_PASSIVE; /* Take the write lock here because it is almost 100% * likely that the lookup will fail and * expectation_create() will be called below. */ ct_rwlock_wrlock(&ct->resources_lock); struct alg_exp_node *alg_exp = expectation_lookup( - &ct->alg_expectations, &alg_exp_node->key, ct->hash_basis); + &ct->alg_expectations, &alg_exp_node->key, ct->hash_basis, src_ip_wc); if (alg_exp) { free(alg_exp_node); ct_rwlock_unlock(&ct->resources_lock); @@ -2701,8 +2719,8 @@ expectation_create(struct conntrack *ct, } alg_exp_node->alg_nat_repl_addr = alg_nat_repl_addr; - uint32_t alg_exp_conn_key_hash = - conn_key_hash(&alg_exp_node->key, ct->hash_basis); + uint32_t alg_exp_conn_key_hash = conn_key_hash(&alg_exp_node->key, + ct->hash_basis); hmap_insert(&ct->alg_expectations, &alg_exp_node->node, alg_exp_conn_key_hash); expectation_ref_create(&ct->alg_expectation_refs, alg_exp_node, @@ -2965,7 +2983,8 @@ process_ftp_ctl_v4(struct conntrack *ct, return CT_FTP_CTL_INVALID; } - expectation_create(ct, port, mode, conn_for_expectation); + expectation_create(ct, port, conn_for_expectation, + !!(pkt->md.ct_state & CS_REPLY_DIR), false, false); return CT_FTP_CTL_INTEREST; } @@ -3067,7 +3086,8 @@ process_ftp_ctl_v6(struct conntrack *ct, OVS_NOT_REACHED(); } - expectation_create(ct, port, *mode, conn_for_expectation); + expectation_create(ct, port, conn_for_expectation, + !!(pkt->md.ct_state & CS_REPLY_DIR), false, false); return CT_FTP_CTL_INTEREST; } @@ -3240,12 +3260,13 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, static void handle_tftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx OVS_UNUSED, - struct dp_packet *pkt OVS_UNUSED, + struct dp_packet *pkt, const struct conn *conn_for_expectation, long long now OVS_UNUSED, enum ftp_ctl_pkt ftp_ctl OVS_UNUSED, bool nat OVS_UNUSED) { - expectation_create(ct, conn_for_expectation->key.src.port, CT_TFTP_MODE, - conn_for_expectation); + expectation_create(ct, conn_for_expectation->key.src.port, + conn_for_expectation, + !!(pkt->md.ct_state & CS_REPLY_DIR), false, false); return; } From patchwork Mon Jan 8 20:54:28 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Darrell Ball X-Patchwork-Id: 857125 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="sAqgTnGK"; dkim-atps=neutral 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 3zFnfG5k3Nz9ryv for ; Tue, 9 Jan 2018 07:57:42 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 29A23FF1; Mon, 8 Jan 2018 20:55:19 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id 0223AEB8 for ; Mon, 8 Jan 2018 20:55:15 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pg0-f68.google.com (mail-pg0-f68.google.com [74.125.83.68]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id BAF84E3 for ; Mon, 8 Jan 2018 20:55:13 +0000 (UTC) Received: by mail-pg0-f68.google.com with SMTP id z20so4102604pgv.6 for ; Mon, 08 Jan 2018 12:55:13 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references; bh=dggAZgBoUVy9Fmtg36pUxL+dTeLsG3dq70hIJr/gMus=; b=sAqgTnGKATUB/9v6QBlcnTpd39EWIgi05VxfqJc0d9hiZycOxUN9kwp5co95FAAOx5 rc2sVw6jI+tb2zsOzRbRgBJxgLy9uuktwxyhvkeNUUfP7zBmOt+M8QerWq+f2YKTLVe3 H+gPvmIzYKZSc2DgGRkdsXMKWeyBbu36lJ64DnpUy6WMh5GOgVf3hQxk1zINMHiiMW99 niP/zb9QFHMUEDx3IACxcK9yaH5fqAdzBhQ/z/V8LOpVKhqhW9KX0rWSIScR0P2ZdqU0 2t6MiTIty4fLUHZArbn1KRdW+Tj5YquwzdHUl6CfMK3u4w+tOjM1330IUzTl7a8qH4Oy Y4hg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=dggAZgBoUVy9Fmtg36pUxL+dTeLsG3dq70hIJr/gMus=; b=tNg7oUSBxjSb8E3SGxnu1gG50WC3EbugoX4mbU5E6xyTiA7vn540cPdab8wK1wFoK3 /l2qpHwMOgBlS8oLKhnjzZngOXP3qbvy/AK1a2IDvsC0Vow9+Nvj3E4qTVsUmUEjRXrl vUmMEbHmWVZ2oOVsYc+TQMwvQDiN60qlhnVLuLeRXSotic8VkRoQiLF+QfaaOWMuv+oi R/rfuuKrx/aeQ7NrDubPuth0ZT+vPEL9mB23/seEEByvjaG1MB9qFXcjJc8s38c9BrQ7 S0kridFrIOaDgJNNoP7snPzGe/PAV1pePwJjEsl0nDJMHHGCxHS1Tt/cyagZRnbhAWI0 qhYQ== X-Gm-Message-State: AKGB3mKUgqgw74KXkyqoMuM2YvZ3haDpeEWdKy94DJYR1l83Cco+9OgO Nj3tkvM/Mpw29addcfQDArticw== X-Google-Smtp-Source: ACJfBosK36tsUibyrJ1yegepuqP4tGBhk8gAkNe1DOe4lBDEvPV5YBi0LzJjseYDDIDNgQG2O6hOHg== X-Received: by 10.84.134.131 with SMTP id 3mr13185983plh.66.1515444913129; Mon, 08 Jan 2018 12:55:13 -0800 (PST) Received: from sc9-mailhost1.vmware.com (c-73-162-236-45.hsd1.ca.comcast.net. [73.162.236.45]) by smtp.gmail.com with ESMTPSA id i85sm28379519pfi.54.2018.01.08.12.55.12 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 08 Jan 2018 12:55:12 -0800 (PST) From: Darrell Ball To: dlu998@gmail.com, dev@openvswitch.org Date: Mon, 8 Jan 2018 12:54:28 -0800 Message-Id: <1515444869-21655-4-git-send-email-dlu998@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1515444869-21655-1-git-send-email-dlu998@gmail.com> References: <1515444869-21655-1-git-send-email-dlu998@gmail.com> X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [patch v3 4/5] conntrack: Some style improvements. 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 Fix up some instances where variable declarations were not close enough to their use, as these were missed before. This is the preferred art in OVS code and flagged heavily in code reviews. This is highly desirable due to code clarity reasons. There are also some cases where newlines were not needed by prior art and some cases where they were needed but missed. There was one case where there was a missing space after "}". There were a few cases where for loop index declarations could be folded into the loop. One function was missing some const qualifiers. Signed-off-by: Darrell Ball --- v2->v3: Incorporate review comments from Flavio. A separate following patch is split out for reordering of ip fragmentation checks. lib/conntrack.c | 144 +++++++++++++++++++++++--------------------------------- 1 file changed, 60 insertions(+), 84 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index a036a12..0902e0e 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -249,8 +249,8 @@ conn_key_cmp(const struct conn_key *key1, const struct conn_key *key2) } static void -ct_print_conn_info(struct conn *c, char *log_msg, enum vlog_level vll, - bool force, bool rl_on) +ct_print_conn_info(const struct conn *c, const char *log_msg, + enum vlog_level vll, bool force, bool rl_on) { #define CT_VLOG(RL_ON, LEVEL, ...) \ do { \ @@ -308,7 +308,6 @@ ct_print_conn_info(struct conn *c, char *log_msg, enum vlog_level vll, void conntrack_init(struct conntrack *ct) { - unsigned i, j; long long now = time_msec(); ct_rwlock_init(&ct->resources_lock); @@ -319,13 +318,13 @@ conntrack_init(struct conntrack *ct) ovs_list_init(&ct->alg_exp_list); ct_rwlock_unlock(&ct->resources_lock); - for (i = 0; i < CONNTRACK_BUCKETS; i++) { + for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) { struct conntrack_bucket *ctb = &ct->buckets[i]; ct_lock_init(&ctb->lock); ct_lock_lock(&ctb->lock); hmap_init(&ctb->connections); - for (j = 0; j < ARRAY_SIZE(ctb->exp_lists); j++) { + for (unsigned j = 0; j < ARRAY_SIZE(ctb->exp_lists); j++) { ovs_list_init(&ctb->exp_lists[j]); } ct_lock_unlock(&ctb->lock); @@ -345,12 +344,10 @@ conntrack_init(struct conntrack *ct) void conntrack_destroy(struct conntrack *ct) { - unsigned i; - latch_set(&ct->clean_thread_exit); pthread_join(ct->clean_thread, NULL); latch_destroy(&ct->clean_thread_exit); - for (i = 0; i < CONNTRACK_BUCKETS; i++) { + for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) { struct conntrack_bucket *ctb = &ct->buckets[i]; struct conn *conn; @@ -415,10 +412,11 @@ write_ct_md(struct dp_packet *pkt, uint16_t zone, const struct conn *conn, pkt->md.ct_label = alg_exp->master_label; key = &alg_exp->master_key; } + pkt->md.ct_orig_tuple_ipv6 = false; + if (key) { if (key->dl_type == htons(ETH_TYPE_IP)) { - pkt->md.ct_orig_tuple.ipv4 = (struct ovs_key_ct_tuple_ipv4) { key->src.addr.ipv4_aligned, key->dst.addr.ipv4_aligned, @@ -650,7 +648,6 @@ reverse_nat_packet(struct dp_packet *pkt, const struct conn *conn) struct ip_header *inner_l3 = (struct ip_header *) (icmp + 1); extract_l3_ipv4(&inner_key, inner_l3, tail - ((char *)inner_l3) - pad, &inner_l4, false); - pkt->l3_ofs += (char *) inner_l3 - (char *) nh; pkt->l4_ofs += inner_l4 - (char *) icmp; @@ -661,6 +658,7 @@ reverse_nat_packet(struct dp_packet *pkt, const struct conn *conn) packet_set_ipv4_addr(pkt, &inner_l3->ip_dst, conn->key.dst.addr.ipv4_aligned); } + reverse_pat_packet(pkt, conn); icmp->icmp_csum = 0; icmp->icmp_csum = csum(icmp, tail - (char *) icmp - pad); @@ -775,20 +773,16 @@ nat_clean(struct conntrack *ct, struct conn *conn, struct conntrack_bucket *ctb) OVS_REQUIRES(ctb->lock) { - long long now = time_msec(); ct_rwlock_wrlock(&ct->resources_lock); nat_conn_keys_remove(&ct->nat_conn_keys, &conn->rev_key, ct->hash_basis); ct_rwlock_unlock(&ct->resources_lock); ct_lock_unlock(&ctb->lock); - uint32_t hash_rev_conn = conn_key_hash(&conn->rev_key, ct->hash_basis); unsigned bucket_rev_conn = hash_to_bucket(hash_rev_conn); - ct_lock_lock(&ct->buckets[bucket_rev_conn].lock); ct_rwlock_wrlock(&ct->resources_lock); - + long long now = time_msec(); struct conn *rev_conn = conn_lookup(ct, &conn->rev_key, now); - struct nat_conn_key_node *nat_conn_key_node = nat_conn_keys_lookup(&ct->nat_conn_keys, &conn->rev_key, ct->hash_basis); @@ -802,8 +796,8 @@ nat_clean(struct conntrack *ct, struct conn *conn, &rev_conn->node); free(rev_conn); } - delete_conn(conn); + delete_conn(conn); ct_rwlock_unlock(&ct->resources_lock); ct_lock_unlock(&ct->buckets[bucket_rev_conn].lock); ct_lock_lock(&ctb->lock); @@ -857,21 +851,21 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt, const struct alg_exp_node *alg_exp, enum ct_alg_ctl_type ct_alg_ctl) { - unsigned bucket = hash_to_bucket(ctx->hash); struct conn *nc = NULL; if (!valid_new(pkt, &ctx->key)) { pkt->md.ct_state = CS_INVALID; return nc; } + pkt->md.ct_state = CS_NEW; + if (alg_exp) { pkt->md.ct_state |= CS_RELATED; } if (commit) { unsigned int n_conn_limit; - atomic_read_relaxed(&ct->n_conn_limit, &n_conn_limit); if (atomic_count_get(&ct->n_conn) >= n_conn_limit) { @@ -879,6 +873,7 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt, return nc; } + unsigned bucket = hash_to_bucket(ctx->hash); nc = new_conn(&ct->buckets[bucket], pkt, &ctx->key, now); ctx->conn = nc; nc->rev_key = nc->key; @@ -922,8 +917,8 @@ conn_not_found(struct conntrack *ct, struct dp_packet *pkt, } else { *conn_for_un_nat_copy = *nc; ct_rwlock_wrlock(&ct->resources_lock); - bool nat_res = nat_select_range_tuple( - ct, nc, conn_for_un_nat_copy); + bool nat_res = nat_select_range_tuple(ct, nc, + conn_for_un_nat_copy); if (!nat_res) { goto nat_res_exhaustion; @@ -982,6 +977,7 @@ conn_update_state(struct conntrack *ct, struct dp_packet *pkt, if ((*conn)->alg_related) { pkt->md.ct_state |= CS_RELATED; } + enum ct_update_res res = conn_update(*conn, &ct->buckets[bucket], pkt, ctx->reply, now); @@ -1037,7 +1033,6 @@ create_un_nat_conn(struct conntrack *ct, struct conn *conn_for_un_nat_copy, nat_conn_keys_lookup(&ct->nat_conn_keys, &nc->key, ct->hash_basis); if (nat_conn_key_node && !conn_key_cmp(&nat_conn_key_node->value, &nc->rev_key) && !rev_conn) { - hmap_insert(&ct->buckets[un_nat_conn_bucket].connections, &nc->node, un_nat_hash); } else { @@ -1128,13 +1123,11 @@ check_orig_tuple(struct conntrack *ct, struct dp_packet *pkt, ctx.key.dl_type = ctx_in->key.dl_type; ctx.key.zone = pkt->md.ct_zone; - ctx.hash = conn_key_hash(&ctx.key, ct->hash_basis); *bucket = hash_to_bucket(ctx.hash); ct_lock_lock(&ct->buckets[*bucket].lock); conn_key_lookup(&ct->buckets[*bucket], &ctx, now); *conn = ctx.conn; - return *conn ? true : false; } @@ -1239,10 +1232,9 @@ process_one(struct conntrack *ct, struct dp_packet *pkt, handle_nat(pkt, conn, zone, ctx->reply, ctx->icmp_related); } - }else if (check_orig_tuple(ct, pkt, ctx, now, &bucket, &conn, + } else if (check_orig_tuple(ct, pkt, ctx, now, &bucket, &conn, nat_action_info)) { - create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now, - bucket); + create_new_conn = conn_update_state(ct, pkt, ctx, &conn, now, bucket); } else { if (ctx->icmp_related) { /* An icmp related conn should always be found; no new @@ -1254,6 +1246,7 @@ process_one(struct conntrack *ct, struct dp_packet *pkt, } const struct alg_exp_node *alg_exp = NULL; + if (OVS_UNLIKELY(create_new_conn)) { struct alg_exp_node alg_exp_entry; @@ -1375,10 +1368,9 @@ sweep_bucket(struct conntrack *ct, struct conntrack_bucket *ctb, { struct conn *conn, *next; long long min_expiration = LLONG_MAX; - unsigned i; size_t count = 0; - for (i = 0; i < N_CT_TM; i++) { + for (unsigned i = 0; i < N_CT_TM; i++) { LIST_FOR_EACH_SAFE (conn, next, exp_node, &ctb->exp_lists[i]) { if (conn->conn_type == CT_CONN_TYPE_DEFAULT) { if (!conn_expired(conn, now) || count >= limit) { @@ -1408,11 +1400,10 @@ conntrack_clean(struct conntrack *ct, long long now) long long next_wakeup = now + CT_TM_MIN; unsigned int n_conn_limit; size_t clean_count = 0; - unsigned i; atomic_read_relaxed(&ct->n_conn_limit, &n_conn_limit); - for (i = 0; i < CONNTRACK_BUCKETS; i++) { + for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) { struct conntrack_bucket *ctb = &ct->buckets[i]; size_t prev_count; long long min_exp; @@ -1483,7 +1474,6 @@ clean_thread_main(void *f_) while (!latch_is_set(&ct->clean_thread_exit)) { long long next_wake; long long now = time_msec(); - next_wake = conntrack_clean(ct, now); if (next_wake < now) { @@ -1510,16 +1500,14 @@ static inline bool extract_l3_ipv4(struct conn_key *key, const void *data, size_t size, const char **new_data, bool validate_checksum) { - const struct ip_header *ip = data; - size_t ip_len; - if (new_data) { if (OVS_UNLIKELY(size < IP_HEADER_LEN)) { return false; } } - ip_len = IP_IHL(ip->ip_ihl_ver) * 4; + const struct ip_header *ip = data; + size_t ip_len = IP_IHL(ip->ip_ihl_ver) * 4; if (new_data) { if (OVS_UNLIKELY(ip_len < IP_HEADER_LEN)) { @@ -1564,11 +1552,10 @@ extract_l3_ipv6(struct conn_key *key, const void *data, size_t size, } } - uint8_t nw_proto = ip6->ip6_nxt; - uint8_t nw_frag = 0; - data = ip6 + 1; size -= sizeof *ip6; + uint8_t nw_proto = ip6->ip6_nxt; + uint8_t nw_frag = 0; if (!parse_ipv6_ext_hdrs(&data, &size, &nw_proto, &nw_frag)) { return false; @@ -1660,12 +1647,11 @@ check_l4_icmp6(const struct conn_key *key, const void *data, size_t size, static inline bool extract_l4_tcp(struct conn_key *key, const void *data, size_t size) { - const struct tcp_header *tcp = data; - if (OVS_UNLIKELY(size < TCP_HEADER_LEN)) { return false; } + const struct tcp_header *tcp = data; key->src.port = tcp->tcp_src; key->dst.port = tcp->tcp_dst; @@ -1676,12 +1662,11 @@ extract_l4_tcp(struct conn_key *key, const void *data, size_t size) static inline bool extract_l4_udp(struct conn_key *key, const void *data, size_t size) { - const struct udp_header *udp = data; - if (OVS_UNLIKELY(size < UDP_HEADER_LEN)) { return false; } + const struct udp_header *udp = data; key->src.port = udp->udp_src; key->dst.port = udp->udp_dst; @@ -1725,12 +1710,12 @@ static inline int extract_l4_icmp(struct conn_key *key, const void *data, size_t size, bool *related) { - const struct icmp_header *icmp = data; - if (OVS_UNLIKELY(size < ICMP_HEADER_LEN)) { return false; } + const struct icmp_header *icmp = data; + switch (icmp->icmp_type) { case ICMP4_ECHO_REQUEST: case ICMP4_ECHO_REPLY: @@ -1757,7 +1742,6 @@ extract_l4_icmp(struct conn_key *key, const void *data, size_t size, const char *l3 = (const char *) (icmp + 1); const char *tail = (const char *) data + size; const char *l4; - bool ok; if (!related) { return false; @@ -1765,7 +1749,7 @@ extract_l4_icmp(struct conn_key *key, const void *data, size_t size, memset(&inner_key, 0, sizeof inner_key); inner_key.dl_type = htons(ETH_TYPE_IP); - ok = extract_l3_ipv4(&inner_key, l3, tail - l3, &l4, false); + bool ok = extract_l3_ipv4(&inner_key, l3, tail - l3, &l4, false); if (!ok) { return false; } @@ -1843,7 +1827,6 @@ extract_l4_icmp6(struct conn_key *key, const void *data, size_t size, const char *l3 = (const char *) icmp6 + 8; const char *tail = (const char *) data + size; const char *l4 = NULL; - bool ok; if (!related) { return false; @@ -1851,7 +1834,7 @@ extract_l4_icmp6(struct conn_key *key, const void *data, size_t size, memset(&inner_key, 0, sizeof inner_key); inner_key.dl_type = htons(ETH_TYPE_IPV6); - ok = extract_l3_ipv6(&inner_key, l3, tail - l3, &l4); + bool ok = extract_l3_ipv6(&inner_key, l3, tail - l3, &l4); if (!ok) { return false; } @@ -1920,8 +1903,6 @@ conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type, const struct eth_header *l2 = dp_packet_eth(pkt); const struct ip_header *l3 = dp_packet_l3(pkt); const char *l4 = dp_packet_l4(pkt); - const char *tail = dp_packet_tail(pkt); - bool ok; memset(ctx, 0, sizeof *ctx); @@ -1963,13 +1944,16 @@ conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type, * we use a sparse representation (miniflow). * */ + const char *tail = dp_packet_tail(pkt); + bool ok; ctx->key.dl_type = dl_type; + if (ctx->key.dl_type == htons(ETH_TYPE_IP)) { - bool hwol_bad_l3_csum = dp_packet_ip_checksum_bad(pkt); + bool hwol_bad_l3_csum = dp_packet_ip_checksum_bad(pkt); if (hwol_bad_l3_csum) { ok = false; } else { - bool hwol_good_l3_csum = dp_packet_ip_checksum_valid(pkt); + bool hwol_good_l3_csum = dp_packet_ip_checksum_valid(pkt); /* Validate the checksum only when hwol is not supported. */ ok = extract_l3_ipv4(&ctx->key, l3, tail - (char *) l3, NULL, !hwol_good_l3_csum); @@ -1980,7 +1964,6 @@ conn_key_extract(struct conntrack *ct, struct dp_packet *pkt, ovs_be16 dl_type, ok = false; } - if (ok) { bool hwol_bad_l4_csum = dp_packet_l4_checksum_bad(pkt); if (!hwol_bad_l4_csum) { @@ -2016,7 +1999,6 @@ static uint32_t conn_key_hash(const struct conn_key *key, uint32_t basis) { uint32_t hsrc, hdst, hash; - hsrc = hdst = basis; hsrc = ct_endpoint_hash_add(hsrc, &key->src); hdst = ct_endpoint_hash_add(hdst, &key->dst); @@ -2035,9 +2017,7 @@ conn_key_hash(const struct conn_key *key, uint32_t basis) static void conn_key_reverse(struct conn_key *key) { - struct ct_endpoint tmp; - - tmp = key->src; + struct ct_endpoint tmp = key->src; key->src = key->dst; key->dst = tmp; } @@ -2062,6 +2042,7 @@ nat_ipv6_addrs_delta(struct in6_addr *ipv6_aligned_min, memcpy(&addr6_64_max_lo, ipv6_max_lo, sizeof addr6_64_max_lo); uint64_t diff; + if (addr6_64_min_hi == addr6_64_max_hi && ntohll(addr6_64_min_lo) <= ntohll(addr6_64_max_lo)) { diff = ntohll(addr6_64_max_lo) - ntohll(addr6_64_min_lo); @@ -2075,6 +2056,7 @@ nat_ipv6_addrs_delta(struct in6_addr *ipv6_aligned_min, * support check, however the practical impact is probably nil. */ diff = 0xfffffffe; } + if (diff > 0xfffffffe) { diff = 0xfffffffe; } @@ -2119,10 +2101,8 @@ nat_range_hash(const struct conn *conn, uint32_t basis) hash = hash_add(hash, (conn->nat_info->max_port << 16) | conn->nat_info->min_port); - hash = ct_endpoint_hash_add(hash, &conn->key.src); hash = ct_endpoint_hash_add(hash, &conn->key.dst); - hash = hash_add(hash, (OVS_FORCE uint32_t) conn->key.dl_type); hash = hash_add(hash, conn->key.nw_proto); hash = hash_add(hash, conn->key.zone); @@ -2143,7 +2123,6 @@ nat_select_range_tuple(struct conntrack *ct, const struct conn *conn, uint16_t min_port; uint16_t max_port; uint16_t first_port; - uint32_t hash = nat_range_hash(conn, ct->hash_basis); if ((conn->nat_info->nat_action & NAT_ACTION_SRC) && @@ -2187,7 +2166,6 @@ nat_select_range_tuple(struct conntrack *ct, const struct conn *conn, * enforcement via max_ct_addr. */ max_ct_addr = conn->nat_info->min_addr; nat_ipv6_addr_increment(&max_ct_addr.ipv6_aligned, deltaa); - address_index = hash % (deltaa + 1); ct_addr.ipv6_aligned = conn->nat_info->min_addr.ipv6_aligned; nat_ipv6_addr_increment(&ct_addr.ipv6_aligned, address_index); @@ -2368,10 +2346,7 @@ static struct conn * new_conn(struct conntrack_bucket *ctb, struct dp_packet *pkt, struct conn_key *key, long long now) { - struct conn *newconn; - - newconn = l4_protos[key->nw_proto]->new_conn(ctb, pkt, now); - + struct conn *newconn = l4_protos[key->nw_proto]->new_conn(ctb, pkt, now); if (newconn) { newconn->key = *key; } @@ -2427,8 +2402,6 @@ static void conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry *entry, long long now, int bkt) { - struct ct_l4_proto *class; - long long expiration; memset(entry, 0, sizeof *entry); conn_key_to_tuple(&conn->key, &entry->tuple_orig); conn_key_to_tuple(&conn->rev_key, &entry->tuple_reply); @@ -2441,10 +2414,10 @@ conn_to_ct_dpif_entry(const struct conn *conn, struct ct_dpif_entry *entry, entry->timestamp.start = 0; entry->timestamp.stop = 0; - expiration = conn->expiration - now; + long long expiration = conn->expiration - now; entry->timeout = (expiration > 0) ? expiration / 1000 : 0; - class = l4_protos[conn->key.nw_proto]; + struct ct_l4_proto *class = l4_protos[conn->key.nw_proto]; if (class->conn_get_protoinfo) { class->conn_get_protoinfo(conn, &entry->protoinfo); } @@ -2462,14 +2435,14 @@ conntrack_dump_start(struct conntrack *ct, struct conntrack_dump *dump, const uint16_t *pzone, int *ptot_bkts) { memset(dump, 0, sizeof(*dump)); + if (pzone) { dump->zone = *pzone; dump->filter_zone = true; } - dump->ct = ct; + dump->ct = ct; *ptot_bkts = CONNTRACK_BUCKETS; - return 0; } @@ -2521,9 +2494,7 @@ conntrack_dump_done(struct conntrack_dump *dump OVS_UNUSED) int conntrack_flush(struct conntrack *ct, const uint16_t *zone) { - unsigned i; - - for (i = 0; i < CONNTRACK_BUCKETS; i++) { + for (unsigned i = 0; i < CONNTRACK_BUCKETS; i++) { struct conn *conn, *next; ct_lock_lock(&ct->buckets[i].lock); @@ -2546,12 +2517,14 @@ expectation_lookup(struct hmap *alg_expectations, const struct conn_key *key, { struct conn_key check_key = *key; check_key.src.port = ALG_WC_SRC_PORT; + if (src_ip_wc) { memset(&check_key.src.addr, 0, sizeof check_key.src.addr); } - struct alg_exp_node *alg_exp_node; + struct alg_exp_node *alg_exp_node; uint32_t alg_exp_conn_key_hash = conn_key_hash(&check_key, basis); + HMAP_FOR_EACH_WITH_HASH (alg_exp_node, node, alg_exp_conn_key_hash, alg_expectations) { @@ -2765,7 +2738,6 @@ repl_ftp_v4_addr(struct dp_packet *pkt, ovs_be32 v4_addr_rep, size_t remain_size = tcp_payload_length(pkt) - addr_offset_from_ftp_data_start; - int overall_delta = 0; char *byte_str = ftp_data_start + addr_offset_from_ftp_data_start; @@ -2834,9 +2806,9 @@ static enum ftp_ctl_pkt detect_ftp_ctl_type(const struct conn_lookup_ctx *ctx, struct dp_packet *pkt) { - char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0}; get_ftp_ctl_msg(pkt, ftp_msg); + if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) { if (strncasecmp(ftp_msg, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD)) && !strcasestr(ftp_msg, FTP_EPSV_REPLY)) { @@ -2867,9 +2839,9 @@ process_ftp_ctl_v4(struct conntrack *ct, *ftp_data_v4_start = tcp_hdr + tcp_hdr_len; char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0}; get_ftp_ctl_msg(pkt, ftp_msg); - char *ftp = ftp_msg; enum ct_alg_mode mode; + if (!strncasecmp(ftp, FTP_PORT_CMD, strlen(FTP_PORT_CMD))) { ftp = ftp_msg + strlen(FTP_PORT_CMD); mode = CT_FTP_MODE_ACTIVE; @@ -2892,8 +2864,8 @@ process_ftp_ctl_v4(struct conntrack *ct, char *ip_addr_start = ftp; *addr_offset_from_ftp_data_start = ip_addr_start - ftp_msg; - uint8_t comma_count = 0; + uint8_t comma_count = 0; while (comma_count < 4 && *ftp) { if (*ftp == ',') { comma_count++; @@ -2957,6 +2929,7 @@ process_ftp_ctl_v4(struct conntrack *ct, if (65535 - port_hs < port_lo_hs) { return CT_FTP_CTL_INVALID; } + port_hs |= port_lo_hs; ovs_be16 port = htons(port_hs); ovs_be32 conn_ipv4_addr; @@ -3010,12 +2983,11 @@ process_ftp_ctl_v6(struct conntrack *ct, size_t tcp_hdr_len = TCP_OFFSET(th->tcp_ctl) * 4; char *tcp_hdr = (char *) th; char ftp_msg[LARGEST_FTP_MSG_OF_INTEREST + 1] = {0}; - get_ftp_ctl_msg(pkt, ftp_msg); *ftp_data_start = tcp_hdr + tcp_hdr_len; - char *ftp = ftp_msg; struct in6_addr ip6_addr; + if (!strncasecmp(ftp, FTP_EPRT_CMD, strlen(FTP_EPRT_CMD))) { ftp = ftp_msg + strlen(FTP_EPRT_CMD); ftp = skip_non_digits(ftp); @@ -3025,8 +2997,8 @@ process_ftp_ctl_v6(struct conntrack *ct, /* Jump over delimiter. */ ftp += 2; - char *ip_addr_start = ftp; memset(&ip6_addr, 0, sizeof ip6_addr); + char *ip_addr_start = ftp; *addr_offset_from_ftp_data_start = ip_addr_start - ftp_msg; ftp = skip_ipv6_digits(ftp); *ftp = 0; @@ -3056,6 +3028,7 @@ process_ftp_ctl_v6(struct conntrack *ct, if (!ftp) { return CT_FTP_CTL_INVALID; } + int value; if (!str_to_int(save_ftp, 10, &value)) { return CT_FTP_CTL_INVALID; @@ -3160,6 +3133,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, struct ovs_16aligned_ip6_hdr *nh6 = dp_packet_l3(pkt); int64_t seq_skew = 0; + if (ftp_ctl == CT_FTP_CTL_OTHER) { seq_skew = conn_for_expectation->seq_skew; } else if (ftp_ctl == CT_FTP_CTL_INTEREST) { @@ -3181,6 +3155,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, return; } else if (rc == CT_FTP_CTL_INTEREST) { uint16_t ip_len; + if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) { seq_skew = repl_ftp_v6_addr(pkt, v6_addr_rep, ftp_data_start, addr_offset_from_ftp_data_start, @@ -3213,6 +3188,7 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, } struct tcp_header *th = dp_packet_l4(pkt); + if (do_seq_skew_adj && seq_skew != 0) { if (ctx->reply != conn_for_expectation->seq_skew_dir) { @@ -3243,8 +3219,6 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, } } - const char *tail = dp_packet_tail(pkt); - uint8_t pad = dp_packet_l2_pad_size(pkt); th->tcp_csum = 0; uint32_t tcp_csum; if (ctx->key.dl_type == htons(ETH_TYPE_IPV6)) { @@ -3252,6 +3226,8 @@ handle_ftp_ctl(struct conntrack *ct, const struct conn_lookup_ctx *ctx, } else { tcp_csum = packet_csum_pseudoheader(l3_hdr); } + const char *tail = dp_packet_tail(pkt); + uint8_t pad = dp_packet_l2_pad_size(pkt); th->tcp_csum = csum_finish( csum_continue(tcp_csum, th, tail - (char *) th - pad)); return; From patchwork Mon Jan 8 20:54:29 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Darrell Ball X-Patchwork-Id: 857124 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=pass (mailfrom) smtp.mailfrom=openvswitch.org (client-ip=140.211.169.12; helo=mail.linuxfoundation.org; envelope-from=ovs-dev-bounces@openvswitch.org; receiver=) Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="hRMfRXS3"; dkim-atps=neutral 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 3zFndJ3ny6z9s71 for ; Tue, 9 Jan 2018 07:56:52 +1100 (AEDT) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id EB6CB103A; Mon, 8 Jan 2018 20:55:16 +0000 (UTC) X-Original-To: dev@openvswitch.org Delivered-To: ovs-dev@mail.linuxfoundation.org Received: from smtp1.linuxfoundation.org (smtp1.linux-foundation.org [172.17.192.35]) by mail.linuxfoundation.org (Postfix) with ESMTPS id C6EA0F11 for ; Mon, 8 Jan 2018 20:55:14 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-pf0-f194.google.com (mail-pf0-f194.google.com [209.85.192.194]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 9667F3D0 for ; Mon, 8 Jan 2018 20:55:14 +0000 (UTC) Received: by mail-pf0-f194.google.com with SMTP id e11so3127188pff.6 for ; Mon, 08 Jan 2018 12:55:14 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:subject:date:message-id:in-reply-to:references; bh=ZTX1gkqHrkSICck+vmavSoC8grnUggtYi02Ikmma4RA=; b=hRMfRXS3PoFAGw/9UamU0E/3akO1KCiHZ7rOasjMyBOwZsa+aSoUyYQHxUQh6jY3Pn 1mpnlpZiDndmNjGMp1DFf64qnr2pNmxNgGwmJwlO8q3jFCGHrQa1UqXCO3NRaF3x8TF5 15nenA7HOPgz3Pzm8JiGCnIWFFxEfxzRh3v/a4wKg56COueYtAgwgVfgY9kLhJLw0kBB K+pMy+GPFGeIcN/5eXP3ANl0muhuJkXJhA6HprvSWaoF85BgYMmpYTgXP9S/2Gm8Ughp CX/PDna2zoyPJKy5LsMTSsjv9yIqRUXXHf3dVyzzMZ3z0lbTB0hLaaWDiDZ1JUpzrLm2 OMXg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=ZTX1gkqHrkSICck+vmavSoC8grnUggtYi02Ikmma4RA=; b=timO2Ops484e+TavCOIVOyUI/fM9I+n9Y7a1M9DZU7JQx5lpKS6wEDt6mLQTpkhn3P UhHGVyA8EwhPd/Q5a4ftDtvaotHmZl+nTjG0ZSBzilRK4o/cWc+bRgR6Cz15veMtw+Nw gk1HMl2cQHML2GHvWKeRkew9CO5KZ0WsZWByI8IlZm5F2t0tNNBUpH/BPZ/c912xrJOq RR2A6+krVFu67/1ROVYxLjj5CY1XCA6NRLA2zz7Vv9Vhq0m/JVXugfqJkdJfiU/ZsXk0 8XHaQ9P2Lm6lx41tMMAHYHOWy398QVnv8Gpt2UHT8oa2uSRmR4Oy82N44xU3MAQq43Cj KT1w== X-Gm-Message-State: AKGB3mLia+AXhWn4VLQfaiiZB1MSElCjbAZvC9PgMxm9zKH+i1Z5DsOj kaXjsKL9yp6lQM4VgAeUQLs= X-Google-Smtp-Source: ACJfBouOEu/ucdl73YMlbeC3u9kmobnhRuZbuFO72JId26qJ+zhgDcVzlbRWvq72HyZYDIyQs7G+0A== X-Received: by 10.84.128.106 with SMTP id 97mr9222493pla.73.1515444914231; Mon, 08 Jan 2018 12:55:14 -0800 (PST) Received: from sc9-mailhost1.vmware.com (c-73-162-236-45.hsd1.ca.comcast.net. [73.162.236.45]) by smtp.gmail.com with ESMTPSA id i85sm28379519pfi.54.2018.01.08.12.55.13 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Mon, 08 Jan 2018 12:55:13 -0800 (PST) From: Darrell Ball To: dlu998@gmail.com, dev@openvswitch.org Date: Mon, 8 Jan 2018 12:54:29 -0800 Message-Id: <1515444869-21655-5-git-send-email-dlu998@gmail.com> X-Mailer: git-send-email 1.9.1 In-Reply-To: <1515444869-21655-1-git-send-email-dlu998@gmail.com> References: <1515444869-21655-1-git-send-email-dlu998@gmail.com> X-Spam-Status: No, score=-1.7 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=no version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [patch v3 5/5] conntrack: Reorder sanity checks in extract_l3_ipvx(). 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 The functions extract_l3_ipv4 and extract_l3_ipv6 check for unsupported ip fragments and return early. The checks were after an assignment that would not be needed when early return happens. This is slightly inefficient, but mostly reads poorly. Hence, reorder the ip fragment checks before the assignments. Signed-off-by: Darrell Ball --- lib/conntrack.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/conntrack.c b/lib/conntrack.c index 0902e0e..a068910 100644 --- a/lib/conntrack.c +++ b/lib/conntrack.c @@ -1517,11 +1517,11 @@ extract_l3_ipv4(struct conn_key *key, const void *data, size_t size, return false; } - *new_data = (char *) data + ip_len; - } + if (IP_IS_FRAGMENT(ip->ip_frag_off)) { + return false; + } - if (IP_IS_FRAGMENT(ip->ip_frag_off)) { - return false; + *new_data = (char *) data + ip_len; } if (validate_checksum && csum(data, ip_len) != 0) { @@ -1561,14 +1561,14 @@ extract_l3_ipv6(struct conn_key *key, const void *data, size_t size, return false; } - if (new_data) { - *new_data = data; - } - if (nw_frag) { return false; } + if (new_data) { + *new_data = data; + } + key->src.addr.ipv6 = ip6->ip6_src; key->dst.addr.ipv6 = ip6->ip6_dst; key->nw_proto = nw_proto;