On 3/18/25 9:17 AM, Ales Musil via dev wrote:
> On Fri, Mar 14, 2025 at 1:59 PM Xavier Simonart <xsimo...@redhat.com> wrote:
>
>> Signed-off-by: Xavier Simonart <xsimo...@redhat.com>
>> ---
>>
>
> Hi Xavier,
>
> thank you for the patch, I have some minor comments down below.
>
Hi Xavier, Ales,
Thanks for the patch and review!
> tests/multinode-macros.at | 4 +
>> tests/multinode.at | 224 +++++++++++++-------------------------
>> 2 files changed, 78 insertions(+), 150 deletions(-)
>>
>> diff --git a/tests/multinode-macros.at b/tests/multinode-macros.at
>> index 29f0711e6..576098a41 100644
>> --- a/tests/multinode-macros.at
>> +++ b/tests/multinode-macros.at
>> @@ -89,6 +89,10 @@ cleanup_multinode_resources() {
>> done
>> }
>>
>> +multinode_sbctl () {
>> + m_as ovn-central-az1-1 ovn-sbctl "$@"
>> +}
>> +
>> multinode_nbctl () {
>> m_as ovn-central-az1-1 ovn-nbctl "$@"
>> }
>> diff --git a/tests/multinode.at b/tests/multinode.at
>> index 7ff8588c9..f0aeeb346 100644
>> --- a/tests/multinode.at
>> +++ b/tests/multinode.at
>> @@ -2802,6 +2802,53 @@ OVS_WAIT_UNTIL([m_as ovn-chassis-1 ip link show |
>> grep -q genev_sys])
>> OVS_WAIT_UNTIL([m_as ovn-chassis-2 ip link show | grep -q genev_sys])
>> OVS_WAIT_UNTIL([m_as ovn-chassis-3 ip link show | grep -q genev_sys])
>>
>> +# check_ping(src_port, src_chassis, dst_port, expected_status)
>> +# Check whether ping, from src_port on src_chassis to dst_port works as
>> expected.
>> +# expected_status:
>> +# - "success" (or empty): ping should succeed between src and dst.
>> +# - "potential-duplicates: ping should succeed, but we might have
>> potential duplicate packets.
>>
>
> nit: Missing " after potential-duplicates, also would "duplicates" be
> enough with explanation?
>
> +# - "lost": ping should fail, no packets should go through.
>> +# Source ip namespace is derived from previous parameters:
>> +# - parent_port name for container ports.
>> +# - same name as port itself for other ports.
>> +# Dest ip is retrieved from dst_port.
>> +check_ping() {
>> + src_port=$1
>> + src_chassis=$2
>> + dst_port=$3
>> + status=${4-success}
>>
>
> Missing colon after 4.
>
> +
>> + parent_port=$(multinode_sbctl get port_binding $src_port parent_port)
>> + if [[ "$parent_port" != "[]" ]]; then
>> + src_ns=$parent_port
>> + else
>> + src_ns=$src_port
>> + fi
>> + dst_chassis_uuid=$(multinode_sbctl get port_binding $dst_port chassis)
>> + requested_chassis_uuid=$(multinode_sbctl get port_binding $dst_port
>> requested_chassis)
>> + dst_chassis=$(multinode_sbctl --bare --columns name list chassis
>> $dst_chassis_uuid)
>> + dst_ip=$(multinode_nbctl lsp-get-addresses $dst_port | awk '{print
>> $2}')
>> + echo "$src_port on $src_chassis => $dst_port on
>> $dst_chassis(requested_chassis=$requested_chassis_uuid)"
>> + M_NS_CHECK_EXEC([$src_chassis], [$src_ns], [ping -q -c 3 -i 0.3 -w 2
>> $dst_ip | FORMAT_PING], \
>> +[0], [stdout])
>> + if [[ "$status" == "success" ]]; then
>> + AT_CHECK([cat stdout | grep -c "3 packets transmitted, 3
>> received, 0% packet loss"], [0],[dnl
>>
>
> nit: space after ,
>
> +1
>> +])
>> + elif [[ "$status" == "potential-duplicates" ]]; then
>> + AT_CHECK([cat stdout | grep "3 packets transmitted" | grep -c "3
>> received"], [0],[dnl
>>
>
> nit: same here
>
> +1
>> +])
>> + elif [[ "$status" == "lost" ]]; then
>> + AT_CHECK([cat stdout | grep -c "100% packet loss"], [0],[dnl
>>
>
> nit: same here
>
>
>> +1
>> +])
>> + else
>> + echo "unexpected status $status"
>> + AT_FAIL_IF([:])
>> + fi
>> +}
>> +
>> check multinode_nbctl ls-add sw0
>> check multinode_nbctl lsp-add sw0 migrator
>> check multinode_nbctl lsp-set-addresses migrator "50:54:00:00:00:03
>> 10.0.0.3 1000::3"
>> @@ -2813,79 +2860,34 @@ check multinode_nbctl --wait=hv set
>> Logical_Switch_Port migrator options:request
>>
>> m_as ovn-chassis-1 /data/create_fake_vm.sh migrator migrator
>> 50:54:00:00:00:03 1342 10.0.0.3 24 10.0.0.1 1000::3/64 1000::a
>> m_as ovn-chassis-3 /data/create_fake_vm.sh migrator migrator
>> 50:54:00:00:00:03 1342 10.0.0.3 24 10.0.0.1 1000::3/64 1000::a
>> -m_as ovn-chassis-2 /data/create_fake_vm.sh sw0-port2 sw0p2
>> 50:54:00:00:00:04 1342 10.0.0.4 24 10.0.0.1 1000::4/64 1000::a
>> +m_as ovn-chassis-2 /data/create_fake_vm.sh sw0-port2 sw0-port2
>> 50:54:00:00:00:04 1342 10.0.0.4 24 10.0.0.1 1000::4/64 1000::a
>>
>> m_wait_for_ports_up
>>
>> M_START_TCPDUMP([ovn-chassis-1], [-neei genev_sys_6081 arp or ip],
>> [ch1_genev])
>> M_START_TCPDUMP([ovn-chassis-1], [-neei migrator-p arp or ip],
>> [ch1_migrator])
>> M_START_TCPDUMP([ovn-chassis-2], [-neei genev_sys_6081 arp or ip],
>> [ch2_genev])
>> -M_START_TCPDUMP([ovn-chassis-2], [-neei sw0p2-p arp or ip], [ch2_sw0p2])
>> +M_START_TCPDUMP([ovn-chassis-2], [-neei sw0-port2-p arp or ip],
>> [ch2_sw0p2])
>> M_START_TCPDUMP([ovn-chassis-3], [-neei genev_sys_6081 arp or ip],
>> [ch3_genev])
>> M_START_TCPDUMP([ovn-chassis-3], [-neei migrator-p arp or ip],
>> [ch3_migrator])
>>
>> AS_BOX([Migration with vifs])
>> -echo "Migrator on chassis-1 => sw0p2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-1], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 10.0.0.4 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> -
>> -echo "sw0p2 on chassis-2 => Migrator on chassis-1"
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ping -q -c 3 -i 0.3 -w 2
>> 10.0.0.3 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> +check_ping migrator ovn-chassis-1 sw0-port2
>> +check_ping sw0-port2 ovn-chassis-2 migrator
>>
>> echo "== Starting migration =="
>> check multinode_nbctl --wait=hv set Logical_Switch_Port migrator
>> options:requested-chassis=ovn-chassis-1,ovn-chassis-3
>>
>> -echo "Migrator on chassis-1 => sw0p2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-1], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 10.0.0.4 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> -
>> +check_ping migrator ovn-chassis-1 sw0-port2
>> +check_ping migrator ovn-chassis-3 sw0-port2
>>
>> -echo "Migrator on chassis-3 => sw0p2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-3], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 10.0.0.4 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> -
>> -echo "sw0p2 on chassis-2 => migrator on ..."
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ping -q -c 3 -i 0.3 -w 2
>> 10.0.0.3 | FORMAT_PING], \
>> -[0], [stdout])
>> -
>> -# Both VM are running ... We might get duplicates replies.
>> -AT_CHECK([cat stdout | grep "3 packets transmitted" | grep -c "3
>> received"], [0],[dnl
>> -1
>> -])
>> +check_ping sw0-port2 ovn-chassis-2 migrator "potential-duplicates"
>>
>> echo "== Finalizing migration =="
>> check multinode_nbctl --wait=hv set Logical_Switch_Port migrator
>> options:requested-chassis=ovn-chassis-3
>>
>> -echo "Migrator on chassis-1 => sw0p2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-1], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 10.0.0.4 | FORMAT_PING], \
>> -[0], [stdout])
>> -
>> -# VM still running on chassis-1 but flows should have been deleted as
>> migration completed.
>> -AT_CHECK([cat stdout | grep -c "100% packet loss"], [0],[dnl
>> -1
>> -])
>> -
>> -echo "Migrator on chassis-3 => sw0p2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-3], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 10.0.0.4 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> -
>> -# We should not have duplicates anymore
>> -echo "sw0p2 on chassis-2 => migrator on chassis-3"
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ping -q -c 3 -i 0.3 -w 2
>> 10.0.0.3 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> +check_ping migrator ovn-chassis-3 sw0-port2
>> +check_ping sw0-port2 ovn-chassis-2 migrator
>>
>> AS_BOX([Migration with container ports])
>> # Create container ports.
>> @@ -2907,71 +2909,27 @@ M_NS_CHECK_EXEC([ovn-chassis-3], [migrator], [ip
>> link set cont up], [0])
>> M_NS_CHECK_EXEC([ovn-chassis-3], [migrator], [ip addr add 20.0.0.3/24
>> dev cont], [0])
>>
>> # Create the interface for lport sw1-port2
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ip link add link sw0p2 name
>> cont2 type vlan id 10], [0])
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ip link set cont2 address
>> f0:00:00:01:02:04], [0])
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ip link set cont2 up], [0])
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ip addr add 20.0.0.4/24 dev
>> cont2], [0])
>> +M_NS_CHECK_EXEC([ovn-chassis-2], [sw0-port2], [ip link add link sw0-port2
>> name cont2 type vlan id 10], [0])
>> +M_NS_CHECK_EXEC([ovn-chassis-2], [sw0-port2], [ip link set cont2 address
>> f0:00:00:01:02:04], [0])
>> +M_NS_CHECK_EXEC([ovn-chassis-2], [sw0-port2], [ip link set cont2 up], [0])
>> +M_NS_CHECK_EXEC([ovn-chassis-2], [sw0-port2], [ip addr add 20.0.0.4/24
>> dev cont2], [0])
>>
>> -echo "mig-cont on chassis-3 => cont2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-3], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.4 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> -
>> -echo "cont2 on chassis-2 => mig-cont on chassis-3"
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.3 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> +check_ping mig-cont ovn-chassis-3 cont2
>> +check_ping cont2 ovn-chassis-2 mig-cont
>>
>> echo "== Starting migration back =="
>> check multinode_nbctl --wait=hv set Logical_Switch_Port migrator
>> options:requested-chassis=ovn-chassis-3,ovn-chassis-1
>>
>> -echo "mig-cont on chassis-3 => cont2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-3], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.4 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> -
>> -echo "mig-cont on chassis-1 => cont2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-1], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.4 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> -
>> -echo "cont2 on chassis-2 => mig-cont on ..."
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.3 | FORMAT_PING], \
>> -[0], [stdout])
>> -
>> -# Both VM are running ... We might get duplicates replies.
>> -AT_CHECK([cat stdout | grep "3 packets transmitted" | grep -c "3
>> received"], [0],[dnl
>> -1
>> -])
>> +check_ping mig-cont ovn-chassis-3 cont2
>> +check_ping mig-cont ovn-chassis-1 cont2
>> +check_ping cont2 ovn-chassis-2 mig-cont "potential-duplicates"
>>
>> echo "== Finalizing migration =="
>> check multinode_nbctl --wait=hv set Logical_Switch_Port migrator
>> options:requested-chassis=ovn-chassis-1
>>
>> -echo "mig-cont on chassis-3 => cont2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-3], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.4 | FORMAT_PING], \
>> -[0], [stdout])
>> -
>> -# VM still running on chassis-3 but flows should have been deleted....
>> -AT_CHECK([cat stdout | grep -c "100% packet loss"], [0],[dnl
>> -1
>> -])
>> -
>> -echo "mig-cont on chassis-1 => cont2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-1], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.4 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> -
>> -# We should not have duplicates anymore
>> -echo "cont2 on chassis-2 => mig-cont on chassis-1"
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.3 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> +check_ping mig-cont ovn-chassis-3 cont2 "lost"
>> +check_ping mig-cont ovn-chassis-1 cont2
>> +check_ping cont2 ovn-chassis-2 mig-cont
>>
>> echo "== Starting another migration, this time before starting dst VM =="
>> # Unbind migrator from chassis-3
>> @@ -2982,51 +2940,17 @@ sleep 1
>> m_as ovn-chassis-3 ovs-vsctl -- set Interface migrator-p
>> external_ids:iface-id=migrator
>>
>>
>> -echo "mig-cont on chassis-3 => cont2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-3], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.4 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> -
>> -echo "mig-cont on chassis-1 => cont2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-1], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.4 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> -
>> -echo "cont2 on chassis-2 => migrator on ..."
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.3 | FORMAT_PING], \
>> -[0], [stdout])
>> +check_ping mig-cont ovn-chassis-3 cont2
>> +check_ping mig-cont ovn-chassis-1 cont2
>> +check_ping cont2 ovn-chassis-2 mig-cont "potential-duplicates"
>>
>> -# Both VM are running ... We might get duplicates replies.
>> -AT_CHECK([cat stdout | grep "3 packets transmitted" | grep -c "3
>> received"], [0],[dnl
>> -1
>> -])
>>
>> echo "== Finalizing migration =="
>> check multinode_nbctl --wait=hv set Logical_Switch_Port migrator
>> options:requested-chassis=ovn-chassis-3
>>
>> -echo "mig-cont on chassis-1 => cont2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-1], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.4 | FORMAT_PING], \
>> -[0], [stdout])
>> -
>> -# VM still running on chassis-1 but flows should have been deleted....
>> -AT_CHECK([cat stdout | grep -c "100% packet loss"], [0],[dnl
>> -1
>> -])
>> -
>> -echo "mig-cont on chassis-3 => cont2 on chassis-2"
>> -M_NS_CHECK_EXEC([ovn-chassis-3], [migrator], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.4 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> -
>> -# We should not have duplicates anymore
>> -echo "cont2 on chassis-2 => mig-cont on chassis-3"
>> -M_NS_CHECK_EXEC([ovn-chassis-2], [sw0p2], [ping -q -c 3 -i 0.3 -w 2
>> 20.0.0.3 | FORMAT_PING], \
>> -[0], [dnl
>> -3 packets transmitted, 3 received, 0% packet loss, time 0ms
>> -])
>> +check_ping mig-cont ovn-chassis-1 cont2 "lost"
>> +check_ping mig-cont ovn-chassis-3 cont2
>> +check_ping cont2 ovn-chassis-2 mig-cont
>>
>> m_as ovn-chassis-1 killall tcpdump
>> m_as ovn-chassis-2 killall tcpdump
>> --
>> 2.47.1
>>
>> _______________________________________________
>> dev mailing list
>> d...@openvswitch.org
>> https://mail.openvswitch.org/mailman/listinfo/ovs-dev
>>
>>
> Other than that it looks good, thanks.
>
> Acked-by: Ales Musil <amu...@redhat.com>
I took care of the nits and applied this 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
