diff mbox series

[ovs-dev,v3,ovn] northd: add empty_lb controller_event for logical router

Message ID f7bf0e7dd985c8c2c18b0c77dba9b3cb99a4f9cd.1568134587.git.lorenzo.bianconi@redhat.com
State Accepted
Headers show
Series [ovs-dev,v3,ovn] northd: add empty_lb controller_event for logical router | expand

Commit Message

Lorenzo Bianconi Sept. 10, 2019, 5 p.m. UTC
Add empty load balancer controller_event support to logical router
pipeline. Update northd documentation even for logical switch pipeline

Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
---
Changes since v2:
- improve code readability
Changes since v1:
- rebase on-top of current ovn master branch
---
 northd/ovn-northd.8.xml | 10 ++++++++
 northd/ovn-northd.c     | 24 ++++++++++++------
 tests/ovn.at            | 56 +++++++++++++++++++++++++++++++++++------
 3 files changed, 76 insertions(+), 14 deletions(-)

Comments

Mark Michelson Sept. 13, 2019, 8:32 p.m. UTC | #1
Acked-by: Mark Michelson <mmichels@redhat.com>

On 9/10/19 1:00 PM, Lorenzo Bianconi wrote:
> Add empty load balancer controller_event support to logical router
> pipeline. Update northd documentation even for logical switch pipeline
> 
> Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
> ---
> Changes since v2:
> - improve code readability
> Changes since v1:
> - rebase on-top of current ovn master branch
> ---
>   northd/ovn-northd.8.xml | 10 ++++++++
>   northd/ovn-northd.c     | 24 ++++++++++++------
>   tests/ovn.at            | 56 +++++++++++++++++++++++++++++++++++------
>   3 files changed, 76 insertions(+), 14 deletions(-)
> 
> diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> index b34ef687a..0f4f1c112 100644
> --- a/northd/ovn-northd.8.xml
> +++ b/northd/ovn-northd.8.xml
> @@ -1785,6 +1785,16 @@ icmp6 {
>       </p>
>   
>       <ul>
> +      <li>
> +        If controller_event has been enabled for all the configured load
> +        balancing rules for a Gateway router or Router with gateway port
> +        in <code>OVN_Northbound</code> database that does not have configured
> +        backends, a priority-130 flow is added to trigger ovn-controller events
> +        whenever the chassis receives a packet for that particular VIP.
> +        If <code>event-elb</code> meter has been previously created, it will be
> +        associated to the empty_lb logical flow
> +      </li>
> +
>         <li>
>           For all the configured load balancing rules for a Gateway router or
>           Router with gateway port in <code>OVN_Northbound</code> database that
> diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> index c24e4d864..f393cebb8 100644
> --- a/northd/ovn-northd.c
> +++ b/northd/ovn-northd.c
> @@ -6148,9 +6148,17 @@ get_force_snat_ip(struct ovn_datapath *od, const char *key_type, ovs_be32 *ip)
>   static void
>   add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
>                      struct ds *match, struct ds *actions, int priority,
> -                   const char *lb_force_snat_ip, char *backend_ips,
> -                   bool is_udp, int addr_family)
> +                   const char *lb_force_snat_ip, struct smap_node *lb_info,
> +                   bool is_udp, int addr_family, char *ip_addr,
> +                   uint16_t l4_port, struct nbrec_load_balancer *lb,
> +                   struct shash *meter_groups)
>   {
> +    char *backend_ips = lb_info->value;
> +
> +    build_empty_lb_event_flow(od, lflows, lb_info, ip_addr, lb,
> +                              l4_port, addr_family, S_ROUTER_IN_DNAT,
> +                              meter_groups);
> +
>       /* A match and actions for new connections. */
>       char *new_match = xasprintf("ct.new && %s", ds_cstr(match));
>       if (lb_force_snat_ip) {
> @@ -6308,7 +6316,7 @@ copy_ra_to_sb(struct ovn_port *op, const char *address_mode)
>   
>   static void
>   build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> -                    struct hmap *lflows)
> +                    struct hmap *lflows, struct shash *meter_groups)
>   {
>       /* This flow table structure is documented in ovn-northd(8), so please
>        * update ovn-northd.8.xml if you change anything. */
> @@ -7525,7 +7533,6 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>                       ds_put_format(&match, "ip && ip6.dst == %s",
>                                   ip_address);
>                   }
> -                free(ip_address);
>   
>                   int prio = 110;
>                   bool is_udp = lb->protocol && !strcmp(lb->protocol, "udp") ?
> @@ -7546,8 +7553,11 @@ build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
>                                     od->l3redirect_port->json_key);
>                   }
>                   add_router_lb_flow(lflows, od, &match, &actions, prio,
> -                                   lb_force_snat_ip, node->value, is_udp,
> -                                   addr_family);
> +                                   lb_force_snat_ip, node, is_udp,
> +                                   addr_family, ip_address, port, lb,
> +                                   meter_groups);
> +
> +                free(ip_address);
>               }
>           }
>           sset_destroy(&all_ips);
> @@ -8328,7 +8338,7 @@ build_lflows(struct northd_context *ctx, struct hmap *datapaths,
>   
>       build_lswitch_flows(datapaths, ports, port_groups, &lflows, mcgroups,
>                           igmp_groups, meter_groups);
> -    build_lrouter_flows(datapaths, ports, &lflows);
> +    build_lrouter_flows(datapaths, ports, &lflows, meter_groups);
>   
>       /* Push changes to the Logical_Flow table to database. */
>       const struct sbrec_logical_flow *sbflow, *next_sbflow;
> diff --git a/tests/ovn.at b/tests/ovn.at
> index de1b3b3ba..2749184eb 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -14683,9 +14683,22 @@ ovn_start
>   # Create hypervisors hv[12].
>   # Add vif1[12] to hv1, vif2[12] to hv2
>   # Add all of the vifs to a single logical switch sw0.
> +# Create logical router lr0
>   
>   net_add n1
> -ovn-nbctl ls-add sw0
> +
> +ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1
> +for i in 0 1; do
> +    idx=$((i+1))
> +    ovn-nbctl ls-add sw$i
> +    ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$idx 192.168.$idx.254/24
> +    ovn-nbctl \
> +        -- lsp-add sw$i lrp$i-attachment \
> +        -- set Logical_Switch_Port lrp$i-attachment type=router \
> +                         options:router-port=lrp$i \
> +                         addresses='"00:00:00:00:ff:'0$idx'"'
> +done
> +
>   for i in 1 2; do
>       sim_add hv$i
>       as hv$i
> @@ -14705,10 +14718,24 @@ for i in 1 2; do
>       done
>   done
>   
> +as hv1
> +ovn-nbctl lsp-add sw1 sw1-p0 \
> +    -- lsp-set-addresses sw1-p0 "00:00:00:00:00:33 192.168.2.11"
> +ovs-vsctl -- add-port br-int vif33 -- \
> +        set interface vif33 \
> +        external-ids:iface-id=sw1-p0 \
> +        options:tx_pcap=hv$i/vif33-tx.pcap \
> +        options:rxq_pcap=hv$i/vif33-rx.pcap \
> +        ofport-request=33
> +
>   ovn-nbctl --wait=hv set NB_Global . options:controller_event=true
>   ovn-nbctl lb-add lb0 192.168.1.100:80 ""
>   ovn-nbctl ls-lb-add sw0 lb0
> -uuid_lb=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb0)
> +uuid_lb0=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb0)
> +
> +ovn-nbctl lb-add lb1 192.168.2.100:80 ""
> +ovn-nbctl lr-lb-add lr0 lb1
> +uuid_lb1=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb1)
>   ovn-nbctl --wait=hv meter-add event-elb drop 100 pktps 10
>   
>   OVN_POPULATE_ARP
> @@ -14716,10 +14743,10 @@ ovn-nbctl --timeout=3 --wait=hv sync
>   ovn-sbctl lflow-list
>   as hv1 ovs-ofctl dump-flows br-int
>   
> -packet="inport==\"sw0-p11\" && eth.src==00:00:00:00:00:11 && eth.dst==00:00:00:00:00:21 &&
> -       ip4 && ip.ttl==64 && ip4.src==192.168.1.11 && ip4.dst==192.168.1.100 &&
> -       tcp && tcp.src==10000 && tcp.dst==80"
> -as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"
> +packet0="inport==\"sw0-p11\" && eth.src==00:00:00:00:00:11 && eth.dst==00:00:00:00:00:21 &&
> +         ip4 && ip.ttl==64 && ip4.src==192.168.1.11 && ip4.dst==192.168.1.100 &&
> +         tcp && tcp.src==10000 && tcp.dst==80"
> +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet0"
>   
>   ovn-sbctl list controller_event
>   uuid=$(ovn-sbctl list controller_event | awk '/_uuid/{print $3}')
> @@ -14733,12 +14760,27 @@ AT_CHECK([ovn-sbctl get controller_event $uuid event_info:protocol], [0], [dnl
>   tcp
>   ])
>   AT_CHECK_UNQUOTED([ovn-sbctl get controller_event $uuid event_info:load_balancer], [0], [dnl
> -"$uuid_lb"
> +"$uuid_lb0"
>   ])
>   AT_CHECK([ovn-sbctl get controller_event $uuid seq_num], [0], [dnl
>   1
>   ])
>   
> +ovn-sbctl destroy controller_event $uuid
> +packet1="inport==\"sw1-p0\" && eth.src==00:00:00:00:00:33 && eth.dst==00:00:00:00:ff:02 &&
> +         ip4 && ip.ttl==64 && ip4.src==192.168.2.11 && ip4.dst==192.168.2.100 &&
> +         tcp && tcp.src==10000 && tcp.dst==80"
> +
> +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet1"
> +ovn-sbctl list controller_event
> +uuid=$(ovn-sbctl list controller_event | awk '/_uuid/{print $3}')
> +AT_CHECK([ovn-sbctl get controller_event $uuid event_type], [0], [dnl
> +empty_lb_backends
> +])
> +AT_CHECK([ovn-sbctl get controller_event $uuid event_info:vip], [0], [dnl
> +"192.168.2.100:80"
> +])
> +
>   OVN_CLEANUP([hv1], [hv2])
>   AT_CLEANUP
>   
>
Numan Siddique Sept. 16, 2019, 10:58 a.m. UTC | #2
On Sat, Sep 14, 2019 at 2:03 AM Mark Michelson <mmichels@redhat.com> wrote:

