The configuration of "discard" and ECMP is prohibited by the
ovn-nbctl. However, nothing prevents from this configuration to
happen directly at NB level. In case of this configuration northd
would crash during deref of the route nexthop which is NULL in the
case of "discard" route. To prevent that make sure we drop all
traffic for given ECMP group, at the same time log a warning into
northd logs as this isn't really a valid configuration.
Fixes: 494e59e341b0 ("Static Routes: Add ability to specify "discard" nexthop")
Reported-at: https://github.com/ovn-org/ovn/issues/302
Signed-off-by: Ales Musil <[email protected]>
---
northd/en-group-ecmp-route.c | 11 ++++-
northd/en-group-ecmp-route.h | 1 +
northd/northd.c | 23 ++++++++---
tests/ovn-northd.at | 80 ++++++++++++++++++++++++++++++++++++
4 files changed, 108 insertions(+), 7 deletions(-)
diff --git a/northd/en-group-ecmp-route.c b/northd/en-group-ecmp-route.c
index cd4cdb991..c4c93fd84 100644
--- a/northd/en-group-ecmp-route.c
+++ b/northd/en-group-ecmp-route.c
@@ -206,12 +206,21 @@ static void
ecmp_groups_add_route(struct ecmp_groups_node *group,
const struct parsed_route *route)
{
+ static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
if (group->route_count == UINT16_MAX) {
- static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
VLOG_WARN_RL(&rl, "too many routes in a single ecmp group.");
return;
}
+ if (route->is_discard_route) {
+ group->has_discard_route = true;
+
+ char *prefix = normalize_v46_prefix(&route->prefix, route->plen);
+ VLOG_WARN_RL(&rl, "The ECMP route \"%s\" contains \"discard\" "
+ "route, the whole group will drop traffic.", prefix);
+ free(prefix);
+ }
+
struct ecmp_route_list_node er = (struct ecmp_route_list_node) {
.route = route,
.id = ++group->route_count,
diff --git a/northd/en-group-ecmp-route.h b/northd/en-group-ecmp-route.h
index 784004347..d4a3248d0 100644
--- a/northd/en-group-ecmp-route.h
+++ b/northd/en-group-ecmp-route.h
@@ -34,6 +34,7 @@ struct ecmp_groups_node {
struct in6_addr prefix;
unsigned int plen;
bool is_src_route;
+ bool has_discard_route;
enum route_source source;
uint32_t route_table_id;
uint16_t route_count;
diff --git a/northd/northd.c b/northd/northd.c
index 0da15629e..7c6876dad 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -12622,7 +12622,21 @@ build_ecmp_route_flow(struct lflow_table *lflows,
struct sset visited_ports = SSET_INITIALIZER(&visited_ports);
VECTOR_FOR_EACH_PTR (&eg->route_list, er) {
const struct parsed_route *route = er->route;
- bool is_ipv4_nexthop = IN6_IS_ADDR_V4MAPPED(route->nexthop);
+
+ ds_clear(&match);
+ ds_put_format(&match, REG_ECMP_GROUP_ID" == %"PRIu16" && "
+ REG_ECMP_MEMBER_ID" == %"PRIu16,
+ eg->id, er->id);
+ ds_clear(&actions);
+
+ if (eg->has_discard_route) {
+ ds_put_cstr(&actions, debug_drop_action());
+ ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, 100,
+ ds_cstr(&match), ds_cstr(&actions), lflow_ref,
+ WITH_HINT(route->source_hint));
+ continue;
+ }
+
/* Symmetric ECMP reply is only usable on gateway routers.
* It is NOT usable on distributed routers with a gateway port.
*/
@@ -12634,11 +12648,8 @@ build_ecmp_route_flow(struct lflow_table *lflows,
route, &route_match,
lflow_ref);
}
- ds_clear(&match);
- ds_put_format(&match, REG_ECMP_GROUP_ID" == %"PRIu16" && "
- REG_ECMP_MEMBER_ID" == %"PRIu16,
- eg->id, er->id);
- ds_clear(&actions);
+
+ bool is_ipv4_nexthop = IN6_IS_ADDR_V4MAPPED(route->nexthop);
ds_put_format(&actions, "%s = ",
is_ipv4_nexthop ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6);
ipv6_format_mapped(route->nexthop, &actions);
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index fd049d096..06808c0dd 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -19939,3 +19939,83 @@ done
OVN_CLEANUP_NORTHD
AT_CLEANUP
+
+OVN_FOR_EACH_NORTHD_NO_HV([
+AT_SETUP([Static routes - ECMP with discard])
+ovn_start
+
+check ovn-nbctl \
+ -- lr-add lr0 \
+ -- lrp-add lr0 lr0-sw1 00:00:00:00:10:01 10.0.10.1/24 \
+ -- --ecmp lr-route-add lr0 192.168.10.0/24 10.0.10.10 \
+ -- lrp-add lr0 lr0-sw2 00:00:00:00:20:01 10.0.20.1/24 \
+ -- --ecmp lr-route-add lr0 192.168.10.0/24 10.0.20.10
+
+check ovn-nbctl --wait=sb sync
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CHECK([grep 'lr_in_ip_routing' lr0flows | grep 'select' |
ovn_strip_lflows], [0], [dnl
+ table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 && ip4.dst
== 192.168.10.0/24), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1;
reg8[[16..31]] = select(1, 2);)
+])
+
+AT_CHECK([grep 'lr_in_ip_routing_ecmp' lr0flows | sed -E 's/== [[1-9]]+\)/==
\?)/' | ovn_strip_lflows], [0], [dnl
+ table=??(lr_in_ip_routing_ecmp), priority=0 , match=(1), action=(drop;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == ?), action=(reg0 = 10.0.10.10; reg5 = 10.0.10.1; eth.src =
00:00:00:00:10:01; outport = "lr0-sw1"; reg9[[9]] = 1; next;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == ?), action=(reg0 = 10.0.20.10; reg5 = 10.0.20.1; eth.src =
00:00:00:00:20:01; outport = "lr0-sw2"; reg9[[9]] = 1; next;)
+ table=??(lr_in_ip_routing_ecmp), priority=150 , match=(reg8[[0..15]] == 0),
action=(next;)
+])
+
+ecmp1=$(fetch_column nb:Logical_Router_Static_Route _uuid nexthop="10.0.10.10")
+ecmp2=$(fetch_column nb:Logical_Router_Static_Route _uuid nexthop="10.0.20.10")
+
+# Set one of the ECMP routes as "discard".
+check ovn-nbctl --wait=sb set Logical_Router_Static_Route $ecmp1
nexthop="discard"
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CHECK([grep 'lr_in_ip_routing' lr0flows | grep 'select' |
ovn_strip_lflows], [0], [dnl
+ table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 && ip4.dst
== 192.168.10.0/24), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1;
reg8[[16..31]] = select(1, 2);)
+])
+
+AT_CHECK([grep 'lr_in_ip_routing_ecmp' lr0flows | sed -E 's/== [[1-9]]+\)/==
\?)/' | ovn_strip_lflows], [0], [dnl
+ table=??(lr_in_ip_routing_ecmp), priority=0 , match=(1), action=(drop;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == ?), action=(drop;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == ?), action=(drop;)
+ table=??(lr_in_ip_routing_ecmp), priority=150 , match=(reg8[[0..15]] == 0),
action=(next;)
+])
+
+# Set the second ECMP route as "discard".
+check ovn-nbctl --wait=sb set Logical_Router_Static_Route $ecmp2
nexthop="discard"
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CHECK([grep 'lr_in_ip_routing' lr0flows | grep 'select' |
ovn_strip_lflows], [0], [dnl
+ table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 && ip4.dst
== 192.168.10.0/24), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1;
reg8[[16..31]] = select(1, 2);)
+])
+
+AT_CHECK([grep 'lr_in_ip_routing_ecmp' lr0flows | sed -E 's/== [[1-9]]+\)/==
\?)/' | ovn_strip_lflows], [0], [dnl
+ table=??(lr_in_ip_routing_ecmp), priority=0 , match=(1), action=(drop;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == ?), action=(drop;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == ?), action=(drop;)
+ table=??(lr_in_ip_routing_ecmp), priority=150 , match=(reg8[[0..15]] == 0),
action=(next;)
+])
+
+# Change both ECMP routes back to valid IP.
+check ovn-nbctl set Logical_Router_Static_Route $ecmp1 nexthop="10.0.10.10"
+check ovn-nbctl --wait=sb set Logical_Router_Static_Route $ecmp2
nexthop="10.0.20.10"
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CHECK([grep 'lr_in_ip_routing' lr0flows | grep 'select' |
ovn_strip_lflows], [0], [dnl
+ table=??(lr_in_ip_routing ), priority=196 , match=(reg7 == 0 && ip4.dst
== 192.168.10.0/24), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1;
reg8[[16..31]] = select(1, 2);)
+])
+
+AT_CHECK([grep 'lr_in_ip_routing_ecmp' lr0flows | sed -E 's/== [[1-9]]+\)/==
\?)/' | ovn_strip_lflows], [0], [dnl
+ table=??(lr_in_ip_routing_ecmp), priority=0 , match=(1), action=(drop;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == ?), action=(reg0 = 10.0.10.10; reg5 = 10.0.10.1; eth.src =
00:00:00:00:10:01; outport = "lr0-sw1"; reg9[[9]] = 1; next;)
+ table=??(lr_in_ip_routing_ecmp), priority=100 , match=(reg8[[0..15]] == 1
&& reg8[[16..31]] == ?), action=(reg0 = 10.0.20.10; reg5 = 10.0.20.1; eth.src =
00:00:00:00:20:01; outport = "lr0-sw2"; reg9[[9]] = 1; next;)
+ table=??(lr_in_ip_routing_ecmp), priority=150 , match=(reg8[[0..15]] == 0),
action=(next;)
+])
+
+check grep -q 'The ECMP route "192.168.10.0/24" contains "discard" route, the
whole group will drop traffic' northd/ovn-northd.log
+
+OVN_CLEANUP_NORTHD
+AT_CLEANUP
+])
--
2.53.0
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
