Re: [ovs-dev] [PATCH 1/3] system-userspace-macros: Check the exit code of ethtool.

2016-08-05 Thread Daniele Di Proietto 





On 05/08/2016 11:16, "Joe Stringer"  wrote:

>On 4 August 2016 at 18:40, Daniele Di Proietto  wrote:
>> If the ethtool command is not available on the system we should fail,
>> since the userspace testsuite cannot work properly without disabling
>> offloads.
>>
>> Also, add ethtool to the list of installed packages on Vagrantfile.
>>
>> Fixes: ddcf96d2dcc1 ("system-tests: Disable offloads in userspace tests.")
>> Reported-by: Joe Stringer 
>> Signed-off-by: Daniele Di Proietto 
>
>Thanks, this should make it more obvious when offloads are causing failures.
>
>The commit message doesn't really explain why the vagrantfile change
>is in the same commit; a simple mention that it's being added here 'to
>ensure that offloads don't cause test failures in the vagrant VM when
>the kernel is updated' would make it more clear why these two changes
>are in the same commit.

Ok

>
>Acked-by: Joe Stringer 

Thanks! I applied this and the next patch to master
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

Re: [ovs-dev] [PATCH 1/3] system-userspace-macros: Check the exit code of ethtool.

2016-08-05 Thread Joe Stringer 
On 4 August 2016 at 18:40, Daniele Di Proietto  wrote:
> If the ethtool command is not available on the system we should fail,
> since the userspace testsuite cannot work properly without disabling
> offloads.
>
> Also, add ethtool to the list of installed packages on Vagrantfile.
>
> Fixes: ddcf96d2dcc1 ("system-tests: Disable offloads in userspace tests.")
> Reported-by: Joe Stringer 
> Signed-off-by: Daniele Di Proietto 

Thanks, this should make it more obvious when offloads are causing failures.

The commit message doesn't really explain why the vagrantfile change
is in the same commit; a simple mention that it's being added here 'to
ensure that offloads don't cause test failures in the vagrant VM when
the kernel is updated' would make it more clear why these two changes
are in the same commit.

Acked-by: Joe Stringer 
___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev

[ovs-dev] [PATCH 1/3] system-userspace-macros: Check the exit code of ethtool.

2016-08-04 Thread Daniele Di Proietto 
If the ethtool command is not available on the system we should fail,
since the userspace testsuite cannot work properly without disabling
offloads.

Also, add ethtool to the list of installed packages on Vagrantfile.

Fixes: ddcf96d2dcc1 ("system-tests: Disable offloads in userspace tests.")
Reported-by: Joe Stringer 
Signed-off-by: Daniele Di Proietto 
---
 Vagrantfile  | 2 +-
 tests/system-userspace-macros.at | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/Vagrantfile b/Vagrantfile
index fb06b42..843d88c 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -11,7 +11,7 @@ dnf -y install autoconf automake openssl-devel libtool \
python-twisted-core python-zope-interface \
desktop-file-utils groff graphviz rpmdevtools nc \
wget python-six pyftpdlib checkpolicy selinux-policy-devel \
-   libcap-ng-devel kernel-devel-`uname -r`
+   libcap-ng-devel kernel-devel-`uname -r` ethtool
 echo "search extra update built-in" >/etc/depmod.d/search_path.conf
 cd /vagrant
 ./boot.sh
diff --git a/tests/system-userspace-macros.at b/tests/system-userspace-macros.at
index 213425f..7e10b6c 100644
--- a/tests/system-userspace-macros.at
+++ b/tests/system-userspace-macros.at
@@ -55,7 +55,7 @@ m4_define([OVS_TRAFFIC_VSWITCHD_STOP],
 # This is a workaround, and should be removed when offloads are properly
 # supported in netdev-linux.
 m4_define([CONFIGURE_VETH_OFFLOADS],
-[ethtool -K $1 tx off]
+[AT_CHECK([ethtool -K $1 tx off], [0], [ignore])]
 )
 
 # CHECK_CONNTRACK()
-- 
2.8.1

___
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev