On Nov 28, Lucas Vargas Dias wrote:
> Hi,
>
> Em sex., 28 de nov. de 2025 às 06:51, Lorenzo Bianconi <
> [email protected]> escreveu:
>
> > > Logical router adv in ovn-ic the sb learned routes from
> > > dynamic routing. Also, it doesn't learn route in ovn-ic
> > > if prefix is already learned in sb learned route table.
> > >
> > > Signed-off-by: Lucas Vargas Dias <[email protected]>
> >
> > Hi Lucas,
> >
> > thx for fixing the comments on the previous version. Just some nitpicks
> > inline
> > this time. Can you please run './utilities/checkpatch.py' on the patch?
> >
> > Regards,
> > Lorenzo
> >
> > > ---
> > > ic/ovn-ic.c | 111 ++++++++++++++++++++++++++++++++++----
> > > tests/ovn-ic.at | 138 ++++++++++++++++++++++++++++++++++++++++++++++++
> > > 2 files changed, 240 insertions(+), 9 deletions(-)
> > >
> > > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> > > index ac8b9596c..f1f270164 100644
> > > --- a/ic/ovn-ic.c
> > > +++ b/ic/ovn-ic.c
> > > @@ -70,7 +70,9 @@ struct ic_context {
> > > struct ovsdb_idl_index *nbrec_lrp_by_name;
> > > struct ovsdb_idl_index *nbrec_port_by_name;
> > > struct ovsdb_idl_index *sbrec_chassis_by_name;
> > > + struct ovsdb_idl_index *sbrec_learned_route_by_datapath;
> > > struct ovsdb_idl_index *sbrec_port_binding_by_name;
> > > + struct ovsdb_idl_index *sbrec_datapath_binding_by_nb_uuid;
> > > struct ovsdb_idl_index *sbrec_service_monitor_by_remote_type;
> > > struct ovsdb_idl_index *sbrec_service_monitor_by_ic_learned;
> > > struct ovsdb_idl_index
> > *sbrec_service_monitor_by_remote_type_logical_port;
> > > @@ -660,6 +662,23 @@ find_sb_pb_by_name(struct ovsdb_idl_index
> > *sbrec_port_binding_by_name,
> > > return pb;
> > > }
> > >
> > > +static const struct sbrec_datapath_binding *
> > > +find_dp_by_uuid(struct ovsdb_idl_index *sbrec_datapath_binding,
> >
> > maybe better something like: sbrec_datapath_binding_by_nb_uuid?
> >
>
> What do you think about find_sb_dp_by_nb_uuid?
> Following the same pattern of find_sb_pb_by_name.
ack, I think that's fine.
Regards,
Lorenzo
>
> Regards,
> Lucas
>
>
> >
> > > + const struct uuid *nb_uuid)
> > > +{
> > > + const struct sbrec_datapath_binding *key =
> > > + sbrec_datapath_binding_index_init_row(sbrec_datapath_binding);
> > > +
> > > + sbrec_datapath_binding_set_nb_uuid(key, nb_uuid, 1);
> > > +
> > > + const struct sbrec_datapath_binding *dp =
> > > + sbrec_datapath_binding_index_find(sbrec_datapath_binding, key);
> > > + sbrec_datapath_binding_index_destroy_row(key);
> > > +
> >
> > drop a new-line here.
> >
> > > +
> > > + return dp;
> > > +}
> > > +
> > > static const struct sbrec_port_binding *
> > > find_peer_port(struct ic_context *ctx,
> > > const struct sbrec_port_binding *sb_pb)
> > > @@ -1575,7 +1594,7 @@ add_to_routes_ad(struct hmap *routes_ad, const
> > struct in6_addr prefix,
> > > const struct nbrec_load_balancer *nb_lb,
> > > const char *route_tag)
> > > {
> > > - ovs_assert(nb_route || nb_lrp || nb_lb);
> > > + ovs_assert(nb_route || nb_lrp || nb_lb || nb_lr);
> > >
> > > if (route_table == NULL) {
> > > route_table = "";
> > > @@ -1607,9 +1626,12 @@ add_to_routes_ad(struct hmap *routes_ad, const
> > struct in6_addr prefix,
> > > } else if (nb_lb) {
> > > VLOG_WARN_RL(&rl, msg_fmt, origin, "loadbalancer",
> > > UUID_ARGS(&nb_lb->header_.uuid));
> > > - } else {
> > > + } else if (nb_lrp){
> >
> > you are missing a space here.
> >
> > > VLOG_WARN_RL(&rl, msg_fmt, origin, "lrp",
> > > UUID_ARGS(&nb_lrp->header_.uuid));
> > > + } else {
> > > + VLOG_WARN_RL(&rl, msg_fmt, origin, "route",
> > > + UUID_ARGS(&nb_lr->header_.uuid));
> > > }
> > > }
> > > }
> > > @@ -1685,8 +1707,16 @@ add_network_to_routes_ad(struct hmap *routes_ad,
> > const char *network,
> > >
> > > if (!route_need_advertise(NULL, &prefix, plen, nb_options,
> > > nb_lr, ts_lrp)) {
> > > - VLOG_DBG("Route ad: skip network %s of lrp %s.",
> > > - network, nb_lrp->name);
> > > + if (VLOG_IS_DBG_ENABLED()) {
> > > + struct ds msg = DS_EMPTY_INITIALIZER;
> > > + ds_put_format(&msg, "Route ad: skip network %s", network);
> > > + if (nb_lrp) {
> > > + ds_put_format(&msg, " of lrp %s", nb_lrp->name);
> > > + }
> > > + ds_put_format(&msg, ".");
> > > + VLOG_DBG("%s", ds_cstr(&msg));
> > > + ds_destroy(&msg);
> > > + }
> > > return;
> > > }
> > >
> > > @@ -1700,8 +1730,12 @@ add_network_to_routes_ad(struct hmap *routes_ad,
> > const char *network,
> > > struct ds msg = DS_EMPTY_INITIALIZER;
> > >
> > > ds_put_format(&msg, "Adding direct network route to <main>
> > routing "
> > > - "table: %s of lrp %s, nexthop ", network,
> > nb_lrp->name);
> > > + "table: %s", network);
> > >
> > > + if (nb_lrp) {
> > > + ds_put_format(&msg, " of lrp %s,", nb_lrp->name);
> > > + }
> > > + ds_put_format(&msg, " nexthop ");
> > > if (IN6_IS_ADDR_V4MAPPED(&nexthop)) {
> > > ds_put_format(&msg, IP_FMT,
> > > IP_ARGS(in6_addr_get_mapped_ipv4(&nexthop)));
> > > @@ -1882,7 +1916,8 @@ route_matches_local_lb(const struct
> > nbrec_load_balancer *nb_lb,
> > > }
> > >
> > > static bool
> > > -route_need_learn(const struct nbrec_logical_router *lr,
> > > +route_need_learn(struct ic_context *ctx,
> > > + const struct nbrec_logical_router *lr,
> > > const struct icsbrec_route *isb_route,
> > > struct in6_addr *prefix, unsigned int plen,
> > > const struct smap *nb_options,
> > > @@ -1952,6 +1987,27 @@ route_need_learn(const struct
> > nbrec_logical_router *lr,
> > > }
> > > }
> > >
> > > + const struct sbrec_datapath_binding *dp =
> > > + find_dp_by_uuid(ctx->sbrec_datapath_binding_by_nb_uuid,
> > > + &lr->header_.uuid);
> > > + if (!dp) {
> > > + return true;
> > > + }
> > > +
> >
> > drop a new-line here.
> >
> > > +
> > > + struct sbrec_learned_route *filter =
> > sbrec_learned_route_index_init_row(
> > > + ctx->sbrec_learned_route_by_datapath);
> > > + sbrec_learned_route_index_set_datapath(filter, dp);
> > > + struct sbrec_learned_route *sb_route;
> > > + SBREC_LEARNED_ROUTE_FOR_EACH_EQUAL (sb_route, filter,
> > > +
> > ctx->sbrec_learned_route_by_datapath) {
> > > + if (!strcmp(isb_route->ip_prefix, sb_route->ip_prefix)) {
> > > + sbrec_learned_route_index_destroy_row(filter);
> > > + return false;
> > > + }
> > > + }
> > > + sbrec_learned_route_index_destroy_row(filter);
> > > +
> > > return true;
> > > }
> > >
> > > @@ -2125,7 +2181,7 @@ sync_learned_routes(struct ic_context *ctx,
> > > isb_route->nexthop);
> > > continue;
> > > }
> > > - if (!route_need_learn(ic_lr->lr, isb_route, &prefix, plen,
> > > + if (!route_need_learn(ctx, ic_lr->lr, isb_route, &prefix,
> > plen,
> > > &nb_global->options, lrp, &nexthop)) {
> > > continue;
> > > }
> > > @@ -2196,7 +2252,8 @@ ad_route_sync_external_ids(const struct
> > ic_route_info *route_adv,
> > > smap_get_uuid(&isb_route->external_ids, "lr-id", &isb_ext_lr_id);
> > > nb_id = route_adv->nb_lb ? route_adv->nb_lb->header_.uuid :
> > > route_adv->nb_route ? route_adv->nb_route->header_.uuid :
> > > - route_adv->nb_lrp->header_.uuid;
> > > + route_adv->nb_lrp ? route_adv->nb_lrp->header_.uuid :
> > > + route_adv->nb_lr->header_.uuid;
> > >
> > > lr_id = route_adv->nb_lr->header_.uuid;
> > > if (!uuid_equals(&isb_ext_id, &nb_id)) {
> > > @@ -2388,6 +2445,26 @@ build_ts_routes_to_adv(struct ic_context *ctx,
> > > }
> > > }
> > > }
> > > +
> > > + const struct sbrec_datapath_binding *dp =
> > > + find_dp_by_uuid(ctx->sbrec_datapath_binding_by_nb_uuid,
> > > + &lr->header_.uuid);
> > > + if (!dp) {
> > > + return;
> > > + }
> > > +
> > > + struct sbrec_learned_route *filter =
> > sbrec_learned_route_index_init_row(
> > > + ctx->sbrec_learned_route_by_datapath);
> > > + sbrec_learned_route_index_set_datapath(filter, dp);
> > > + struct sbrec_learned_route *sb_route;
> > > + SBREC_LEARNED_ROUTE_FOR_EACH_EQUAL (sb_route, filter,
> > > +
> > ctx->sbrec_learned_route_by_datapath) {
> > > + add_network_to_routes_ad(routes_ad, sb_route->ip_prefix, NULL,
> > > + ts_port_addrs,
> > > + &nb_global->options,
> > > + lr, route_tag, ts_lrp);
> > > + }
> > > + sbrec_learned_route_index_destroy_row(filter);
> > > }
> > >
> > > static void
> > > @@ -2484,7 +2561,7 @@ delete_orphan_ic_routes(struct ic_context *ctx,
> > > static void
> > > route_run(struct ic_context *ctx)
> > > {
> > > - if (!ctx->ovnisb_txn || !ctx->ovnnb_txn) {
> > > + if (!ctx->ovnisb_txn || !ctx->ovnnb_txn || !ctx->ovnsb_txn) {
> > > return;
> > > }
> > >
> > > @@ -3472,6 +3549,11 @@ main(int argc, char *argv[])
> > > ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> > > &sbrec_service_monitor_col_options);
> > >
> > > + ovsdb_idl_add_table(ovnsb_idl_loop.idl, &sbrec_table_learned_route);
> > > + ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> > > + &sbrec_learned_route_col_ip_prefix);
> > > + ovsdb_idl_add_column(ovnsb_idl_loop.idl,
> > > + &sbrec_learned_route_col_datapath);
> > > /* Create IDL indexes */
> > > struct ovsdb_idl_index *nbrec_ls_by_name
> > > = ovsdb_idl_index_create1(ovnnb_idl_loop.idl,
> > > @@ -3487,10 +3569,17 @@ main(int argc, char *argv[])
> > > struct ovsdb_idl_index *sbrec_port_binding_by_name
> > > = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
> > > &sbrec_port_binding_col_logical_port);
> > > + struct ovsdb_idl_index *sbrec_datapath_binding_by_nb_uuid
> > > + = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
> > > + &sbrec_datapath_binding_col_nb_uuid);
> > > struct ovsdb_idl_index *sbrec_chassis_by_name
> > > = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
> > > &sbrec_chassis_col_name);
> > >
> > > + struct ovsdb_idl_index *sbrec_learned_route_by_datapath
> > > + = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
> > > + &sbrec_learned_route_col_datapath);
> > > +
> > > struct ovsdb_idl_index *sbrec_service_monitor_by_remote_type
> > > = ovsdb_idl_index_create1(ovnsb_idl_loop.idl,
> > > &sbrec_service_monitor_col_remote);
> > > @@ -3598,7 +3687,11 @@ main(int argc, char *argv[])
> > > .nbrec_lrp_by_name = nbrec_lrp_by_name,
> > > .nbrec_port_by_name = nbrec_port_by_name,
> > > .sbrec_port_binding_by_name =
> > sbrec_port_binding_by_name,
> > > + .sbrec_datapath_binding_by_nb_uuid =
> > > + sbrec_datapath_binding_by_nb_uuid,
> > > .sbrec_chassis_by_name = sbrec_chassis_by_name,
> > > + .sbrec_learned_route_by_datapath =
> > > + sbrec_learned_route_by_datapath,
> > > .sbrec_service_monitor_by_remote_type =
> > > sbrec_service_monitor_by_remote_type,
> > > .sbrec_service_monitor_by_ic_learned =
> > > diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> > > index c444cc214..809420111 100644
> > > --- a/tests/ovn-ic.at
> > > +++ b/tests/ovn-ic.at
> > > @@ -3502,6 +3502,144 @@ 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])
> > > +
> > > +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
> > > +
> > > + for j in 1; do
> >
> > Why do you need this 'for' here? I guess you can drop it
> >
> > > + ts=ts1$j
> > > + 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:a$j:0$i
> > 169.254.10$j.$i/24
> > > + check ovn-nbctl lsp-add $ts $lsp \
> > > + -- lsp-set-addresses $lsp router \
> > > + -- lsp-set-type $lsp router \
> > > + -- lsp-set-options $lsp router-port=$lrp
> > > + done
> > > +done
> > > +
> > > +# Create directly-connected route in lr11
> > > +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
> > 192.168 |
> > > + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> > > +192.168.0.0/24 169.254.101.2
> > > +192.168.1.0/24 169.254.101.2
> > > +])
> > > +
> > > +ovn_as az2
> > > +learned=$(fetch_column Learned_Route _uuid ip_prefix=192.168.1.0/24)
> > > +check ovn-sbctl destroy Learned_Route $learned
> > > +
> > > +# Check Learned_Route removed is not adv and learned.
> > > +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
> > > +])
> > > +
> > > +
> > > +# Duplicate routes, SB learn route by two paths.
> > > +ovn_as az2
> > > +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 ovn-nbctl --wait=sb lrp-add lr12 lr12-dr2 00:00:00:00:ff:02
> > 169.254.254.1/24
> > > +dr2=$(fetch_column port_binding _uuid logical_port=lr12-dr2)
> > > +
> > > +check_uuid ovn-sbctl create Learned_Route \
> > > + datapath=$datapath \
> > > + logical_port=$dr2 \
> > > + ip_prefix=192.168.1.0/24 \
> > > + nexthop=169.254.254.3
> > > +
> > > +check_uuid ovn-sbctl create Learned_Route \
> > > + datapath=$datapath \
> > > + logical_port=$dr2 \
> > > + ip_prefix=192.168.2.0/24 \
> > > + nexthop=169.254.254.3
> > > +
> > > +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
> > > +192.168.1.0/24 169.254.101.2
> > > +192.168.2.0/24 169.254.101.2
> > > +])
> > > +
> > > +# Insert the same route Learned Route in the lr11 and don't learn by
> > ovn-ic.
> > > +ovn_as az1
> > > +datapath_lr11=$(fetch_column datapath_binding _uuid
> > external_ids:name=lr11)
> > > +check ovn-nbctl --wait=sb lrp-add lr11 lr11-dr1 00:00:00:00:ff:03
> > 169.254.254.2/24
> > > +sw1_dr1=$(fetch_column port_binding _uuid logical_port=lr11-dr1)
> > > +check_uuid ovn-sbctl create Learned_Route \
> > > + datapath=$datapath_lr11 \
> > > + logical_port=$sw1_dr1 \
> > > + ip_prefix=192.168.1.0/24 \
> > > + nexthop=169.254.254.3
> > > +
> > > +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
> > > +192.168.2.0/24 169.254.101.2
> > > +])
> > > +
> > > +OVS_WAIT_FOR_OUTPUT([ovn_as az2 ovn-nbctl lr-route-list lr12 | grep
> > 192.168 |
> > > + grep learned | awk '{print $1, $2}' | sort ], [0], [dnl
> > > +])
> > > +
> > > +OVN_CLEANUP_IC([az1], [az2])
> > > +
> > > +AT_CLEANUP
> > > +])
> > > +
> > > AT_BANNER([OVN Interconnection Service Monitor synchronization])
> > > OVN_FOR_EACH_NORTHD([
> > > AT_SETUP([ovn-ic -- Service Monitor synchronization: mock_test])
> > > --
> > > 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
