diff mbox series

[ovs-dev] northd: Combine router arp flows.

Message ID 20210507162256.3661118-1-i.maximets@ovn.org
State Accepted
Headers show
Series [ovs-dev] northd: Combine router arp flows. | expand

Commit Message

Ilya Maximets May 7, 2021, 4:22 p.m. UTC
This flow already matches on 'arp.tpa == ip_address', so instead of
setting 'arp.spa' to this 'ip_address' we can just swap values of
'arp.tpa' and 'arp.spa'.

This elimintes the variable from the 'action', allowing us to just
use the list of all IPs in the match.

Current OVN generates one ARP flow for each load balancer IP for
each router port.  So, if we have 100 ports and 1000 IP addresses,
northd will generate 10^5 ARP flows.  With this change it will
generate only 100 of them.

In case of ovn-k8s scale test, this change alone reduced the database
size from 500 to 200 MB.

Also added more test cses for loadbalancer flows.

Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
---

Similar change could be applied to ipv6 ND flows, but it's harder to
implement (at least in ddlog), so it's not implemented for now.

Alternatively, it's possible to create an address set with all these
addtesses and use it in the flow, but I'm not sure if it's needed.

 northd/ovn-northd.c  | 20 +++++++----
 northd/ovn_northd.dl | 23 +++++++------
 tests/ovn-northd.at  | 81 +++++++++++++++++++++++++++++++++++---------
 tests/ovn.at         |  2 +-
 4 files changed, 91 insertions(+), 35 deletions(-)

Comments

Numan Siddique May 12, 2021, 12:14 p.m. UTC | #1
On Fri, May 7, 2021 at 12:23 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> This flow already matches on 'arp.tpa == ip_address', so instead of
> setting 'arp.spa' to this 'ip_address' we can just swap values of
> 'arp.tpa' and 'arp.spa'.
>
> This elimintes the variable from the 'action', allowing us to just
> use the list of all IPs in the match.
>
> Current OVN generates one ARP flow for each load balancer IP for
> each router port.  So, if we have 100 ports and 1000 IP addresses,
> northd will generate 10^5 ARP flows.  With this change it will
> generate only 100 of them.
>
> In case of ovn-k8s scale test, this change alone reduced the database
> size from 500 to 200 MB.
>
> Also added more test cses for loadbalancer flows.
>
> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>

Thanks Ilya for the improvements.

I applied this patch to the main branch with the below changes to
ovn-northd.8.xml

--------
diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index b63b69b5d0..bbc28d7ab3 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -2352,8 +2352,7 @@ eth.src = xreg0[0..47];
 arp.op = 2; /* ARP reply. */
 arp.tha = arp.sha;
 arp.sha = xreg0[0..47];
-arp.tpa = arp.spa;
-arp.spa = <var>A</var>;
+arp.tpa <-> arp.spa;
 outport = inport;
 flags.loopback = 1;
 output;
---------

Thanks
Numan

> ---
>
> Similar change could be applied to ipv6 ND flows, but it's harder to
> implement (at least in ddlog), so it's not implemented for now.
>
> Alternatively, it's possible to create an address set with all these
> addtesses and use it in the flow, but I'm not sure if it's needed.
>
>  northd/ovn-northd.c  | 20 +++++++----
>  northd/ovn_northd.dl | 23 +++++++------
>  tests/ovn-northd.at  | 81 +++++++++++++++++++++++++++++++++++---------
>  tests/ovn.at         |  2 +-
>  4 files changed, 91 insertions(+), 35 deletions(-)
>
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index 1d4c5b67b..de8052f21 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -9016,14 +9016,12 @@ build_lrouter_arp_flow(struct ovn_datapath *od, struct ovn_port *op,
>                        "arp.op = 2; /* ARP reply */ "
>                        "arp.tha = arp.sha; "
>                        "arp.sha = %s; "
> -                      "arp.tpa = arp.spa; "
> -                      "arp.spa = %s; "
> +                      "arp.tpa <-> arp.spa; "
>                        "outport = inport; "
>                        "flags.loopback = 1; "
>                        "output;",
>                        eth_addr,
> -                      eth_addr,
> -                      ip_address);
> +                      eth_addr);
>      }
>
>      ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
> @@ -10978,16 +10976,24 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
>          get_router_load_balancer_ips(op->od, &all_ips_v4, &all_ips_v6);
>
>          const char *ip_address;
> -        SSET_FOR_EACH (ip_address, &all_ips_v4) {
> +        if (sset_count(&all_ips_v4)) {
>              ds_clear(match);
>              if (op == op->od->l3dgw_port) {
>                  ds_put_format(match, "is_chassis_resident(%s)",
>                                op->od->l3redirect_port->json_key);
>              }
>
> -            build_lrouter_arp_flow(op->od, op,
> -                                   ip_address, REG_INPORT_ETH_ADDR,
> +            struct ds load_balancer_ips_v4 = DS_EMPTY_INITIALIZER;
> +
> +            /* For IPv4 we can just create one rule with all required IPs. */
> +            ds_put_cstr(&load_balancer_ips_v4, "{ ");
> +            ds_put_and_free_cstr(&load_balancer_ips_v4,
> +                                 sset_join(&all_ips_v4, ", ", " }"));
> +
> +            build_lrouter_arp_flow(op->od, op, ds_cstr(&load_balancer_ips_v4),
> +                                   REG_INPORT_ETH_ADDR,
>                                     match, false, 90, NULL, lflows);
> +            ds_destroy(&load_balancer_ips_v4);
>          }
>
>          SSET_FOR_EACH (ip_address, &all_ips_v6) {
> diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> index ffd09c35f..a63da8e66 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -4874,7 +4874,7 @@ relation LogicalRouterArpNdFlow(
>      extra_match: Option<string>,
>      drop: bool,
>      priority: integer)
> -LogicalRouterArpFlow(router, lrp, ipv4, mac, extra_match, drop, priority,
> +LogicalRouterArpFlow(router, lrp, "${ipv4}", mac, extra_match, drop, priority,
>                       stage_hint(nat.nat._uuid)) :-
>      LogicalRouterArpNdFlow(router, nat@NAT{.external_ip = IPv4{ipv4}}, lrp,
>                             mac, extra_match, drop, priority).
> @@ -4886,7 +4886,7 @@ LogicalRouterNdFlow(router, lrp, "nd_na", ipv6, true, mac, extra_match, drop, pr
>  relation LogicalRouterArpFlow(
>      lr: Ref<Router>,
>      lrp: Option<nb::Logical_Router_Port>,
> -    ip: in_addr,
> +    ip: string,
>      mac: string,
>      extra_match: Option<string>,
>      drop: bool,
> @@ -4919,8 +4919,7 @@ Flow(.logical_datapath = lr.lr._uuid,
>          "arp.op = 2; /* ARP reply */ "
>          "arp.tha = arp.sha; "
>          "arp.sha = ${mac}; "
> -        "arp.tpa = arp.spa; "
> -        "arp.spa = ${ip}; "
> +        "arp.tpa <-> arp.spa; "
>          "outport = inport; "
>          "flags.loopback = 1; "
>          "output;"
> @@ -5007,7 +5006,7 @@ for (RouterPortNetworksIPv4Addr(.port = &RouterPort{.lrp = lrp,
>              } else "" in
>          LogicalRouterArpFlow(.lr = router,
>                               .lrp = Some{lrp},
> -                             .ip = addr.addr,
> +                             .ip = "${addr.addr}",
>                               .mac = rEG_INPORT_ETH_ADDR(),
>                               .extra_match = Some{__match},
>                               .drop = false,
> @@ -5025,18 +5024,20 @@ var residence_check = match (is_redirect) {
>      true -> Some{"is_chassis_resident(${router.redirect_port_name})"},
>      false -> None
>  } in {
> -    for (RouterLBVIP(.router = &Router{.lr = nb::Logical_Router{._uuid= lr._uuid}}, .vip = vip)) {
> -        Some{(var ip_address, _)} = ip_address_and_port_from_lb_key(vip) in {
> -            IPv4{var ipv4} = ip_address in
> +    (var all_ips_v4, _) = get_router_load_balancer_ips(deref(router)) in {
> +        if (not all_ips_v4.is_empty()) {
>              LogicalRouterArpFlow(.lr = router,
>                                   .lrp = Some{lrp},
> -                                 .ip = ipv4,
> +                                 .ip = "{ " ++ all_ips_v4.join(", ") ++ " }",
>                                   .mac = rEG_INPORT_ETH_ADDR(),
>                                   .extra_match = residence_check,
>                                   .drop = false,
>                                   .priority = 90,
> -                                 .external_ids = map_empty());
> -
> +                                 .external_ids = map_empty())
> +        }
> +    };
> +    for (RouterLBVIP(.router = &Router{.lr = nb::Logical_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},
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 10b204624..9a8409f0c 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -1654,6 +1654,19 @@ ovn-nbctl lr-nat-add lr dnat_and_snat 43.43.43.4 42.42.42.4 ls-vm 00:00:00:00:00
>  ovn-nbctl lr-nat-add lr snat 43.43.43.150 43.43.43.50
>  ovn-nbctl lr-nat-add lr snat 43.43.43.150 43.43.43.51
>
> +ovn-nbctl lb-add lb1 "192.168.2.1:8080" "10.0.0.4:8080"
> +ovn-nbctl lb-add lb2 "192.168.2.4:8080" "10.0.0.5:8080" udp
> +ovn-nbctl lb-add lb3 "192.168.2.5:8080" "10.0.0.6:8080"
> +ovn-nbctl lb-add lb4 "192.168.2.6:8080" "10.0.0.7:8080"
> +ovn-nbctl lb-add lb5 "fe80::200:ff:fe00:101:8080" "fe02::200:ff:fe00:101:8080"
> +ovn-nbctl lb-add lb5 "fe80::200:ff:fe00:102:8080" "fe02::200:ff:fe00:102:8080"
> +
> +ovn-nbctl lr-lb-add lr lb1
> +ovn-nbctl lr-lb-add lr lb2
> +ovn-nbctl lr-lb-add lr lb3
> +ovn-nbctl lr-lb-add lr lb4
> +ovn-nbctl lr-lb-add lr lb5
> +
>  ovn-nbctl --wait=sb sync
>
>  # Ingress router port ETH address is stored in lr_in_admission.
> @@ -1676,28 +1689,46 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
>  AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
>    table=3 (lr_in_ip_input     ), priority=90   , dnl
>  match=(arp.op == 1 && arp.tpa == 43.43.43.150), 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; arp.spa = 43.43.43.150; outport = inport; flags.loopback = 1; output;)
> +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=(arp.op == 1 && arp.tpa == 43.43.43.2), 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; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
> +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=(arp.op == 1 && arp.tpa == 43.43.43.3), 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; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
> +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=(arp.op == 1 && arp.tpa == 43.43.43.4), 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; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
> +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" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.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; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
> +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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 }), 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;)
>    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; };)
>    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; };)
> +  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; arp.spa = 43.43.43.1; outport = inport; flags.loopback = 1; output;)
> +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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 }), 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;)
>    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; };)
> +  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; };)
>  ])
>
>  # xreg0[0..47] isn't used anywhere else.
> @@ -1733,28 +1764,46 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
>  AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
>    table=3 (lr_in_ip_input     ), priority=90   , dnl
>  match=(arp.op == 1 && arp.tpa == 43.43.43.150), 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; arp.spa = 43.43.43.150; outport = inport; flags.loopback = 1; output;)
> +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=(arp.op == 1 && arp.tpa == 43.43.43.2), 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; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
> +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=(arp.op == 1 && arp.tpa == 43.43.43.3), 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; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
> +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=(arp.op == 1 && arp.tpa == 43.43.43.4), 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; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
> +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" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.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; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
> +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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 }), 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;)
>    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; };)
>    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; };)
> +  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; arp.spa = 43.43.43.1; outport = inport; flags.loopback = 1; output;)
> +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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 } && is_chassis_resident("cr-lrp-public")), 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;)
>    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; };)
> +  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; };)
>  ])
>
>  # Priority 91 drop flows (per distributed gw port), if port is not resident.
> @@ -1776,16 +1825,16 @@ action=(drop;)
>  AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=92" | grep "arp\|nd" | sort], [0], [dnl
>    table=3 (lr_in_ip_input     ), priority=92   , dnl
>  match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.150 && is_chassis_resident("cr-lrp-public")), 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; arp.spa = 43.43.43.150; outport = inport; flags.loopback = 1; output;)
> +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=92   , dnl
>  match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2 && is_chassis_resident("cr-lrp-public")), 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; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
> +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=92   , dnl
>  match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3 && is_chassis_resident("cr-lrp-public")), 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; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
> +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=92   , dnl
>  match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.4 && is_chassis_resident("ls-vm")), dnl
> -action=(eth.dst = eth.src; eth.src = 00:00:00:00:00:02; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 00:00:00:00:00:02; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
> +action=(eth.dst = eth.src; eth.src = 00:00:00:00:00:02; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 00:00:00:00:00:02; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
>  ])
>
>  # xreg0[0..47] isn't used anywhere else.
> diff --git a/tests/ovn.at b/tests/ovn.at
> index b3e6ab45d..92048d740 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -17181,7 +17181,7 @@ check_virtual_offlows_present() {
>
>      AT_CHECK([as $hv ovs-ofctl dump-flows br-int table=11 | ofctl_strip_all | \
>      grep "priority=92" | grep 172.168.0.50], [0], [dnl
> - table=11, priority=92,arp,reg14=0x3,metadata=0x3,arp_tpa=172.168.0.50,arp_op=1 actions=move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],mod_dl_src:10:54:00:00:00:10,load:0x2->NXM_OF_ARP_OP[[]],move:NXM_NX_ARP_SHA[[]]->NXM_NX_ARP_THA[[]],load:0x105400000010->NXM_NX_ARP_SHA[[]],move:NXM_OF_ARP_SPA[[]]->NXM_OF_ARP_TPA[[]],load:0xaca80032->NXM_OF_ARP_SPA[[]],move:NXM_NX_REG14[[]]->NXM_NX_REG15[[]],load:0x1->NXM_NX_REG10[[0]],resubmit(,37)
> + table=11, priority=92,arp,reg14=0x3,metadata=0x3,arp_tpa=172.168.0.50,arp_op=1 actions=move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],mod_dl_src:10:54:00:00:00:10,load:0x2->NXM_OF_ARP_OP[[]],move:NXM_NX_ARP_SHA[[]]->NXM_NX_ARP_THA[[]],load:0x105400000010->NXM_NX_ARP_SHA[[]],push:NXM_OF_ARP_SPA[[]],push:NXM_OF_ARP_TPA[[]],pop:NXM_OF_ARP_SPA[[]],pop:NXM_OF_ARP_TPA[[]],move:NXM_NX_REG14[[]]->NXM_NX_REG15[[]],load:0x1->NXM_NX_REG10[[0]],resubmit(,37)
>  ])
>  }
>
> --
> 2.26.3
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
Numan Siddique May 12, 2021, 12:29 p.m. UTC | #2
On Wed, May 12, 2021 at 8:14 AM Numan Siddique <numans@ovn.org> wrote:
>
> On Fri, May 7, 2021 at 12:23 PM Ilya Maximets <i.maximets@ovn.org> wrote:
> >
> > This flow already matches on 'arp.tpa == ip_address', so instead of
> > setting 'arp.spa' to this 'ip_address' we can just swap values of
> > 'arp.tpa' and 'arp.spa'.
> >
> > This elimintes the variable from the 'action', allowing us to just
> > use the list of all IPs in the match.
> >
> > Current OVN generates one ARP flow for each load balancer IP for
> > each router port.  So, if we have 100 ports and 1000 IP addresses,
> > northd will generate 10^5 ARP flows.  With this change it will
> > generate only 100 of them.
> >
> > In case of ovn-k8s scale test, this change alone reduced the database
> > size from 500 to 200 MB.
> >
> > Also added more test cses for loadbalancer flows.
> >
> > Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>
> Thanks Ilya for the improvements.
>
> I applied this patch to the main branch with the below changes to
> ovn-northd.8.xml
>
> --------
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index b63b69b5d0..bbc28d7ab3 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -2352,8 +2352,7 @@ eth.src = xreg0[0..47];
>  arp.op = 2; /* ARP reply. */
>  arp.tha = arp.sha;
>  arp.sha = xreg0[0..47];
> -arp.tpa = arp.spa;
> -arp.spa = <var>A</var>;
> +arp.tpa <-> arp.spa;

Oops. Sorry Ilya for goofing up here.
It should have been
arp.tpa &lt;-&gt; arp.spa;

I fixed this in a separate commit.

Thanks
Numan

>  outport = inport;
>  flags.loopback = 1;
>  output;
> ---------
>
> Thanks
> Numan
>
> > ---
> >
> > Similar change could be applied to ipv6 ND flows, but it's harder to
> > implement (at least in ddlog), so it's not implemented for now.
> >
> > Alternatively, it's possible to create an address set with all these
> > addtesses and use it in the flow, but I'm not sure if it's needed.
> >
> >  northd/ovn-northd.c  | 20 +++++++----
> >  northd/ovn_northd.dl | 23 +++++++------
> >  tests/ovn-northd.at  | 81 +++++++++++++++++++++++++++++++++++---------
> >  tests/ovn.at         |  2 +-
> >  4 files changed, 91 insertions(+), 35 deletions(-)
> >
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index 1d4c5b67b..de8052f21 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -9016,14 +9016,12 @@ build_lrouter_arp_flow(struct ovn_datapath *od, struct ovn_port *op,
> >                        "arp.op = 2; /* ARP reply */ "
> >                        "arp.tha = arp.sha; "
> >                        "arp.sha = %s; "
> > -                      "arp.tpa = arp.spa; "
> > -                      "arp.spa = %s; "
> > +                      "arp.tpa <-> arp.spa; "
> >                        "outport = inport; "
> >                        "flags.loopback = 1; "
> >                        "output;",
> >                        eth_addr,
> > -                      eth_addr,
> > -                      ip_address);
> > +                      eth_addr);
> >      }
> >
> >      ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
> > @@ -10978,16 +10976,24 @@ build_lrouter_ipv4_ip_input(struct ovn_port *op,
> >          get_router_load_balancer_ips(op->od, &all_ips_v4, &all_ips_v6);
> >
> >          const char *ip_address;
> > -        SSET_FOR_EACH (ip_address, &all_ips_v4) {
> > +        if (sset_count(&all_ips_v4)) {
> >              ds_clear(match);
> >              if (op == op->od->l3dgw_port) {
> >                  ds_put_format(match, "is_chassis_resident(%s)",
> >                                op->od->l3redirect_port->json_key);
> >              }
> >
> > -            build_lrouter_arp_flow(op->od, op,
> > -                                   ip_address, REG_INPORT_ETH_ADDR,
> > +            struct ds load_balancer_ips_v4 = DS_EMPTY_INITIALIZER;
> > +
> > +            /* For IPv4 we can just create one rule with all required IPs. */
> > +            ds_put_cstr(&load_balancer_ips_v4, "{ ");
> > +            ds_put_and_free_cstr(&load_balancer_ips_v4,
> > +                                 sset_join(&all_ips_v4, ", ", " }"));
> > +
> > +            build_lrouter_arp_flow(op->od, op, ds_cstr(&load_balancer_ips_v4),
> > +                                   REG_INPORT_ETH_ADDR,
> >                                     match, false, 90, NULL, lflows);
> > +            ds_destroy(&load_balancer_ips_v4);
> >          }
> >
> >          SSET_FOR_EACH (ip_address, &all_ips_v6) {
> > diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
> > index ffd09c35f..a63da8e66 100644
> > --- a/northd/ovn_northd.dl
> > +++ b/northd/ovn_northd.dl
> > @@ -4874,7 +4874,7 @@ relation LogicalRouterArpNdFlow(
> >      extra_match: Option<string>,
> >      drop: bool,
> >      priority: integer)
> > -LogicalRouterArpFlow(router, lrp, ipv4, mac, extra_match, drop, priority,
> > +LogicalRouterArpFlow(router, lrp, "${ipv4}", mac, extra_match, drop, priority,
> >                       stage_hint(nat.nat._uuid)) :-
> >      LogicalRouterArpNdFlow(router, nat@NAT{.external_ip = IPv4{ipv4}}, lrp,
> >                             mac, extra_match, drop, priority).
> > @@ -4886,7 +4886,7 @@ LogicalRouterNdFlow(router, lrp, "nd_na", ipv6, true, mac, extra_match, drop, pr
> >  relation LogicalRouterArpFlow(
> >      lr: Ref<Router>,
> >      lrp: Option<nb::Logical_Router_Port>,
> > -    ip: in_addr,
> > +    ip: string,
> >      mac: string,
> >      extra_match: Option<string>,
> >      drop: bool,
> > @@ -4919,8 +4919,7 @@ Flow(.logical_datapath = lr.lr._uuid,
> >          "arp.op = 2; /* ARP reply */ "
> >          "arp.tha = arp.sha; "
> >          "arp.sha = ${mac}; "
> > -        "arp.tpa = arp.spa; "
> > -        "arp.spa = ${ip}; "
> > +        "arp.tpa <-> arp.spa; "
> >          "outport = inport; "
> >          "flags.loopback = 1; "
> >          "output;"
> > @@ -5007,7 +5006,7 @@ for (RouterPortNetworksIPv4Addr(.port = &RouterPort{.lrp = lrp,
> >              } else "" in
> >          LogicalRouterArpFlow(.lr = router,
> >                               .lrp = Some{lrp},
> > -                             .ip = addr.addr,
> > +                             .ip = "${addr.addr}",
> >                               .mac = rEG_INPORT_ETH_ADDR(),
> >                               .extra_match = Some{__match},
> >                               .drop = false,
> > @@ -5025,18 +5024,20 @@ var residence_check = match (is_redirect) {
> >      true -> Some{"is_chassis_resident(${router.redirect_port_name})"},
> >      false -> None
> >  } in {
> > -    for (RouterLBVIP(.router = &Router{.lr = nb::Logical_Router{._uuid= lr._uuid}}, .vip = vip)) {
> > -        Some{(var ip_address, _)} = ip_address_and_port_from_lb_key(vip) in {
> > -            IPv4{var ipv4} = ip_address in
> > +    (var all_ips_v4, _) = get_router_load_balancer_ips(deref(router)) in {
> > +        if (not all_ips_v4.is_empty()) {
> >              LogicalRouterArpFlow(.lr = router,
> >                                   .lrp = Some{lrp},
> > -                                 .ip = ipv4,
> > +                                 .ip = "{ " ++ all_ips_v4.join(", ") ++ " }",
> >                                   .mac = rEG_INPORT_ETH_ADDR(),
> >                                   .extra_match = residence_check,
> >                                   .drop = false,
> >                                   .priority = 90,
> > -                                 .external_ids = map_empty());
> > -
> > +                                 .external_ids = map_empty())
> > +        }
> > +    };
> > +    for (RouterLBVIP(.router = &Router{.lr = nb::Logical_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},
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 10b204624..9a8409f0c 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -1654,6 +1654,19 @@ ovn-nbctl lr-nat-add lr dnat_and_snat 43.43.43.4 42.42.42.4 ls-vm 00:00:00:00:00
> >  ovn-nbctl lr-nat-add lr snat 43.43.43.150 43.43.43.50
> >  ovn-nbctl lr-nat-add lr snat 43.43.43.150 43.43.43.51
> >
> > +ovn-nbctl lb-add lb1 "192.168.2.1:8080" "10.0.0.4:8080"
> > +ovn-nbctl lb-add lb2 "192.168.2.4:8080" "10.0.0.5:8080" udp
> > +ovn-nbctl lb-add lb3 "192.168.2.5:8080" "10.0.0.6:8080"
> > +ovn-nbctl lb-add lb4 "192.168.2.6:8080" "10.0.0.7:8080"
> > +ovn-nbctl lb-add lb5 "fe80::200:ff:fe00:101:8080" "fe02::200:ff:fe00:101:8080"
> > +ovn-nbctl lb-add lb5 "fe80::200:ff:fe00:102:8080" "fe02::200:ff:fe00:102:8080"
> > +
> > +ovn-nbctl lr-lb-add lr lb1
> > +ovn-nbctl lr-lb-add lr lb2
> > +ovn-nbctl lr-lb-add lr lb3
> > +ovn-nbctl lr-lb-add lr lb4
> > +ovn-nbctl lr-lb-add lr lb5
> > +
> >  ovn-nbctl --wait=sb sync
> >
> >  # Ingress router port ETH address is stored in lr_in_admission.
> > @@ -1676,28 +1689,46 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
> >  AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
> >    table=3 (lr_in_ip_input     ), priority=90   , dnl
> >  match=(arp.op == 1 && arp.tpa == 43.43.43.150), 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; arp.spa = 43.43.43.150; outport = inport; flags.loopback = 1; output;)
> > +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=(arp.op == 1 && arp.tpa == 43.43.43.2), 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; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
> > +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=(arp.op == 1 && arp.tpa == 43.43.43.3), 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; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
> > +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=(arp.op == 1 && arp.tpa == 43.43.43.4), 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; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
> > +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" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.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; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
> > +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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 }), 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;)
> >    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; };)
> >    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; };)
> > +  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; arp.spa = 43.43.43.1; outport = inport; flags.loopback = 1; output;)
> > +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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 }), 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;)
> >    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; };)
> > +  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; };)
> >  ])
> >
> >  # xreg0[0..47] isn't used anywhere else.
> > @@ -1733,28 +1764,46 @@ action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
> >  AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
> >    table=3 (lr_in_ip_input     ), priority=90   , dnl
> >  match=(arp.op == 1 && arp.tpa == 43.43.43.150), 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; arp.spa = 43.43.43.150; outport = inport; flags.loopback = 1; output;)
> > +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=(arp.op == 1 && arp.tpa == 43.43.43.2), 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; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
> > +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=(arp.op == 1 && arp.tpa == 43.43.43.3), 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; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
> > +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=(arp.op == 1 && arp.tpa == 43.43.43.4), 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; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
> > +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" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.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; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
> > +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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 }), 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;)
> >    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; };)
> >    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; };)
> > +  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; arp.spa = 43.43.43.1; outport = inport; flags.loopback = 1; output;)
> > +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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 } && is_chassis_resident("cr-lrp-public")), 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;)
> >    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; };)
> > +  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; };)
> >  ])
> >
> >  # Priority 91 drop flows (per distributed gw port), if port is not resident.
> > @@ -1776,16 +1825,16 @@ action=(drop;)
> >  AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=92" | grep "arp\|nd" | sort], [0], [dnl
> >    table=3 (lr_in_ip_input     ), priority=92   , dnl
> >  match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.150 && is_chassis_resident("cr-lrp-public")), 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; arp.spa = 43.43.43.150; outport = inport; flags.loopback = 1; output;)
> > +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=92   , dnl
> >  match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2 && is_chassis_resident("cr-lrp-public")), 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; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
> > +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=92   , dnl
> >  match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3 && is_chassis_resident("cr-lrp-public")), 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; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
> > +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=92   , dnl
> >  match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.4 && is_chassis_resident("ls-vm")), dnl
> > -action=(eth.dst = eth.src; eth.src = 00:00:00:00:00:02; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 00:00:00:00:00:02; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
> > +action=(eth.dst = eth.src; eth.src = 00:00:00:00:00:02; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 00:00:00:00:00:02; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
> >  ])
> >
> >  # xreg0[0..47] isn't used anywhere else.
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index b3e6ab45d..92048d740 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -17181,7 +17181,7 @@ check_virtual_offlows_present() {
> >
> >      AT_CHECK([as $hv ovs-ofctl dump-flows br-int table=11 | ofctl_strip_all | \
> >      grep "priority=92" | grep 172.168.0.50], [0], [dnl
> > - table=11, priority=92,arp,reg14=0x3,metadata=0x3,arp_tpa=172.168.0.50,arp_op=1 actions=move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],mod_dl_src:10:54:00:00:00:10,load:0x2->NXM_OF_ARP_OP[[]],move:NXM_NX_ARP_SHA[[]]->NXM_NX_ARP_THA[[]],load:0x105400000010->NXM_NX_ARP_SHA[[]],move:NXM_OF_ARP_SPA[[]]->NXM_OF_ARP_TPA[[]],load:0xaca80032->NXM_OF_ARP_SPA[[]],move:NXM_NX_REG14[[]]->NXM_NX_REG15[[]],load:0x1->NXM_NX_REG10[[0]],resubmit(,37)
> > + table=11, priority=92,arp,reg14=0x3,metadata=0x3,arp_tpa=172.168.0.50,arp_op=1 actions=move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],mod_dl_src:10:54:00:00:00:10,load:0x2->NXM_OF_ARP_OP[[]],move:NXM_NX_ARP_SHA[[]]->NXM_NX_ARP_THA[[]],load:0x105400000010->NXM_NX_ARP_SHA[[]],push:NXM_OF_ARP_SPA[[]],push:NXM_OF_ARP_TPA[[]],pop:NXM_OF_ARP_SPA[[]],pop:NXM_OF_ARP_TPA[[]],move:NXM_NX_REG14[[]]->NXM_NX_REG15[[]],load:0x1->NXM_NX_REG10[[0]],resubmit(,37)
> >  ])
> >  }
> >
> > --
> > 2.26.3
> >
> > _______________________________________________
> > dev mailing list
> > dev@openvswitch.org
> > https://mail.openvswitch.org/mailman/listinfo/ovs-dev
> >
Ilya Maximets May 14, 2021, 3:49 p.m. UTC | #3
On 5/12/21 2:29 PM, Numan Siddique wrote:
> On Wed, May 12, 2021 at 8:14 AM Numan Siddique <numans@ovn.org> wrote:
>>
>> On Fri, May 7, 2021 at 12:23 PM Ilya Maximets <i.maximets@ovn.org> wrote:
>>>
>>> This flow already matches on 'arp.tpa == ip_address', so instead of
>>> setting 'arp.spa' to this 'ip_address' we can just swap values of
>>> 'arp.tpa' and 'arp.spa'.
>>>
>>> This elimintes the variable from the 'action', allowing us to just
>>> use the list of all IPs in the match.
>>>
>>> Current OVN generates one ARP flow for each load balancer IP for
>>> each router port.  So, if we have 100 ports and 1000 IP addresses,
>>> northd will generate 10^5 ARP flows.  With this change it will
>>> generate only 100 of them.
>>>
>>> In case of ovn-k8s scale test, this change alone reduced the database
>>> size from 500 to 200 MB.
>>>
>>> Also added more test cses for loadbalancer flows.
>>>
>>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
>>
>> Thanks Ilya for the improvements.
>>
>> I applied this patch to the main branch with the below changes to
>> ovn-northd.8.xml
>>
>> --------
>> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
>> index b63b69b5d0..bbc28d7ab3 100644
>> --- a/northd/ovn-northd.8.xml
>> +++ b/northd/ovn-northd.8.xml
>> @@ -2352,8 +2352,7 @@ eth.src = xreg0[0..47];
>>  arp.op = 2; /* ARP reply. */
>>  arp.tha = arp.sha;
>>  arp.sha = xreg0[0..47];
>> -arp.tpa = arp.spa;
>> -arp.spa = <var>A</var>;
>> +arp.tpa <-> arp.spa;
> 
> Oops. Sorry Ilya for goofing up here.
> It should have been
> arp.tpa &lt;-&gt; arp.spa;
> 
> I fixed this in a separate commit.
> 