> Acked-by: Mark Michelson <mmichels@redhat.com>
>
> On 9/10/19 1:00 PM, Lorenzo Bianconi wrote:
> > Add empty load balancer controller_event support to logical router
> > pipeline. Update northd documentation even for logical switch pipeline
> >
> > Signed-off-by: Lorenzo Bianconi <lorenzo.bianconi@redhat.com>
>

Thanks.
I applied this patch to master.

Numan


> > ---
> > Changes since v2:
> > - improve code readability
> > Changes since v1:
> > - rebase on-top of current ovn master branch
> > ---
> >   northd/ovn-northd.8.xml | 10 ++++++++
> >   northd/ovn-northd.c     | 24 ++++++++++++------
> >   tests/ovn.at            | 56 +++++++++++++++++++++++++++++++++++------
> >   3 files changed, 76 insertions(+), 14 deletions(-)
> >
> > diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
> > index b34ef687a..0f4f1c112 100644
> > --- a/northd/ovn-northd.8.xml
> > +++ b/northd/ovn-northd.8.xml
> > @@ -1785,6 +1785,16 @@ icmp6 {
> >       </p>
> >
> >       <ul>
> > +      <li>
> > +        If controller_event has been enabled for all the configured load
> > +        balancing rules for a Gateway router or Router with gateway port
> > +        in <code>OVN_Northbound</code> database that does not have
> configured
> > +        backends, a priority-130 flow is added to trigger
> ovn-controller events
> > +        whenever the chassis receives a packet for that particular VIP.
> > +        If <code>event-elb</code> meter has been previously created, it
> will be
> > +        associated to the empty_lb logical flow
> > +      </li>
> > +
> >         <li>
> >           For all the configured load balancing rules for a Gateway
> router or
> >           Router with gateway port in <code>OVN_Northbound</code>
> database that
> > diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
> > index c24e4d864..f393cebb8 100644
> > --- a/northd/ovn-northd.c
> > +++ b/northd/ovn-northd.c
> > @@ -6148,9 +6148,17 @@ get_force_snat_ip(struct ovn_datapath *od, const
> char *key_type, ovs_be32 *ip)
> >   static void
> >   add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
> >                      struct ds *match, struct ds *actions, int priority,
> > -                   const char *lb_force_snat_ip, char *backend_ips,
> > -                   bool is_udp, int addr_family)
> > +                   const char *lb_force_snat_ip, struct smap_node
> *lb_info,
> > +                   bool is_udp, int addr_family, char *ip_addr,
> > +                   uint16_t l4_port, struct nbrec_load_balancer *lb,
> > +                   struct shash *meter_groups)
> >   {
> > +    char *backend_ips = lb_info->value;
> > +
> > +    build_empty_lb_event_flow(od, lflows, lb_info, ip_addr, lb,
> > +                              l4_port, addr_family, S_ROUTER_IN_DNAT,
> > +                              meter_groups);
> > +
> >       /* A match and actions for new connections. */
> >       char *new_match = xasprintf("ct.new && %s", ds_cstr(match));
> >       if (lb_force_snat_ip) {
> > @@ -6308,7 +6316,7 @@ copy_ra_to_sb(struct ovn_port *op, const char
> *address_mode)
> >
> >   static void
> >   build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
> > -                    struct hmap *lflows)
> > +                    struct hmap *lflows, struct shash *meter_groups)
> >   {
> >       /* This flow table structure is documented in ovn-northd(8), so
> please
> >        * update ovn-northd.8.xml if you change anything. */
> > @@ -7525,7 +7533,6 @@ build_lrouter_flows(struct hmap *datapaths, struct
> hmap *ports,
> >                       ds_put_format(&match, "ip && ip6.dst == %s",
> >                                   ip_address);
> >                   }
> > -                free(ip_address);
> >
> >                   int prio = 110;
> >                   bool is_udp = lb->protocol && !strcmp(lb->protocol,
> "udp") ?
> > @@ -7546,8 +7553,11 @@ build_lrouter_flows(struct hmap *datapaths,
> struct hmap *ports,
> >                                     od->l3redirect_port->json_key);
> >                   }
> >                   add_router_lb_flow(lflows, od, &match, &actions, prio,
> > -                                   lb_force_snat_ip, node->value,
> is_udp,
> > -                                   addr_family);
> > +                                   lb_force_snat_ip, node, is_udp,
> > +                                   addr_family, ip_address, port, lb,
> > +                                   meter_groups);
> > +
> > +                free(ip_address);
> >               }
> >           }
> >           sset_destroy(&all_ips);
> > @@ -8328,7 +8338,7 @@ build_lflows(struct northd_context *ctx, struct
> hmap *datapaths,
> >
> >       build_lswitch_flows(datapaths, ports, port_groups, &lflows,
> mcgroups,
> >                           igmp_groups, meter_groups);
> > -    build_lrouter_flows(datapaths, ports, &lflows);
> > +    build_lrouter_flows(datapaths, ports, &lflows, meter_groups);
> >
> >       /* Push changes to the Logical_Flow table to database. */
> >       const struct sbrec_logical_flow *sbflow, *next_sbflow;
> > diff --git a/tests/ovn.at b/tests/ovn.at
> > index de1b3b3ba..2749184eb 100644
> > --- a/tests/ovn.at
> > +++ b/tests/ovn.at
> > @@ -14683,9 +14683,22 @@ ovn_start
> >   # Create hypervisors hv[12].
> >   # Add vif1[12] to hv1, vif2[12] to hv2
> >   # Add all of the vifs to a single logical switch sw0.
> > +# Create logical router lr0
> >
> >   net_add n1
> > -ovn-nbctl ls-add sw0
> > +
> > +ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1
> > +for i in 0 1; do
> > +    idx=$((i+1))
> > +    ovn-nbctl ls-add sw$i
> > +    ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$idx 192.168.$idx.254/24
> > +    ovn-nbctl \
> > +        -- lsp-add sw$i lrp$i-attachment \
> > +        -- set Logical_Switch_Port lrp$i-attachment type=router \
> > +                         options:router-port=lrp$i \
> > +                         addresses='"00:00:00:00:ff:'0$idx'"'
> > +done
> > +
> >   for i in 1 2; do
> >       sim_add hv$i
> >       as hv$i
> > @@ -14705,10 +14718,24 @@ for i in 1 2; do
> >       done
> >   done
> >
> > +as hv1
> > +ovn-nbctl lsp-add sw1 sw1-p0 \
> > +    -- lsp-set-addresses sw1-p0 "00:00:00:00:00:33 192.168.2.11"
> > +ovs-vsctl -- add-port br-int vif33 -- \
> > +        set interface vif33 \
> > +        external-ids:iface-id=sw1-p0 \
> > +        options:tx_pcap=hv$i/vif33-tx.pcap \
> > +        options:rxq_pcap=hv$i/vif33-rx.pcap \
> > +        ofport-request=33
> > +
> >   ovn-nbctl --wait=hv set NB_Global . options:controller_event=true
> >   ovn-nbctl lb-add lb0 192.168.1.100:80 ""
> >   ovn-nbctl ls-lb-add sw0 lb0
> > -uuid_lb=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb0)
> > +uuid_lb0=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb0)
> > +
> > +ovn-nbctl lb-add lb1 192.168.2.100:80 ""
> > +ovn-nbctl lr-lb-add lr0 lb1
> > +uuid_lb1=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb1)
> >   ovn-nbctl --wait=hv meter-add event-elb drop 100 pktps 10
> >
> >   OVN_POPULATE_ARP
> > @@ -14716,10 +14743,10 @@ ovn-nbctl --timeout=3 --wait=hv sync
> >   ovn-sbctl lflow-list
> >   as hv1 ovs-ofctl dump-flows br-int
> >
> > -packet="inport==\"sw0-p11\" && eth.src==00:00:00:00:00:11 &&
> eth.dst==00:00:00:00:00:21 &&
> > -       ip4 && ip.ttl==64 && ip4.src==192.168.1.11 &&
> ip4.dst==192.168.1.100 &&
> > -       tcp && tcp.src==10000 && tcp.dst==80"
> > -as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"
> > +packet0="inport==\"sw0-p11\" && eth.src==00:00:00:00:00:11 &&
> eth.dst==00:00:00:00:00:21 &&
> > +         ip4 && ip.ttl==64 && ip4.src==192.168.1.11 &&
> ip4.dst==192.168.1.100 &&
> > +         tcp && tcp.src==10000 && tcp.dst==80"
> > +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet0"
> >
> >   ovn-sbctl list controller_event
> >   uuid=$(ovn-sbctl list controller_event | awk '/_uuid/{print $3}')
> > @@ -14733,12 +14760,27 @@ AT_CHECK([ovn-sbctl get controller_event $uuid
> event_info:protocol], [0], [dnl
> >   tcp
> >   ])
> >   AT_CHECK_UNQUOTED([ovn-sbctl get controller_event $uuid
> event_info:load_balancer], [0], [dnl
> > -"$uuid_lb"
> > +"$uuid_lb0"
> >   ])
> >   AT_CHECK([ovn-sbctl get controller_event $uuid seq_num], [0], [dnl
> >   1
> >   ])
> >
> > +ovn-sbctl destroy controller_event $uuid
> > +packet1="inport==\"sw1-p0\" && eth.src==00:00:00:00:00:33 &&
> eth.dst==00:00:00:00:ff:02 &&
> > +         ip4 && ip.ttl==64 && ip4.src==192.168.2.11 &&
> ip4.dst==192.168.2.100 &&
> > +         tcp && tcp.src==10000 && tcp.dst==80"
> > +
> > +as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet1"
> > +ovn-sbctl list controller_event
> > +uuid=$(ovn-sbctl list controller_event | awk '/_uuid/{print $3}')
> > +AT_CHECK([ovn-sbctl get controller_event $uuid event_type], [0], [dnl
> > +empty_lb_backends
> > +])
> > +AT_CHECK([ovn-sbctl get controller_event $uuid event_info:vip], [0],
> [dnl
> > +"192.168.2.100:80"
> > +])
> > +
> >   OVN_CLEANUP([hv1], [hv2])
> >   AT_CLEANUP
> >
> >
>
> _______________________________________________
> dev mailing list
> dev@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
diff mbox series

Patch

diff --git a/northd/ovn-northd.8.xml b/northd/ovn-northd.8.xml
index b34ef687a..0f4f1c112 100644
--- a/northd/ovn-northd.8.xml
+++ b/northd/ovn-northd.8.xml
@@ -1785,6 +1785,16 @@  icmp6 {
     </p>
 
     <ul>
+      <li>
+        If controller_event has been enabled for all the configured load
+        balancing rules for a Gateway router or Router with gateway port
+        in <code>OVN_Northbound</code> database that does not have configured
+        backends, a priority-130 flow is added to trigger ovn-controller events
+        whenever the chassis receives a packet for that particular VIP.
+        If <code>event-elb</code> meter has been previously created, it will be
+        associated to the empty_lb logical flow
+      </li>
+
       <li>
         For all the configured load balancing rules for a Gateway router or
         Router with gateway port in <code>OVN_Northbound</code> database that
diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c
index c24e4d864..f393cebb8 100644
--- a/northd/ovn-northd.c
+++ b/northd/ovn-northd.c
@@ -6148,9 +6148,17 @@  get_force_snat_ip(struct ovn_datapath *od, const char *key_type, ovs_be32 *ip)
 static void
 add_router_lb_flow(struct hmap *lflows, struct ovn_datapath *od,
                    struct ds *match, struct ds *actions, int priority,
-                   const char *lb_force_snat_ip, char *backend_ips,
-                   bool is_udp, int addr_family)
+                   const char *lb_force_snat_ip, struct smap_node *lb_info,
+                   bool is_udp, int addr_family, char *ip_addr,
+                   uint16_t l4_port, struct nbrec_load_balancer *lb,
+                   struct shash *meter_groups)
 {
+    char *backend_ips = lb_info->value;
+
+    build_empty_lb_event_flow(od, lflows, lb_info, ip_addr, lb,
+                              l4_port, addr_family, S_ROUTER_IN_DNAT,
+                              meter_groups);
+
     /* A match and actions for new connections. */
     char *new_match = xasprintf("ct.new && %s", ds_cstr(match));
     if (lb_force_snat_ip) {
@@ -6308,7 +6316,7 @@  copy_ra_to_sb(struct ovn_port *op, const char *address_mode)
 
 static void
 build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
-                    struct hmap *lflows)
+                    struct hmap *lflows, struct shash *meter_groups)
 {
     /* This flow table structure is documented in ovn-northd(8), so please
      * update ovn-northd.8.xml if you change anything. */
@@ -7525,7 +7533,6 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                     ds_put_format(&match, "ip && ip6.dst == %s",
                                 ip_address);
                 }
