On 3/7/25 7:36 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,
Thanks for the patch! And sorry for the delay in reviews.
> ic/ovn-ic.c | 113 ++++++++++++++++++++++++-
> tests/ovn-ic.at | 220 ++++++++++++++++++++++++++++++++++++++++++++++++
> 2 files changed, 331 insertions(+), 2 deletions(-)
>
> diff --git a/ic/ovn-ic.c b/ic/ovn-ic.c
> index c8796680b..ee5e7dbf4 100644
> --- a/ic/ovn-ic.c
> +++ b/ic/ovn-ic.c
> @@ -1315,11 +1315,63 @@ 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)
> +{
> + bool has_neighbor_in_same_tw = false;
> + struct in6_addr lrp_address;
> + unsigned int lrp_mask_len;
> + if (!lrp || !nexthop) {
> + return false;
> + }
> +
> + for (size_t n_addr = 0; n_addr < lrp->n_networks; n_addr++) {
> + char *network = lrp->networks[n_addr];
> + if (!network) {
> + continue;
> + }
> +
> + if (!ip46_parse_cidr(network, &lrp_address, &lrp_mask_len)) {
> + continue;
> + }
> +
> + if (IN6_IS_ADDR_V4MAPPED(&lrp_address) !=
> + IN6_IS_ADDR_V4MAPPED(nexthop)) {
> + continue;
> + }
> +
> + if (IN6_IS_ADDR_V4MAPPED(&lrp_address)) {
> + ovs_be32 neigh_prefix_v4 = in6_addr_get_mapped_ipv4(nexthop);
> + ovs_be32 prefix_v4 = in6_addr_get_mapped_ipv4(&lrp_address);
> + ovs_be32 mask = be32_prefix_mask(lrp_mask_len);
> + if ((prefix_v4 & mask) == (neigh_prefix_v4 & mask)) {
> + has_neighbor_in_same_tw = true;
No need for the local variable, we could just return true here.
> + break;
> + }
> + } else {
> + struct in6_addr mask = ipv6_create_mask(lrp_mask_len);
> + struct in6_addr lrp_ipv6_prefix = ipv6_addr_bitand(&lrp_address,
> + &mask);
> + struct in6_addr neihgbor_ipv6_prefix = ipv6_addr_bitand(nexthop,
> + &mask);
> + if (ipv6_addr_equals(&lrp_ipv6_prefix, &neihgbor_ipv6_prefix)) {
> + has_neighbor_in_same_tw = true;
> + break;
Same here.
> + }
> + }
> + }
> +
> + return has_neighbor_in_same_tw;
And here return false.
However, we already have the extract_lrp_networks() function in
ovn-util.c. Please use that one instead of manually parsing
NB.Logical_Router_Port networks.
> +}
> +
> 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 +1396,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;
> }
>
> @@ -1467,7 +1523,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 +1536,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);
> +
> if (!uuid_equals(&ext_id, &isb_route->header_.uuid)) {
> char *uuid_s =
> xasprintf(UUID_FMT,
> @@ -1785,6 +1842,58 @@ delete_orphan_ic_routes(struct ic_context *ctx,
> isb_route->nexthop, isb_route->origin,
> isb_route->route_table, isb_route->transit_switch);
> icsbrec_route_delete(isb_route);
> + continue;
> + }
> +
> + 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) {
> + bool found_lrp_in_ts = false;
> + struct in6_addr lrp_address, nexthop;
> + unsigned int prefix_len;
> + if (!ip46_parse(isb_route->nexthop, &nexthop)) {
> + continue;
> + }
> +
> + for (size_t i = 0; i < ls->n_ports; i++) {
> + char * lsp_name = ls->ports[i]->name;
Nit: should be "char *lsp_name".
> + const char *lrp_name = get_lrp_name_by_ts_port_name(ctx,
> +
> lsp_name);
> + if (lrp_name) {
> + lrp = get_lrp_by_lrp_name(ctx, lrp_name);
> + size_t n_networks = 0;
> + if (lrp) {
> + n_networks = lrp->n_networks;
> + }
> + for (size_t n_addr = 0; n_addr < n_networks; n_addr++) {
> + char *network = lrp->networks[n_addr];
> + if (!network) {
> + continue;
> + }
> +
> + if (!ip46_parse_cidr(network, &lrp_address,
> + &prefix_len)) {
> + continue;
> + }
> +
> + if (ipv6_addr_equals(&lrp_address, &nexthop)) {
> + found_lrp_in_ts = true;
> + break;
> + }
> + }
> + }
> + }
> +
> + if (!found_lrp_in_ts) {
> + 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, isb_route->nexthop,
> + isb_route->origin, isb_route->route_table,
> + isb_route->transit_switch);
> + icsbrec_route_delete(isb_route);
> + }
Most of this chunk should probably be extracted into a separate
function. It would also allow you to avoid the found_lrp_in_ts variable
and just return early.
Also, I have the same comment about not parsing the LRP networks
manually, please use extract_lrp_networks().
> }
> }
> icsbrec_route_index_destroy_row(isb_route_key);
> diff --git a/tests/ovn-ic.at b/tests/ovn-ic.at
> index 0fd38acb2..acc8c68f5 100644
> --- a/tests/ovn-ic.at
> +++ b/tests/ovn-ic.at
> @@ -3001,3 +3001,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
> +
> +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 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
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
