diff mbox series

[ovs-dev,v2] northd: reduce number of nd_na lb logical flows

Message ID b5365739f9e904e62f93c9fdfa13883b4dbb7ab3.1629222526.git.lorenzo.bianconi@redhat.com
State Superseded
Headers show
Series [ovs-dev,v2] northd: reduce number of nd_na lb logical flows | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/github-robot-_ovn-kubernetes fail github build: failed

Commit Message

Lorenzo Bianconi Aug. 17, 2021, 5:51 p.m. UTC
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 <lorenzo.bianconi@redhat.com>
---
Changes since v1:
- rebase on top of ovn master
- add DDlog support
---
 northd/ovn-northd.c  | 41 +++++++++++++++++++--------
 northd/ovn_northd.dl | 66 ++++++++++++++++++++++++++++++++------------
 tests/ovn-northd.at  | 36 ++++++++----------------
 3 files changed, 90 insertions(+), 53 deletions(-)

Comments

Mark Michelson Aug. 27, 2021, 5:30 p.m. UTC | #1
Hi Lorenzo,

This mostly looks good, but I have one question below:

On 8/17/21 1:51 PM, 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 <lorenzo.bianconi@redhat.com>
> ---
> Changes since v1:
> - rebase on top of ovn master
> - add DDlog support
> ---
>   northd/ovn-northd.c  | 41 +++++++++++++++++++--------
>   northd/ovn_northd.dl | 66 ++++++++++++++++++++++++++++++++------------
>   tests/ovn-northd.at  | 36 ++++++++----------------
>   3 files changed, 90 insertions(+), 53 deletions(-)
> 
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index e80876af1..ee08281c3 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -9694,8 +9694,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; "
> @@ -9703,8 +9702,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,
> @@ -11742,7 +11739,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)) {
> @@ -11763,17 +11759,40 @@ 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, ", ", " }"));
> +
> +            ds_put_format(match, "inport == %s && nd_ns && nd.target == %s",
> +                          op->json_key, ds_cstr(&load_balancer_ips_v6));
>               if (is_l3dgw_port(op)) {
> -                ds_put_format(match, "is_chassis_resident(%s)",
> +                ds_put_format(match, " && is_chassis_resident(%s)",
>                                 op->cr_port->json_key);
>               }
> +            ds_put_format(actions,
> +                          "nd_na { "
> +                            "eth.src = %s; "
> +                            "ip6.src = nd.target; "
> +                            "nd.tll = %s; "
> +                            "outport = inport; "
> +                            "flags.loopback = 1; "
> +                            "output; "
> +                          "};",
> +                          REG_INPORT_ETH_ADDR,
> +                          REG_INPORT_ETH_ADDR);
> +            ovn_lflow_add_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
> +                                      ds_cstr(match), ds_cstr(actions), NULL,
> +                                      copp_meter_get(COPP_ND_NA,
> +                                                     op->od->nbr->copp,
> +                                                     meter_groups), NULL);

I don't understand why you replaced the call to build_lrouter_nd_flow() 
with manual creation of the match and actions. As long as you pass 
ds_cstr(&load_balancer_ips_v6) as the "ip_address" parameter, I think 
build_lrouter_nd_flow() will generate the proper flow.