-                free(ip_address);
 
                 int prio = 110;
                 bool is_udp = lb->protocol && !strcmp(lb->protocol, "udp") ?
@@ -7546,8 +7553,11 @@  build_lrouter_flows(struct hmap *datapaths, struct hmap *ports,
                                   od->l3redirect_port->json_key);
                 }
                 add_router_lb_flow(lflows, od, &match, &actions, prio,
-                                   lb_force_snat_ip, node->value, is_udp,
-                                   addr_family);
+                                   lb_force_snat_ip, node, is_udp,
+                                   addr_family, ip_address, port, lb,
+                                   meter_groups);
+
+                free(ip_address);
             }
         }
         sset_destroy(&all_ips);
@@ -8328,7 +8338,7 @@  build_lflows(struct northd_context *ctx, struct hmap *datapaths,
 
     build_lswitch_flows(datapaths, ports, port_groups, &lflows, mcgroups,
                         igmp_groups, meter_groups);
-    build_lrouter_flows(datapaths, ports, &lflows);
+    build_lrouter_flows(datapaths, ports, &lflows, meter_groups);
 
     /* Push changes to the Logical_Flow table to database. */
     const struct sbrec_logical_flow *sbflow, *next_sbflow;
diff --git a/tests/ovn.at b/tests/ovn.at
index de1b3b3ba..2749184eb 100644
--- a/tests/ovn.at
+++ b/tests/ovn.at
@@ -14683,9 +14683,22 @@  ovn_start
 # Create hypervisors hv[12].
 # Add vif1[12] to hv1, vif2[12] to hv2
 # Add all of the vifs to a single logical switch sw0.
+# Create logical router lr0
 
 net_add n1
-ovn-nbctl ls-add sw0
+
+ovn-nbctl create Logical_Router name=lr0 options:chassis=hv1
+for i in 0 1; do
+    idx=$((i+1))
+    ovn-nbctl ls-add sw$i
+    ovn-nbctl lrp-add lr0 lrp$i 00:00:00:00:ff:0$idx 192.168.$idx.254/24
+    ovn-nbctl \
+        -- lsp-add sw$i lrp$i-attachment \
+        -- set Logical_Switch_Port lrp$i-attachment type=router \
+                         options:router-port=lrp$i \
+                         addresses='"00:00:00:00:ff:'0$idx'"'
+done
+
 for i in 1 2; do
     sim_add hv$i
     as hv$i
@@ -14705,10 +14718,24 @@  for i in 1 2; do
     done
 done
 
+as hv1
+ovn-nbctl lsp-add sw1 sw1-p0 \
+    -- lsp-set-addresses sw1-p0 "00:00:00:00:00:33 192.168.2.11"
+ovs-vsctl -- add-port br-int vif33 -- \
+        set interface vif33 \
+        external-ids:iface-id=sw1-p0 \
+        options:tx_pcap=hv$i/vif33-tx.pcap \
+        options:rxq_pcap=hv$i/vif33-rx.pcap \
+        ofport-request=33
+
 ovn-nbctl --wait=hv set NB_Global . options:controller_event=true
 ovn-nbctl lb-add lb0 192.168.1.100:80 ""
 ovn-nbctl ls-lb-add sw0 lb0
-uuid_lb=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb0)
+uuid_lb0=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb0)
+
+ovn-nbctl lb-add lb1 192.168.2.100:80 ""
+ovn-nbctl lr-lb-add lr0 lb1
+uuid_lb1=$(ovn-nbctl --bare --columns=_uuid find load_balancer name=lb1)
 ovn-nbctl --wait=hv meter-add event-elb drop 100 pktps 10
 
 OVN_POPULATE_ARP
