Re: [PATCH net-next 2/2] openvswitch: Support conntrack zone limit

2018-04-18 Thread Gregory Rose 

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

2018-04-17 Thread Yi-Hung Wei 
> 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)

2018-04-16 Thread Julia Lawall 
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 <l...@intel.com>
To: kbu...@01.org
Cc: Julia Lawall <julia.law...@lip6.fr>
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 <yihung@gmail.com>
CC: netdev@vger.kernel.org
CC: Yi-Hung Wei <yihung@gmail.com>

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(_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

2018-04-16 Thread Gregory Rose 

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, },
+};

[PATCH net-next 2/2] openvswitch: Support conntrack zone limit

2018-04-16 Thread Yi-Hung Wei 
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
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 
---
 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
 };
 
+#ifIS_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 labels_nonzero(const struct ovs_key_ct_labels *labels);
 
 static void __ovs_ct_free_action(struct ovs_conntrack_info *ct_info);
@@ -1036,6 +1070,94 @@ static bool labels_nonzero(const struct 
ovs_key_ct_labels *labels)
return false;
 }
 
+#ifIS_ENABLED(CONFIG_NETFILTER_CONNCOUNT)
+static struct hlist_head *ct_limit_hash_bucket(
+   const struct ovs_ct_limit_info *info, u16 zone)
+{
+   return >limits[zone & (CT_LIMIT_HASH_BUCKETS - 1)];
+}
+
+/* Call with ovs_mutex */
+static