| Message ID | 1468727905-11771-1-git-send-email-kangh@us.ibm.com |
|---|---|
| State | Changes Requested |
| Headers | show |
"dev" <dev-bounces@openvswitch.org> wrote on 07/16/2016 10:58:25 PM: > From: Hui Kang <hkang.sunysb@gmail.com> > To: dev@openvswitch.org > Date: 07/16/2016 10:58 PM > Subject: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb > Sent by: "dev" <dev-bounces@openvswitch.org> > > Improve performance by scanning only changed port binding entries > when determining whether to mark the logical switch port up or > down > > Signed-off-by: Hui Kang <kangh@us.ibm.com> > --- Reapplying the acked-by from v2... Acked-by: Ryan Moats <rmoats@us.ibm.com>
On Sat, Jul 16, 2016 at 11:58:25PM -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 > > Signed-off-by: Hui Kang <kangh@us.ibm.com> Won't this skip an initial round of updates at ovn-northd startup time? (Certainly ovn-northd might get killed and restarted occasionally, especially if we're doing failover to a second host.)
"dev" <dev-bounces@openvswitch.org> wrote on 07/26/2016 02:20:27 PM: > From: Ben Pfaff <blp@ovn.org> > To: Hui Kang <hkang.sunysb@gmail.com> > Cc: dev@openvswitch.org > Date: 07/26/2016 02:20 PM > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb > Sent by: "dev" <dev-bounces@openvswitch.org> > > On Sat, Jul 16, 2016 at 11:58:25PM -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 > > > > Signed-off-by: Hui Kang <kangh@us.ibm.com> > > Won't this skip an initial round of updates at ovn-northd startup time? > (Certainly ovn-northd might get killed and restarted occasionally, > especially if we're doing failover to a second host.) Good call, you are right. Do you think an initialization of iterating all entries in ovnsb before entering the main loop will correct this patch on northd restarting or failover? Thanks. - Hui > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
"dev" <dev-bounces@openvswitch.org> wrote on 07/26/2016 02:20:27 PM: > From: Ben Pfaff <blp@ovn.org> > To: Hui Kang <hkang.sunysb@gmail.com> > Cc: dev@openvswitch.org > Date: 07/26/2016 02:20 PM > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb > Sent by: "dev" <dev-bounces@openvswitch.org> > > On Sat, Jul 16, 2016 at 11:58:25PM -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 > > > > Signed-off-by: Hui Kang <kangh@us.ibm.com> > > Won't this skip an initial round of updates at ovn-northd startup time? > (Certainly ovn-northd might get killed and restarted occasionally, > especially if we're doing failover to a second host.) Hi, Ben, After second thought, I think skipping the initial round is the purpose of this patch. ovsdb_idl_create(ovsdb) copies the the Port_binding table from southbound database whenever ovn-northd gets started. In this case, the northbound DB and southbound db are synced. In ovnsb_db_run, ovn-northd only gets notified when there is change to the Chassis column [1]. Therefore, ovnsb_db_run should only look the entry that are changed with its Chassis column. There is no need to initialize by iterating every entry in the Port_binding table. Please correct me if my understanding is incorrect. Thanks. - Hui [1] https://github.com/openvswitch/ovs/blob/master/ovn/northd/ovn-northd.c#L3034 > _______________________________________________ > dev mailing list > dev@openvswitch.org > http://openvswitch.org/mailman/listinfo/dev
On Tue, Jul 26, 2016 at 3:44 PM, Hui Kang <kangh@us.ibm.com> wrote: > > > "dev" <dev-bounces@openvswitch.org> wrote on 07/26/2016 02:20:27 PM: > > > From: Ben Pfaff <blp@ovn.org> > > To: Hui Kang <hkang.sunysb@gmail.com> > > Cc: dev@openvswitch.org > > Date: 07/26/2016 02:20 PM > > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the > ovnsb > > Sent by: "dev" <dev-bounces@openvswitch.org> > > > > On Sat, Jul 16, 2016 at 11:58:25PM -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 > > > > > > Signed-off-by: Hui Kang <kangh@us.ibm.com> > > > > Won't this skip an initial round of updates at ovn-northd startup time? > > (Certainly ovn-northd might get killed and restarted occasionally, > > especially if we're doing failover to a second host.) > > Hi, Ben, > After second thought, I think skipping the initial round is the purpose of > this patch. > > ovsdb_idl_create(ovsdb) copies the the Port_binding table from southbound > database whenever ovn-northd gets started. In this case, the northbound > DB and southbound db are synced. In ovnsb_db_run, ovn-northd only gets > notified when there is change to the Chassis column [1]. Therefore, > ovnsb_db_run should only look the entry that are changed with its Chassis > column. There is no need to initialize by iterating every entry in the > Port_binding table. Please correct me if my understanding is incorrect. > Thanks. > What if the Chassis column changes in some Port_Binding records while ovn-northd isn't running?
Russell Bryant <russell@ovn.org> wrote on 07/26/2016 03:50:44 PM: > From: Russell Bryant <russell@ovn.org> > To: Hui Kang/Watson/IBM@IBMUS > Cc: Ben Pfaff <blp@ovn.org>, Hui Kang <hkang.sunysb@gmail.com>, ovs > dev <dev@openvswitch.org> > Date: 07/26/2016 03:51 PM > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb > > On Tue, Jul 26, 2016 at 3:44 PM, Hui Kang <kangh@us.ibm.com> wrote: > > > "dev" <dev-bounces@openvswitch.org> wrote on 07/26/2016 02:20:27 PM: > > > From: Ben Pfaff <blp@ovn.org> > > To: Hui Kang <hkang.sunysb@gmail.com> > > Cc: dev@openvswitch.org > > Date: 07/26/2016 02:20 PM > > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the > ovnsb > > Sent by: "dev" <dev-bounces@openvswitch.org> > > > > On Sat, Jul 16, 2016 at 11:58:25PM -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 > > > > > > Signed-off-by: Hui Kang <kangh@us.ibm.com> > > > > Won't this skip an initial round of updates at ovn-northd startup time? > > (Certainly ovn-northd might get killed and restarted occasionally, > > especially if we're doing failover to a second host.) > > Hi, Ben, > After second thought, I think skipping the initial round is the purpose of > this patch. > > ovsdb_idl_create(ovsdb) copies the the Port_binding table from southbound > database whenever ovn-northd gets started. In this case, the northbound > DB and southbound db are synced. In ovnsb_db_run, ovn-northd only gets > notified when there is change to the Chassis column [1]. Therefore, > ovnsb_db_run should only look the entry that are changed with its Chassis > column. There is no need to initialize by iterating every entry in the > Port_binding table. Please correct me if my understanding is incorrect. > Thanks. > > What if the Chassis column changes in some Port_Binding records > while ovn-northd isn't running? Hi, Russel, Thanks for correcting me. In this case, initialization is necessary each time ovn-northd restarts. - Hui > > -- > Russell Bryant
On Tue, Jul 26, 2016 at 03:44:57PM -0400, Hui Kang wrote: > > > "dev" <dev-bounces@openvswitch.org> wrote on 07/26/2016 02:20:27 PM: > > > From: Ben Pfaff <blp@ovn.org> > > To: Hui Kang <hkang.sunysb@gmail.com> > > Cc: dev@openvswitch.org > > Date: 07/26/2016 02:20 PM > > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the > ovnsb > > Sent by: "dev" <dev-bounces@openvswitch.org> > > > > On Sat, Jul 16, 2016 at 11:58:25PM -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 > > > > > > Signed-off-by: Hui Kang <kangh@us.ibm.com> > > > > Won't this skip an initial round of updates at ovn-northd startup time? > > (Certainly ovn-northd might get killed and restarted occasionally, > > especially if we're doing failover to a second host.) > > Hi, Ben, > After second thought, I think skipping the initial round is the purpose of > this patch. > > ovsdb_idl_create(ovsdb) copies the the Port_binding table from southbound > database whenever ovn-northd gets started. In this case, the northbound > DB and southbound db are synced. I don't understand this statement. ovsdb_idl_create() doesn't sync anything between nb and sb dbs.
Ben Pfaff <blp@ovn.org> wrote on 07/27/2016 12:30:25 PM: > From: Ben Pfaff <blp@ovn.org> > To: Hui Kang/Watson/IBM@IBMUS > Cc: Hui Kang <hkang.sunysb@gmail.com>, dev@openvswitch.org > Date: 07/27/2016 12:30 PM > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the ovnsb > > On Tue, Jul 26, 2016 at 03:44:57PM -0400, Hui Kang wrote: > > > > > > "dev" <dev-bounces@openvswitch.org> wrote on 07/26/2016 02:20:27 PM: > > > > > From: Ben Pfaff <blp@ovn.org> > > > To: Hui Kang <hkang.sunysb@gmail.com> > > > Cc: dev@openvswitch.org > > > Date: 07/26/2016 02:20 PM > > > Subject: Re: [ovs-dev] [PATCH v3] Scanning only changed entries in the > > ovnsb > > > Sent by: "dev" <dev-bounces@openvswitch.org> > > > > > > On Sat, Jul 16, 2016 at 11:58:25PM -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 > > > > > > > > Signed-off-by: Hui Kang <kangh@us.ibm.com> > > > > > > Won't this skip an initial round of updates at ovn-northd startup time? > > > (Certainly ovn-northd might get killed and restarted occasionally, > > > especially if we're doing failover to a second host.) > > > > Hi, Ben, > > After second thought, I think skipping the initial round is the purpose of > > this patch. > > > > ovsdb_idl_create(ovsdb) copies the the Port_binding table from southbound > > database whenever ovn-northd gets started. In this case, the northbound > > DB and southbound db are synced. > > I don't understand this statement. ovsdb_idl_create() doesn't sync > anything between nb and sb dbs. Sorry for the confusion. I have updated the patch to v4 addressing your comment. - Hui >
diff --git a/ovn/northd/ovn-northd.c b/ovn/northd/ovn-northd.c index 7ce509d..bb6b853 100644 --- a/ovn/northd/ovn-northd.c +++ b/ovn/northd/ovn-northd.c @@ -2938,7 +2938,7 @@ ovnsb_db_run(struct northd_context *ctx) hmap_insert(&lports_hmap, &hash_node->node, hash_string(nb->name, 0)); } - SBREC_PORT_BINDING_FOR_EACH(sb, ctx->ovnsb_idl) { + SBREC_PORT_BINDING_FOR_EACH_TRACKED (sb, ctx->ovnsb_idl) { nb = NULL; HMAP_FOR_EACH_WITH_HASH(hash_node, node, hash_string(sb->logical_port, 0), @@ -3139,7 +3139,7 @@ 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_track_add_column(ovnsb_idl_loop.idl, &sbrec_port_binding_col_chassis); ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_address_set); add_column_noalert(ovnsb_idl_loop.idl, &sbrec_address_set_col_name); @@ -3165,6 +3165,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()) {
Improve performance by scanning only changed port binding entries when determining whether to mark the logical switch port up or down Signed-off-by: Hui Kang <kangh@us.ibm.com> --- ovn/northd/ovn-northd.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-)