On 3/14/25 11:02 AM, Ales Musil wrote:
> 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>
Thanks, Xavier and Ales! Applied to main, 25.03, 24.09 and 24.03.
Regards,
Dumitru
_______________________________________________
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