No problem.  Thanks for spotting the missing documentation part!

It would be great if we can backport it down to 20.12 as this
is essential change for large scale deployments.

Best regards, Ilya Maximets.
Numan Siddique May 14, 2021, 7:10 p.m. UTC | #4
On Fri, May 14, 2021 at 11:49 AM Ilya Maximets <i.maximets@ovn.org> wrote:
>
> On 5/12/21 2:29 PM, Numan Siddique wrote:
> > On Wed, May 12, 2021 at 8:14 AM Numan Siddique <numans@ovn.org> wrote:
> >>
> >> On Fri, May 7, 2021 at 12:23 PM Ilya Maximets <i.maximets@ovn.org> wrote:
> >>>
> >>> This flow already matches on 'arp.tpa == ip_address', so instead of
> >>> setting 'arp.spa' to this 'ip_address' we can just swap values of
> >>> 'arp.tpa' and 'arp.spa'.
> >>>
> >>> This elimintes the variable from the 'action', allowing us to just
> >>> use the list of all IPs in the match.
> >>>
> >>> Current OVN generates one ARP flow for each load balancer IP for
> >>> each router port.  So, if we have 100 ports and 1000 IP addresses,
> >>> northd will generate 10^5 ARP flows.  With this change it will
> >>> generate only 100 of them.
> >>>
> >>> In case of ovn-k8s scale test, this change alone reduced the database
> >>> size from 500 to 200 MB.
> >>>
> >>> Also added more test cses for loadbalancer flows.
> >>>
> >>> Signed-off-by: Ilya Maximets <i.maximets@ovn.org>
> >>
> >> Thanks Ilya for the improvements.
> >>
> >> I applied this patch to the main branch with the below changes to
> >> ovn-northd.8.xml
> >>
> >> --------
> >> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> >> index b63b69b5d0..bbc28d7ab3 100644
> >> --- a/northd/ovn-northd.8.xml
> >> +++ b/northd/ovn-northd.8.xml
> >> @@ -2352,8 +2352,7 @@ eth.src = xreg0[0..47];
> >>  arp.op = 2; /* ARP reply. */
> >>  arp.tha = arp.sha;
> >>  arp.sha = xreg0[0..47];
> >> -arp.tpa = arp.spa;
> >> -arp.spa = <var>A</var>;
> >> +arp.tpa <-> arp.spa;
> >
> > Oops. Sorry Ilya for goofing up here.
> > It should have been
> > arp.tpa &lt;-&gt; arp.spa;
> >
> > I fixed this in a separate commit.
> >
>
> No problem.  Thanks for spotting the missing documentation part!
>
> It would be great if we can backport it down to 20.12 as this
> is essential change for large scale deployments.

Done. Backported to branch-21.03 and branch-20.12

Numan

>
> Best regards, Ilya Maximets.
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index 1d4c5b67b..de8052f21 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -9016,14 +9016,12 @@  build_lrouter_arp_flow(struct ovn_datapath *od, struct ovn_port *op,
                       "arp.op = 2; /* ARP reply */ "
                       "arp.tha = arp.sha; "
                       "arp.sha = %s; "
-                      "arp.tpa = arp.spa; "
-                      "arp.spa = %s; "
+                      "arp.tpa <-> arp.spa; "
                       "outport = inport; "
                       "flags.loopback = 1; "
                       "output;",
                       eth_addr,
-                      eth_addr,
-                      ip_address);
+                      eth_addr);
     }
 
     ovn_lflow_add_with_hint(lflows, od, S_ROUTER_IN_IP_INPUT, priority,
@@ -10978,16 +10976,24 @@  build_lrouter_ipv4_ip_input(struct ovn_port *op,
         get_router_load_balancer_ips(op->od, &all_ips_v4, &all_ips_v6);
 
         const char *ip_address;
-        SSET_FOR_EACH (ip_address, &all_ips_v4) {
+        if (sset_count(&all_ips_v4)) {
             ds_clear(match);
             if (op == op->od->l3dgw_port) {
                 ds_put_format(match, "is_chassis_resident(%s)",
                               op->od->l3redirect_port->json_key);
             }
 
-            build_lrouter_arp_flow(op->od, op,
-                                   ip_address, REG_INPORT_ETH_ADDR,
+            struct ds load_balancer_ips_v4 = DS_EMPTY_INITIALIZER;
+
+            /* For IPv4 we can just create one rule with all required IPs. */
+            ds_put_cstr(&load_balancer_ips_v4, "{ ");
+            ds_put_and_free_cstr(&load_balancer_ips_v4,
+                                 sset_join(&all_ips_v4, ", ", " }"));
+
+            build_lrouter_arp_flow(op->od, op, ds_cstr(&load_balancer_ips_v4),
+                                   REG_INPORT_ETH_ADDR,
                                    match, false, 90, NULL, lflows);
+            ds_destroy(&load_balancer_ips_v4);
         }
 
         SSET_FOR_EACH (ip_address, &all_ips_v6) {
diff --git a/northd/ovn_northd.dl b/northd/ovn_northd.dl
index ffd09c35f..a63da8e66 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -4874,7 +4874,7 @@  relation LogicalRouterArpNdFlow(
     extra_match: Option<string>,
     drop: bool,
     priority: integer)
-LogicalRouterArpFlow(router, lrp, ipv4, mac, extra_match, drop, priority,
+LogicalRouterArpFlow(router, lrp, "${ipv4}", mac, extra_match, drop, priority,
                      stage_hint(nat.nat._uuid)) :-
     LogicalRouterArpNdFlow(router, nat@NAT{.external_ip = IPv4{ipv4}}, lrp,
                            mac, extra_match, drop, priority).
@@ -4886,7 +4886,7 @@  LogicalRouterNdFlow(router, lrp, "nd_na", ipv6, true, mac, extra_match, drop, pr
 relation LogicalRouterArpFlow(
     lr: Ref<Router>,
     lrp: Option<nb::Logical_Router_Port>,
-    ip: in_addr,
+    ip: string,
     mac: string,
     extra_match: Option<string>,
     drop: bool,
@@ -4919,8 +4919,7 @@  Flow(.logical_datapath = lr.lr._uuid,
         "arp.op = 2; /* ARP reply */ "
         "arp.tha = arp.sha; "
         "arp.sha = ${mac}; "
-        "arp.tpa = arp.spa; "
-        "arp.spa = ${ip}; "
+        "arp.tpa <-> arp.spa; "
         "outport = inport; "
         "flags.loopback = 1; "
         "output;"
@@ -5007,7 +5006,7 @@  for (RouterPortNetworksIPv4Addr(.port = &RouterPort{.lrp = lrp,
             } else "" in
         LogicalRouterArpFlow(.lr = router,
                              .lrp = Some{lrp},
-                             .ip = addr.addr,
+                             .ip = "${addr.addr}",
                              .mac = rEG_INPORT_ETH_ADDR(),
                              .extra_match = Some{__match},
                              .drop = false,
@@ -5025,18 +5024,20 @@  var residence_check = match (is_redirect) {
     true -> Some{"is_chassis_resident(${router.redirect_port_name})"},
     false -> None
 } in {
-    for (RouterLBVIP(.router = &Router{.lr = nb::Logical_Router{._uuid= lr._uuid}}, .vip = vip)) {
-        Some{(var ip_address, _)} = ip_address_and_port_from_lb_key(vip) in {
-            IPv4{var ipv4} = ip_address in
+    (var all_ips_v4, _) = get_router_load_balancer_ips(deref(router)) in {
+        if (not all_ips_v4.is_empty()) {
             LogicalRouterArpFlow(.lr = router,
                                  .lrp = Some{lrp},
-                                 .ip = ipv4,
+                                 .ip = "{ " ++ all_ips_v4.join(", ") ++ " }",
                                  .mac = rEG_INPORT_ETH_ADDR(),
                                  .extra_match = residence_check,
                                  .drop = false,
                                  .priority = 90,
-                                 .external_ids = map_empty());
-
+                                 .external_ids = map_empty())
+        }
+    };
+    for (RouterLBVIP(.router = &Router{.lr = nb::Logical_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},
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 10b204624..9a8409f0c 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1654,6 +1654,19 @@  ovn-nbctl lr-nat-add lr dnat_and_snat 43.43.43.4 42.42.42.4 ls-vm 00:00:00:00:00
 ovn-nbctl lr-nat-add lr snat 43.43.43.150 43.43.43.50
 ovn-nbctl lr-nat-add lr snat 43.43.43.150 43.43.43.51
 
+ovn-nbctl lb-add lb1 "192.168.2.1:8080" "10.0.0.4:8080"
+ovn-nbctl lb-add lb2 "192.168.2.4:8080" "10.0.0.5:8080" udp
+ovn-nbctl lb-add lb3 "192.168.2.5:8080" "10.0.0.6:8080"
+ovn-nbctl lb-add lb4 "192.168.2.6:8080" "10.0.0.7:8080"
+ovn-nbctl lb-add lb5 "fe80::200:ff:fe00:101:8080" "fe02::200:ff:fe00:101:8080"
+ovn-nbctl lb-add lb5 "fe80::200:ff:fe00:102:8080" "fe02::200:ff:fe00:102:8080"
+
+ovn-nbctl lr-lb-add lr lb1
+ovn-nbctl lr-lb-add lr lb2
+ovn-nbctl lr-lb-add lr lb3
+ovn-nbctl lr-lb-add lr lb4
+ovn-nbctl lr-lb-add lr lb5
+
 ovn-nbctl --wait=sb sync
 
 # Ingress router port ETH address is stored in lr_in_admission.
@@ -1676,28 +1689,46 @@  action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
 AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
   table=3 (lr_in_ip_input     ), priority=90   , dnl
 match=(arp.op == 1 && arp.tpa == 43.43.43.150), 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; arp.spa = 43.43.43.150; outport = inport; flags.loopback = 1; output;)
+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=(arp.op == 1 && arp.tpa == 43.43.43.2), 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; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
+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=(arp.op == 1 && arp.tpa == 43.43.43.3), 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; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
+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=(arp.op == 1 && arp.tpa == 43.43.43.4), 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; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
+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" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.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; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
+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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 }), 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;)
   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; };)
   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; };)
+  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; arp.spa = 43.43.43.1; outport = inport; flags.loopback = 1; output;)
+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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 }), 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;)
   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; };)
+  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; };)
 ])
 
 # xreg0[0..47] isn't used anywhere else.
