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/ovs-discuss@openvswitch.org/msg10304.html > Signed-off-by: Lucas Vargas Dias <lucas.vd...@luizalabs.com> > ---
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 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
