Re: [ovs-dev] [PATCH 1/3] ovn-controller: Add extend_table instead of group_table to expand meter.

[Miguel Angel Ajo Pelayo]

Acked-By: Miguel Angel Ajo 

The refactor looks good to me.

On Wed, Jan 24, 2018 at 2:40 AM Ben Pfaff  wrote:

> From: Guoshuai Li 
>
> The structure and function of the group table and meter table are similar,
> refactoring code is used to extend for add the meter table.
> The following function as lib: table init/destroy/clear,
> install contents from desired, remove contents from existing,
> Move the contents of desired to existing.
>
> Signed-off-by: Guoshuai Li 
> Signed-off-by: Ben Pfaff 
> ---
>  include/ovn/actions.h   |  21 +
>  ovn/controller/lflow.c  |   8 +-
>  ovn/controller/lflow.h  |   4 +-
>  ovn/controller/ofctrl.c | 180 +---
>  ovn/controller/ofctrl.h |   7 +-
>  ovn/controller/ovn-controller.c |  20 +---
>  ovn/lib/actions.c   |  58 ++--
>  ovn/lib/automake.mk |   2 +
>  ovn/lib/extend-table.c  | 198
> 
>  ovn/lib/extend-table.h  |  69 ++
>  tests/test-ovn.c|   8 +-
>  11 files changed, 356 insertions(+), 219 deletions(-)
>  create mode 100644 ovn/lib/extend-table.c
>  create mode 100644 ovn/lib/extend-table.h
>
> diff --git a/include/ovn/actions.h b/include/ovn/actions.h
> index 85a484ffac20..ea90dbb2a69a 100644
> --- a/include/ovn/actions.h
> +++ b/include/ovn/actions.h
> @@ -31,6 +31,7 @@ struct lexer;
>  struct ofpbuf;
>  struct shash;
>  struct simap;
> +struct ovn_extend_table;
>
>  /* List of OVN logical actions.
>   *
> @@ -338,24 +339,6 @@ void *ovnact_put(struct ofpbuf *, enum ovnact_type,
> size_t len);
>  OVNACTS
>  #undef OVNACT
>
> -#define MAX_OVN_GROUPS 65535
> -
> -struct group_table {
> -unsigned long *group_ids;  /* Used as a bitmap with value set
> -* for allocated group ids in either
> -* desired_groups or existing_groups. */
> -struct hmap desired_groups;
> -struct hmap existing_groups;
> -};
> -
> -struct group_info {
> -struct hmap_node hmap_node;
> -struct ds group;
> -uint32_t group_id;
> -bool new_group_id;  /* 'True' if 'group_id' was reserved from
> - * group_table's 'group_ids' bitmap. */
> -};
> -
>  enum action_opcode {
>  /* "arp { ...actions... }".
>   *
> @@ -505,7 +488,7 @@ struct ovnact_encode_params {
>  bool is_gateway_router;
>
>  /* A struct to figure out the group_id for group actions. */
> -struct group_table *group_table;
> +struct ovn_extend_table *group_table;
>
>  /* OVN maps each logical flow table (ltable), one-to-one, onto a
> physical
>   * OpenFlow flow table (ptable).  A number of parameters describe this
> diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
> index a62ec6ebe09f..3d990c49c195 100644
> --- a/ovn/controller/lflow.c
> +++ b/ovn/controller/lflow.c
> @@ -61,7 +61,7 @@ static void consider_logical_flow(struct controller_ctx
> *ctx,
>const struct chassis_index
> *chassis_index,
>const struct sbrec_logical_flow *lflow,
>const struct hmap *local_datapaths,
> -  struct group_table *group_table,
> +  struct ovn_extend_table *group_table,
>const struct sbrec_chassis *chassis,
>struct hmap *dhcp_opts,
>struct hmap *dhcpv6_opts,
> @@ -143,7 +143,7 @@ static void
>  add_logical_flows(struct controller_ctx *ctx,
>const struct chassis_index *chassis_index,
>const struct hmap *local_datapaths,
> -  struct group_table *group_table,
> +  struct ovn_extend_table *group_table,
>const struct sbrec_chassis *chassis,
>const struct shash *addr_sets,
>struct hmap *flow_table,
> @@ -190,7 +190,7 @@ consider_logical_flow(struct controller_ctx *ctx,
>const struct chassis_index *chassis_index,
>const struct sbrec_logical_flow *lflow,
>const struct hmap *local_datapaths,
> -  struct group_table *group_table,
> +  struct ovn_extend_table *group_table,
>const struct sbrec_chassis *chassis,
>struct hmap *dhcp_opts,
>struct hmap *dhcpv6_opts,
> @@ -434,7 +434,7 @@ lflow_run(struct controller_ctx *ctx,
>const struct sbrec_chassis *chassis,
>const struct chassis_index *chassis_index,
>const struct hmap *local_datapaths,
> -  struct group_table *group_table,
> +  struct ovn_extend_table 

[ovs-dev] [PATCH 1/3] ovn-controller: Add extend_table instead of group_table to expand meter.

[Ben Pfaff]

From: Guoshuai Li 

The structure and function of the group table and meter table are similar,
refactoring code is used to extend for add the meter table.
The following function as lib: table init/destroy/clear,
install contents from desired, remove contents from existing,
Move the contents of desired to existing.

Signed-off-by: Guoshuai Li 
Signed-off-by: Ben Pfaff 
---
 include/ovn/actions.h   |  21 +
 ovn/controller/lflow.c  |   8 +-
 ovn/controller/lflow.h  |   4 +-
 ovn/controller/ofctrl.c | 180 +---
 ovn/controller/ofctrl.h |   7 +-
 ovn/controller/ovn-controller.c |  20 +---
 ovn/lib/actions.c   |  58 ++--
 ovn/lib/automake.mk |   2 +
 ovn/lib/extend-table.c  | 198 
 ovn/lib/extend-table.h  |  69 ++
 tests/test-ovn.c|   8 +-
 11 files changed, 356 insertions(+), 219 deletions(-)
 create mode 100644 ovn/lib/extend-table.c
 create mode 100644 ovn/lib/extend-table.h

diff --git a/include/ovn/actions.h b/include/ovn/actions.h
index 85a484ffac20..ea90dbb2a69a 100644
--- a/include/ovn/actions.h
+++ b/include/ovn/actions.h
@@ -31,6 +31,7 @@ struct lexer;
 struct ofpbuf;
 struct shash;
 struct simap;
+struct ovn_extend_table;
 
 /* List of OVN logical actions.
  *
@@ -338,24 +339,6 @@ void *ovnact_put(struct ofpbuf *, enum ovnact_type, size_t 
len);
 OVNACTS
 #undef OVNACT
 
-#define MAX_OVN_GROUPS 65535
-
-struct group_table {
-unsigned long *group_ids;  /* Used as a bitmap with value set
-* for allocated group ids in either
-* desired_groups or existing_groups. */
-struct hmap desired_groups;
-struct hmap existing_groups;
-};
-
-struct group_info {
-struct hmap_node hmap_node;
-struct ds group;
-uint32_t group_id;
-bool new_group_id;  /* 'True' if 'group_id' was reserved from
- * group_table's 'group_ids' bitmap. */
-};
-
 enum action_opcode {
 /* "arp { ...actions... }".
  *
@@ -505,7 +488,7 @@ struct ovnact_encode_params {
 bool is_gateway_router;
 
 /* A struct to figure out the group_id for group actions. */
-struct group_table *group_table;
+struct ovn_extend_table *group_table;
 
 /* OVN maps each logical flow table (ltable), one-to-one, onto a physical
  * OpenFlow flow table (ptable).  A number of parameters describe this
diff --git a/ovn/controller/lflow.c b/ovn/controller/lflow.c
index a62ec6ebe09f..3d990c49c195 100644
--- a/ovn/controller/lflow.c
+++ b/ovn/controller/lflow.c
@@ -61,7 +61,7 @@ static void consider_logical_flow(struct controller_ctx *ctx,
   const struct chassis_index *chassis_index,
   const struct sbrec_logical_flow *lflow,
   const struct hmap *local_datapaths,
-  struct group_table *group_table,
+  struct ovn_extend_table *group_table,
   const struct sbrec_chassis *chassis,
   struct hmap *dhcp_opts,
   struct hmap *dhcpv6_opts,
@@ -143,7 +143,7 @@ static void
 add_logical_flows(struct controller_ctx *ctx,
   const struct chassis_index *chassis_index,
   const struct hmap *local_datapaths,
-  struct group_table *group_table,
+  struct ovn_extend_table *group_table,
   const struct sbrec_chassis *chassis,
   const struct shash *addr_sets,
   struct hmap *flow_table,
@@ -190,7 +190,7 @@ consider_logical_flow(struct controller_ctx *ctx,
   const struct chassis_index *chassis_index,
   const struct sbrec_logical_flow *lflow,
   const struct hmap *local_datapaths,
-  struct group_table *group_table,
+  struct ovn_extend_table *group_table,
   const struct sbrec_chassis *chassis,
   struct hmap *dhcp_opts,
   struct hmap *dhcpv6_opts,
@@ -434,7 +434,7 @@ lflow_run(struct controller_ctx *ctx,
   const struct sbrec_chassis *chassis,
   const struct chassis_index *chassis_index,
   const struct hmap *local_datapaths,
-  struct group_table *group_table,
+  struct ovn_extend_table *group_table,
   const struct shash *addr_sets,
   struct hmap *flow_table,
   struct sset *active_tunnels,
diff --git a/ovn/controller/lflow.h b/ovn/controller/lflow.h
index bfb7415e2b13..087b0ed8da35 100644
--- a/ovn/controller/lflow.h
+++ b/ovn/controller/lflow.h
@@ -37,7 +37,7 @@
 
 struct chassis_index;
 struct controller_ctx;
-struct