>   
> -            build_lrouter_nd_flow(op->od, op, "nd_na",
> -                                  ip_address, 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 6cc5d7248..947637f2a 100644
> --- a/northd/ovn_northd.dl
> +++ b/northd/ovn_northd.dl
> @@ -5260,6 +5260,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>,
> +    external_ids: Map<string,string>)
> +MeteredFlow(.logical_datapath = lr._uuid,
> +            .stage = s_ROUTER_IN_IP_INPUT(),
> +            .priority = 90,
> +            .__match = __match,
> +            .actions = actions,
> +            .tags             = map_empty(),
> +            .controller_meter = lr.copp.get(cOPP_ND_NA()),
> +            .external_ids = external_ids) :-
> +    LogicalRouterNdFlowLB(.lr = lr, .lrp = lrp, .ip = ip,
> +                          .mac = mac, .extra_match = extra_match,
> +                          .external_ids = external_ids),
> +    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 =
> +        "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>>,
> @@ -5343,8 +5381,7 @@ MeteredFlow(.logical_datapath = lr._uuid,
>       } else {
>           ("${action} { "
>              "eth.src = ${mac}; "
> -           "ip6.src = ${ip}; "
> -           "nd.target = ${ip}; "
> +           "ip6.src = nd.target; "
>              "nd.tll = ${mac}; "
>              "outport = inport; "
>              "flags.loopback = 1; "
> @@ -5412,7 +5449,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},
> @@ -5422,21 +5459,14 @@ var residence_check = match (is_redirect) {
>                                    .drop = false,
>                                    .priority = 90,
>                                    .external_ids = map_empty())
> -        }
> -    };
> -    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,
> -                                .external_ids = map_empty())
> +        };
> +        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,
> +                                  .external_ids = map_empty())
>           }
>       }
>   }
> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> index 190f78261..b55b01f82 100644
> --- a/tests/ovn-northd.at
> +++ b/tests/ovn-northd.at
> @@ -1698,13 +1698,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;)
> @@ -1713,13 +1710,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.
> @@ -1773,13 +1767,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;)
> @@ -1788,13 +1779,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.
>
Lorenzo Bianconi Aug. 27, 2021, 6 p.m. UTC | #2
> Hi Lorenzo,
> 
> This mostly looks good, but I have one question below:
> 
> On 8/17/21 1:51 PM, 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 <lorenzo.bianconi@redhat.com>
> > ---
> > Changes since v1:
> > - rebase on top of ovn master
> > - add DDlog support
> > ---
> >   northd/ovn-northd.c  | 41 +++++++++++++++++++--------
> >   northd/ovn_northd.dl | 66 ++++++++++++++++++++++++++++++++------------
> >   tests/ovn-northd.at  | 36 ++++++++----------------
> >   3 files changed, 90 insertions(+), 53 deletions(-)
> > 
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index e80876af1..ee08281c3 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -9694,8 +9694,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; "
> > @@ -9703,8 +9702,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,
> > @@ -11742,7 +11739,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)) {
> > @@ -11763,17 +11759,40 @@ 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, ", ", " }"));
> > +
> > +            ds_put_format(match, "inport == %s && nd_ns && nd.target == %s",
> > +                          op->json_key, ds_cstr(&load_balancer_ips_v6));
> >               if (is_l3dgw_port(op)) {
> > -                ds_put_format(match, "is_chassis_resident(%s)",
> > +                ds_put_format(match, " && is_chassis_resident(%s)",
> >                                 op->cr_port->json_key);
> >               }
> > +            ds_put_format(actions,
> > +                          "nd_na { "
> > +                            "eth.src = %s; "
> > +                            "ip6.src = nd.target; "
> > +                            "nd.tll = %s; "
> > +                            "outport = inport; "
> > +                            "flags.loopback = 1; "
> > +                            "output; "
> > +                          "};",
> > +                          REG_INPORT_ETH_ADDR,
> > +                          REG_INPORT_ETH_ADDR);
> > +            ovn_lflow_add_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
> > +                                      ds_cstr(match), ds_cstr(actions), NULL,
> > +                                      copp_meter_get(COPP_ND_NA,
> > +                                                     op->od->nbr->copp,
> > +                                                     meter_groups), NULL);
> 
> I don't understand why you replaced the call to build_lrouter_nd_flow() with
> manual creation of the match and actions. As long as you pass
> ds_cstr(&load_balancer_ips_v6) as the "ip_address" parameter, I think
> build_lrouter_nd_flow() will generate the proper flow.

Hi Mark,

IIRC it is because build_lrouter_nd_flow() is run even in other places where it is
not correct to have  match = ... nd.target == {addr0, addr1} and action = ..
ip6.src = nd.target since addr1 can be a multicast address.

Regards,
Lorenzo

