[ovs-dev] [PATCH ovn 2/2] northd: allow to configure routes with no nexthop
Lorenzo Bianconi
lorenzo.bianconi at redhat.com
Fri Aug 6 12:58:36 UTC 2021
Allow ovn-nbctl to add a valid route without a nexthop. E.g:
ovn-nbctl lr-route-add lr0 192.168.0.0/24 lr0-sw
https://bugzilla.redhat.com/show_bug.cgi?id=1982551
Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi at redhat.com>
---
northd/lrouter.dl | 60 +++++++++++++++++++++++++++++++++++++++++++
northd/ovn-northd.c | 7 ++---
northd/ovn_northd.dl | 5 ++++
tests/ovn-nbctl.at | 9 +++++++
tests/ovn-northd.at | 21 +++++++++++++--
utilities/ovn-nbctl.c | 30 ++++++++++++++++------
6 files changed, 119 insertions(+), 13 deletions(-)
diff --git a/northd/lrouter.dl b/northd/lrouter.dl
index 4a24f3f61..1713c8783 100644
--- a/northd/lrouter.dl
+++ b/northd/lrouter.dl
@@ -717,6 +717,17 @@ relation &StaticRoute(lrsr: nb::Logical_Router_Static_Route,
},
var esr = lrsr.options.get_bool_def("ecmp_symmetric_reply", false).
+relation &StaticRouteEmptyNextHop(lrsr: nb::Logical_Router_Static_Route,
+ key: route_key,
+ output_port: Option<string>)
+&StaticRouteEmptyNextHop(.lrsr = lrsr,
+ .key = RouteKey{policy, ip_prefix, plen},
+ .output_port = lrsr.output_port) :-
+ lrsr in nb::Logical_Router_Static_Route(.nexthop = ""),
+ not StaticRouteDown(lrsr._uuid),
+ var policy = route_policy_from_string(lrsr.policy),
+ Some{(var ip_prefix, var plen)} = ip46_parse_cidr(lrsr.ip_prefix).
+
/* Returns the IP address of the router port 'op' that
* overlaps with 'ip'. If one is not found, returns None. */
function find_lrp_member_ip(networks: lport_addresses, ip: v46_ip): Option<v46_ip> =
@@ -768,6 +779,19 @@ RouterStaticRoute_(.router = router,
var route_id = FlatMap(routes),
route in &StaticRoute(.lrsr = nb::Logical_Router_Static_Route{._uuid = route_id}).
+relation RouterStaticRouteEmptyNextHop_(
+ router : Intern<Router>,
+ key : route_key,
+ output_port : Option<string>)
+
+RouterStaticRouteEmptyNextHop_(.router = router,
+ .key = route.key,
+ .output_port = route.output_port) :-
+ router in &Router(),
+ nb::Logical_Router(._uuid = router._uuid, .static_routes = routes),
+ var route_id = FlatMap(routes),
+ route in &StaticRouteEmptyNextHop(.lrsr = nb::Logical_Router_Static_Route{._uuid = route_id}).
+
/* Step-2: compute output_port for each pair */
typedef route_dst = RouteDst {
nexthop: v46_ip,
@@ -830,6 +854,42 @@ RouterStaticRoute(router, key, dsts) :-
},
var dsts = set_singleton(RouteDst{nexthop, src_ip, port, ecmp_symmetric_reply}).
+relation RouterStaticRouteEmptyNextHop(
+ router : Intern<Router>,
+ key : route_key,
+ dsts : Set<route_dst>)
+
+RouterStaticRouteEmptyNextHop(router, key, dsts) :-
+ RouterStaticRouteEmptyNextHop_(.router = router,
+ .key = key,
+ .output_port = Some{oport}),
+ /* output_port specified */
+ port in &RouterPort(.lrp = &nb::Logical_Router_Port{.name = oport},
+ .networks = networks),
+ /* There are no IP networks configured on the router's port via
+ * which 'route->nexthop' is theoretically reachable. But since
+ * 'out_port' has been specified, we honor it by trying to reach
+ * 'route->nexthop' via the first IP address of 'out_port'.
+ * (There are cases, e.g in GCE, where each VM gets a /32 IP
+ * address and the default gateway is still reachable from it.) */
+ Some{var src_ip} = match (key.ip_prefix) {
+ IPv4{_} -> match (networks.ipv4_addrs.nth(0)) {
+ Some{addr} -> Some{IPv4{addr.addr}},
+ None -> {
+ warn("No path for static route ${key.ip_prefix}");
+ None
+ }
+ },
+ IPv6{_} -> match (networks.ipv6_addrs.nth(0)) {
+ Some{addr} -> Some{IPv6{addr.addr}},
+ None -> {
+ warn("No path for static route ${key.ip_prefix}");
+ None
+ }
+ }
+ },
+ var dsts = set_singleton(RouteDst{src_ip, src_ip, port, false}).
+
/* compute route-route pairs for nexthop = "discard" routes */
relation &DiscardRoute(lrsr: nb::Logical_Router_Static_Route,
key: route_key)
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 4c164a744..91a592c71 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -8354,7 +8354,8 @@ parsed_routes_add(struct ovn_datapath *od, struct hmap *ports,
struct in6_addr nexthop;
unsigned int plen;
bool is_discard_route = !strcmp(route->nexthop, "discard");
- if (!is_discard_route) {
+ bool valid_nexthop = strlen(route->nexthop) && !is_discard_route;
+ if (valid_nexthop) {
if (!ip46_parse_cidr(route->nexthop, &nexthop, &plen)) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
VLOG_WARN_RL(&rl, "bad 'nexthop' %s in static route"
@@ -8383,7 +8384,7 @@ parsed_routes_add(struct ovn_datapath *od, struct hmap *ports,
}
/* Verify that ip_prefix and nexthop have same address familiy. */
- if (!is_discard_route) {
+ if (valid_nexthop) {
if (IN6_IS_ADDR_V4MAPPED(&prefix) != IN6_IS_ADDR_V4MAPPED(&nexthop)) {
static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
VLOG_WARN_RL(&rl, "Address family doesn't match between 'ip_prefix'"
@@ -8879,7 +8880,7 @@ add_route(struct hmap *lflows, struct ovn_datapath *od,
} else {
ds_put_format(&common_actions, REG_ECMP_GROUP_ID" = 0; %s = ",
is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6);
- if (gateway) {
+ if (gateway && strlen(gateway)) {
ds_put_cstr(&common_actions, gateway);
} else {
ds_put_format(&common_actions, "ip%s.dst", is_ipv4 ? "4" : "6");
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index 091fe10b3..79cb53497 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -6809,6 +6809,11 @@ Route(key, dst.port, dst.src_ip, Some{dst.nexthop}) :-
dsts.size() == 1,
Some{var dst} = dsts.nth(0).
+Route(key, dst.port, dst.src_ip, None) :-
+ RouterStaticRouteEmptyNextHop(.router = router, .key = key, .dsts = dsts),
+ dsts.size() == 1,
+ Some{var dst} = dsts.nth(0).
+
/* Create routes from peer to port's routable addresses */
Route(key, peer, src_ip, None) :-
RouterPortRoutableAddresses(port, addresses),
diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
index 5d05be387..f2b263844 100644
--- a/tests/ovn-nbctl.at
+++ b/tests/ovn-nbctl.at
@@ -1451,6 +1451,10 @@ dnl Check IPv4 routes
AT_CHECK([ovn-nbctl lr-route-add lr0 0.0.0.0/0 192.168.0.1])
AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.1.0/24 11.0.1.1 lp0])
AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.0.1/24 11.0.0.2])
+AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.10.0/24 lp0])
+AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.10.0/24 lp1], [1], [],
+ [ovn-nbctl: bad IPv4 nexthop argument: lp1
+])
dnl Add overlapping route with 10.0.0.1/24
AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.0.111/24 11.0.0.1], [1], [],
@@ -1499,6 +1503,7 @@ AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl
IPv4 Routes
10.0.0.0/24 11.0.0.1 dst-ip
10.0.1.0/24 11.0.1.1 dst-ip lp0
+ 10.0.10.0/24 dst-ip lp0
20.0.0.0/24 discard dst-ip
9.16.1.0/24 11.0.0.1 src-ip
10.0.0.0/24 11.0.0.2 src-ip
@@ -1512,6 +1517,7 @@ AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl
IPv4 Routes
10.0.0.0/24 11.0.0.1 dst-ip lp1
10.0.1.0/24 11.0.1.1 dst-ip lp0
+ 10.0.10.0/24 dst-ip lp0
20.0.0.0/24 discard dst-ip
9.16.1.0/24 11.0.0.1 src-ip
10.0.0.0/24 11.0.0.2 src-ip
@@ -1540,6 +1546,7 @@ AT_CHECK([ovn-nbctl --policy=src-ip lr-route-del lr0 9.16.1.0/24])
AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl
IPv4 Routes
10.0.0.0/24 11.0.0.1 dst-ip lp1
+ 10.0.10.0/24 dst-ip lp0
10.0.0.0/24 11.0.0.2 src-ip
0.0.0.0/0 192.168.0.1 dst-ip
])
@@ -1549,6 +1556,7 @@ AT_CHECK([ovn-nbctl --policy=dst-ip lr-route-del lr0 10.0.0.0/24])
AT_CHECK([ovn-nbctl --policy=src-ip lr-route-del lr0 10.0.0.0/24])
AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl
IPv4 Routes
+ 10.0.10.0/24 dst-ip lp0
0.0.0.0/0 192.168.0.1 dst-ip
])
@@ -1558,6 +1566,7 @@ AT_CHECK([ovn-nbctl --policy=src-ip lr-route-add lr0 10.0.0.0/24 11.0.0.2])
AT_CHECK([ovn-nbctl lr-route-del lr0 10.0.0.0/24])
AT_CHECK([ovn-nbctl lr-route-list lr0], [0], [dnl
IPv4 Routes
+ 10.0.10.0/24 dst-ip lp0
0.0.0.0/0 192.168.0.1 dst-ip
])
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index d4d3c9b65..3e5e5f832 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -4978,8 +4978,8 @@ AT_CLEANUP
])
OVN_FOR_EACH_NORTHD([
-AT_SETUP([ovn -- ecmp routes flows])
-AT_KEYWORDS([ecmp-routes-flows])
+AT_SETUP([ovn -- static routes flows])
+AT_KEYWORDS([static-routes-flows])
ovn_start
check ovn-sbctl chassis-add ch1 geneve 127.0.0.1
@@ -5027,5 +5027,22 @@ AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed 's/192\.168\.0\..0/192.
table=11(lr_in_ip_routing_ecmp), priority=150 , match=(reg8[[0..15]] == 0), action=(next;)
])
+check ovn-nbctl lr-route-del lr0
+wait_row_count nb:Logical_Router_Static_Route 0
+
+check ovn-nbctl --wait=sb lr-route-add lr0 1.0.0.0/24 192.168.0.10
+ovn-sbctl dump-flows lr0 > lr0flows
+
+AT_CHECK([grep -e "lr_in_ip_routing.*192.168.0.10" lr0flows |sort], [0], [dnl
+ table=10(lr_in_ip_routing ), priority=49 , match=(ip4.dst == 1.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 192.168.0.10; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; next;)
+])
+
+check ovn-nbctl --wait=sb lr-route-add lr0 2.0.0.0/24 lr0-public
+
+ovn-sbctl dump-flows lr0 > lr0flows
+AT_CHECK([grep -e "lr_in_ip_routing.*2.0.0.0" lr0flows |sort], [0], [dnl
+ table=10(lr_in_ip_routing ), priority=49 , match=(ip4.dst == 2.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 = 192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; flags.loopback = 1; next;)
+])
+
AT_CLEANUP
])
diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
index 972a637ff..024f1662f 100644
--- a/utilities/ovn-nbctl.c
+++ b/utilities/ovn-nbctl.c
@@ -4071,9 +4071,15 @@ nbctl_lr_route_add(struct ctl_context *ctx)
? normalize_ipv6_addr_str(ctx->argv[3])
: normalize_ipv4_addr_str(ctx->argv[3]);
if (!next_hop) {
- ctl_error(ctx, "bad %s nexthop argument: %s",
- v6_prefix ? "IPv6" : "IPv4", ctx->argv[3]);
- goto cleanup;
+ /* check if it is a output port. */
+ error = lrp_by_name_or_uuid(ctx, ctx->argv[3], true, &out_lrp);
+ if (error) {
+ ctl_error(ctx, "bad %s nexthop argument: %s",
+ v6_prefix ? "IPv6" : "IPv4", ctx->argv[3]);
+ free(error);
+ goto cleanup;
+ }
+ next_hop = "";
}
}
@@ -4218,7 +4224,9 @@ nbctl_lr_route_add(struct ctl_context *ctx)
}
cleanup:
- free(next_hop);
+ if (next_hop && strlen(next_hop)) {
+ free(next_hop);
+ }
free(prefix);
}
@@ -5903,12 +5911,18 @@ print_route(const struct nbrec_logical_router_static_route *route,
{
char *prefix = normalize_prefix_str(route->ip_prefix);
- char *next_hop = !strcmp(route->nexthop, "discard")
- ? xasprintf("discard")
- : normalize_prefix_str(route->nexthop);
+ char *next_hop = "";
+
+ if (!strcmp(route->nexthop, "discard")) {
+ next_hop = xasprintf("discard");
+ } else if (strlen(route->nexthop)) {
+ next_hop = normalize_prefix_str(route->nexthop);
+ }
ds_put_format(s, "%25s %25s", prefix, next_hop);
free(prefix);
- free(next_hop);
+ if (strlen(next_hop)) {
+ free(next_hop);
+ }
if (route->policy) {
ds_put_format(s, " %s", route->policy);
--
2.31.1
More information about the dev
mailing list