Skip to content

Commit

Permalink
northd: do not configure ECMP routes with wrong next-hop
Browse files Browse the repository at this point in the history 
Check if the nexthop is reachable using router interfaces configuring
ECMP routes. DDlog northd implementation is already checking the
condition above.

https://bugzilla.redhat.com/show_bug.cgi?id=1978796

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
Signed-off-by: Numan Siddique <numans@ovn.org>
  • Loading branch information
@LorenzoBianconi @numansiddique
LorenzoBianconi authored and numansiddique committed Aug 5, 2021
1 parent 9ad348c commit 9cd6478
Show file tree
Hide file tree
Showing 2 changed files with 89 additions and 18 deletions.
28 changes: 23 additions & 5 deletions northd/ovn-northd.c
View file
Expand Up @@ -8337,10 +8337,16 @@ route_hash(struct parsed_route *route)

static struct ovs_mutex bfd_lock = OVS_MUTEX_INITIALIZER;

static bool
find_static_route_outport(struct ovn_datapath *od, struct hmap *ports,
const struct nbrec_logical_router_static_route *route, bool is_ipv4,
const char **p_lrp_addr_s, struct ovn_port **p_out_port);

/* Parse and validate the route. Return the parsed route if successful.
* Otherwise return NULL. */
static struct parsed_route *
parsed_routes_add(struct ovs_list *routes,
parsed_routes_add(struct ovn_datapath *od, struct hmap *ports,
struct ovs_list *routes,
const struct nbrec_logical_router_static_route *route,
struct hmap *bfd_connections)
{
Expand Down Expand Up @@ -8388,6 +8394,14 @@ parsed_routes_add(struct ovs_list *routes,
}
}

/* Verify that ip_prefix and nexthop are on the same network. */
if (!is_discard_route &&
!find_static_route_outport(od, ports, route,
IN6_IS_ADDR_V4MAPPED(&prefix),
NULL, NULL)) {
return NULL;
}

const struct nbrec_bfd *nb_bt = route->bfd;
if (nb_bt && !strcmp(nb_bt->dst_ip, route->nexthop)) {
struct bfd_entry *bfd_e;
Expand Down Expand Up @@ -8662,8 +8676,12 @@ find_static_route_outport(struct ovn_datapath *od, struct hmap *ports,
route->ip_prefix, route->nexthop);
return false;
}
*p_out_port = out_port;
*p_lrp_addr_s = lrp_addr_s;
if (p_out_port) {
*p_out_port = out_port;
}
if (p_lrp_addr_s) {
*p_lrp_addr_s = lrp_addr_s;
}

return true;
}
Expand Down Expand Up @@ -10367,8 +10385,8 @@ build_static_route_flows_for_lrouter(
struct ecmp_groups_node *group;
for (int i = 0; i < od->nbr->n_static_routes; i++) {
struct parsed_route *route =
parsed_routes_add(&parsed_routes, od->nbr->static_routes[i],
bfd_connections);
parsed_routes_add(od, ports, &parsed_routes,
od->nbr->static_routes[i], bfd_connections);
if (!route) {
continue;
}
Expand Down
79 changes: 66 additions & 13 deletions tests/ovn-northd.at
View file
Expand Up @@ -3015,16 +3015,16 @@ for i in $(seq 1 5); do
check ovn-nbctl --wait=sb lsp-set-addresses sw$i-r0 00:00:00:00:00:0$i
done

uuid=$(ovn-nbctl create bfd logical_port=r0-sw1 dst_ip=192.168.10.2 status=down min_tx=250 min_rx=250 detect_mult=10)
ovn-nbctl create bfd logical_port=r0-sw2 dst_ip=192.168.20.2 status=down min_tx=500 min_rx=500 detect_mult=20
ovn-nbctl create bfd logical_port=r0-sw3 dst_ip=192.168.30.2 status=down
ovn-nbctl create bfd logical_port=r0-sw4 dst_ip=192.168.40.2 status=down min_tx=0 detect_mult=0
uuid=$(ovn-nbctl create bfd logical_port=r0-sw1 dst_ip=192.168.1.2 status=down min_tx=250 min_rx=250 detect_mult=10)
ovn-nbctl create bfd logical_port=r0-sw2 dst_ip=192.168.2.2 status=down min_tx=500 min_rx=500 detect_mult=20
ovn-nbctl create bfd logical_port=r0-sw3 dst_ip=192.168.3.2 status=down
ovn-nbctl create bfd logical_port=r0-sw4 dst_ip=192.168.4.2 status=down min_tx=0 detect_mult=0

wait_row_count bfd 1 logical_port=r0-sw1 detect_mult=10 dst_ip=192.168.10.2 \
wait_row_count bfd 1 logical_port=r0-sw1 detect_mult=10 dst_ip=192.168.1.2 \
min_rx=250 min_tx=250 status=admin_down
wait_row_count bfd 1 logical_port=r0-sw2 detect_mult=20 dst_ip=192.168.20.2 \
wait_row_count bfd 1 logical_port=r0-sw2 detect_mult=20 dst_ip=192.168.2.2 \
min_rx=500 min_tx=500 status=admin_down
wait_row_count bfd 1 logical_port=r0-sw3 detect_mult=5 dst_ip=192.168.30.2 \
wait_row_count bfd 1 logical_port=r0-sw3 detect_mult=5 dst_ip=192.168.3.2 \
min_rx=1000 min_tx=1000 status=admin_down

uuid=$(fetch_column nb:bfd _uuid logical_port=r0-sw1)
Expand All @@ -3035,17 +3035,17 @@ check ovn-nbctl clear bfd $uuid_2 min_rx
wait_row_count bfd 1 logical_port=r0-sw2 min_rx=1000
wait_row_count bfd 1 logical_port=r0-sw1 min_rx=1000 min_tx=1000 detect_mult=100

check ovn-nbctl --bfd=$uuid lr-route-add r0 100.0.0.0/8 192.168.10.2
check ovn-nbctl --bfd=$uuid lr-route-add r0 100.0.0.0/8 192.168.1.2
wait_column down bfd status logical_port=r0-sw1
AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.10.2 | grep -q bfd],[0])
AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.1.2 | grep -q bfd],[0])

