With this patch routes to directly-connected networks
have higher priority than static routes with same ip_prefix.

This brings commonly-used behaviour for routes lookup order:
1: longest prefix match
2: metric

The metric has next lookup order:
1: directly connected routes
2: static routes

Earlier static and connected routes with same ip_prefix had
the same priority, so it was impossible to predict which one
is used for routing decision.

Now each route's prefix length has its own 'slot' in lflow prios.
Prefix length space is calculated using next information:
route's perfixlen multiplied by 'slots' count (currently 3)
plus route origin offset (0 - source-based route; 1 - directly-
connected route; 2 - static route).

Also, enlarge prio for generic records in lr_in_ip_routing stage
by 10000.

Signed-off-by: Vladislav Odintsov <odiv...@gmail.com>
---
 northd/northd.c     | 42 +++++++++++++++++++++++++++---------------
 tests/ovn-northd.at |  8 ++++----
 2 files changed, 31 insertions(+), 19 deletions(-)

diff --git a/northd/northd.c b/northd/northd.c
index 1e8a3457c..960c5f6e0 100644
--- a/northd/northd.c
+++ b/northd/northd.c
@@ -305,6 +305,15 @@ enum ovn_stage {
  *
  */
 
+/*
+ * Route offsets implement logic to prioritize traffic for routes with
+ * same ip_prefix values:
+ *  -  directly-connected route overrides static one;
+ *  -  static route overrides directly-connected route. */
+#define ROUTE_PRIO_OFFSET_MULTIPLIER 3
+#define ROUTE_PRIO_OFFSET_STATIC 1
+#define ROUTE_PRIO_OFFSET_CONNECTED 2
+
 /* Returns an "enum ovn_stage" built from the arguments. */
 static enum ovn_stage
 ovn_stage_build(enum ovn_datapath_type dp_type, enum ovn_pipeline pipeline,
@@ -8919,19 +8928,20 @@ build_route_prefix_s(const struct in6_addr *prefix, 
unsigned int plen)
 static void
 build_route_match(const struct ovn_port *op_inport, const char *network_s,
                   int plen, bool is_src_route, bool is_ipv4, struct ds *match,
-                  uint16_t *priority)
+                  uint16_t *priority, int ofs)
 {
     const char *dir;
     /* The priority here is calculated to implement longest-prefix-match
      * routing. */
     if (is_src_route) {
         dir = "src";
-        *priority = plen * 2;
+        ofs = 0;
     } else {
         dir = "dst";
-        *priority = (plen * 2) + 1;
     }
 
+    *priority = (plen * ROUTE_PRIO_OFFSET_MULTIPLIER) + ofs;
+
     if (op_inport) {
         ds_put_format(match, "inport == %s && ", op_inport->json_key);
     }
@@ -9073,7 +9083,7 @@ add_ecmp_symmetric_reply_flows(struct hmap *lflows,
                   out_port->lrp_networks.ea_s,
                   IN6_IS_ADDR_V4MAPPED(&route->prefix) ? "" : "xx",
                   port_ip, out_port->json_key);
-    ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_ROUTING, 300,
+    ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_ROUTING, 10300,
                            ds_cstr(&match), ds_cstr(&actions),
                            &st_route->header_);
 
@@ -9104,7 +9114,7 @@ build_ecmp_route_flow(struct hmap *lflows, struct 
ovn_datapath *od,
 
     char *prefix_s = build_route_prefix_s(&eg->prefix, eg->plen);
     build_route_match(NULL, prefix_s, eg->plen, eg->is_src_route, is_ipv4,
-                      &route_match, &priority);
+                      &route_match, &priority, ROUTE_PRIO_OFFSET_STATIC);
     free(prefix_s);
 
     struct ds actions = DS_EMPTY_INITIALIZER;
@@ -9180,7 +9190,7 @@ 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_discard_route, int ofs)
 {
     bool is_ipv4 = strchr(network_s, '.') ? true : false;
     struct ds match = DS_EMPTY_INITIALIZER;
@@ -9196,7 +9206,7 @@ add_route(struct hmap *lflows, struct ovn_datapath *od,
         }
     }
     build_route_match(op_inport, network_s, plen, is_src_route, is_ipv4,
-                      &match, &priority);
+                      &match, &priority, ofs);
 
     struct ds common_actions = DS_EMPTY_INITIALIZER;
     struct ds actions = DS_EMPTY_INITIALIZER;
@@ -9259,7 +9269,8 @@ build_static_route_flow(struct hmap *lflows, struct 
ovn_datapath *od,
     char *prefix_s = build_route_prefix_s(&route_->prefix, route_->plen);
     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);
+              route_->is_src_route, &route->header_, route_->is_discard_route,
+              ROUTE_PRIO_OFFSET_STATIC);
 
     free(prefix_s);
 }
@@ -10672,14 +10683,14 @@ build_ip_routing_flows_for_lrouter_port(
             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_, false);
+                      &op->nbrp->header_, false, ROUTE_PRIO_OFFSET_CONNECTED);
         }
 
         for (int i = 0; i < op->lrp_networks.n_ipv6_addrs; i++) {
             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_, false);
+                      &op->nbrp->header_, false, ROUTE_PRIO_OFFSET_CONNECTED);
         }
     } else if (lsp_is_router(op->nbsp)) {
         struct ovn_port *peer = ovn_port_get_peer(ports, op);
@@ -10702,7 +10713,8 @@ build_ip_routing_flows_for_lrouter_port(
                               peer->lrp_networks.ipv4_addrs[0].addr_s,
                               laddrs->ipv4_addrs[k].network_s,
                               laddrs->ipv4_addrs[k].plen, NULL, false,
-                              &peer->nbrp->header_, false);
+                              &peer->nbrp->header_, false,
+                              ROUTE_PRIO_OFFSET_CONNECTED);
                 }
             }
         }
