Hi Harsha,

Comments below.

On Mon, May 28, 2018 at 11:58:11PM +0200, Harsha Sharma wrote:
> This patch allows to add, list and delete connection tracking timeout
> policies via nft objref infrastructure and assigning these timeout
> via nft rule.
> Ruleset:
> 
> table ip raw {
>    ct timeout cttime {
>        protocol tcp
>        established 111 close 13
>        l3proto ip
>    }
> 
>    chain output {
>        type filter hook output priority -300; policy accept;
>        ct timeout set "cttime"
>    }
> }
> 
> Signed-off-by: Harsha Sharma <harshasharmai...@gmail.com>
> ---
> Changes in v3:
>  - Use nf_ct_tmpl_alloc to attach timeout via template conntrack.
> Changes in v2:
>  - Add code for nft_ct_timeout_obj_eval
>  - remove likely() from code
>  - remove vla in ctnl_timeout_parse_policy
> 
>  include/net/netfilter/nf_conntrack_timeout.h |   1 +
>  include/uapi/linux/netfilter/nf_tables.h     |  16 ++-
>  net/netfilter/nft_ct.c                       | 196 
> ++++++++++++++++++++++++++-
>  3 files changed, 210 insertions(+), 3 deletions(-)
> 
> diff --git a/include/net/netfilter/nf_conntrack_timeout.h 
> b/include/net/netfilter/nf_conntrack_timeout.h
> index 9468ab4ad12d..ae0f59cc1ffa 100644
> --- a/include/net/netfilter/nf_conntrack_timeout.h
> +++ b/include/net/netfilter/nf_conntrack_timeout.h
> @@ -8,6 +8,7 @@
>  #include <linux/refcount.h>
>  #include <net/netfilter/nf_conntrack.h>
>  #include <net/netfilter/nf_conntrack_extend.h>
> +#include <net/netfilter/nf_conntrack_l4proto.h>
>  
>  #define CTNL_TIMEOUT_NAME_MAX        32
>  
> diff --git a/include/uapi/linux/netfilter/nf_tables.h 
> b/include/uapi/linux/netfilter/nf_tables.h
> index 66dceee0ae30..3915f898607b 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -920,6 +920,7 @@ enum nft_rt_attributes {
>   * @NFT_CT_AVGPKT: conntrack average bytes per packet
>   * @NFT_CT_ZONE: conntrack zone
>   * @NFT_CT_EVENTMASK: ctnetlink events to be generated for this conntrack
> + * @NFT_CT_TIMEOUT: connection tracking timeout policy assigned to conntrack
>   */
>  enum nft_ct_keys {
>       NFT_CT_STATE,
> @@ -941,6 +942,7 @@ enum nft_ct_keys {
>       NFT_CT_AVGPKT,
>       NFT_CT_ZONE,
>       NFT_CT_EVENTMASK,
> +     NFT_CT_TIMEOUT,
>  };
>  
>  /**
> @@ -1302,12 +1304,24 @@ enum nft_ct_helper_attributes {
>  };
>  #define NFTA_CT_HELPER_MAX   (__NFTA_CT_HELPER_MAX - 1)
>  
> +enum nft_ct_timeout_timeout_attributes {
> +     NFTA_CT_TIMEOUT_UNSPEC,
> +     NFTA_CT_TIMEOUT_L3PROTO,
> +     NFTA_CT_TIMEOUT_L4PROTO,
> +     NFTA_CT_TIMEOUT_DATA,
> +     NFTA_CT_TIMEOUT_USE,

This NFTA_CT_TIMEOUT_USE is not used, remove it.

> +     NFTA_CT_TIMEOUT_EXPR,

Same thing with this attribute.

> +     __NFTA_CT_TIMEOUT_MAX,
> +};
> +#define NFTA_CT_TIMEOUT_MAX  (__NFTA_CT_TIMEOUT_MAX - 1)
> +
>  #define NFT_OBJECT_UNSPEC    0
>  #define NFT_OBJECT_COUNTER   1
>  #define NFT_OBJECT_QUOTA     2
>  #define NFT_OBJECT_CT_HELPER 3
>  #define NFT_OBJECT_LIMIT     4
> -#define __NFT_OBJECT_MAX     5
> +#define NFT_OBJECT_CT_TIMEOUT 5
> +#define __NFT_OBJECT_MAX     6
>  #define NFT_OBJECT_MAX               (__NFT_OBJECT_MAX - 1)
>  
>  /**
> diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
> index 6ab274b14484..2d1a2127a00b 100644
> --- a/net/netfilter/nft_ct.c
> +++ b/net/netfilter/nft_ct.c
[...]
> @@ -727,6 +733,163 @@ static struct nft_expr_type nft_notrack_type 
> __read_mostly = {
>       .owner          = THIS_MODULE,
>  };
>  
> +static int
> +ctnl_timeout_parse_policy(void *timeouts,
> +                       const struct nf_conntrack_l4proto *l4proto,
> +                       struct net *net, const struct nlattr *attr)
> +{
> +     int ret = 0;
> +     struct nlattr **tb;
> +
> +     if (!l4proto->ctnl_timeout.nlattr_to_obj)
> +             return 0;
> +
> +     tb = kcalloc(l4proto->ctnl_timeout.nlattr_max + 1, sizeof(*tb),
> +                  GFP_KERNEL);
> +
> +     if (!tb)
> +             return -ENOMEM;
> +
> +     ret = nla_parse_nested(tb, l4proto->ctnl_timeout.nlattr_max,
> +                            attr, l4proto->ctnl_timeout.nla_policy,
> +                            NULL);
> +     if (ret < 0)
> +             goto err;
> +
> +     ret = l4proto->ctnl_timeout.nlattr_to_obj(tb, net, timeouts);
> +
> +err:
> +     kfree(tb);
> +     return ret;
> +}
> +
> +static void nft_ct_timeout_obj_eval(struct nft_object *obj,
> +                                 struct nft_regs *regs,
> +                                 const struct nft_pktinfo *pkt)
> +{
> +     const struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
> +     struct nf_conn *ct;
> +     struct ctnl_timeout *to_assign = NULL;
> +     struct nf_conn_timeout *timeout_ext;
> +     struct nf_conntrack_zone zone;
> +
> +     memset(&zone, 0, sizeof(zone));

Please, use:

        const struct nf_conntrack_zone *zone = &nf_ct_zone_dflt;

instead to initialize the zone.

> +
> +     ct = nf_ct_tmpl_alloc(nft_net(pkt), &zone, GFP_KERNEL);

Allocate template from the _init() path, then store reference to it
via:

        priv->tmpl.

And from _eval() path, attach it to the skbuff.

See:

https://elixir.bootlin.com/linux/latest/source/net/netfilter/nf_conntrack_core.c#L1200

The idea is to offer a template conntrack containing the timeout
policy via skbuff, that nf_conntrack_in() will take to refresh the
timeout.

You have to do this from the raw table, ie. in nft, create a filter
chain using priority -300, to test this.

> +
> +     if (!ct ||
> +         nf_ct_is_confirmed(ct)) {
> +             return;
> +     }
> +
> +     to_assign = priv->timeout;
> +
> +     timeout_ext = nf_ct_timeout_ext_add(ct, priv->timeout, GFP_ATOMIC);
> +
> +     if (timeout_ext) {
> +             rcu_assign_pointer(timeout_ext->timeout, to_assign);
> +             atomic_inc(&ct->ct_general.use);
> +     }
> +}
> +
> +static int nft_ct_timeout_obj_init(const struct nft_ctx *ctx,
> +                                const struct nlattr * const tb[],
> +                                struct nft_object *obj)
> +{
> +     __u16 l3num;
> +     __u8 l4num;
> +     struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
> +     const struct nf_conntrack_l4proto *l4proto;
> +     struct ctnl_timeout *timeout, *matching = NULL;
> +     int ret;
> +
> +     if (!tb[NFTA_CT_TIMEOUT_L4PROTO] ||
> +         !tb[NFTA_CT_TIMEOUT_L3PROTO] ||
> +         !tb[NFTA_CT_TIMEOUT_DATA])
> +             return -EINVAL;
> +
> +     l3num = ntohs(nla_get_be16(tb[NFTA_CT_TIMEOUT_L3PROTO]));
> +     l4num = nla_get_u8(tb[NFTA_CT_TIMEOUT_L4PROTO]);
> +
> +     INIT_LIST_HEAD(&ctx->net->nfct_timeout_list);
> +     list_for_each_entry(timeout, &ctx->net->nfct_timeout_list, head) {
> +             matching = timeout;
> +             break;
> +     }
> +
> +     if (matching) {
> +             if (matching->l3num != l3num ||
> +                 matching->l4proto->l4proto != l4num)
> +                     return -EINVAL;
> +             return ctnl_timeout_parse_policy(&matching->data,
> +                                              matching->l4proto, ctx->net,
> +                                              tb[NFTA_CT_TIMEOUT_DATA]);
> +     }
> +
> +     l4proto = nf_ct_l4proto_find_get(l3num, l4num);
> +
> +     if (l4proto->l4proto != l4num) {
> +             ret = -EOPNOTSUPP;
> +             goto err_proto_put;
> +     }
> +
> +     timeout = kzalloc(sizeof(struct ctnl_timeout) +
> +                       l4proto->ctnl_timeout.obj_size, GFP_KERNEL);
> +     if (timeout == NULL) {
> +             ret = -ENOMEM;
> +             goto err_proto_put;
> +     }
> +
> +     ret = ctnl_timeout_parse_policy(&timeout->data, l4proto, ctx->net,
> +                                     tb[NFTA_CT_TIMEOUT_DATA]);
> +     if (ret < 0)
> +             goto err;
> +     timeout->l3num = l3num;
> +     timeout->l4proto = l4proto;
> +     list_add_tail_rcu(&timeout->head, &ctx->net->nfct_timeout_list);
> +     priv->timeout = timeout;
> +
> +     return 0;
> +err:
> +     kfree(timeout);
> +err_proto_put:
> +     nf_ct_l4proto_put(l4proto);
> +     return ret;
> +}
> +
> +static void nft_ct_timeout_obj_destroy(struct nft_object *obj)
> +{
> +     struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
> +
> +     if (refcount_dec_if_one(&priv->timeout->refcnt)) {
> +             nf_ct_l4proto_put(priv->timeout->l4proto);
> +             list_del_rcu(&priv->timeout->head);
> +     }
> +}
> +
> +static int nft_ct_timeout_obj_dump(struct sk_buff *skb,
> +                                struct nft_object *obj, bool reset)
> +{
> +     const struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
> +     const struct ctnl_timeout *timeout = priv->timeout;
> +     struct nlattr *nest_params;
> +     int ret;
> +
> +     if (nla_put_u8(skb, NFTA_CT_TIMEOUT_L4PROTO, timeout->l4proto->l4proto) 
> ||
> +         nla_put_be16(skb, NFTA_CT_TIMEOUT_L3PROTO, 
> htons(priv->timeout->l3num)))
> +             return -1;
> +
> +     nest_params = nla_nest_start(skb, NFTA_CT_TIMEOUT_DATA | NLA_F_NESTED);
> +     if (!nest_params)
> +             return -1;
> +
> +     ret = timeout->l4proto->ctnl_timeout.obj_to_nlattr(skb, &timeout->data);
> +     if (ret < 0)
> +             return -1;
> +     nla_nest_end(skb, nest_params);
> +     return 0;
> +}
> +
>  static int nft_ct_helper_obj_init(const struct nft_ctx *ctx,
>                                 const struct nlattr * const tb[],
>                                 struct nft_object *obj)
> @@ -889,6 +1052,30 @@ static struct nft_object_type nft_ct_helper_obj_type 
> __read_mostly = {
>       .owner          = THIS_MODULE,
>  };
>  
> +static const struct nla_policy nft_ct_timeout_policy[NFTA_CT_TIMEOUT_MAX + 
> 1] = {
> +     [NFTA_CT_TIMEOUT_L3PROTO] = {.type = NLA_U16 },
> +     [NFTA_CT_TIMEOUT_L4PROTO] = {.type = NLA_U8 },
> +     [NFTA_CT_TIMEOUT_DATA]    = {.type = NLA_NESTED },
> +};
> +
> +static struct nft_object_type nft_ct_timeout_obj_type;
> +static const struct nft_object_ops nft_ct_timeout_obj_ops = {
> +     .type           = &nft_ct_timeout_obj_type,
> +     .size           = sizeof(struct nft_ct_timeout_obj),
> +     .eval           = nft_ct_timeout_obj_eval,
> +     .init           = nft_ct_timeout_obj_init,
> +     .destroy        = nft_ct_timeout_obj_destroy,
> +     .dump           = nft_ct_timeout_obj_dump,
> +};
> +
> +static struct nft_object_type nft_ct_timeout_obj_type __read_mostly = {
> +     .type           = NFT_OBJECT_CT_TIMEOUT,
> +     .ops            = &nft_ct_timeout_obj_ops,
> +     .maxattr = NFTA_CT_TIMEOUT_MAX,
> +     .policy = nft_ct_timeout_policy,
               ^
missing tab here.

> +     .owner          = THIS_MODULE,
> +};
> +
>  static int __init nft_ct_module_init(void)
>  {
>       int err;
> @@ -904,6 +1091,9 @@ static int __init nft_ct_module_init(void)
>               goto err1;
>  
>       err = nft_register_obj(&nft_ct_helper_obj_type);
> +     if (err < 0)
> +             goto err2;

You probably need:

                goto err3;

to unwind the error.

> +     err = nft_register_obj(&nft_ct_timeout_obj_type);
>       if (err < 0)
>               goto err2;
>  
> @@ -919,6 +1109,7 @@ static int __init nft_ct_module_init(void)
>  static void __exit nft_ct_module_exit(void)
>  {
>       nft_unregister_obj(&nft_ct_helper_obj_type);
> +     nft_unregister_obj(&nft_ct_timeout_obj_type);
>       nft_unregister_expr(&nft_notrack_type);
>       nft_unregister_expr(&nft_ct_type);
>  }
> @@ -931,3 +1122,4 @@ MODULE_AUTHOR("Patrick McHardy <ka...@trash.net>");
>  MODULE_ALIAS_NFT_EXPR("ct");
>  MODULE_ALIAS_NFT_EXPR("notrack");
>  MODULE_ALIAS_NFT_OBJ(NFT_OBJECT_CT_HELPER);
> +MODULE_ALIAS_NFT_OBJ(NFT_OBJECT_CT_TIMEOUT);
> -- 
> 2.14.1
> 
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to