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

2023-12-01 Thread Ilya Maximets
On 12/1/23 15:14, 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.
> 
> $ DPDK_DIR=/root/ovs/dpdk-dir/v22.11 make -C build check-dpdk \
>   TESTSUITEFLAGS="-d 15"
> ...
>  15: OVS-DPDK - MTU decrease phy portFAILED
>   (system-dpdk.at:574)
> 
> Test log:
> ...
> ../../tests/system-dpdk.at:572: ovs-vsctl set Interface phy0
>   mtu_request=2000
> ../../tests/system-dpdk.at:574: ovs-vsctl get Interface phy0
>   mtu
> --- -   2023-12-01 08:55:46.896454338 -0500
> +++ .../tests/system-dpdk-testsuite.dir/at-groups/15/stdout

Hi, David.  Thanks for the fix!

But, please, don't add diffs like that to the commit message without
indenting or otherwise escaping the '---' and other similar lines.
That confuses different tools.  You may see a 0-day bot failure, for
example.

Please indent quoted text with at least 2 spaces.

Also, exact long paths are not necessary.  They can be trimmed down
to be easier to read.  E.g. '../../tests/' part can be just removed.

>   2023-12-01 08:55:46.894532711 -0500
> @@ -1,2 +1,2 @@
> -2000
> +9000
> 
> ovs-vswitchd log:
> 2023-12-01T13:55:44.666Z|00098|netdev_dpdk|INFO|Port 0:
>   50:7c:6f:3c:0c:26
> 2023-12-01T13:55:44.667Z|00099|netdev_dpdk|INFO|phy0: rx-steering:
>   default rss
> ...
> 2023-12-01T13:55:44.686Z|00102|timeval|WARN|Unreasonably long 1070ms
>   poll interval (339ms user, 728ms system)
> 2023-12-01T13:55:44.686Z|00103|timeval|WARN|faults: 1 minor, 0 major
> 2023-12-01T13:55:44.686Z|00104|timeval|WARN|context switches: 0 voluntary,
>   9 involuntary
> ...
> 2023-12-01T13:55:45.692Z|00150|poll_loop|INFO|wakeup due to [POLLIN] on
>   fd 54 (FIFO pipe:[1781849]) at ../vswitchd/bridge.c:421
>   (68% CPU usage)
> 2023-12-01T13:55:46.883Z|00151|netdev_dpdk|INFO|Port 0:
>   50:7c:6f:3c:0c:26
> 2023-12-01T13:55:46.884Z|00152|netdev_dpdk|INFO|phy0: rx-steering:
>   default rss

Here also, exact date/times and sequence numbers of the logs are
not important and can be removed to make the log easier to read.

Slight indentation would also be nice.

Best regards, Ilya Maximets.
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

2023-12-01 Thread 0-day Robot
Bleep bloop.  Greetings David Marchand, I am a robot and I have tried out your 
patch.
Thanks for your contribution.

I encountered some error that I wasn't expecting.  See the details below.


git-am:
error: patch fragment without header at line 4: @@ -1,2 +1,2 @@
error: could not build fake ancestor
hint: Use 'git am --show-current-patch=diff' to see the failed patch
Patch failed at 0001 system-dpdk: Wait for MTU changes to be applied.
When you have resolved this problem, run "git am --continue".
If you prefer to skip this patch, run "git am --skip" instead.
To restore the original branch and stop patching, run "git am --abort".


Patch skipped due to previous failure.

Please check this out.  If you feel there has been an error, please email 
acon...@redhat.com

Thanks,
0-day Robot
___
dev mailing list
d...@openvswitch.org
https://mail.openvswitch.org/mailman/listinfo/ovs-dev


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

2023-12-01 Thread David Marchand
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.

$ DPDK_DIR=/root/ovs/dpdk-dir/v22.11 make -C build check-dpdk \
TESTSUITEFLAGS="-d 15"
...
 15: OVS-DPDK - MTU decrease phy portFAILED
(system-dpdk.at:574)

Test log:
...
../../tests/system-dpdk.at:572: ovs-vsctl set Interface phy0
mtu_request=2000
../../tests/system-dpdk.at:574: ovs-vsctl get Interface phy0
mtu
--- -   2023-12-01 08:55:46.896454338 -0500
+++ .../tests/system-dpdk-testsuite.dir/at-groups/15/stdout
2023-12-01 08:55:46.894532711 -0500
@@ -1,2 +1,2 @@
-2000
+9000

ovs-vswitchd log:
2023-12-01T13:55:44.666Z|00098|netdev_dpdk|INFO|Port 0:
50:7c:6f:3c:0c:26
2023-12-01T13:55:44.667Z|00099|netdev_dpdk|INFO|phy0: rx-steering:
default rss
...
2023-12-01T13:55:44.686Z|00102|timeval|WARN|Unreasonably long 1070ms
poll interval (339ms user, 728ms system)
2023-12-01T13:55:44.686Z|00103|timeval|WARN|faults: 1 minor, 0 major
2023-12-01T13:55:44.686Z|00104|timeval|WARN|context switches: 0 voluntary,
9 involuntary
...
2023-12-01T13:55:45.692Z|00150|poll_loop|INFO|wakeup due to [POLLIN] on
fd 54 (FIFO pipe:[1781849]) at ../vswitchd/bridge.c:421
(68% CPU usage)
2023-12-01T13:55:46.883Z|00151|netdev_dpdk|INFO|Port 0:
50:7c:6f:3c:0c:26
2023-12-01T13:55:46.884Z|00152|netdev_dpdk|INFO|phy0: rx-steering:
default rss

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 
---
 tests/system-dpdk.at | 42 --
 1 file changed, 12 insertions(+), 30 deletions(-)

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([o