diff mbox series

[ovs-dev,v2] system-dpdk: Wait for MTU changes to be applied.

Message ID 20231201142931.1782046-1-david.marchand@redhat.com
State Accepted
Delegated to: Kevin Traynor
Headers show
Series [ovs-dev,v2] system-dpdk: Wait for MTU changes to be applied. | expand

Checks

Context Check Description
ovsrobot/apply-robot success apply and check: success
ovsrobot/github-robot-_Build_and_Test success github build: passed
ovsrobot/intel-ovs-compilation fail test: fail

Commit Message

David Marchand Dec. 1, 2023, 2:29 p.m. UTC 
Because a DPDK backed netdev configuration is done in an asynchronous way,
and a MTU change requires a reconfiguration, directly checking
ovs-vswitchd logs or querying ovsdb for the interface current MTU value
is racy.

Add synchronisation points on the interface MTU value in ovsdb as it
ensures that a netdev (re)configuration did happen.
With those synchronisation points in place, error messages may be checked
in logs afterward.

Fixes: bf47829116a8 ("tests: Add OVS-DPDK MTU unit tests.")
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changes since v1:
- dropped test output,


---
 tests/system-dpdk.at | 42 ++++++++++++------------------------------
 1 file changed, 12 insertions(+), 30 deletions(-)

Comments

Kevin Traynor Dec. 5, 2023, 2:18 p.m. UTC | #1 
On 01/12/2023 14:29, David Marchand wrote:
> Because a DPDK backed netdev configuration is done in an asynchronous way,
> and a MTU change requires a reconfiguration, directly checking
> ovs-vswitchd logs or querying ovsdb for the interface current MTU value
> is racy.
> 
> Add synchronisation points on the interface MTU value in ovsdb as it
> ensures that a netdev (re)configuration did happen.
> With those synchronisation points in place, error messages may be checked
> in logs afterward.
> 
> Fixes: bf47829116a8 ("tests: Add OVS-DPDK MTU unit tests.")
> Signed-off-by: David Marchand<david.marchand@redhat.com>
> ---
> Changes since v1:
> - dropped test output,
> 
> 
> ---
>   tests/system-dpdk.at | 42 ++++++++++++------------------------------
>   1 file changed, 12 insertions(+), 30 deletions(-)


Thanks David, this removes the race.

Acked-by: Kevin Traynor <ktraynor@redhat.com>
Eelco Chaudron Dec. 7, 2023, 10 a.m. UTC | #2 
On 1 Dec 2023, at 15:29, David Marchand wrote:

> Because a DPDK backed netdev configuration is done in an asynchronous way,
> and a MTU change requires a reconfiguration, directly checking
> ovs-vswitchd logs or querying ovsdb for the interface current MTU value
> is racy.
>
> Add synchronisation points on the interface MTU value in ovsdb as it
> ensures that a netdev (re)configuration did happen.
> With those synchronisation points in place, error messages may be checked
> in logs afterward.
>
> Fixes: bf47829116a8 ("tests: Add OVS-DPDK MTU unit tests.")
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changes since v1:
> - dropped test output,

Thanks for following this true. The changes look good to me.

Acked-by: Eelco Chaudron <echaudro@redhat.com>
Kevin Traynor Dec. 7, 2023, 1:34 p.m. UTC | #3 
On 01/12/2023 14:29, David Marchand wrote:
> Because a DPDK backed netdev configuration is done in an asynchronous way,
> and a MTU change requires a reconfiguration, directly checking
> ovs-vswitchd logs or querying ovsdb for the interface current MTU value
> is racy.
> 
> Add synchronisation points on the interface MTU value in ovsdb as it
> ensures that a netdev (re)configuration did happen.
> With those synchronisation points in place, error messages may be checked
> in logs afterward.
> 
> Fixes: bf47829116a8 ("tests: Add OVS-DPDK MTU unit tests.")
> Signed-off-by: David Marchand<david.marchand@redhat.com>
> ---
> Changes since v1:
> - dropped test output,
> 
> 
> ---
>   tests/system-dpdk.at | 42 ++++++++++++------------------------------
>   1 file changed, 12 insertions(+), 30 deletions(-)

Thanks David. Applied on master branch and backported with a minor 
rebase to the relevant branches (3.0/3.1/3.2).
diff mbox series

Patch

diff --git a/tests/system-dpdk.at b/tests/system-dpdk.at
index 17742d20a0..af092a2000 100644
--- a/tests/system-dpdk.at
+++ b/tests/system-dpdk.at
@@ -511,15 +511,13 @@  dnl Add userspace bridge and attach it to OVS with default MTU value
 AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
 AT_CHECK([ovs-vsctl add-port br10 phy0 -- set Interface phy0 type=dpdk options:dpdk-devargs=$(cat PCI_ADDR)], [], [stdout], [stderr])
 AT_CHECK([ovs-vsctl show], [], [stdout])
-sleep 2
 
 dnl Check default MTU value in the datapath
-AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl
-1500
-])
+OVS_WAIT_UNTIL_EQUAL([ovs-vsctl get Interface phy0 mtu], [1500])
 
 dnl Increase MTU value and check in the datapath
 AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=9000])
+OVS_WAIT_UNTIL_EQUAL([ovs-vsctl get Interface phy0 mtu], [9000])
 
 dnl Fail if MTU is not supported
 AT_FAIL_IF([grep "Interface phy0 does not support MTU configuration" ovs-vswitchd.log], [], [stdout])
@@ -527,10 +525,6 @@  AT_FAIL_IF([grep "Interface phy0 does not support MTU configuration" ovs-vswitch
 dnl Fail if error is encountered during MTU setup
 AT_FAIL_IF([grep "Interface phy0 MTU (9000) setup error" ovs-vswitchd.log], [], [stdout])
 
-AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl
-9000
-])
-
 
 dnl Clean up
 AT_CHECK([ovs-vsctl del-port br10 phy0], [], [stdout], [stderr])
@@ -555,7 +549,9 @@  AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
 AT_CHECK([ovs-vsctl add-port br10 phy0 -- set Interface phy0 type=dpdk options:dpdk-devargs=$(cat PCI_ADDR)], [], [stdout], [stderr])
 AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=9000])
 AT_CHECK([ovs-vsctl show], [], [stdout])
-sleep 2
+
+dnl Check MTU value in the datapath
+OVS_WAIT_UNTIL_EQUAL([ovs-vsctl get Interface phy0 mtu], [9000])
 
 dnl Fail if MTU is not supported
 AT_FAIL_IF([grep "Interface phy0 does not support MTU configuration" ovs-vswitchd.log], [], [stdout])
@@ -563,17 +559,9 @@  AT_FAIL_IF([grep "Interface phy0 does not support MTU configuration" ovs-vswitch
 dnl Fail if error is encountered during MTU setup
 AT_FAIL_IF([grep "Interface phy0 MTU (9000) setup error" ovs-vswitchd.log], [], [stdout])
 
-dnl Check MTU value in the datapath
-AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl
-9000
-])
-
 dnl Decrease MTU value and check in the datapath
 AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=2000])
-
-AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl
-2000
-])
+OVS_WAIT_UNTIL_EQUAL([ovs-vsctl get Interface phy0 mtu], [2000])
 
 
 dnl Clean up
@@ -680,7 +668,9 @@  AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
 AT_CHECK([ovs-vsctl add-port br10 phy0 -- set Interface phy0 type=dpdk options:dpdk-devargs=$(cat PCI_ADDR)], [], [stdout], [stderr])
 AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=9702])
 AT_CHECK([ovs-vsctl show], [], [stdout])
-sleep 2
+
+dnl Check MTU value in the datapath
+OVS_WAIT_UNTIL_EQUAL([ovs-vsctl get Interface phy0 mtu], [9702])
 
 dnl Fail if MTU is not supported
 AT_FAIL_IF([grep "Interface phy0 does not support MTU configuration" ovs-vswitchd.log], [], [stdout])
@@ -688,11 +678,6 @@  AT_FAIL_IF([grep "Interface phy0 does not support MTU configuration" ovs-vswitch
 dnl Fail if error is encountered during MTU setup
 AT_FAIL_IF([grep "Interface phy0 MTU (9702) setup error" ovs-vswitchd.log], [], [stdout])
 
-dnl Check MTU value in the datapath
-AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl
-9702
-])
-
 dnl Set MTU value above upper bound and check for error
 AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=9711])
 AT_CHECK([grep "phy0: unsupported MTU 9711" ovs-vswitchd.log], [], [stdout])
@@ -721,7 +706,9 @@  AT_CHECK([ovs-vsctl add-br br10 -- set bridge br10 datapath_type=netdev])
 AT_CHECK([ovs-vsctl add-port br10 phy0 -- set Interface phy0 type=dpdk options:dpdk-devargs=$(cat PCI_ADDR)], [], [stdout], [stderr])
 AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=68])
 AT_CHECK([ovs-vsctl show], [], [stdout])
-sleep 2
+
+dnl Check MTU value in the datapath
+OVS_WAIT_UNTIL_EQUAL([ovs-vsctl get Interface phy0 mtu], [68])
 
 dnl Fail if MTU is not supported
 AT_FAIL_IF([grep "Interface phy0 does not support MTU configuration" ovs-vswitchd.log], [], [stdout])
@@ -729,11 +716,6 @@  AT_FAIL_IF([grep "Interface phy0 does not support MTU configuration" ovs-vswitch
 dnl Fail if error is encountered during MTU setup
 AT_FAIL_IF([grep "Interface phy0 MTU (68) setup error" ovs-vswitchd.log], [], [stdout])
 
-dnl Check MTU value in the datapath
-AT_CHECK([ovs-vsctl get Interface phy0 mtu], [0], [dnl
-68
-])
-
 dnl Set MTU value below lower bound and check for error
 AT_CHECK([ovs-vsctl set Interface phy0 mtu_request=67])
 AT_CHECK([grep "phy0: unsupported MTU 67" ovs-vswitchd.log], [], [stdout])