@@ -14716,10 +14743,10 @@  ovn-nbctl --timeout=3 --wait=hv sync
 ovn-sbctl lflow-list
 as hv1 ovs-ofctl dump-flows br-int
 
-packet="inport==\"sw0-p11\" && eth.src==00:00:00:00:00:11 && eth.dst==00:00:00:00:00:21 &&
-       ip4 && ip.ttl==64 && ip4.src==192.168.1.11 && ip4.dst==192.168.1.100 &&
-       tcp && tcp.src==10000 && tcp.dst==80"
-as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet"
+packet0="inport==\"sw0-p11\" && eth.src==00:00:00:00:00:11 && eth.dst==00:00:00:00:00:21 &&
+         ip4 && ip.ttl==64 && ip4.src==192.168.1.11 && ip4.dst==192.168.1.100 &&
+         tcp && tcp.src==10000 && tcp.dst==80"
+as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet0"
 
 ovn-sbctl list controller_event
 uuid=$(ovn-sbctl list controller_event | awk '/_uuid/{print $3}')
@@ -14733,12 +14760,27 @@  AT_CHECK([ovn-sbctl get controller_event $uuid event_info:protocol], [0], [dnl
 tcp
 ])
 AT_CHECK_UNQUOTED([ovn-sbctl get controller_event $uuid event_info:load_balancer], [0], [dnl
-"$uuid_lb"
+"$uuid_lb0"
 ])
 AT_CHECK([ovn-sbctl get controller_event $uuid seq_num], [0], [dnl
 1
 ])
 
+ovn-sbctl destroy controller_event $uuid
+packet1="inport==\"sw1-p0\" && eth.src==00:00:00:00:00:33 && eth.dst==00:00:00:00:ff:02 &&
+         ip4 && ip.ttl==64 && ip4.src==192.168.2.11 && ip4.dst==192.168.2.100 &&
+         tcp && tcp.src==10000 && tcp.dst==80"
+
+as hv1 ovs-appctl -t ovn-controller inject-pkt "$packet1"
+ovn-sbctl list controller_event
+uuid=$(ovn-sbctl list controller_event | awk '/_uuid/{print $3}')
+AT_CHECK([ovn-sbctl get controller_event $uuid event_type], [0], [dnl
+empty_lb_backends
+])
+AT_CHECK([ovn-sbctl get controller_event $uuid event_info:vip], [0], [dnl
+"192.168.2.100:80"
+])
+
 OVN_CLEANUP([hv1], [hv2])
 AT_CLEANUP