diff mbox

[ovs-dev,v4] Scanning only changed entries in the ovnsb

Message ID 1469586307-21158-1-git-send-email-kangh@us.ibm.com
State Not Applicable
Headers show

Commit Message

Hui Kang July 27, 2016, 2:25 a.m. UTC
- Improve performance by scanning only changed port binding entries
when determining whether to mark the logical switch port up or
down

v3->v4:
- Add an initialization function to scan all entries in Port_binding table
  when ovn-northd restarts or fails over

Signed-off-by: Hui Kang <kangh@us.ibm.com>
---
 ovn/northd/ovn-northd.c | 95 +++++++++++++++++++++++++++++++++----------------
 1 file changed, 65 insertions(+), 30 deletions(-)

Comments

Ben Pfaff July 27, 2016, 9:53 p.m. UTC | #1
On Tue, Jul 26, 2016 at 10:25:07PM -0400, Hui Kang wrote:
> - Improve performance by scanning only changed port binding entries
> when determining whether to mark the logical switch port up or
> down
> 
> v3->v4:
> - Add an initialization function to scan all entries in Port_binding table
>   when ovn-northd restarts or fails over
> 
> Signed-off-by: Hui Kang <kangh@us.ibm.com>

Hi, thanks for the revision.  This gets a few patch rejects, would you
mind rebasing?

Thanks,

Ben.
Hui Kang July 28, 2016, 12:11 a.m. UTC | #2
"dev" <dev-bounces@openvswitch.org> wrote on 07/27/2016 05:53:17 PM:

> From: Ben Pfaff <blp@ovn.org>
> To: Hui Kang <hkang.sunysb@gmail.com>
> Cc: dev@openvswitch.org
> Date: 07/27/2016 05:53 PM
> Subject: Re: [ovs-dev] [PATCH, v4] Scanning only changed entries in the
ovnsb
> Sent by: "dev" <dev-bounces@openvswitch.org>
>
> On Tue, Jul 26, 2016 at 10:25:07PM -0400, Hui Kang wrote:
> > - Improve performance by scanning only changed port binding entries
> > when determining whether to mark the logical switch port up or
> > down
> >
> > v3->v4:
> > - Add an initialization function to scan all entries in Port_binding
table
> >   when ovn-northd restarts or fails over
> >
> > Signed-off-by: Hui Kang <kangh@us.ibm.com>
>
> Hi, thanks for the revision.  This gets a few patch rejects, would you
> mind rebasing?

Not a problem; will submit an update soon.

- Hui

>
> Thanks,
>
> Ben.
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> http://openvswitch.org/mailman/listinfo/dev
diff mbox

Patch

diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c
index 17fbf29..35566a7 100644
--- a/ovn/northd/ovn-northd.c
+++ b/ovn/northd/ovn-northd.c
@@ -53,6 +53,11 @@  struct northd_context {
     struct ovsdb_idl_txn *ovnsb_txn;
 };
 
+struct lport_hash_node {
+    struct hmap_node node;
+    const struct nbrec_logical_switch_port *nbsp;
+};
+
 static const char *ovnnb_db;
 static const char *ovnsb_db;
 
@@ -3196,13 +3201,47 @@  ovnnb_db_run(struct northd_context *ctx)
     hmap_destroy(&ports);
 }
 
+static void
+sb_chassis_update_nbsec(const struct sbrec_port_binding *sb,
+                        struct hmap *lports_hmap)
+
+{
+    struct lport_hash_node *hash_node;
+    const struct nbrec_logical_switch_port *nbsp;
+
+    nbsp = NULL;
+    HMAP_FOR_EACH_WITH_HASH(hash_node, node,
+                            hash_string(sb->logical_port, 0),
+                            lports_hmap) {
+        if (!strcmp(sb->logical_port, hash_node->nbsp->name)) {
+            nbsp = hash_node->nbsp;
+            break;
+        }
+    }
+
+    if (!nbsp) {
+        /* The logical port doesn't exist for this port binding.  This can
+         * happen under normal circumstances when ovn-northd hasn't gotten
+         * around to pruning the Port_Binding yet. */
+        return;
+    }
+
+    if (sb->chassis && (!nbsp->up || !*nbsp->up)) {
+        bool up = true;
+        nbrec_logical_switch_port_set_up(nbsp, &up, 1);
+    } else if (!sb->chassis && (!nbsp->up || *nbsp->up)) {
+        bool up = false;
+        nbrec_logical_switch_port_set_up(nbsp, &up, 1);
+    }
+}
+
 /*
  * The only change we get notified about is if the 'chassis' column of the
  * 'Port_Binding' table changes.  When this column is not empty, it means we
  * need to set the corresponding logical port as 'up' in the northbound DB.
  */
 static void
