Hi Thanks for your review. Em ter., 11 de ago. de 2026 às 15:05, Jacob Tanenbaum <[email protected]> escreveu:
> > > On Fri, Jul 24, 2026 at 12:41 PM Lucas Vargas Dias > <[email protected]> wrote: > >> Until now any change to a logical switch port of type "router" fell back >> to a full northd recompute, because lsp_can_be_inc_processed() only >> allowed plain VIF and remote ports. Enable incremental processing for >> creation and deletion of router ports as well. >> >> Unlike VIF/remote ports, a router port is not self-contained: a full >> recompute wires a peer relationship to the logical router port (LRP) in >> join_logical_ports() and populates aggregate datapath state >> (od->router_ports, peer->od->ls_peers, the "router" address, ...). The >> incremental path now wires (and tears down) this peer relationship >> itself, mirroring the router branch of join_logical_ports(). The wiring >> runs before the SB port binding is synced, as ovn_port_update_sbrec() >> consults op->peer. >> >> Some flows that toggle with the presence of a router port are owned by >> lflow_refs other than the port's own and are handled explicitly: >> >> - Sibling switch ports' ARP-resolve flows depend on od->router_ports, >> so the existing ports are re-tracked to the lflow engine. >> - The peer LRP's SB Port_Binding options:peer is (re)synced. >> - The per-switch ls_stateful lflow_ref (skip-conntrack flows generated >> for each router port) is regenerated in the lflow port-change handler. >> - The router port's stateful_lflow_ref is cleared on deletion (only >> router ports populate it via build_lbnat_lflows_iterate_by_lsp()). >> >> Dependencies that live outside the router port's lflow_ref and that this >> path does not keep in sync trigger a fall back to a full recompute >> (router_lsp_needs_recompute()): distributed gateway ports, gateway >> routers, NAT, load balancers, static or dynamic routing, IPv6 RA, >> arp_proxy, ACLs, vtep ports, mcast relay, or more than one router port on >> the switch. >> >> An update to a router port other than its "up" column also falls back to >> recompute, as re-wiring the peer relationship on reinit is not supported. >> >> Add tests covering incremental create/delete of a router port (including >> regeneration of a sibling VIF's ARP-resolve flow) and the recompute >> fallbacks for the distributed-gateway and dynamic-routing cases. >> >> Assisted-by: Claude Opus 4.8, ClaudeCode >> Signed-off-by: Lucas Vargas Dias <[email protected]> >> --- >> northd/en-ls-stateful.c | 7 + >> northd/northd.c | 340 +++++++++++++++++++++++++++++++++++++++- >> tests/ovn-northd.at | 139 +++++++++++++++- >> 3 files changed, 471 insertions(+), 15 deletions(-) >> >> diff --git a/northd/en-ls-stateful.c b/northd/en-ls-stateful.c >> index 1127b7d50..19c44a05e 100644 >> --- a/northd/en-ls-stateful.c >> +++ b/northd/en-ls-stateful.c >> @@ -336,6 +336,13 @@ ls_stateful_table_find_(const struct >> ls_stateful_table *table, >> return NULL; >> } >> >> +const struct ls_stateful_record * >> +ls_stateful_table_find(const struct ls_stateful_table *table, >> + const struct nbrec_logical_switch *nbs) >> +{ >> + return ls_stateful_table_find_(table, nbs); >> +} >> + >> static struct ls_stateful_record * >> ls_stateful_record_create(struct ls_stateful_table *table, >> const struct ovn_datapath *od, >> diff --git a/northd/northd.c b/northd/northd.c >> index f4bb5096e..404ab50a9 100644 >> --- a/northd/northd.c >> +++ b/northd/northd.c >> @@ -4331,13 +4331,35 @@ sync_pbs_for_northd_changed_ovn_ports( >> const struct lr_stateful_table *lr_stateful_table) >> { >> struct hmapx_node *hmapx_node; >> + struct ovn_port *op; >> >> HMAPX_FOR_EACH (hmapx_node, &trk_ovn_ports->created) { >> - sync_pb_for_lsp(hmapx_node->data, lr_stateful_table); >> + op = hmapx_node->data; >> + sync_pb_for_lsp(op, lr_stateful_table); >> + /* A newly created router port must set options:peer on its peer >> LRP's >> + * port binding. */ >> + if (lsp_is_router(op->nbsp) && op->peer && op->peer->nbrp) { >> + sync_pb_for_lrp(op->peer, lr_stateful_table); >> + } >> } >> >> HMAPX_FOR_EACH (hmapx_node, &trk_ovn_ports->updated) { >> - sync_pb_for_lsp(hmapx_node->data, lr_stateful_table); >> + op = hmapx_node->data; >> + sync_pb_for_lsp(op, lr_stateful_table); >> + if (lsp_is_router(op->nbsp) && op->peer && op->peer->nbrp) { >> + sync_pb_for_lrp(op->peer, lr_stateful_table); >> + } >> + } >> + >> + /* A deleted router port must clear options:peer on its (still >> existing) >> + * peer LRP's port binding. ls_router_port_unwire_peer() already >> reset >> + * op->peer->peer to NULL, so sync_pb_for_lrp() will omit the peer >> + * option. */ >> + HMAPX_FOR_EACH (hmapx_node, &trk_ovn_ports->deleted) { >> + op = hmapx_node->data; >> + if (lsp_is_router(op->nbsp) && op->peer && op->peer->nbrp) { >> + sync_pb_for_lrp(op->peer, lr_stateful_table); >> + } >> } >> } >> >> @@ -4689,10 +4711,10 @@ destroy_northd_tracked_data(struct northd_data >> *nd) >> static bool >> lsp_can_be_inc_processed(const struct nbrec_logical_switch_port *nbsp) >> { >> - /* Support only normal VIF, remote, localport, and virtual ports for >> - * now. */ >> + /* Support only normal VIF, remote, localport, virtual, and router >> + * ports for now. */ >> if (nbsp->type[0] && !lsp_is_remote(nbsp) && !lsp_is_localport(nbsp) >> && >> - !lsp_is_virtual(nbsp)) { >> + !lsp_is_virtual(nbsp) && !lsp_is_router(nbsp)) { >> return false; >> } >> >> @@ -4779,6 +4801,177 @@ virtual_lsp_needs_recompute(struct ovn_datapath >> *od, const char *lport) >> return false; >> } >> >> + >> +/* A logical switch port of type "router" is not self-contained: in a >> full >> + * recompute join_logical_ports() wires a peer relationship to the >> logical >> + * router port (LRP) and populates aggregate datapath state (see >> + * ls_router_port_wire_peer()). Several flows that toggle with the >> presence of >> + * such a port are owned by lflow_refs other than the port's own (the >> peer >> + * LRP's ref, the ls_stateful ref, the switch datapath ref, ...), which >> the >> + * incremental LSP path does not keep in sync. Return true when any such >> + * dependency is present so the caller falls back to a full recompute. >> + * >> + * 'is_delete' is true when 'nbsp' is being removed (the port is still >> counted >> + * in od->router_ports at this point). */ >> +static bool >> +router_lsp_needs_recompute(struct ovn_datapath *od, >> + const struct nbrec_logical_switch_port *nbsp, >> + const struct hmap *lr_ports, bool is_delete) >> +{ >> + /* arp_proxy adds proxy-arp admission flows owned by the peer LRP's >> + * lflow_ref and sets od->has_arp_proxy_port. */ >> + if (smap_get( ->options, "arp_proxy")) { >> + return true; >> + } >> + >> + /* Handle a single router port per switch for now. With more than >> one >> + * router port the router ports generate inter-router-port >> + * ARP-resolve/routable flows for each other (owned by sibling refs) >> that >> + * this path does not regenerate. */ >> + if (is_delete ? vector_len(&od->router_ports) > 1 >> + : !vector_is_empty(&od->router_ports)) { >> + return true; >> + } >> + >> + /* Switch-level stateful/aggregate dependencies that live outside the >> + * port's own lflow_ref: ls_stateful skip-conntrack flows over >> + * od->router_ports, LB install set (od->ls_peers), and vtep hairpin >> flows >> + * owned by od->datapath_lflows. */ >> + if (od->nbs->n_acls || od->nbs->n_load_balancer || >> + od->nbs->n_load_balancer_group || od->has_vtep_lports) { >> + return true; >> + } >> > > why do you fall back on n_acls here? doesn't that get regenerated in > lflow_handle_northd_port_changes(), specifically by the hmapx > ls_stateful_regen? > so the skip-conntrack flows should be rebuilt correctly even with ACLs > present. Is there another dependancy that I am not seeing? > You're right, fall back is not necessary here. I verified by removing the check and adding a test with stateful, stateless, logged and port-group ACLs on the switch: both create and delete stay incremental. Yes, it's regenerated. I'll adjust. > > >> + >> + const char *peer_name = smap_get( ->options, "router-port"); >> + if (!peer_name) { >> + /* No peer to wire; the port is inert. */ >> + return false; >> + } >> + >> + struct ovn_port *peer = ovn_port_find(lr_ports, peer_name); >> + if (!peer || !peer->nbrp) { >> + /* Peer LRP not present yet. This matches join_logical_ports(), >> which >> + * leaves op->peer NULL and does not add the port to >> od->router_ports; >> + * the port is inert and can be processed incrementally. */ >> + return false; >> + } >> + >> + /* Bad LRP-to-LRP peering or a disabled LRP; let recompute deal with >> it. */ >> + if (peer->nbrp->peer || !lrport_is_enabled(peer->nbrp)) { >> + return true; >> + } >> + >> + /* Distributed gateway / gateway-router complexity: l3gateway and >> + * chassisredirect SB port types, GARP nat_addresses, cr_port. */ >> + if (lrp_is_l3dgw(peer) || peer->cr_port || >> + !vector_is_empty(&peer->od->l3dgw_ports) || >> + peer->od->is_gw_router || >> + smap_get(&peer->od->nbr->options, "chassis")) { >> + return true; >> + } >> + >> + /* NAT, static routes, LBs and dynamic routing on the peer router >> pull in >> + * stateful/routable/advertised-route dependencies not tracked here. >> */ >> + const struct nbrec_logical_router *nbr = peer->od->nbr; >> + if (nbr->n_nat || nbr->n_static_routes || nbr->n_load_balancer || >> + nbr->n_load_balancer_group) { >> + return true; >> + } >> + if (peer->od->dynamic_routing || >> + peer->od->dynamic_routing_redistribute != DRRM_NONE) { >> + return true; >> + } >> + >> + /* IPv6 RA flows are owned by the peer LRP's lflow_ref and toggle >> with the >> + * peer's presence. */ >> + if (!smap_is_empty(&peer->nbrp->ipv6_ra_configs)) { >> + return true; >> + } >> + >> + /* mcast relay would flip od->mcast_info.sw.flood_relay, changing >> flows >> + * owned by od->datapath_lflows. */ >> + if (peer->od->mcast_info.rtr.relay) { >> + return true; >> + } >> + >> + return false; >> +} >> + >> +/* Wire the peer relationship of a logical switch port 'op' of type >> "router", >> + * mirroring the router branch of join_logical_ports(). 'op->od' must >> be set. >> + * Must run before the SB port binding is synced, as >> ovn_port_update_sbrec() >> + * consults op->peer. */ >> +static void >> +ls_router_port_wire_peer(struct ovn_port *op, const struct hmap >> *lr_ports) >> +{ >> + const char *peer_name = smap_get(&op->nbsp->options, "router-port"); >> + if (!peer_name) { >> + return; >> + } >> + >> + struct ovn_port *peer = ovn_port_find(lr_ports, peer_name); >> + if (!peer || !peer->nbrp || peer->nbrp->peer) { >> + return; >> + } >> + >> + vector_push(&op->od->router_ports, &op); >> + vector_push(&peer->od->ls_peers, &op->od); >> + peer->peer = op; >> + op->peer = peer; >> +} >> + >> +/* Fill op->lsp_addrs for the "router" address of a router-type LSP from >> its >> + * peer LRP networks (skipped by parse_lsp_addrs()). Must run after >> + * ls_port_init() and with op->peer set. */ >> +static void >> +ls_router_port_add_peer_networks(struct ovn_port *op) >> +{ >> + for (size_t j = 0; j < op->nbsp->n_addresses; j++) { >> + if (!strcmp(op->nbsp->addresses[j], "router")) { >> + if (extract_lrp_networks(op->peer->nbrp, >> + &op->lsp_addrs[op->n_lsp_addrs])) { >> + op->n_lsp_addrs++; >> + } >> + break; >> + } >> + } >> +} >> + >> +/* Tear down the peer relationship wired by ls_router_port_wire_peer() >> when a >> + * router-type LSP is deleted. Keeps op->peer set so the SB >> port-binding sync >> + * node can still reach the peer LRP to clear its options:peer (deleted >> tracked >> + * ports are freed only at the end of the engine run). */ >> +static void >> +ls_router_port_unwire_peer(struct ovn_port *op) >> +{ >> + struct ovn_port *peer = op->peer; >> + if (!peer) { >> + return; >> + } >> + >> + struct ovn_port *rp; >> + size_t i = 0; >> + VECTOR_FOR_EACH (&op->od->router_ports, rp) { >> + if (rp == op) { >> + vector_remove(&op->od->router_ports, i, NULL); >> + break; >> + } >> + i++; >> + } >> + >> + struct ovn_datapath *ls_od; >> + i = 0; >> + VECTOR_FOR_EACH (&peer->od->ls_peers, ls_od) { >> + if (ls_od == op->od) { >> + vector_remove(&peer->od->ls_peers, i, NULL); >> + break; >> + } >> + i++; >> + } >> + >> + peer->peer = NULL; >> +} >> + >> static bool >> ls_port_has_changed(const struct nbrec_logical_switch_port *new) >> { >> @@ -4846,7 +5039,7 @@ ls_port_init(struct ovn_port *op, struct >> ovsdb_idl_txn *ovnsb_txn, >> static struct ovn_port * >> ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, struct hmap *ls_ports, >> const char *key, const struct nbrec_logical_switch_port >> *nbsp, >> - struct ovn_datapath *od, >> + struct ovn_datapath *od, const struct hmap *lr_ports, >> const struct sbrec_mirror_table *sbrec_mirror_table, >> struct ovsdb_idl_index *sbrec_chassis_by_name, >> struct ovsdb_idl_index *sbrec_chassis_by_hostname, >> @@ -4855,6 +5048,16 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, >> struct hmap *ls_ports, >> struct ovn_port *op = ovn_port_create(ls_ports, key, nbsp, NULL, >> NULL); >> hmap_insert(&od->ports, &op->dp_node, hmap_node_hash(&op->key_node)); >> + >> + /* A router-type LSP must have its peer LRP wired before the SB port >> + * binding is synced by ls_port_init() (ovn_port_update_sbrec() >> consults >> + * op->peer). op->od is normally set inside ls_port_init(); set it >> early >> + * so the peer wiring can use op->od->router_ports. */ >> + if (lsp_is_router(nbsp)) { >> + op->od = od; >> + ls_router_port_wire_peer(op, lr_ports); >> + } >> + >> if (!ls_port_init(op, ovnsb_txn, od, NULL, sbrec_mirror_table, >> sbrec_chassis_by_name, sbrec_chassis_by_hostname, >> sbrec_encap_by_ip)) { >> > > if ls_port_init() fails after ls_router_port_wire_peer() pushes op into > the datapaths router_ports vector and pushes the router_ports datapath into > the peers ls_peers vector that leaves a stale entry in peer->od->ls_peers > and a reference to the freed op in od->router_ports. > > in practice this is not a big deal because this failure triggers a > database recalculation but it would be cleaner to call > ls_router_port_unwire_peer() before ovn_port_destroy() in the failure > branch. > > I agree > > @@ -4862,6 +5065,10 @@ ls_port_create(struct ovsdb_idl_txn *ovnsb_txn, >> struct hmap *ls_ports, >> return NULL; >> } >> >> + if (lsp_is_router(nbsp) && op->peer) { >> + ls_router_port_add_peer_networks(op); >> + } >> + >> return op; >> } >> >> @@ -5056,6 +5263,7 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn >> *ovnsb_idl_txn, >> >> bool ls_had_only_router_ports = (!vector_is_empty(&od->router_ports) >> && (vector_len(&od->router_ports) == >> hmap_count(&od->ports))); >> + bool router_ports_changed = false; >> >> struct ovs_list existing_virtual_ports; >> struct ovn_port *op; >> @@ -5083,9 +5291,17 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn >> *ovnsb_idl_txn, >> * to recompute. */ >> goto fail; >> } >> + if (lsp_is_router(new_nbsp) && >> + router_lsp_needs_recompute(od, new_nbsp, >> &nd->lr_ports, >> + false)) { >> + /* This router port has a dependency on a connected >> router >> + * that can't be handled incrementally. Fall back to >> + * recompute. */ >> + goto fail; >> + } >> op = ls_port_create(ovnsb_idl_txn, &nd->ls_ports, >> new_nbsp->name, new_nbsp, od, >> - ni->sbrec_mirror_table, >> + &nd->lr_ports, >> ni->sbrec_mirror_table, >> ni->sbrec_chassis_by_name, >> ni->sbrec_chassis_by_hostname, >> ni->sbrec_encap_by_ip); >> @@ -5093,9 +5309,30 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn >> *ovnsb_idl_txn, >> goto fail; >> } >> add_op_to_northd_tracked_ports(&trk_lsps->created, op); >> + if (lsp_is_router(new_nbsp) && op->peer) { >> + /* A router port was added to od->router_ports; >> sibling >> + * ports' ARP-resolve flows must be regenerated. */ >> + router_ports_changed = true; >> + } >> } else if (ls_port_has_changed(new_nbsp)) { >> /* Existing port updated */ >> bool temp = false; >> + if (lsp_is_router(new_nbsp)) { >> + /* The SB port binding type of a router port >> ("patch", >> + * "l3gateway", ...) never matches its NB type >> ("router"), >> + * so lsp_is_type_changed() can't be used here. >> Re-wiring >> + * the peer relationship on reinit is not supported, >> so >> + * fall back to recompute on any change other than >> the "up" >> + * column; an "up"-only change does not affect >> router-port >> + * flows, so ignore it. */ >> + if (!op->lsp_can_be_inc_processed || >> + !lsp_can_be_inc_processed(new_nbsp) || >> + check_lsp_changes_other_than_up(new_nbsp)) { >> + goto fail; >> + } >> + op->visited = true; >> + continue; >> + } >> if (lsp_is_type_changed(op->sb, new_nbsp, &temp) || >> !op->lsp_can_be_inc_processed || >> !lsp_can_be_inc_processed(new_nbsp)) { >> @@ -5174,6 +5411,14 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn >> *ovnsb_idl_txn, >> * recompute. */ >> goto fail; >> } >> + if (lsp_is_router(op->nbsp) && >> + router_lsp_needs_recompute(od, op->nbsp, &nd->lr_ports, >> + true)) { >> + /* This router port has a dependency on a connected >> router that >> + * can't be regenerated incrementally. Fall back to >> + * recompute. */ >> + goto fail; >> + } >> if (sset_contains(&nd->svc_monitor_lsps, op->key)) { >> /* This port was used for svc monitor, which may be >> * impacted by this deletion. Fallback to recompute. */ >> @@ -5187,6 +5432,14 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn >> *ovnsb_idl_txn, >> * resolved; fall back to recompute. */ >> goto fail; >> } >> + if (lsp_is_router(op->nbsp) && op->peer) { >> + /* Tear down the peer wiring and flag that sibling ports' >> + * ARP-resolve flows must be regenerated. op->peer is >> kept so >> + * the SB port-binding sync node can clear the peer LRP's >> + * options:peer. */ >> + ls_router_port_unwire_peer(op); >> + router_ports_changed = true; >> + } >> add_op_to_northd_tracked_ports(&trk_lsps->deleted, op); >> hmap_remove(&nd->ls_ports, &op->key_node); >> hmap_remove(&od->ports, &op->dp_node); >> @@ -5218,6 +5471,19 @@ ls_handle_lsp_changes(struct ovsdb_idl_txn >> *ovnsb_idl_txn, >> } >> } >> >> + /* Adding or removing a router port changes od->router_ports, on >> which the >> + * ARP-resolve flows of the sibling switch ports depend (see >> + * build_arp_resolve_flows_for_lsp()). Re-track the existing >> sibling ports >> + * so the lflow engine regenerates their flows. Newly created ports >> (inD >> + * trk_lsps->created) already get their flows generated. */ >> > > nit: could you mention that deleted ports are already removed from the > hmap dp_node above so we don't have to worry about them in this loop > > I agree Regards, Lucas > > >> + if (router_ports_changed) { >> + HMAP_FOR_EACH (op, dp_node, &od->ports) { >> + if (!hmapx_contains(&trk_lsps->created, op)) { >> + add_op_to_northd_tracked_ports(&trk_lsps->updated, op); >> + } >> + } >> + } >> + >> /* Update old virtual ports that have newly created or newly deleted >> * VIF as parent port. This code handles cases where the virtual >> port was >> * created before the parent port or when the parent port was >> recreated. >> @@ -20857,16 +21123,35 @@ lflow_handle_northd_port_changes(struct >> ovsdb_idl_txn *ovnsb_txn, >> struct hmapx_node *hmapx_node; >> struct ovn_port *op; >> >> + /* Logical switches whose set of router ports changed. The >> per-switch >> + * ls_stateful lflow_ref contains skip-conntrack flows generated for >> each >> + * router port (see build_ls_stateful_rec_pre_lb()/_pre_acls()), so >> it must >> + * be regenerated when a router port is created or deleted. */ >> + struct hmapx ls_stateful_regen = >> HMAPX_INITIALIZER(&ls_stateful_regen); >> + >> HMAPX_FOR_EACH (hmapx_node, &trk_lsps->deleted) { >> op = hmapx_node->data; >> /* Make sure 'op' is an lsp and not lrp. */ >> ovs_assert(op->nbsp); >> + if (lsp_is_router(op->nbsp) && op->peer) { >> + hmapx_add(&ls_stateful_regen, op->od); >> + } >> bool handled = lflow_ref_resync_flows( >> op->lflow_ref, lflows, ovnsb_txn, lflow_input->dps, >> lflow_input->ovn_internal_version_changed, >> lflow_input->sbrec_logical_flow_table, >> lflow_input->sbrec_logical_dp_group_table); >> + if (handled) { >> + /* Router ports also own flows on their stateful_lflow_ref >> (see >> + * build_lbnat_lflows_iterate_by_lsp()); clear those too. */ >> + handled = lflow_ref_resync_flows( >> + op->stateful_lflow_ref, lflows, ovnsb_txn, >> lflow_input->dps, >> + lflow_input->ovn_internal_version_changed, >> + lflow_input->sbrec_logical_flow_table, >> + lflow_input->sbrec_logical_dp_group_table); >> + } >> if (!handled) { >> + hmapx_destroy(&ls_stateful_regen); >> return false; >> } >> /* No need to update SB multicast groups, thanks to weak >> @@ -20914,6 +21199,7 @@ lflow_handle_northd_port_changes(struct >> ovsdb_idl_txn *ovnsb_txn, >> ds_destroy(&actions); >> >> if (!handled) { >> + hmapx_destroy(&ls_stateful_regen); >> return false; >> } >> } >> @@ -20922,6 +21208,9 @@ lflow_handle_northd_port_changes(struct >> ovsdb_idl_txn *ovnsb_txn, >> op = hmapx_node->data; >> /* Make sure 'op' is an lsp and not lrp. */ >> ovs_assert(op->nbsp); >> + if (lsp_is_router(op->nbsp) && op->peer) { >> + hmapx_add(&ls_stateful_regen, op->od); >> + } >> >> struct ds match = DS_EMPTY_INITIALIZER; >> struct ds actions = DS_EMPTY_INITIALIZER; >> @@ -20955,11 +21244,46 @@ lflow_handle_northd_port_changes(struct >> ovsdb_idl_txn *ovnsb_txn, >> ds_destroy(&actions); >> >> if (!handled) { >> + hmapx_destroy(&ls_stateful_regen); >> return false; >> } >> } >> >> - return true; >> + /* Regenerate the ls_stateful lflows of switches whose set of router >> ports >> + * changed (the skip-conntrack flows for router ports are owned by >> the >> + * per-switch ls_stateful lflow_ref, not by the port). */ >> + bool handled = true; >> + HMAPX_FOR_EACH (hmapx_node, &ls_stateful_regen) { >> + struct ovn_datapath *od = hmapx_node->data; >> + const struct ls_stateful_record *ls_stateful_rec = >> + ls_stateful_table_find(lflow_input->ls_stateful_table, >> od->nbs); >> + if (!ls_stateful_rec) { >> + continue; >> + } >> + >> + lflow_ref_unlink_lflows(ls_stateful_rec->lflow_ref); >> + build_ls_stateful_flows(ls_stateful_rec, od, >> + lflow_input->ls_port_groups, >> + lflow_input->meter_groups, >> + lflow_input->sampling_apps, >> + lflow_input->features, >> + lflows, >> + lflow_input->sbrec_acl_id_table); >> + build_network_function(od, lflows, lflow_input->ls_port_groups, >> + ls_stateful_rec->lflow_ref); >> + handled = lflow_ref_sync_lflows( >> + ls_stateful_rec->lflow_ref, lflows, ovnsb_txn, >> + lflow_input->dps, >> + lflow_input->ovn_internal_version_changed, >> + lflow_input->sbrec_logical_flow_table, >> + lflow_input->sbrec_logical_dp_group_table); >> + if (!handled) { >> + break; >> + } >> + } >> + >> + hmapx_destroy(&ls_stateful_regen); >> + return handled; >> } >> >> bool >> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at >> index 558ce10d8..13df2b712 100644 >> --- a/tests/ovn-northd.at >> +++ b/tests/ovn-northd.at >> @@ -12285,6 +12285,132 @@ CHECK_NO_CHANGE_AFTER_RECOMPUTE >> OVN_CLEANUP_NORTHD >> AT_CLEANUP >> >> +AT_SETUP([Router port incremental processing]) >> +AT_KEYWORDS([incremental processing]) >> +ovn_start >> + >> +check ovn-nbctl ls-add sw0 >> +check ovn-nbctl lr-add lr0 >> +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 >> +check ovn-nbctl --wait=sb lsp-add sw0 vif0 \ >> + -- lsp-set-addresses vif0 "00:00:00:00:00:01 10.0.0.4" >> + >> +# Connecting the switch to the pre-existing LRP should be incrementally >> +# processed (the Logical_Router row is untouched, so it is only a new >> LSP). >> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >> +check ovn-nbctl --wait=sb lsp-add-router-port sw0 sw0-lr0 lr0-sw0 >> +check_engine_compute northd incremental >> +check_engine_compute lflow incremental >> + >> +# The peer relationship is reflected on both SB port bindings. >> +AT_CHECK([ovn-sbctl get port_binding sw0-lr0 type], [0], [dnl >> +patch >> +]) >> +AT_CHECK([ovn-sbctl get port_binding sw0-lr0 options:peer], [0], [dnl >> +lr0-sw0 >> +]) >> +AT_CHECK([ovn-sbctl get port_binding lr0-sw0 options:peer], [0], [dnl >> +sw0-lr0 >> +]) >> + >> +# The router port L2 lookup flow is present on the switch. >> +AT_CHECK([ovn-sbctl dump-flows sw0 | grep ls_in_l2_lkup | grep sw0-lr0 \ >> + | grep -c 'eth.dst == 00:00:00:00:ff:01'], [0], [1 >> +]) >> + >> +# The sibling VIF's ARP-resolve flow was regenerated on the router >> pipeline >> +# (it depends on sw0's set of router ports). >> +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_in_arp_resolve \ >> + | grep 'reg0 == 10.0.0.4' | grep -c 'eth.dst = 00:00:00:00:00:01'], >> [0], [1 >> +]) >> + >> +CHECK_NO_CHANGE_AFTER_RECOMPUTE >> + >> +# Disconnecting the switch from the router should be incrementally >> processed. >> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >> +check ovn-nbctl --wait=sb lsp-del sw0-lr0 >> +check_engine_compute northd incremental >> +check_engine_compute lflow incremental >> + >> +# The peer LRP port binding no longer references the deleted switch port. >> +AT_CHECK([ovn-sbctl get port_binding lr0-sw0 options:peer], [1], [], [dnl >> +ovn-sbctl: no key "peer" in Port_Binding record "lr0-sw0" column options >> +]) >> + >> +# The router port L2 lookup flow and the VIF's ARP-resolve flow are gone. >> +AT_CHECK([ovn-sbctl dump-flows sw0 | grep ls_in_l2_lkup | grep -c >> sw0-lr0], >> + [1], [0 >> +]) >> +AT_CHECK([ovn-sbctl dump-flows lr0 | grep lr_in_arp_resolve \ >> + | grep -c 'reg0 == 10.0.0.4'], [1], [0 >> +]) >> + >> +CHECK_NO_CHANGE_AFTER_RECOMPUTE >> + >> +OVN_CLEANUP_NORTHD >> +AT_CLEANUP >> + >> +AT_SETUP([Router port incremental processing fallback with distributed >> gateway]) >> +AT_KEYWORDS([incremental processing]) >> +ovn_start >> + >> +check ovn-sbctl chassis-add gw1 geneve 127.0.0.1 >> + >> +check ovn-nbctl ls-add sw0 >> +check ovn-nbctl lr-add lr0 >> + >> +# Distributed gateway port on lr0. >> +check ovn-nbctl ls-add public >> +check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 >> 172.168.0.100/24 >> +check ovn-nbctl --wait=sb lsp-add-router-port public public-lr0 >> lr0-public >> +check ovn-nbctl lsp-add-localnet-port public ln-public public >> +check ovn-nbctl --wait=sb lrp-set-gateway-chassis lr0-public gw1 20 >> + >> +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 >> + >> +# Connecting a switch to a router that has a distributed gateway port >> must fall >> +# back to recompute (l3gateway/chassisredirect SB types and GARP >> nat_addresses >> +# depend on state outside the switch port's lflow_ref). >> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >> +check ovn-nbctl --wait=sb lsp-add-router-port sw0 sw0-lr0 lr0-sw0 >> +check_engine_compute northd recompute >> + >> +# Deleting it must also fall back to recompute. >> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >> +check ovn-nbctl --wait=sb lsp-del sw0-lr0 >> +check_engine_compute northd recompute >> + >> +CHECK_NO_CHANGE_AFTER_RECOMPUTE >> + >> +OVN_CLEANUP_NORTHD >> +AT_CLEANUP >> + >> +AT_SETUP([Router port incremental processing fallback with dynamic >> routing]) >> +AT_KEYWORDS([incremental processing]) >> +ovn_start >> + >> +check ovn-nbctl ls-add sw0 >> +check ovn-nbctl lr-add lr0 \ >> + -- set Logical_Router lr0 options:dynamic-routing=true >> +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.1/24 >> + >> +# Connecting a switch to a dynamic-routing router must fall back to >> recompute: >> +# advertised/routable flows depend on state outside the switch port's >> +# lflow_ref. >> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >> +check ovn-nbctl --wait=sb lsp-add-router-port sw0 sw0-lr0 lr0-sw0 >> +check_engine_compute northd recompute >> + >> +# Deleting it must also fall back to recompute. >> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >> +check ovn-nbctl --wait=sb lsp-del sw0-lr0 >> +check_engine_compute northd recompute >> + >> +CHECK_NO_CHANGE_AFTER_RECOMPUTE >> + >> +OVN_CLEANUP_NORTHD >> +AT_CLEANUP >> + >> OVN_FOR_EACH_NORTHD_NO_HV([ >> AT_SETUP([SB Port binding incremental processing]) >> ovn_start >> @@ -12327,17 +12453,16 @@ check ovn-nbctl --wait=sb sync >> check_recompute_counter 0 0 >> CHECK_NO_CHANGE_AFTER_RECOMPUTE >> >> -# Test lsp of type router >> +# Test lsp of type router. This port has no "router-port" option, so it >> has no >> +# peer LRP and is inert; both the NB logical switch port change and the >> +# subsequent "up" change set by ovn-northd are now incrementally >> processed. >> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >> check ovn-nbctl --wait=sb lsp-add ls0 rp -- lsp-set-type rp router >> - >> -# northd engine recomputes twice. Both the times for handling NB logical >> switch port >> -# changes and not because of SB port binding changes. This is because >> ovn-northd >> -# sets the "up" to true. >> -check_recompute_counter 2 2 >> +check_recompute_counter 0 0 >> CHECK_NO_CHANGE_AFTER_RECOMPUTE >> >> -# Set some options to 'rp'. northd should only recompute once. >> +# Set some options to 'rp'. Updating a router port other than its "up" >> column >> +# falls back to recompute (re-wiring the peer on reinit is not >> supported). >> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats >> check ovn-nbctl --wait=sb lsp-set-options rp foo=bar >> check_recompute_counter 1 1 >> -- >> 2.43.0 >> >> >> -- >> >> >> >> >> _'Esta mensagem é direcionada apenas para os endereços constantes no >> cabeçalho inicial. Se você não está listado nos endereços constantes no >> cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa >> mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas >> estão >> imediatamente anuladas e proibidas'._ >> >> >> * **'Apesar do Magazine Luiza tomar >> todas as precauções razoáveis para assegurar que nenhum vírus esteja >> presente nesse e-mail, a empresa não poderá aceitar a responsabilidade >> por >> quaisquer perdas ou danos causados por esse e-mail ou por seus anexos'.* >> >> >> >> _______________________________________________ >> dev mailing list >> [email protected] >> https://mail.openvswitch.org/mailman/listinfo/ovs-dev >> >> -- _‘Esta mensagem é direcionada apenas para os endereços constantes no cabeçalho inicial. Se você não está listado nos endereços constantes no cabeçalho, pedimos-lhe que desconsidere completamente o conteúdo dessa mensagem e cuja cópia, encaminhamento e/ou execução das ações citadas estão imediatamente anuladas e proibidas’._ * **‘Apesar do Magazine Luiza tomar todas as precauções razoáveis para assegurar que nenhum vírus esteja presente nesse e-mail, a empresa não poderá aceitar a responsabilidade por quaisquer perdas ou danos causados por esse e-mail ou por seus anexos’.* _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
