Hi all, I think a good idea is to create a new function extract_lrp_networks_and_mac to indicate that it will extract network and mac, and change the function extract_lrp_networks to extract ipv4/ipv6 addresses only.
Regards, Lucas Em qui., 3 de abr. de 2025 às 14:19, Lucas Vargas Dias (Dev - Cloud IaaS Network R&D) <[email protected]> escreveu: > Hi Dumitru, > > I'm checking patch again, for some reason nbrec_logical_router_port > pointer from get_lrp_by_lrp_name returns lrp without MAC and > extract_lrp_networks fail. Do you think it's better to investigate the > return from get_lrp_by_lrp_name or > use another function to extract network addresses from lrp? > > > Regards, > Lucas > > > Em qui., 3 de abr. de 2025 às 06:33, Dumitru Ceara <[email protected]> > escreveu: > >> On 3/21/25 6:09 PM, Lucas Vargas Dias via dev wrote: >> > Check if lsp exists in TS, if router-port of lsp exist >> > and lrp has the same network address of adversing. >> > If some of conditions is false, remove the route of routes >> > advertising. >> > Also, check if lrp that's learning a new route has address in the >> > same subnet of neighbor. >> > These verifications avoid loop that generates a high CPU load viewed in >> > ovsdb-server. >> > >> > Reported-at: >> https://www.mail-archive.com/[email protected]/msg10304.html >> > Signed-off-by: Lucas Vargas Dias <[email protected]> >> > --- >> >> Hi Lucas, >> >> This patch fails quite a few of the ovn-ic tests in CI (also the new >> ones you're adding fail): >> >> >> https://github.com/ovsrobot/ovn/actions/runs/13998069125/job/39198211082#step:10:5554 >> >> I didn't dig too much into why that happens. I did leave a few minor >> comments below. >> >> > ic/ovn-ic.c | 105 ++++++++++++++++++++++- >> > tests/ovn-ic.at | 220 ++++++++++++++++++++++++++++++++++++++++++++++++ >> > 2 files changed, 322 insertions(+), 3 deletions(-) >> > >> > diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c >> > index c8796680b..959885d9d 100644 >> > --- a/ic/ovn-ic.c >> > +++ b/ic/ovn-ic.c >> > @@ -1315,11 +1315,48 @@ route_has_local_gw(const struct >> nbrec_logical_router *lr, >> > return false; >> > } >> > >> > +static bool >> > +lrp_has_neighbor_in_ts(const struct nbrec_logical_router_port *lrp, >> > + struct in6_addr *nexthop) >> > +{ >> > + if (!lrp || !nexthop) { >> > + return false; >> > + } >> > + >> > + struct lport_addresses lrp_networks; >> > + if (!extract_lrp_networks(lrp, &lrp_networks)) { >> > + return false; >> > + } >> > + >> > + if (IN6_IS_ADDR_V4MAPPED(nexthop)) { >> > + ovs_be32 neigh_prefix_v4 = in6_addr_get_mapped_ipv4(nexthop); >> > + for (size_t i = 0; i < lrp_networks.n_ipv4_addrs; i++) { >> >> Nit: extra space. >> >> > + struct ipv4_netaddr address = lrp_networks.ipv4_addrs[i]; >> >> Nit: extra space. >> >> > + if (address.network == (neigh_prefix_v4 & address.mask)) { >> > + return true; >> > + } >> > + } >> > + } else { >> > + for (size_t i = 0; i < lrp_networks.n_ipv6_addrs; i++) { >> > + struct ipv6_netaddr address = lrp_networks.ipv6_addrs[i]; >> >> Nit: extra spaces. >> >> > + struct in6_addr neigh_prefix = ipv6_addr_bitand(nexthop, >> > + >> &address.mask); >> > + if (ipv6_addr_equals(&address.network, &neigh_prefix)) { >> > + return true; >> > + } >> > + } >> > + } >> > + >> > + return false; >> > +} >> > + >> > static bool >> > route_need_learn(const struct nbrec_logical_router *lr, >> > const struct icsbrec_route *isb_route, >> > struct in6_addr *prefix, unsigned int plen, >> > - const struct smap *nb_options) >> > + const struct smap *nb_options, >> > + const struct nbrec_logical_router_port *ts_lrp, >> > + struct in6_addr *nexthop) >> > { >> > if (!smap_get_bool(nb_options, "ic-route-learn", false)) { >> > return false; >> > @@ -1344,6 +1381,10 @@ route_need_learn(const struct >> nbrec_logical_router *lr, >> > return false; >> > } >> > >> > + if (!lrp_has_neighbor_in_ts(ts_lrp, nexthop)) { >> > + return false; >> > + } >> > + >> > return true; >> > } >> > >> > @@ -1374,6 +1415,63 @@ get_lrp_by_lrp_name(struct ic_context *ctx, >> const char *lrp_name) >> > return lrp; >> > } >> > >> > +static const struct nbrec_logical_router_port * >> > +find_lrp_of_nexthop(struct ic_context *ctx, >> > + const struct icsbrec_route *isb_route) >> > +{ >> > + const struct nbrec_logical_router_port *lrp; >> > + const struct nbrec_logical_switch *ls; >> > + ls = find_ts_in_nb(ctx, isb_route->transit_switch); >> > + if (!ls) { >> > + return NULL; >> > + } >> > + >> > + struct in6_addr nexthop; >> > + if (!ip46_parse(isb_route->nexthop, &nexthop)) { >> > + return NULL; >> > + } >> > + >> > + for (size_t i = 0; i < ls->n_ports; i++) { >> > + char *lsp_name = ls->ports[i]->name; >> > + const char *lrp_name = get_lrp_name_by_ts_port_name(ctx, >> > + lsp_name); >> > + if (!lrp_name) { >> > + continue; >> > + } >> > + >> > + lrp = get_lrp_by_lrp_name(ctx, lrp_name); >> > + if (!lrp) { >> > + continue; >> > + } >> > + >> > + struct lport_addresses lrp_networks; >> > + if (!extract_lrp_networks(lrp, &lrp_networks)) { >> > + continue; >> > + } >> > + >> > + if (IN6_IS_ADDR_V4MAPPED(&nexthop)) { >> > + ovs_be32 nexthop_v4 = in6_addr_get_mapped_ipv4(&nexthop); >> > + for (size_t i_v4 = 0; i_v4 < lrp_networks.n_ipv4_addrs; >> i_v4++) { >> >> Nit: extra space. >> >> > + struct ipv4_netaddr address = >> lrp_networks.ipv4_addrs[i_v4]; >> > + if (address.addr == nexthop_v4) { >> > + return lrp; >> > + } >> > + } >> > + } else { >> > + for (size_t i_v6 = 0; i_v6 < lrp_networks.n_ipv6_addrs; >> i_v6++) { >> > + struct ipv6_netaddr address = >> lrp_networks.ipv6_addrs[i_v6]; >> >> Nit: extra space. >> >> > + struct in6_addr nexthop_v6 = ipv6_addr_bitand(&nexthop, >> > + >> &address.mask); >> > + if (ipv6_addr_equals(&address.network, &nexthop_v6)) { >> > + return lrp; >> > + } >> > + } >> > + } >> > + } >> > + >> > + return NULL; >> > +} >> > + >> > static bool >> > lrp_is_ts_port(struct ic_context *ctx, struct ic_router_info *ic_lr, >> > const char *lrp_name) >> > @@ -1467,7 +1565,7 @@ sync_learned_routes(struct ic_context *ctx, >> > continue; >> > } >> > if (!route_need_learn(ic_lr->lr, isb_route, &prefix, plen, >> > - &nb_global->options)) { >> > + &nb_global->options, lrp, &nexthop)) >> { >> > continue; >> > } >> > >> > @@ -1480,6 +1578,7 @@ sync_learned_routes(struct ic_context *ctx, >> > struct uuid ext_id; >> > smap_get_uuid(&route_learned->nb_route->external_ids, >> > "ic-learned-route", &ext_id); >> > + >> >> Nit: unrelated. >> >> > if (!uuid_equals(&ext_id, &isb_route->header_.uuid)) { >> > char *uuid_s = >> > xasprintf(UUID_FMT, >> > @@ -1778,7 +1877,7 @@ delete_orphan_ic_routes(struct ic_context *ctx, >> > ctx->icnbrec_transit_switch_by_name, t_sw_key); >> > icnbrec_transit_switch_index_destroy_row(t_sw_key); >> > >> > - if (!t_sw) { >> > + if (!t_sw || !find_lrp_of_nexthop(ctx, isb_route)) { >> > static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, >> 1); >> > VLOG_INFO_RL(&rl, "Deleting orphan ICDB:Route: %s->%s (%s, >> rtb:%s," >> > " transit switch: %s)", isb_route->ip_prefix, >> > diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at >> > index e62ae3a35..9c871672b 100644 >> > --- a/tests/ovn-ic.at >> > +++ b/tests/ovn-ic.at >> > @@ -3002,3 +3002,223 @@ OVN_CHECK_PACKETS([hv3/vif5-tx.pcap], >> [expected-vif5]) >> > OVN_CLEANUP_IC([az1], [az2], [az3]) >> > AT_CLEANUP >> > ]) >> > + >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([ovn-ic -- Delete route lsp orphan]) >> > + >> > +ovn_init_ic_db >> > +ovn-ic-nbctl ts-add ts1 >> >> Nit: missing check. >> >> > + >> > +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: >> > +# >> > +# / transit switch (ts11) \ >> > +# logical router (lr11) - - logical router (lr12) >> > +# \ transit switch (ts12) / >> > +# >> > + >> > +# VPC1 >> > +# create lr11, lr12, ts11, ts12 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 2; do >> > + ts=ts1$j >> > + ovn-ic-nbctl --wait=sb --may-exist ts-add $ts >> >> Nit: missing check (there's a few more below). >> >> > + >> > + 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 VPC1 >> > +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 >> > +192.168.0.0/24 169.254.102.2 >> > +]) >> > + >> > +check ovn_as az2 ovn-nbctl lrp-del lrp-lr12-ts12 >> > + >> > +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_CLEANUP_IC([az1], [az2]) >> > + >> > +AT_CLEANUP >> > +]) >> > + >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([ovn-ic -- Delete route lrp orphan]) >> > + >> > +ovn_init_ic_db >> > +ovn-ic-nbctl ts-add ts1 >> > + >> > +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: >> > +# >> > +# / transit switch (ts11) \ >> > +# logical router (lr11) - - logical router (lr12) >> > +# \ transit switch (ts12) / >> > +# >> > + >> > +# VPC1 >> > +# create lr11, lr12, ts11, ts12 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 2; do >> > + ts=ts1$j >> > + 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 VPC1 >> > +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 >> > +192.168.0.0/24 169.254.102.2 >> > +]) >> > + >> > +check ovn_as az2 ovn-nbctl lsp-del lsp-ts12-lr12 >> > + >> > +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_CLEANUP_IC([az1], [az2]) >> > + >> > +AT_CLEANUP >> > +]) >> > + >> > +OVN_FOR_EACH_NORTHD([ >> > +AT_SETUP([ovn-ic -- Check loop with lsp orphan in same subnet of new >> lsp in other TS]) >> > + >> > +ovn_init_ic_db >> > +ovn-ic-nbctl ts-add ts1 >> > + >> > +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: >> > +# >> > +# / transit switch (ts11) \ >> > +# logical router (lr11) - - logical router (lr12) >> > +# \ transit switch (ts12) / >> > +# >> > + >> > +# 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 >> > + 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 VPC1 >> > +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 >> > +]) >> > + >> > +# Delete LRP connection from lr11 to ts11, keep lsp orphan >> > +check ovn_as az1 ovn-nbctl lrp-del lrp-lr11-ts11 >> > + >> > +# create ts12, and connect lr11 and lr21 to it in the same subnet of >> ts11 where >> > +# exist one lsp orphan >> > +for i in 1 2; do >> > + ovn_as az$i >> > + ts=ts12 >> > + >> > + lr=lr1$i >> > + for j in 1; do >> > + 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$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 >> > + >> > +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.22 >> > +]) >> > + >> > +OVN_CLEANUP_IC([az1], [az2]) >> > + >> > +AT_CLEANUP >> > +]) >> >> Regards, >> Dumitru >> >> >> -- _‘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
