Thank you, this looks great.

Signed-off-by: Ihar Hrachyshka <[email protected]>

On Wed, Apr 20, 2022 at 4:09 AM Ales Musil <[email protected]> wrote:
>
> The GARP was sent even on chassis that were not serving
> as l3gw for the specified router. This could lead to race
> on the physical network when multiple chassis send the same
> ARP but the physical interfaces have different port numbers
> on the external bridge.
>
> Add check to filter out port_bindings for l3gw that
> are sitting on different chassis.
>
> Reported-at: https://bugzilla.redhat.com/2062580
> Signed-off-by: Ales Musil <[email protected]>
> ---
>  controller/pinctrl.c |   2 +-
>  tests/ovn.at         | 108 +++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 109 insertions(+), 1 deletion(-)
>
> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
> index 25b37ee88..42d1c2a46 100644
> --- a/controller/pinctrl.c
> +++ b/controller/pinctrl.c
> @@ -5554,7 +5554,7 @@ get_localnet_vifs_l3gwports(
>          sbrec_port_binding_index_set_datapath(target, ld->datapath);
>          SBREC_PORT_BINDING_FOR_EACH_EQUAL (pb, target,
>                                             sbrec_port_binding_by_datapath) {
> -            if (!strcmp(pb->type, "l3gateway")
> +            if ((!strcmp(pb->type, "l3gateway") && pb->chassis == chassis)
>                  || !strcmp(pb->type, "patch")) {
>                  sset_add(local_l3gw_ports, pb->logical_port);
>              }
> diff --git a/tests/ovn.at b/tests/ovn.at
> index f9551b843..f370e99ae 100644
> --- a/tests/ovn.at
> +++ b/tests/ovn.at
> @@ -8707,6 +8707,114 @@ OVN_CLEANUP([hv1])
>  AT_CLEANUP
>  ])
>
> +OVN_FOR_EACH_NORTHD([
> +AT_SETUP([send gratuitous arp for l3gateway only on selected chassis])
> +ovn_start
> +
> +# Create logical switch
> +ovn-nbctl ls-add ls0
> +# Create gateway router
> +ovn-nbctl lr-add lr0
> +# Add router port to gateway router
> +ovn-nbctl lrp-add lr0 lr0-ls0 f0:00:00:00:00:01 192.168.0.1/24
> +ovn-nbctl lsp-add ls0 ls0-lr0 -- set Logical_Switch_Port ls0-lr0 \
> +    type=router options:router-port=lr0-ls0 addresses='"f0:00:00:00:00:01"'
> +
> +# Create a localnet port.
> +ovn-nbctl lsp-add ls0 ln_port
> +ovn-nbctl lsp-set-addresses ln_port unknown
> +ovn-nbctl lsp-set-type ln_port localnet
> +ovn-nbctl --wait=hv lsp-set-options ln_port network_name=physnet1
> +
> +# Prepare packets
> +touch empty_expected
> +echo 
> "fffffffffffff0000000000108060001080006040001f00000000001c0a80001000000000000c0a80001"
>  > arp_expected
> +
> +net_add n1
> +sim_add hv1
> +as hv1
> +ovs-vsctl \
> +    -- add-br br-phys \
> +    -- add-br br-eth0
> +
> +ovn_attach n1 br-phys 192.168.0.10
> +
> +AT_CHECK([ovs-vsctl set Open_vSwitch . 
> external-ids:ovn-bridge-mappings=physnet1:br-eth0])
> +AT_CHECK([ovs-vsctl add-port br-eth0 snoopvif -- set Interface snoopvif 
> options:tx_pcap=hv1/snoopvif-tx.pcap options:rxq_pcap=hv1/snoopvif-rx.pcap])
> +
> +sim_add hv2
> +as hv2
> +ovs-vsctl \
> +    -- add-br br-phys \
> +    -- add-br br-eth0
> +
> +ovn_attach n1 br-phys 192.168.0.20
> +
> +AT_CHECK([ovs-vsctl set Open_vSwitch . 
> external-ids:ovn-bridge-mappings=physnet1:br-eth0])
> +AT_CHECK([ovs-vsctl add-port br-eth0 snoopvif -- set Interface snoopvif 
> options:tx_pcap=hv2/snoopvif-tx.pcap options:rxq_pcap=hv2/snoopvif-rx.pcap])
> +
> +ovn-sbctl dump-flows > sbflows
> +AT_CAPTURE_FILE([sbflows])
> +
> +# Wait until the patch ports are created in hv1 and hv2 to connect br-int to 
> br-eth0
> +AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
> +OVS_WAIT_UNTIL([test 1 = `as hv1 ovs-vsctl show | \
> +grep "Port patch-br-int-to-ln_port" | wc -l`])
> +AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv2])
> +OVS_WAIT_UNTIL([test 1 = `as hv2 ovs-vsctl show | \
> +grep "Port patch-br-int-to-ln_port" | wc -l`])
> +
> +# Temporarily remove lr0 chassis
> +AT_CHECK([ovn-nbctl remove logical_router lr0 options chassis])
> +
> +reset_pcap_file() {
> +    local hv=$1
> +    local iface=$2
> +    local pcap_file=$3
> +    as $hv
> +    ovs-vsctl -- set Interface $iface options:tx_pcap=dummy-tx.pcap \
> +options:rxq_pcap=dummy-rx.pcap
> +    rm -f ${pcap_file}*.pcap
> +    ovs-vsctl -- set Interface $iface options:tx_pcap=${pcap_file}-tx.pcap \
> +options:rxq_pcap=${pcap_file}-rx.pcap
> +}
> +
> +reset_pcap_file hv1 snoopvif hv1/snoopvif
> +reset_pcap_file hv2 snoopvif hv2/snoopvif
> +
> +hv1_uuid=$(ovn-sbctl --bare --columns _uuid list chassis hv1)
> +AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv1])
> +OVS_WAIT_UNTIL([
> +    ls0_lr0=$(ovn-sbctl --bare --columns chassis list port_binding ls0-lr0)
> +    test "$ls0_lr0" = $hv1_uuid
> +])
> +
> +sleep 2
> +OVN_CHECK_PACKETS_CONTAIN([hv1/snoopvif-tx.pcap], [arp_expected])
> +OVN_CHECK_PACKETS([hv2/snoopvif-tx.pcap], [empty_expected])
> +
> +# Temporarily remove lr0 chassis
> +AT_CHECK([ovn-nbctl remove logical_router lr0 options chassis])
> +
> +reset_pcap_file hv1 snoopvif hv1/snoopvif
> +reset_pcap_file hv2 snoopvif hv2/snoopvif
> +
> +hv2_uuid=$(ovn-sbctl --bare --columns _uuid list chassis hv2)
> +AT_CHECK([ovn-nbctl set logical_router lr0 options:chassis=hv2])
> +OVS_WAIT_UNTIL([
> +    ls0_lr0=$(ovn-sbctl --bare --columns chassis list port_binding ls0-lr0)
> +    test "$ls0_lr0" = $hv2_uuid
> +])
> +
> +sleep 2
> +OVN_CHECK_PACKETS_CONTAIN([hv2/snoopvif-tx.pcap], [arp_expected])
> +OVN_CHECK_PACKETS([hv1/snoopvif-tx.pcap], [empty_expected])
> +
> +OVN_CLEANUP([hv1],[hv2])
> +
> +AT_CLEANUP
> +])
> +
>  OVN_FOR_EACH_NORTHD([
>  AT_SETUP([send gratuitous arp with nat-addresses router in localnet])
>  ovn_start
> --
> 2.35.1
>
> _______________________________________________
> dev mailing list
> [email protected]
> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>

_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev

Reply via email to