Message ID | 1553116295-16359-2-git-send-email-yihung.wei@gmail.com |
---|---|
State | Awaiting Upstream |
Delegated to: | David Miller |
Headers | show |
Series | [net-next,1/2] netfilter: Export nf_ct_destroy_timeout() | expand |
On Wed, Mar 20, 2019 at 2:19 PM Yi-Hung Wei <yihung.wei@gmail.com> wrote: > > Add support for fine-grain timeout support to conntrack action. > The new OVS_CT_ATTR_TIMEOUT attribute of the conntrack action > specifies a timeout to be associated with this connection. > If no timeout is specified, it acts as is, that is the default > timeout for the connection will be automatically applied. > > Example usage: > $ nfct timeout add timeout_1 inet tcp syn_sent 100 established 200 > $ ovs-ofctl add-flow br0 in_port=1,ip,tcp,action=ct(commit,timeout=timeout_1) > > CC: Pravin Shelar <pshelar@ovn.org> > Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> > --- > include/uapi/linux/openvswitch.h | 3 ++ > net/openvswitch/conntrack.c | 81 +++++++++++++++++++++++++++++++++++++++- > 2 files changed, 83 insertions(+), 1 deletion(-) > > diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h > index dbe0cbe4f1b7..9bccc6b9ed3d 100644 > --- a/include/uapi/linux/openvswitch.h > +++ b/include/uapi/linux/openvswitch.h > @@ -734,6 +734,7 @@ struct ovs_action_hash { > * be received on NFNLGRP_CONNTRACK_NEW and NFNLGRP_CONNTRACK_DESTROY groups, > * respectively. Remaining bits control the changes for which an event is > * delivered on the NFNLGRP_CONNTRACK_UPDATE group. > + * @OVS_CT_ATTR_TIMEOUT: Variable length string defining conntrack timeout. > */ > enum ovs_ct_attr { > OVS_CT_ATTR_UNSPEC, > @@ -746,6 +747,8 @@ enum ovs_ct_attr { > OVS_CT_ATTR_NAT, /* Nested OVS_NAT_ATTR_* */ > OVS_CT_ATTR_FORCE_COMMIT, /* No argument */ > OVS_CT_ATTR_EVENTMASK, /* u32 mask of IPCT_* events. */ > + OVS_CT_ATTR_TIMEOUT, /* Associate timeout with this connection for > + fine-grain timeout tuning. */ > __OVS_CT_ATTR_MAX > }; > > diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c > index 1b6896896fff..10a2c73f22f2 100644 > --- a/net/openvswitch/conntrack.c > +++ b/net/openvswitch/conntrack.c > @@ -24,6 +24,7 @@ > #include <net/netfilter/nf_conntrack_helper.h> > #include <net/netfilter/nf_conntrack_labels.h> > #include <net/netfilter/nf_conntrack_seqadj.h> > +#include <net/netfilter/nf_conntrack_timeout.h> > #include <net/netfilter/nf_conntrack_zones.h> > #include <net/netfilter/ipv6/nf_defrag_ipv6.h> > #include <net/ipv6_frag.h> > @@ -73,6 +74,7 @@ struct ovs_conntrack_info { > u32 eventmask; /* Mask of 1 << IPCT_*. */ > struct md_mark mark; > struct md_labels labels; > + char timeout[CTNL_TIMEOUT_NAME_MAX]; > #ifdef CONFIG_NF_NAT_NEEDED > struct nf_nat_range2 range; /* Only present for SRC NAT and DST NAT. */ > #endif > @@ -1139,6 +1141,59 @@ static int ovs_ct_check_limit(struct net *net, > } > #endif > > +static void ovs_ct_add_timeout(struct net *net, struct nf_conn *ct, > + const char *timeout_name, u16 l3num, u8 l4num) > +{ > +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT > + typeof(nf_ct_timeout_find_get_hook) timeout_find_get; > + typeof(nf_ct_timeout_put_hook) timeout_put; > + struct nf_ct_timeout *timeout; > + struct nf_conn_timeout *timeout_ext; > + > + rcu_read_lock(); > + timeout_find_get = rcu_dereference(nf_ct_timeout_find_get_hook); > + if (!timeout_find_get) { > + net_info_ratelimited("Timeout policy base is empty"); > + goto out; > + } > + > + timeout = timeout_find_get(net, timeout_name); > + if (!timeout) { > + net_info_ratelimited("No such timeout policy \"%s\"\n", > + timeout_name); > + goto out; > + } > + > + if (timeout->l3num != l3num) { > + net_info_ratelimited("Timeout policy `%s' can only be used by " > + "L3 protocol number %d\n", timeout_name, > + timeout->l3num); > + goto err_put_timeout; > + } > + > + if (timeout->l4proto->l4proto != l4num) { > + net_info_ratelimited("Timeout policy `%s' can only be used by " > + "L4 protocol number %d\n", timeout_name, > + timeout->l4proto->l4proto); > + goto err_put_timeout; > + } > + > + timeout_ext = nf_ct_timeout_ext_add(ct, timeout, GFP_ATOMIC); > + if (!timeout_ext) > + goto err_put_timeout; > + > + goto out; > + > +err_put_timeout: > + timeout_put = rcu_dereference(nf_ct_timeout_put_hook); > + if (timeout_put) > + timeout_put(timeout); > +out: > + rcu_read_unlock(); > + return; This code looks very similar to xt_ct_set_timeout(), can you refactor it to avoid code duplication? > +#endif > +} > + > /* Lookup connection and confirm if unconfirmed. */ > static int ovs_ct_commit(struct net *net, struct sw_flow_key *key, > const struct ovs_conntrack_info *info, > @@ -1465,6 +1520,8 @@ static const struct ovs_ct_len_tbl ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = { > #endif > [OVS_CT_ATTR_EVENTMASK] = { .minlen = sizeof(u32), > .maxlen = sizeof(u32) }, > + [OVS_CT_ATTR_TIMEOUT] = { .minlen = 1, > + .maxlen = CTNL_TIMEOUT_NAME_MAX }, > }; > > static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info, > @@ -1550,6 +1607,15 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info, > info->have_eventmask = true; > info->eventmask = nla_get_u32(a); > break; > +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT > + case OVS_CT_ATTR_TIMEOUT: > + memcpy(info->timeout, nla_data(a), nla_len(a)); Before copying timeout, we need to check sizeof source string. 'nla_len(a)' needs to be less than CTNL_TIMEOUT_NAME_MAX. otherwise looks good. Thanks.
> > +static void ovs_ct_add_timeout(struct net *net, struct nf_conn *ct, > > + const char *timeout_name, u16 l3num, u8 l4num) > > +{ > This code looks very similar to xt_ct_set_timeout(), can you refactor > it to avoid code duplication? Thanks Prvain's feedback. I will export the set timeout function and use that to avoid duplication. > > static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info, > > @@ -1550,6 +1607,15 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info, > > info->have_eventmask = true; > > info->eventmask = nla_get_u32(a); > > break; > > +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT > > + case OVS_CT_ATTR_TIMEOUT: > > + memcpy(info->timeout, nla_data(a), nla_len(a)); > Before copying timeout, we need to check sizeof source string. > 'nla_len(a)' needs to be less than CTNL_TIMEOUT_NAME_MAX. There are some exiting checking in parse_ct() as below. That should check the sizeof source string already. .... nla_for_each_nested(a, attr, rem) { int type = nla_type(a); int maxlen; int minlen; ...... maxlen = ovs_ct_attr_lens[type].maxlen; minlen = ovs_ct_attr_lens[type].minlen; if (nla_len(a) < minlen || nla_len(a) > maxlen) { OVS_NLERR(log, "Conntrack attr type has unexpected length (type=%d, length=%d, expected=%d)", type, nla_len(a), maxlen); return -EINVAL; } I will respin v2 soon. Thanks, -Yi-Hung
Hi Yi-Hung, Thank you for the patch! Yet something to improve: [auto build test ERROR on net-next/master] url: https://github.com/0day-ci/linux/commits/Yi-Hung-Wei/netfilter-Export-nf_ct_destroy_timeout/20190323-094843 config: i386-randconfig-x004-201911 (attached as .config) compiler: gcc-7 (Debian 7.3.0-1) 7.3.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): net//openvswitch/conntrack.c: In function '__ovs_ct_free_action': >> net//openvswitch/conntrack.c:1854:4: error: implicit declaration of function 'xt_ct_destroy_timeout'; did you mean 'nf_ct_destroy_timeout'? [-Werror=implicit-function-declaration] xt_ct_destroy_timeout(ct_info->ct); ^~~~~~~~~~~~~~~~~~~~~ nf_ct_destroy_timeout cc1: some warnings being treated as errors vim +1854 net//openvswitch/conntrack.c 1846 1847 static void __ovs_ct_free_action(struct ovs_conntrack_info *ct_info) 1848 { 1849 if (ct_info->helper) 1850 nf_conntrack_helper_put(ct_info->helper); 1851 if (ct_info->ct) { 1852 nf_ct_tmpl_free(ct_info->ct); 1853 if (ct_info->timeout[0]) > 1854 xt_ct_destroy_timeout(ct_info->ct); 1855 } 1856 } 1857 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Hi Yi-Hung, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on net-next/master] url: https://github.com/0day-ci/linux/commits/Yi-Hung-Wei/netfilter-Export-nf_ct_destroy_timeout/20190323-094843 reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' sparse warnings: (new ones prefixed by >>) net//openvswitch/conntrack.c:1854:25: sparse: undefined identifier 'xt_ct_destroy_timeout' >> net//openvswitch/conntrack.c:1154:28: sparse: incompatible types in comparison expression (different address spaces) net//openvswitch/conntrack.c:1188:23: sparse: incompatible types in comparison expression (different address spaces) include/linux/slab.h:664:13: sparse: undefined identifier '__builtin_mul_overflow' >> net//openvswitch/conntrack.c:1854:46: sparse: call with no type! include/linux/slab.h:664:13: sparse: call with no type! vim +1154 net//openvswitch/conntrack.c 1143 1144 static void ovs_ct_add_timeout(struct net *net, struct nf_conn *ct, 1145 const char *timeout_name, u16 l3num, u8 l4num) 1146 { 1147 #ifdef CONFIG_NF_CONNTRACK_TIMEOUT 1148 typeof(nf_ct_timeout_find_get_hook) timeout_find_get; 1149 typeof(nf_ct_timeout_put_hook) timeout_put; 1150 struct nf_ct_timeout *timeout; 1151 struct nf_conn_timeout *timeout_ext; 1152 1153 rcu_read_lock(); > 1154 timeout_find_get = rcu_dereference(nf_ct_timeout_find_get_hook); 1155 if (!timeout_find_get) { 1156 net_info_ratelimited("Timeout policy base is empty"); 1157 goto out; 1158 } 1159 1160 timeout = timeout_find_get(net, timeout_name); 1161 if (!timeout) { 1162 net_info_ratelimited("No such timeout policy \"%s\"\n", 1163 timeout_name); 1164 goto out; 1165 } 1166 1167 if (timeout->l3num != l3num) { 1168 net_info_ratelimited("Timeout policy `%s' can only be used by " 1169 "L3 protocol number %d\n", timeout_name, 1170 timeout->l3num); 1171 goto err_put_timeout; 1172 } 1173 1174 if (timeout->l4proto->l4proto != l4num) { 1175 net_info_ratelimited("Timeout policy `%s' can only be used by " 1176 "L4 protocol number %d\n", timeout_name, 1177 timeout->l4proto->l4proto); 1178 goto err_put_timeout; 1179 } 1180 1181 timeout_ext = nf_ct_timeout_ext_add(ct, timeout, GFP_ATOMIC); 1182 if (!timeout_ext) 1183 goto err_put_timeout; 1184 1185 goto out; 1186 1187 err_put_timeout: 1188 timeout_put = rcu_dereference(nf_ct_timeout_put_hook); 1189 if (timeout_put) 1190 timeout_put(timeout); 1191 out: 1192 rcu_read_unlock(); 1193 return; 1194 #endif 1195 } 1196 --- 0-DAY kernel test infrastructure Open Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
diff --git a/include/uapi/linux/openvswitch.h b/include/uapi/linux/openvswitch.h index dbe0cbe4f1b7..9bccc6b9ed3d 100644 --- a/include/uapi/linux/openvswitch.h +++ b/include/uapi/linux/openvswitch.h @@ -734,6 +734,7 @@ struct ovs_action_hash { * be received on NFNLGRP_CONNTRACK_NEW and NFNLGRP_CONNTRACK_DESTROY groups, * respectively. Remaining bits control the changes for which an event is * delivered on the NFNLGRP_CONNTRACK_UPDATE group. + * @OVS_CT_ATTR_TIMEOUT: Variable length string defining conntrack timeout. */ enum ovs_ct_attr { OVS_CT_ATTR_UNSPEC, @@ -746,6 +747,8 @@ enum ovs_ct_attr { OVS_CT_ATTR_NAT, /* Nested OVS_NAT_ATTR_* */ OVS_CT_ATTR_FORCE_COMMIT, /* No argument */ OVS_CT_ATTR_EVENTMASK, /* u32 mask of IPCT_* events. */ + OVS_CT_ATTR_TIMEOUT, /* Associate timeout with this connection for + fine-grain timeout tuning. */ __OVS_CT_ATTR_MAX }; diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c index 1b6896896fff..10a2c73f22f2 100644 --- a/net/openvswitch/conntrack.c +++ b/net/openvswitch/conntrack.c @@ -24,6 +24,7 @@ #include <net/netfilter/nf_conntrack_helper.h> #include <net/netfilter/nf_conntrack_labels.h> #include <net/netfilter/nf_conntrack_seqadj.h> +#include <net/netfilter/nf_conntrack_timeout.h> #include <net/netfilter/nf_conntrack_zones.h> #include <net/netfilter/ipv6/nf_defrag_ipv6.h> #include <net/ipv6_frag.h> @@ -73,6 +74,7 @@ struct ovs_conntrack_info { u32 eventmask; /* Mask of 1 << IPCT_*. */ struct md_mark mark; struct md_labels labels; + char timeout[CTNL_TIMEOUT_NAME_MAX]; #ifdef CONFIG_NF_NAT_NEEDED struct nf_nat_range2 range; /* Only present for SRC NAT and DST NAT. */ #endif @@ -1139,6 +1141,59 @@ static int ovs_ct_check_limit(struct net *net, } #endif +static void ovs_ct_add_timeout(struct net *net, struct nf_conn *ct, + const char *timeout_name, u16 l3num, u8 l4num) +{ +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT + typeof(nf_ct_timeout_find_get_hook) timeout_find_get; + typeof(nf_ct_timeout_put_hook) timeout_put; + struct nf_ct_timeout *timeout; + struct nf_conn_timeout *timeout_ext; + + rcu_read_lock(); + timeout_find_get = rcu_dereference(nf_ct_timeout_find_get_hook); + if (!timeout_find_get) { + net_info_ratelimited("Timeout policy base is empty"); + goto out; + } + + timeout = timeout_find_get(net, timeout_name); + if (!timeout) { + net_info_ratelimited("No such timeout policy \"%s\"\n", + timeout_name); + goto out; + } + + if (timeout->l3num != l3num) { + net_info_ratelimited("Timeout policy `%s' can only be used by " + "L3 protocol number %d\n", timeout_name, + timeout->l3num); + goto err_put_timeout; + } + + if (timeout->l4proto->l4proto != l4num) { + net_info_ratelimited("Timeout policy `%s' can only be used by " + "L4 protocol number %d\n", timeout_name, + timeout->l4proto->l4proto); + goto err_put_timeout; + } + + timeout_ext = nf_ct_timeout_ext_add(ct, timeout, GFP_ATOMIC); + if (!timeout_ext) + goto err_put_timeout; + + goto out; + +err_put_timeout: + timeout_put = rcu_dereference(nf_ct_timeout_put_hook); + if (timeout_put) + timeout_put(timeout); +out: + rcu_read_unlock(); + return; +#endif +} + /* Lookup connection and confirm if unconfirmed. */ static int ovs_ct_commit(struct net *net, struct sw_flow_key *key, const struct ovs_conntrack_info *info, @@ -1465,6 +1520,8 @@ static const struct ovs_ct_len_tbl ovs_ct_attr_lens[OVS_CT_ATTR_MAX + 1] = { #endif [OVS_CT_ATTR_EVENTMASK] = { .minlen = sizeof(u32), .maxlen = sizeof(u32) }, + [OVS_CT_ATTR_TIMEOUT] = { .minlen = 1, + .maxlen = CTNL_TIMEOUT_NAME_MAX }, }; static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info, @@ -1550,6 +1607,15 @@ static int parse_ct(const struct nlattr *attr, struct ovs_conntrack_info *info, info->have_eventmask = true; info->eventmask = nla_get_u32(a); break; +#ifdef CONFIG_NF_CONNTRACK_TIMEOUT + case OVS_CT_ATTR_TIMEOUT: + memcpy(info->timeout, nla_data(a), nla_len(a)); + if (!memchr(info->timeout, '\0', nla_len(a))) { + OVS_NLERR(log, "Invalid conntrack helper"); + return -EINVAL; + } + break; +#endif default: OVS_NLERR(log, "Unknown conntrack attr (%d)", @@ -1631,6 +1697,12 @@ int ovs_ct_copy_action(struct net *net, const struct nlattr *attr, OVS_NLERR(log, "Failed to allocate conntrack template"); return -ENOMEM; } + + if (ct_info.timeout[0]) { + ovs_ct_add_timeout(net, ct_info.ct, ct_info.timeout, family, + key->ip.proto); + } + if (helper) { err = ovs_ct_add_helper(&ct_info, helper, key, log); if (err) @@ -1751,6 +1823,10 @@ int ovs_ct_action_to_attr(const struct ovs_conntrack_info *ct_info, if (ct_info->have_eventmask && nla_put_u32(skb, OVS_CT_ATTR_EVENTMASK, ct_info->eventmask)) return -EMSGSIZE; + if (ct_info->timeout[0]) { + if (nla_put_string(skb, OVS_CT_ATTR_TIMEOUT, ct_info->timeout)) + return -EMSGSIZE; + } #ifdef CONFIG_NF_NAT_NEEDED if (ct_info->nat && !ovs_ct_nat_to_attr(ct_info, skb)) @@ -1772,8 +1848,11 @@ static void __ovs_ct_free_action(struct ovs_conntrack_info *ct_info) { if (ct_info->helper) nf_conntrack_helper_put(ct_info->helper); - if (ct_info->ct) + if (ct_info->ct) { nf_ct_tmpl_free(ct_info->ct); + if (ct_info->timeout[0]) + xt_ct_destroy_timeout(ct_info->ct); + } } #if IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
Add support for fine-grain timeout support to conntrack action. The new OVS_CT_ATTR_TIMEOUT attribute of the conntrack action specifies a timeout to be associated with this connection. If no timeout is specified, it acts as is, that is the default timeout for the connection will be automatically applied. Example usage: $ nfct timeout add timeout_1 inet tcp syn_sent 100 established 200 $ ovs-ofctl add-flow br0 in_port=1,ip,tcp,action=ct(commit,timeout=timeout_1) CC: Pravin Shelar <pshelar@ovn.org> Signed-off-by: Yi-Hung Wei <yihung.wei@gmail.com> --- include/uapi/linux/openvswitch.h | 3 ++ net/openvswitch/conntrack.c | 81 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 1 deletion(-)