-ovnsb_db_run(struct northd_context *ctx)
+ovnsb_db_run(struct northd_context *ctx, bool is_init)
 {
     if (!ctx->ovnnb_txn) {
         return;
@@ -3211,10 +3250,7 @@  ovnsb_db_run(struct northd_context *ctx)
     const struct sbrec_port_binding *sb;
     const struct nbrec_logical_switch_port *nbsp;
 
-    struct lport_hash_node {
-        struct hmap_node node;
-        const struct nbrec_logical_switch_port *nbsp;
-    } *hash_node;
+    struct lport_hash_node *hash_node;
 
     hmap_init(&lports_hmap);
 
@@ -3224,30 +3260,13 @@  ovnsb_db_run(struct northd_context *ctx)
         hmap_insert(&lports_hmap, &hash_node->node, hash_string(nbsp->name, 0));
     }
 
-    SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
-        nbsp = NULL;
-        HMAP_FOR_EACH_WITH_HASH(hash_node, node,
-                                hash_string(sb->logical_port, 0),
-                                &lports_hmap) {
-            if (!strcmp(sb->logical_port, hash_node->nbsp->name)) {
-                nbsp = hash_node->nbsp;
-                break;
-            }
-        }
-
-        if (!nbsp) {
-            /* The logical port doesn't exist for this port binding.  This can
-             * happen under normal circumstances when ovn-northd hasn't gotten
-             * around to pruning the Port_Binding yet. */
-            continue;
+    if (is_init) {
+        SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) {
+            sb_chassis_update_nbsec(sb, &lports_hmap);
         }
-
-        if (sb->chassis && (!nbsp->up || !*nbsp->up)) {
-            bool up = true;
-            nbrec_logical_switch_port_set_up(nbsp, &up, 1);
-        } else if (!sb->chassis && (!nbsp->up || *nbsp->up)) {
-            bool up = false;
-            nbrec_logical_switch_port_set_up(nbsp, &up, 1);
+    } else {
+        SBREC_PORT_BINDING_FOR_EACH_TRACKED(sb, ctx->ovnsb_idl) {
+            sb_chassis_update_nbsec(sb, &lports_hmap);
         }
     }
 
@@ -3417,6 +3436,19 @@  add_column_noalert(struct ovsdb_idl *idl,
     ovsdb_idl_omit_alert(idl, column);
 }
 
+static void
+ovnsb_db_run_init(struct ovsdb_idl_loop *ovnsb_idl_loop)
+{
+    struct northd_context ctx = {
+        .ovnnb_idl = NULL,
+        .ovnnb_txn = NULL,
+        .ovnsb_idl = ovnsb_idl_loop->idl,
+        .ovnsb_txn = ovsdb_idl_loop_run(ovnsb_idl_loop),
+    };
+
+    ovnsb_db_run(&ctx, true);
+}
+
 int
 main(int argc, char *argv[])
 {
@@ -3485,7 +3517,6 @@  main(int argc, char *argv[])
     add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_type);
     add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_options);
     add_column_noalert(ovnsb_idl_loop.idl, &sbrec_port_binding_col_mac);
-    ovsdb_idl_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis);
     ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_dhcp_options);
     add_column_noalert(ovnsb_idl_loop.idl, &sbrec_dhcp_options_col_code);
     add_column_noalert(ovnsb_idl_loop.idl, &sbrec_dhcp_options_col_type);
@@ -3495,6 +3526,9 @@  main(int argc, char *argv[])
     add_column_noalert(ovnsb_idl_loop.idl, &sbrec_address_set_col_name);
     add_column_noalert(ovnsb_idl_loop.idl, &sbrec_address_set_col_addresses);
 
+    ovnsb_db_run_init(&ovnsb_idl_loop);
+    ovsdb_idl_track_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis);
+
     /* Main loop. */
     exiting = false;
     while (!exiting) {
@@ -3506,7 +3540,7 @@  main(int argc, char *argv[])
         };
 
         ovnnb_db_run(&ctx);
-        ovnsb_db_run(&ctx);
+        ovnsb_db_run(&ctx, false);
         if (ctx.ovnsb_txn) {
             check_and_add_supported_dhcp_opts_to_sb_db(&ctx);
         }
@@ -3518,6 +3552,7 @@  main(int argc, char *argv[])
         }
         ovsdb_idl_loop_commit_and_wait(&ovnnb_idl_loop);
         ovsdb_idl_loop_commit_and_wait(&ovnsb_idl_loop);
+        ovsdb_idl_track_clear(ctx.ovnsb_idl);
 
         poll_block();
         if (should_service_stop()) {