On 8/6/26 11:25 AM, Jun Gu wrote:
> route_runtime_data_handler() only requests a recompute for a tracked
> datapath that already takes part in route exchange, or for a tracked
> lport that is route-exchange relevant. A datapath that has just become
> local is neither, yet it contributes its router peer ports to the
> local_datapath of every logical router it is attached to, and route_run()
> walks exactly those peer ports to decide which routers take part in route
> exchange.
>
Hi Jun Gu,
Thanks for the new revision!
> So binding the first local port of a logical switch that is attached to a
> dynamic-routing logical router leaves en_route unprocessed. en_route is
> the only input that makes en_route_exchange run, and en_route_exchange is
> the only place that calls route_table_notify_update(). The netlink route
> table watches therefore stay empty and routes learned from the VRF are
> never reported in the Learned_Route table, with nothing in the log.
>
> This is currently masked in most topologies: if the logical switch has a
> localnet port, ovn-controller creates the peer OVS patch port when the
> datapath becomes local, and the resulting ovs interface change is not
> handled by binding_handle_ovs_interface_changes(), which recomputes
> runtime_data and, transitively, en_route. Dropping the localnet port
> from the "dynamic-routing - BGP learned routes" system test is enough to
> reproduce the failure on current main.
>
I wasn't able to verify this claim. The tests pass just fine with the
current main branch code and the localnet port configuration removed.
Can you please post a new version that also includes a short test?
> Request a recompute whenever a datapath is added to or removed from the
> local datapaths.
>
> Fixes: ccb0b6b9109c ("controller: Introduce route node.")
> Assisted-by: Claude Opus 4.8, Claude Code
> Signed-off-by: Jun Gu <[email protected]>
> ---
> controller/ovn-controller.c | 9 +++++++++
> 1 file changed, 9 insertions(+)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index ea9952a19..26bf756dd 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -5400,6 +5400,9 @@ route_runtime_data_handler(struct engine_node *node,
> void *data)
> * 2. A route-exchange relevant port went form local to remote or the
> * other way round.
> * 3. A tracked_port went from local to remote or the other way round.
> + * 4. A datapath became local or stopped being local. Such a datapath
> + * contributes peer ports to the router datapaths it is connected to,
> + * so it can make a router start or stop taking part in route
> exchange.
> * */
> struct tracked_datapath *t_dp;
> HMAP_FOR_EACH (t_dp, node, &rt_data->tracked_dp_bindings) {
> @@ -5412,6 +5415,12 @@ route_runtime_data_handler(struct engine_node *node,
> void *data)
> return EN_UNHANDLED;
> }
>
> + if (t_dp->tracked_type != TRACKED_RESOURCE_UPDATED) {
> + /* XXX: Until we get I-P support for route exchange we need to
> + * request recompute. */
> + return EN_UNHANDLED;
> + }
> +
On a closer look, isn't what you're trying to fix already fixed by
https://github.com/ovn-org/ovn/commit/a3f72e4?
That is, this part on the current main branch:
https://github.com/ovn-org/ovn/blob/6138fe4014669977a3595b6e84f1e37e3c022719/controller/ovn-controller.c#L5474-L5480
CC: Ales
> struct shash_node *shash_node;
> SHASH_FOR_EACH (shash_node, &t_dp->lports) {
> struct tracked_lport *lport = shash_node->data;
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
