Skip to content

Commit

Permalink
ic: prevent advertising/learning multiple same routes
Browse files Browse the repository at this point in the history 
Advertize one route (ip_prefix, nexthop, route_table, transit_switch,
availability_zone) only once.

Acked-by: Numan Siddique <numans@ovn.org>
Signed-off-by: Vladislav Odintsov <odivlad@gmail.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
(cherry picked from commit ac1dc2b)
  • Loading branch information
@odivlad @dceara
odivlad authored and dceara committed Dec 16, 2022
1 parent 2a92afa commit 5cefb15
Show file tree
Hide file tree
Showing 2 changed files with 111 additions and 33 deletions.
84 changes: 51 additions & 33 deletions ic/ovn-ic.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -888,10 +888,12 @@ ic_route_hash(const struct in6_addr *prefix, unsigned int plen,
static struct ic_route_info *
ic_route_find(struct hmap *routes, const struct in6_addr *prefix,
unsigned int plen, const struct in6_addr *nexthop,
const char *origin, char *route_table)
const char *origin, const char *route_table, uint32_t hash)
{
struct ic_route_info *r;
uint32_t hash = ic_route_hash(prefix, plen, nexthop, origin, route_table);
if (!hash) {
hash = ic_route_hash(prefix, plen, nexthop, origin, route_table);
}
HMAP_FOR_EACH_WITH_HASH (r, node, hash, routes) {
if (ipv6_addr_equals(&r->prefix, prefix) &&
r->plen == plen &&
Expand Down Expand Up @@ -949,8 +951,8 @@ add_to_routes_learned(struct hmap *routes_learned,
}
const char *origin = smap_get_def(&nb_route->options, "origin", "");
if (ic_route_find(routes_learned, &prefix, plen, &nexthop, origin,
nb_route->route_table)) {
/* Route is already added to learned in previous iteration. */
nb_route->route_table, 0)) {
/* Route was added to learned on previous iteration. */
return true;
}

Expand Down Expand Up @@ -1097,10 +1099,43 @@ route_need_advertise(const char *policy,
}

static void
add_to_routes_ad(struct hmap *routes_ad,
const struct nbrec_logical_router_static_route *nb_route,
const struct lport_addresses *nexthop_addresses,
const struct smap *nb_options, const char *route_table)
add_to_routes_ad(struct hmap *routes_ad, const struct in6_addr prefix,
unsigned int plen, const struct in6_addr nexthop,
const char *origin, const char *route_table,
const struct nbrec_logical_router_port *nb_lrp,
const struct nbrec_logical_router_static_route *nb_route)
{
if (route_table == NULL) {
route_table = "";
}

uint hash = ic_route_hash(&prefix, plen, &nexthop, origin, route_table);

if (!ic_route_find(routes_ad, &prefix, plen, &nexthop, origin, route_table,
hash)) {
struct ic_route_info *ic_route = xzalloc(sizeof *ic_route);
ic_route->prefix = prefix;
ic_route->plen = plen;
ic_route->nexthop = nexthop;
ic_route->nb_route = nb_route;
ic_route->origin = origin;
ic_route->route_table = route_table;
ic_route->nb_lrp = nb_lrp;
hmap_insert(routes_ad, &ic_route->node, hash);
} else {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
VLOG_WARN_RL(&rl, "Duplicate route advertisement was suppressed! NB "
"route uuid: "UUID_FMT,
UUID_ARGS(&nb_route->header_.uuid));
}
}

static void
add_static_to_routes_ad(
struct hmap *routes_ad,
const struct nbrec_logical_router_static_route *nb_route,
const struct lport_addresses *nexthop_addresses,
const struct smap *nb_options, const char *route_table)
{
if (strcmp(route_table, nb_route->route_table)) {
if (VLOG_IS_DBG_ENABLED()) {
Expand Down Expand Up @@ -1149,16 +1184,8 @@ add_to_routes_ad(struct hmap *routes_ad,
ds_destroy(&msg);
}

struct ic_route_info *ic_route = xzalloc(sizeof *ic_route);
ic_route->prefix = prefix;
ic_route->plen = plen;
ic_route->nexthop = nexthop;
ic_route->nb_route = nb_route;
ic_route->origin = ROUTE_ORIGIN_STATIC;
ic_route->route_table = nb_route->route_table;
hmap_insert(routes_ad, &ic_route->node,
ic_route_hash(&prefix, plen, &nexthop, ROUTE_ORIGIN_STATIC,
nb_route->route_table));
add_to_routes_ad(routes_ad, prefix, plen, nexthop, ROUTE_ORIGIN_STATIC,
nb_route->route_table, NULL, nb_route);
}

static void
Expand Down Expand Up @@ -1202,18 +1229,9 @@ add_network_to_routes_ad(struct hmap *routes_ad, const char *network,
ds_destroy(&msg);
}

struct ic_route_info *ic_route = xzalloc(sizeof *ic_route);
ic_route->prefix = prefix;
ic_route->plen = plen;
ic_route->nexthop = nexthop;
ic_route->nb_lrp = nb_lrp;
ic_route->origin = ROUTE_ORIGIN_CONNECTED;

/* directly-connected routes go to <main> route table */
ic_route->route_table = NULL;
hmap_insert(routes_ad, &ic_route->node,
ic_route_hash(&prefix, plen, &nexthop,
ROUTE_ORIGIN_CONNECTED, ""));
add_to_routes_ad(routes_ad, prefix, plen, nexthop, ROUTE_ORIGIN_CONNECTED,
NULL, nb_lrp, NULL);
}

static bool
Expand Down Expand Up @@ -1373,7 +1391,7 @@ sync_learned_routes(struct ic_context *ctx,
struct ic_route_info *route_learned
= ic_route_find(&ic_lr->routes_learned, &prefix, plen,
&nexthop, isb_route->origin,
isb_route->route_table);
isb_route->route_table, 0);
if (route_learned) {
/* Sync external-ids */
struct uuid ext_id;
Expand Down Expand Up @@ -1472,7 +1490,7 @@ advertise_routes(struct ic_context *ctx,
}
struct ic_route_info *route_adv =
ic_route_find(routes_ad, &prefix, plen, &nexthop,
isb_route->origin, isb_route->route_table);
isb_route->origin, isb_route->route_table, 0);
if (!route_adv) {
/* Delete the extra route from IC-SB. */
VLOG_DBG("Delete route %s -> %s from IC-SB, which is not found"
Expand Down Expand Up @@ -1554,8 +1572,8 @@ build_ts_routes_to_adv(struct ic_context *ctx,
}
} else {
/* It may be a route to be advertised */
add_to_routes_ad(routes_ad, nb_route, ts_port_addrs,
&nb_global->options, ts_route_table);
add_static_to_routes_ad(routes_ad, nb_route, ts_port_addrs,
&nb_global->options, ts_route_table);
}
}

Expand Down
60 changes: 60 additions & 0 deletions tests/ovn-ic.at
View file
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,66 @@ OVN_CLEANUP_IC
AT_CLEANUP
])

OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- duplicate NB route adv/learn])

