diff mbox series

[ovs-dev,ovn] ovn-controller: Don't monitor connection table columns

Message ID 20191224125312.175418-1-numans@ovn.org
State Accepted
Headers show
Series [ovs-dev,ovn] ovn-controller: Don't monitor connection table columns | expand

Commit Message

Numan Siddique Dec. 24, 2019, 12:53 p.m. UTC 
From: Numan Siddique <numans@ovn.org>

ovn-controller doesn't need to know any changes to the connection
table row. This patch omits alerts for the Connection table
columns.

In a large scale deployment like 1000 chassis, this can cause lot of
CPU cycle wastages as ovsdb-server has to send out updates to all
the ovn-controller connections.

Signed-off-by: Numan Siddique <numans@ovn.org>
---
 controller/ovn-controller.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

Dumitru Ceara Jan. 2, 2020, 3:51 p.m. UTC | #1 
On Tue, Dec 24, 2019 at 1:53 PM <numans@ovn.org> wrote:
>
> From: Numan Siddique <numans@ovn.org>
>
> ovn-controller doesn't need to know any changes to the connection
> table row. This patch omits alerts for the Connection table
> columns.
>
> In a large scale deployment like 1000 chassis, this can cause lot of
> CPU cycle wastages as ovsdb-server has to send out updates to all
> the ovn-controller connections.
>
> Signed-off-by: Numan Siddique <numans@ovn.org>

Looks good to me.

Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks,
Dumitru

> ---
>  controller/ovn-controller.c | 13 ++++++++++++-
>  1 file changed, 12 insertions(+), 1 deletion(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index ad6dff4a2..17744d416 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -1868,7 +1868,6 @@ main(int argc, char *argv[])
>      ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_sb_global_col_external_ids);
>      ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_logical_flow_col_external_ids);
>      ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_port_binding_col_external_ids);
> -    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_external_ids);
>      ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_ssl_col_external_ids);
>      ovsdb_idl_omit(ovnsb_idl_loop.idl,
>                     &sbrec_gateway_chassis_col_external_ids);
> @@ -1876,6 +1875,18 @@ main(int argc, char *argv[])
>      ovsdb_idl_omit(ovnsb_idl_loop.idl,
>                     &sbrec_ha_chassis_group_col_external_ids);
>
> +    /* We don't want to monitor Connection table at all. So omit all the
> +     * columns. */
> +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_external_ids);
> +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_inactivity_probe);
> +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_is_connected);
> +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_max_backoff);
> +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_other_config);
> +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_read_only);
> +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_role);
> +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_status);
> +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_target);
> +
>      update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL);
>
>      stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS);
> --
> 2.23.0
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Numan Siddique Jan. 2, 2020, 7:21 p.m. UTC | #2 
On Thu, Jan 2, 2020 at 9:22 PM Dumitru Ceara <dceara@redhat.com> wrote:
>
> On Tue, Dec 24, 2019 at 1:53 PM <numans@ovn.org> wrote:
> >
> > From: Numan Siddique <numans@ovn.org>
> >
> > ovn-controller doesn't need to know any changes to the connection
> > table row. This patch omits alerts for the Connection table
> > columns.
> >
> > In a large scale deployment like 1000 chassis, this can cause lot of
> > CPU cycle wastages as ovsdb-server has to send out updates to all
> > the ovn-controller connections.
> >
> > Signed-off-by: Numan Siddique <numans@ovn.org>
>
> Looks good to me.
>
> Acked-by: Dumitru Ceara <dceara@redhat.com>

Thanks for the review. I applied this patch to master.

Numan

>
> Thanks,
> Dumitru
>
> > ---
> >  controller/ovn-controller.c | 13 ++++++++++++-
> >  1 file changed, 12 insertions(+), 1 deletion(-)
> >
> > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> > index ad6dff4a2..17744d416 100644
> > --- a/controller/ovn-controller.c
> > +++ b/controller/ovn-controller.c
> > @@ -1868,7 +1868,6 @@ main(int argc, char *argv[])
> >      ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_sb_global_col_external_ids);
> >      ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_logical_flow_col_external_ids);
> >      ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_port_binding_col_external_ids);
> > -    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_external_ids);
> >      ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_ssl_col_external_ids);
> >      ovsdb_idl_omit(ovnsb_idl_loop.idl,
> >                     &sbrec_gateway_chassis_col_external_ids);
> > @@ -1876,6 +1875,18 @@ main(int argc, char *argv[])
> >      ovsdb_idl_omit(ovnsb_idl_loop.idl,
> >                     &sbrec_ha_chassis_group_col_external_ids);
> >
> > +    /* We don't want to monitor Connection table at all. So omit all the
> > +     * columns. */
> > +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_external_ids);
> > +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_inactivity_probe);
> > +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_is_connected);
> > +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_max_backoff);
> > +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_other_config);
> > +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_read_only);
> > +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_role);
> > +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_status);
> > +    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_target);
> > +
> >      update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL);
> >
> >      stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS);
> > --
> > 2.23.0
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
index ad6dff4a2..17744d416 100644
--- a/controller/ovn-controller.c
+++ b/controller/ovn-controller.c
@@ -1868,7 +1868,6 @@  main(int argc, char *argv[])
     ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_sb_global_col_external_ids);
     ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_logical_flow_col_external_ids);
     ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_port_binding_col_external_ids);
-    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_external_ids);
     ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_ssl_col_external_ids);
     ovsdb_idl_omit(ovnsb_idl_loop.idl,
                    &sbrec_gateway_chassis_col_external_ids);
@@ -1876,6 +1875,18 @@  main(int argc, char *argv[])
     ovsdb_idl_omit(ovnsb_idl_loop.idl,
                    &sbrec_ha_chassis_group_col_external_ids);
 
+    /* We don't want to monitor Connection table at all. So omit all the
+     * columns. */
+    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_external_ids);
+    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_inactivity_probe);
+    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_is_connected);
+    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_max_backoff);
+    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_other_config);
+    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_read_only);
+    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_role);
+    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_status);
+    ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_connection_col_target);
+
     update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL);
 
     stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS);