It looks good to me, thanks Lorenzo.

Acked-by: Mark Michelson <[email protected]>

On 9/9/21 8:50 AM, Lorenzo Bianconi wrote:
As it has been already done for IPv4, collapse IPv6 Neighbour
Advertisment flows for load balancer using address list and
reduce the number of logical flows from N*M to N where N is
the number of logical router port and M is the number of
Virtual IPs.

https://bugzilla.redhat.com/show_bug.cgi?id=1970258
Signed-off-by: Lorenzo Bianconi <[email protected]>
---
Changes since v2:
- remove open code and run build_lrouter_nd_flow() instead

Changes since v1:
- rebase on top of ovn master
- add DDlog support
---
  northd/ovn-northd.c  | 25 ++++++++++-------
  northd/ovn_northd.dl | 66 ++++++++++++++++++++++++++++++++------------
  tests/ovn-northd.at  | 36 ++++++++----------------
  3 files changed, 75 insertions(+), 52 deletions(-)

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index ee761cef0..fc623bcbe 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -9851,8 +9851,7 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct 
ovn_port *op,
          ds_put_format(&actions,
                        "%s { "
                          "eth.src = %s; "
-                        "ip6.src = %s; "
-                        "nd.target = %s; "
+                        "ip6.src = nd.target; "
                          "nd.tll = %s; "
                          "outport = inport; "
                          "flags.loopback = 1; "
@@ -9860,8 +9859,6 @@ build_lrouter_nd_flow(struct ovn_datapath *od, struct 
ovn_port *op,
                        "};",
                        action,
                        eth_addr,
-                      ip_address,
-                      ip_address,
                        eth_addr);
          ovn_lflow_add_with_hint__(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
                                    ds_cstr(&match), ds_cstr(&actions), NULL,
@@ -11899,7 +11896,6 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
                                     &op->nbrp->header_, lflows);
          }
- const char *ip_address;
          if (sset_count(&op->od->lb_ips_v4)) {
              ds_clear(match);
              if (is_l3dgw_port(op)) {
@@ -11920,17 +11916,26 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
              ds_destroy(&load_balancer_ips_v4);
          }
- SSET_FOR_EACH (ip_address, &op->od->lb_ips_v6) {
+        if (sset_count(&op->od->lb_ips_v6)) {
              ds_clear(match);
+            ds_clear(actions);
+
+            struct ds load_balancer_ips_v6 = DS_EMPTY_INITIALIZER;
+
+            ds_put_cstr(&load_balancer_ips_v6, "{ ");
+            ds_put_and_free_cstr(&load_balancer_ips_v6,
+                                 sset_join(&op->od->lb_ips_v6, ", ", " }"));
+
              if (is_l3dgw_port(op)) {
                  ds_put_format(match, "is_chassis_resident(%s)",
                                op->cr_port->json_key);
              }
-
              build_lrouter_nd_flow(op->od, op, "nd_na",
-                                  ip_address, NULL, REG_INPORT_ETH_ADDR,
-                                  match, false, 90, NULL,
-                                  lflows, meter_groups);
+                                  ds_cstr(&load_balancer_ips_v6), NULL,
+                                  REG_INPORT_ETH_ADDR, match, false, 90,
+                                  NULL, lflows, meter_groups);
+
+            ds_destroy(&load_balancer_ips_v6);
          }
if (!op->od->is_gw_router && !op->od->n_l3dgw_ports) {
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index ff92c989c..d91f8111f 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -5537,6 +5537,44 @@ LogicalRouterNdFlow(router, lrp, "nd_na", ipv6, true, 
mac, extra_match, drop, pr
      LogicalRouterArpNdFlow(router, nat@NAT{.external_ip = IPv6{ipv6}}, lrp,
                             mac, extra_match, drop, priority).
+relation LogicalRouterNdFlowLB(
+    lr: Intern<Router>,
+    lrp: Option<Intern<nb::Logical_Router_Port>>,
+    ip: string,
+    mac: string,
+    extra_match: Option<string>,
+    stage_hint: bit<32>)
+Flow(.logical_datapath = lr._uuid,
+     .stage = s_ROUTER_IN_IP_INPUT(),
+     .priority = 90,
+     .__match = __match.intern(),
+     .actions = actions,
+     .stage_hint = stage_hint,
+     .io_port = None,
+     .controller_meter = lr.copp.get(cOPP_ND_NA())) :-
+    LogicalRouterNdFlowLB(.lr = lr, .lrp = lrp, .ip = ip,
+                          .mac = mac, .extra_match = extra_match,
+                          .stage_hint = stage_hint),
+    var __match = {
+        var clauses = vec_with_capacity(4);
+        match (lrp) {
+            Some{p} -> clauses.push("inport == ${json_string_escape(p.name)}"),
+            None -> ()
+        };
+        clauses.push("nd_ns && nd.target == ${ip}");
+        clauses.append(extra_match.to_vec());
+        clauses.join(" && ")
+    },
+    var actions =
+        i"nd_na { "
+           "eth.src = ${mac}; "
+           "ip6.src = nd.target; "
+           "nd.tll = ${mac}; "
+           "outport = inport; "
+           "flags.loopback = 1; "
+           "output; "
+         "};".
+
  relation LogicalRouterArpFlow(
      lr: Intern<Router>,
      lrp: Option<Intern<nb::Logical_Router_Port>>,
@@ -5622,8 +5660,7 @@ Flow(.logical_datapath = lr._uuid,
      } else {
          (i"${action} { "
             "eth.src = ${mac}; "
-           "ip6.src = ${ip}; "
-           "nd.target = ${ip}; "
+           "ip6.src = nd.target; "
             "nd.tll = ${mac}; "
             "outport = inport; "
             "flags.loopback = 1; "
@@ -5693,7 +5730,7 @@ var residence_check = match (is_redirect) {
      true -> 
Some{"is_chassis_resident(${json_string_escape(chassis_redirect_name(lrp.name))})"},
      false -> None
  } in {
-    (var all_ips_v4, _) = get_router_load_balancer_ips(router, false) in {
+    (var all_ips_v4, var all_ips_v6) = get_router_load_balancer_ips(router, 
false) in {
          if (not all_ips_v4.is_empty()) {
              LogicalRouterArpFlow(.lr = router,
                                   .lrp = Some{lrp},
@@ -5703,21 +5740,14 @@ var residence_check = match (is_redirect) {
                                   .drop = false,
                                   .priority = 90,
                                   .stage_hint = 0)
-        }
-    };
-    for (RouterLBVIP(.router = &Router{._uuid= lr_uuid}, .vip = vip)) {
-        Some{(var ip_address, _)} = ip_address_and_port_from_lb_key(vip) in {
-            IPv6{var ipv6} = ip_address in
-            LogicalRouterNdFlow(.lr = router,
-                                .lrp = Some{lrp},
-                                .action = "nd_na",
-                                .ip = ipv6,
-                                .sn_ip = false,
-                                .mac = rEG_INPORT_ETH_ADDR(),
-                                .extra_match = residence_check,
-                                .drop = false,
-                                .priority = 90,
-                                .stage_hint = 0)
+        };
+        if (not all_ips_v6.is_empty()) {
+            LogicalRouterNdFlowLB(.lr = router,
+                                  .lrp = Some{lrp},
+                                  .ip = "{ " ++ all_ips_v6.join(", ") ++ " }",
+                                  .mac = rEG_INPORT_ETH_ADDR(),
+                                  .extra_match = residence_check,
+                                  .stage_hint = 0)
          }
      }
  }
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 11886b94e..8200eb655 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1707,13 +1707,10 @@ match=(inport == "lrp" && arp.op == 1 && arp.tpa == { 
192.168.2.1, 192.168.2.4,
  action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ 
arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = 
inport; flags.loopback = 1; output;)
    table=3 (lr_in_ip_input     ), priority=90   , dnl
  match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns 
&& nd.target == fe80::200:ff:fe00:1), dnl
-action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:1; nd.target = fe80::200:ff:fe00:1; nd.tll = xreg0[[0..47]]; 
outport = inport; flags.loopback = 1; output; };)
+action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
    table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && nd_ns && nd.target == fe80::200:ff:fe00:101:8080), 
dnl
-action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:101:8080; nd.target = fe80::200:ff:fe00:101:8080; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && nd_ns && nd.target == fe80::200:ff:fe00:102:8080), 
dnl
-action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:102:8080; nd.target = fe80::200:ff:fe00:102:8080; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
+match=(inport == "lrp" && nd_ns && nd.target == { fe80::200:ff:fe00:101:8080, 
fe80::200:ff:fe00:102:8080 }), dnl
+action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
    table=3 (lr_in_ip_input     ), priority=90   , dnl
  match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && 
arp.spa == 43.43.43.0/24), dnl
  action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ 
arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = 
inport; flags.loopback = 1; output;)
@@ -1722,13 +1719,10 @@ match=(inport == "lrp-public" && arp.op == 1 && arp.tpa 
== { 192.168.2.1, 192.16
  action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ 
arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = 
inport; flags.loopback = 1; output;)
    table=3 (lr_in_ip_input     ), priority=90   , dnl
  match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} 
&& nd_ns && nd.target == fe80::200:ff:fe00:100), dnl
-action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:100; nd.target = fe80::200:ff:fe00:100; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && nd_ns && nd.target == 
fe80::200:ff:fe00:101:8080), dnl
-action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:101:8080; nd.target = fe80::200:ff:fe00:101:8080; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
+action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
    table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && nd_ns && nd.target == 
fe80::200:ff:fe00:102:8080), dnl
-action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:102:8080; nd.target = fe80::200:ff:fe00:102:8080; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
+match=(inport == "lrp-public" && nd_ns && nd.target == { 
fe80::200:ff:fe00:101:8080, fe80::200:ff:fe00:102:8080 }), dnl
+action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
  ])
# xreg0[0..47] isn't used anywhere else.
@@ -1782,13 +1776,10 @@ match=(inport == "lrp" && arp.op == 1 && arp.tpa == { 
192.168.2.1, 192.168.2.4,
  action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ 
arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = 
inport; flags.loopback = 1; output;)
    table=3 (lr_in_ip_input     ), priority=90   , dnl
  match=(inport == "lrp" && ip6.dst == {fe80::200:ff:fe00:1, ff02::1:ff00:1} && nd_ns 
&& nd.target == fe80::200:ff:fe00:1), dnl
-action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:1; nd.target = fe80::200:ff:fe00:1; nd.tll = xreg0[[0..47]]; 
outport = inport; flags.loopback = 1; output; };)
+action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
    table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && nd_ns && nd.target == fe80::200:ff:fe00:101:8080), 
dnl
-action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:101:8080; nd.target = fe80::200:ff:fe00:101:8080; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp" && nd_ns && nd.target == fe80::200:ff:fe00:102:8080), 
dnl
-action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:102:8080; nd.target = fe80::200:ff:fe00:102:8080; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
+match=(inport == "lrp" && nd_ns && nd.target == { fe80::200:ff:fe00:101:8080, 
fe80::200:ff:fe00:102:8080 }), dnl
+action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
    table=3 (lr_in_ip_input     ), priority=90   , dnl
  match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.1 && 
arp.spa == 43.43.43.0/24), dnl
  action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ 
arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = 
inport; flags.loopback = 1; output;)
@@ -1797,13 +1788,10 @@ match=(inport == "lrp-public" && arp.op == 1 && arp.tpa 
== { 192.168.2.1, 192.16
  action=(eth.dst = eth.src; eth.src = xreg0[[0..47]]; arp.op = 2; /* ARP reply */ 
arp.tha = arp.sha; arp.sha = xreg0[[0..47]]; arp.tpa <-> arp.spa; outport = 
inport; flags.loopback = 1; output;)
    table=3 (lr_in_ip_input     ), priority=90   , dnl
  match=(inport == "lrp-public" && ip6.dst == {fe80::200:ff:fe00:100, ff02::1:ff00:100} && nd_ns 
&& nd.target == fe80::200:ff:fe00:100 && is_chassis_resident("cr-lrp-public")), dnl
-action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:100; nd.target = fe80::200:ff:fe00:100; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
-  table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && nd_ns && nd.target == fe80::200:ff:fe00:101:8080 && 
is_chassis_resident("cr-lrp-public")), dnl
-action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:101:8080; nd.target = fe80::200:ff:fe00:101:8080; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
+action=(nd_na_router { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
    table=3 (lr_in_ip_input     ), priority=90   , dnl
-match=(inport == "lrp-public" && nd_ns && nd.target == fe80::200:ff:fe00:102:8080 && 
is_chassis_resident("cr-lrp-public")), dnl
-action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = 
fe80::200:ff:fe00:102:8080; nd.target = fe80::200:ff:fe00:102:8080; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
+match=(inport == "lrp-public" && nd_ns && nd.target == { fe80::200:ff:fe00:101:8080, 
fe80::200:ff:fe00:102:8080 } && is_chassis_resident("cr-lrp-public")), dnl
+action=(nd_na { eth.src = xreg0[[0..47]]; ip6.src = nd.target; nd.tll = 
xreg0[[0..47]]; outport = inport; flags.loopback = 1; output; };)
  ])
# Priority 91 drop flows (per distributed gw port), if port is not resident.


_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to