On 2/24/26 6:21 PM, Lucas Vargas Dias via dev wrote:
> Hi Dumitru,
>
Hi Lucas,
> I'm looking at the test dynamic-routing - Gateway Router
> You could comment the lines after
> # If we remove the route it is also gone.
> and before
> # By setting a learning interface filter will prevent us from learning the
> # route again. The Port referenced by the name does not even exist.
> and comment check ip route add 233.252.0.0/24 via 192.168.10.10 dev lo
> onlink vrf ovnvrf1337 proto zebra
> something like:
>
> # If we remove the route it is also gone.
> #check ip route del 233.252.0.0/24 via 192.168.10.10 dev lo onlink vrf
> ovnvrf1337 proto zebra
> #check ip route del 233.253.0.0/24 via 192.168.10.10 dev lo onlink vrf
> ovnvrf1337
> #check ovn-nbctl --wait=hv sync
> #check_row_count Learned_Route 0
>
> # By setting a learning interface filter will prevent us from learning the
> # route again. The Port referenced by the name does not even exist.
> check ovn-nbctl --wait=hv set Logical_Router_Port internet-phys \
> options:dynamic-routing-port-name=thisportdoesnotexist
> #check ip route add 233.252.0.0/24 via 192.168.10.10 dev lo onlink vrf
> ovnvrf1337 proto zebra
> check ovn-nbctl --wait=hv sync
> check_row_count Learned_Route 0 logical_port=$lp
>
At this point:
- router "internet" is a gateway router, bound to hv1 (so all its ports
are resident on hv1)
- port "internet-phys" has 'dynamic-routing="true",
dynamic-routing-port-name=thisportdoesnotexist'
- port "internet-public" has 'dynamic-routing="true"'
So then only "internet-phys" should be learning routes and only if
'thisportdoesnotexist' is bound locally. But 'thisportdoesnotexist'
doesn't exist as a port indeed. :)
>
> You'll see the learned route in internet-phys is not deleted.
> Maybe there's another issue in the sb_sync_learned_routes function.
>
So yeah, the route previously learned on internet-phys should have
been deleted. It does seem the sync has another issue, yes.
The learned_route has logical_port set to "internet-phys", which
has the "dynamic-routing-port-name" option set so we add it to the
uuid_set as _not stale_:
if (sb_route->logical_port->chassis == chassis ||
(cr_pb && cr_pb->chassis == chassis)) {
route_e = route_add_entry(&sync_routes, sb_route, false);
if (dynamic_routing_port_name) {
uuidset_insert(&uuid_set,
&sb_route->logical_port->header_.uuid);
}
}
But then just after:
/* If the port is not local we don't care about it.
* Some other ovn-controller will handle it.
* We may not use smap_get since the value might be validly NULL. */
if (!smap_get_node(bound_ports,
sb_route->logical_port->logical_port)) {
route_e = NULL;
continue;
}
"internet-phys" is not in 'bound_ports', is it?
So we set route_e to NULL and continue. We shouldn't set route_e to NULL
anyway because we continue, route_e should be defined inside the loop to
reduce its scope but that's a different story.
Anyhow, the route is in 'sync_routes' as _not stale_ at this point.
After finishing with all the existing SB learned_routes we try to mark
the ones in sync_routes that didn't have a logical_port that's part of
'uuid_set' as "stale":
if (!uuidset_is_empty(&uuid_set)) {
HMAP_FOR_EACH_SAFE (route_e, hmap_node, &sync_routes) {
if (!uuidset_find(&uuid_set,
&route_e->sb_route->logical_port->header_.uuid)) {
route_e->stale = true;
}
}
}
But we only had that one route, its logical port did have
"dynamic-routing-port-name" set, so we find it in the uuid_set.
That means we don't mark it as stale. So the route is still _not stale_.
Then we walk the learned routes (from the kernel):
struct re_nl_received_route_node *learned_route;
VECTOR_FOR_EACH_PTR (learned_routes, learned_route) {
char *ip_prefix = normalize_v46_prefix(&learned_route->prefix,
learned_route->plen);
char *nexthop = normalize_v46(&learned_route->nexthop);
struct smap_node *port_node;
SMAP_FOR_EACH (port_node, bound_ports) {
...
But "bound_ports" is empty so we don't do anything.
Finally we remove the "stale" routes from 'sync_routes':
HMAP_FOR_EACH_POP (route_e, hmap_node, &sync_routes) {
if (route_e->stale) {
sbrec_learned_route_delete(route_e->sb_route);
}
free(route_e);
}
But we had none (again, the remaining learned route is _not stale_).
I think the problem is here:
/* If the port is not local we don't care about it.
* Some other ovn-controller will handle it.
* We may not use smap_get since the value might be validly NULL. */
if (!smap_get_node(bound_ports,
sb_route->logical_port->logical_port)) {
route_e = NULL;
continue;
}
The logical_port is actually bound locally, in this case because the
router is a gateway router bound to the local chassis. But its
"dynamic-routing-port-name" is _not_ bound locally, i.e., logical_port
is not in 'bound_ports'.
Shouldn't we change the condition to something like below?
if (!smap_get_node(bound_ports,
sb_route->logical_port->logical_port)) {
if (<logical_port port_binding is bound locally>) {
route_e->stale = true;
}
continue;
}
i.e., I think:
if (!smap_get_node(bound_ports,
sb_route->logical_port->logical_port)) {
if (lport_pb_is_chassis_resident(chassis, sb_route->logical_port)) {
route_e->stale = true;
}
continue;
}
>
> Regards,
> Lucas
>
Regards,
Dumitru
>
>
>
> Em ter., 24 de fev. de 2026 às 13:11, Lucas Vargas Dias <
> [email protected]> escreveu:
>
>> Hi Dumitru
>> Thanks for the review.
>>
>> Em sex., 13 de fev. de 2026 às 12:03, Dumitru Ceara <[email protected]>
>> escreveu:
>>
>>> On 1/30/26 12:55 PM, Lucas Vargas Dias via dev wrote:
>>>> If logical router has more than one LRP as gateway router port
>>>> and dynamic routing configured, dynamic-routing-port-name could be
>>>> used to specify the LRP that will be used to dynamic routing.
>>>> However, if all LRPs learning routes, routes from LRP without
>>>> dynamic-routing-port-name must be flushed.
>>>> This happens when LRPs are scheduled in the same chassis.
>>>>
>>>> Signed-off-by: Lucas Vargas Dias <[email protected]>
>>>> ---
>>>
>>> Hi Lucas,
>>>
>>> Thanks for the new revision!
>>>
>>>> controller/ovn-controller.c | 16 ++++
>>>> controller/route-exchange.c | 47 +++++++++++-
>>>> controller/route-exchange.h | 1 +
>>>> tests/ovn-inc-proc-graph-dump.at | 2 +
>>>> tests/system-ovn.at | 125 +++++++++++++++++++++++++++++++
>>>
>>> On the v0 review I was asking if it's possible to add a multinode.at
>>> test, should we do that? It was quite hard for me to reason about all
>>> possible cases just with the system test you added. Checking with two
>>> hypervisors would create more confidence in the behavior of this code.
>>>
>>>
>> I see the problem related to order of configurations when DGPs are in the
>> same chassis.
>> But you're right, I'll add multinode tests to checking with two
>> hypervisors and to be more
>> easy understanding the code.
>>
>>
>>
>>>> 5 files changed, 187 insertions(+), 4 deletions(-)
>>>>
>>>> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
>>>> index 2d9b3e033..1c7b3b256 100644
>>>> --- a/controller/ovn-controller.c
>>>> +++ b/controller/ovn-controller.c
>>>> @@ -5494,10 +5494,24 @@ en_route_exchange_run(struct engine_node *node,
>>> void *data)
>>>> return EN_STALE;
>>>> }
>>>>
>>>> + const struct ovsrec_open_vswitch_table *ovs_table =
>>>> + EN_OVSDB_GET(engine_get_input("OVS_open_vswitch", node));
>>>> + const char *chassis_id = get_ovs_chassis_id(ovs_table);
>>>> + ovs_assert(chassis_id);
>>>> +
>>>> + struct ovsdb_idl_index *sbrec_chassis_by_name =
>>>> + engine_ovsdb_node_get_index(
>>>> + engine_get_input("SB_chassis", node),
>>>> + "name");
>>>> + const struct sbrec_chassis *chassis
>>>> + = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
>>>> + ovs_assert(chassis);
>>>> +
>>>> struct route_exchange_ctx_in r_ctx_in = {
>>>> .ovnsb_idl_txn = engine_get_context()->ovnsb_idl_txn,
>>>> .sbrec_learned_route_by_datapath =
>>> sbrec_learned_route_by_datapath,
>>>> .sbrec_port_binding_by_name = sbrec_port_binding_by_name,
>>>> + .chassis = chassis,
>>>> .announce_routes = &route_data->announce_routes,
>>>> };
>>>> struct route_exchange_ctx_out r_ctx_out = {
>>>> @@ -6679,6 +6693,8 @@ inc_proc_ovn_controller_init(
>>>> engine_add_input(&en_route, &en_sb_datapath_binding,
>>>> route_sb_datapath_binding_handler);
>>>>
>>>> + engine_add_input(&en_route_exchange, &en_ovs_open_vswitch, NULL);
>>>> + engine_add_input(&en_route_exchange, &en_sb_chassis, NULL);
>>>> engine_add_input(&en_route_exchange, &en_route, NULL);
>>>> engine_add_input(&en_route_exchange, &en_sb_learned_route,
>>>> engine_noop_handler);
>>>> diff --git a/controller/route-exchange.c b/controller/route-exchange.c
>>>> index ae44ffe69..f8a36f55d 100644
>>>> --- a/controller/route-exchange.c
>>>> +++ b/controller/route-exchange.c
>>>> @@ -26,6 +26,7 @@
>>>> #include "openvswitch/list.h"
>>>>
>>>> #include "lib/ovn-sb-idl.h"
>>>> +#include "lib/uuidset.h"
>>>>
>>>> #include "binding.h"
>>>> #include "ha-chassis.h"
>>>> @@ -86,7 +87,7 @@ maintained_route_table_add(uint32_t table_id)
>>>> hmap_insert(&_maintained_route_tables, &mrt->node, hash);
>>>> }
>>>>
>>>> -static void
>>>> +static struct route_entry *
>>>> route_add_entry(struct hmap *routes,
>>>> const struct sbrec_learned_route *sb_route,
>>>> bool stale)
>>>> @@ -102,6 +103,7 @@ route_add_entry(struct hmap *routes,
>>>> hash = hash_string(sb_route->ip_prefix, hash);
>>>>
>>>> hmap_insert(routes, &route_e->hmap_node, hash);
>>>> + return route_e;
>>>> }
>>>>
>>>> static struct route_entry *
>>>> @@ -144,28 +146,64 @@ sb_sync_learned_routes(const struct vector
>>> *learned_routes,
>>>> struct ovsdb_idl_txn *ovnsb_idl_txn,
>>>> struct ovsdb_idl_index
>>> *sbrec_port_binding_by_name,
>>>> struct ovsdb_idl_index
>>> *sbrec_learned_route_by_datapath,
>>>> - bool *sb_changes_pending)
>>>> + bool *sb_changes_pending,
>>>> + const struct sbrec_chassis *chassis)
>>>> {
>>>> struct hmap sync_routes = HMAP_INITIALIZER(&sync_routes);
>>>> const struct sbrec_learned_route *sb_route;
>>>> - struct route_entry *route_e;
>>>> + struct route_entry *route_e = NULL;
>>>> + struct uuidset uuid_set = UUIDSET_INITIALIZER(&uuid_set);
>>>
>>> On the v0 review I was asking if it's possible to use a more
>>> descriptive name instead of 'uuid_set'. I guess that slipped
>>> through.
>>>
>>
>> Sorry, I forget this.
>>
>>
>>>
>>>>
>>>> struct sbrec_learned_route *filter =
>>>>
>>> sbrec_learned_route_index_init_row(sbrec_learned_route_by_datapath);
>>>> sbrec_learned_route_index_set_datapath(filter, datapath);
>>>> SBREC_LEARNED_ROUTE_FOR_EACH_EQUAL (sb_route, filter,
>>>>
>>> sbrec_learned_route_by_datapath) {
>>>> + const struct sbrec_port_binding *cr_pb =
>>>> + lport_get_cr_port(sbrec_port_binding_by_name,
>>>> + sb_route->logical_port, NULL);
>>>> + const char *dynamic_routing_port_name =
>>>> + smap_get(&sb_route->logical_port->options,
>>>> + "dynamic-routing-port-name");
>>>> + if (!dynamic_routing_port_name && cr_pb) {
>>>> + dynamic_routing_port_name =
>>>> + smap_get(&cr_pb->options, "dynamic-routing-port-name");
>>>> + }
>>>> +
>>>> + if (sb_route->logical_port->chassis == chassis ||
>>>> + (cr_pb && cr_pb->chassis == chassis)) {
>>>> + route_e = route_add_entry(&sync_routes, sb_route, false);
>>>> + if (dynamic_routing_port_name) {
>>>> + uuidset_insert(&uuid_set,
>>>> + &sb_route->logical_port->header_.uuid);
>>>> + }
>>>> + }
>>>> +
>>>> /* If the port is not local we don't care about it.
>>>> * Some other ovn-controller will handle it.
>>>> * We may not use smap_get since the value might be validly
>>> NULL. */
>>>> if (!smap_get_node(bound_ports,
>>>> sb_route->logical_port->logical_port)) {
>>>> + route_e = NULL;
>>>> + continue;
>>>> + }
>>>> + if (route_e) {
>>>> + route_e->stale = true;
>>>> continue;
>>>> }
>>>> route_add_entry(&sync_routes, sb_route, true);
>>>> }
>>>> sbrec_learned_route_index_destroy_row(filter);
>>>>
>>>> + if (!uuidset_is_empty(&uuid_set)) {
>>>> + HMAP_FOR_EACH_SAFE (route_e, hmap_node, &sync_routes) {
>>>> + if (!uuidset_find(&uuid_set,
>>>> + &route_e->sb_route->logical_port->header_.uuid)) {
>>>> + route_e->stale = true;
>>>> + }
>>>> + }
>>>> + }
>>>> + uuidset_destroy(&uuid_set);
>>>
>>> I'm not sure the changes in sb_sync_learned_routes() are correct. Please
>>> see below in the system test part, I commented about why I think the test
>>> is wrong, which would mean that ovn-controller's behavior is also wrong.
>>>
>>> I tried to simplify the test for my case.
>>
>>
>>
>>>> struct re_nl_received_route_node *learned_route;
>>>> VECTOR_FOR_EACH_PTR (learned_routes, learned_route) {
>>>> char *ip_prefix = normalize_v46_prefix(&learned_route->prefix,
>>>> @@ -304,7 +342,8 @@ route_exchange_run(const struct
>>> route_exchange_ctx_in *r_ctx_in,
>>>> &ad->bound_ports,
>>> r_ctx_in->ovnsb_idl_txn,
>>>> r_ctx_in->sbrec_port_binding_by_name,
>>>>
>>> r_ctx_in->sbrec_learned_route_by_datapath,
>>>> - &r_ctx_out->sb_changes_pending);
>>>> + &r_ctx_out->sb_changes_pending,
>>>> + r_ctx_in->chassis);
>>>>
>>>> route_table_add_watch_request(&r_ctx_out->route_table_watches,
>>>> table_id);
>>>> diff --git a/controller/route-exchange.h b/controller/route-exchange.h
>>>> index e3791c331..53828a8b9 100644
>>>> --- a/controller/route-exchange.h
>>>> +++ b/controller/route-exchange.h
>>>> @@ -24,6 +24,7 @@ struct route_exchange_ctx_in {
>>>> struct ovsdb_idl_txn *ovnsb_idl_txn;
>>>> struct ovsdb_idl_index *sbrec_port_binding_by_name;
>>>> struct ovsdb_idl_index *sbrec_learned_route_by_datapath;
>>>> + const struct sbrec_chassis *chassis;
>>>>
>>>> /* Contains struct advertise_datapath_entry */
>>>> const struct hmap *announce_routes;
>>>> diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/
>>> ovn-inc-proc-graph-dump.at
>>>> index 3fe7b8fbd..0a514ffe5 100644
>>>> --- a/tests/ovn-inc-proc-graph-dump.at
>>>> +++ b/tests/ovn-inc-proc-graph-dump.at
>>>> @@ -460,6 +460,8 @@ digraph "Incremental-Processing-Engine" {
>>>> route_table_notify [[style=filled, shape=box, fillcolor=white,
>>> label="route_table_notify"]];
>>>> route_exchange_status [[style=filled, shape=box, fillcolor=white,
>>> label="route_exchange_status"]];
>>>> route_exchange [[style=filled, shape=box, fillcolor=white,
>>> label="route_exchange"]];
>>>> + OVS_open_vswitch -> route_exchange [[label=""]];
>>>> + SB_chassis -> route_exchange [[label=""]];
>>>> route -> route_exchange [[label=""]];
>>>> SB_learned_route -> route_exchange
>>> [[label="engine_noop_handler"]];
>>>> SB_port_binding -> route_exchange [[label="engine_noop_handler"]];
>>>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
>>>> index 67f03e3be..b0a023791 100644
>>>> --- a/tests/system-ovn.at
>>>> +++ b/tests/system-ovn.at
>>>> @@ -20593,3 +20593,128 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query
>>> port patch-.*/d
>>>> /connection dropped.*/d"])
>>>> AT_CLEANUP
>>>> ])
>>>> +
>>>> +OVN_FOR_EACH_NORTHD([
>>>> +AT_SETUP([dynamic-routing - BGP learned routes with router filter name
>>> and multiple DGPs])
>>>> +
>>>> +# This test validates that BGP learned routes work correctly:
>>>> +# 1. Routes added to the VRF appear in Learned_Route table
>>>> +# 2. Stopping ovn-controller remove learned routes
>>>> +#
>>>> +# Topology:
>>>> +# +---------+
>>>> +# | public |
>>>> +# +----+----+
>>>> +# |
>>>> +# +----+---+
>>>> +# | lr-frr | (in VRF 10)
>>>> +# +----+---+
>>>
>>> There's also a ls-dummy in the test, not depicted here. It might
>>> be useful to tag the DGPs and their chassis here.
>>>
>>
>> I agree
>>
>>
>>>
>>>> +# |
>>>> +# +------+-------+
>>>> +# |local-bgp-port| (in VRF 10)
>>>> +# +--------------+
>>>
>>> This is a bit confusing... local-bgp-port is actually a LSP on
>>> "public" and it has the same IP as the lr-frr port towards "public".
>>>
>>>> +
>>>> +ovn_start
>>>> +OVS_TRAFFIC_VSWITCHD_START()
>>>> +ADD_BR([br-int])
>>>> +ADD_BR([br-ex])
>>>> +
>>>> +check ovs-ofctl add-flow br-ex action=normal
>>>> +
>>>> +# Set external-ids in br-int needed for ovn-controller
>>>
>>> All comments should be sentences, they should end with period. That's
>>> the case for more comments in this patch.
>>>
>>
>> I agree
>>
>>
>>>
>>>> +check ovs-vsctl \
>>>> + -- set Open_vSwitch . external-ids:system-id=hv1 \
>>>> + -- set Open_vSwitch .
>>> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
>>>> + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
>>>> + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
>>>> + -- set bridge br-int fail-mode=secure
>>> other-config:disable-in-band=true
>>>> +
>>>> +# Configure bridge mappings for localnet
>>>> +check ovs-vsctl set Open_vSwitch .
>>> external-ids:ovn-bridge-mappings=phys:br-ex
>>>> +
>>>> +id=10
>>>
>>> As mentioned in my review of v0, I'd call this "vrf" instead of "id.
>>>
>>>
>> Sorry, I forget
>>
>>
>>> Also, we need:
>>>
>>> VRF_RESERVE([$vrf])
>>>
>>>
>> I agree. I'll adjust
>>
>>
>>>> +
>>>> +# Start ovn-controller
>>>> +start_daemon ovn-controller
>>>> +
>>>> +check ip link add vrf-$id type vrf table $id
>>>> +on_exit "ip link del vrf-$id"
>>>> +check ip link set vrf-$id up
>>>> +
>>>> +# Create public logical switch with localnet port
>>>> +check ovn-nbctl ls-add public
>>>> +check ovn-nbctl lsp-add-localnet-port public ln_port phys
>>>> +
>>>> +# Create lr-frr with dynamic routing in VRF $id
>>>> +check ovn-nbctl lr-add lr-frr \
>>>> + -- set Logical_Router lr-frr \
>>>> + options:dynamic-routing=true \
>>>> + options:dynamic-routing-vrf-id=$id \
>>>> + options:dynamic-routing-redistribute=static
>>>> +
>>>> +check ovn-nbctl lrp-add lr-frr lrp-local-bgp-port 00:00:00:00:00:03
>>> 20.0.0.3/24 \
>>>> + -- set Logical_Router_Port lrp-local-bgp-port
>>> options:dynamic-routing-maintain-vrf=false \
>>>> + -- set Logical_Router_Port lrp-local-bgp-port
>>> options:routing-protocol-redirect=local-bgp-port
>>>> +
>>>> +check ovn-nbctl lrp-set-gateway-chassis lrp-local-bgp-port hv1
>>>> +check ovn-nbctl lsp-add-router-port public public-lr-frr
>>> lrp-local-bgp-port
>>>> +
>>>> +check ovn-nbctl lrp-add lr-frr lrp-dgp-dummy 00:00:00:00:00:04
>>> 20.0.1.3/24
>>>> +check ovn-nbctl lrp-set-gateway-chassis lrp-dgp-dummy hv1
>>>> +check ovn-nbctl ls-add ls-dummy
>>>> +check ovn-nbctl lsp-add-router-port ls-dummy lsp-dummy lrp-dgp-dummy
>>>> +
>>>> +
>>>
>>> Nit: one empty line is enough.
>>>
>>> Agree
>>
>>
>>>> +# Create local-bgp-port in VRF 10
>>>> +check ovs-vsctl add-port br-int local-bgp-port \
>>>> + -- set Interface local-bgp-port type=internal \
>>>> + -- set Interface local-bgp-port
>>> external_ids:iface-id=local-bgp-port
>>>> +
>>>> +check ovn-nbctl lsp-add public local-bgp-port \
>>>> + -- lsp-set-addresses local-bgp-port unknown
>>>> +
>>>> +# Configure local-bgp-port interface and add to VRF
>>>> +check ip link set local-bgp-port master vrf-$id
>>>> +check ip link set local-bgp-port address 00:00:00:00:00:03
>>>> +check ip addr add dev local-bgp-port 20.0.0.3/24
>>>> +check ip link set local-bgp-port up
>>>> +
>>>> +# Wait for everything to be ready
>>>> +wait_for_ports_up
>>>> +check ovn-nbctl --wait=hv sync
>>>> +
>>>> +# Check lrp-local-bgp-port has dynamic-routing option set.
>>>> +check_row_count Port_Binding 1 logical_port=cr-lrp-local-bgp-port
>>> 'options:dynamic-routing=true'
>>>> +
>>>
>>> I think we should check that both DGPs are bound and with
>>> dynamic routing enabled:
>>>
>>
>> I agree
>>
>>
>>>
>>> check_row_count Port_Binding 1 logical_port=cr-lrp-dgp-dummy
>>> 'options:dynamic-routing=true'
>>>
>>>> +# Add static routes
>>>> +check ovn-nbctl lr-route-add lr-frr 10.10.2.1 20.0.0.42
>>> lrp-local-bgp-port
>>>> +
>>>> +# Verify advertised routes exist
>>>> +AS_BOX([Advertised_Route])
>>>> +wait_row_count Advertised_Route 1 ip_prefix=10.10.2.1
>>>> +
>>>> +# Add a route to the VRF (simulating BGP learning a route)
>>>> +check ip route add 10.10.3.1 via 20.0.0.25 vrf vrf-$id proto zebra
>>>> +
>>>> +# Verify learned route appears in SB database
>>>> +check_row_count Learned_Route 2 ip_prefix=10.10.3.1
>>>> +
>>>> +check ovn-nbctl --wait=hv set Logical_Router_Port lrp-local-bgp-port
>>> options:dynamic-routing-port-name=bgpvrf1002
>>>> +
>>>> +check_row_count Learned_Route 1 ip_prefix=10.10.3.1
>>>
>>> This was hard to decipher for me... what happens here is that the
>>> cr-lrp-local-bgp-port now ends up with:
>>>
>>> _uuid : cce1f38d-51b3-4734-b451-f53910f3e137
>>> options : {always-redirect="true",
>>> distributed-port=lrp-local-bgp-port, dynamic-routing="true",
>>> dynamic-routing-port-name=bgpvrf1002}
>>> logical_port : cr-lrp-local-bgp-port
>>> chassis : 85f900e2-a456-42ff-a90f-682bf0bb6ba6
>>>
>>> And cr-lrp-dgp-dummy with:
>>>
>>> _uuid : 586ead78-a51b-4b6e-a25a-f45936a2f2c2
>>> options : {always-redirect="true",
>>> distributed-port=lrp-dgp-dummy, dynamic-routing="true"}
>>> logical_port : cr-lrp-dgp-dummy
>>> chassis : 85f900e2-a456-42ff-a90f-682bf0bb6ba6
>>>
>>> And because lr-frr has at least one logical router port with
>>> dynamic-routing-port-name set (cr-lrp-local-bgp-port) we ignore dynamic
>>> routing on all the other ports (that don't have dynamic-routing-port-name
>>> set).
>>>
>>> So cr-lrp-dgp-dummy should not be used for dynamic routing
>>> anymore.
>>>
>>
>> When I added this test, we could configure dynamic-routing just by
>> logical_router,
>> so it'll copy the dynamic-routing for other LRPs.
>>
>>
>>
>>>
>>> However, the actual remaining learned route is:
>>> # ovn-sbctl list learned_route
>>> _uuid : e850df2d-0644-4aad-a606-94c998399672
>>> datapath : 8ce61f1b-e838-42b9-a26a-014b310eebcc
>>> external_ids : {}
>>> ip_prefix : "10.10.3.1"
>>> logical_port : a0838191-c9aa-4d6d-a539-741e11d97bdd
>>> nexthop : "20.0.0.25"
>>>
>>> And that's on port:
>>> _uuid : a0838191-c9aa-4d6d-a539-741e11d97bdd
>>> options : {chassis-redirect-port=cr-lrp-local-bgp-port,
>>> peer=public-lr-frr}
>>> logical_port : lrp-local-bgp-port
>>> chassis : []
>>>
>>> AKA on DGP:
>>> _uuid : cce1f38d-51b3-4734-b451-f53910f3e137
>>> options : {always-redirect="true",
>>> distributed-port=lrp-local-bgp-port, dynamic-routing="true",
>>> dynamic-routing-port-name=bgpvrf1002}
>>> logical_port : cr-lrp-local-bgp-port
>>> chassis : 85f900e2-a456-42ff-a90f-682bf0bb6ba6
>>>
>>> So the route that had initially been learned on cr-lrp-dgp-dummy
>>> has now been removed. That's correct.
>>>
>>> BUT, the remaining route should also be deleted, if I'm not wrong.
>>>
>>>
>> You're right, both routes must be deleted. But, here I
>> configured dynamic-routing-port-name with wrong value in the test.
>>
>>
>>> The port it's learned on has dynamic-routing="true" and
>>> dynamic-routing-port-name set. But the dynamic-routing-port-name
>>> is set to bgpvrf1002. And the DGP is bound to the local chassis.
>>> But if there's no bgpvrf1002 interface on the local chassis the
>>> route shouldn't be learned (see the docs in ovn-nb(5)).
>>>
>>
>> So I think what would be correct to check for at this point is that
>>> all learned routes are removed from the SB database.
>>>
>>> Then, after that check, we should add another LSP, bound to bgpvrf1002,
>>> or set dynamic-routing-port-mapping like we do in other tests, e.g.:
>>>
>>> check ovs-vsctl set Open_vSwitch .
>>> external-ids:dynamic-routing-port-mapping="bgpvrf1002=lo"
>>>
>>
>> And add the Linux route on the "lo" device:
>>> check ip route add10.10.3.1 via 20.0.0.25 dev lo onlink vrf vrf-$id proto
>>> zebra
>>>
>>
>> Now, because the route is learnt on "dev lo" and because ovn-controller
>>> reads the mapping "lo <-> bgpvrf1002" it should create Learned_Routes
>>> for all DGPs that have dynamic-routing-port-name=bgpvrf1002.
>>>
>>> Or am I missing something?
>>>
>>>
>> Actually, I'm thinking to use local-bgp-port.
>> check ovn-nbctl --wait=hv set Logical_Router_Port lrp-local-bgp-port
>> options:dynamic-routing-port-name=local-bgp-port
>>
>>
>>
>>
>>>> +
>>>> +# Stop ovn-controller
>>>> +OVN_CLEANUP_CONTROLLER([hv1], [], [], [lr-frr])
>>>
>>> This is unstable in CI, it sometimes fails with:
>>>
>>> I see the same problem in other tests, they fix it with something like:
>> check ovn-nbctl --wait=hv ls-del ls-int1
>> check ovn-nbctl --wait=hv ls-del ls-int2
>> check ovn-nbctl --wait=hv ls-del ls
>> check ovn-nbctl --wait=hv lr-del lr
>>
>>
>> Regards,
>> Lucas
>>
>>
>>> Checking after recompute for
>>> ovn-nbctl --wait=hv sync
>>> ./ovn-macros.at:898: "$@"
>>> ./system-ovn.at:21001: wc -l < related-ports-diff
>>> Ignoring lr-frr i.e. tunnel_key 0x2
>>> ./system-ovn.at:21001: wc -l < flow-diff-$hv
>>> --- - 2026-02-13 12:26:26.530011416 +0000
>>> +++
>>> /workspace/ovn-tmp/tests/system-kmod-testsuite.dir/at-groups/272/stdout
>>> 2026-02-13 12:26:26.527566777 +0000
>>> @@ -1,2 +1,2 @@
>>> -0
>>> +2
>>>
>>> related-ports-diff:
>>> 272. system-ovn.at:21001: 272. dynamic-routing - BGP learned routes with
>>> router filter name and multiple DGPs -- parallelization=yes --
>>> ovn_monitor_all=yes (system-ovn.at:21001): FAILED (system-ovn.at:21001)
>>>
>>>> +check ovn-nbctl --wait=sb sync
>>>> +
>>>> +# Verify routes are removed in SB database.
>>>> +wait_row_count Learned_Route 0
>>>> +
>>>> +OVN_CLEANUP_NORTHD
>>>> +
>>>> +as
>>>> +OVS_TRAFFIC_VSWITCHD_STOP(["/.*error receiving.*/d
>>>> +/failed to query port patch-.*/d
>>>> +/.*terminating with signal 15.*/d"])
>>>> +AT_CLEANUP
>>>> +])
>>>
>>> Thanks,
>>> Dumitru
>>>
>>>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
