| Message ID | 20180719001947.15900-1-harshasharmaiitr@gmail.com |
|---|---|
| State | Changes Requested |
| Delegated to: | Pablo Neira |
| Headers | show |
| Series | [nf-next,v8] netfilter: nft_ct: add ct timeout support | expand |
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@gmail.com> > --- > 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) 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. > +} > + > +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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 19, 2018 at 2:33 AM, Pablo Neira Ayuso <pablo@netfilter.org> 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@gmail.com> >> --- >> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 19, 2018 at 03:10:14PM +0200, Harsha Sharma wrote: > On Thu, Jul 19, 2018 at 2:33 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: [...] > >> 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. No. A structure definition doesn't create a dependency that would break things. You just have to include the header file and use it, that's all. Problems are function calls, those are real dependencies between modules. > 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 ? Not really, those are really only useful for NF_CT_NETLINK_TIMEOUT. Why do you want to update those? [...] > >> +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. Thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 20, 2018 at 3:21 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > On Thu, Jul 19, 2018 at 03:10:14PM +0200, Harsha Sharma wrote: >> On Thu, Jul 19, 2018 at 2:33 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > [...] >> >> 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. > > No. A structure definition doesn't create a dependency that would > break things. > > You just have to include the header file and use it, that's all. With CONFIG_NF_CT_NETLINK_TIMEOUT = n and CONFIG_NF_CONNTRACK_TIMEOUT = y, it doesn't compile properly if I change it to IS_ENABLED(CONFIG_NF_CONNTRACK_TIMEOUT) in nft_ct.c with error "‘const struct nf_conntrack_l4proto’ has no member named ‘ctnl_timeout’". Do you still want me to change it here ? Thank you very much. > Problems are function calls, those are real dependencies between > modules. > >> 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 ? > > Not really, those are really only useful for NF_CT_NETLINK_TIMEOUT. > > Why do you want to update those? > > [...] >> >> +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. > > Thanks. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 20, 2018 at 11:13:37PM +0200, Harsha Sharma wrote: > On Fri, Jul 20, 2018 at 3:21 PM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > On Thu, Jul 19, 2018 at 03:10:14PM +0200, Harsha Sharma wrote: > >> On Thu, Jul 19, 2018 at 2:33 AM, Pablo Neira Ayuso <pablo@netfilter.org> wrote: > > [...] > >> >> 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. > > > > No. A structure definition doesn't create a dependency that would > > break things. > > > > You just have to include the header file and use it, that's all. > > With CONFIG_NF_CT_NETLINK_TIMEOUT = n and CONFIG_NF_CONNTRACK_TIMEOUT > = y, it doesn't compile properly if I change it to > IS_ENABLED(CONFIG_NF_CONNTRACK_TIMEOUT) in nft_ct.c with error "‘const > struct nf_conntrack_l4proto’ has no member named ‘ctnl_timeout’". > Do you still want me to change it here ? Leave it as it is, we can revisit this later. Address other feedback in this patch. Specifically, you have to add net->nft.cttimeout_list, that's an important change. Otherwise, we'll have problems mixing iptables with nftables. Thanks Harsha. -- To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
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) + INIT_LIST_HEAD(&net->nfct_timeout_list); +#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> +#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); + nf_ct_set(skb, priv->tmpl, IP_CT_NEW); + 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; + 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]); + 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 = nft_ct_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; + timeout_ext = nf_ct_timeout_ext_add(tmpl, priv->timeout, + GFP_ATOMIC); + + if (!timeout_ext) { + nf_ct_tmpl_free(tmpl); + return -ENOMEM; + } + + priv->tmpl = tmpl; + refcount_set(&priv->timeout->refcnt, 1); + 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); + + nf_ct_tmpl_free(priv->tmpl); + + if (refcount_dec_if_one(&priv->timeout->refcnt)) { + nf_ct_l4proto_put(priv->timeout->l4proto); + list_del_rcu(&priv->timeout->head); + nf_ct_untimeout(ctx->net, priv->timeout); + } +} + +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(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; +} +#endif + static int nft_ct_helper_obj_init(const struct nft_ctx *ctx, const struct nlattr * const tb[], struct nft_object *obj) @@ -932,6 +1093,33 @@ 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), +#ifdef CONFIG_NF_CT_NETLINK_TIMEOUT + .eval = nft_ct_timeout_obj_eval, + .init = nft_ct_timeout_obj_init, + .destroy = nft_ct_timeout_obj_destroy, + .dump = nft_ct_timeout_obj_dump, +#endif +}; + +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, + .owner = THIS_MODULE, +}; + static int __init nft_ct_module_init(void) { int err; @@ -949,9 +1137,14 @@ static int __init nft_ct_module_init(void) err = nft_register_obj(&nft_ct_helper_obj_type); if (err < 0) goto err2; + err = nft_register_obj(&nft_ct_timeout_obj_type); + if (err < 0) + goto err3; return 0; +err3: + nft_unregister_obj(&nft_ct_helper_obj_type); err2: nft_unregister_expr(&nft_notrack_type); err1: @@ -962,6 +1155,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); } @@ -974,3 +1168,4 @@ MODULE_AUTHOR("Patrick McHardy <kaber@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);
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@gmail.com> --- 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(-)