On Fri, Jul 06, 2018 at 01:47:58AM +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"
> }
> }
>
> %./libnftnl/examples/nft-ct-timeout-add ip raw cttime tcp
> %./libnftnl/examples/nft-rule-ct-timeout-add ip raw output cttime
>
> %conntrack -E
> [NEW] tcp 6 111 ESTABLISHED src=172.16.19.128 dst=172.16.19.1
> sport=22 dport=41360 [UNREPLIED] src=172.16.19.1 dst=172.16.19.128
> sport=41360 dport=22 zone=4
>
> Signed-off-by: Harsha Sharma <harshasharmaiitr@xxxxxxxxx>
> ---
> Chenges in v6:
> - Remove unnecessary chunks
> - initialise timeout list in nf_tables_api.c
> - minor changes
> Changes in v5:
> - wrap with NF_CT_NETLINK_TIMEOUT option
> - attach timeout template in init
> - other minor changes
> Changes in v4:
> - Remove unused attributes
> - allocate template from init() path
> - minor changes
> - updated log message
> - pull to latest tree
> 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/uapi/linux/netfilter/nf_tables.h | 14 ++-
> net/netfilter/nf_tables_api.c | 4 +
> net/netfilter/nft_ct.c | 200 +++++++++++++++++++++++++++++++
> 3 files changed, 217 insertions(+), 1 deletion(-)
>
> diff --git a/include/uapi/linux/netfilter/nf_tables.h b/include/uapi/linux/netfilter/nf_tables.h
> index 89438e68dc03..552fa5a6b7c3 100644
> --- a/include/uapi/linux/netfilter/nf_tables.h
> +++ b/include/uapi/linux/netfilter/nf_tables.h
> @@ -955,6 +955,7 @@ enum nft_socket_keys {
> * @NFT_CT_DST_IP: conntrack layer 3 protocol destination (IPv4 address)
> * @NFT_CT_SRC_IP6: conntrack layer 3 protocol source (IPv6 address)
> * @NFT_CT_DST_IP6: conntrack layer 3 protocol destination (IPv6 address)
> + * @NFT_CT_TIMEOUT: connection tracking timeout policy assigned to conntrack
> */
> enum nft_ct_keys {
> NFT_CT_STATE,
> @@ -980,6 +981,7 @@ enum nft_ct_keys {
> NFT_CT_DST_IP,
> NFT_CT_SRC_IP6,
> NFT_CT_DST_IP6,
> + NFT_CT_TIMEOUT,
> __NFT_CT_MAX
> };
> #define NFT_CT_MAX (__NFT_CT_MAX - 1)
> @@ -1392,13 +1394,23 @@ 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_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_CONNLIMIT 5
> -#define __NFT_OBJECT_MAX 6
> +#define NFT_OBJECT_CT_TIMEOUT 6
> +#define __NFT_OBJECT_MAX 7
> #define NFT_OBJECT_MAX (__NFT_OBJECT_MAX - 1)
>
> /**
> diff --git a/net/netfilter/nf_tables_api.c b/net/netfilter/nf_tables_api.c
> index 3f211e1025c1..c1a6a57eb162 100644
> --- a/net/netfilter/nf_tables_api.c
> +++ b/net/netfilter/nf_tables_api.c
> @@ -82,6 +82,10 @@ static void nft_ctx_init(struct nft_ctx *ctx,
> ctx->portid = NETLINK_CB(skb).portid;
> ctx->report = nlmsg_report(nlh);
> ctx->seq = nlh->nlmsg_seq;
> +
> +#if IS_ENABLED(CONFIG_NF_CT_NETLINK_TIMEOUT)
> + INIT_LIST_HEAD(&ctx->net->nfct_timeout_list);
> +#endif
Not here.
Do this from __init nf_tables_module_init() path, so this is only done
once from the module initialization path :-)
> }
>
> static struct nft_trans *nft_trans_alloc_gfp(const struct nft_ctx *ctx,
> diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
> index 1435ffc5f57e..f866ed665cdd 100644
> --- a/net/netfilter/nft_ct.c
> +++ b/net/netfilter/nft_ct.c
> @@ -22,6 +22,9 @@
> #include <net/netfilter/nf_conntrack_helper.h>
> #include <net/netfilter/nf_conntrack_ecache.h>
> #include <net/netfilter/nf_conntrack_labels.h>
> +#include <linux/netfilter/nfnetlink_cttimeout.h>
> +#include <net/netfilter/nf_conntrack_timeout.h>
> +#include <net/netfilter/nf_conntrack_l4proto.h>
>
> struct nft_ct {
> enum nft_ct_keys key:8;
> @@ -38,6 +41,11 @@ struct nft_ct_helper_obj {
> u8 l4proto;
> };
>
> +struct nft_ct_timeout_obj {
> + struct ctnl_timeout *timeout;
> + struct nf_conn *tmpl;
> +};
> +
> #ifdef CONFIG_NF_CONNTRACK_ZONES
> static DEFINE_PER_CPU(struct nf_conn *, nft_ct_pcpu_template);
> static unsigned int nft_ct_pcpu_template_refcnt __read_mostly;
> @@ -765,6 +773,167 @@ static struct nft_expr_type nft_notrack_type __read_mostly = {
> .owner = THIS_MODULE,
> };
>
> +#ifdef CONFIG_NF_CT_NETLINK_TIMEOUT
> +static int
> +ctnl_timeout_parse_policy(void *timeouts,
Rename this to nft_ct_timeout_parse_policy.
> + const struct nf_conntrack_l4proto *l4proto,
> + struct net *net, const struct nlattr *attr)
> +{
> + struct nlattr **tb;
> + int ret = 0;
> +
> + 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 ctnl_timeout *to_assign = NULL;
> + struct nf_conn_timeout *timeout_ext;
> + struct sk_buff *skb = pkt->skb;
> + enum ip_conntrack_info ctinfo;
> +
> + to_assign = priv->timeout;
> + timeout_ext = nf_ct_timeout_find(priv->tmpl);
Better do this assignments above...
> +
> + if (nf_ct_get(skb, &ctinfo))
> + return;
... after we have checked that there is no ct for this skbuff, OK? I
mean, place those assignments here :-).
> + nf_ct_set(skb, priv->tmpl, IP_CT_NEW);
> + if (timeout_ext)
timeout_ext should be always set, right? I mean, we should now ever
have a timeout_ext that is NULL here. The nft_ct_timeout_obj_init()
path already guarantees that this is always non-NULL.
> + rcu_assign_pointer(timeout_ext->timeout, to_assign);
> +}
> +
> +static int nft_ct_timeout_obj_init(const struct nft_ctx *ctx,
> + const struct nlattr * const tb[],
> + struct nft_object *obj)
> +{
> + const struct nf_conntrack_zone *zone = &nf_ct_zone_dflt;
> + struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
> + struct ctnl_timeout *timeout, *matching = NULL;
> + const struct nf_conntrack_l4proto *l4proto;
> + struct nf_conn_timeout *timeout_ext;
> + int l3num = ctx->family;
> + struct nf_conn *tmpl;
> + __u8 l4num;
> + int ret;
> +
> + if (!tb[NFTA_CT_TIMEOUT_L3PROTO] ||
> + !tb[NFTA_CT_TIMEOUT_L4PROTO] ||
> + !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]);
> +
> + 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;
> + priv->timeout = timeout;
> + tmpl = nf_ct_tmpl_alloc(ctx->net, zone, GFP_ATOMIC);
> + if (!tmpl)
> + return -ENOMEM;
> + priv->tmpl = tmpl;
> + timeout_ext = nf_ct_timeout_ext_add(priv->tmpl, priv->timeout,
> + GFP_ATOMIC);
Check here if timeout_ext is NULL, in that case, release the tmpl
object - don't forget this! - and return -ENOMEM;
> + list_add_tail_rcu(&timeout->head, &ctx->net->nfct_timeout_list);
> + return 0;
> +
> +err:
> + kfree(timeout);
> +err_proto_put:
> + nf_ct_l4proto_put(l4proto);
> + return ret;
> +}
> +
> +static void nft_ct_timeout_obj_destroy(const struct nft_ctx *ctx,
> + struct nft_object *obj)
> +{
> + struct nft_ct_timeout_obj *priv = nft_obj_data(obj);
> +
> + if (refcount_dec_if_one(&priv->timeout->refcnt)) {
Is priv->timeout->refcnt initialized to 1 from the
nft_ct_timeout_obj_init() path?
> + nf_ct_l4proto_put(priv->timeout->l4proto);
> + list_del_rcu(&priv->timeout->head);
BTW, you need an initial patch that move ctnl_untimeout() from
nfnetlink_cttimeout.c to net/netfilter/nf_conntrack_timeout.c, so we
can call it from here too. Rename ctnl_untimeout() to
nf_ct_untimeout() and call it from here. As said, you will need an
initial patch for this.
> + nf_ct_tmpl_free(priv->tmpl);
I think priv->tmpl should be release...
> + }
... here, I mean: inconditionally.
> +}
That's all, please address feedback and resubmit.
Thanks!
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
