@@ -199,7 +199,7 @@ northd_sb_port_binding_handler(struct engine_node *node,
northd_get_input_data(node, &input_data);
if (!northd_handle_sb_port_binding_changes(
- input_data.sbrec_port_binding_table, &nd->ls_ports)) {
+ input_data.sbrec_port_binding_table, &nd->ls_ports, &nd->lr_ports)) {
return false;
}
@@ -276,7 +276,8 @@ en_sync_to_sb_pb_run(struct engine_node *node, void *data OVS_UNUSED)
const struct engine_context *eng_ctx = engine_get_context();
struct northd_data *northd_data = engine_get_input_data("northd", node);
- sync_pbs(eng_ctx->ovnsb_idl_txn, &northd_data->ls_ports);
+ sync_pbs(eng_ctx->ovnsb_idl_txn, &northd_data->ls_ports,
+ &northd_data->lr_ports);
engine_set_node_state(node, EN_UPDATED);
}
@@ -3222,27 +3222,33 @@ op_get_name(const struct ovn_port *op)
}
static void
-ovn_update_ipv6_prefix(struct hmap *lr_ports)
+update_nb_ipv6_prefix(const struct ovn_port *op)
{
- const struct ovn_port *op;
- HMAP_FOR_EACH (op, key_node, lr_ports) {
- ovs_assert(op->nbrp);
+ ovs_assert(op->nbrp);
- if (!smap_get_bool(&op->nbrp->options, "prefix", false)) {
- continue;
- }
+ if (!smap_get_bool(&op->nbrp->options, "prefix", false)) {
+ return;
+ }
- char prefix[IPV6_SCAN_LEN + 6];
- unsigned aid;
- const char *ipv6_pd_list = smap_get(&op->sb->options,
- "ipv6_ra_pd_list");
- if (!ipv6_pd_list ||
- !ovs_scan(ipv6_pd_list, "%u:%s", &aid, prefix)) {
- continue;
- }
+ char prefix[IPV6_SCAN_LEN + 6];
+ unsigned aid;
+ const char *ipv6_pd_list = smap_get(&op->sb->options,
+ "ipv6_ra_pd_list");
+ if (!ipv6_pd_list ||
+ !ovs_scan(ipv6_pd_list, "%u:%s", &aid, prefix)) {
+ return;
+ }
+
+ const char *prefix_ptr = prefix;
+ nbrec_logical_router_port_set_ipv6_prefix(op->nbrp, &prefix_ptr, 1);
+}
- const char *prefix_ptr = prefix;
- nbrec_logical_router_port_set_ipv6_prefix(op->nbrp, &prefix_ptr, 1);
+static void
+ovn_update_ipv6_prefix(struct hmap *lr_ports)
+{
+ const struct ovn_port *op;
+ HMAP_FOR_EACH (op, key_node, lr_ports) {
+ update_nb_ipv6_prefix(op);
}
}
@@ -3390,6 +3396,9 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
{
sbrec_port_binding_set_datapath(op->sb, op->od->sb);
if (op->nbrp) {
+ /* Note: SB port binding options for router ports are set in
+ * sync_pbs(). */
+
/* If the router is for l3 gateway, it resides on a chassis
* and its port type is "l3gateway". */
const char *chassis_name = smap_get(&op->od->nbr->options, "chassis");
@@ -3401,15 +3410,11 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
sbrec_port_binding_set_type(op->sb, "patch");
}
- struct smap new;
- smap_init(&new);
if (is_cr_port(op)) {
ovs_assert(sbrec_chassis_by_name);
ovs_assert(sbrec_chassis_by_hostname);
ovs_assert(sbrec_ha_chassis_grp_by_name);
ovs_assert(active_ha_chassis_grps);
- const char *redirect_type = smap_get(&op->nbrp->options,
- "redirect-type");
if (op->nbrp->ha_chassis_group) {
if (op->nbrp->n_gateway_chassis) {
@@ -3451,49 +3456,8 @@ ovn_port_update_sbrec(struct ovsdb_idl_txn *ovnsb_txn,
/* Delete the legacy gateway_chassis from the pb. */
sbrec_port_binding_set_gateway_chassis(op->sb, NULL, 0);
}
- smap_add(&new, "distributed-port", op->nbrp->name);
-
- bool always_redirect =
- !op->od->has_distributed_nat &&
- !l3dgw_port_has_associated_vtep_lports(op->l3dgw_port);
-
- if (redirect_type) {
- smap_add(&new, "redirect-type", redirect_type);
- /* XXX Why can't we enable always-redirect when redirect-type
- * is bridged? */
- if (!strcmp(redirect_type, "bridged")) {
- always_redirect = false;
- }
- }
-
- if (always_redirect) {
- smap_add(&new, "always-redirect", "true");
- }
- } else {
- if (op->peer) {
- smap_add(&new, "peer", op->peer->key);
- if (op->nbrp->ha_chassis_group ||
- op->nbrp->n_gateway_chassis) {
- char *redirect_name =
- ovn_chassis_redirect_name(op->nbrp->name);
- smap_add(&new, "chassis-redirect-port", redirect_name);
- free(redirect_name);
- }
- }
- if (chassis_name) {
- smap_add(&new, "l3gateway-chassis", chassis_name);
- }
}
- const char *ipv6_pd_list = smap_get(&op->sb->options,
- "ipv6_ra_pd_list");
- if (ipv6_pd_list) {
- smap_add(&new, "ipv6_ra_pd_list", ipv6_pd_list);
- }
-
- sbrec_port_binding_set_options(op->sb, &new);
- smap_destroy(&new);
-
sbrec_port_binding_set_parent_port(op->sb, NULL);
sbrec_port_binding_set_tag(op->sb, NULL, 0);
@@ -4667,10 +4631,23 @@ sync_lbs(struct ovsdb_idl_txn *ovnsb_txn,
}
}
-/* Sync the SB Port bindings which needs to be updated.
- * Presently it syncs the nat column of port bindings corresponding to
- * the logical switch ports. */
-void sync_pbs(struct ovsdb_idl_txn *ovnsb_idl_txn, struct hmap *ls_ports)
+static void ovn_update_ipv6_options(struct hmap *lr_ports);
+static void ovn_update_ipv6_prefix(struct hmap *lr_ports);
+
+/* Sync the SB Port bindings which needs to be updated after the northd
+ * engine is run.
+ * Presently it syncs
+ * - the nat column of port bindings corresponding to
+ * the logical switch ports.
+ * - 'always-redirect' flag of chassis resident gateway ports.
+ *
+ * Note: Ideally we should move ovn_port_update_sbrec() here.
+ * But until we add full incremental processing to northd engine,
+ * it is difficult.
+ * */
+void
+sync_pbs(struct ovsdb_idl_txn *ovnsb_idl_txn, struct hmap *ls_ports,
+ struct hmap *lr_ports)
{
ovs_assert(ovnsb_idl_txn);
@@ -4796,6 +4773,63 @@ void sync_pbs(struct ovsdb_idl_txn *ovnsb_idl_txn, struct hmap *ls_ports)
sbrec_port_binding_set_nat_addresses(op->sb, NULL, 0);
}
}
+
+ /* Only sets the port binding options column for the router ports.
+ * */
+ HMAP_FOR_EACH (op, key_node, lr_ports) {
+ ovs_assert(op->nbrp);
+ struct smap new;
+ smap_init(&new);
+
+ const char *chassis_name = smap_get(&op->od->nbr->options, "chassis");
+ if (is_cr_port(op)) {
+ smap_add(&new, "distributed-port", op->nbrp->name);
+
+ bool always_redirect =
+ !op->od->has_distributed_nat &&
+ !l3dgw_port_has_associated_vtep_lports(op->l3dgw_port);
+
+ const char *redirect_type = smap_get(&op->nbrp->options,
+ "redirect-type");
+ if (redirect_type) {
+ smap_add(&new, "redirect-type", redirect_type);
+ /* XXX Why can't we enable always-redirect when redirect-type
+ * is bridged? */
+ if (!strcmp(redirect_type, "bridged")) {
+ always_redirect = false;
+ }
+ }
+
+ if (always_redirect) {
+ smap_add(&new, "always-redirect", "true");
+ }
+ } else {
+ if (op->peer) {
+ smap_add(&new, "peer", op->peer->key);
+ if (op->nbrp->ha_chassis_group ||
+ op->nbrp->n_gateway_chassis) {
+ char *redirect_name =
+ ovn_chassis_redirect_name(op->nbrp->name);
+ smap_add(&new, "chassis-redirect-port", redirect_name);
+ free(redirect_name);
+ }
+ }
+ if (chassis_name) {
+ smap_add(&new, "l3gateway-chassis", chassis_name);
+ }
+ }
+
+ const char *ipv6_pd_list = smap_get(&op->sb->options,
+ "ipv6_ra_pd_list");
+ if (ipv6_pd_list) {
+ smap_add(&new, "ipv6_ra_pd_list", ipv6_pd_list);
+ }
+
+ sbrec_port_binding_set_options(op->sb, &new);
+ smap_destroy(&new);
+ }
+
+ ovn_update_ipv6_options(lr_ports);
}
static bool
@@ -5690,20 +5724,23 @@ fail:
bool
northd_handle_sb_port_binding_changes(
const struct sbrec_port_binding_table *sbrec_port_binding_table,
- struct hmap *ls_ports)
+ struct hmap *ls_ports, struct hmap *lr_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) {
/* Return false if the 'pb' belongs to a router port. We don't handle
* I-P for router ports yet. */
- if (is_pb_router_type(pb)) {
- return false;
- }
+ bool is_router_port = is_pb_router_type(pb);
- struct ovn_port *op = ovn_port_find(ls_ports, pb->logical_port);
- if (op && !op->lsp_can_be_inc_processed) {
- return false;
+ struct ovn_port *op;
+ if (is_router_port) {
+ op = ovn_port_find(lr_ports, pb->logical_port);
+ } else {
+ 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)) {
@@ -5712,7 +5749,8 @@ northd_handle_sb_port_binding_changes(
* 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);
+ "%s is not found.", pb->logical_port,
+ is_router_port ? "LRP" : "LSP");
return false;
}
op->sb = pb;
@@ -5723,7 +5761,8 @@ northd_handle_sb_port_binding_changes(
* 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);
+ "%s still exists.", pb->logical_port,
+ is_router_port ? "LRP" : "LSP");
return false;
}
} else {
@@ -5733,7 +5772,8 @@ northd_handle_sb_port_binding_changes(
* 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);
+ "%s is not found.", pb->logical_port,
+ is_router_port ? "LRP" : "LSP");
return false;
}
if (op->sb != pb) {
@@ -5741,6 +5781,10 @@ northd_handle_sb_port_binding_changes(
"IDL row, which is unusual.", pb->logical_port);
return false;
}
+
+ if (op && is_router_port) {
+ update_nb_ipv6_prefix(op);
+ }
}
}
return true;
@@ -19487,7 +19531,6 @@ ovnnb_db_run(struct northd_input *input_data,
&data->lr_ports);
stopwatch_stop(BUILD_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
stopwatch_start(CLEAR_LFLOWS_CTX_STOPWATCH_NAME, time_msec());
- ovn_update_ipv6_options(&data->lr_ports);
ovn_update_ipv6_prefix(&data->lr_ports);
sync_port_groups(ovnsb_txn, input_data->sbrec_port_group_table,
@@ -394,7 +394,8 @@ bool lflow_handle_northd_lb_changes(struct ovsdb_idl_txn *ovnsb_txn,
struct lflow_input *lflow_input,
struct lflow_data *lflow_data);
bool northd_handle_sb_port_binding_changes(
- const struct sbrec_port_binding_table *, struct hmap *ls_ports);
+ const struct sbrec_port_binding_table *, struct hmap *ls_ports,
+ struct hmap *lr_ports);
struct tracked_lb_data;
bool northd_handle_lb_data_changes_pre_od(struct tracked_lb_data *,
@@ -426,6 +427,7 @@ const char *northd_get_svc_monitor_mac(void);
void sync_lbs(struct ovsdb_idl_txn *, const struct sbrec_load_balancer_table *,
struct ovn_datapaths *ls_datapaths, struct hmap *lbs);
-void sync_pbs(struct ovsdb_idl_txn *, struct hmap *ls_ports);
+void sync_pbs(struct ovsdb_idl_txn *, struct hmap *ls_ports,
+ struct hmap *lr_ports);
#endif /* NORTHD_H */
@@ -10346,3 +10346,52 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE
AT_CLEANUP
])
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([SB port binding incremental processing])
+ovn_start
+
+check ovn-nbctl ls-add sw0
+check ovn-nbctl lsp-add sw0 sw0-port1
+check ovn-nbctl lsp-set-addresses sw0-port1 "50:54:00:00:00:03 10.0.0.3"
+
+# Create a logical router and attach both logical switches
+check ovn-nbctl lr-add lr0
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
+check ovn-nbctl --wait=sb lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24
+# northd engine node should have only one recompute count.
+node_recompute_ct=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute)
+check test "$node_recompute_ct" -eq "1"
+lflow_recompute_ct=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute)
+check test "$node_recompute_ct" -eq "1"
+
+# Add a gw port to lr0
+ovn-sbctl chassis-add gw1 geneve 127.0.0.1
+ovn-nbctl --wait=sb sync
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
+check ovn-nbctl --wait=sb lrp-add lr0 lr0-public 00:00:00:00:ff:01 10.0.0.1/24
+
+node_recompute_ct=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute)
+check test "$node_recompute_ct" -eq "1"
+lflow_recompute_ct=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute)
+check test "$node_recompute_ct" -eq "1"
+
+as northd ovn-appctl -t NORTHD_TYPE vlog/set dbg
+
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
+check ovn-nbctl lrp-set-gateway-chassis lr0-public gw1 10
+node_recompute_ct=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute)
+# recompute count would be 2 because we don't handle SB HA_Chassis_Group changes.
+check test "$node_recompute_ct" -eq "2"
+lflow_recompute_ct=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute)
+check test "$node_recompute_ct" -eq "2"
+
+check as northd ovn-appctl -t NORTHD_TYPE inc-engine/clear-stats
+check ovn-nbctl set logical_router_port lr0-public options:foo=bar
+node_recompute_ct=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats northd recompute)
+check test "$node_recompute_ct" -eq "1"
+lflow_recompute_ct=$(as northd ovn-appctl -t NORTHD_TYPE inc-engine/show-stats lflow recompute)
+check test "$node_recompute_ct" -eq "1"
+
+AT_CLEANUP
+])