Hi Xavier

Looks good to me.

Acked-by: Jacob Tanenbaum <[email protected]>

On Tue, May 19, 2026 at 8:56 AM Xavier Simonart via dev <
[email protected]> wrote:

> If dynamic routing was enabled on a logical router port with no
> explicit IPs configured, then when the BGP control plane was learning
> a route in the VRF monitored by that logical router port, ovn-northd
> was generating syntactically incorrect logical flows (with
> reg5 = (null) action) causing that route to never actually be
> installed.
>
> Reported-at: https://redhat.atlassian.net/browse/FDP-3583a
> Fixes: 966ca1c919ce ("northd: Handle learned routes.")
> Assisted-by: Claude Opus 4.6, Claude Code
> Signed-off-by: Xavier Simonart <[email protected]>
> ---
>  northd/northd.c     | 37 +++++++++++++++++++------------
>  tests/ovn-northd.at | 53 +++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 76 insertions(+), 14 deletions(-)
>
> diff --git a/northd/northd.c b/northd/northd.c
> index 4162143de..0fdf6db42 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -12748,10 +12748,15 @@ add_ecmp_symmetric_reply_flows(struct
> lflow_table *lflows,
>                    ds_cstr(route_match));
>      ds_clear(&actions);
>      ds_put_format(&actions, "ip.ttl--; flags.loopback = 1; "
> -                  "eth.src = %s; %s = %s; outport = %s; next;",
> -                  out_port->lrp_networks.ea_s,
> -                  is_ipv4_nexthop ? REG_SRC_IPV4 : REG_SRC_IPV6,
> -                  port_ip, out_port->json_key);
> +                  "eth.src = %s; ",
> +                  out_port->lrp_networks.ea_s);
> +    if (port_ip) {
> +        ds_put_format(&actions, "%s = %s; ",
> +                      is_ipv4_nexthop ? REG_SRC_IPV4 : REG_SRC_IPV6,
> +                      port_ip);
> +    }
> +    ds_put_format(&actions, "outport = %s; next;",
> +                  out_port->json_key);
>      ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10300,
> ds_cstr(&match),
>                    ds_cstr(&actions), lflow_ref,
> WITH_HINT(route->source_hint));
>
> @@ -12891,13 +12896,15 @@ build_ecmp_route_flow(struct lflow_table *lflows,
>          ds_put_format(&actions, "%s = ",
>                        is_ipv4_nexthop ? REG_NEXT_HOP_IPV4 :
> REG_NEXT_HOP_IPV6);
>          ipv6_format_mapped(route->nexthop, &actions);
> -        ds_put_format(&actions, "; "
> -                      "%s = %s; "
> -                      "eth.src = %s; "
> +        if (route->lrp_addr_s) {
> +            ds_put_format(&actions, "; %s = %s",
> +                          is_ipv4_nexthop ? REG_SRC_IPV4 : REG_SRC_IPV6,
> +                          route->lrp_addr_s);
> +        }
> +        ds_put_format(&actions,
> +                      "; eth.src = %s; "
>                        "outport = %s; "
>                        REGBIT_NEXTHOP_IS_IPV4" = %d; ",
> -                      is_ipv4_nexthop ? REG_SRC_IPV4 : REG_SRC_IPV6,
> -                      route->lrp_addr_s,
>                        route->out_port->lrp_networks.ea_s,
>                        route->out_port->json_key,
>                        is_ipv4_nexthop);
> @@ -12957,15 +12964,17 @@ add_route(struct lflow_table *lflows, const
> struct ovn_datapath *od,
>                                             REG_NEXT_HOP_IPV6,
>                            is_ipv4_prefix ? "4" : "6");
>          }
> -        ds_put_format(&common_actions, "; "
> -                      "%s = %s; "
> -                      "eth.src = %s; "
> +        if (lrp_addr_s) {
> +            ds_put_format(&common_actions, "; %s = %s",
> +                          is_ipv4_nexthop ? REG_SRC_IPV4 : REG_SRC_IPV6,
> +                          lrp_addr_s);
> +        }
> +        ds_put_format(&common_actions,
> +                      "; eth.src = %s; "
>                        "outport = %s; "
>                        "flags.loopback = 1; "
>                        REGBIT_NEXTHOP_IS_IPV4" = %d; "
>                        "next;",
> -                      is_ipv4_nexthop ? REG_SRC_IPV4 : REG_SRC_IPV6,
> -                      lrp_addr_s,
>                        op->lrp_networks.ea_s,
>                        op->json_key,
>                        is_ipv4_nexthop);
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 501f78b67..6f8c1d30e 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -16317,6 +16317,59 @@ OVN_CLEANUP_NORTHD
>  AT_CLEANUP
>  ])
>
> +OVN_FOR_EACH_NORTHD_NO_HV([
> +AT_SETUP([dynamic-routing - learned routes on unnumbered LRP])
> +AT_KEYWORDS([dynamic-routing])
> +ovn_start
> +
> +check ovn-nbctl lr-add lr0
> +check ovn-nbctl set Logical_Router lr0 option:dynamic-routing=true
> +check ovn-nbctl lrp-add lr0 lrp1 00:00:00:00:ff:01 10.0.0.1/24
> +check ovn-nbctl lrp-add lr0 lrp2 00:00:00:00:ff:02
> +check ovn-nbctl --wait=sb lrp-add lr0 lrp3 00:00:00:00:ff:03
> +datapath=$(fetch_column datapath_binding _uuid external_ids:name=lr0)
> +lrp2=$(fetch_column port_binding _uuid logical_port=lrp2)
> +lrp3=$(fetch_column port_binding _uuid logical_port=lrp3)
> +
> +check_uuid ovn-sbctl create Learned_Route \
> +    datapath=$datapath                    \
> +    logical_port=$lrp2                      \
> +    ip_prefix=42.42.42.42/32              \
> +    nexthop=192.168.2.42
> +check ovn-nbctl --wait=sb sync
> +
> +ovn-sbctl dump-flows lr0 > lr0flows
> +AT_CHECK([grep -F "42.42.42.42" lr0flows | grep -c '(null)'], [1], [0
> +])
> +
> +AT_CHECK([grep -F "42.42.42.42" lr0flows | ovn_strip_lflows], [0], [dnl
> +  table=??(lr_in_ip_routing   ), priority=258  , match=(reg7 == 0 &&
> ip4.dst == 42.42.42.42/32), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 =
> 192.168.2.42; eth.src = 00:00:00:00:ff:02; outport = "lrp2"; flags.loopback
> = 1; reg9[[9]] = 1; next;)
> +])
> +
> +check_uuid ovn-sbctl create Learned_Route \
> +    datapath=$datapath                    \
> +    logical_port=$lrp3                    \
> +    ip_prefix=42.42.42.42/32              \
> +    nexthop=192.168.3.42
> +check ovn-nbctl --wait=sb sync
> +
> +ovn-sbctl dump-flows lr0 > lr0flows
> +AT_CHECK([grep -c '(null)' lr0flows], [1], [0
> +])
> +
> +AT_CHECK([grep -w "lr_in_ip_routing" lr0flows | grep -F "42.42.42.42" |
> ovn_strip_lflows], [0], [dnl
> +  table=??(lr_in_ip_routing   ), priority=258  , match=(reg7 == 0 &&
> ip4.dst == 42.42.42.42/32), action=(ip.ttl--; flags.loopback = 1;
> reg8[[0..15]] = 1; reg8[[16..31]] = select(1, 2);)
> +])
> +
> +AT_CHECK([grep "lr_in_ip_routing_ecmp" lr0flows | grep "42" | sed -e
> 's/reg8\[[16..31\]] == [[12]]/reg8\[[16..31\]] == ??/g' | ovn_strip_lflows
> | sort], [0], [dnl
> +  table=??(lr_in_ip_routing_ecmp), priority=100  , match=(reg8[[0..15]]
> == 1 && reg8[[16..31]] == ??), action=(reg0 = 192.168.2.42; eth.src =
> 00:00:00:00:ff:02; outport = "lrp2"; reg9[[9]] = 1; next;)
> +  table=??(lr_in_ip_routing_ecmp), priority=100  , match=(reg8[[0..15]]
> == 1 && reg8[[16..31]] == ??), action=(reg0 = 192.168.3.42; eth.src =
> 00:00:00:00:ff:03; outport = "lrp3"; reg9[[9]] = 1; next;)
> +])
> +
> +OVN_CLEANUP_NORTHD
> +AT_CLEANUP
> +])
> +
>  OVN_FOR_EACH_NORTHD_NO_HV([
>  AT_SETUP([dynamic-routing - route learning cleanup - router recreation])
>  AT_KEYWORDS([dynamic-routing])
> --
> 2.47.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to