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
