Message ID | 20210402210858.56673-1-svc.eng.git-mail@nutanix.com |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev,v3] Static Routes: Add ability to specify "discard" nexthop | expand |
Hello Karthik, This all looks good to me. Thanks! Acked-by: Mark Michelson <mmichels@redhat.com> On 4/2/21 5:08 PM, svc.eng.git-mail@nutanix.com wrote: > From: Karthik Chandrashekar <karthik.c@nutanix.com> > > Physical switches have the ability to specify "discard" or sometimes > "NULL interface" as a nexthop for routes. This can be used to prevent > routing loops in the network. Add a similar configuration for ovn > where nexthop accepts the string "discard". When the nexthop is discard > the action in the routing table will be to drop the packets. > > Signed-off-by: Karthik Chandrashekar <karthik.c@nutanix.com> > > ---- > v1 -> v2 > ---- > * Add ddlog support > * Don't allow nexthop = "discard" when ECMP and BFD is set > ---- > v2 -> v3 > ---- > * Use ddlog optimizations from review > --- > northd/lrouter.dl | 19 ++++++ > northd/ovn-northd.c | 126 +++++++++++++++++++++----------------- > northd/ovn_northd.dl | 10 +++ > ovn-nb.xml | 4 +- > tests/ovn-nbctl.at | 25 ++++++++ > tests/ovn.at | 94 ++++++++++++++++++++++++++++ > utilities/ovn-nbctl.8.xml | 3 +- > utilities/ovn-nbctl.c | 67 +++++++++++++++----- > 8 files changed, 276 insertions(+), 72 deletions(-) > > diff --git a/northd/lrouter.dl b/northd/lrouter.dl > index 36cedd2dc..163bcd57c 100644 > --- a/northd/lrouter.dl > +++ b/northd/lrouter.dl > @@ -769,6 +769,25 @@ RouterStaticRoute(router, key, dsts) :- > }, > var dsts = set_singleton(RouteDst{nexthop, src_ip, port, ecmp_symmetric_reply}). > > +/* compute route-route pairs for nexthop = "discard" routes */ > +relation &DiscardRoute(lrsr: nb::Logical_Router_Static_Route, > + key: route_key) > +&DiscardRoute(.lrsr = lrsr, > + .key = RouteKey{policy, ip_prefix, plen}) :- > + lrsr in nb::Logical_Router_Static_Route(.nexthop = "discard"), > + var policy = route_policy_from_string(lrsr.policy), > + Some{(var ip_prefix, var plen)} = ip46_parse_cidr(lrsr.ip_prefix). > + > +relation RouterDiscardRoute_( > + router : Ref<Router>, > + key : route_key) > + > +RouterDiscardRoute_(.router = router, > + .key = route.key) :- > + router in &Router(.lr = nb::Logical_Router{.static_routes = routes}), > + var route_id = FlatMap(routes), > + route in &DiscardRoute(.lrsr = nb::Logical_Router_Static_Route{._uuid = route_id}). > + > Warning[message] :- > RouterStaticRoute_(.router = router, .key = key, .nexthop = nexthop), > not RouterStaticRoute(.router = router, .key = key), > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > index 9839b8c4f..c6d947518 100644 > --- a/northd/ovn-northd.c > +++ b/northd/ovn-northd.c > @@ -7938,6 +7938,7 @@ struct parsed_route { > uint32_t hash; > const struct nbrec_logical_router_static_route *route; > bool ecmp_symmetric_reply; > + bool is_discard_route; > }; > > static uint32_t > @@ -7957,20 +7958,23 @@ parsed_routes_add(struct ovs_list *routes, > /* Verify that the next hop is an IP address with an all-ones mask. */ > struct in6_addr nexthop; > unsigned int plen; > - 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" > - UUID_FMT, route->nexthop, > - UUID_ARGS(&route->header_.uuid)); > - return NULL; > - } > - if ((IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 32) || > - (!IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 128)) { > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > - VLOG_WARN_RL(&rl, "bad next hop mask %s in static route" > - UUID_FMT, route->nexthop, > - UUID_ARGS(&route->header_.uuid)); > - return NULL; > + bool is_discard_route = !strcmp(route->nexthop, "discard"); > + if (!is_discard_route) { > + 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" > + UUID_FMT, route->nexthop, > + UUID_ARGS(&route->header_.uuid)); > + return NULL; > + } > + if ((IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 32) || > + (!IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 128)) { > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > + VLOG_WARN_RL(&rl, "bad next hop mask %s in static route" > + UUID_FMT, route->nexthop, > + UUID_ARGS(&route->header_.uuid)); > + return NULL; > + } > } > > /* Parse ip_prefix */ > @@ -7984,13 +7988,15 @@ parsed_routes_add(struct ovs_list *routes, > } > > /* Verify that ip_prefix and nexthop have same address familiy. */ > - 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' %s" > - " and 'nexthop' %s in static route"UUID_FMT, > - route->ip_prefix, route->nexthop, > - UUID_ARGS(&route->header_.uuid)); > - return NULL; > + if (!is_discard_route) { > + 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'" > + " %s and 'nexthop' %s in static route"UUID_FMT, > + route->ip_prefix, route->nexthop, > + UUID_ARGS(&route->header_.uuid)); > + return NULL; > + } > } > > const struct nbrec_bfd *nb_bt = route->bfd; > @@ -8021,6 +8027,7 @@ parsed_routes_add(struct ovs_list *routes, > pr->route = route; > pr->ecmp_symmetric_reply = smap_get_bool(&route->options, > "ecmp_symmetric_reply", false); > + pr->is_discard_route = is_discard_route; > ovs_list_insert(routes, &pr->list_node); > return pr; > } > @@ -8433,10 +8440,11 @@ build_ecmp_route_flow(struct hmap *lflows, struct ovn_datapath *od, > } > > static void > -add_route(struct hmap *lflows, const struct ovn_port *op, > - const char *lrp_addr_s, const char *network_s, int plen, > - const char *gateway, bool is_src_route, > - const struct ovsdb_idl_row *stage_hint) > +add_route(struct hmap *lflows, struct ovn_datapath *od, > + const struct ovn_port *op, const char *lrp_addr_s, > + const char *network_s, int plen, const char *gateway, > + bool is_src_route, const struct ovsdb_idl_row *stage_hint, > + bool is_discard_route) > { > bool is_ipv4 = strchr(network_s, '.') ? true : false; > struct ds match = DS_EMPTY_INITIALIZER; > @@ -8455,30 +8463,34 @@ add_route(struct hmap *lflows, const struct ovn_port *op, > &match, &priority); > > struct ds common_actions = DS_EMPTY_INITIALIZER; > - ds_put_format(&common_actions, REG_ECMP_GROUP_ID" = 0; %s = ", > - is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6); > - if (gateway) { > - ds_put_cstr(&common_actions, gateway); > - } else { > - ds_put_format(&common_actions, "ip%s.dst", is_ipv4 ? "4" : "6"); > - } > - ds_put_format(&common_actions, "; " > - "%s = %s; " > - "eth.src = %s; " > - "outport = %s; " > - "flags.loopback = 1; " > - "next;", > - is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, > - lrp_addr_s, > - op->lrp_networks.ea_s, > - op->json_key); > struct ds actions = DS_EMPTY_INITIALIZER; > - ds_put_format(&actions, "ip.ttl--; %s", ds_cstr(&common_actions)); > + if (is_discard_route) { > + ds_put_format(&actions, "drop;"); > + } else { > + ds_put_format(&common_actions, REG_ECMP_GROUP_ID" = 0; %s = ", > + is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6); > + if (gateway) { > + ds_put_cstr(&common_actions, gateway); > + } else { > + ds_put_format(&common_actions, "ip%s.dst", is_ipv4 ? "4" : "6"); > + } > + ds_put_format(&common_actions, "; " > + "%s = %s; " > + "eth.src = %s; " > + "outport = %s; " > + "flags.loopback = 1; " > + "next;", > + is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, > + lrp_addr_s, > + op->lrp_networks.ea_s, > + op->json_key); > + ds_put_format(&actions, "ip.ttl--; %s", ds_cstr(&common_actions)); > + } > > - ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_ROUTING, priority, > + ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_ROUTING, priority, > ds_cstr(&match), ds_cstr(&actions), > stage_hint); > - if (op->has_bfd) { > + if (op && op->has_bfd) { > ds_put_format(&match, " && udp.dst == 3784"); > ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_ROUTING, > priority + 1, ds_cstr(&match), > @@ -8500,16 +8512,18 @@ build_static_route_flow(struct hmap *lflows, struct ovn_datapath *od, > const struct nbrec_logical_router_static_route *route = route_->route; > > /* Find the outgoing port. */ > - if (!find_static_route_outport(od, ports, route, > - IN6_IS_ADDR_V4MAPPED(&route_->prefix), > - &lrp_addr_s, &out_port)) { > - return; > + if (!route_->is_discard_route) { > + if (!find_static_route_outport(od, ports, route, > + IN6_IS_ADDR_V4MAPPED(&route_->prefix), > + &lrp_addr_s, &out_port)) { > + return; > + } > } > > char *prefix_s = build_route_prefix_s(&route_->prefix, route_->plen); > - add_route(lflows, out_port, lrp_addr_s, prefix_s, route_->plen, > - route->nexthop, route_->is_src_route, > - &route->header_); > + add_route(lflows, route_->is_discard_route ? od : out_port->od, out_port, > + lrp_addr_s, prefix_s, route_->plen, route->nexthop, > + route_->is_src_route, &route->header_, route_->is_discard_route); > > free(prefix_s); > } > @@ -9735,17 +9749,17 @@ build_ip_routing_flows_for_lrouter_port( > if (op->nbrp) { > > for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { > - add_route(lflows, op, op->lrp_networks.ipv4_addrs[i].addr_s, > + add_route(lflows, op->od, op, op->lrp_networks.ipv4_addrs[i].addr_s, > op->lrp_networks.ipv4_addrs[i].network_s, > op->lrp_networks.ipv4_addrs[i].plen, NULL, false, > - &op->nbrp->header_); > + &op->nbrp->header_, false); > } > > for (int i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { > - add_route(lflows, op, op->lrp_networks.ipv6_addrs[i].addr_s, > + add_route(lflows, op->od, op, op->lrp_networks.ipv6_addrs[i].addr_s, > op->lrp_networks.ipv6_addrs[i].network_s, > op->lrp_networks.ipv6_addrs[i].plen, NULL, false, > - &op->nbrp->header_); > + &op->nbrp->header_, false); > } > } > } > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl > index 0063021e1..3b1e7fa1a 100644 > --- a/northd/ovn_northd.dl > +++ b/northd/ovn_northd.dl > @@ -6370,6 +6370,16 @@ for (Route(.port = port, > } > } > > +/* Install drop routes for all the static routes with nexthop = "discard" */ > +Flow(.logical_datapath = router.lr._uuid, > + .stage = s_ROUTER_IN_IP_ROUTING(), > + .priority = priority as integer, > + .__match = ip_match, > + .actions = "drop;", > + .external_ids = map_empty()) :- > + r in RouterDiscardRoute_(.router = router, .key = key), > + (var ip_match, var priority) = build_route_match(r.key). > + > /* Logical router ingress table IP_ROUTING & IP_ROUTING_ECMP: IP Routing. > * > * A packet that arrives at this table is an IP packet that should be > diff --git a/ovn-nb.xml b/ovn-nb.xml > index b0a4adffe..98543404e 100644 > --- a/ovn-nb.xml > +++ b/ovn-nb.xml > @@ -2649,7 +2649,9 @@ > <column name="nexthop"> > <p> > Nexthop IP address for this route. Nexthop IP address should be the IP > - address of a connected router port or the IP address of a logical port. > + address of a connected router port or the IP address of a logical port > + or can be set to <code>discard</code> for dropping packets which match > + the given route. > </p> > </column> > > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at > index 6d91aa4c5..0392cd968 100644 > --- a/tests/ovn-nbctl.at > +++ b/tests/ovn-nbctl.at > @@ -1467,18 +1467,38 @@ AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.0.111/24 11.0.0.1/24], [1], [], > AT_CHECK([ovn-nbctl lr-route-add lr0 2001:0db8:1::/64 2001:0db8:0:f103::1/64], [1], [], > [ovn-nbctl: bad IPv6 nexthop argument: 2001:0db8:0:f103::1/64 > ]) > +AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 20.0.0.0/24 discard], [1], [], > + [ovn-nbctl: ecmp is not valid for discard routes. > +]) > +AT_CHECK([ovn-nbctl --ecmp-symmetric-reply lr-route-add lr0 20.0.0.0/24 discard], [1], [], > + [ovn-nbctl: ecmp is not valid for discard routes. > +]) > +AT_CHECK([ovn-nbctl --bfd lr-route-add lr0 20.0.0.0/24 discard lp0], [1], [], > + [ovn-nbctl: bfd dst_ip cannot be discard. > +]) > +AT_CHECK([ovn-nbctl lr-route-add lr0 20.0.0.0/24 discard lp0], [1], [], > + [ovn-nbctl: outport is not valid for discard routes. > +]) > > AT_CHECK([ovn-nbctl --may-exist lr-route-add lr0 10.0.0.111/24 11.0.0.1]) > AT_CHECK([ovn-nbctl --policy=src-ip lr-route-add lr0 9.16.1.0/24 11.0.0.1]) > dnl Add a route with existed prefix but different policy (src-ip) > 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-add lr0 20.0.0.0/24 discard]) > +AT_CHECK([ovn-nbctl --policy=src-ip lr-route-add lr0 20.0.0.0/24 discard]) > +AT_CHECK([ovn-nbctl --ecmp --policy=src-ip lr-route-add lr0 20.0.0.0/24 11.0.0.1], [1], [], > + [ovn-nbctl: discard nexthop for the same ECMP route exists. > +]) > + > 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 > + 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 > + 20.0.0.0/24 discard src-ip > 0.0.0.0/0 192.168.0.1 dst-ip > ]) > > @@ -1487,11 +1507,16 @@ 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 > + 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 > + 20.0.0.0/24 discard src-ip > 0.0.0.0/0 192.168.0.1 dst-ip > ]) > > +AT_CHECK([ovn-nbctl --policy=src-ip lr-route-del lr0 20.0.0.0/24]) > +AT_CHECK([ovn-nbctl lr-route-del lr0 20.0.0.0/24 discard]) > + > dnl Delete non-existent prefix > AT_CHECK([ovn-nbctl lr-route-del lr0 10.0.2.1/24], [1], [], > [ovn-nbctl: no matching route: policy 'any', prefix '10.0.2.0/24', nexthop 'any', output_port 'any'. > diff --git a/tests/ovn.at b/tests/ovn.at > index 73cc8bf02..176d08512 100644 > --- a/tests/ovn.at > +++ b/tests/ovn.at > @@ -25856,3 +25856,97 @@ primary lport : [[sw0p2]] > OVN_CLEANUP([hv1], [hv2]) > AT_CLEANUP > ]) > + > +OVN_FOR_EACH_NORTHD([ > +AT_SETUP([ovn -- Static route with discard nexthop]) > +ovn_start > + > +# Logical network: > +# Logical Router lr1 has Logical Switch sw1 (10.0.0.0/24) connected to it. sw1 has one > +# switch port sw1-ls1 (10.0.0.100) > + > +ovn-nbctl lr-add lr1 > + > +ovn-nbctl ls-add sw1 > + > +# Connect sw1 to lr1 > +ovn-nbctl lrp-add lr1 lr1-sw1 00:00:01:01:02:03 10.0.0.1/24 > +ovn-nbctl lsp-add sw1 sw1-lr1 > +ovn-nbctl lsp-set-type sw1-lr1 router > +ovn-nbctl lsp-set-addresses sw1-lr1 router > +ovn-nbctl lsp-set-options sw1-lr1 router-port=lr1-sw1 > + > +# Create logical port sw1-lp1 in sw1 > +ovn-nbctl lsp-add sw1 sw1-lp1 \ > +-- lsp-set-addresses sw1-lp1 "00:00:04:01:02:03 10.0.0.100" > + > +# Create one hypervisor and create OVS ports corresponding to logical ports. > +net_add n1 > + > +sim_add hv1 > +as hv1 > +ovs-vsctl add-br br-phys > +ovn_attach n1 br-phys 192.168.0.1 > + > +ovs-vsctl -- add-port br-int hv1-vif1 -- \ > + set interface hv1-vif1 external-ids:iface-id=sw1-lp1 \ > + options:tx_pcap=hv1/vif1-tx.pcap \ > + options:rxq_pcap=hv1/vif1-rx.pcap \ > + ofport-request=1 > + > +# Install static routes to drop traffic > +ovn-nbctl lr-route-add lr1 20.0.0.0/24 discard > + > +# Allow some time for ovn-controller to catch up. > +ovn-nbctl --wait=hv sync > + > +# Check logical flows for drop rule > +AT_CHECK([ovn-sbctl dump-flows | grep lr_in_ip_routing | grep "20.0.0.0/24" | \ > + grep drop | wc -l], [0], [dnl > +1 > +]) > + > +# Send packet. > +packet="inport==\"sw1-lp1\" && eth.src==00:00:04:01:02:03 && > + eth.dst==00:00:01:01:02:03 && ip4 && ip.ttl==64 && > + ip4.src==10.0.0.100 && ip4.dst==20.0.0.200 && > + udp && udp.src==53 && udp.dst==4369" > + > +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet" > + > +# Check if packet hit the drop rule > +AT_CHECK([ovs-ofctl dump-flows br-int | grep "nw_dst=20.0.0.0/24" | \ > + grep "actions=drop" | grep "n_packets=1" | wc -l], [0], [dnl > +1 > +]) > + > +# Remove destination match and add discard route with source match > +ovn-nbctl lr-route-del lr1 20.0.0.0/24 > +ovn-nbctl --policy="src-ip" lr-route-add lr1 10.0.0.0/24 discard > + > +# Allow some time for ovn-controller to catch up. > +ovn-nbctl --wait=hv sync > + > +# Check logical flows for drop rule > +AT_CHECK([ovn-sbctl dump-flows | grep lr_in_ip_routing | grep "10.0.0.0/24" | \ > + grep drop | wc -l], [0], [dnl > +1 > +]) > + > +# Send packet. > +packet="inport==\"sw1-lp1\" && eth.src==00:00:04:01:02:03 && > + eth.dst==00:00:01:01:02:03 && ip4 && ip.ttl==64 && > + ip4.src==10.0.0.100 && ip4.dst==20.0.0.200 && > + udp && udp.src==53 && udp.dst==4369" > + > +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet" > + > +# Check if packet hit the drop rule > +AT_CHECK([ovs-ofctl dump-flows br-int "nw_src=10.0.0.0/24" | \ > + grep "actions=drop" | grep "n_packets=1" | wc -l], [0], [dnl > +1 > +]) > + > +OVN_CLEANUP([hv1]) > +AT_CLEANUP > +]) > \ No newline at end of file > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml > index 03d47dba5..1ab63d413 100644 > --- a/utilities/ovn-nbctl.8.xml > +++ b/utilities/ovn-nbctl.8.xml > @@ -673,7 +673,8 @@ > logical port. If <var>port</var> is specified, packets that > match this route will be sent out that port. When > <var>port</var> is omitted, OVN infers the output port based > - on <var>nexthop</var>. > + on <var>nexthop</var>. Nexthop can be set to <var>discard</var> > + for dropping packets which match the given route. > </p> > > <p> > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > index 184058356..2cbd324b7 100644 > --- a/utilities/ovn-nbctl.c > +++ b/utilities/ovn-nbctl.c > @@ -3971,6 +3971,7 @@ nbctl_lr_route_add(struct ctl_context *ctx) > > const char *policy = shash_find_data(&ctx->options, "--policy"); > bool is_src_route = false; > + > if (policy) { > if (!strcmp(policy, "src-ip")) { > is_src_route = true; > @@ -3991,13 +3992,18 @@ nbctl_lr_route_add(struct ctl_context *ctx) > return; > } > > - next_hop = v6_prefix > - ? 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; > + bool is_discard_route = !strcmp(ctx->argv[3], "discard"); > + if (is_discard_route) { > + next_hop = xasprintf("discard"); > + } else { > + next_hop = v6_prefix > + ? 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; > + } > } > > struct shash_node *bfd = shash_find(&ctx->options, "--bfd"); > @@ -4030,6 +4036,25 @@ nbctl_lr_route_add(struct ctl_context *ctx) > ecmp_symmetric_reply; > struct nbrec_logical_router_static_route *route = > nbctl_lr_get_route(lr, prefix, next_hop, is_src_route, ecmp); > + > + /* Validations for nexthop = "discard" */ > + if (is_discard_route) { > + if (ecmp) { > + ctl_error(ctx, "ecmp is not valid for discard routes."); > + goto cleanup; > + } > + if (bfd) { > + ctl_error(ctx, "bfd dst_ip cannot be discard."); > + goto cleanup; > + } > + if (ctx->argc == 5) { > + if (is_discard_route) { > + ctl_error(ctx, "outport is not valid for discard routes."); > + goto cleanup; > + } > + } > + } > + > if (!ecmp) { > if (route) { > if (!may_exist) { > @@ -4071,6 +4096,13 @@ nbctl_lr_route_add(struct ctl_context *ctx) > goto cleanup; > } > > + struct nbrec_logical_router_static_route *discard_route = > + nbctl_lr_get_route(lr, prefix, "discard", is_src_route, true); > + if (discard_route) { > + ctl_error(ctx, "discard nexthop for the same ECMP route exists."); > + goto cleanup; > + } > + > route = nbrec_logical_router_static_route_insert(ctx->txn); > nbrec_logical_router_static_route_set_ip_prefix(route, prefix); > nbrec_logical_router_static_route_set_nexthop(route, next_hop); > @@ -4145,10 +4177,14 @@ nbctl_lr_route_del(struct ctl_context *ctx) > > char *nexthop = NULL; > if (ctx->argc >= 4) { > - nexthop = normalize_prefix_str(ctx->argv[3]); > - if (!nexthop) { > - ctl_error(ctx, "bad nexthop argument: %s", ctx->argv[3]); > - return; > + if (!strcmp(ctx->argv[3], "discard")) { > + nexthop = xasprintf("discard"); > + } else { > + nexthop = normalize_prefix_str(ctx->argv[3]); > + if (!nexthop) { > + ctl_error(ctx, "bad nexthop argument: %s", ctx->argv[3]); > + return; > + } > } > } > > @@ -4189,8 +4225,9 @@ nbctl_lr_route_del(struct ctl_context *ctx) > > /* Compare nexthop, if specified. */ > if (nexthop) { > - char *rt_nexthop = > - normalize_prefix_str(lr->static_routes[i]->nexthop); > + char *rt_nexthop = !strcmp(lr->static_routes[i]->nexthop, "discard") > + ? xasprintf("discard") > + : normalize_prefix_str(lr->static_routes[i]->nexthop); > if (!rt_nexthop) { > /* Ignore existing nexthop we couldn't parse. */ > continue; > @@ -5578,7 +5615,9 @@ print_route(const struct nbrec_logical_router_static_route *route, > { > > char *prefix = normalize_prefix_str(route->ip_prefix); > - char *next_hop = normalize_prefix_str(route->nexthop); > + char *next_hop = !strcmp(route->nexthop, "discard") > + ? xasprintf("discard") > + : normalize_prefix_str(route->nexthop); > ds_put_format(s, "%25s %25s", prefix, next_hop); > free(prefix); > free(next_hop); >
Looks like this patch was applied and It unfortunately broke the compilation with ddlog enabled. I get the below error --- ddlog -i ../northd/ovn_northd.dl -o ./northd -L /home/nusiddiq/.local/ddlog/lib -L ./northd error: /home/nusiddiq/workspace_cpp/ovn-org/ovn/northd/lrouter.dl:823.24-823.27: Unknown field lr router in &Router(.lr = nb::Logical_Router{.static_routes = routes}), ^^^ make: *** [Makefile:3598: northd/ddlog.stamp] Error 1 ---- I've sent a patch to fix this - https://patchwork.ozlabs.org/project/ovn/patch/20210527224826.1713821-1-numans@ovn.org/ Please take a look. Numan On Mon, May 10, 2021 at 2:52 PM Mark Michelson <mmichels@redhat.com> wrote: > > Hello Karthik, > > This all looks good to me. Thanks! > > Acked-by: Mark Michelson <mmichels@redhat.com> > > On 4/2/21 5:08 PM, svc.eng.git-mail@nutanix.com wrote: > > From: Karthik Chandrashekar <karthik.c@nutanix.com> > > > > Physical switches have the ability to specify "discard" or sometimes > > "NULL interface" as a nexthop for routes. This can be used to prevent > > routing loops in the network. Add a similar configuration for ovn > > where nexthop accepts the string "discard". When the nexthop is discard > > the action in the routing table will be to drop the packets. > > > > Signed-off-by: Karthik Chandrashekar <karthik.c@nutanix.com> > > > > ---- > > v1 -> v2 > > ---- > > * Add ddlog support > > * Don't allow nexthop = "discard" when ECMP and BFD is set > > ---- > > v2 -> v3 > > ---- > > * Use ddlog optimizations from review > > --- > > northd/lrouter.dl | 19 ++++++ > > northd/ovn-northd.c | 126 +++++++++++++++++++++----------------- > > northd/ovn_northd.dl | 10 +++ > > ovn-nb.xml | 4 +- > > tests/ovn-nbctl.at | 25 ++++++++ > > tests/ovn.at | 94 ++++++++++++++++++++++++++++ > > utilities/ovn-nbctl.8.xml | 3 +- > > utilities/ovn-nbctl.c | 67 +++++++++++++++----- > > 8 files changed, 276 insertions(+), 72 deletions(-) > > > > diff --git a/northd/lrouter.dl b/northd/lrouter.dl > > index 36cedd2dc..163bcd57c 100644 > > --- a/northd/lrouter.dl > > +++ b/northd/lrouter.dl > > @@ -769,6 +769,25 @@ RouterStaticRoute(router, key, dsts) :- > > }, > > var dsts = set_singleton(RouteDst{nexthop, src_ip, port, ecmp_symmetric_reply}). > > > > +/* compute route-route pairs for nexthop = "discard" routes */ > > +relation &DiscardRoute(lrsr: nb::Logical_Router_Static_Route, > > + key: route_key) > > +&DiscardRoute(.lrsr = lrsr, > > + .key = RouteKey{policy, ip_prefix, plen}) :- > > + lrsr in nb::Logical_Router_Static_Route(.nexthop = "discard"), > > + var policy = route_policy_from_string(lrsr.policy), > > + Some{(var ip_prefix, var plen)} = ip46_parse_cidr(lrsr.ip_prefix). > > + > > +relation RouterDiscardRoute_( > > + router : Ref<Router>, > > + key : route_key) > > + > > +RouterDiscardRoute_(.router = router, > > + .key = route.key) :- > > + router in &Router(.lr = nb::Logical_Router{.static_routes = routes}), > > + var route_id = FlatMap(routes), > > + route in &DiscardRoute(.lrsr = nb::Logical_Router_Static_Route{._uuid = route_id}). > > + > > Warning[message] :- > > RouterStaticRoute_(.router = router, .key = key, .nexthop = nexthop), > > not RouterStaticRoute(.router = router, .key = key), > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c > > index 9839b8c4f..c6d947518 100644 > > --- a/northd/ovn-northd.c > > +++ b/northd/ovn-northd.c > > @@ -7938,6 +7938,7 @@ struct parsed_route { > > uint32_t hash; > > const struct nbrec_logical_router_static_route *route; > > bool ecmp_symmetric_reply; > > + bool is_discard_route; > > }; > > > > static uint32_t > > @@ -7957,20 +7958,23 @@ parsed_routes_add(struct ovs_list *routes, > > /* Verify that the next hop is an IP address with an all-ones mask. */ > > struct in6_addr nexthop; > > unsigned int plen; > > - 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" > > - UUID_FMT, route->nexthop, > > - UUID_ARGS(&route->header_.uuid)); > > - return NULL; > > - } > > - if ((IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 32) || > > - (!IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 128)) { > > - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > > - VLOG_WARN_RL(&rl, "bad next hop mask %s in static route" > > - UUID_FMT, route->nexthop, > > - UUID_ARGS(&route->header_.uuid)); > > - return NULL; > > + bool is_discard_route = !strcmp(route->nexthop, "discard"); > > + if (!is_discard_route) { > > + 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" > > + UUID_FMT, route->nexthop, > > + UUID_ARGS(&route->header_.uuid)); > > + return NULL; > > + } > > + if ((IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 32) || > > + (!IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 128)) { > > + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); > > + VLOG_WARN_RL(&rl, "bad next hop mask %s in static route" > > + UUID_FMT, route->nexthop, > > + UUID_ARGS(&route->header_.uuid)); > > + return NULL; > > + } > > } > > > > /* Parse ip_prefix */ > > @@ -7984,13 +7988,15 @@ parsed_routes_add(struct ovs_list *routes, > > } > > > > /* Verify that ip_prefix and nexthop have same address familiy. */ > > - 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' %s" > > - " and 'nexthop' %s in static route"UUID_FMT, > > - route->ip_prefix, route->nexthop, > > - UUID_ARGS(&route->header_.uuid)); > > - return NULL; > > + if (!is_discard_route) { > > + 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'" > > + " %s and 'nexthop' %s in static route"UUID_FMT, > > + route->ip_prefix, route->nexthop, > > + UUID_ARGS(&route->header_.uuid)); > > + return NULL; > > + } > > } > > > > const struct nbrec_bfd *nb_bt = route->bfd; > > @@ -8021,6 +8027,7 @@ parsed_routes_add(struct ovs_list *routes, > > pr->route = route; > > pr->ecmp_symmetric_reply = smap_get_bool(&route->options, > > "ecmp_symmetric_reply", false); > > + pr->is_discard_route = is_discard_route; > > ovs_list_insert(routes, &pr->list_node); > > return pr; > > } > > @@ -8433,10 +8440,11 @@ build_ecmp_route_flow(struct hmap *lflows, struct ovn_datapath *od, > > } > > > > static void > > -add_route(struct hmap *lflows, const struct ovn_port *op, > > - const char *lrp_addr_s, const char *network_s, int plen, > > - const char *gateway, bool is_src_route, > > - const struct ovsdb_idl_row *stage_hint) > > +add_route(struct hmap *lflows, struct ovn_datapath *od, > > + const struct ovn_port *op, const char *lrp_addr_s, > > + const char *network_s, int plen, const char *gateway, > > + bool is_src_route, const struct ovsdb_idl_row *stage_hint, > > + bool is_discard_route) > > { > > bool is_ipv4 = strchr(network_s, '.') ? true : false; > > struct ds match = DS_EMPTY_INITIALIZER; > > @@ -8455,30 +8463,34 @@ add_route(struct hmap *lflows, const struct ovn_port *op, > > &match, &priority); > > > > struct ds common_actions = DS_EMPTY_INITIALIZER; > > - ds_put_format(&common_actions, REG_ECMP_GROUP_ID" = 0; %s = ", > > - is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6); > > - if (gateway) { > > - ds_put_cstr(&common_actions, gateway); > > - } else { > > - ds_put_format(&common_actions, "ip%s.dst", is_ipv4 ? "4" : "6"); > > - } > > - ds_put_format(&common_actions, "; " > > - "%s = %s; " > > - "eth.src = %s; " > > - "outport = %s; " > > - "flags.loopback = 1; " > > - "next;", > > - is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, > > - lrp_addr_s, > > - op->lrp_networks.ea_s, > > - op->json_key); > > struct ds actions = DS_EMPTY_INITIALIZER; > > - ds_put_format(&actions, "ip.ttl--; %s", ds_cstr(&common_actions)); > > + if (is_discard_route) { > > + ds_put_format(&actions, "drop;"); > > + } else { > > + ds_put_format(&common_actions, REG_ECMP_GROUP_ID" = 0; %s = ", > > + is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6); > > + if (gateway) { > > + ds_put_cstr(&common_actions, gateway); > > + } else { > > + ds_put_format(&common_actions, "ip%s.dst", is_ipv4 ? "4" : "6"); > > + } > > + ds_put_format(&common_actions, "; " > > + "%s = %s; " > > + "eth.src = %s; " > > + "outport = %s; " > > + "flags.loopback = 1; " > > + "next;", > > + is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, > > + lrp_addr_s, > > + op->lrp_networks.ea_s, > > + op->json_key); > > + ds_put_format(&actions, "ip.ttl--; %s", ds_cstr(&common_actions)); > > + } > > > > - ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_ROUTING, priority, > > + ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_ROUTING, priority, > > ds_cstr(&match), ds_cstr(&actions), > > stage_hint); > > - if (op->has_bfd) { > > + if (op && op->has_bfd) { > > ds_put_format(&match, " && udp.dst == 3784"); > > ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_ROUTING, > > priority + 1, ds_cstr(&match), > > @@ -8500,16 +8512,18 @@ build_static_route_flow(struct hmap *lflows, struct ovn_datapath *od, > > const struct nbrec_logical_router_static_route *route = route_->route; > > > > /* Find the outgoing port. */ > > - if (!find_static_route_outport(od, ports, route, > > - IN6_IS_ADDR_V4MAPPED(&route_->prefix), > > - &lrp_addr_s, &out_port)) { > > - return; > > + if (!route_->is_discard_route) { > > + if (!find_static_route_outport(od, ports, route, > > + IN6_IS_ADDR_V4MAPPED(&route_->prefix), > > + &lrp_addr_s, &out_port)) { > > + return; > > + } > > } > > > > char *prefix_s = build_route_prefix_s(&route_->prefix, route_->plen); > > - add_route(lflows, out_port, lrp_addr_s, prefix_s, route_->plen, > > - route->nexthop, route_->is_src_route, > > - &route->header_); > > + add_route(lflows, route_->is_discard_route ? od : out_port->od, out_port, > > + lrp_addr_s, prefix_s, route_->plen, route->nexthop, > > + route_->is_src_route, &route->header_, route_->is_discard_route); > > > > free(prefix_s); > > } > > @@ -9735,17 +9749,17 @@ build_ip_routing_flows_for_lrouter_port( > > if (op->nbrp) { > > > > for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { > > - add_route(lflows, op, op->lrp_networks.ipv4_addrs[i].addr_s, > > + add_route(lflows, op->od, op, op->lrp_networks.ipv4_addrs[i].addr_s, > > op->lrp_networks.ipv4_addrs[i].network_s, > > op->lrp_networks.ipv4_addrs[i].plen, NULL, false, > > - &op->nbrp->header_); > > + &op->nbrp->header_, false); > > } > > > > for (int i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { > > - add_route(lflows, op, op->lrp_networks.ipv6_addrs[i].addr_s, > > + add_route(lflows, op->od, op, op->lrp_networks.ipv6_addrs[i].addr_s, > > op->lrp_networks.ipv6_addrs[i].network_s, > > op->lrp_networks.ipv6_addrs[i].plen, NULL, false, > > - &op->nbrp->header_); > > + &op->nbrp->header_, false); > > } > > } > > } > > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl > > index 0063021e1..3b1e7fa1a 100644 > > --- a/northd/ovn_northd.dl > > +++ b/northd/ovn_northd.dl > > @@ -6370,6 +6370,16 @@ for (Route(.port = port, > > } > > } > > > > +/* Install drop routes for all the static routes with nexthop = "discard" */ > > +Flow(.logical_datapath = router.lr._uuid, > > + .stage = s_ROUTER_IN_IP_ROUTING(), > > + .priority = priority as integer, > > + .__match = ip_match, > > + .actions = "drop;", > > + .external_ids = map_empty()) :- > > + r in RouterDiscardRoute_(.router = router, .key = key), > > + (var ip_match, var priority) = build_route_match(r.key). > > + > > /* Logical router ingress table IP_ROUTING & IP_ROUTING_ECMP: IP Routing. > > * > > * A packet that arrives at this table is an IP packet that should be > > diff --git a/ovn-nb.xml b/ovn-nb.xml > > index b0a4adffe..98543404e 100644 > > --- a/ovn-nb.xml > > +++ b/ovn-nb.xml > > @@ -2649,7 +2649,9 @@ > > <column name="nexthop"> > > <p> > > Nexthop IP address for this route. Nexthop IP address should be the IP > > - address of a connected router port or the IP address of a logical port. > > + address of a connected router port or the IP address of a logical port > > + or can be set to <code>discard</code> for dropping packets which match > > + the given route. > > </p> > > </column> > > > > diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at > > index 6d91aa4c5..0392cd968 100644 > > --- a/tests/ovn-nbctl.at > > +++ b/tests/ovn-nbctl.at > > @@ -1467,18 +1467,38 @@ AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.0.111/24 11.0.0.1/24], [1], [], > > AT_CHECK([ovn-nbctl lr-route-add lr0 2001:0db8:1::/64 2001:0db8:0:f103::1/64], [1], [], > > [ovn-nbctl: bad IPv6 nexthop argument: 2001:0db8:0:f103::1/64 > > ]) > > +AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 20.0.0.0/24 discard], [1], [], > > + [ovn-nbctl: ecmp is not valid for discard routes. > > +]) > > +AT_CHECK([ovn-nbctl --ecmp-symmetric-reply lr-route-add lr0 20.0.0.0/24 discard], [1], [], > > + [ovn-nbctl: ecmp is not valid for discard routes. > > +]) > > +AT_CHECK([ovn-nbctl --bfd lr-route-add lr0 20.0.0.0/24 discard lp0], [1], [], > > + [ovn-nbctl: bfd dst_ip cannot be discard. > > +]) > > +AT_CHECK([ovn-nbctl lr-route-add lr0 20.0.0.0/24 discard lp0], [1], [], > > + [ovn-nbctl: outport is not valid for discard routes. > > +]) > > > > AT_CHECK([ovn-nbctl --may-exist lr-route-add lr0 10.0.0.111/24 11.0.0.1]) > > AT_CHECK([ovn-nbctl --policy=src-ip lr-route-add lr0 9.16.1.0/24 11.0.0.1]) > > dnl Add a route with existed prefix but different policy (src-ip) > > 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-add lr0 20.0.0.0/24 discard]) > > +AT_CHECK([ovn-nbctl --policy=src-ip lr-route-add lr0 20.0.0.0/24 discard]) > > +AT_CHECK([ovn-nbctl --ecmp --policy=src-ip lr-route-add lr0 20.0.0.0/24 11.0.0.1], [1], [], > > + [ovn-nbctl: discard nexthop for the same ECMP route exists. > > +]) > > + > > 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 > > + 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 > > + 20.0.0.0/24 discard src-ip > > 0.0.0.0/0 192.168.0.1 dst-ip > > ]) > > > > @@ -1487,11 +1507,16 @@ 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 > > + 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 > > + 20.0.0.0/24 discard src-ip > > 0.0.0.0/0 192.168.0.1 dst-ip > > ]) > > > > +AT_CHECK([ovn-nbctl --policy=src-ip lr-route-del lr0 20.0.0.0/24]) > > +AT_CHECK([ovn-nbctl lr-route-del lr0 20.0.0.0/24 discard]) > > + > > dnl Delete non-existent prefix > > AT_CHECK([ovn-nbctl lr-route-del lr0 10.0.2.1/24], [1], [], > > [ovn-nbctl: no matching route: policy 'any', prefix '10.0.2.0/24', nexthop 'any', output_port 'any'. > > diff --git a/tests/ovn.at b/tests/ovn.at > > index 73cc8bf02..176d08512 100644 > > --- a/tests/ovn.at > > +++ b/tests/ovn.at > > @@ -25856,3 +25856,97 @@ primary lport : [[sw0p2]] > > OVN_CLEANUP([hv1], [hv2]) > > AT_CLEANUP > > ]) > > + > > +OVN_FOR_EACH_NORTHD([ > > +AT_SETUP([ovn -- Static route with discard nexthop]) > > +ovn_start > > + > > +# Logical network: > > +# Logical Router lr1 has Logical Switch sw1 (10.0.0.0/24) connected to it. sw1 has one > > +# switch port sw1-ls1 (10.0.0.100) > > + > > +ovn-nbctl lr-add lr1 > > + > > +ovn-nbctl ls-add sw1 > > + > > +# Connect sw1 to lr1 > > +ovn-nbctl lrp-add lr1 lr1-sw1 00:00:01:01:02:03 10.0.0.1/24 > > +ovn-nbctl lsp-add sw1 sw1-lr1 > > +ovn-nbctl lsp-set-type sw1-lr1 router > > +ovn-nbctl lsp-set-addresses sw1-lr1 router > > +ovn-nbctl lsp-set-options sw1-lr1 router-port=lr1-sw1 > > + > > +# Create logical port sw1-lp1 in sw1 > > +ovn-nbctl lsp-add sw1 sw1-lp1 \ > > +-- lsp-set-addresses sw1-lp1 "00:00:04:01:02:03 10.0.0.100" > > + > > +# Create one hypervisor and create OVS ports corresponding to logical ports. > > +net_add n1 > > + > > +sim_add hv1 > > +as hv1 > > +ovs-vsctl add-br br-phys > > +ovn_attach n1 br-phys 192.168.0.1 > > + > > +ovs-vsctl -- add-port br-int hv1-vif1 -- \ > > + set interface hv1-vif1 external-ids:iface-id=sw1-lp1 \ > > + options:tx_pcap=hv1/vif1-tx.pcap \ > > + options:rxq_pcap=hv1/vif1-rx.pcap \ > > + ofport-request=1 > > + > > +# Install static routes to drop traffic > > +ovn-nbctl lr-route-add lr1 20.0.0.0/24 discard > > + > > +# Allow some time for ovn-controller to catch up. > > +ovn-nbctl --wait=hv sync > > + > > +# Check logical flows for drop rule > > +AT_CHECK([ovn-sbctl dump-flows | grep lr_in_ip_routing | grep "20.0.0.0/24" | \ > > + grep drop | wc -l], [0], [dnl > > +1 > > +]) > > + > > +# Send packet. > > +packet="inport==\"sw1-lp1\" && eth.src==00:00:04:01:02:03 && > > + eth.dst==00:00:01:01:02:03 && ip4 && ip.ttl==64 && > > + ip4.src==10.0.0.100 && ip4.dst==20.0.0.200 && > > + udp && udp.src==53 && udp.dst==4369" > > + > > +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet" > > + > > +# Check if packet hit the drop rule > > +AT_CHECK([ovs-ofctl dump-flows br-int | grep "nw_dst=20.0.0.0/24" | \ > > + grep "actions=drop" | grep "n_packets=1" | wc -l], [0], [dnl > > +1 > > +]) > > + > > +# Remove destination match and add discard route with source match > > +ovn-nbctl lr-route-del lr1 20.0.0.0/24 > > +ovn-nbctl --policy="src-ip" lr-route-add lr1 10.0.0.0/24 discard > > + > > +# Allow some time for ovn-controller to catch up. > > +ovn-nbctl --wait=hv sync > > + > > +# Check logical flows for drop rule > > +AT_CHECK([ovn-sbctl dump-flows | grep lr_in_ip_routing | grep "10.0.0.0/24" | \ > > + grep drop | wc -l], [0], [dnl > > +1 > > +]) > > + > > +# Send packet. > > +packet="inport==\"sw1-lp1\" && eth.src==00:00:04:01:02:03 && > > + eth.dst==00:00:01:01:02:03 && ip4 && ip.ttl==64 && > > + ip4.src==10.0.0.100 && ip4.dst==20.0.0.200 && > > + udp && udp.src==53 && udp.dst==4369" > > + > > +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet" > > + > > +# Check if packet hit the drop rule > > +AT_CHECK([ovs-ofctl dump-flows br-int "nw_src=10.0.0.0/24" | \ > > + grep "actions=drop" | grep "n_packets=1" | wc -l], [0], [dnl > > +1 > > +]) > > + > > +OVN_CLEANUP([hv1]) > > +AT_CLEANUP > > +]) > > \ No newline at end of file > > diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml > > index 03d47dba5..1ab63d413 100644 > > --- a/utilities/ovn-nbctl.8.xml > > +++ b/utilities/ovn-nbctl.8.xml > > @@ -673,7 +673,8 @@ > > logical port. If <var>port</var> is specified, packets that > > match this route will be sent out that port. When > > <var>port</var> is omitted, OVN infers the output port based > > - on <var>nexthop</var>. > > + on <var>nexthop</var>. Nexthop can be set to <var>discard</var> > > + for dropping packets which match the given route. > > </p> > > > > <p> > > diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c > > index 184058356..2cbd324b7 100644 > > --- a/utilities/ovn-nbctl.c > > +++ b/utilities/ovn-nbctl.c > > @@ -3971,6 +3971,7 @@ nbctl_lr_route_add(struct ctl_context *ctx) > > > > const char *policy = shash_find_data(&ctx->options, "--policy"); > > bool is_src_route = false; > > + > > if (policy) { > > if (!strcmp(policy, "src-ip")) { > > is_src_route = true; > > @@ -3991,13 +3992,18 @@ nbctl_lr_route_add(struct ctl_context *ctx) > > return; > > } > > > > - next_hop = v6_prefix > > - ? 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; > > + bool is_discard_route = !strcmp(ctx->argv[3], "discard"); > > + if (is_discard_route) { > > + next_hop = xasprintf("discard"); > > + } else { > > + next_hop = v6_prefix > > + ? 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; > > + } > > } > > > > struct shash_node *bfd = shash_find(&ctx->options, "--bfd"); > > @@ -4030,6 +4036,25 @@ nbctl_lr_route_add(struct ctl_context *ctx) > > ecmp_symmetric_reply; > > struct nbrec_logical_router_static_route *route = > > nbctl_lr_get_route(lr, prefix, next_hop, is_src_route, ecmp); > > + > > + /* Validations for nexthop = "discard" */ > > + if (is_discard_route) { > > + if (ecmp) { > > + ctl_error(ctx, "ecmp is not valid for discard routes."); > > + goto cleanup; > > + } > > + if (bfd) { > > + ctl_error(ctx, "bfd dst_ip cannot be discard."); > > + goto cleanup; > > + } > > + if (ctx->argc == 5) { > > + if (is_discard_route) { > > + ctl_error(ctx, "outport is not valid for discard routes."); > > + goto cleanup; > > + } > > + } > > + } > > + > > if (!ecmp) { > > if (route) { > > if (!may_exist) { > > @@ -4071,6 +4096,13 @@ nbctl_lr_route_add(struct ctl_context *ctx) > > goto cleanup; > > } > > > > + struct nbrec_logical_router_static_route *discard_route = > > + nbctl_lr_get_route(lr, prefix, "discard", is_src_route, true); > > + if (discard_route) { > > + ctl_error(ctx, "discard nexthop for the same ECMP route exists."); > > + goto cleanup; > > + } > > + > > route = nbrec_logical_router_static_route_insert(ctx->txn); > > nbrec_logical_router_static_route_set_ip_prefix(route, prefix); > > nbrec_logical_router_static_route_set_nexthop(route, next_hop); > > @@ -4145,10 +4177,14 @@ nbctl_lr_route_del(struct ctl_context *ctx) > > > > char *nexthop = NULL; > > if (ctx->argc >= 4) { > > - nexthop = normalize_prefix_str(ctx->argv[3]); > > - if (!nexthop) { > > - ctl_error(ctx, "bad nexthop argument: %s", ctx->argv[3]); > > - return; > > + if (!strcmp(ctx->argv[3], "discard")) { > > + nexthop = xasprintf("discard"); > > + } else { > > + nexthop = normalize_prefix_str(ctx->argv[3]); > > + if (!nexthop) { > > + ctl_error(ctx, "bad nexthop argument: %s", ctx->argv[3]); > > + return; > > + } > > } > > } > > > > @@ -4189,8 +4225,9 @@ nbctl_lr_route_del(struct ctl_context *ctx) > > > > /* Compare nexthop, if specified. */ > > if (nexthop) { > > - char *rt_nexthop = > > - normalize_prefix_str(lr->static_routes[i]->nexthop); > > + char *rt_nexthop = !strcmp(lr->static_routes[i]->nexthop, "discard") > > + ? xasprintf("discard") > > + : normalize_prefix_str(lr->static_routes[i]->nexthop); > > if (!rt_nexthop) { > > /* Ignore existing nexthop we couldn't parse. */ > > continue; > > @@ -5578,7 +5615,9 @@ print_route(const struct nbrec_logical_router_static_route *route, > > { > > > > char *prefix = normalize_prefix_str(route->ip_prefix); > > - char *next_hop = normalize_prefix_str(route->nexthop); > > + char *next_hop = !strcmp(route->nexthop, "discard") > > + ? xasprintf("discard") > > + : normalize_prefix_str(route->nexthop); > > ds_put_format(s, "%25s %25s", prefix, next_hop); > > free(prefix); > > free(next_hop); > > > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
diff --git a/northd/lrouter.dl b/northd/lrouter.dl index 36cedd2dc..163bcd57c 100644 --- a/northd/lrouter.dl +++ b/northd/lrouter.dl @@ -769,6 +769,25 @@ RouterStaticRoute(router, key, dsts) :- }, var dsts = set_singleton(RouteDst{nexthop, src_ip, port, ecmp_symmetric_reply}). +/* compute route-route pairs for nexthop = "discard" routes */ +relation &DiscardRoute(lrsr: nb::Logical_Router_Static_Route, + key: route_key) +&DiscardRoute(.lrsr = lrsr, + .key = RouteKey{policy, ip_prefix, plen}) :- + lrsr in nb::Logical_Router_Static_Route(.nexthop = "discard"), + var policy = route_policy_from_string(lrsr.policy), + Some{(var ip_prefix, var plen)} = ip46_parse_cidr(lrsr.ip_prefix). + +relation RouterDiscardRoute_( + router : Ref<Router>, + key : route_key) + +RouterDiscardRoute_(.router = router, + .key = route.key) :- + router in &Router(.lr = nb::Logical_Router{.static_routes = routes}), + var route_id = FlatMap(routes), + route in &DiscardRoute(.lrsr = nb::Logical_Router_Static_Route{._uuid = route_id}). + Warning[message] :- RouterStaticRoute_(.router = router, .key = key, .nexthop = nexthop), not RouterStaticRoute(.router = router, .key = key), diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 9839b8c4f..c6d947518 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -7938,6 +7938,7 @@ struct parsed_route { uint32_t hash; const struct nbrec_logical_router_static_route *route; bool ecmp_symmetric_reply; + bool is_discard_route; }; static uint32_t @@ -7957,20 +7958,23 @@ parsed_routes_add(struct ovs_list *routes, /* Verify that the next hop is an IP address with an all-ones mask. */ struct in6_addr nexthop; unsigned int plen; - 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" - UUID_FMT, route->nexthop, - UUID_ARGS(&route->header_.uuid)); - return NULL; - } - if ((IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 32) || - (!IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 128)) { - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); - VLOG_WARN_RL(&rl, "bad next hop mask %s in static route" - UUID_FMT, route->nexthop, - UUID_ARGS(&route->header_.uuid)); - return NULL; + bool is_discard_route = !strcmp(route->nexthop, "discard"); + if (!is_discard_route) { + 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" + UUID_FMT, route->nexthop, + UUID_ARGS(&route->header_.uuid)); + return NULL; + } + if ((IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 32) || + (!IN6_IS_ADDR_V4MAPPED(&nexthop) && plen != 128)) { + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1); + VLOG_WARN_RL(&rl, "bad next hop mask %s in static route" + UUID_FMT, route->nexthop, + UUID_ARGS(&route->header_.uuid)); + return NULL; + } } /* Parse ip_prefix */ @@ -7984,13 +7988,15 @@ parsed_routes_add(struct ovs_list *routes, } /* Verify that ip_prefix and nexthop have same address familiy. */ - 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' %s" - " and 'nexthop' %s in static route"UUID_FMT, - route->ip_prefix, route->nexthop, - UUID_ARGS(&route->header_.uuid)); - return NULL; + if (!is_discard_route) { + 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'" + " %s and 'nexthop' %s in static route"UUID_FMT, + route->ip_prefix, route->nexthop, + UUID_ARGS(&route->header_.uuid)); + return NULL; + } } const struct nbrec_bfd *nb_bt = route->bfd; @@ -8021,6 +8027,7 @@ parsed_routes_add(struct ovs_list *routes, pr->route = route; pr->ecmp_symmetric_reply = smap_get_bool(&route->options, "ecmp_symmetric_reply", false); + pr->is_discard_route = is_discard_route; ovs_list_insert(routes, &pr->list_node); return pr; } @@ -8433,10 +8440,11 @@ build_ecmp_route_flow(struct hmap *lflows, struct ovn_datapath *od, } static void -add_route(struct hmap *lflows, const struct ovn_port *op, - const char *lrp_addr_s, const char *network_s, int plen, - const char *gateway, bool is_src_route, - const struct ovsdb_idl_row *stage_hint) +add_route(struct hmap *lflows, struct ovn_datapath *od, + const struct ovn_port *op, const char *lrp_addr_s, + const char *network_s, int plen, const char *gateway, + bool is_src_route, const struct ovsdb_idl_row *stage_hint, + bool is_discard_route) { bool is_ipv4 = strchr(network_s, '.') ? true : false; struct ds match = DS_EMPTY_INITIALIZER; @@ -8455,30 +8463,34 @@ add_route(struct hmap *lflows, const struct ovn_port *op, &match, &priority); struct ds common_actions = DS_EMPTY_INITIALIZER; - ds_put_format(&common_actions, REG_ECMP_GROUP_ID" = 0; %s = ", - is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6); - if (gateway) { - ds_put_cstr(&common_actions, gateway); - } else { - ds_put_format(&common_actions, "ip%s.dst", is_ipv4 ? "4" : "6"); - } - ds_put_format(&common_actions, "; " - "%s = %s; " - "eth.src = %s; " - "outport = %s; " - "flags.loopback = 1; " - "next;", - is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, - lrp_addr_s, - op->lrp_networks.ea_s, - op->json_key); struct ds actions = DS_EMPTY_INITIALIZER; - ds_put_format(&actions, "ip.ttl--; %s", ds_cstr(&common_actions)); + if (is_discard_route) { + ds_put_format(&actions, "drop;"); + } else { + ds_put_format(&common_actions, REG_ECMP_GROUP_ID" = 0; %s = ", + is_ipv4 ? REG_NEXT_HOP_IPV4 : REG_NEXT_HOP_IPV6); + if (gateway) { + ds_put_cstr(&common_actions, gateway); + } else { + ds_put_format(&common_actions, "ip%s.dst", is_ipv4 ? "4" : "6"); + } + ds_put_format(&common_actions, "; " + "%s = %s; " + "eth.src = %s; " + "outport = %s; " + "flags.loopback = 1; " + "next;", + is_ipv4 ? REG_SRC_IPV4 : REG_SRC_IPV6, + lrp_addr_s, + op->lrp_networks.ea_s, + op->json_key); + ds_put_format(&actions, "ip.ttl--; %s", ds_cstr(&common_actions)); + } - ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_ROUTING, priority, + ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_ROUTING, priority, ds_cstr(&match), ds_cstr(&actions), stage_hint); - if (op->has_bfd) { + if (op && op->has_bfd) { ds_put_format(&match, " && udp.dst == 3784"); ovn_lflow_add_with_hint(lflows, op->od, S_ROUTER_IN_IP_ROUTING, priority + 1, ds_cstr(&match), @@ -8500,16 +8512,18 @@ build_static_route_flow(struct hmap *lflows, struct ovn_datapath *od, const struct nbrec_logical_router_static_route *route = route_->route; /* Find the outgoing port. */ - if (!find_static_route_outport(od, ports, route, - IN6_IS_ADDR_V4MAPPED(&route_->prefix), - &lrp_addr_s, &out_port)) { - return; + if (!route_->is_discard_route) { + if (!find_static_route_outport(od, ports, route, + IN6_IS_ADDR_V4MAPPED(&route_->prefix), + &lrp_addr_s, &out_port)) { + return; + } } char *prefix_s = build_route_prefix_s(&route_->prefix, route_->plen); - add_route(lflows, out_port, lrp_addr_s, prefix_s, route_->plen, - route->nexthop, route_->is_src_route, - &route->header_); + add_route(lflows, route_->is_discard_route ? od : out_port->od, out_port, + lrp_addr_s, prefix_s, route_->plen, route->nexthop, + route_->is_src_route, &route->header_, route_->is_discard_route); free(prefix_s); } @@ -9735,17 +9749,17 @@ build_ip_routing_flows_for_lrouter_port( if (op->nbrp) { for (int i = 0; i < op->lrp_networks.n_ipv4_addrs; i++) { - add_route(lflows, op, op->lrp_networks.ipv4_addrs[i].addr_s, + add_route(lflows, op->od, op, op->lrp_networks.ipv4_addrs[i].addr_s, op->lrp_networks.ipv4_addrs[i].network_s, op->lrp_networks.ipv4_addrs[i].plen, NULL, false, - &op->nbrp->header_); + &op->nbrp->header_, false); } for (int i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) { - add_route(lflows, op, op->lrp_networks.ipv6_addrs[i].addr_s, + add_route(lflows, op->od, op, op->lrp_networks.ipv6_addrs[i].addr_s, op->lrp_networks.ipv6_addrs[i].network_s, op->lrp_networks.ipv6_addrs[i].plen, NULL, false, - &op->nbrp->header_); + &op->nbrp->header_, false); } } } diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl index 0063021e1..3b1e7fa1a 100644 --- a/northd/ovn_northd.dl +++ b/northd/ovn_northd.dl @@ -6370,6 +6370,16 @@ for (Route(.port = port, } } +/* Install drop routes for all the static routes with nexthop = "discard" */ +Flow(.logical_datapath = router.lr._uuid, + .stage = s_ROUTER_IN_IP_ROUTING(), + .priority = priority as integer, + .__match = ip_match, + .actions = "drop;", + .external_ids = map_empty()) :- + r in RouterDiscardRoute_(.router = router, .key = key), + (var ip_match, var priority) = build_route_match(r.key). + /* Logical router ingress table IP_ROUTING & IP_ROUTING_ECMP: IP Routing. * * A packet that arrives at this table is an IP packet that should be diff --git a/ovn-nb.xml b/ovn-nb.xml index b0a4adffe..98543404e 100644 --- a/ovn-nb.xml +++ b/ovn-nb.xml @@ -2649,7 +2649,9 @@ <column name="nexthop"> <p> Nexthop IP address for this route. Nexthop IP address should be the IP - address of a connected router port or the IP address of a logical port. + address of a connected router port or the IP address of a logical port + or can be set to <code>discard</code> for dropping packets which match + the given route. </p> </column> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at index 6d91aa4c5..0392cd968 100644 --- a/tests/ovn-nbctl.at +++ b/tests/ovn-nbctl.at @@ -1467,18 +1467,38 @@ AT_CHECK([ovn-nbctl lr-route-add lr0 10.0.0.111/24 11.0.0.1/24], [1], [], AT_CHECK([ovn-nbctl lr-route-add lr0 2001:0db8:1::/64 2001:0db8:0:f103::1/64], [1], [], [ovn-nbctl: bad IPv6 nexthop argument: 2001:0db8:0:f103::1/64 ]) +AT_CHECK([ovn-nbctl --ecmp lr-route-add lr0 20.0.0.0/24 discard], [1], [], + [ovn-nbctl: ecmp is not valid for discard routes. +]) +AT_CHECK([ovn-nbctl --ecmp-symmetric-reply lr-route-add lr0 20.0.0.0/24 discard], [1], [], + [ovn-nbctl: ecmp is not valid for discard routes. +]) +AT_CHECK([ovn-nbctl --bfd lr-route-add lr0 20.0.0.0/24 discard lp0], [1], [], + [ovn-nbctl: bfd dst_ip cannot be discard. +]) +AT_CHECK([ovn-nbctl lr-route-add lr0 20.0.0.0/24 discard lp0], [1], [], + [ovn-nbctl: outport is not valid for discard routes. +]) AT_CHECK([ovn-nbctl --may-exist lr-route-add lr0 10.0.0.111/24 11.0.0.1]) AT_CHECK([ovn-nbctl --policy=src-ip lr-route-add lr0 9.16.1.0/24 11.0.0.1]) dnl Add a route with existed prefix but different policy (src-ip) 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-add lr0 20.0.0.0/24 discard]) +AT_CHECK([ovn-nbctl --policy=src-ip lr-route-add lr0 20.0.0.0/24 discard]) +AT_CHECK([ovn-nbctl --ecmp --policy=src-ip lr-route-add lr0 20.0.0.0/24 11.0.0.1], [1], [], + [ovn-nbctl: discard nexthop for the same ECMP route exists. +]) + 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 + 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 + 20.0.0.0/24 discard src-ip 0.0.0.0/0 192.168.0.1 dst-ip ]) @@ -1487,11 +1507,16 @@ 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 + 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 + 20.0.0.0/24 discard src-ip 0.0.0.0/0 192.168.0.1 dst-ip ]) +AT_CHECK([ovn-nbctl --policy=src-ip lr-route-del lr0 20.0.0.0/24]) +AT_CHECK([ovn-nbctl lr-route-del lr0 20.0.0.0/24 discard]) + dnl Delete non-existent prefix AT_CHECK([ovn-nbctl lr-route-del lr0 10.0.2.1/24], [1], [], [ovn-nbctl: no matching route: policy 'any', prefix '10.0.2.0/24', nexthop 'any', output_port 'any'. diff --git a/tests/ovn.at b/tests/ovn.at index 73cc8bf02..176d08512 100644 --- a/tests/ovn.at +++ b/tests/ovn.at @@ -25856,3 +25856,97 @@ primary lport : [[sw0p2]] OVN_CLEANUP([hv1], [hv2]) AT_CLEANUP ]) + +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn -- Static route with discard nexthop]) +ovn_start + +# Logical network: +# Logical Router lr1 has Logical Switch sw1 (10.0.0.0/24) connected to it. sw1 has one +# switch port sw1-ls1 (10.0.0.100) + +ovn-nbctl lr-add lr1 + +ovn-nbctl ls-add sw1 + +# Connect sw1 to lr1 +ovn-nbctl lrp-add lr1 lr1-sw1 00:00:01:01:02:03 10.0.0.1/24 +ovn-nbctl lsp-add sw1 sw1-lr1 +ovn-nbctl lsp-set-type sw1-lr1 router +ovn-nbctl lsp-set-addresses sw1-lr1 router +ovn-nbctl lsp-set-options sw1-lr1 router-port=lr1-sw1 + +# Create logical port sw1-lp1 in sw1 +ovn-nbctl lsp-add sw1 sw1-lp1 \ +-- lsp-set-addresses sw1-lp1 "00:00:04:01:02:03 10.0.0.100" + +# Create one hypervisor and create OVS ports corresponding to logical ports. +net_add n1 + +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 + +ovs-vsctl -- add-port br-int hv1-vif1 -- \ + set interface hv1-vif1 external-ids:iface-id=sw1-lp1 \ + options:tx_pcap=hv1/vif1-tx.pcap \ + options:rxq_pcap=hv1/vif1-rx.pcap \ + ofport-request=1 + +# Install static routes to drop traffic +ovn-nbctl lr-route-add lr1 20.0.0.0/24 discard + +# Allow some time for ovn-controller to catch up. +ovn-nbctl --wait=hv sync + +# Check logical flows for drop rule +AT_CHECK([ovn-sbctl dump-flows | grep lr_in_ip_routing | grep "20.0.0.0/24" | \ + grep drop | wc -l], [0], [dnl +1 +]) + +# Send packet. +packet="inport==\"sw1-lp1\" && eth.src==00:00:04:01:02:03 && + eth.dst==00:00:01:01:02:03 && ip4 && ip.ttl==64 && + ip4.src==10.0.0.100 && ip4.dst==20.0.0.200 && + udp && udp.src==53 && udp.dst==4369" + +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet" + +# Check if packet hit the drop rule +AT_CHECK([ovs-ofctl dump-flows br-int | grep "nw_dst=20.0.0.0/24" | \ + grep "actions=drop" | grep "n_packets=1" | wc -l], [0], [dnl +1 +]) + +# Remove destination match and add discard route with source match +ovn-nbctl lr-route-del lr1 20.0.0.0/24 +ovn-nbctl --policy="src-ip" lr-route-add lr1 10.0.0.0/24 discard + +# Allow some time for ovn-controller to catch up. +ovn-nbctl --wait=hv sync + +# Check logical flows for drop rule +AT_CHECK([ovn-sbctl dump-flows | grep lr_in_ip_routing | grep "10.0.0.0/24" | \ + grep drop | wc -l], [0], [dnl +1 +]) + +# Send packet. +packet="inport==\"sw1-lp1\" && eth.src==00:00:04:01:02:03 && + eth.dst==00:00:01:01:02:03 && ip4 && ip.ttl==64 && + ip4.src==10.0.0.100 && ip4.dst==20.0.0.200 && + udp && udp.src==53 && udp.dst==4369" + +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet" + +# Check if packet hit the drop rule +AT_CHECK([ovs-ofctl dump-flows br-int "nw_src=10.0.0.0/24" | \ + grep "actions=drop" | grep "n_packets=1" | wc -l], [0], [dnl +1 +]) + +OVN_CLEANUP([hv1]) +AT_CLEANUP +]) \ No newline at end of file diff --git a/utilities/ovn-nbctl.8.xml b/utilities/ovn-nbctl.8.xml index 03d47dba5..1ab63d413 100644 --- a/utilities/ovn-nbctl.8.xml +++ b/utilities/ovn-nbctl.8.xml @@ -673,7 +673,8 @@ logical port. If <var>port</var> is specified, packets that match this route will be sent out that port. When <var>port</var> is omitted, OVN infers the output port based - on <var>nexthop</var>. + on <var>nexthop</var>. Nexthop can be set to <var>discard</var> + for dropping packets which match the given route. </p> <p> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c index 184058356..2cbd324b7 100644 --- a/utilities/ovn-nbctl.c +++ b/utilities/ovn-nbctl.c @@ -3971,6 +3971,7 @@ nbctl_lr_route_add(struct ctl_context *ctx) const char *policy = shash_find_data(&ctx->options, "--policy"); bool is_src_route = false; + if (policy) { if (!strcmp(policy, "src-ip")) { is_src_route = true; @@ -3991,13 +3992,18 @@ nbctl_lr_route_add(struct ctl_context *ctx) return; } - next_hop = v6_prefix - ? 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; + bool is_discard_route = !strcmp(ctx->argv[3], "discard"); + if (is_discard_route) { + next_hop = xasprintf("discard"); + } else { + next_hop = v6_prefix + ? 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; + } } struct shash_node *bfd = shash_find(&ctx->options, "--bfd"); @@ -4030,6 +4036,25 @@ nbctl_lr_route_add(struct ctl_context *ctx) ecmp_symmetric_reply; struct nbrec_logical_router_static_route *route = nbctl_lr_get_route(lr, prefix, next_hop, is_src_route, ecmp); + + /* Validations for nexthop = "discard" */ + if (is_discard_route) { + if (ecmp) { + ctl_error(ctx, "ecmp is not valid for discard routes."); + goto cleanup; + } + if (bfd) { + ctl_error(ctx, "bfd dst_ip cannot be discard."); + goto cleanup; + } + if (ctx->argc == 5) { + if (is_discard_route) { + ctl_error(ctx, "outport is not valid for discard routes."); + goto cleanup; + } + } + } + if (!ecmp) { if (route) { if (!may_exist) { @@ -4071,6 +4096,13 @@ nbctl_lr_route_add(struct ctl_context *ctx) goto cleanup; } + struct nbrec_logical_router_static_route *discard_route = + nbctl_lr_get_route(lr, prefix, "discard", is_src_route, true); + if (discard_route) { + ctl_error(ctx, "discard nexthop for the same ECMP route exists."); + goto cleanup; + } + route = nbrec_logical_router_static_route_insert(ctx->txn); nbrec_logical_router_static_route_set_ip_prefix(route, prefix); nbrec_logical_router_static_route_set_nexthop(route, next_hop); @@ -4145,10 +4177,14 @@ nbctl_lr_route_del(struct ctl_context *ctx) char *nexthop = NULL; if (ctx->argc >= 4) { - nexthop = normalize_prefix_str(ctx->argv[3]); - if (!nexthop) { - ctl_error(ctx, "bad nexthop argument: %s", ctx->argv[3]); - return; + if (!strcmp(ctx->argv[3], "discard")) { + nexthop = xasprintf("discard"); + } else { + nexthop = normalize_prefix_str(ctx->argv[3]); + if (!nexthop) { + ctl_error(ctx, "bad nexthop argument: %s", ctx->argv[3]); + return; + } } } @@ -4189,8 +4225,9 @@ nbctl_lr_route_del(struct ctl_context *ctx) /* Compare nexthop, if specified. */ if (nexthop) { - char *rt_nexthop = - normalize_prefix_str(lr->static_routes[i]->nexthop); + char *rt_nexthop = !strcmp(lr->static_routes[i]->nexthop, "discard") + ? xasprintf("discard") + : normalize_prefix_str(lr->static_routes[i]->nexthop); if (!rt_nexthop) { /* Ignore existing nexthop we couldn't parse. */ continue; @@ -5578,7 +5615,9 @@ print_route(const struct nbrec_logical_router_static_route *route, { char *prefix = normalize_prefix_str(route->ip_prefix); - char *next_hop = normalize_prefix_str(route->nexthop); + char *next_hop = !strcmp(route->nexthop, "discard") + ? xasprintf("discard") + : normalize_prefix_str(route->nexthop); ds_put_format(s, "%25s %25s", prefix, next_hop); free(prefix); free(next_hop);