Skip to content

Commit

Permalink
datapath: Add support for conntrack timeout policy
Browse files Browse the repository at this point in the history 
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 <[email protected]>
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 <[email protected]>
    CC: Pablo Neira Ayuso <[email protected]>
    Signed-off-by: Yi-Hung Wei <[email protected]>
    Acked-by: Pravin B Shelar <[email protected]>
    Signed-off-by: David S. Miller <[email protected]>

commit 6d670497e01803b486aa72cc1a718401ab986896
Author: Dan Carpenter <[email protected]>
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 <[email protected]>
    Acked-by: Yi-Hung Wei <[email protected]>
    Signed-off-by: David S. Miller <[email protected]>

Signed-off-by: Yi-Hung Wei <[email protected]>
Signed-off-by: Justin Pettit <[email protected]>
  • Loading branch information
@YiHungWei @justinpettit
YiHungWei authored and justinpettit committed Sep 26, 2019
1 parent 2fc8309 commit ebe62ec
Show file tree
Hide file tree
Showing 5 changed files with 63 additions and 5 deletions.
30 changes: 29 additions & 1 deletion datapath/conntrack.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <net/netfilter/nf_conntrack_helper.h>
#include <net/netfilter/nf_conntrack_labels.h>
#include <net/netfilter/nf_conntrack_seqadj.h>
#include <net/netfilter/nf_conntrack_timeout.h>
#include <net/netfilter/nf_conntrack_zones.h>
#include <net/netfilter/ipv6/nf_defrag_ipv6.h>
#include <net/ipv6_frag.h>
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1524,6 +1526,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,
Expand Down Expand Up @@ -1609,6 +1613,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)",
Expand Down Expand Up @@ -1690,6 +1703,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)
Expand Down Expand Up @@ -1814,6 +1835,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))
Expand All @@ -1835,8 +1860,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)
Expand Down
4 changes: 4 additions & 0 deletions datapath/linux/compat/include/linux/openvswitch.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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
};

Expand Down
4 changes: 4 additions & 0 deletions lib/dpif-netdev.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7223,6 +7223,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;
Expand Down
29 changes: 25 additions & 4 deletions lib/odp-util.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
Expand All @@ -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,");
Expand All @@ -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);
}
Expand Down Expand Up @@ -1910,8 +1917,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;
Expand Down Expand Up @@ -1988,6 +1995,16 @@ parse_conntrack_action(const char *s_, struct ofpbuf *actions)
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) {
Expand Down Expand Up @@ -2028,6 +2045,10 @@ parse_conntrack_action(const char *s_, struct ofpbuf *actions)
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);
}
Expand Down
1 change: 1 addition & 0 deletions tests/odp.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down

0 comments on commit ebe62ec

Please sign in to comment.