Most SB port_binding changes do not need to be handled by "northd" node, because they are either result of SB transactions from northd itself, or changes already handled by sync-from-sb node. This change reduces number of recompute of both "northd" and "lflow" nodes that would have been triggered by SB port_binding changes. Now for common VIF changes there is zero recompute of "northd" node.
Signed-off-by: Han Zhou <[email protected]> --- northd/en-northd.c | 18 ++++++++++++++ northd/en-northd.h | 1 + northd/inc-proc-northd.c | 12 ++++++---- northd/northd.c | 52 ++++++++++++++++++++++++++++++++++++++++ northd/northd.h | 2 ++ tests/ovn-northd.at | 16 ++++++------- 6 files changed, 88 insertions(+), 13 deletions(-) diff --git a/northd/en-northd.c b/northd/en-northd.c index 785a592516c0..044fa7019023 100644 --- a/northd/en-northd.c +++ b/northd/en-northd.c @@ -187,6 +187,24 @@ northd_nb_logical_switch_handler(struct engine_node *node, return true; } +bool +northd_sb_port_binding_handler(struct engine_node *node, + void *data) +{ + struct northd_data *nd = data; + + struct northd_input input_data; + + northd_get_input_data(node, &input_data); + + if (!northd_handle_sb_port_binding_changes( + input_data.sbrec_port_binding_table, &nd->ls_ports)) { + return false; + } + + return true; +} + void *en_northd_init(struct engine_node *node OVS_UNUSED, struct engine_arg *arg OVS_UNUSED) diff --git a/northd/en-northd.h b/northd/en-northd.h index a53a162bda48..20cc77f108a3 100644 --- a/northd/en-northd.h +++ b/northd/en-northd.h @@ -16,5 +16,6 @@ void en_northd_cleanup(void *data); void en_northd_clear_tracked_data(void *data); bool northd_nb_nb_global_handler(struct engine_node *, void *data OVS_UNUSED); bool northd_nb_logical_switch_handler(struct engine_node *, void *data); +bool northd_sb_port_binding_handler(struct engine_node *, void *data); #endif /* EN_NORTHD_H */ diff --git a/northd/inc-proc-northd.c b/northd/inc-proc-northd.c index 0642c9514b7c..3ec791c8941c 100644 --- a/northd/inc-proc-northd.c +++ b/northd/inc-proc-northd.c @@ -143,10 +143,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, { /* Define relationships between nodes where first argument is dependent * on the second argument */ - engine_add_input(&en_northd, &en_nb_nb_global, - northd_nb_nb_global_handler); - engine_add_input(&en_northd, &en_nb_logical_switch, - northd_nb_logical_switch_handler); engine_add_input(&en_northd, &en_nb_port_group, NULL); engine_add_input(&en_northd, &en_nb_load_balancer, NULL); engine_add_input(&en_northd, &en_nb_load_balancer_group, NULL); @@ -163,7 +159,6 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_northd, &en_sb_mirror, NULL); engine_add_input(&en_northd, &en_sb_meter, NULL); engine_add_input(&en_northd, &en_sb_datapath_binding, NULL); - engine_add_input(&en_northd, &en_sb_port_binding, NULL); engine_add_input(&en_northd, &en_sb_mac_binding, NULL); engine_add_input(&en_northd, &en_sb_dns, NULL); engine_add_input(&en_northd, &en_sb_ha_chassis_group, NULL); @@ -174,6 +169,13 @@ void inc_proc_northd_init(struct ovsdb_idl_loop *nb, engine_add_input(&en_northd, &en_sb_static_mac_binding, NULL); engine_add_input(&en_northd, &en_sb_chassis_template_var, NULL); + engine_add_input(&en_northd, &en_sb_port_binding, + northd_sb_port_binding_handler); + engine_add_input(&en_northd, &en_nb_nb_global, + northd_nb_nb_global_handler); + engine_add_input(&en_northd, &en_nb_logical_switch, + northd_nb_logical_switch_handler); + engine_add_input(&en_mac_binding_aging, &en_nb_nb_global, NULL); engine_add_input(&en_mac_binding_aging, &en_sb_mac_binding, NULL); engine_add_input(&en_mac_binding_aging, &en_northd, NULL); diff --git a/northd/northd.c b/northd/northd.c index 2dbcf219dcc9..ce0912570afd 100644 --- a/northd/northd.c +++ b/northd/northd.c @@ -5144,6 +5144,58 @@ fail: return false; } +bool +northd_handle_sb_port_binding_changes( + const struct sbrec_port_binding_table *sbrec_port_binding_table, + struct hmap *ls_ports) +{ + const struct sbrec_port_binding *pb; + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(1, 5); + SBREC_PORT_BINDING_TABLE_FOR_EACH_TRACKED (pb, sbrec_port_binding_table) { + struct ovn_port *op = ovn_port_find(ls_ports, pb->logical_port); + if (op && !op->lsp_can_be_inc_processed) { + return false; + } + if (sbrec_port_binding_is_new(pb)) { + /* Most likely the PB was created by northd and this is the + * notification of that trasaction. So we just update the sb + * pointer in northd data. Fallback to recompute otherwise. */ + if (!op) { + VLOG_WARN_RL(&rl, "A port-binding for %s is created but the " + "LSP is not found.", pb->logical_port); + return false; + } + op->sb = pb; + } else if (sbrec_port_binding_is_deleted(pb)) { + /* Most likely the PB was deleted by northd and this is the + * notification of that transaction, and we can ignore in this + * case. Fallback to recompute otherwise, to avoid dangling + * sb idl pointers and other unexpected behavior. */ + if (op) { + VLOG_WARN_RL(&rl, "A port-binding for %s is deleted but the " + "LSP still exists.", pb->logical_port); + return false; + } + } else { + /* The PB is updated, most likely because of binding/unbinding + * to/from a chassis, and we can ignore the change (updating NB + * "up" will be handled in the engine node "sync_from_sb"). + * Fallback to recompute for anything unexpected. */ + if (!op) { + VLOG_WARN_RL(&rl, "A port-binding for %s is updated but the " + "LSP is not found.", pb->logical_port); + return false; + } + if (op->sb != pb) { + VLOG_WARN_RL(&rl, "A port-binding for %s is updated with a new" + "IDL row, which is unusual.", pb->logical_port); + return false; + } + } + } + return true; +} + struct multicast_group { const char *name; uint16_t key; /* OVN_MIN_MULTICAST...OVN_MAX_MULTICAST. */ diff --git a/northd/northd.h b/northd/northd.h index c77b6b3685bc..1a36ddb385c5 100644 --- a/northd/northd.h +++ b/northd/northd.h @@ -339,6 +339,8 @@ void build_lflows(struct ovsdb_idl_txn *ovnsb_txn, bool lflow_handle_northd_ls_changes(struct ovsdb_idl_txn *ovnsb_txn, struct tracked_ls_changes *, struct lflow_input *, struct hmap *lflows); +bool northd_handle_sb_port_binding_changes( + const struct sbrec_port_binding_table *, struct hmap *ls_ports); void build_bfd_table(struct ovsdb_idl_txn *ovnsb_txn, const struct nbrec_bfd_table *, diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index deeb1d310925..efe6073b6a2c 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -8998,30 +8998,30 @@ OVS_WAIT_UNTIL([test `as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=hv lsp-add ls0 lsp0-1 -- lsp-set-addresses lsp0-1 "aa:aa:aa:00:00:01 192.168.0.11" -AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute], [0], [3 +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute], [0], [0 ]) -AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute], [0], [5 +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute], [0], [2 ]) check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=hv lsp-add ls0 lsp0-2 -- lsp-set-addresses lsp0-2 "aa:aa:aa:00:00:02 192.168.0.12" -AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute], [0], [3 +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute], [0], [0 ]) -AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute], [0], [5 +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute], [0], [2 ]) check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=hv lsp-del lsp0-1 -AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute], [0], [1 +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute], [0], [0 ]) -AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute], [0], [2 +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute], [0], [1 ]) check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats check ovn-nbctl --wait=hv lsp-set-addresses lsp0-2 "aa:aa:aa:00:00:88 192.168.0.88" -AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute], [0], [1 +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute], [0], [0 ]) -AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute], [0], [2 +AT_CHECK([as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute], [0], [1 ]) # No change, no recompute -- 2.30.2 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