@@ -1733,28 +1764,46 @@  action=(xreg0[[0..47]] = 00:00:00:00:01:00; next;)
 AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=90" | grep "arp\|nd" | sort], [0], [dnl
   table=3 (lr_in_ip_input     ), priority=90   , dnl
 match=(arp.op == 1 && arp.tpa == 43.43.43.150), 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; arp.spa = 43.43.43.150; outport = inport; flags.loopback = 1; output;)
+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=(arp.op == 1 && arp.tpa == 43.43.43.2), 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; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
+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=(arp.op == 1 && arp.tpa == 43.43.43.3), 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; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
+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=(arp.op == 1 && arp.tpa == 43.43.43.4), 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; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
+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" && arp.op == 1 && arp.tpa == 42.42.42.1 && arp.spa == 42.42.42.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; arp.spa = 42.42.42.1; outport = inport; flags.loopback = 1; output;)
+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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 }), 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;)
   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; };)
   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; };)
+  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; arp.spa = 43.43.43.1; outport = inport; flags.loopback = 1; output;)
+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" && arp.op == 1 && arp.tpa == { 192.168.2.1, 192.168.2.4, 192.168.2.5, 192.168.2.6 } && is_chassis_resident("cr-lrp-public")), 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;)
   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; };)
+  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; };)
 ])
 
 # Priority 91 drop flows (per distributed gw port), if port is not resident.
@@ -1776,16 +1825,16 @@  action=(drop;)
 AT_CHECK([ovn-sbctl lflow-list | grep -E "lr_in_ip_input.*priority=92" | grep "arp\|nd" | sort], [0], [dnl
   table=3 (lr_in_ip_input     ), priority=92   , dnl
 match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.150 && is_chassis_resident("cr-lrp-public")), 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; arp.spa = 43.43.43.150; outport = inport; flags.loopback = 1; output;)
+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=92   , dnl
 match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.2 && is_chassis_resident("cr-lrp-public")), 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; arp.spa = 43.43.43.2; outport = inport; flags.loopback = 1; output;)
+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=92   , dnl
 match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.3 && is_chassis_resident("cr-lrp-public")), 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; arp.spa = 43.43.43.3; outport = inport; flags.loopback = 1; output;)
+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=92   , dnl
 match=(inport == "lrp-public" && arp.op == 1 && arp.tpa == 43.43.43.4 && is_chassis_resident("ls-vm")), dnl
-action=(eth.dst = eth.src; eth.src = 00:00:00:00:00:02; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 00:00:00:00:00:02; arp.tpa = arp.spa; arp.spa = 43.43.43.4; outport = inport; flags.loopback = 1; output;)
+action=(eth.dst = eth.src; eth.src = 00:00:00:00:00:02; arp.op = 2; /* ARP reply */ arp.tha = arp.sha; arp.sha = 00:00:00:00:00:02; arp.tpa <-> arp.spa; outport = inport; flags.loopback = 1; output;)
 ])
 
 # xreg0[0..47] isn't used anywhere else.
diff --git a/tests/ovn.at b/tests/ovn.at
index b3e6ab45d..92048d740 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -17181,7 +17181,7 @@  check_virtual_offlows_present() {
 
     AT_CHECK([as $hv ovs-ofctl dump-flows br-int table=11 | ofctl_strip_all | \
     grep "priority=92" | grep 172.168.0.50], [0], [dnl
- table=11, priority=92,arp,reg14=0x3,metadata=0x3,arp_tpa=172.168.0.50,arp_op=1 actions=move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],mod_dl_src:10:54:00:00:00:10,load:0x2->NXM_OF_ARP_OP[[]],move:NXM_NX_ARP_SHA[[]]->NXM_NX_ARP_THA[[]],load:0x105400000010->NXM_NX_ARP_SHA[[]],move:NXM_OF_ARP_SPA[[]]->NXM_OF_ARP_TPA[[]],load:0xaca80032->NXM_OF_ARP_SPA[[]],move:NXM_NX_REG14[[]]->NXM_NX_REG15[[]],load:0x1->NXM_NX_REG10[[0]],resubmit(,37)
+ table=11, priority=92,arp,reg14=0x3,metadata=0x3,arp_tpa=172.168.0.50,arp_op=1 actions=move:NXM_OF_ETH_SRC[[]]->NXM_OF_ETH_DST[[]],mod_dl_src:10:54:00:00:00:10,load:0x2->NXM_OF_ARP_OP[[]],move:NXM_NX_ARP_SHA[[]]->NXM_NX_ARP_THA[[]],load:0x105400000010->NXM_NX_ARP_SHA[[]],push:NXM_OF_ARP_SPA[[]],push:NXM_OF_ARP_TPA[[]],pop:NXM_OF_ARP_SPA[[]],pop:NXM_OF_ARP_TPA[[]],move:NXM_NX_REG14[[]]->NXM_NX_REG15[[]],load:0x1->NXM_NX_REG10[[0]],resubmit(,37)
 ])
 }