Re: [PATCH nf-next v8] netfilter: nft_ct: add ct timeout support

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Thu, Jul 19, 2018 at 2:33 AM, Pablo Neira Ayuso <pablo@xxxxxxxxxxxxx> wrote:
> On Thu, Jul 19, 2018 at 02:19:47AM +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
>>
>> Signed-off-by: Harsha Sharma <harshasharmaiitr@xxxxxxxxx>
>> ---
>> Changes in v8:
>>  - Add ifdef for CONFIG_NF_CT_NETLINK_TIMEOUT in nft_ct_timeout_obj_ops
>> Changes in v7:
>>  - initialise list_head for nfct_timeout_list in nf_tables_api
>>  - use nf_ct_untimeout for cleanup
>>  - other minor changes
>> Changes 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                   | 195 +++++++++++++++++++++++++++++++
>>  3 files changed, 212 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..c1cf24b6db96 100644
>> --- a/net/netfilter/nf_tables_api.c
>> +++ b/net/netfilter/nf_tables_api.c
>> @@ -7152,6 +7152,10 @@ static int __net_init nf_tables_init_net(struct net *net)
>>  {
>>       INIT_LIST_HEAD(&net->nft.tables);
>>       INIT_LIST_HEAD(&net->nft.commit_list);
>> +
>> +#if IS_ENABLED(CONFIG_NF_CT_NETLINK_TIMEOUT)
>
> Probably better:
>
> #if IS_ENABLED(CONFIG_NF_CONNTRACK_TIMEOUT)

CONFIG_NF_CT_NETLINK_TIMEOUT is required for struct
nf_conntrack_l4proto to have a member struct ctnl_timeout.
Since, NF_CT_NETLINK_TIMEOUT already depends on NF_CONNTRACK_CORE,  it
will make sense to change it in nf_conntrack_l4proto.h#L20.
Do you also want to change this in files like nf_conntrack_proto_tcp.c ?
If yes, should I send it as different patch or next version of Kconfig patch ?

> for all these IS_ENABLED.
>
>> +     INIT_LIST_HEAD(&net->nfct_timeout_list);
>
> You have to add:
>
>         net->nft.cttimeout_list;
>
> Have a look add include/net/netns/nftables.h
>
> Otherwise this will reset the existing list of nfct_timeout_list which
> is used by nfnetlink_cttimeout.
>
>> +#endif
>>       net->nft.base_seq = 1;
>>       net->nft.validate_state = NFT_VALIDATE_SKIP;
>>
>> diff --git a/net/netfilter/nft_ct.c b/net/netfilter/nft_ct.c
>> index 1435ffc5f57e..020c02a3823c 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>
>
> We don't need definitions in nfnetlink_cttimeout.h, right? I think
> this line above can be removed.
>
>> +#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,159 @@ static struct nft_expr_type nft_notrack_type __read_mostly = {
>>       .owner          = THIS_MODULE,
>>  };
>>
>> +#ifdef CONFIG_NF_CT_NETLINK_TIMEOUT
>> +static int
>> +nft_ct_timeout_parse_policy(void *timeouts,
>> +                         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;
>> +
>> +     if (nf_ct_get(skb, &ctinfo))
>> +             return;
>> +
>> +     to_assign = priv->timeout;
>> +     timeout_ext = nf_ct_timeout_find(priv->tmpl);
>
> This two lines above.
>
>> +     nf_ct_set(skb, priv->tmpl, IP_CT_NEW);
>> +     rcu_assign_pointer(timeout_ext->timeout, to_assign);
>
> And this one above... belong to the nft_ct_timeout_obj_init() path.
>
> So, only nf_ct_set(skb, ...) is sufficient to set the custom timeout,
> if the tmpl object is correct initialization from the init path.

I'll do the other changes. thanks.

>> +}
>> +
>> +static int nft_ct_timeout_obj_init(const struct nft_ctx *ctx,
>> +                                const struct nlattr * const tb[],
>> +                                struct nft_object *obj)
--
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



[Index of Archives]     [Netfitler Users]     [LARTC]     [Bugtraq]     [Yosemite Forum]

  Powered by Linux