Re: [PATCH net-next 2/2] openvswitch: Support conntrack zone limit
On 4/17/2018 5:30 PM, Yi-Hung Wei wrote: s/to commit/from committing/ s/entry/entries/ Thanks, will fix that in both patches in v2. I think this is a great idea but I suggest porting to the iproute2 package so everyone can use it. Then git rid of the OVS specific prefixes. Presuming of course that the conntrack connection limit backend works there as well I guess. If it doesn't, then I'd suggest extending it. This is a nice feature for all users in my opinion and then OVS can take advantage of it as well. Thanks for the comment. And yes, I think currently, iptables’s connlimit extension does support limiting the # of connections. Users need to configure the zone properly, and the iptable’s connlimit extension is using netfilter's nf_conncount backend already. The main goal for this patch is to utilize netfilter backend (nf_conncount) to count and limit the number of connections. OVS needs the proposed OVS_CT_LIMIT netlink API and the corresponding booking data structure because the current nf_conncount backend only counts the # of connections, but it does not keep track of the connection limit in nf_conncount. Thanks, -Yi-Hung Thanks Yi-hung, I figured I was just missing something there. I appreciate the explanation. - Greg
Re: [PATCH net-next 2/2] openvswitch: Support conntrack zone limit
> s/to commit/from committing/ > s/entry/entries/ Thanks, will fix that in both patches in v2. > I think this is a great idea but I suggest porting to the iproute2 package > so everyone can use it. Then git rid of the OVS specific prefixes. > Presuming of course that the conntrack connection > limit backend works there as well I guess. If it doesn't, then I'd suggest > extending > it. This is a nice feature for all users in my opinion and then OVS > can take advantage of it as well. Thanks for the comment. And yes, I think currently, iptables’s connlimit extension does support limiting the # of connections. Users need to configure the zone properly, and the iptable’s connlimit extension is using netfilter's nf_conncount backend already. The main goal for this patch is to utilize netfilter backend (nf_conncount) to count and limit the number of connections. OVS needs the proposed OVS_CT_LIMIT netlink API and the corresponding booking data structure because the current nf_conncount backend only counts the # of connections, but it does not keep track of the connection limit in nf_conncount. Thanks, -Yi-Hung
Re: [PATCH net-next 2/2] openvswitch: Support conntrack zone limit (fwd)
Line 1814 frees something that is dereferenced on the next line. julia -- Forwarded message -- Date: Tue, 17 Apr 2018 10:32:17 +0800 From: kbuild test robot To: kbu...@01.org Cc: Julia Lawall Subject: Re: [PATCH net-next 2/2] openvswitch: Support conntrack zone limit CC: kbuild-...@01.org In-Reply-To: <1523902550-10767-3-git-send-email-yihung@gmail.com> References: <1523902550-10767-3-git-send-email-yihung@gmail.com> TO: Yi-Hung Wei CC: netdev@vger.kernel.org CC: Yi-Hung Wei 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/openvswitch-Support-conntrack-zone-limit/20180417-085035 :: branch date: 2 hours ago :: commit date: 2 hours ago >> net/openvswitch/conntrack.c:1815:17-39: ERROR: reference preceded by free on >> line 1814 # https://github.com/0day-ci/linux/commit/01487f05f032565b952e344abace672a12045d9e git remote add linux-review https://github.com/0day-ci/linux git remote update linux-review git checkout 01487f05f032565b952e344abace672a12045d9e vim +1815 net/openvswitch/conntrack.c c2ac6673 Joe Stringer 2015-08-26 1786 01487f05 Yi-Hung Wei 2018-04-16 1787 #if IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT) 01487f05 Yi-Hung Wei 2018-04-16 1788 static int ovs_ct_limit_init(struct net *net, struct ovs_net *ovs_net) 01487f05 Yi-Hung Wei 2018-04-16 1789 { 01487f05 Yi-Hung Wei 2018-04-16 1790 int i; 01487f05 Yi-Hung Wei 2018-04-16 1791 01487f05 Yi-Hung Wei 2018-04-16 1792 ovs_net->ct_limit_info = kmalloc(sizeof *ovs_net->ct_limit_info, 01487f05 Yi-Hung Wei 2018-04-16 1793 GFP_KERNEL); 01487f05 Yi-Hung Wei 2018-04-16 1794 if (!ovs_net->ct_limit_info) 01487f05 Yi-Hung Wei 2018-04-16 1795 return -ENOMEM; 01487f05 Yi-Hung Wei 2018-04-16 1796 01487f05 Yi-Hung Wei 2018-04-16 1797 ovs_net->ct_limit_info->default_limit = OVS_CT_LIMIT_DEFAULT; 01487f05 Yi-Hung Wei 2018-04-16 1798 ovs_net->ct_limit_info->limits = 01487f05 Yi-Hung Wei 2018-04-16 1799 kmalloc_array(CT_LIMIT_HASH_BUCKETS, sizeof(struct hlist_head), 01487f05 Yi-Hung Wei 2018-04-16 1800 GFP_KERNEL); 01487f05 Yi-Hung Wei 2018-04-16 1801 if (!ovs_net->ct_limit_info->limits) { 01487f05 Yi-Hung Wei 2018-04-16 1802 kfree(ovs_net->ct_limit_info); 01487f05 Yi-Hung Wei 2018-04-16 1803 return -ENOMEM; 01487f05 Yi-Hung Wei 2018-04-16 1804 } 01487f05 Yi-Hung Wei 2018-04-16 1805 01487f05 Yi-Hung Wei 2018-04-16 1806 for (i = 0; i < CT_LIMIT_HASH_BUCKETS; i++) 01487f05 Yi-Hung Wei 2018-04-16 1807 INIT_HLIST_HEAD(&ovs_net->ct_limit_info->limits[i]); 01487f05 Yi-Hung Wei 2018-04-16 1808 01487f05 Yi-Hung Wei 2018-04-16 1809 ovs_net->ct_limit_info->data = 01487f05 Yi-Hung Wei 2018-04-16 1810 nf_conncount_init(net, NFPROTO_INET, sizeof(u32)); 01487f05 Yi-Hung Wei 2018-04-16 1811 01487f05 Yi-Hung Wei 2018-04-16 1812 if (IS_ERR(ovs_net->ct_limit_info->data)) { 01487f05 Yi-Hung Wei 2018-04-16 1813 kfree(ovs_net->ct_limit_info->limits); 01487f05 Yi-Hung Wei 2018-04-16 @1814 kfree(ovs_net->ct_limit_info); 01487f05 Yi-Hung Wei 2018-04-16 @1815 return PTR_ERR(ovs_net->ct_limit_info->data); 01487f05 Yi-Hung Wei 2018-04-16 1816 } 01487f05 Yi-Hung Wei 2018-04-16 1817 return 0; 01487f05 Yi-Hung Wei 2018-04-16 1818 } 01487f05 Yi-Hung Wei 2018-04-16 1819 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: [PATCH net-next 2/2] openvswitch: Support conntrack zone limit
On 4/16/2018 11:15 AM, Yi-Hung Wei wrote: Currently, nf_conntrack_max is used to limit the maximum number of conntrack entries in the conntrack table for every network namespace. For the VMs and containers that reside in the same namespace, they share the same conntrack table, and the total # of conntrack entries for all the VMs and containers are limited by nf_conntrack_max. In this case, if one of the VM/container abuses the usage the conntrack entries, it blocks the others to commit valid conntrack entry into the conntrack s/to commit/from committing/ s/entry/entries/ table. Even if we can possibly put the VM in different network namespace, the current nf_conntrack_max configuration is kind of rigid that we cannot limit different VM/container to have different # conntrack entries. To address the aforementioned issue, this patch proposes to have a fine-grained mechanism that could further limit the # of conntrack entries per-zone. For example, we can designate different zone to different VM, and set conntrack limit to each zone. By providing this isolation, a mis-behaved VM only consumes the conntrack entries in its own zone, and it will not influence other well-behaved VMs. Moreover, the users can set various conntrack limit to different zone based on their preference. The proposed implementation utilizes Netfilter's nf_conncount backend to count the number of connections in a particular zone. If the number of connection is above a configured limitation, ovs will return ENOMEM to the userspace. If userspace does not configure the zone limit, the limit defaults to zero that is no limitation, which is backward compatible to the behavior without this patch. The following high leve APIs are provided to the userspace: - OVS_CT_LIMIT_CMD_SET: * set default connection limit for all zones * set the connection limit for a particular zone - OVS_CT_LIMIT_CMD_DEL: * remove the connection limit for a particular zone - OVS_CT_LIMIT_CMD_GET: * get the default connection limit for all zones * get the connection limit for a particular zone Signed-off-by: Yi-Hung Wei I think this is a great idea but I suggest porting to the iproute2 package so everyone can use it. Then git rid of the OVS specific prefixes. Presuming of course that the conntrack connection limit backend works there as well I guess. If it doesn't, then I'd suggest extending it. This is a nice feature for all users in my opinion and then OVS can take advantage of it as well. Thanks! - Greg --- net/openvswitch/Kconfig | 3 +- net/openvswitch/conntrack.c | 497 +++- net/openvswitch/conntrack.h | 9 +- net/openvswitch/datapath.c | 7 +- net/openvswitch/datapath.h | 1 + 5 files changed, 511 insertions(+), 6 deletions(-) diff --git a/net/openvswitch/Kconfig b/net/openvswitch/Kconfig index 2650205cdaf9..89da9512ec1e 100644 --- a/net/openvswitch/Kconfig +++ b/net/openvswitch/Kconfig @@ -9,7 +9,8 @@ config OPENVSWITCH (NF_CONNTRACK && ((!NF_DEFRAG_IPV6 || NF_DEFRAG_IPV6) && \ (!NF_NAT || NF_NAT) && \ (!NF_NAT_IPV4 || NF_NAT_IPV4) && \ -(!NF_NAT_IPV6 || NF_NAT_IPV6))) +(!NF_NAT_IPV6 || NF_NAT_IPV6) && \ +(!NETFILTER_CONNCOUNT || NETFILTER_CONNCOUNT))) select LIBCRC32C select MPLS select NET_MPLS_GSO diff --git a/net/openvswitch/conntrack.c b/net/openvswitch/conntrack.c index c5904f629091..2f51da91d056 100644 --- a/net/openvswitch/conntrack.c +++ b/net/openvswitch/conntrack.c @@ -17,7 +17,9 @@ #include #include #include +#include #include +#include #include #include #include @@ -76,6 +78,38 @@ struct ovs_conntrack_info { #endif }; +#if IS_ENABLED(CONFIG_NETFILTER_CONNCOUNT) +#define OVS_CT_LIMIT_UNLIMITED 0 +#define OVS_CT_LIMIT_DEFAULT OVS_CT_LIMIT_UNLIMITED +#define CT_LIMIT_HASH_BUCKETS 512 + +struct ovs_ct_limit { + /* Elements in ovs_ct_limit_info->limits hash table */ + struct hlist_node hlist_node; + struct rcu_head rcu; + u16 zone; + u32 limit; +}; + +struct ovs_ct_limit_info { + u32 default_limit; + struct hlist_head *limits; + struct nf_conncount_data *data __aligned(8); +}; + +static const struct nla_policy ct_limit_policy[OVS_CT_LIMIT_ATTR_MAX + 1] = { + [OVS_CT_LIMIT_ATTR_OPTION] = { .type = NLA_NESTED, }, +}; + +static const struct nla_policy + ct_zone_limit_policy[OVS_CT_ZONE_LIMIT_ATTR_MAX + 1] = { + [OVS_CT_ZONE_LIMIT_ATTR_DEFAULT_LIMIT] = { .type = NLA_U32, }, + [OVS_CT_ZONE_LIMIT_ATTR_ZONE] = { .type = NLA_U16, }, + [OVS_CT_ZONE_LIMIT_ATTR_LIMIT] = { .type = NLA_U32, }, + [OVS_CT_ZONE_LIMIT_ATTR_COUNT] = { .type = NLA_U32, }, +}; +#endif + static bool label