[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <CAOrHB_D0Yhj4ZNj0b8WEiKbriW8ThRZd8vQa7Ucr3LQMGEJO7Q@mail.gmail.com>
Date: Tue, 26 Mar 2019 10:56:22 -0700
From: Pravin Shelar <pshelar@....org>
To: Yi-Hung Wei <yihung.wei@...il.com>
Cc: Linux Kernel Network Developers <netdev@...r.kernel.org>,
Pablo Neira Ayuso <pablo@...filter.org>
Subject: Re: [PATCH net-next v3 1/2] netfilter: Export nf_ct_{set,destroy}_timeout()
On Mon, Mar 25, 2019 at 2:40 PM Yi-Hung Wei <yihung.wei@...il.com> wrote:
>
> This patch exports nf_ct_set_timeout() and nf_ct_destroy_timeout().
> The two functions are derived from xt_ct_destroy_timeout() and
> xt_ct_set_timeout() in xt_CT.c, and moved to nf_conntrack_timeout.c
> without any functional change.
> It would be useful for other users (i.e. OVS) that utilizes the
> finer-grain conntrack timeout feature.
>
> CC: Pablo Neira Ayuso <pablo@...filter.org>
> CC: Pravin Shelar <pshelar@....org>
> Signed-off-by: Yi-Hung Wei <yihung.wei@...il.com>
> ---
> v1-> v2: Export nf_ct_set_timeout().
> v2-> v3: Fix build issue when CONFIG_NF_CONNTRACK_TIMEOUT is not set.
> ---
> include/net/netfilter/nf_conntrack_timeout.h | 15 +++++
> net/netfilter/nf_conntrack_timeout.c | 97 ++++++++++++++++++++++++++++
> net/netfilter/xt_CT.c | 93 ++------------------------
> 3 files changed, 118 insertions(+), 87 deletions(-)
>
> diff --git a/include/net/netfilter/nf_conntrack_timeout.h b/include/net/netfilter/nf_conntrack_timeout.h
> index 3394d75e1c80..00a8fbb2d735 100644
> --- a/include/net/netfilter/nf_conntrack_timeout.h
> +++ b/include/net/netfilter/nf_conntrack_timeout.h
> @@ -88,6 +88,9 @@ static inline unsigned int *nf_ct_timeout_lookup(const struct nf_conn *ct)
> int nf_conntrack_timeout_init(void);
> void nf_conntrack_timeout_fini(void);
> void nf_ct_untimeout(struct net *net, struct nf_ct_timeout *timeout);
> +int nf_ct_set_timeout(struct net *net, struct nf_conn *ct, u8 l3num, u8 l4num,
> + const char *timeout_name);
> +void nf_ct_destroy_timeout(struct nf_conn *ct);
> #else
> static inline int nf_conntrack_timeout_init(void)
> {
> @@ -98,6 +101,18 @@ static inline void nf_conntrack_timeout_fini(void)
> {
> return;
> }
> +
> +static inline int nf_ct_set_timeout(struct net *net, struct nf_conn *ct,
> + u8 l3num, u8 l4num,
> + const char *timeout_name)
> +{
> + return -EOPNOTSUPP;
> +}
> +
> +static inline void nf_ct_destroy_timeout(struct nf_conn *ct)
> +{
> + return;
> +}
> #endif /* CONFIG_NF_CONNTRACK_TIMEOUT */
>
> #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
> diff --git a/net/netfilter/nf_conntrack_timeout.c b/net/netfilter/nf_conntrack_timeout.c
> index 91fbd183da2d..c2dee577548e 100644
> --- a/net/netfilter/nf_conntrack_timeout.c
> +++ b/net/netfilter/nf_conntrack_timeout.c
> @@ -48,6 +48,103 @@ void nf_ct_untimeout(struct net *net, struct nf_ct_timeout *timeout)
> }
> EXPORT_SYMBOL_GPL(nf_ct_untimeout);
>
> +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
> +static void __nf_ct_timeout_put(struct nf_ct_timeout *timeout)
> +{
> + typeof(nf_ct_timeout_put_hook) timeout_put;
> +
> + timeout_put = rcu_dereference(nf_ct_timeout_put_hook);
> + if (timeout_put)
> + timeout_put(timeout);
> +}
> +#endif
This whole module is protected by CONFIG_NF_CONNTRACK_TIMEOUT, There
is no need for these #ifdefs.
> +
> +int nf_ct_set_timeout(struct net *net, struct nf_conn *ct,
> + u8 l3num, u8 l4num, const char *timeout_name)
> +{
> +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
> + typeof(nf_ct_timeout_find_get_hook) timeout_find_get;
> + struct nf_ct_timeout *timeout;
> + struct nf_conn_timeout *timeout_ext;
> + const char *errmsg = NULL;
> + int ret = 0;
> +
> + rcu_read_lock();
> + timeout_find_get = rcu_dereference(nf_ct_timeout_find_get_hook);
> + if (!timeout_find_get) {
> + ret = -ENOENT;
> + errmsg = "Timeout policy base is empty";
> + goto out;
> + }
> +
> + timeout = timeout_find_get(net, timeout_name);
> + if (!timeout) {
> + ret = -ENOENT;
> + pr_info_ratelimited("No such timeout policy \"%s\"\n",
> + timeout_name);
> + goto out;
> + }
> +
> + if (timeout->l3num != l3num) {
> + ret = -EINVAL;
> + pr_info_ratelimited("Timeout policy `%s' can only be used by "
> + "L%d protocol number %d\n",
> + timeout_name, 3, timeout->l3num);
> + goto err_put_timeout;
> + }
> + /* Make sure the timeout policy matches any existing protocol tracker,
> + * otherwise default to generic.
> + */
> + if (timeout->l4proto->l4proto != l4num) {
> + ret = -EINVAL;
> + pr_info_ratelimited("Timeout policy `%s' can only be used by "
> + "L%d protocol number %d\n",
> + timeout_name, 4, timeout->l4proto->l4proto);
> + goto err_put_timeout;
> + }
> + timeout_ext = nf_ct_timeout_ext_add(ct, timeout, GFP_ATOMIC);
> + if (!timeout_ext) {
> + ret = -ENOMEM;
> + goto err_put_timeout;
> + }
> +
> + rcu_read_unlock();
> + return ret;
> +
> +err_put_timeout:
> + __nf_ct_timeout_put(timeout);
> +out:
> + rcu_read_unlock();
> + if (errmsg)
> + pr_info_ratelimited("%s\n", errmsg);
> + return ret;
> +#else
> + return -EOPNOTSUPP;
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(nf_ct_set_timeout);
> +
> +void nf_ct_destroy_timeout(struct nf_conn *ct)
> +{
> +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
> + struct nf_conn_timeout *timeout_ext;
> + typeof(nf_ct_timeout_put_hook) timeout_put;
> +
> + rcu_read_lock();
> + timeout_put = rcu_dereference(nf_ct_timeout_put_hook);
> +
> + if (timeout_put) {
> + timeout_ext = nf_ct_timeout_find(ct);
> + if (timeout_ext) {
> + timeout_put(timeout_ext->timeout);
> + RCU_INIT_POINTER(timeout_ext->timeout, NULL);
> + }
> + }
> + rcu_read_unlock();
> +#endif
> +}
> +EXPORT_SYMBOL_GPL(nf_ct_destroy_timeout);
> +
> static const struct nf_ct_ext_type timeout_extend = {
> .len = sizeof(struct nf_conn_timeout),
> .align = __alignof__(struct nf_conn_timeout),
> diff --git a/net/netfilter/xt_CT.c b/net/netfilter/xt_CT.c
> index 0fa863f57575..d59cb4730fac 100644
> --- a/net/netfilter/xt_CT.c
> +++ b/net/netfilter/xt_CT.c
> @@ -103,85 +103,24 @@ xt_ct_set_helper(struct nf_conn *ct, const char *helper_name,
> return 0;
> }
>
> -#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
> -static void __xt_ct_tg_timeout_put(struct nf_ct_timeout *timeout)
> -{
> - typeof(nf_ct_timeout_put_hook) timeout_put;
> -
> - timeout_put = rcu_dereference(nf_ct_timeout_put_hook);
> - if (timeout_put)
> - timeout_put(timeout);
> -}
> -#endif
> -
> static int
> xt_ct_set_timeout(struct nf_conn *ct, const struct xt_tgchk_param *par,
> const char *timeout_name)
> {
> #ifdef CONFIG_NF_CONNTRACK_TIMEOUT
> - typeof(nf_ct_timeout_find_get_hook) timeout_find_get;
> const struct nf_conntrack_l4proto *l4proto;
> - struct nf_ct_timeout *timeout;
> - struct nf_conn_timeout *timeout_ext;
> - const char *errmsg = NULL;
> - int ret = 0;
> u8 proto;
>
> - rcu_read_lock();
> - timeout_find_get = rcu_dereference(nf_ct_timeout_find_get_hook);
> - if (timeout_find_get == NULL) {
> - ret = -ENOENT;
> - errmsg = "Timeout policy base is empty";
> - goto out;
> - }
> -
> proto = xt_ct_find_proto(par);
> if (!proto) {
> - ret = -EINVAL;
> - errmsg = "You must specify a L4 protocol and not use inversions on it";
> - goto out;
> - }
> -
> - timeout = timeout_find_get(par->net, timeout_name);
> - if (timeout == NULL) {
> - ret = -ENOENT;
> - pr_info_ratelimited("No such timeout policy \"%s\"\n",
> - timeout_name);
> - goto out;
> - }
> -
> - if (timeout->l3num != par->family) {
> - ret = -EINVAL;
> - pr_info_ratelimited("Timeout policy `%s' can only be used by L%d protocol number %d\n",
> - timeout_name, 3, timeout->l3num);
> - goto err_put_timeout;
> + pr_info_ratelimited("You must specify a L4 protocol and not "
> + "use inversions on it");
> + return -EINVAL;
> }
> - /* Make sure the timeout policy matches any existing protocol tracker,
> - * otherwise default to generic.
> - */
> l4proto = nf_ct_l4proto_find(proto);
> - if (timeout->l4proto->l4proto != l4proto->l4proto) {
> - ret = -EINVAL;
> - pr_info_ratelimited("Timeout policy `%s' can only be used by L%d protocol number %d\n",
> - timeout_name, 4, timeout->l4proto->l4proto);
> - goto err_put_timeout;
> - }
> - timeout_ext = nf_ct_timeout_ext_add(ct, timeout, GFP_ATOMIC);
> - if (!timeout_ext) {
> - ret = -ENOMEM;
> - goto err_put_timeout;
> - }
> + return nf_ct_set_timeout(par->net, ct, par->family, l4proto->l4proto,
> + timeout_name);
>
> - rcu_read_unlock();
> - return ret;
> -
> -err_put_timeout:
> - __xt_ct_tg_timeout_put(timeout);
> -out:
> - rcu_read_unlock();
> - if (errmsg)
> - pr_info_ratelimited("%s\n", errmsg);
> - return ret;
> #else
> return -EOPNOTSUPP;
> #endif
> @@ -328,26 +267,6 @@ static int xt_ct_tg_check_v2(const struct xt_tgchk_param *par)
> return xt_ct_tg_check(par, par->targinfo);
> }
>
> -static void xt_ct_destroy_timeout(struct nf_conn *ct)
> -{
> -#ifdef CONFIG_NF_CONNTRACK_TIMEOUT
> - struct nf_conn_timeout *timeout_ext;
> - typeof(nf_ct_timeout_put_hook) timeout_put;
> -
> - rcu_read_lock();
> - timeout_put = rcu_dereference(nf_ct_timeout_put_hook);
> -
> - if (timeout_put) {
> - timeout_ext = nf_ct_timeout_find(ct);
> - if (timeout_ext) {
> - timeout_put(timeout_ext->timeout);
> - RCU_INIT_POINTER(timeout_ext->timeout, NULL);
> - }
> - }
> - rcu_read_unlock();
> -#endif
> -}
> -
> static void xt_ct_tg_destroy(const struct xt_tgdtor_param *par,
> struct xt_ct_target_info_v1 *info)
> {
> @@ -361,7 +280,7 @@ static void xt_ct_tg_destroy(const struct xt_tgdtor_param *par,
>
> nf_ct_netns_put(par->net, par->family);
>
> - xt_ct_destroy_timeout(ct);
> + nf_ct_destroy_timeout(ct);
> nf_ct_put(info->ct);
> }
> }
> --
> 2.7.4
>
Powered by blists - more mailing lists