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 <[email protected]> --- 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 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