ovn_init_ic_db
net_add n1

# 1 GW per AZ
for i in 1 2; do
az=az$i
ovn_start $az
sim_add gw-$az
as gw-$az
check ovs-vsctl add-br br-phys
ovn_az_attach $az n1 br-phys 192.168.1.$i
check ovs-vsctl set open . external-ids:ovn-is-interconn=true
check ovn-nbctl set nb-global . \
options:ic-route-adv=true \
options:ic-route-adv-default=true \
options:ic-route-learn=true \
options:ic-route-learn-default=true
done

ovn_as az1

# create transit switch and connect to LR
check ovn-ic-nbctl ts-add ts1
for i in 1 2; do
ovn_as az$i

check ovn-nbctl lr-add lr1
check ovn-nbctl lrp-add lr1 lrp$i 00:00:00:00:0$i:01 10.0.$i.1/24
check ovn-nbctl lrp-set-gateway-chassis lrp$i gw-az$i

check ovn-nbctl lsp-add ts1 lsp$i -- \
lsp-set-addresses lsp$i router -- \
lsp-set-type lsp$i router -- \
lsp-set-options lsp$i router-port=lrp$i
done

ovn_as az1

ovn-nbctl \
--id=@id create logical-router-static-route ip_prefix=1.1.1.1/32 nexthop=10.0.1.10 -- \
add logical-router lr1 static_routes @id
ovn-nbctl \
--id=@id create logical-router-static-route ip_prefix=1.1.1.1/32 nexthop=10.0.1.10 -- \
add logical-router lr1 static_routes @id

wait_row_count ic-sb:route 1 ip_prefix=1.1.1.1/32

for i in 1 2; do
az=az$i
OVN_CLEANUP_SBOX(gw-$az)
OVN_CLEANUP_AZ([$az])
done

OVN_CLEANUP_IC
AT_CLEANUP
])

OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn-ic -- gateway sync])

Expand Down

0 comments on commit 5cefb15

Please sign in to comment.