Hi Ales

Thanks for your review.


Em sex., 10 de abr. de 2026 às 06:06, Ales Musil <[email protected]>
escreveu:

>
>
> On Tue, Feb 3, 2026 at 9:24 PM Lucas Vargas Dias via dev <
> [email protected]> wrote:
>
>> Consider the scenario where there are 2 AZs using ovn-ic, each AZ has
>> one LR and its LS. The LRs are interconnected by a transit switch.
>> On each AZ, we can configure the dynamic routing and exchange routes
>> via BGP with external BGP speakers.
>>
>> AZ1
>> LS1 - LR1 - BGP Speaker1 - Local subnet1
>>        \
>>    -----------------------
>>     transit switch
>>    -----------------------
>> AZ2    /
>> LS2 - LR2 - BGP Speaker2 - Local subnet2
>>
>> The LR2 will learn the subnet1 prefix via ovn-ic but this prefix will
>> not be redistributed to BGP Speaker2 and it also happens with LR1.
>> This scenario uses the network's hub-and-spoke terminology where we can
>> enable the hub-spoke on the LR for such redistribution.
>> subnet1 and subnet2 are configured as ic-source-dynamic=true in ovn-ic.
>> Also, consider the same scenario, but LR1 and LR2 learn the same subnet
>> for redundancy, hub-spoke option is disabled to not adv the subnet learned
>> from BGP and advertised in ovn-ic in BGP speakers.
>>
>> Signed-off-by: Lucas Vargas Dias <[email protected]>
>> ---
>>
>
> Hi Lucas,
>
> sorry for the delay. I have a few comments down below. It also needs a
> rebase.
>
>
>>  NEWS                              |  6 ++
>>  ic/ovn-ic.c                       | 28 +++++++---
>>  northd/en-advertised-route-sync.c |  3 +
>>  northd/northd.c                   | 10 +++-
>>  northd/northd.h                   |  5 +-
>>  northd/ovn-northd.c               |  1 -
>>  ovn-nb.xml                        | 21 +++++++
>>  tests/ovn-ic.at                   | 92 +++++++++++++++++++++++++++++++
>>  8 files changed, 156 insertions(+), 10 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 2a2b5e12d..6002820f3 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -40,6 +40,8 @@ Post v25.09.0
>>       * Add the "options:dynamic-routing-no-learning" to Logical Routers
>> ports.
>>         If set to true, router port will not learn routes and will forget
>>         learned routes. This option has priority over its router
>> counterpart.
>> +     * Add support for hub-and-spoke propagation via the "hub-spoke"
>> option
>> +       in dynamic-routing-redistribute settings.
>>     - Add support for Network Function insertion in OVN with stateful
>> traffic
>>       redirection capability in Logical Switch datapath. The feature
>> introduces
>>       three new NB database tables:
>> @@ -98,6 +100,10 @@ Post v25.09.0
>>       reserving an unused IP from the backend's subnet. This change allows
>>       using LRP IPs directly, eliminating the need to reserve additional
>> IPs
>>       per backend port.
>> +   - Add the external_ids:ic-source-dynamic key for
>> +     Logical_Router_Static_Route to indicate whether a learned OVN-IC
>> route
>> +     originated from dynamic routing sources in the advertising
>> availability
>> +     zone.
>>
>>  OVN v25.09.0 - xxx xx xxxx
>>  --------------------------
>> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
>> index fd5ecefb3..e9fff2d4d 100644
>> --- a/ic/ovn-ic.c
>> +++ b/ic/ovn-ic.c
>> @@ -1283,6 +1283,7 @@ struct ic_route_info {
>>      struct in6_addr prefix;
>>      unsigned int plen;
>>      struct in6_addr nexthop;
>> +    bool is_src_dynamic;
>>      const char *origin;
>>      const char *route_table;
>>      const char *route_tag;
>> @@ -1554,7 +1555,7 @@ add_to_routes_ad(struct hmap *routes_ad, const
>> struct in6_addr prefix,
>>                   const struct nbrec_logical_router_static_route
>> *nb_route,
>>                   const struct nbrec_logical_router *nb_lr,
>>                   const struct nbrec_load_balancer *nb_lb,
>> -                 const char *route_tag)
>> +                 const char *route_tag, bool is_src_dynamic)
>>  {
>>      ovs_assert(nb_route || nb_lrp || nb_lb || nb_lr);
>>
>> @@ -1573,6 +1574,7 @@ add_to_routes_ad(struct hmap *routes_ad, const
>> struct in6_addr prefix,
>>          ic_route->nb_route = nb_route;
>>          ic_route->origin = origin;
>>          ic_route->route_table = route_table;
>> +        ic_route->is_src_dynamic = is_src_dynamic;
>>          ic_route->nb_lrp = nb_lrp;
>>          ic_route->nb_lr = nb_lr;
>>          ic_route->nb_lb = nb_lb;
>> @@ -1649,7 +1651,7 @@ add_static_to_routes_ad(
>>
>>      add_to_routes_ad(routes_ad, prefix, plen, nexthop,
>> ROUTE_ORIGIN_STATIC,
>>                       nb_route->route_table, NULL, nb_route, nb_lr,
>> -                     NULL, route_tag);
>> +                     NULL, route_tag, false);
>>  }
>>
>>  static void
>> @@ -1659,7 +1661,8 @@ add_network_to_routes_ad(struct hmap *routes_ad,
>> const char *network,
>>                           const struct smap *nb_options,
>>                           const struct nbrec_logical_router *nb_lr,
>>                           const char *route_tag,
>> -                         const struct nbrec_logical_router_port *ts_lrp)
>> +                         const struct nbrec_logical_router_port *ts_lrp,
>> +                         bool is_src_dynamic)
>>  {
>>      struct in6_addr prefix, nexthop;
>>      unsigned int plen;
>> @@ -1711,7 +1714,8 @@ add_network_to_routes_ad(struct hmap *routes_ad,
>> const char *network,
>>
>>      /* directly-connected routes go to <main> route table */
>>      add_to_routes_ad(routes_ad, prefix, plen, nexthop,
>> ROUTE_ORIGIN_CONNECTED,
>> -                     NULL, nb_lrp, NULL, nb_lr, NULL, route_tag);
>> +                     NULL, nb_lrp, NULL, nb_lr, NULL, route_tag,
>> +                     is_src_dynamic);
>>  }
>>
>>  static void
>> @@ -1769,7 +1773,7 @@ add_lb_vip_to_routes_ad(struct hmap *routes_ad,
>> const char *vip_key,
>>
>>      /* Lb vip routes go to <main> route table */
>>      add_to_routes_ad(routes_ad, vip_ip, plen, nexthop, ROUTE_ORIGIN_LB,
>> -                     NULL, NULL, NULL, nb_lr, nb_lb, route_tag);
>> +                     NULL, NULL, NULL, nb_lr, nb_lb, route_tag, false);
>>  out:
>>      free(vip_str);
>>  }
>> @@ -2187,6 +2191,12 @@ sync_learned_routes(struct ic_context *ctx,
>>                  nbrec_logical_router_static_route_update_options_setkey(
>>                      nb_route, "origin", isb_route->origin);
>>                  free(uuid_s);
>> +                bool is_src_dynamic =
>> smap_get_bool(&isb_route->external_ids,
>> +                    "ic-source-dynamic", false);
>> +                char *ic_source_dynamic_str = is_src_dynamic ?
>> +                    "true" : "false";
>> +
>> nbrec_logical_router_static_route_update_external_ids_setkey(
>> +                    nb_route, "ic-source-dynamic",
>> ic_source_dynamic_str);
>>
>
> This is never updated for existing routes, should we do that too?
>

I forgot this for the existing routes.


>
>
>>
>>  nbrec_logical_router_update_static_routes_addvalue(ic_lr->lr,
>>                      nb_route);
>>              }
>> @@ -2243,6 +2253,10 @@ ad_route_sync_external_ids(const struct
>> ic_route_info *route_adv,
>>                                                       "ic-route-tag");
>>          }
>>      }
>> +
>> +    char *ic_src_dynamic_str = route_adv->is_src_dynamic ? "true" :
>> "false";
>> +    icsbrec_route_update_external_ids_setkey(isb_route,
>> "ic-source-dynamic",
>> +                                             ic_src_dynamic_str);
>>  }
>>
>>  /* Sync routes from routes_ad to IC-SB. */
>> @@ -2372,7 +2386,7 @@ build_ts_routes_to_adv(struct ic_context *ctx,
>>                  add_network_to_routes_ad(routes_ad, lrp->networks[j],
>> lrp,
>>                                           ts_port_addrs,
>>                                           &nb_global->options,
>> -                                         lr, route_tag, ts_lrp);
>> +                                         lr, route_tag, ts_lrp, false);
>>              }
>>          } else {
>>              /* The router port of the TS port is ignored. */
>> @@ -2427,7 +2441,7 @@ build_ts_routes_to_adv(struct ic_context *ctx,
>>          add_network_to_routes_ad(routes_ad, sb_route->ip_prefix, NULL,
>>                                   ts_port_addrs,
>>                                   &nb_global->options,
>> -                                 lr, route_tag, ts_lrp);
>> +                                 lr, route_tag, ts_lrp, true);
>>      }
>>      sbrec_learned_route_index_destroy_row(filter);
>>  }
>> diff --git a/northd/en-advertised-route-sync.c
>> b/northd/en-advertised-route-sync.c
>> index be771391d..be046769f 100644
>> --- a/northd/en-advertised-route-sync.c
>> +++ b/northd/en-advertised-route-sync.c
>> @@ -675,6 +675,8 @@ should_advertise_route(const struct uuidset
>> *host_route_lrps,
>>          return drr_mode_NAT_is_set(drr);
>>      case ROUTE_SOURCE_LB:
>>          return drr_mode_LB_is_set(drr);
>> +    case ROUTE_SOURCE_IC_DYNAMIC:
>> +        return drr_mode_IC_DYNAMIC_is_set(drr);
>>      case ROUTE_SOURCE_LEARNED:
>>          OVS_NOT_REACHED();
>>      default:
>> @@ -745,6 +747,7 @@ advertise_route_track_od(struct
>> advertised_route_sync_data *data,
>>                             &tracked_op->od->nbr->header_.uuid);
>>          }
>>          break;
>> +    case ROUTE_SOURCE_IC_DYNAMIC:
>>      case ROUTE_SOURCE_CONNECTED:
>>      case ROUTE_SOURCE_STATIC:
>>          break;
>> diff --git a/northd/northd.c b/northd/northd.c
>> index b4bb4ba6d..fbb75533c 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -870,6 +870,10 @@ parse_dynamic_routing_redistribute(
>>              out |= DRRM_LB;
>>              continue;
>>          }
>> +        if (!strcmp(token, "hub-spoke")) {
>> +            out |= DRRM_IC_DYNAMIC;
>> +            continue;
>> +        }
>>          static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>>          VLOG_WARN_RL(&rl,
>>                       "unknown dynamic-routing-redistribute option '%s'
>> on %s",
>> @@ -12034,7 +12038,10 @@ parsed_routes_add_static(const struct
>> ovn_datapath *od,
>>      enum route_source source;
>>      if (!strcmp(smap_get_def(&route->options, "origin", ""),
>>                  ROUTE_ORIGIN_CONNECTED)) {
>> -        source = ROUTE_SOURCE_CONNECTED;
>> +        bool ic_src_dynamic = smap_get_bool(&route->external_ids,
>> +                                            "ic-source-dynamic", false);
>> +        source = ic_src_dynamic ?
>> +                 ROUTE_SOURCE_IC_DYNAMIC : ROUTE_SOURCE_CONNECTED;
>>      } else {
>>          source = ROUTE_SOURCE_STATIC;
>>      }
>> @@ -12129,6 +12136,7 @@ route_source_to_offset(enum route_source source)
>>  {
>>      switch (source) {
>>      case ROUTE_SOURCE_CONNECTED:
>> +    case ROUTE_SOURCE_IC_DYNAMIC:
>>          return ROUTE_PRIO_OFFSET_CONNECTED;
>>      case ROUTE_SOURCE_STATIC:
>>          return ROUTE_PRIO_OFFSET_STATIC;
>> diff --git a/northd/northd.h b/northd/northd.h
>> index eb5c15f34..e6ed2cc3e 100644
>> --- a/northd/northd.h
>> +++ b/northd/northd.h
>> @@ -372,7 +372,8 @@ struct mcast_port_info {
>>      DRR_MODE(CONNECTED_AS_HOST, 1) \
>>      DRR_MODE(STATIC,            2) \
>>      DRR_MODE(NAT,               3) \
>> -    DRR_MODE(LB,                4)
>> +    DRR_MODE(LB,                4) \
>> +    DRR_MODE(IC_DYNAMIC,        5)
>>
>>  enum dynamic_routing_redistribute_mode_bits {
>>  #define DRR_MODE(PROTOCOL, BIT) DRRM_##PROTOCOL##_BIT = BIT,
>> @@ -826,6 +827,8 @@ enum route_source {
>>      ROUTE_SOURCE_NAT,
>>      /* The route is derived from a LB's VIP. */
>>      ROUTE_SOURCE_LB,
>> +    /* The route is derived from an ovn-controller and advertised to IC.
>> */
>> +    ROUTE_SOURCE_IC_DYNAMIC,
>>  };
>>
>>  struct parsed_route {
>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>> index 7d7568c6f..bc3969053 100644
>> --- a/northd/ovn-northd.c
>> +++ b/northd/ovn-northd.c
>> @@ -905,7 +905,6 @@ main(int argc, char *argv[])
>>          &nbrec_load_balancer_col_external_ids,
>>          &nbrec_load_balancer_health_check_col_external_ids,
>>          &nbrec_logical_router_policy_col_external_ids,
>> -        &nbrec_logical_router_static_route_col_external_ids,
>>
>
> This is very dangerous, it means that any change
> of external_ids will wake up northd. I think it will be safer
> to make it an option as we track those already.
>
> On a second thought and looking through the
> is there a reason why we can't just add another origin?
> We check the origin anyway in northd, so having
> something like:
>
> #define ROUTE_ORIGIN_CONNECTED_DYNAMIC "connected-dynamic"
>
> That would avoid the tracking of external_ids, extra definiton
> of an option and would be probably smaller change in the
> ovn-ic overall, WDYT?
>

I agree, we can add a new origin. I'll adjust.



>
>          &nbrec_meter_col_external_ids,
>>          &nbrec_meter_band_col_external_ids,
>>          &nbrec_mirror_col_external_ids,
>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>> index 1acbf202b..5f9b47491 100644
>> --- a/ovn-nb.xml
>> +++ b/ovn-nb.xml
>> @@ -3374,6 +3374,13 @@ or
>>            Logical Switch.
>>          </p>
>>
>> +        <p>
>> +          If <code>hub-spoke</code> is in the list then northd will
>> synchronize
>> +          dynamic routes learned through OVN-IC from other routers into
>> the
>> +          <ref table="Advertised_Route" db="OVN_Southbound"/> table,
>> enabling
>> +          hub-and-spoke propagation.
>> +        </p>
>> +
>>          <p>
>>            This value can be overwritten on a per LRP basis using
>>            <ref column="options" key="dynamic-routing-redistribute"
>> @@ -4461,6 +4468,13 @@ or
>>            via shared Logical Switch.
>>          </p>
>>
>> +        <p>
>> +          If <code>hub-spoke</code> is in the list then northd will
>> synchronize
>> +          dynamic routes learned through OVN-IC from other routers into
>> the
>> +          <ref table="Advertised_Route" db="OVN_Southbound"/> table,
>> enabling
>> +          hub-and-spoke propagation.
>> +        </p>
>> +
>>          <p>
>>            If not set the value from <ref column="options"
>>            key="dynamic-routing-redistribute" table="Logical_Router"/> on
>> the
>> @@ -4823,6 +4837,13 @@ or
>>        database.
>>      </column>
>>
>> +    <column name="external_ids" key="ic-source-dynamic">
>> +      <code>ovn-ic</code> populates this key for routes learned from
>> +      <ref db="OVN_IC_Southbound"/>. The value is <code>true</code> if
>> the
>> +      learned route originated from dynamic sources (e.g. learned routes)
>> +      in the advertising availability zone, otherwise <code>false</code>.
>> +    </column>
>> +
>>      <group title="Common Columns">
>>        <column name="external_ids">
>>          See <em>External IDs</em> at the beginning of this document.
>> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
>> index 370a755be..a62d02da0 100644
>> --- a/tests/ovn-ic.at
>> +++ b/tests/ovn-ic.at
>> @@ -4586,3 +4586,95 @@ OVN_CLEANUP_IC([az1], [az2])
>>
>>  AT_CLEANUP
>>  ])
>> +
>> +
>> +OVN_FOR_EACH_NORTHD([
>> +AT_SETUP([ovn-ic -- Check ovn-ic adv and learn from SB Learned Route -
>> hub and spoke mode])
>> +
>> +ovn_init_ic_db
>> +
>> +for i in 1 2; do
>> +    ovn_start az$i
>> +    ovn_as az$i
>> +
>> +    # Enable route learning at AZ level
>> +    check ovn-nbctl set nb_global . options:ic-route-learn=true
>> +    # Enable route advertising at AZ level
>> +    check ovn-nbctl set nb_global . options:ic-route-adv=true
>> +done
>> +
>> +# Create new transit switches and LRs. Test topology is next:
>> +#
>> +#
>> +# logical router (lr11) - transit switch (ts11) - logical router (lr12)
>> +#
>> +#
>> +
>> +# Create lr11, lr12 and ts11 and connect them
>> +for i in 1 2; do
>> +    ovn_as az$i
>> +
>> +    lr=lr1$i
>> +    check ovn-nbctl lr-add $lr
>> +
>> +    ts=ts11
>> +    check ovn-ic-nbctl --wait=sb --may-exist ts-add $ts
>> +
>> +    lrp=lrp-$lr-$ts
>> +    lsp=lsp-$ts-$lr
>> +    # Create LRP and connect to TS
>> +    check ovn-nbctl lrp-add $lr $lrp aa:aa:aa:aa:a1:0$i 169.254.101.$i/24
>> +    check ovn-nbctl lsp-add-router-port $ts $lsp $lrp
>> +done
>> +
>> +# Create directly-connected route in lr12
>> +check ovn_as az2 ovn-nbctl lrp-add lr12 lrp-lr12 aa:aa:aa:aa:bb:01 "
>> 192.168.0.1/24"
>> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep
>> 192.168 |
>> +             grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
>> +192.168.0.0/24 169.254.101.2
>> +])
>> +
>> +ovn_as az2
>> +check ovn-nbctl --wait=sb set Logical_Router lr12
>> option:dynamic-routing=true \
>> +    option:dynamic-routing-redistribute="connected,static"
>> +check ovn_as az2 ovn-nbctl --wait=sb lrp-add lr12 lr12-dr1
>> 00:00:00:00:ff:01 10.0.0.1/24
>> +dr1=$(fetch_column port_binding _uuid logical_port=lr12-dr1)
>> +datapath=$(fetch_column datapath_binding _uuid external_ids:name=lr12)
>> +
>> +check_uuid ovn-sbctl create Learned_Route \
>> +    datapath=$datapath                    \
>> +    logical_port=$dr1                     \
>> +    ip_prefix=192.168.1.0/24              \
>> +    nexthop=10.0.0.20
>> +
>> +# Check Learned_Route adv in ovn-ic
>> +OVS_WAIT_FOR_OUTPUT([ovn_as az1 ovn-nbctl lr-route-list lr11 |
>> +             grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
>> +10.0.0.0/24 169.254.101.2
>> +192.168.0.0/24 169.254.101.2
>> +192.168.1.0/24 169.254.101.2
>> +])
>> +
>> +ovn_as az1
>> +check ovn-nbctl --wait=sb set Logical_Router lr11
>> option:dynamic-routing=true \
>> +    option:dynamic-routing-redistribute="connected,static"
>> +# Advertise just 10.0.0.0/24 and 192.168.0.0/24 routes
>> +check_row_count Advertised_Route 1 ip_prefix=192.168.0.0/24
>> +check_row_count Advertised_Route 1 ip_prefix=10.0.0.0/24
>> +check_row_count Advertised_Route 0 ip_prefix=192.168.1.0/24
>> +
>> +
>> +ovn_as az1
>> +check ovn-nbctl --wait=sb set Logical_Router lr11 \
>> +    option:dynamic-routing-redistribute="connected,static,hub-spoke"
>> +# Advertise just 10.0.0.0/24, 192.168.0.0/24 and 192.168.1.0/24 routes
>> +# Route 192.168.1.0/24 is learned by DR from other logical routes (lr12)
>> +# And lr12 Advertised it in ovn-ic. Hub-spoke option enable re-routing
>> in lr11
>> +check_row_count Advertised_Route 1 ip_prefix=192.168.0.0/24
>> +check_row_count Advertised_Route 1 ip_prefix=10.0.0.0/24
>> +check_row_count Advertised_Route 1 ip_prefix=192.168.1.0/24
>> +
>>
>
> I would add an extra check that removed the hub-spoke again just to be
> sure.
>

I agree


>
>
>> +OVN_CLEANUP_IC([az1], [az2])
>> +
>> +AT_CLEANUP
>> +])
>> --
>> 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
>>
>>
> Regards,
> Ales
>

-- 




_‘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

Reply via email to