> 
> > -            build_lrouter_nd_flow(op->od, op, "nd_na",
> > -                                  ip_address, 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 6cc5d7248..947637f2a 100644
> > --- a/northd/ovn_northd.dl
> > +++ b/northd/ovn_northd.dl
> > @@ -5260,6 +5260,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>,
> > +    external_ids: Map<string,string>)
> > +MeteredFlow(.logical_datapath = lr._uuid,
> > +            .stage = s_ROUTER_IN_IP_INPUT(),
> > +            .priority = 90,
> > +            .__match = __match,
> > +            .actions = actions,
> > +            .tags             = map_empty(),
> > +            .controller_meter = lr.copp.get(cOPP_ND_NA()),
> > +            .external_ids = external_ids) :-
> > +    LogicalRouterNdFlowLB(.lr = lr, .lrp = lrp, .ip = ip,
> > +                          .mac = mac, .extra_match = extra_match,
> > +                          .external_ids = external_ids),
> > +    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 =
> > +        "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>>,
> > @@ -5343,8 +5381,7 @@ MeteredFlow(.logical_datapath = lr._uuid,
> >       } else {
> >           ("${action} { "
> >              "eth.src = ${mac}; "
> > -           "ip6.src = ${ip}; "
> > -           "nd.target = ${ip}; "
> > +           "ip6.src = nd.target; "
> >              "nd.tll = ${mac}; "
> >              "outport = inport; "
> >              "flags.loopback = 1; "
> > @@ -5412,7 +5449,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},
> > @@ -5422,21 +5459,14 @@ var residence_check = match (is_redirect) {
> >                                    .drop = false,
> >                                    .priority = 90,
> >                                    .external_ids = map_empty())
> > -        }
> > -    };
> > -    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,
> > -                                .external_ids = map_empty())
> > +        };
> > +        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,
> > +                                  .external_ids = map_empty())
> >           }
> >       }
> >   }
> > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > index 190f78261..b55b01f82 100644
> > --- a/tests/ovn-northd.at
> > +++ b/tests/ovn-northd.at
> > @@ -1698,13 +1698,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;)
> > @@ -1713,13 +1710,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.
> > @@ -1773,13 +1767,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;)
> > @@ -1788,13 +1779,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.
> > 
>
Mark Michelson Sept. 8, 2021, 6:49 p.m. UTC | #3
On 8/27/21 2:00 PM, Lorenzo Bianconi wrote:
>> Hi Lorenzo,
>>
>> This mostly looks good, but I have one question below:
>>
>> On 8/17/21 1:51 PM, 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 <lorenzo.bianconi@redhat.com>
>>> ---
>>> Changes since v1:
>>> - rebase on top of ovn master
>>> - add DDlog support
>>> ---
>>>    northd/ovn-northd.c  | 41 +++++++++++++++++++--------
>>>    northd/ovn_northd.dl | 66 ++++++++++++++++++++++++++++++++------------
>>>    tests/ovn-northd.at  | 36 ++++++++----------------
>>>    3 files changed, 90 insertions(+), 53 deletions(-)
>>>
>>> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
>>> index e80876af1..ee08281c3 100644
>>> --- a/northd/ovn-northd.c
>>> +++ b/northd/ovn-northd.c
>>> @@ -9694,8 +9694,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; "
>>> @@ -9703,8 +9702,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,
>>> @@ -11742,7 +11739,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)) {
>>> @@ -11763,17 +11759,40 @@ 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, ", ", " }"));
>>> +
>>> +            ds_put_format(match, "inport == %s && nd_ns && nd.target == %s",
>>> +                          op->json_key, ds_cstr(&load_balancer_ips_v6));
>>>                if (is_l3dgw_port(op)) {
>>> -                ds_put_format(match, "is_chassis_resident(%s)",
>>> +                ds_put_format(match, " && is_chassis_resident(%s)",
>>>                                  op->cr_port->json_key);
>>>                }
>>> +            ds_put_format(actions,
>>> +                          "nd_na { "
>>> +                            "eth.src = %s; "
>>> +                            "ip6.src = nd.target; "
>>> +                            "nd.tll = %s; "
>>> +                            "outport = inport; "
>>> +                            "flags.loopback = 1; "
>>> +                            "output; "
>>> +                          "};",
>>> +                          REG_INPORT_ETH_ADDR,
>>> +                          REG_INPORT_ETH_ADDR);
>>> +            ovn_lflow_add_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
>>> +                                      ds_cstr(match), ds_cstr(actions), NULL,
>>> +                                      copp_meter_get(COPP_ND_NA,
>>> +                                                     op->od->nbr->copp,
>>> +                                                     meter_groups), NULL);
>>
>> I don't understand why you replaced the call to build_lrouter_nd_flow() with
>> manual creation of the match and actions. As long as you pass
>> ds_cstr(&load_balancer_ips_v6) as the "ip_address" parameter, I think
>> build_lrouter_nd_flow() will generate the proper flow.
> 
> Hi Mark,
> 
> IIRC it is because build_lrouter_nd_flow() is run even in other places where it is
> not correct to have  match = ... nd.target == {addr0, addr1} and action = ..
> ip6.src = nd.target since addr1 can be a multicast address.
> 
> Regards,
> Lorenzo

Hi Lorenzo, sorry I'm not following your reasoning here. My thought here 
was that with your change, you generate a logical flow like:

