Re: [ovs-dev] [PATCH ovn 1/2] Refactor lflow functions to take one context argument - lflow_ctx.
On Sat, Jan 11, 2020 at 11:02 PM Han Zhou wrote:
>
> On Fri, Jan 10, 2020 at 11:26 AM wrote:
> >
> > From: Numan Siddique
> >
> > Presently most of the lflow_*() functions called from ovn-controller.c
> > takes lots of arguments. This patch adds 'struct lflow_ctx' to simplify
> > the code a bit. This also reduces some code in ovn-controller.c and
> > improves readability to some degree.
> >
> > No functional changes are introduced in this patch.
>
> Thanks Numan for the patch. Please see my comments below.
>
> >
> > Signed-off-by: Numan Siddique
> > ---
> > controller/lflow.c | 255 ++--
> > controller/lflow.h | 83
> > controller/ovn-controller.c | 240 -
> > 3 files changed, 186 insertions(+), 392 deletions(-)
> >
> > diff --git a/controller/lflow.c b/controller/lflow.c
> > index a6893201e..311f8e2be 100644
> > --- a/controller/lflow.c
> > +++ b/controller/lflow.c
> > @@ -61,25 +61,12 @@ struct condition_aux {
> > const struct sset *active_tunnels;
> > };
> >
> > -static bool consider_logical_flow(
> > -struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath,
> > -struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > -const struct sbrec_logical_flow *,
> > -const struct hmap *local_datapaths,
> > -const struct sbrec_chassis *,
> > -struct hmap *dhcp_opts,
> > -struct hmap *dhcpv6_opts,
> > -struct hmap *nd_ra_opts,
> > -struct controller_event_options *controller_event_opts,
> > -const struct shash *addr_sets,
> > -const struct shash *port_groups,
> > -const struct sset *active_tunnels,
> > -const struct sset *local_lport_ids,
> > -struct ovn_desired_flow_table *,
> > -struct ovn_extend_table *group_table,
> > -struct ovn_extend_table *meter_table,
> > -struct lflow_resource_ref *lfrr,
> > -uint32_t *conj_id_ofs);
> > +static bool
> > +consider_logical_flow(struct lflow_ctx *l_ctx,
> > + const struct sbrec_logical_flow *lflow,
> > + struct hmap *dhcp_opts, struct hmap *dhcpv6_opts,
> > + struct hmap *nd_ra_opts,
> > + struct controller_event_options
> *controller_event_opts);
> >
> > static bool
> > lookup_port_cb(const void *aux_, const char *port_name, unsigned int
> *portp)
> > @@ -257,30 +244,15 @@ lflow_resource_destroy_lflow(struct
> lflow_resource_ref *lfrr,
> >
> > /* Adds the logical flows from the Logical_Flow table to flow tables. */
> > static void
> > -add_logical_flows(
> > -struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath,
> > -struct ovsdb_idl_index *sbrec_port_binding_by_name,
> > -const struct sbrec_dhcp_options_table *dhcp_options_table,
> > -const struct sbrec_dhcpv6_options_table *dhcpv6_options_table,
> > -const struct sbrec_logical_flow_table *logical_flow_table,
> > -const struct hmap *local_datapaths,
> > -const struct sbrec_chassis *chassis,
> > -const struct shash *addr_sets,
> > -const struct shash *port_groups,
> > -const struct sset *active_tunnels,
> > -const struct sset *local_lport_ids,
> > -struct ovn_desired_flow_table *flow_table,
> > -struct ovn_extend_table *group_table,
> > -struct ovn_extend_table *meter_table,
> > -struct lflow_resource_ref *lfrr,
> > -uint32_t *conj_id_ofs)
> > +add_logical_flows(struct lflow_ctx *l_ctx)
> > {
> > const struct sbrec_logical_flow *lflow;
> >
> > struct hmap dhcp_opts = HMAP_INITIALIZER(_opts);
> > struct hmap dhcpv6_opts = HMAP_INITIALIZER(_opts);
> > const struct sbrec_dhcp_options *dhcp_opt_row;
> > -SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row, dhcp_options_table)
> {
> > +SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row,
> > + l_ctx->dhcp_options_table) {
> > dhcp_opt_add(_opts, dhcp_opt_row->name, dhcp_opt_row->code,
> > dhcp_opt_row->type);
> > }
> > @@ -288,7 +260,7 @@ add_logical_flows(
> >
> > const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
> > SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH (dhcpv6_opt_row,
> > - dhcpv6_options_table) {
> > + l_ctx->dhcpv6_options_table) {
> > dhcp_opt_add(_opts, dhcpv6_opt_row->name,
> dhcpv6_opt_row->code,
> > dhcpv6_opt_row->type);
> > }
> > @@ -299,16 +271,9 @@ add_logical_flows(
> > struct controller_event_options controller_event_opts;
> > controller_event_opts_init(_event_opts);
> >
> > -SBREC_LOGICAL_FLOW_TABLE_FOR_EACH (lflow, logical_flow_table) {
> > -if
> (!consider_logical_flow(sbrec_multicast_group_by_name_datapath,
> > - sbrec_port_binding_by_name,
> > - lflow, local_datapaths,
> > -
Re: [ovs-dev] [PATCH ovn 1/2] Refactor lflow functions to take one context argument - lflow_ctx.
On Fri, Jan 10, 2020 at 11:26 AM wrote:
>
> From: Numan Siddique
>
> Presently most of the lflow_*() functions called from ovn-controller.c
> takes lots of arguments. This patch adds 'struct lflow_ctx' to simplify
> the code a bit. This also reduces some code in ovn-controller.c and
> improves readability to some degree.
>
> No functional changes are introduced in this patch.
Thanks Numan for the patch. Please see my comments below.
>
> Signed-off-by: Numan Siddique
> ---
> controller/lflow.c | 255 ++--
> controller/lflow.h | 83
> controller/ovn-controller.c | 240 -
> 3 files changed, 186 insertions(+), 392 deletions(-)
>
> diff --git a/controller/lflow.c b/controller/lflow.c
> index a6893201e..311f8e2be 100644
> --- a/controller/lflow.c
> +++ b/controller/lflow.c
> @@ -61,25 +61,12 @@ struct condition_aux {
> const struct sset *active_tunnels;
> };
>
> -static bool consider_logical_flow(
> -struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath,
> -struct ovsdb_idl_index *sbrec_port_binding_by_name,
> -const struct sbrec_logical_flow *,
> -const struct hmap *local_datapaths,
> -const struct sbrec_chassis *,
> -struct hmap *dhcp_opts,
> -struct hmap *dhcpv6_opts,
> -struct hmap *nd_ra_opts,
> -struct controller_event_options *controller_event_opts,
> -const struct shash *addr_sets,
> -const struct shash *port_groups,
> -const struct sset *active_tunnels,
> -const struct sset *local_lport_ids,
> -struct ovn_desired_flow_table *,
> -struct ovn_extend_table *group_table,
> -struct ovn_extend_table *meter_table,
> -struct lflow_resource_ref *lfrr,
> -uint32_t *conj_id_ofs);
> +static bool
> +consider_logical_flow(struct lflow_ctx *l_ctx,
> + const struct sbrec_logical_flow *lflow,
> + struct hmap *dhcp_opts, struct hmap *dhcpv6_opts,
> + struct hmap *nd_ra_opts,
> + struct controller_event_options
*controller_event_opts);
>
> static bool
> lookup_port_cb(const void *aux_, const char *port_name, unsigned int
*portp)
> @@ -257,30 +244,15 @@ lflow_resource_destroy_lflow(struct
lflow_resource_ref *lfrr,
>
> /* Adds the logical flows from the Logical_Flow table to flow tables. */
> static void
> -add_logical_flows(
> -struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath,
> -struct ovsdb_idl_index *sbrec_port_binding_by_name,
> -const struct sbrec_dhcp_options_table *dhcp_options_table,
> -const struct sbrec_dhcpv6_options_table *dhcpv6_options_table,
> -const struct sbrec_logical_flow_table *logical_flow_table,
> -const struct hmap *local_datapaths,
> -const struct sbrec_chassis *chassis,
> -const struct shash *addr_sets,
> -const struct shash *port_groups,
> -const struct sset *active_tunnels,
> -const struct sset *local_lport_ids,
> -struct ovn_desired_flow_table *flow_table,
> -struct ovn_extend_table *group_table,
> -struct ovn_extend_table *meter_table,
> -struct lflow_resource_ref *lfrr,
> -uint32_t *conj_id_ofs)
> +add_logical_flows(struct lflow_ctx *l_ctx)
> {
> const struct sbrec_logical_flow *lflow;
>
> struct hmap dhcp_opts = HMAP_INITIALIZER(_opts);
> struct hmap dhcpv6_opts = HMAP_INITIALIZER(_opts);
> const struct sbrec_dhcp_options *dhcp_opt_row;
> -SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row, dhcp_options_table)
{
> +SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row,
> + l_ctx->dhcp_options_table) {
> dhcp_opt_add(_opts, dhcp_opt_row->name, dhcp_opt_row->code,
> dhcp_opt_row->type);
> }
> @@ -288,7 +260,7 @@ add_logical_flows(
>
> const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
> SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH (dhcpv6_opt_row,
> - dhcpv6_options_table) {
> + l_ctx->dhcpv6_options_table) {
> dhcp_opt_add(_opts, dhcpv6_opt_row->name,
dhcpv6_opt_row->code,
> dhcpv6_opt_row->type);
> }
> @@ -299,16 +271,9 @@ add_logical_flows(
> struct controller_event_options controller_event_opts;
> controller_event_opts_init(_event_opts);
>
> -SBREC_LOGICAL_FLOW_TABLE_FOR_EACH (lflow, logical_flow_table) {
> -if
(!consider_logical_flow(sbrec_multicast_group_by_name_datapath,
> - sbrec_port_binding_by_name,
> - lflow, local_datapaths,
> - chassis, _opts, _opts,
> - _ra_opts, _event_opts,
> - addr_sets, port_groups,
> - active_tunnels, local_lport_ids,
> - flow_table, group_table,
[ovs-dev] [PATCH ovn 1/2] Refactor lflow functions to take one context argument - lflow_ctx.
From: Numan Siddique
Presently most of the lflow_*() functions called from ovn-controller.c
takes lots of arguments. This patch adds 'struct lflow_ctx' to simplify
the code a bit. This also reduces some code in ovn-controller.c and
improves readability to some degree.
No functional changes are introduced in this patch.
Signed-off-by: Numan Siddique
---
controller/lflow.c | 255 ++--
controller/lflow.h | 83
controller/ovn-controller.c | 240 -
3 files changed, 186 insertions(+), 392 deletions(-)
diff --git a/controller/lflow.c b/controller/lflow.c
index a6893201e..311f8e2be 100644
--- a/controller/lflow.c
+++ b/controller/lflow.c
@@ -61,25 +61,12 @@ struct condition_aux {
const struct sset *active_tunnels;
};
-static bool consider_logical_flow(
-struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath,
-struct ovsdb_idl_index *sbrec_port_binding_by_name,
-const struct sbrec_logical_flow *,
-const struct hmap *local_datapaths,
-const struct sbrec_chassis *,
-struct hmap *dhcp_opts,
-struct hmap *dhcpv6_opts,
-struct hmap *nd_ra_opts,
-struct controller_event_options *controller_event_opts,
-const struct shash *addr_sets,
-const struct shash *port_groups,
-const struct sset *active_tunnels,
-const struct sset *local_lport_ids,
-struct ovn_desired_flow_table *,
-struct ovn_extend_table *group_table,
-struct ovn_extend_table *meter_table,
-struct lflow_resource_ref *lfrr,
-uint32_t *conj_id_ofs);
+static bool
+consider_logical_flow(struct lflow_ctx *l_ctx,
+ const struct sbrec_logical_flow *lflow,
+ struct hmap *dhcp_opts, struct hmap *dhcpv6_opts,
+ struct hmap *nd_ra_opts,
+ struct controller_event_options *controller_event_opts);
static bool
lookup_port_cb(const void *aux_, const char *port_name, unsigned int *portp)
@@ -257,30 +244,15 @@ lflow_resource_destroy_lflow(struct lflow_resource_ref
*lfrr,
/* Adds the logical flows from the Logical_Flow table to flow tables. */
static void
-add_logical_flows(
-struct ovsdb_idl_index *sbrec_multicast_group_by_name_datapath,
-struct ovsdb_idl_index *sbrec_port_binding_by_name,
-const struct sbrec_dhcp_options_table *dhcp_options_table,
-const struct sbrec_dhcpv6_options_table *dhcpv6_options_table,
-const struct sbrec_logical_flow_table *logical_flow_table,
-const struct hmap *local_datapaths,
-const struct sbrec_chassis *chassis,
-const struct shash *addr_sets,
-const struct shash *port_groups,
-const struct sset *active_tunnels,
-const struct sset *local_lport_ids,
-struct ovn_desired_flow_table *flow_table,
-struct ovn_extend_table *group_table,
-struct ovn_extend_table *meter_table,
-struct lflow_resource_ref *lfrr,
-uint32_t *conj_id_ofs)
+add_logical_flows(struct lflow_ctx *l_ctx)
{
const struct sbrec_logical_flow *lflow;
struct hmap dhcp_opts = HMAP_INITIALIZER(_opts);
struct hmap dhcpv6_opts = HMAP_INITIALIZER(_opts);
const struct sbrec_dhcp_options *dhcp_opt_row;
-SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row, dhcp_options_table) {
+SBREC_DHCP_OPTIONS_TABLE_FOR_EACH (dhcp_opt_row,
+ l_ctx->dhcp_options_table) {
dhcp_opt_add(_opts, dhcp_opt_row->name, dhcp_opt_row->code,
dhcp_opt_row->type);
}
@@ -288,7 +260,7 @@ add_logical_flows(
const struct sbrec_dhcpv6_options *dhcpv6_opt_row;
SBREC_DHCPV6_OPTIONS_TABLE_FOR_EACH (dhcpv6_opt_row,
- dhcpv6_options_table) {
+ l_ctx->dhcpv6_options_table) {
dhcp_opt_add(_opts, dhcpv6_opt_row->name, dhcpv6_opt_row->code,
dhcpv6_opt_row->type);
}
@@ -299,16 +271,9 @@ add_logical_flows(
struct controller_event_options controller_event_opts;
controller_event_opts_init(_event_opts);
-SBREC_LOGICAL_FLOW_TABLE_FOR_EACH (lflow, logical_flow_table) {
-if (!consider_logical_flow(sbrec_multicast_group_by_name_datapath,
- sbrec_port_binding_by_name,
- lflow, local_datapaths,
- chassis, _opts, _opts,
- _ra_opts, _event_opts,
- addr_sets, port_groups,
- active_tunnels, local_lport_ids,
- flow_table, group_table, meter_table,
- lfrr, conj_id_ofs)) {
+SBREC_LOGICAL_FLOW_TABLE_FOR_EACH (lflow, l_ctx->logical_flow_table) {
+if (!consider_logical_flow(l_ctx, lflow, _opts, _opts,
+ _ra_opts, _event_opts)) {
static struct
