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