match: inport == <port> && nd_ns && nd_target == {IP1, IP2, ...} [&& 
is_chassis_resident(<cr_port>)]
action: nd_na {
     eth.src = REG_INPORT_ETH_ADDR;
     ip6.src = nd.target;
     nd.tll = REG_INPORT_ETH_ADDR;
     outport = inport;
     flags.loopback = 1;
     output;
};

If you instead called

build_router_nd_flow(op->od, op, "nd_na", 
ds_cstr(&load_balancer_ips_v6), NULL, REG_INPORT_ETH_ADDR, match, false, 
90, NULL, lflows, meter_groups)

You end up with the exact same match and actions.

I'm not sure what bearing this has on other places in the code calling 
build_router_nd_flow()

> 
>>
>>> -            build_lrouter_nd_flow(op->od, op, "nd_na",
>>> -                                  ip_address, 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 6cc5d7248..947637f2a 100644
>>> --- a/northd/ovn_northd.dl
>>> +++ b/northd/ovn_northd.dl
>>> @@ -5260,6 +5260,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>,
>>> +    external_ids: Map<string,string>)
>>> +MeteredFlow(.logical_datapath = lr._uuid,
>>> +            .stage = s_ROUTER_IN_IP_INPUT(),
>>> +            .priority = 90,
>>> +            .__match = __match,
>>> +            .actions = actions,
>>> +            .tags             = map_empty(),
>>> +            .controller_meter = lr.copp.get(cOPP_ND_NA()),
>>> +            .external_ids = external_ids) :-
>>> +    LogicalRouterNdFlowLB(.lr = lr, .lrp = lrp, .ip = ip,
>>> +                          .mac = mac, .extra_match = extra_match,
>>> +                          .external_ids = external_ids),
>>> +    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 =
>>> +        "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>>,
>>> @@ -5343,8 +5381,7 @@ MeteredFlow(.logical_datapath = lr._uuid,
>>>        } else {
>>>            ("${action} { "
>>>               "eth.src = ${mac}; "
>>> -           "ip6.src = ${ip}; "
>>> -           "nd.target = ${ip}; "
>>> +           "ip6.src = nd.target; "
>>>               "nd.tll = ${mac}; "
>>>               "outport = inport; "
>>>               "flags.loopback = 1; "
>>> @@ -5412,7 +5449,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},
>>> @@ -5422,21 +5459,14 @@ var residence_check = match (is_redirect) {
>>>                                     .drop = false,
>>>                                     .priority = 90,
>>>                                     .external_ids = map_empty())
>>> -        }
>>> -    };
>>> -    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,
>>> -                                .external_ids = map_empty())
>>> +        };
>>> +        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,
>>> +                                  .external_ids = map_empty())
>>>            }
>>>        }
>>>    }
>>> diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
>>> index 190f78261..b55b01f82 100644
>>> --- a/tests/ovn-northd.at
>>> +++ b/tests/ovn-northd.at
>>> @@ -1698,13 +1698,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;)
>>> @@ -1713,13 +1710,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.
>>> @@ -1773,13 +1767,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;)
>>> @@ -1788,13 +1779,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.
>>>
>>
Lorenzo Bianconi Sept. 9, 2021, 12:42 p.m. UTC | #4
> On 8/27/21 2:00 PM, Lorenzo Bianconi wrote:
> > > Hi Lorenzo,
> > > 
> > > This mostly looks good, but I have one question below:
> > > 
> > > On 8/17/21 1:51 PM, 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 <lorenzo.bianconi@redhat.com>
> > > > ---
> > > > Changes since v1:
> > > > - rebase on top of ovn master
> > > > - add DDlog support
> > > > ---
> > > >    northd/ovn-northd.c  | 41 +++++++++++++++++++--------
> > > >    northd/ovn_northd.dl | 66 ++++++++++++++++++++++++++++++++------------
> > > >    tests/ovn-northd.at  | 36 ++++++++----------------
> > > >    3 files changed, 90 insertions(+), 53 deletions(-)
> > > > 
> > > > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > > > index e80876af1..ee08281c3 100644
> > > > --- a/northd/ovn-northd.c
> > > > +++ b/northd/ovn-northd.c
> > > > @@ -9694,8 +9694,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; "
> > > > @@ -9703,8 +9702,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,
> > > > @@ -11742,7 +11739,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)) {
> > > > @@ -11763,17 +11759,40 @@ 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, ", ", " }"));
> > > > +
> > > > +            ds_put_format(match, "inport == %s && nd_ns && nd.target == %s",
> > > > +                          op->json_key, ds_cstr(&load_balancer_ips_v6));
> > > >                if (is_l3dgw_port(op)) {
> > > > -                ds_put_format(match, "is_chassis_resident(%s)",
> > > > +                ds_put_format(match, " && is_chassis_resident(%s)",
> > > >                                  op->cr_port->json_key);
> > > >                }
> > > > +            ds_put_format(actions,
> > > > +                          "nd_na { "
> > > > +                            "eth.src = %s; "
> > > > +                            "ip6.src = nd.target; "
> > > > +                            "nd.tll = %s; "
> > > > +                            "outport = inport; "
> > > > +                            "flags.loopback = 1; "
> > > > +                            "output; "
> > > > +                          "};",
> > > > +                          REG_INPORT_ETH_ADDR,
> > > > +                          REG_INPORT_ETH_ADDR);
> > > > +            ovn_lflow_add_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
> > > > +                                      ds_cstr(match), ds_cstr(actions), NULL,
> > > > +                                      copp_meter_get(COPP_ND_NA,
> > > > +                                                     op->od->nbr->copp,
> > > > +                                                     meter_groups), NULL);
> > > 
> > > I don't understand why you replaced the call to build_lrouter_nd_flow() with
> > > manual creation of the match and actions. As long as you pass
> > > ds_cstr(&load_balancer_ips_v6) as the "ip_address" parameter, I think
> > > build_lrouter_nd_flow() will generate the proper flow.
> > 
> > Hi Mark,
> > 
> > IIRC it is because build_lrouter_nd_flow() is run even in other places where it is
> > not correct to have  match = ... nd.target == {addr0, addr1} and action = ..
> > ip6.src = nd.target since addr1 can be a multicast address.
> > 
> > Regards,
> > Lorenzo
> 
> Hi Lorenzo, sorry I'm not following your reasoning here. My thought here was
> that with your change, you generate a logical flow like:

