Em seg., 8 de set. de 2025 às 15:09, Jacob Tanenbaum <[email protected]> escreveu:
> I am by no means an expert in this area of code, I have one comment. > > On Mon, Sep 8, 2025 at 12:12 PM Lucas Vargas Dias via dev < > [email protected]> wrote: > >> 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]> >> --- >> ic/ovn-ic.c | 98 ++++++++++++++++++++++++++++++++---- >> tests/ovn-ic.at | 130 ++++++++++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 219 insertions(+), 9 deletions(-) >> >> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c >> index d1a59d372..a98749817 100644 >> --- a/ic/ovn-ic.c >> +++ b/ic/ovn-ic.c >> @@ -513,6 +513,27 @@ 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_name(struct ic_context *ctx, >> + const char *name) >> > > Why not make this find_dp_by_uuid()? Using the uuid stored in the > ovn_datapath at header_.uuid comparing using uuid_equals() to the > sbrec_datapath_bindings->nb_uuid? > > You're right. I forgot about nb_uuid. Thanks for the advice. > >> +{ >> + struct smap external_ids = SMAP_INITIALIZER(&external_ids); >> + smap_add(&external_ids, "name", name); >> + >> + const struct sbrec_datapath_binding_table *dp_t = >> + sbrec_datapath_binding_table_get(ctx->ovnsb_idl); >> + const struct sbrec_datapath_binding *dp; >> + SBREC_DATAPATH_BINDING_TABLE_FOR_EACH (dp, dp_t) { >> + const char *found_value = smap_get(&dp->external_ids, "name"); >> + if (found_value && !strcmp(found_value, name)) { >> + smap_destroy(&external_ids); >> + return dp; >> + } >> + } >> + smap_destroy(&external_ids); >> + return NULL; >> +} >> + >> static const struct sbrec_port_binding * >> find_peer_port(struct ic_context *ctx, >> const struct sbrec_port_binding *sb_pb) >> @@ -1191,7 +1212,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 = ""; >> @@ -1223,9 +1244,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){ >> 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)); >> } >> } >> } >> @@ -1301,8 +1325,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; >> } >> >> @@ -1316,8 +1348,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))); >> @@ -1498,7 +1534,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, >> @@ -1564,6 +1601,21 @@ route_need_learn(const struct nbrec_logical_router >> *lr, >> } >> } >> >> + const struct sbrec_datapath_binding *dp = find_dp_by_name(ctx, >> lr->name); >> + if (!dp) { >> + return true; >> + } >> + const struct sbrec_learned_route_table *sbrec_learned_route_table = >> + >> sbrec_learned_route_table_get(ctx->ovnsb_idl); >> + const struct sbrec_learned_route *sb_route; >> + SBREC_LEARNED_ROUTE_TABLE_FOR_EACH_SAFE (sb_route, >> + sbrec_learned_route_table) { >> + if (uuid_equals(&sb_route->datapath->header_.uuid, >> &dp->header_.uuid) >> + && !strcmp(isb_route->ip_prefix, sb_route->ip_prefix)) { >> + return false; >> + } >> + } >> + >> return true; >> } >> >> @@ -1686,7 +1738,7 @@ sync_learned_routes(struct ic_context *ctx, >> const struct nbrec_logical_router_port *lrp; >> VECTOR_FOR_EACH (&ic_lr->isb_pbs, isb_pb) { >> lrp_name = get_lrp_name_by_ts_port_name(ctx, >> isb_pb->logical_port); >> - lrp = get_lrp_by_lrp_name(ctx, lrp_name); >> + lrp = lrp_name ? get_lrp_by_lrp_name(ctx, lrp_name) : NULL; >> if (lrp) { >> ts_route_table = smap_get_def(&lrp->options, "route_table", >> ""); >> route_filter_tag = smap_get_def(&lrp->options, >> @@ -1745,7 +1797,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; >> } >> @@ -1816,7 +1868,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)) { >> @@ -2008,6 +2061,24 @@ build_ts_routes_to_adv(struct ic_context *ctx, >> } >> } >> } >> + >> + const struct sbrec_datapath_binding *dp = find_dp_by_name(ctx, >> lr->name); >> + if (!dp) { >> + return; >> + } >> + const struct sbrec_learned_route_table *sbrec_learned_route_table = >> + >> sbrec_learned_route_table_get(ctx->ovnsb_idl); >> + const struct sbrec_learned_route *sb_route; >> + SBREC_LEARNED_ROUTE_TABLE_FOR_EACH_SAFE (sb_route, >> + sbrec_learned_route_table) { >> + if (uuid_equals(&sb_route->datapath->header_.uuid, >> + &dp->header_.uuid)) { >> + add_network_to_routes_ad(routes_ad, sb_route->ip_prefix, >> NULL, >> + ts_port_addrs, >> + &nb_global->options, >> + lr, route_tag, ts_lrp); >> + } >> + } >> } >> >> static void >> @@ -3067,6 +3138,15 @@ 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_nexthop); >> + 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_logical_port); >> + 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, >> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at >> index 368b6701b..cf280ea7b 100644 >> --- a/tests/ovn-ic.at >> +++ b/tests/ovn-ic.at >> @@ -4047,3 +4047,133 @@ OVN_CLEANUP_IC([az1], [az2], [az3]) >> 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 >> + 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 >> + >> + >> +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 >> +]) >> + >> +# 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 >> +]) >> + >> +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 >> +]) >> -- >> 2.34.1 >> >> >> -- >> >> >> >> >> _'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 >> >> Best, Lucas Dias -- _‘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
