Re: [ovs-dev] [PATCH net-next v3 1/5] net: openvswitch: expand the meters supported number

2020-04-22 Thread Pravin Shelar
On Wed, Apr 22, 2020 at 10:10 AM  wrote:
>
> From: Tonghao Zhang 
>
> In kernel datapath of Open vSwitch, there are only 1024
> buckets of meter in one datapath. If installing more than
> 1024 (e.g. 8192) meters, it may lead to the performance drop.
> But in some case, for example, Open vSwitch used as edge
> gateway, there should be 20K at least, where meters used for
> IP address bandwidth limitation.
>
> [Open vSwitch userspace datapath has this issue too.]
>
> For more scalable meter, this patch use meter array instead of
> hash tables, and expand/shrink the array when necessary. So we
> can install more meters than before in the datapath.
> Introducing the struct *dp_meter_instance, it's easy to
> expand meter though changing the *ti point in the struct
> *dp_meter_table.
>
> Cc: Pravin B Shelar 
> Cc: Andy Zhou 
> Signed-off-by: Tonghao Zhang 
> ---
>  net/openvswitch/datapath.h |   2 +-
>  net/openvswitch/meter.c| 240 -
>  net/openvswitch/meter.h|  16 ++-
>  3 files changed, 195 insertions(+), 63 deletions(-)
>
Acked-by: Pravin B Shelar 

Thanks.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


[ovs-dev] [PATCH net-next v3 1/5] net: openvswitch: expand the meters supported number

2020-04-22 Thread xiangxia . m . yue
From: Tonghao Zhang 

In kernel datapath of Open vSwitch, there are only 1024
buckets of meter in one datapath. If installing more than
1024 (e.g. 8192) meters, it may lead to the performance drop.
But in some case, for example, Open vSwitch used as edge
gateway, there should be 20K at least, where meters used for
IP address bandwidth limitation.

[Open vSwitch userspace datapath has this issue too.]

For more scalable meter, this patch use meter array instead of
hash tables, and expand/shrink the array when necessary. So we
can install more meters than before in the datapath.
Introducing the struct *dp_meter_instance, it's easy to
expand meter though changing the *ti point in the struct
*dp_meter_table.

Cc: Pravin B Shelar 
Cc: Andy Zhou 
Signed-off-by: Tonghao Zhang 
---
 net/openvswitch/datapath.h |   2 +-
 net/openvswitch/meter.c| 240 -
 net/openvswitch/meter.h|  16 ++-
 3 files changed, 195 insertions(+), 63 deletions(-)

diff --git a/net/openvswitch/datapath.h b/net/openvswitch/datapath.h
index e239a46c2f94..2016dd107939 100644
--- a/net/openvswitch/datapath.h
+++ b/net/openvswitch/datapath.h
@@ -82,7 +82,7 @@ struct datapath {
u32 max_headroom;
 
/* Switch meters. */
-   struct hlist_head *meters;
+   struct dp_meter_table meter_tbl;
 };
 
 /**
diff --git a/net/openvswitch/meter.c b/net/openvswitch/meter.c
index 5010d1ddd4bd..f806ded1dd0a 100644
--- a/net/openvswitch/meter.c
+++ b/net/openvswitch/meter.c
@@ -19,8 +19,6 @@
 #include "datapath.h"
 #include "meter.h"
 
-#define METER_HASH_BUCKETS 1024
-
 static const struct nla_policy meter_policy[OVS_METER_ATTR_MAX + 1] = {
[OVS_METER_ATTR_ID] = { .type = NLA_U32, },
[OVS_METER_ATTR_KBPS] = { .type = NLA_FLAG },
@@ -39,6 +37,11 @@ static const struct nla_policy band_policy[OVS_BAND_ATTR_MAX 
+ 1] = {
[OVS_BAND_ATTR_STATS] = { .len = sizeof(struct ovs_flow_stats) },
 };
 
+static u32 meter_hash(struct dp_meter_instance *ti, u32 id)
+{
+   return id % ti->n_meters;
+}
+
 static void ovs_meter_free(struct dp_meter *meter)
 {
if (!meter)
@@ -47,40 +50,153 @@ static void ovs_meter_free(struct dp_meter *meter)
kfree_rcu(meter, rcu);
 }
 
-static struct hlist_head *meter_hash_bucket(const struct datapath *dp,
-   u32 meter_id)
-{
-   return >meters[meter_id & (METER_HASH_BUCKETS - 1)];
-}
-
 /* Call with ovs_mutex or RCU read lock. */
-static struct dp_meter *lookup_meter(const struct datapath *dp,
+static struct dp_meter *lookup_meter(const struct dp_meter_table *tbl,
 u32 meter_id)
 {
+   struct dp_meter_instance *ti = rcu_dereference_ovsl(tbl->ti);
+   u32 hash = meter_hash(ti, meter_id);
struct dp_meter *meter;
-   struct hlist_head *head;
 
-   head = meter_hash_bucket(dp, meter_id);
-   hlist_for_each_entry_rcu(meter, head, dp_hash_node,
-   lockdep_ovsl_is_held()) {
-   if (meter->id == meter_id)
-   return meter;
-   }
+   meter = rcu_dereference_ovsl(ti->dp_meters[hash]);
+   if (meter && likely(meter->id == meter_id))
+   return meter;
+
return NULL;
 }
 
-static void attach_meter(struct datapath *dp, struct dp_meter *meter)
+static struct dp_meter_instance *dp_meter_instance_alloc(const u32 size)
+{
+   struct dp_meter_instance *ti;
+
+   ti = kvzalloc(sizeof(*ti) +
+ sizeof(struct dp_meter *) * size,
+ GFP_KERNEL);
+   if (!ti)
+   return NULL;
+
+   ti->n_meters = size;
+
+   return ti;
+}
+
+static void dp_meter_instance_free(struct dp_meter_instance *ti)
+{
+   kvfree(ti);
+}
+
+static void dp_meter_instance_free_rcu(struct rcu_head *rcu)
+{
+   struct dp_meter_instance *ti;
+
+   ti = container_of(rcu, struct dp_meter_instance, rcu);
+   kvfree(ti);
+}
+
+static int
+dp_meter_instance_realloc(struct dp_meter_table *tbl, u32 size)
+{
+   struct dp_meter_instance *ti = rcu_dereference_ovsl(tbl->ti);
+   int n_meters = min(size, ti->n_meters);
+   struct dp_meter_instance *new_ti;
+   int i;
+
+   new_ti = dp_meter_instance_alloc(size);
+   if (!new_ti)
+   return -ENOMEM;
+
+   for (i = 0; i < n_meters; i++)
+   new_ti->dp_meters[i] =
+   rcu_dereference_ovsl(ti->dp_meters[i]);
+
+   rcu_assign_pointer(tbl->ti, new_ti);
+   call_rcu(>rcu, dp_meter_instance_free_rcu);
+
+   return 0;
+}
+
+static void dp_meter_instance_insert(struct dp_meter_instance *ti,
+struct dp_meter *meter)
+{
+   u32 hash;
+
+   hash = meter_hash(ti, meter->id);
+   rcu_assign_pointer(ti->dp_meters[hash], meter);
+}
+
+static void dp_meter_instance_remove(struct dp_meter_instance *ti,
+struct dp_meter *meter)
 {