ack Mark, you are right. I will address your comments in v3.

Regrads,
Lorenzo

> 
> match: inport == <port> && nd_ns && nd_target == {IP1, IP2, ...} [&&
> is_chassis_resident(<cr_port>)]
> action: nd_na {
>     eth.src = REG_INPORT_ETH_ADDR;
>     ip6.src = nd.target;
>     nd.tll = REG_INPORT_ETH_ADDR;
>     outport = inport;
>     flags.loopback = 1;
>     output;
> };
> 
> If you instead called
> 
> build_router_nd_flow(op->od, op, "nd_na", ds_cstr(&load_balancer_ips_v6),
> NULL, REG_INPORT_ETH_ADDR, match, false, 90, NULL, lflows, meter_groups)
> 
> You end up with the exact same match and actions.
> 
> I'm not sure what bearing this has on other places in the code calling
> build_router_nd_flow()
> 
> > 
> > > 
> > > > -            build_lrouter_nd_flow(op->od, op, "nd_na",
> > > > -                                  ip_address, 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 6cc5d7248..947637f2a 100644
> > > > --- a/northd/ovn_northd.dl
> > > > +++ b/northd/ovn_northd.dl
> > > > @@ -5260,6 +5260,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>,
> > > > +    external_ids: Map<string,string>)
> > > > +MeteredFlow(.logical_datapath = lr._uuid,
> > > > +            .stage = s_ROUTER_IN_IP_INPUT(),
> > > > +            .priority = 90,
> > > > +            .__match = __match,
> > > > +            .actions = actions,
> > > > +            .tags             = map_empty(),
> > > > +            .controller_meter = lr.copp.get(cOPP_ND_NA()),
> > > > +            .external_ids = external_ids) :-
> > > > +    LogicalRouterNdFlowLB(.lr = lr, .lrp = lrp, .ip = ip,
> > > > +                          .mac = mac, .extra_match = extra_match,
> > > > +                          .external_ids = external_ids),
> > > > +    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 =
> > > > +        "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>>,
> > > > @@ -5343,8 +5381,7 @@ MeteredFlow(.logical_datapath = lr._uuid,
> > > >        } else {
> > > >            ("${action} { "
> > > >               "eth.src = ${mac}; "
> > > > -           "ip6.src = ${ip}; "
> > > > -           "nd.target = ${ip}; "
> > > > +           "ip6.src = nd.target; "
> > > >               "nd.tll = ${mac}; "
> > > >               "outport = inport; "
> > > >               "flags.loopback = 1; "
> > > > @@ -5412,7 +5449,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},
> > > > @@ -5422,21 +5459,14 @@ var residence_check = match (is_redirect) {
> > > >                                     .drop = false,
> > > >                                     .priority = 90,
> > > >                                     .external_ids = map_empty())
> > > > -        }
> > > > -    };
> > > > -    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,
> > > > -                                .external_ids = map_empty())
> > > > +        };
> > > > +        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,
> > > > +                                  .external_ids = map_empty())
> > > >            }
> > > >        }
> > > >    }
> > > > diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
> > > > index 190f78261..b55b01f82 100644
> > > > --- a/tests/ovn-northd.at
> > > > +++ b/tests/ovn-northd.at
> > > > @@ -1698,13 +1698,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;)
> > > > @@ -1713,13 +1710,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.
> > > > @@ -1773,13 +1767,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;)
> > > > @@ -1788,13 +1779,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.
> > > > 
> > > 
>
diff mbox series

