Skip to content

Commit

Permalink
ic: don't learn routes which have local GW
Browse files Browse the repository at this point in the history
In case we have ovn-ic-interconnected Logical_Routers
and install same ip_prefix route with GW in local AZ
in each LR in each AZ, this route would be learned in
other AZs and L3 loop is possible.
There could be next routes output:

[az1 ~]$ ovn-nbctl lr-route-list lr0
IPv4 Routes
Route Table global:
              128.0.0.0/1               169.254.1.1 dst-ip ecmp
              128.0.0.0/1             169.254.100.2 dst-ip (learned) ecmp

[az2 ~]$ ovn-nbctl lr-route-list lr0
IPv4 Routes
Route Table global:
              128.0.0.0/1               169.254.2.1 dst-ip ecmp
              128.0.0.0/1             169.254.100.1 dst-ip (learned) ecmp

So, there is a possible routing loop. Packets going
to 128.0.0.0/1 could go from AZ1 to AZ2 and on AZ2
they can be routed back.

This commit adds check for installed local (non-learned)
routes. If OVN IC route's ip_prefix, route_table are
the same with already installed non-learned NB route, such
route wouldn't be learned.

Signed-off-by: Vladislav Odintsov <odivlad@gmail.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
  • Loading branch information
odivlad authored and numansiddique committed Nov 22, 2021
1 parent cf444ed commit 68105f6
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 3 deletions.
30 changes: 28 additions & 2 deletions ic/ovn-ic.c
Expand Up @@ -1209,7 +1209,25 @@ add_network_to_routes_ad(struct hmap *routes_ad, const char *network,
}

static bool
route_need_learn(struct in6_addr *prefix, unsigned int plen,
route_has_local_gw(const struct nbrec_logical_router *lr,
const char *route_table, const char *ip_prefix) {

const struct nbrec_logical_router_static_route *route;
for (int i = 0; i < lr->n_static_routes; i++) {
route = lr->static_routes[i];
if (!smap_get(&route->external_ids, "ic-learned-route") &&
!strcmp(route->route_table, route_table) &&
!strcmp(route->ip_prefix, ip_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)
{
if (!smap_get_bool(nb_options, "ic-route-learn", false)) {
Expand All @@ -1229,6 +1247,12 @@ route_need_learn(struct in6_addr *prefix, unsigned int plen,
return false;
}

if (route_has_local_gw(lr, isb_route->route_table, isb_route->ip_prefix)) {
VLOG_DBG("Skip learning %s (rtb:%s) route, as we've got one with "
"local GW", isb_route->ip_prefix, isb_route->route_table);
return false;
}

return true;
}

Expand Down Expand Up @@ -1333,9 +1357,11 @@ sync_learned_routes(struct ic_context *ctx,
isb_route->nexthop);
continue;
}
if (!route_need_learn(&prefix, plen, &nb_global->options)) {
if (!route_need_learn(ic_lr->lr, isb_route, &prefix, plen,
&nb_global->options)) {
continue;
}

struct ic_route_info *route_learned
= ic_route_find(&ic_lr->routes_learned, &prefix, plen,
&nexthop, isb_route->origin,
Expand Down
49 changes: 49 additions & 0 deletions tests/ovn-ic.at
Expand Up @@ -928,3 +928,52 @@ OVN_CLEANUP_IC([az1], [az2])

AT_CLEANUP
])

OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- same routes destination])

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
ovn-nbctl set nb_global . options:ic-route-learn=true
ovn-nbctl set nb_global . options:ic-route-learn-default=true
# Enable route advertising at AZ level
ovn-nbctl set nb_global . options:ic-route-adv=true
ovn-nbctl set nb_global . options:ic-route-adv-default=true

lr=lr1$i
ovn-nbctl lr-add $lr

lrp=lrp-$lr-ts1
lsp=lsp-ts1-$lr
# Create LRP and connect to TS
ovn-nbctl lrp-add $lr $lrp aa:aa:aa:aa:aa:0$i 169.254.100.$i/24
ovn-nbctl lsp-add ts1 $lsp \
-- lsp-set-addresses $lsp router \
-- lsp-set-type $lsp router \
-- lsp-set-options $lsp router-port=$lrp
ovn-nbctl lrp-add $lr lrp-local-subnet 00:00:00:00:00:0$i 192.168.$i.1/24
ovn-nbctl list logical-router-static-route
check ovn-nbctl lr-route-add $lr 10.0.0.0/24 192.168.$i.10
check ovn-nbctl lr-route-add $lr 0.0.0.0/0 192.168.$i.11
done

AT_CHECK([ovn_as az1 ovn-nbctl lr-route-list lr11 | grep dst-ip | sort], [0], [dnl
0.0.0.0/0 192.168.1.11 dst-ip
10.0.0.0/24 192.168.1.10 dst-ip
192.168.2.0/24 169.254.100.2 dst-ip (learned)
])

AT_CHECK([ovn_as az2 ovn-nbctl lr-route-list lr12 | grep dst-ip | sort], [0], [dnl
0.0.0.0/0 192.168.2.11 dst-ip
10.0.0.0/24 192.168.2.10 dst-ip
192.168.1.0/24 169.254.100.1 dst-ip (learned)
])

AT_CLEANUP
])
4 changes: 3 additions & 1 deletion utilities/ovn-nbctl.c
Expand Up @@ -4104,6 +4104,8 @@ nbctl_pre_lr_route_add(struct ctl_context *ctx)
&nbrec_logical_router_static_route_col_options);
ovsdb_idl_add_column(ctx->idl,
&nbrec_logical_router_static_route_col_route_table);
ovsdb_idl_add_column(ctx->idl,
&nbrec_logical_router_static_route_col_external_ids);
}

static char * OVS_WARN_UNUSED_RESULT
Expand Down Expand Up @@ -4233,7 +4235,7 @@ nbctl_lr_route_add(struct ctl_context *ctx)
}

if (!ecmp) {
if (route) {
if (route && !smap_get(&route->external_ids, "ic-learned-route")) {
if (!may_exist) {
ctl_error(ctx, "duplicate prefix: %s (policy: %s). Use option"
" --ecmp to allow this for ECMP routing.",
Expand Down

0 comments on commit 68105f6

Please sign in to comment.