On 9/23/25 8:40 AM, Ales Musil wrote:
> On Mon, Sep 22, 2025 at 4:27 PM Dumitru Ceara via dev <
> [email protected]> wrote:
>
>> This closes the gap with the static routes support and allows users to
>> explicitly break the tie for cases that are ambiguous, e.g., when the
>> reroute next hop IP could be theoretically reachable on multiple
>> connected router ports. An initial proposal tried to solve this problem
>> in a different way [0] but upon further discussions [1] we agreed to match
>> the support and static routes currently offer.
>>
>> The NB router policy schema is extended to include an "output_port"
>> optional field which will be used by the CMS to explicitly determine
>> which router port to be used when the reroute policy is applied. This
>> overrides any automatic port detection that happens for policies without
>> an "output_port" explicitly specified.
>>
>> As mentioned in the documentation and in the TODO.rst file, the support
>> is limited to non-ECMP policies. That's mainly because the current
>> router policy schema makes it extremely cumbersome to attach different
>> ports to different next hops that may be defined for the policy. Unlike
>> NB static routes, the router policies store the ECMP nexthops as
>> comma-separated strings in the NB policy record.
>>
>> [0] https://mail.openvswitch.org/pipermail/ovs-dev/2025-July/424894.html
>> [1]
>> https://mail.openvswitch.org/pipermail/ovs-dev/2025-September/426231.html
>>
>> Reported-at: https://issues.redhat.com/browse/FDP-1554
>> Reported-by: Jakub Libosvar <[email protected]>
>> Signed-off-by: Dumitru Ceara <[email protected]>
>> ---
>>
>
> Hi Dumitru,
>
Hi Ales,
> thank you for the patch. I have two small nits below.
>
Thanks for the review!
> NEWS | 1 +
>> TODO.rst | 6 ++
>> northd/en-learned-route-sync.c | 2 +-
>> northd/northd.c | 137 +++++++++++++++++++++------------
>> northd/northd.h | 3 +-
>> ovn-nb.ovsschema | 10 ++-
>> ovn-nb.xml | 17 ++++
>> tests/ovn-nbctl.at | 17 ++++
>> tests/ovn-northd.at | 45 +++++++++++
>> tests/system-ovn.at | 98 +++++++++++++++++++++++
>> utilities/ovn-nbctl.c | 33 +++++++-
>> 11 files changed, 313 insertions(+), 56 deletions(-)
>>
>> diff --git a/NEWS b/NEWS
>> index 4d0c45e4dc..ede89aea35 100644
>> --- a/NEWS
>> +++ b/NEWS
>> @@ -2,6 +2,7 @@ Post v25.09.0
>> -------------
>> - Added disable_garp_rarp option to logical_router table in order to
>> disable
>> GARP/RARP announcements by all the peer ports of this logical router.
>> + - Support for specifying output_port for logical router reroute
>> policies.
>>
>> OVN v25.09.0 - xxx xx xxxx
>> --------------------------
>> diff --git a/TODO.rst b/TODO.rst
>> index cda5f0d996..b992b8e5ae 100644
>> --- a/TODO.rst
>> +++ b/TODO.rst
>> @@ -171,6 +171,12 @@ OVN To-do List
>> allow for the eventual removal of the ovn\_datapath structure from the
>> codebase.
>>
>> +* Logical Router Policies
>> +
>> + * Add support for configuring output\_port for reroute router policies
>> that
>> + have more than one nexthop (ECMP). This probably requires a redesign
>> of
>> + the Northbound Logical_Router_Policy database schema.
>> +
>> * CI
>>
>> * ovn-kubernetes: Only a subset of the ovn-kubernetes features is
>> currently
>> diff --git a/northd/en-learned-route-sync.c
>> b/northd/en-learned-route-sync.c
>> index 7ba43ec10a..c8e5ade0b4 100644
>> --- a/northd/en-learned-route-sync.c
>> +++ b/northd/en-learned-route-sync.c
>> @@ -192,7 +192,7 @@ parse_route_from_sbrec_route(struct hmap
>> *parsed_routes_out,
>> const char *lrp_addr_s = NULL;
>> struct ovn_port *out_port = NULL;
>> if (!find_route_outport(lr_ports, route->logical_port->logical_port,
>> - route->ip_prefix, route->nexthop,
>> + "static route", route->ip_prefix,
>> route->nexthop,
>> IN6_IS_ADDR_V4MAPPED(&prefix),
>> false,
>> &out_port, &lrp_addr_s)) {
>> diff --git a/northd/northd.c b/northd/northd.c
>> index 23a4c0784f..e83aacc57a 100644
>> --- a/northd/northd.c
>> +++ b/northd/northd.c
>> @@ -10754,29 +10754,59 @@ lrp_find_member_ip(const struct ovn_port *op,
>> const char *ip_s)
>> return find_lport_address(&op->lrp_networks, ip_s);
>> }
>>
>> -static struct ovn_port*
>> -get_outport_for_routing_policy_nexthop(struct ovn_datapath *od,
>> - const struct hmap *lr_ports,
>> - int priority, const char *nexthop)
>> +/* Returns true if the output port to be used for forwarding traffic
>> through
>> + * this policy could be determined. Stores a pointer to the output port
>> + * in 'p_output_port' and a pointer to the router IP address to be used
>> for
>> + * this policy, in 'p_lrp_addr_s'. */
>> +static bool
>> +find_policy_outport(struct ovn_datapath *od, const struct hmap *lr_ports,
>> + const struct nbrec_logical_router_policy *policy,
>> + const char *nexthop, bool is_ipv4,
>> + const char **p_lrp_addr_s, struct ovn_port
>> **p_out_port)
>> {
>> if (nexthop == NULL) {
>> return NULL;
>>
>
> nit: The signature changed to bool, it should return false.
>
>
>> }
>>
>> - /* Find the router port matching the next hop. */
>> - for (int i = 0; i < od->nbr->n_ports; i++) {
>> - struct nbrec_logical_router_port *lrp = od->nbr->ports[i];
>> + struct ovn_port *out_port = NULL;
>> + const char *lrp_addr_s = NULL;
>>
>> - struct ovn_port *out_port = ovn_port_find(lr_ports, lrp->name);
>> - if (out_port && lrp_find_member_ip(out_port, nexthop)) {
>> - return out_port;
>> - }
>> + if (policy->output_port) {
>> + if (!find_route_outport(lr_ports, policy->output_port->name,
>> + "policy", policy->match,
>> + nexthop, is_ipv4, true, &out_port,
>> + &lrp_addr_s)) {
>> + return NULL;
>>
>
> nit: Same here it should return false.
>
>
Oops, good catch, thanks!
>> + }
>> + } else {
>> + /* If output_port is not specified, find the router port matching
>> + * the next hop. */
>> + HMAP_FOR_EACH (out_port, dp_node, &od->ports) {
>> + lrp_addr_s = lrp_find_member_ip(out_port, nexthop);
>> + if (lrp_addr_s) {
>> + break;
>> + }
>> + }
>> }
>>
>> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> - VLOG_WARN_RL(&rl, "No path for routing policy priority %d on router
>> %s; "
>> - "next hop %s", priority, od->nbr->name, nexthop);
>> - return NULL;
>> + if (!out_port || !lrp_addr_s) {
>> + static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> + VLOG_WARN_RL(&rl, "Logical Router: %s, policy "
>> + "(chain: '%s', match: '%s', priority
>> %"PRId64"): "
>> + "no path for next hop %s",
>> + od->nbr->name,
>> + policy->chain ? policy->chain : "<Default>",
>> + policy->match, policy->priority, nexthop);
>> + return false;
>> + }
>> + if (p_out_port) {
>> + *p_out_port = out_port;
>> + }
>> + if (p_lrp_addr_s) {
>> + *p_lrp_addr_s = lrp_addr_s;
>> + }
>> +
>> + return true;
>> }
>>
>> static bool check_bfd_state(const struct nbrec_logical_router_policy
>> *rule,
>> @@ -10852,18 +10882,12 @@ build_routing_policy_flow(struct lflow_table
>> *lflows, struct ovn_datapath *od,
>> }
>>
>> char *nexthop = rp->valid_nexthops[0];
>> - struct ovn_port *out_port =
>> get_outport_for_routing_policy_nexthop(
>> - od, lr_ports, rule->priority, nexthop);
>> - if (!out_port) {
>> - return;
>> - }
>> + bool is_ipv4 = strchr(nexthop, '.') ? true : false;
>> + const char *lrp_addr_s = NULL;
>> + struct ovn_port *out_port = NULL;
>>
>> - const char *lrp_addr_s = lrp_find_member_ip(out_port, nexthop);
>> - if (!lrp_addr_s) {
>> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> - VLOG_WARN_RL(&rl, "lrp_addr not found for routing policy "
>> - " priority %"PRId64" nexthop %s",
>> - rule->priority, nexthop);
>> + if (!find_policy_outport(od, lr_ports, rule, nexthop, is_ipv4,
>> + &lrp_addr_s, &out_port)) {
>> return;
>> }
>>
>> @@ -10872,7 +10896,6 @@ build_routing_policy_flow(struct lflow_table
>> *lflows, struct ovn_datapath *od,
>> ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark);
>> }
>>
>> - bool is_ipv4 = strchr(nexthop, '.') ? true : false;
>> ds_put_format(&actions, "%s = %s; "
>> "%s = %s; "
>> "eth.src = %s; "
>> @@ -10952,19 +10975,12 @@ build_ecmp_routing_policy_flows(struct
>> lflow_table *lflows,
>> struct ds actions = DS_EMPTY_INITIALIZER;
>>
>> for (size_t i = 0; i < rp->n_valid_nexthops; i++) {
>> - struct ovn_port *out_port =
>> get_outport_for_routing_policy_nexthop(
>> - od, lr_ports, rule->priority, rp->valid_nexthops[i]);
>> - if (!out_port) {
>> - goto cleanup;
>> - }
>> + bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false;
>> + const char *lrp_addr_s = NULL;
>> + struct ovn_port *out_port = NULL;
>>
>> - const char *lrp_addr_s =
>> - lrp_find_member_ip(out_port, rp->valid_nexthops[i]);
>> - if (!lrp_addr_s) {
>> - static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> - VLOG_WARN_RL(&rl, "lrp_addr not found for routing policy "
>> - " priority %"PRId64" nexthop %s",
>> - rule->priority, rp->valid_nexthops[i]);
>> + if (!find_policy_outport(od, lr_ports, rule,
>> rp->valid_nexthops[i],
>> + is_ipv4, &lrp_addr_s, &out_port)) {
>> goto cleanup;
>> }
>>
>> @@ -10974,7 +10990,6 @@ build_ecmp_routing_policy_flows(struct lflow_table
>> *lflows,
>> ds_put_format(&actions, "pkt.mark = %u; ", pkt_mark);
>> }
>>
>> - bool is_ipv4 = strchr(rp->valid_nexthops[i], '.') ? true : false;
>>
>> ds_put_format(&actions, "%s = %s; "
>> "%s = %s; "
>> @@ -11571,15 +11586,16 @@ build_route_match(const struct ovn_port
>> *op_inport, uint32_t rtb_id,
>>
>> bool
>> find_route_outport(const struct hmap *lr_ports, const char *output_port,
>> - const char *ip_prefix, const char *nexthop, bool
>> is_ipv4,
>> + const char *route_type, const char *route_desc,
>> + const char *nexthop, bool is_ipv4,
>> bool force_out_port,
>> struct ovn_port **out_port, const char **lrp_addr_s)
>> {
>> *out_port = ovn_port_find(lr_ports, output_port);
>> if (!*out_port) {
>> static struct vlog_rate_limit rl = VLOG_RATE_LIMIT_INIT(5, 1);
>> - VLOG_WARN_RL(&rl, "Bad out port %s for static route %s",
>> - output_port, ip_prefix);
>> + VLOG_WARN_RL(&rl, "Bad out port %s for %s %s",
>> + output_port, route_type, route_desc);
>> return false;
>> }
>> if (nexthop[0]) {
>> @@ -11619,8 +11635,10 @@ find_static_route_outport(const struct
>> ovn_datapath *od,
>> struct ovn_port *out_port = NULL;
>> if (route->output_port) {
>> /* XXX: we should be able to use &od->ports instead of lr_ports.
>> */
>> - if (!find_route_outport(lr_ports, route->output_port,
>> route->ip_prefix,
>> - route->nexthop, is_ipv4, true, &out_port, &lrp_addr_s)) {
>> + if (!find_route_outport(lr_ports, route->output_port,
>> + "static route", route->ip_prefix,
>> + route->nexthop, is_ipv4, true, &out_port,
>> + &lrp_addr_s)) {
>> return false;
>> }
>> } else {
>> @@ -14217,16 +14235,33 @@ build_route_policies(struct ovn_datapath *od,
>> const struct hmap *lr_ports,
>> if (!strcmp(rule->action, "reroute")) {
>> size_t n_nexthops = rule->n_nexthops ? rule->n_nexthops : 1;
>>
>> + if (rule->output_port && n_nexthops != 1) {
>> + static struct vlog_rate_limit rl =
>> VLOG_RATE_LIMIT_INIT(5, 1);
>> + VLOG_WARN_RL(&rl,
>> + "Logical router: %s, policy "
>> + "(chain: '%s', match: '%s', priority
>> %"PRId64"): "
>> + "output_port only supported on non-ECMP "
>> + "reroute policies",
>> + od->nbr->name,
>> + rule->chain ? rule->chain : "<Default>",
>> + rule->match, rule->priority);
>> + continue;
>> + }
>> +
>> valid_nexthops = xcalloc(n_nexthops, sizeof *valid_nexthops);
>> for (size_t j = 0; j < n_nexthops; j++) {
>> char *nexthop = rule->n_nexthops
>> ? rule->nexthops[j] : rule->nexthop;
>> - struct ovn_port *out_port =
>> - get_outport_for_routing_policy_nexthop(
>> - od, lr_ports, rule->priority, nexthop);
>> - if (!out_port || !check_bfd_state(rule, out_port, nexthop,
>> - bfd_connections,
>> -
>> bfd_active_connections)) {
>> + struct ovn_port *out_port = NULL;
>> + bool is_ipv4 = strchr(nexthop, '.') ? true : false;
>> +
>> + if (!find_policy_outport(od, lr_ports, rule, nexthop,
>> is_ipv4,
>> + NULL, &out_port)) {
>> + continue;
>> + }
>> + if (!check_bfd_state(rule, out_port, nexthop,
>> + bfd_connections,
>> + bfd_active_connections)) {
>> continue;
>> }
>> valid_nexthops[n_valid_nexthops++] = nexthop;
>> diff --git a/northd/northd.h b/northd/northd.h
>> index ca35eb91e4..0348a3234f 100644
>> --- a/northd/northd.h
>> +++ b/northd/northd.h
>> @@ -855,7 +855,8 @@ struct svc_monitors_map_data {
>>
>> bool
>> find_route_outport(const struct hmap *lr_ports, const char *output_port,
>> - const char *ip_prefix, const char *nexthop, bool
>> is_ipv4,
>> + const char *route_type, const char *route_desc,
>> + const char *nexthop, bool is_ipv4,
>> bool force_out_port,
>> struct ovn_port **out_port, const char **lrp_addr_s);
>>
>> diff --git a/ovn-nb.ovsschema b/ovn-nb.ovsschema
>> index f55930a2e5..cfb1f5f7dc 100644
>> --- a/ovn-nb.ovsschema
>> +++ b/ovn-nb.ovsschema
>> @@ -1,7 +1,7 @@
>> {
>> "name": "OVN_Northbound",
>> - "version": "7.12.0",
>> - "cksum": "2749576410 39903",
>> + "version": "7.13.0",
>> + "cksum": "296941325 40198",
>> "tables": {
>> "NB_Global": {
>> "columns": {
>> @@ -555,6 +555,12 @@
>> "nexthop": {"type": {"key": "string", "min": 0, "max":
>> 1}},
>> "nexthops": {"type": {
>> "key": "string", "min": 0, "max": "unlimited"}},
>> + "output_port": {
>> + "type": {"key": {"type": "uuid",
>> + "refTable": "Logical_Router_Port",
>> + "refType": "weak"},
>> + "min": 0,
>> + "max": 1}},
>> "bfd_sessions": {"type": {"key": {"type": "uuid",
>> "refTable": "BFD",
>> "refType": "weak"},
>> diff --git a/ovn-nb.xml b/ovn-nb.xml
>> index 1f5c58490c..c83ec1b7f4 100644
>> --- a/ovn-nb.xml
>> +++ b/ovn-nb.xml
>> @@ -4725,6 +4725,23 @@ or
>> </p>
>> </column>
>>
>> + <column name="output_port">
>> + <p>
>> + The <ref table="Logical_Router_Port"/> via which the packet
>> + needs to be sent out. This is optional and when not specified,
>> + OVN will automatically figure this out based on the
>> + <ref column="nexthops"/> column. When this is specified and
>> there are
>> + multiple IP addresses on the router port and none of them are in
>> the
>> + same subnet of <ref column="nexthops"/>, OVN chooses the first IP
>> + address as the one via which the <ref column="nexthops"/> are
>> + reachable.
>> +
>> + NOTE: for now OVN does not support configuring the output port on
>> + ECMP reroute policies (with more than one value in the
>> + <ref column="nexthops"/> column).
>> + </p>
>> + </column>
>> +
>> <column name="bfd_sessions">
>> <p>
>> Reference to <ref table="BFD"/> row if the route policy has
>> associated
>> diff --git a/tests/ovn-nbctl.at b/tests/ovn-nbctl.at
>> index fe4a984d07..c6edd0f17b 100644
>> --- a/tests/ovn-nbctl.at
>> +++ b/tests/ovn-nbctl.at
>> @@ -2512,6 +2512,21 @@ AT_CHECK([ovn-nbctl lr-policy-add lr0 100 "ip4.src
>> == 2.1.1.0/24" jump], [1], []
>> [ovn-nbctl: Chain name is required when action is jump.
>> ])
>>
>> +AT_CHECK([ovn-nbctl --output-port=lrp0 lr-policy-add lr0 101 "ip4.src ==
>> 3.1.1.0/24" drop], [1], [],
>> + [ovn-nbctl: Specifying output-port is only allowed for reroute policies.
>> +])
>> +
>> +AT_CHECK([ovn-nbctl --output-port=lrp0 lr-policy-add lr0 101 "ip4.src ==
>> 3.1.1.0/24" reroute 192.168.0.10,192.168.0.11], [1], [],
>> + [ovn-nbctl: output-port option is supported only for policies with a
>> single reroute next hop
>> +])
>> +
>> +AT_CHECK([ovn-nbctl --output-port=lrp0 lr-policy-add lr0 101 "ip4.src ==
>> 3.1.1.0/24" reroute 192.168.0.10], [1], [],
>> + [ovn-nbctl: unknown output-port: lrp0
>> +])
>> +
>> +check ovn-nbctl lrp-add lr0 lrp0 00:00:00:00:00:01 192.168.0.1/24
>> +check ovn-nbctl --output-port=lrp0 lr-policy-add lr0 101 "ip4.src ==
>> 3.1.1.0/24" reroute 192.168.0.10
>> +
>> dnl Add duplicated policy
>> AT_CHECK([ovn-nbctl lr-policy-add lr0 100 "ip4.src == 1.1.1.0/24" drop],
>> [1], [],
>> [ovn-nbctl: Same routing policy already existed on the logical router
>> lr0.
>> @@ -2532,6 +2547,7 @@ Routing Policies
>> Chain <Default>:
>> 101 ip4.src == 2.1.1.0/24
>> allow
>> 101 ip4.src == 2.1.2.0/24
>> drop
>> + 101 ip4.src == 3.1.1.0/24
>> reroute 192.168.0.10 lrp0
>> 101 ip6.src == 2002::/64
>> drop
>> 100 ip4.src == 1.1.2.0/24
>> allow pkt_mark=100,foo=bar
>>
>> @@ -2549,6 +2565,7 @@ Routing Policies
>> Chain <Default>:
>> 101 ip4.src == 2.1.1.0/24
>> allow
>> 101 ip4.src == 2.1.2.0/24
>> drop
>> + 101 ip4.src == 3.1.1.0/24
>> reroute 192.168.0.10 lrp0
>> 101 ip6.src == 2002::/64
>> drop
>> 100 ip4.src == 1.1.2.0/24
>> allow pkt_mark=100,foo=bar
>>
>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>> index 6298222247..83a142d20f 100644
>> --- a/tests/ovn-northd.at
>> +++ b/tests/ovn-northd.at
>> @@ -3643,6 +3643,39 @@ AT_CHECK([grep "lr_in_policy[[^_]]" lr0flows2 |
>> ovn_strip_lflows | sort], [0], [
>> AT_CLEANUP
>> ])
>>
>> +OVN_FOR_EACH_NORTHD_NO_HV([
>> +AT_SETUP([Router policies - output_port])
>> +ovn_start
>> +
>> +dnl Test with a simple non-ECMP reroute policy with an explicit
>> output-port set.
>> +check ovn-nbctl \
>> + -- lr-add lr0 \
>> + -- lrp-add lr0 lrp1 00:00:00:00:00:01 1.1.1.1/24
>> \
>> + -- lrp-add lr0 lrp2 00:00:00:00:00:02 2.2.2.1/24
>> \
>> + -- --output-port=lrp2 lr-policy-add lr0 100 "ip4.src == 42.42.42.42" \
>> + reroute 3.3.3.3
>> +
>> +dnl Also add an ECMP reroute policy and forcefully set output-port;
>> ovn-northd
>> +dnl should warn and ignore this second policy.
>> +check ovn-nbctl lr-policy-add lr0 200 "ip4.src == 43.43.43.43" reroute
>> 3.3.3.4,3.3.3.5
>> +lrpol=$(fetch_column nb:logical_router_policy _uuid priority=200)
>> +lrp2=$(fetch_column nb:logical_router_port _uuid name=lrp2)
>> +check ovn-nbctl set logical_router_policy $lrpol output_port=$lrp2
>> +check ovn-nbctl --wait=sb sync
>> +
>> +check grep -qE "output_port only supported on non-ECMP reroute policies"
>> northd/ovn-northd.log
>> +
>> +ovn-sbctl dump-flows lr0 > lr0flows
>> +AT_CAPTURE_FILE([lr0flows])
>> +
>> +AT_CHECK([grep "lr_in_policy[[^_]]" lr0flows | ovn_strip_lflows | sort],
>> [0], [dnl
>> + table=??(lr_in_policy ), priority=0 , match=(1),
>> action=(reg8[[0..15]] = 0; next;)
>> + table=??(lr_in_policy ), priority=100 , match=(ip4.src ==
>> 42.42.42.42), action=(reg0 = 3.3.3.3; reg5 = 2.2.2.1; eth.src =
>> 00:00:00:00:00:02; outport = "lrp2"; flags.loopback = 1; reg8[[0..15]] = 0;
>> reg9[[9]] = 1; next;)
>> +])
>> +
>> +AT_CLEANUP
>> +])
>> +
>> OVN_FOR_EACH_NORTHD_NO_HV([
>> AT_SETUP([ACL allow-stateless omit conntrack - Logical_Switch])
>> ovn_start
>> @@ -12739,6 +12772,18 @@ check_engine_stats sync_to_sb_lb recompute
>> nocompute
>> check_engine_stats lflow recompute nocompute
>> CHECK_NO_CHANGE_AFTER_RECOMPUTE
>>
>> +# Change router Policy to use explicit output port.
>> +lrp_lr0_sw0=$(fetch_column nb:logical_router_port _uuid name=lr0-sw0)
>> +check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>> +check ovn-nbctl --wait=sb set logical_router_policy .
>> output_port=$lrp_lr0_sw0
>> +check_engine_stats northd recompute nocompute
>> +check_engine_stats lr_nat recompute nocompute
>> +check_engine_stats lr_stateful recompute nocompute
>> +check_engine_stats sync_to_sb_pb recompute nocompute
>> +check_engine_stats sync_to_sb_lb recompute nocompute
>> +check_engine_stats lflow recompute nocompute
>> +CHECK_NO_CHANGE_AFTER_RECOMPUTE
>> +
>> check as northd ovn-appctl -t ovn-northd inc-engine/clear-stats
>> check ovn-nbctl --wait=sb lr-policy-del lr0 10 "ip4.src == 10.0.0.3"
>> check_engine_stats northd recompute nocompute
>> diff --git a/tests/system-ovn.at b/tests/system-ovn.at
>> index b2319b1801..e5d97bf152 100644
>> --- a/tests/system-ovn.at
>> +++ b/tests/system-ovn.at
>> @@ -18485,3 +18485,101 @@ OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query
>> port patch-.*/d
>> /connection dropped.*/d"])
>> AT_CLEANUP
>> ])
>> +
>> +OVN_FOR_EACH_NORTHD([
>> +AT_SETUP([Router reroute policies - output port])
>> +AT_SKIP_IF([test $HAVE_NC = no])
>> +
>> +ovn_start
>> +OVS_TRAFFIC_VSWITCHD_START()
>> +ADD_BR([br-int])
>> +
>> +check ovs-vsctl \
>> + -- set Open_vSwitch . external-ids:system-id=hv1 \
>> + -- set Open_vSwitch .
>> external-ids:ovn-remote=unix:$ovs_base/ovn-sb/ovn-sb.sock \
>> + -- set Open_vSwitch . external-ids:ovn-encap-type=geneve \
>> + -- set Open_vSwitch . external-ids:ovn-encap-ip=169.0.0.1 \
>> + -- set bridge br-int fail-mode=secure
>> other-config:disable-in-band=true
>> +start_daemon ovn-controller
>> +
>> +dnl Logical network:
>> +dnl +--- LR2 --- LS2 --- VM2 (20.0.0.1)
>> +dnl |
>> +dnl vm1 -- LS1 --- LR1 --- +
>> +dnl +--- LR3 --- LS3 --- VM3
>> +dnl
>> +dnl - Link-Local only connections between LR1 and LR2 and between LR1 and
>> LR3.
>> +dnl - LR1 with a default route via LR2's LLA
>> +dnl - LR1 with a routing policy to reroute traffic from vm1 via LR3's LLA
>> +dnl and, to avoid ambiguity, with output-port set to be the port
>> towards LR3.
>> +dnl
>> +check ovn-nbctl \
>> + -- lr-add lr1 \
>> + -- lrp-add lr1 lr1-ls1 00:00:00:00:00:01 10.0.0.1/24 \
>> + -- lrp-add lr1 lr1-lr2 00:00:00:00:00:02 peer=lr2-lr1 \
>> + -- lrp-add lr1 lr1-lr3 00:00:00:00:00:03 peer=lr3-lr1 \
>> + -- lr-route-add lr1 0.0.0.0/0 fe80::200:ff:fe00:200 \
>> + -- --output-port=lr1-lr3 lr-policy-add lr1 100 \
>> + "ip4.src == 10.0.0.2" \
>> + reroute fe80::200:ff:fe00:300 \
>> + -- lr-add lr2 \
>> + -- lrp-add lr2 lr2-lr1 00:00:00:00:02:00 peer=lr1-lr2 \
>> + -- lrp-add lr2 lr2-ls2 00:00:00:00:00:12 20.0.0.1/24 \
>> + -- lr-route-add lr2 10.0.0.0/24 fe80::200:ff:fe00:2 lr2-lr1 \
>> + -- lr-add lr3 \
>> + -- lrp-add lr3 lr3-lr1 00:00:00:00:03:00 peer=lr1-lr3 \
>> + -- lrp-add lr3 lr3-ls3 00:00:00:00:00:13 30.0.0.1/24 \
>> + -- lr-route-add lr3 10.0.0.0/24 fe80::200:ff:fe00:3 lr3-lr1 \
>> + -- ls-add ls1 \
>> + -- lsp-add ls1 ls1-lr1 \
>> + -- lsp-set-type ls1-lr1 router \
>> + -- lsp-set-addresses ls1-lr1 router \
>> + -- lsp-set-options ls1-lr1 router-port=lr1-ls1 \
>> + -- ls-add ls2 \
>> + -- lsp-add ls2 ls2-lr2 \
>> + -- lsp-set-type ls2-lr2 router \
>> + -- lsp-set-addresses ls2-lr2 router \
>> + -- lsp-set-options ls2-lr2 router-port=lr2-ls2 \
>> + -- ls-add ls3 \
>> + -- lsp-add ls3 ls3-lr3 \
>> + -- lsp-set-type ls3-lr3 router \
>> + -- lsp-set-addresses ls3-lr3 router \
>> + -- lsp-set-options ls3-lr3 router-port=lr3-ls3 \
>> + -- lsp-add ls1 vm1 \
>> + -- lsp-set-addresses vm1 "00:00:00:00:01:00 10.0.0.2" \
>> + -- lsp-add ls2 vm2 \
>> + -- lsp-set-addresses vm2 "00:00:00:00:02:00 20.0.0.2" \
>> + -- lsp-add ls3 vm3 \
>> + -- lsp-set-addresses vm3 "00:00:00:00:03:00 30.0.0.2"
>> +
>> +ADD_NAMESPACES(vm1)
>> +ADD_VETH(vm1, vm1, br-int, "10.0.0.2/24", "00:00:00:00:01:00",
>> "10.0.0.1")
>> +ADD_NAMESPACES(vm2)
>> +ADD_VETH(vm2, vm2, br-int, "20.0.0.2/24", "00:00:00:00:02:00",
>> "20.0.0.1")
>> +ADD_NAMESPACES(vm3)
>> +ADD_VETH(vm3, vm3, br-int, "30.0.0.2/24", "00:00:00:00:03:00",
>> "30.0.0.1")
>> +
>> +OVN_POPULATE_ARP
>> +check ovn-nbctl --wait=hv sync
>> +wait_for_ports_up
>> +
>> +NETNS_DAEMONIZE([vm3], [nc -l -k 80], [vm3.pid])
>> +NS_CHECK_EXEC([vm1], [nc 30.0.0.2 80 -z])
>> +
>> +OVN_CLEANUP_CONTROLLER([hv1])
>> +
>> +as ovn-sb
>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>> +
>> +as ovn-nb
>> +OVS_APP_EXIT_AND_WAIT([ovsdb-server])
>> +
>> +as northd
>> +OVS_APP_EXIT_AND_WAIT([ovn-northd])
>> +
>> +as
>> +OVS_TRAFFIC_VSWITCHD_STOP(["/failed to query port patch-.*/d
>> +/Failed to acquire.*/d
>> +/connection dropped.*/d"])
>> +AT_CLEANUP
>> +])
>> diff --git a/utilities/ovn-nbctl.c b/utilities/ovn-nbctl.c
>> index 58517f9668..9d58094e98 100644
>> --- a/utilities/ovn-nbctl.c
>> +++ b/utilities/ovn-nbctl.c
>> @@ -4189,6 +4189,9 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
>> const struct nbrec_bfd **bfd_sessions = NULL;
>> char *chain_s = shash_find_data(&ctx->options, "--chain");
>>
>> + const char *output_port = shash_find_data(&ctx->options,
>> "--output-port");
>> + const struct nbrec_logical_router_port *out_lrp = NULL;
>> +
>> bool reroute = false;
>> bool jump = false;
>>
>> @@ -4205,6 +4208,10 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
>> return;
>> }
>> reroute = true;
>> + } else if (output_port) {
>> + ctl_error(ctx, "Specifying output-port is only allowed for "
>> + "reroute policies.");
>> + return;
>> }
>>
>> if (!strcmp(action, "jump")) {
>> @@ -4264,6 +4271,21 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
>> }
>>
>> free(nexthops_arg);
>> +
>> + if (output_port) {
>> + if (vector_len(&nexthops) != 1) {
>> + ctl_error(ctx, "output-port option is supported only for "
>> + "policies with a single reroute next hop");
>> + goto free_nexthops;
>> + }
>> +
>> + error = lrp_by_name_or_uuid(ctx, output_port, true, &out_lrp);
>> + if (error) {
>> + ctl_error(ctx, "unknown output-port: %s", output_port);
>> + free(error);
>> + goto free_nexthops;
>> + }
>> + }
>> }
>>
>> struct nbrec_logical_router_policy *policy;
>> @@ -4283,6 +4305,7 @@ nbctl_lr_policy_add(struct ctl_context *ctx)
>> if (reroute) {
>> nbrec_logical_router_policy_set_nexthops(
>> policy, vector_get_array(&nexthops), vector_len(&nexthops));
>> + nbrec_logical_router_policy_set_output_port(policy, out_lrp);
>> }
>>
>> /* Parse the options. */
>> @@ -4505,6 +4528,10 @@ print_routing_policy(const struct
>> nbrec_logical_router_policy *policy,
>> free(next_hop);
>> }
>>
>> + if (policy->output_port) {
>> + ds_put_format(s, " %s", policy->output_port->name);
>> + }
>> +
>> if (!smap_is_empty(&policy->options) || policy->n_bfd_sessions) {
>> ds_put_format(s, "%15s", "");
>> if (policy->n_bfd_sessions) {
>> @@ -4526,10 +4553,14 @@ nbctl_pre_lr_policy_list(struct ctl_context *ctx)
>> ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_col_name);
>> ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_col_policies);
>>
>> + ovsdb_idl_add_column(ctx->idl, &nbrec_logical_router_port_col_name);
>> +
>> ovsdb_idl_add_column(ctx->idl,
>> &nbrec_logical_router_policy_col_priority);
>> ovsdb_idl_add_column(ctx->idl,
>> &nbrec_logical_router_policy_col_match);
>> ovsdb_idl_add_column(ctx->idl,
>> &nbrec_logical_router_policy_col_nexthops);
>> ovsdb_idl_add_column(ctx->idl,
>> &nbrec_logical_router_policy_col_action);
>> + ovsdb_idl_add_column(ctx->idl,
>> + &nbrec_logical_router_policy_col_output_port);
>> ovsdb_idl_add_column(ctx->idl,
>> &nbrec_logical_router_policy_col_options);
>> ovsdb_idl_add_column(ctx->idl,
>> &nbrec_logical_router_policy_col_bfd_sessions);
>> @@ -8418,7 +8449,7 @@ static const struct ctl_command_syntax
>> nbctl_commands[] = {
>> { "lr-policy-add", 4, INT_MAX,
>> "ROUTER PRIORITY MATCH ACTION [NEXTHOP] [OPTIONS - KEY=VALUE ...]",
>> nbctl_pre_lr_policy_add, nbctl_lr_policy_add, NULL,
>> - "--may-exist,--bfd?,--chain=", RW },
>> + "--may-exist,--bfd?,--chain=,--output-port=", RW },
>> { "lr-policy-del", 1, 3, "ROUTER [{PRIORITY | UUID} [MATCH]]",
>> nbctl_pre_lr_policy_del, nbctl_lr_policy_del, NULL,
>> "--if-exists,--chain=", RW },
>> --
>> 2.51.0
>>
>> _______________________________________________
>> dev mailing list
>> [email protected]
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
> With that addressed:
> Acked-by: Ales Musil <[email protected]>
>
I fixed up the two returns and applied this to main.
Regards,
Dumitru
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
