Hi Martin,
Thanks for the RFC!
On 4/3/25 2:43 PM, Martin Morgenstern via dev wrote:
> Add LR option "fallback_to_routing_policy" that, when set to true,
> changes router behavior to always evaluate routing policies, even if
> there was no match in the routing table (otherwise it would simply drop
> the packet).
>
> If the option is set, we adjust the lflows in such a way that the
> decision to drop the packet is delayed until we reach the lr_in_policy
> pipeline.
>
> The change takes advantage of the fact that the IPv4 and IPv6 src ip
> and nexthop registers share bits. Thus, at least at the moment, we
> don't need to use another register to save the lookup result (match or
> no match) from the lr_in_ip_routing pipeline.
>
> Having this option is useful for routers that do not have a default
> route and one still wants to make use of the more advanced matching
> capabilities of routing policies.
>
I didn't look at the code yet but I was thinking of the use case you
mentioned above. Aren't the two parts of the phrase above a bit
contradicting eachother?
I mean sure, there's no default route but you'd like a default action,
i.e., apply routing policies. Isn't that essentially achievable by
adding a default route with any next-hop IP and a default, lowest
priority, policy that drops everything not matched by policies?
Or am I simplifying the use case too much?
I'm asking because I'd like to avoid new knobs if possible.
Thanks,
Dumitru
> Signed-off-by: Martin Morgenstern <martin.morgenst...@cloudandheat.com>
> ---
> NEWS | 3 +++
> northd/northd.c | 51 +++++++++++++++++++++++++++++++++++++++++--------
> ovn-nb.xml | 10 ++++++++++
> 3 files changed, 56 insertions(+), 8 deletions(-)
>
> diff --git a/NEWS b/NEWS
> index 656176d20..bcbc8fb65 100644
> --- a/NEWS
> +++ b/NEWS
> @@ -4,6 +4,9 @@ Post v25.03.0
> external-ids, this option allows to specify if ovn-controller should
> perform cleanup when exiting. The "--restart" exit always has priority
> to keep the backward compatibility.
> + - Added a new logical router option "fallback_to_routing_policy" that
> + changes router behavior to always evaluate routing policies even if
> + there was no match in the routing table, instead of dropping the packet.
>
> OVN v25.03.0 - 07 Mar 2025
> --------------------------
> diff --git a/northd/northd.c b/northd/northd.c
> index 471d9969b..41e3f973f 100644
> --- a/northd/northd.c
> +++ b/northd/northd.c
> @@ -275,7 +275,7 @@ BUILD_ASSERT_DECL(ACL_OBS_STAGE_MAX < (1 << 2));
> * | | | | | E |
> NEXT_HOP_IPV6 (>= IN_IP_ROUTING) |
> * +-----+---------------------------+---+-----------------+ G |
> |
> * | R2 | REG_DHCP_RELAY_DIP_IPV4 | | | 0 |
> |
> - * | | REG_LB_PORT | X | | 0 |
> |
> + * | | REG_LB_PORT | X | | |
> |
> * | | (>= IN_LB_AFF_CHECK | R | | |
> |
> * | | <= IN_LB_AFF_LEARN) | E | | |
> |
> * +-----+---------------------------+ G | UNUSED | |
> |
> @@ -283,11 +283,11 @@ BUILD_ASSERT_DECL(ACL_OBS_STAGE_MAX < (1 << 2));
> * | | | | | |
> |
> *
> +-----+---------------------------+---+-----------------+---+------------------------------------+
> * | R4 | REG_LB_IPV4 | X | | |
> |
> - * | | (>= IN_LB_AFF_CHECK && | R | | |
> |
> - * | | <= IN_LB_AFF_LEARN) | R | | |
> |
> - * +-----+---------------------------+ E | UNUSED | X |
> |
> - * | R5 | SRC_IPV4 for ARP-REQ | G | | X |
> REG_LB_IPV6 |
> - * | | (>= IP_INPUT) | 2 | | R | (>=
> IN_LB_AFF_CHECK && |
> + * | | (>= IN_LB_AFF_CHECK && | R | | |
> REG_SRC_IPV6 |
> + * | | <= IN_LB_AFF_LEARN) | E | | | (>=
> IN_IP_ROUTING && |
> + * +-----+---------------------------+ G | UNUSED | X | <=
> IN_POLICY_ECMP) / |
> + * | R5 | SRC_IPV4 for ARP-REQ | 2 | | X |
> REG_LB_IPV6 |
> + * | | (>= IP_INPUT) | | | R | (>=
> IN_LB_AFF_CHECK && |
> * +-----+---------------------------+---+-----------------+ E | <=
> IN_LB_AFF_LEARN) |
> * | R6 | UNUSED | X | | G |
> |
> * | | | R | | 1 |
> |
> @@ -1432,6 +1432,12 @@ lsp_disable_arp_nd_rsp(const struct
> nbrec_logical_switch_port *nbsp)
> return smap_get_bool( ->options, "disable_arp_nd_rsp", false);
> }
>
> +static bool
> +lr_fallback_to_routing_policy(const struct nbrec_logical_router *nbr)
> +{
> + return smap_get_bool(&nbr->options, "fallback_to_routing_policy", false);
> +}
> +
> static bool
> lsp_is_type_changed(const struct sbrec_port_binding *sb,
> const struct nbrec_logical_switch_port *nbsp,
> @@ -13719,8 +13725,24 @@ build_default_route_flows_for_lrouter(
> {
> ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP,
> NULL);
> - ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_IP_ROUTING,
> - NULL);
> + if (lr_fallback_to_routing_policy(od->nbr)) {
> + /* If we have no match in the routing table, delay the decision to
> drop
> + * the packet until we reach the policy table.
> + * We zero the IPv6 src ip and nexthop registers and later use them
> in
> + * the lr_in_policy pipeline for the decision whether to proceed or
> drop
> + * the packet.
> + */
> + ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 0, "1",
> + "ip.ttl--; flags.loopback = 1; "
> + REG_ECMP_GROUP_ID" = 0; "
> + REG_NEXT_HOP_IPV6" = 0; "
> + REG_SRC_IPV6" = 0; "
> + "next;",
> + NULL);
> + } else {
> + ovn_lflow_add_default_drop(lflows, od, S_ROUTER_IN_IP_ROUTING,
> + NULL);
> + }
> ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING_ECMP, 150,
> REG_ECMP_GROUP_ID" == 0", "next;",
> NULL);
> @@ -14096,6 +14118,19 @@ build_ingress_policy_flows_for_lrouter(
> struct lflow_ref *lflow_ref)
> {
> ovs_assert(od->nbr);
> + if (lr_fallback_to_routing_policy(od->nbr)) {
> + /* Drop the packet if there was neither a matching rule in the
> routing
> + * table nor a match in the policy table that set the IPv4/IPv6 src
> ip
> + * and nexthop registers to a non-zero value. (This takes advantage
> of
> + * the fact that the IPv6 registers are non-zero if IPv4 registers
> are
> + * non-zero, i.e., that the IPv4 register bits are a subset of the
> IPv6
> + * register bits.)
> + */
> + ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY, 1,
> + REG_NEXT_HOP_IPV6" == 0 && "
> + REG_SRC_IPV6" == 0",
> + "drop;", lflow_ref);
> + }
> /* This is a catch-all rule. It has the lowest priority (0)
> * does a match-all("1") and pass-through (next) */
> ovn_lflow_add(lflows, od, S_ROUTER_IN_POLICY, 0, "1",
> diff --git a/ovn-nb.xml b/ovn-nb.xml
> index ff5f2f249..ef9f799bc 100644
> --- a/ovn-nb.xml
> +++ b/ovn-nb.xml
> @@ -3151,6 +3151,16 @@ or
> </p>
> </column>
>
> + <column name="options" key="fallback_to_routing_policy"
> + type='{"type": "boolean"}'>
> + <p>
> + If set to <code>true</code>, the routing behavior of the logical
> + router will be changed to always evaluate routing policies instead
> + of dropping the packet, even if there was no routing rule match in
> + the routing table. Defaults to <code>false</code>.
> + </p>
> + </column>
> +
> <column name="options" key="ct-commit-all" type='{"type": "boolean"}'>
> When enabled the LR will commit traffic in a zone that is stateful.
> The traffic is not commited to both zones but it is selective based
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
