From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, INCLUDES_PATCH,MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id E5B1AC43381 for ; Thu, 21 Mar 2019 04:40:05 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id AD4A7218AE for ; Thu, 21 Mar 2019 04:40:05 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725950AbfCUEjf (ORCPT ); Thu, 21 Mar 2019 00:39:35 -0400 Received: from relay1-d.mail.gandi.net ([217.70.183.193]:40909 "EHLO relay1-d.mail.gandi.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725899AbfCUEjf (ORCPT ); Thu, 21 Mar 2019 00:39:35 -0400 X-Originating-IP: 209.85.217.47 Received: from mail-vs1-f47.google.com (mail-vs1-f47.google.com [209.85.217.47]) (Authenticated sender: pshelar@ovn.org) by relay1-d.mail.gandi.net (Postfix) with ESMTPSA id 60E8D240005 for ; Thu, 21 Mar 2019 04:39:32 +0000 (UTC) Received: by mail-vs1-f47.google.com with SMTP id z25so2933518vsk.8 for ; Wed, 20 Mar 2019 21:39:32 -0700 (PDT) X-Gm-Message-State: APjAAAV6w7pl70kPe+WPVXZr4Gc3aNNwBI6zJJvIHW/teI4MryU9WpxK qa8OCPI427I1AY1i8phhLkAlPYW9WZaObSMb8Vw= X-Google-Smtp-Source: APXvYqwmHvFJ2ujyA+g2hVyg3QZ5/fIV6Ew91eAu/8HXUavKGTHtKwWBI27BGMDG6j4fFFgO7ED00xuQyI2B3rBOvIM= X-Received: by 2002:a05:6102:190:: with SMTP id r16mr784192vsq.58.1553143170831; Wed, 20 Mar 2019 21:39:30 -0700 (PDT) MIME-Version: 1.0 References: <1553116295-16359-1-git-send-email-yihung.wei@gmail.com> <1553116295-16359-2-git-send-email-yihung.wei@gmail.com> In-Reply-To: <1553116295-16359-2-git-send-email-yihung.wei@gmail.com> From: Pravin Shelar Date: Wed, 20 Mar 2019 21:39:19 -0700 X-Gmail-Original-Message-ID: Message-ID: Subject: Re: [PATCH net-next 2/2] openvswitch: Add timeout support to ct action To: Yi-Hung Wei Cc: Linux Kernel Network Developers Content-Type: text/plain; charset="UTF-8" Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On Wed, Mar 20, 2019 at 2:19 PM Yi-Hung Wei wrote: > > 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 > Signed-off-by: Yi-Hung Wei > --- > include/uapi/linux/openvswitch.h | 3 ++ > net/openvswitch/conntrack.c | 81 +++++++++++++++++++++++++++++++++++++++- > 2 files changed, 83 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h > index dbe0cbe4f1b7..9bccc6b9ed3d 100644 > --- a/include/uapi/linux/openvswitch.h > +++ b/include/uapi/linux/openvswitch.h > @@ -734,6 +734,7 @@ struct ovs_action_hash { > * 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, > @@ -746,6 +747,8 @@ 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/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c > index 1b6896896fff..10a2c73f22f2 100644 > --- a/net/openvswitch/conntrack.c > +++ b/net/openvswitch/conntrack.c > @@ -24,6 +24,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -73,6 +74,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 > @@ -1139,6 +1141,59 @@ static int ovs_ct_check_limit(struct net *net, > } > #endif > > +static void ovs_ct_add_timeout(struct net *net, struct nf_conn *ct, > + const char *timeout_name, u16 l3num, u8 l4num) > +{ > +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT > + typeof(nf_ct_timeout_find_get_hook) timeout_find_get; > + typeof(nf_ct_timeout_put_hook) timeout_put; > + struct nf_ct_timeout *timeout; > + struct nf_conn_timeout *timeout_ext; > + > + rcu_read_lock(); > + timeout_find_get = rcu_dereference(nf_ct_timeout_find_get_hook); > + if (!timeout_find_get) { > + net_info_ratelimited("Timeout policy base is empty"); > + goto out; > + } > + > + timeout = timeout_find_get(net, timeout_name); > + if (!timeout) { > + net_info_ratelimited("No such timeout policy \"%s\"\n", > + timeout_name); > + goto out; > + } > + > + if (timeout->l3num != l3num) { > + net_info_ratelimited("Timeout policy `%s' can only be used by " > + "L3 protocol number %d\n", timeout_name, > + timeout->l3num); > + goto err_put_timeout; > + } > + > + if (timeout->l4proto->l4proto != l4num) { > + net_info_ratelimited("Timeout policy `%s' can only be used by " > + "L4 protocol number %d\n", timeout_name, > + timeout->l4proto->l4proto); > + goto err_put_timeout; > + } > + > + timeout_ext = nf_ct_timeout_ext_add(ct, timeout, GFP_ATOMIC); > + if (!timeout_ext) > + goto err_put_timeout; > + > + goto out; > + > +err_put_timeout: > + timeout_put = rcu_dereference(nf_ct_timeout_put_hook); > + if (timeout_put) > + timeout_put(timeout); > +out: > + rcu_read_unlock(); > + return; This code looks very similar to xt_ct_set_timeout(), can you refactor it to avoid code duplication? > +#endif > +} > + > /* Lookup connection and confirm if unconfirmed. */ > static int ovs_ct_commit(struct net *net, struct sw_flow_key *key, > const struct ovs_conntrack_info *info, > @@ -1465,6 +1520,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, > @@ -1550,6 +1607,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)); Before copying timeout, we need to check sizeof source string. 'nla_len(a)' needs to be less than CTNL_TIMEOUT_NAME_MAX. otherwise looks good. Thanks.