Patch

diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index e80876af1..ee08281c3 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -9694,8 +9694,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; "
@@ -9703,8 +9702,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,
@@ -11742,7 +11739,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)) {
@@ -11763,17 +11759,40 @@  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, ", ", " }"));
+
+            ds_put_format(match, "inport == %s && nd_ns && nd.target == %s",
+                          op->json_key, ds_cstr(&load_balancer_ips_v6));
             if (is_l3dgw_port(op)) {
-                ds_put_format(match, "is_chassis_resident(%s)",
+                ds_put_format(match, " && is_chassis_resident(%s)",
                               op->cr_port->json_key);
             }
+            ds_put_format(actions,
+                          "nd_na { "
+                            "eth.src = %s; "
+                            "ip6.src = nd.target; "
+                            "nd.tll = %s; "
+                            "outport = inport; "
+                            "flags.loopback = 1; "
+                            "output; "
+                          "};",
+                          REG_INPORT_ETH_ADDR,
+                          REG_INPORT_ETH_ADDR);
+            ovn_lflow_add_with_hint__(lflows, op->od, S_ROUTER_IN_IP_INPUT, 90,
+                                      ds_cstr(match), ds_cstr(actions), NULL,
+                                      copp_meter_get(COPP_ND_NA,
+                                                     op->od->nbr->copp,
+                                                     meter_groups), NULL);
 
-            build_lrouter_nd_flow(op->od, op, "nd_na",
-                                  ip_address, 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 6cc5d7248..947637f2a 100644
--- a/northd/ovn_northd.dl
+++ b/northd/ovn_northd.dl
@@ -5260,6 +5260,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>,
+    external_ids: Map<string,string>)
+MeteredFlow(.logical_datapath = lr._uuid,
+            .stage = s_ROUTER_IN_IP_INPUT(),
+            .priority = 90,
+            .__match = __match,
+            .actions = actions,
+            .tags             = map_empty(),
+            .controller_meter = lr.copp.get(cOPP_ND_NA()),
+            .external_ids = external_ids) :-
+    LogicalRouterNdFlowLB(.lr = lr, .lrp = lrp, .ip = ip,
+                          .mac = mac, .extra_match = extra_match,
+                          .external_ids = external_ids),
+    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 =
+        "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>>,
@@ -5343,8 +5381,7 @@  MeteredFlow(.logical_datapath = lr._uuid,
     } else {
         ("${action} { "
            "eth.src = ${mac}; "
-           "ip6.src = ${ip}; "
-           "nd.target = ${ip}; "
+           "ip6.src = nd.target; "
            "nd.tll = ${mac}; "
            "outport = inport; "
            "flags.loopback = 1; "
@@ -5412,7 +5449,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},
@@ -5422,21 +5459,14 @@  var residence_check = match (is_redirect) {
                                  .drop = false,
                                  .priority = 90,
                                  .external_ids = map_empty())
-        }
-    };
-    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,
-                                .external_ids = map_empty())
+        };
+        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,
+                                  .external_ids = map_empty())
         }
     }
 }
diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at
index 190f78261..b55b01f82 100644
--- a/tests/ovn-northd.at
+++ b/tests/ovn-northd.at
@@ -1698,13 +1698,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;)
@@ -1713,13 +1710,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.
@@ -1773,13 +1767,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;)
@@ -1788,13 +1779,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.