From patchwork Thu Aug 1 22:07:31 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Yi-Hung Wei X-Patchwork-Id: 1140794 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; dmarc=fail (p=none dis=none) header.from=gmail.com Authentication-Results: ozlabs.org; dkim=fail reason="signature verification failed" (2048-bit key; unprotected) header.d=gmail.com header.i=@gmail.com header.b="tdqqMWFF"; 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 4604Jh6jTqz9s3Z for ; Fri, 2 Aug 2019 08:11:48 +1000 (AEST) Received: from mail.linux-foundation.org (localhost [127.0.0.1]) by mail.linuxfoundation.org (Postfix) with ESMTP id 037F91701; Thu, 1 Aug 2019 22:07:57 +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 A8EFA1689 for ; Thu, 1 Aug 2019 22:07:55 +0000 (UTC) X-Greylist: whitelisted by SQLgrey-1.7.6 Received: from mail-wr1-f43.google.com (mail-wr1-f43.google.com [209.85.221.43]) by smtp1.linuxfoundation.org (Postfix) with ESMTPS id 3C12182B for ; Thu, 1 Aug 2019 22:07:54 +0000 (UTC) Received: by mail-wr1-f43.google.com with SMTP id f9so75116215wre.12 for ; Thu, 01 Aug 2019 15:07:54 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references; bh=A8L9182TgerUr6TVkr6ePfyOinwZJ2JU6eS/zvKBuL8=; b=tdqqMWFFmbj2XJ+vO2lBIDMbsU87gC0/ZB6Tn15JAAiX4Cbneqw2SkpuOHK8ocNyo8 eTMq/sZH+mw/bNSj0rOuq0QfeacPt+qkOH7s1BgFsDZNJQ+AsD2AjIo6lb1OosDmrAIq qWRtSSSRRDw1/IZOvdF3xYfsXfMQ6bjerhBT+/w2ldHzF8isZ/Q4M/kRiPWgKrTxZ5iw uxNXGp6xUNi1jE2MSjKs5qOvnUZjtgUFALagOpyKxL7VcS7EvSzHID3durQstL5RhoUB MymtTufF7GiuHoxyX7yTi/P5r7AIbyTp/1xghPc200tgM5Bj+QM8jQ2Yt1LXm0CyaxwN QN7w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references; bh=A8L9182TgerUr6TVkr6ePfyOinwZJ2JU6eS/zvKBuL8=; b=NWwD0pHL1bmP8AN6an90ExOxXV4wSJk2/5KmgB2P3D8zQv82nlzCXH5Px6s4Ok5D+B tw7kyxRxUSHk50O404BsSk+NbbRDeu2j+BRKAvv9e3zvB9f5nNSIoha+HF0BmusAi7zf GiNAYZ87I35mjdeblswRy60mPng5J3jP7RWsa2kIGwhZaZgEE4eHuOGpv3iuyZvfXlcJ Xw28AHTe+iO7FG13e9X1WZygb4jeADk9ldmmjG0k85s/vUHfTzYMi0U66lVCJREoI2cV TG+2qvwqkYk5L/fcIJ+w7ipgnZwYVq9Zkr1Y/nrMIqsJqffLz68Bz6ZjxKI+UDR1XVuC VSLQ== X-Gm-Message-State: APjAAAWqZiGNwMmCt20jhJTtzipEJMPRzPSw4IelTzsEtz4+FhUzr+Br E3n6eKMbpXfBHa+BejNcWfuHllnv X-Google-Smtp-Source: APXvYqwZmPv9Eqs96Bf+GKUBOGpAu2SGNdXPkj8APvC43rQT/qSULySAF65w61ok1XS5DlOHrXZzjA== X-Received: by 2002:a5d:6287:: with SMTP id k7mr22941339wru.108.1564697272447; Thu, 01 Aug 2019 15:07:52 -0700 (PDT) Received: from vm-main.eng.vmware.com ([66.170.99.1]) by smtp.gmail.com with ESMTPSA id z19sm54128982wmi.7.2019.08.01.15.07.50 (version=TLS1_2 cipher=ECDHE-RSA-AES128-SHA bits=128/128); Thu, 01 Aug 2019 15:07:51 -0700 (PDT) From: Yi-Hung Wei To: dev@openvswitch.org Date: Thu, 1 Aug 2019 15:07:31 -0700 Message-Id: <1564697253-37992-8-git-send-email-yihung.wei@gmail.com> X-Mailer: git-send-email 2.7.4 In-Reply-To: <1564697253-37992-1-git-send-email-yihung.wei@gmail.com> References: <1564697253-37992-1-git-send-email-yihung.wei@gmail.com> X-Spam-Status: No, score=-2.0 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, FREEMAIL_FROM, RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.1 X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on smtp1.linux-foundation.org Subject: [ovs-dev] [PATCH v2 7/9] datapath: Add support for conntrack timeout policy 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 This patch adds support for specifying a timeout policy for a connection in connection tracking system in kernel datapath. The timeout policy will be attached to a connection when the connection is committed to conntrack. This patch introduces a new odp field OVS_CT_ATTR_TIMEOUT in the ct action that specifies the timeout policy in the datapath. In the following patch, during the upcall process, the vswitchd will use the ct_zone to look up the corresponding timeout policy and fill OVS_CT_ATTR_TIMEOUT if it is available. The datapath code is from the following two net-next upstream commits. Upstream commit: commit 06bd2bdf19d2f3d22731625e1a47fa1dff5ac407 Author: Yi-Hung Wei Date: Tue Mar 26 11:31:14 2019 -0700 openvswitch: Add timeout support to ct action Add support for fine-grain timeout support to conntrack action. The new OVS_CT_ATTR_TIMEOUT attribute of the conntrack action specifies a timeout to be associated with this connection. If no timeout is specified, it acts as is, that is the default timeout for the connection will be automatically applied. Example usage: $ nfct timeout add timeout_1 inet tcp syn_sent 100 established 200 $ ovs-ofctl add-flow br0 in_port=1,ip,tcp,action=ct(commit,timeout=timeout_1) CC: Pravin Shelar CC: Pablo Neira Ayuso Signed-off-by: Yi-Hung Wei Acked-by: Pravin B Shelar Signed-off-by: David S. Miller commit 6d670497e01803b486aa72cc1a718401ab986896 Author: Dan Carpenter Date: Tue Apr 2 09:53:14 2019 +0300 openvswitch: use after free in __ovs_ct_free_action() We free "ct_info->ct" and then use it on the next line when we pass it to nf_ct_destroy_timeout(). This patch swaps the order to avoid the use after free. Fixes: 06bd2bdf19d2 ("openvswitch: Add timeout support to ct action") Signed-off-by: Dan Carpenter Acked-by: Yi-Hung Wei Signed-off-by: David S. Miller Signed-off-by: Yi-Hung Wei --- datapath/conntrack.c | 30 ++++++++++++++++++++++- datapath/linux/compat/include/linux/openvswitch.h | 4 +++ lib/dpif-netdev.c | 4 +++ lib/odp-util.c | 29 +++++++++++++++++++--- tests/odp.at | 1 + 5 files changed, 63 insertions(+), 5 deletions(-) diff --git a/datapath/conntrack.c b/datapath/conntrack.c index 292febb3c83e..f85d0a2572f6 100644 --- a/datapath/conntrack.c +++ b/datapath/conntrack.c @@ -29,6 +29,7 @@ #include #include #include +#include #include #include #include @@ -86,6 +87,7 @@ struct ovs_conntrack_info { u32 eventmask; /* Mask of 1 << IPCT_*. */ struct md_mark mark; struct md_labels labels; + char timeout[CTNL_TIMEOUT_NAME_MAX]; #ifdef CONFIG_NF_NAT_NEEDED struct nf_nat_range2 range; /* Only present for SRC NAT and DST NAT. */ #endif @@ -1519,6 +1521,8 @@ static const struct ovs_ct_len_tbl ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = { #endif [OVS_CT_ATTR_EVENTMASK] = { .minlen = sizeof(u32), .maxlen = sizeof(u32) }, + [OVS_CT_ATTR_TIMEOUT] = { .minlen = 1, + .maxlen = CTNL_TIMEOUT_NAME_MAX }, }; static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info, @@ -1604,6 +1608,15 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info, info->have_eventmask = true; info->eventmask = nla_get_u32(a); break; +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT + case OVS_CT_ATTR_TIMEOUT: + memcpy(info->timeout, nla_data(a), nla_len(a)); + if (!memchr(info->timeout, '\0', nla_len(a))) { + OVS_NLERR(log, "Invalid conntrack helper"); + return -EINVAL; + } + break; +#endif default: OVS_NLERR(log, "Unknown conntrack attr (%d)", @@ -1685,6 +1698,14 @@ int ovs_ct_copy_action(struct net *net, const struct nlattr *attr, OVS_NLERR(log, "Failed to allocate conntrack template"); return -ENOMEM; } + + if (ct_info.timeout[0]) { + if (nf_ct_set_timeout(net, ct_info.ct, family, key->ip.proto, + ct_info.timeout)) + pr_info_ratelimited("Failed to associated timeout " + "policy `%s'\n", ct_info.timeout); + } + if (helper) { err = ovs_ct_add_helper(&ct_info, helper, key, log); if (err) @@ -1809,6 +1830,10 @@ int ovs_ct_action_to_attr(const struct ovs_conntrack_info *ct_info, if (ct_info->have_eventmask && nla_put_u32(skb, OVS_CT_ATTR_EVENTMASK, ct_info->eventmask)) return -EMSGSIZE; + if (ct_info->timeout[0]) { + if (nla_put_string(skb, OVS_CT_ATTR_TIMEOUT, ct_info->timeout)) + return -EMSGSIZE; + } #ifdef CONFIG_NF_NAT_NEEDED if (ct_info->nat && !ovs_ct_nat_to_attr(ct_info, skb)) @@ -1830,8 +1855,11 @@ static void __ovs_ct_free_action(struct ovs_conntrack_info *ct_info) { if (ct_info->helper) nf_conntrack_helper_put(ct_info->helper); - if (ct_info->ct) + if (ct_info->ct) { + if (ct_info->timeout[0]) + nf_ct_destroy_timeout(ct_info->ct); nf_ct_tmpl_free(ct_info->ct); + } } #if IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT) diff --git a/datapath/linux/compat/include/linux/openvswitch.h b/datapath/linux/compat/include/linux/openvswitch.h index 65a003a62cf5..7b16b1d5bfe0 100644 --- a/datapath/linux/compat/include/linux/openvswitch.h +++ b/datapath/linux/compat/include/linux/openvswitch.h @@ -801,6 +801,7 @@ struct ovs_action_push_tnl { * be received on NFNLGRP_CONNTRACK_NEW and NFNLGRP_CONNTRACK_DESTROY groups, * respectively. Remaining bits control the changes for which an event is * delivered on the NFNLGRP_CONNTRACK_UPDATE group. + * @OVS_CT_ATTR_TIMEOUT: Variable length string defining conntrack timeout. */ enum ovs_ct_attr { OVS_CT_ATTR_UNSPEC, @@ -813,6 +814,9 @@ enum ovs_ct_attr { OVS_CT_ATTR_NAT, /* Nested OVS_NAT_ATTR_* */ OVS_CT_ATTR_FORCE_COMMIT, /* No argument */ OVS_CT_ATTR_EVENTMASK, /* u32 mask of IPCT_* events. */ + OVS_CT_ATTR_TIMEOUT, /* Associate timeout with this connection for + * fine-grain timeout tuning. */ + __OVS_CT_ATTR_MAX }; diff --git a/lib/dpif-netdev.c b/lib/dpif-netdev.c index 2079e368fb52..7240a3e6f3c8 100644 --- a/lib/dpif-netdev.c +++ b/lib/dpif-netdev.c @@ -7204,6 +7204,10 @@ dp_execute_cb(void *aux_, struct dp_packet_batch *packets_, /* Silently ignored, as userspace datapath does not generate * netlink events. */ break; + case OVS_CT_ATTR_TIMEOUT: + /* Userspace datapath does not support customized timeout + * policy yet. */ + break; case OVS_CT_ATTR_NAT: { const struct nlattr *b_nest; unsigned int left_nest; diff --git a/lib/odp-util.c b/lib/odp-util.c index 84ea4c148f11..28c3209031ce 100644 --- a/lib/odp-util.c +++ b/lib/odp-util.c @@ -930,6 +930,8 @@ static const struct nl_policy ovs_conntrack_policy[] = { [OVS_CT_ATTR_HELPER] = { .type = NL_A_STRING, .optional = true, .min_len = 1, .max_len = 16 }, [OVS_CT_ATTR_NAT] = { .type = NL_A_UNSPEC, .optional = true }, + [OVS_CT_ATTR_TIMEOUT] = { .type = NL_A_STRING, .optional = true, + .min_len = 1, .max_len = 32 }, }; static void @@ -941,7 +943,7 @@ format_odp_conntrack_action(struct ds *ds, const struct nlattr *attr) ovs_32aligned_u128 mask; } *label; const uint32_t *mark; - const char *helper; + const char *helper, *timeout; uint16_t zone; bool commit, force; const struct nlattr *nat; @@ -957,10 +959,12 @@ format_odp_conntrack_action(struct ds *ds, const struct nlattr *attr) 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; helper = a[OVS_CT_ATTR_HELPER] ? nl_attr_get(a[OVS_CT_ATTR_HELPER]) : NULL; + timeout = a[OVS_CT_ATTR_TIMEOUT] ? + nl_attr_get(a[OVS_CT_ATTR_TIMEOUT]) : NULL; nat = a[OVS_CT_ATTR_NAT]; ds_put_format(ds, "ct"); - if (commit || force || zone || mark || label || helper || nat) { + if (commit || force || zone || mark || label || helper || timeout || nat) { ds_put_cstr(ds, "("); if (commit) { ds_put_format(ds, "commit,"); @@ -983,6 +987,9 @@ format_odp_conntrack_action(struct ds *ds, const struct nlattr *attr) if (helper) { ds_put_format(ds, "helper=%s,", helper); } + if (timeout) { + ds_put_format(ds, "timeout=%s", timeout); + } if (nat) { format_odp_ct_nat(ds, nat); } @@ -1909,8 +1916,8 @@ parse_conntrack_action(const char *s_, struct ofpbuf *actions) const char *s = s_; if (ovs_scan(s, "ct")) { - const char *helper = NULL; - size_t helper_len = 0; + const char *helper = NULL, *timeout = NULL; + size_t helper_len = 0, timeout_len = 0; bool commit = false; bool force_commit = false; uint16_t zone = 0; @@ -1987,6 +1994,16 @@ find_end: s += helper_len; continue; } + if (ovs_scan(s, "timeout=%n", &n)) { + s += n; + timeout_len = strcspn(s, delimiters_end); + if (!timeout_len || timeout_len > 31) { + return -EINVAL; + } + timeout = s; + s += timeout_len; + continue; + } n = scan_ct_nat(s, &nat_params); if (n > 0) { @@ -2027,6 +2044,10 @@ find_end: nl_msg_put_string__(actions, OVS_CT_ATTR_HELPER, helper, helper_len); } + if (timeout) { + nl_msg_put_string__(actions, OVS_CT_ATTR_TIMEOUT, timeout, + timeout_len); + } if (have_nat) { nl_msg_put_ct_nat(&nat_params, actions); } diff --git a/tests/odp.at b/tests/odp.at index 8e4ba4615548..3ab9ad62dda2 100644 --- a/tests/odp.at +++ b/tests/odp.at @@ -345,6 +345,7 @@ ct(commit,mark=0xa0a0a0a0/0xfefefefe) ct(commit,label=0x1234567890abcdef1234567890abcdef/0xf1f2f3f4f5f6f7f8f9f0fafbfcfdfeff) ct(commit,helper=ftp) ct(commit,helper=tftp) +ct(commit,timeout=ovs_tp_1_tcp4) ct(nat) ct(commit,nat(src)) ct(commit,nat(dst))