check ovn-nbctl --bfd lr-route-add r0 200.0.0.0/8 192.168.20.2
check ovn-nbctl --bfd lr-route-add r0 200.0.0.0/8 192.168.2.2
wait_column down bfd status logical_port=r0-sw2
AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.20.2 | grep -q bfd],[0])
AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.2.2 | grep -q bfd],[0])

check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.50.2 r0-sw5
check ovn-nbctl --bfd lr-route-add r0 240.0.0.0/8 192.168.5.2 r0-sw5
wait_column down bfd status logical_port=r0-sw5
AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.50.2 | grep -q bfd],[0])
AT_CHECK([ovn-nbctl lr-route-list r0 | grep 192.168.5.2 | grep -q bfd],[0])

route_uuid=$(fetch_column nb:logical_router_static_route _uuid ip_prefix="100.0.0.0/8")
check ovn-nbctl clear logical_router_static_route $route_uuid bfd
Expand Down Expand Up @@ -4976,3 +4976,56 @@ AT_CHECK([grep -e "chk_pkt_len" -e "lr_in_larger_pkts" lr0flows | sort], [0], [d

AT_CLEANUP
])

OVN_FOR_EACH_NORTHD([
AT_SETUP([ovn -- ecmp routes flows])
AT_KEYWORDS([ecmp-routes-flows])
ovn_start

check ovn-sbctl chassis-add ch1 geneve 127.0.0.1

check ovn-nbctl lr-add lr0
check ovn-nbctl ls-add public
check ovn-nbctl lrp-add lr0 lr0-public 00:00:20:20:12:13 192.168.0.1/24
check ovn-nbctl lsp-add public public-lr0
check ovn-nbctl lsp-set-type public-lr0 router
check ovn-nbctl lsp-set-addresses public-lr0 router
check ovn-nbctl lsp-set-options public-lr0 router-port=lr0-public

check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 192.168.0.10

ovn-sbctl dump-flows lr0 > lr0flows

AT_CHECK([grep -e "lr_in_ip_routing.*select" lr0flows |sort], [0], [dnl
])
AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows |sort], [0], [dnl
table=11(lr_in_ip_routing_ecmp), priority=150 , match=(reg8[[0..15]] == 0), action=(next;)
])

check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 192.168.0.20

ovn-sbctl dump-flows lr0 > lr0flows
AT_CHECK([grep -e "lr_in_ip_routing.*select" lr0flows |sort], [0], [dnl
table=10(lr_in_ip_routing ), priority=65 , match=(ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1; reg8[[16..31]] = select(1, 2);)
])
AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed 's/192\.168\.0\..0/192.168.0.??/' |sort], [0], [dnl
table=11(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1 && reg8[[16..31]] == 1), action=(reg0 = 192.168.0.??; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;)
table=11(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1 && reg8[[16..31]] == 2), action=(reg0 = 192.168.0.??; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;)
table=11(lr_in_ip_routing_ecmp), priority=150 , match=(reg8[[0..15]] == 0), action=(next;)
])

# add ecmp route with wrong nexthop
check ovn-nbctl --wait=sb --ecmp-symmetric-reply lr-route-add lr0 1.0.0.1 192.168.1.20

ovn-sbctl dump-flows lr0 > lr0flows
AT_CHECK([grep -e "lr_in_ip_routing.*select" lr0flows |sort], [0], [dnl
table=10(lr_in_ip_routing ), priority=65 , match=(ip4.dst == 1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1; reg8[[16..31]] = select(1, 2);)
])
AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed 's/192\.168\.0\..0/192.168.0.??/' |sort], [0], [dnl
table=11(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1 && reg8[[16..31]] == 1), action=(reg0 = 192.168.0.??; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;)
table=11(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1 && reg8[[16..31]] == 2), action=(reg0 = 192.168.0.??; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;)
table=11(lr_in_ip_routing_ecmp), priority=150 , match=(reg8[[0..15]] == 0), action=(next;)
])

AT_CLEANUP
])

0 comments on commit 9cd6478

Please sign in to comment.