On Thu, Mar 13, 2025 at 9:33 AM Xavier Simonart <xsimo...@redhat.com> wrote:
> If we add a datapath local to the current hv to a load balancer which was
> only added to
> non local datapaths, then hairpin flows were missing.
>
> For instance, let's have sw0 and sw0p1 bound to hv0, and sw1 and sw1p1
> bound to hv1
> Let's also have a load balancer lb1 added to sw0.
> When lb1 is added to sw1, hairpin flows were missing.
>
> Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
> ---
> controller/ovn-controller.c | 6 +--
> tests/ovn.at | 86 +++++++++++++++++++++++++++++++++++++
> 2 files changed, 88 insertions(+), 4 deletions(-)
>
> diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c
> index c0f167c54..bc8acddf1 100644
> --- a/controller/ovn-controller.c
> +++ b/controller/ovn-controller.c
> @@ -2861,11 +2861,9 @@ lb_data_sb_load_balancer_handler(struct engine_node
> *node, void *data)
> if (!sbrec_load_balancer_is_new(sbrec_lb)) {
> lb = ovn_controller_lb_find(&lb_data->local_lbs,
> &sbrec_lb->header_.uuid);
> - if (!lb) {
> - continue;
> + if (lb) {
> + lb_data_local_lb_remove(lb_data, lb);
> }
> -
> - lb_data_local_lb_remove(lb_data, lb);
> }
>
> if (sbrec_load_balancer_is_deleted(sbrec_lb) ||
> diff --git a/tests/ovn.at b/tests/ovn.at
> index f9e1c96c9..f4abfb345 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -42164,3 +42164,89 @@ wait_row_count ACL_ID 0
>
> AT_CLEANUP
> ])
> +
> +# Test when a load balancer is added to two logical switches
> +# which are local to different hypervisors.
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([Load balancer hairpin flows])
> +AT_KEYWORDS([lb])
> +ovn_start
> +
> +net_add n1
> +
> +sim_add hv1
> +as hv1
> +check ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.1
> +check ovs-vsctl -- add-port br-int hv1-vif1 -- \
> + set interface hv1-vif1 external-ids:iface-id=sw0-p1 \
> + options:tx_pcap=hv1/vif1-tx.pcap \
> + options:rxq_pcap=hv1/vif1-rx.pcap
> +
> +sim_add hv2
> +as hv2
> +ovs-vsctl add-br br-phys
> +ovn_attach n1 br-phys 192.168.0.2
> +ovs-vsctl -- add-port br-int hv2-vif1 -- \
> + set interface hv2-vif1 external-ids:iface-id=sw1-p1 \
> + options:tx_pcap=hv2/vif1-tx.pcap \
> + options:rxq_pcap=hv2/vif1-rx.pcap
> +
> +check ovn-nbctl ls-add sw0
> +check ovn-nbctl ls-add sw1
> +
> +check ovn-nbctl lsp-add sw0 sw0-p1
> +check ovn-nbctl lsp-set-addresses sw0-p1 "50:54:10:00:00:01 10.0.0.1"
> +
> +check ovn-nbctl lsp-add sw1 sw1-p1
> +check ovn-nbctl lsp-set-addresses sw1-p1 "50:54:20:00:00:01 20.0.0.1"
> +
> +# Create a logical router and attach both logical switches.
> +# Make the logical router a GW router.
> +check ovn-nbctl lr-add lr0
> +check ovn-nbctl set logical_router lr0 options:chassis=hv1
> +check ovn-nbctl lrp-add lr0 lr0-sw0 00:00:00:00:ff:01 10.0.0.254/24
> +check ovn-nbctl lsp-add sw0 sw0-lr0
> +check ovn-nbctl lsp-set-type sw0-lr0 router
> +check ovn-nbctl lsp-set-addresses sw0-lr0 router
> +check ovn-nbctl lsp-set-options sw0-lr0 router-port=lr0-sw0
> +
> +check ovn-nbctl lrp-add lr0 lr0-sw1 00:00:00:00:ff:02 20.0.0.254/24
> +check ovn-nbctl lsp-add sw1 sw1-lr0
> +check ovn-nbctl lsp-set-type sw1-lr0 router
> +check ovn-nbctl lsp-set-addresses sw1-lr0 router
> +check ovn-nbctl lsp-set-options sw1-lr0 router-port=lr0-sw1
> +
> +check ovn-nbctl lb-add lb1 10.0.0.10:80 20.0.0.1:80 udp
> +check ovn-nbctl ls-lb-add sw0 lb1
> +check ovn-nbctl ls-lb-add sw1 lb1
> +
> +OVN_POPULATE_ARP
> +wait_for_ports_up
> +check ovn-nbctl --wait=hv sync
> +
> +# Send a packet from the lb backend to the lb VIP (hairpin).
> +src_mac="50:54:20:00:00:03"
> +dst_mac="00:00:00:00:ff:02"
> +src_ip="20.0.0.1"
> +dst_ip="10.0.0.10"
> +sport=4369
> +dport=80
> +
> +packet=$(fmt_pkt "Ether(dst='${dst_mac}', src='${src_mac}')/
> + IP(dst='${dst_ip}', src='${src_ip}')/ \
> + UDP(sport=$sport, dport=$dport)")
> +
> +check as hv2 ovs-appctl netdev-dummy/receive hv2-vif1 $packet
> +
> +# Packet expected from the load balancer VIP.
> +# It should have snatted the src_ip to the VIP of the LB.
> +packet=$(fmt_pkt "Ether(dst='${src_mac}', src='${dst_mac}')/
> + IP(dst='${src_ip}', src='${dst_ip}', ttl=64)/ \
> + UDP(sport=$sport, dport=$dport)")
> +echo $packet > expected
> +OVN_CHECK_PACKETS_REMOVE_BROADCAST([hv2/vif1-tx.pcap], [expected])
> +
> +OVN_CLEANUP([hv1], [hv2])
> +AT_CLEANUP
> +])
> --
> 2.47.1
>
> _______________________________________________
> dev mailing list
> d...@openvswitch.org
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>
>
Looks good to me, thanks.
Acked-by: Ales Musil <amu...@redhat.com>
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