@@ -10773,7 +10785,7 @@ build_mcast_lookup_flows_for_lrouter(
         /* Drop IPv6 multicast traffic that shouldn't be forwarded,
          * i.e., router solicitation and router advertisement.
          */
-        ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 550,
+        ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10550,
                       "nd_rs || nd_ra", "drop;");
         if (!od->mcast_info.rtr.relay) {
             return;
@@ -10801,7 +10813,7 @@ build_mcast_lookup_flows_for_lrouter(
             }
             ds_put_format(actions, "outport = \"%s\"; ip.ttl--; next;",
                           igmp_group->mcgroup.name);
-            ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 500,
+            ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10500,
                           ds_cstr(match), ds_cstr(actions));
         }
 
@@ -10809,7 +10821,7 @@ build_mcast_lookup_flows_for_lrouter(
          * ports. Otherwise drop any multicast traffic.
          */
         if (od->mcast_info.rtr.flood_static) {
-            ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 450,
+            ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10450,
                           "ip4.mcast || ip6.mcast",
                           "clone { "
                                 "outport = \""MC_STATIC"\"; "
@@ -10817,7 +10829,7 @@ build_mcast_lookup_flows_for_lrouter(
                                 "next; "
                           "};");
         } else {
-            ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 450,
+            ovn_lflow_add(lflows, od, S_ROUTER_IN_IP_ROUTING, 10450,
                           "ip4.mcast || ip6.mcast", "drop;");
         }
     }
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 85b47a18f..3c1a97f73 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -5430,7 +5430,7 @@ check ovn-nbctl --wait=sb --ecmp-symmetric-reply 
lr-route-add lr0 1.0.0.1 192.16
 
 ovn-sbctl dump-flows lr0 > lr0flows
 AT_CHECK([grep -e "lr_in_ip_routing.*select" lr0flows | sed 
's/table=../table=??/' | sort], [0], [dnl
-  table=??(lr_in_ip_routing   ), priority=65   , match=(ip4.dst == 
1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1; 
reg8[[16..31]] = select(1, 2);)
+  table=??(lr_in_ip_routing   ), priority=97   , match=(ip4.dst == 
1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1; 
reg8[[16..31]] = select(1, 2);)
 ])
 AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed 
's/192\.168\.0\..0/192.168.0.??/' | sed 's/table=../table=??/' | sort], [0], 
[dnl
   table=??(lr_in_ip_routing_ecmp), priority=100  , match=(reg8[[0..15]] == 1 
&& reg8[[16..31]] == 1), action=(reg0 = 192.168.0.??; reg1 = 192.168.0.1; 
eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;)
@@ -5443,7 +5443,7 @@ check ovn-nbctl --wait=sb --ecmp-symmetric-reply 
lr-route-add lr0 1.0.0.1 192.16
 
 ovn-sbctl dump-flows lr0 > lr0flows
 AT_CHECK([grep -e "lr_in_ip_routing.*select" lr0flows | sed 
's/table=../table=??/' | sort], [0], [dnl
-  table=??(lr_in_ip_routing   ), priority=65   , match=(ip4.dst == 
1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1; 
reg8[[16..31]] = select(1, 2);)
+  table=??(lr_in_ip_routing   ), priority=97   , match=(ip4.dst == 
1.0.0.1/32), action=(ip.ttl--; flags.loopback = 1; reg8[[0..15]] = 1; 
reg8[[16..31]] = select(1, 2);)
 ])
 AT_CHECK([grep -e "lr_in_ip_routing_ecmp" lr0flows | sed 
's/192\.168\.0\..0/192.168.0.??/' | sed 's/table=../table=??/' | sort], [0], 
[dnl
   table=??(lr_in_ip_routing_ecmp), priority=100  , match=(reg8[[0..15]] == 1 
&& reg8[[16..31]] == 1), action=(reg0 = 192.168.0.??; reg1 = 192.168.0.1; 
eth.src = 00:00:20:20:12:13; outport = "lr0-public"; next;)
@@ -5458,14 +5458,14 @@ check ovn-nbctl --wait=sb lr-route-add lr0 1.0.0.0/24 
192.168.0.10
 ovn-sbctl dump-flows lr0 > lr0flows
 
 AT_CHECK([grep -e "lr_in_ip_routing.*192.168.0.10" lr0flows | sed 
's/table=../table=??/' | sort], [0], [dnl
-  table=??(lr_in_ip_routing   ), priority=49   , match=(ip4.dst == 
1.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 192.168.0.10; reg1 = 
192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; 
flags.loopback = 1; next;)
+  table=??(lr_in_ip_routing   ), priority=73   , match=(ip4.dst == 
1.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = 192.168.0.10; reg1 = 
192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; 
flags.loopback = 1; next;)
 ])
 
 check ovn-nbctl --wait=sb lr-route-add lr0 2.0.0.0/24 lr0-public
 
 ovn-sbctl dump-flows lr0 > lr0flows
 AT_CHECK([grep -e "lr_in_ip_routing.*2.0.0.0" lr0flows | sed 
's/table=../table=??/' | sort], [0], [dnl
-  table=??(lr_in_ip_routing   ), priority=49   , match=(ip4.dst == 
2.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 = 
192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; 
flags.loopback = 1; next;)
+  table=??(lr_in_ip_routing   ), priority=73   , match=(ip4.dst == 
2.0.0.0/24), action=(ip.ttl--; reg8[[0..15]] = 0; reg0 = ip4.dst; reg1 = 
192.168.0.1; eth.src = 00:00:20:20:12:13; outport = "lr0-public"; 
flags.loopback = 1; next;)
 ])
 
 AT_CLEANUP
-- 
2.30.0

_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to