diff mbox series

[Openvpn-devel] t_net.sh: wait for NO-CARRIER bit to settle before starting test

Message ID 20190919072820.9913-1-a@unstable.cc
State Accepted
Headers
Series [Openvpn-devel] t_net.sh: wait for NO-CARRIER bit to settle before starting test |

Commit Message

Antonio Quartulli Sept. 18, 2019, 9:28 p.m. UTC 
  Interfaces of type tun are marked as NO-CARRIER when no process is
attached to them. However, this bit gets set with some delay after
creation.

For this reason, it is better to wait for the bit to settle before
starting any test, otherwise any timing influence on the test may lead
to inconsistencies due to the NO-CARRIER bit randomly being or not in
the snapshot output taken by t_net.sh.

This patch add a 'sleep 1' command right after creation of the
interface, to give the NO-CARRIER bit a chance to settle.

This issue has been witnessed on a buildbot that is
apparently slowler than average to run the unit tests.

Signed-off-by: Antonio Quartulli <a@unstable.cc>
---
 tests/t_net.sh | 7 +++++++
 1 file changed, 7 insertions(+)

Comments

Gert Doering Sept. 18, 2019, 9:44 p.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

Your patch has been applied to the master branch.

"Your script, you know what is happening behind the scenes, so it's good
enough for me" :-)

commit 8aa037161f37c42585ac4a8d99f37af624211da6
Author: Antonio Quartulli
Date:   Thu Sep 19 09:28:20 2019 +0200

     t_net.sh: wait for NO-CARRIER bit to settle before starting test

     Signed-off-by: Antonio Quartulli <a@unstable.cc>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20190919072820.9913-1-a@unstable.cc>
     URL: https://www.mail-archive.com/search?l=mid&q=20190919072820.9913-1-a@unstable.cc
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
Steffan Karger Sept. 18, 2019, 11:17 p.m. UTC | #2 
Hi,

On 19-09-19 09:28, Antonio Quartulli wrote:
> Interfaces of type tun are marked as NO-CARRIER when no process is
> attached to them. However, this bit gets set with some delay after
> creation.
> 
> For this reason, it is better to wait for the bit to settle before
> starting any test, otherwise any timing influence on the test may lead
> to inconsistencies due to the NO-CARRIER bit randomly being or not in
> the snapshot output taken by t_net.sh.
> 
> This patch add a 'sleep 1' command right after creation of the
> interface, to give the NO-CARRIER bit a chance to settle.
> 
> This issue has been witnessed on a buildbot that is
> apparently slowler than average to run the unit tests.
> 
> Signed-off-by: Antonio Quartulli <a@unstable.cc>
> ---
>  tests/t_net.sh | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/tests/t_net.sh b/tests/t_net.sh
> index 18799d12..97e947ab 100755
> --- a/tests/t_net.sh
> +++ b/tests/t_net.sh
> @@ -34,6 +34,13 @@ reload_dummy()
>  {
>      $RUN_SUDO $openvpn --dev $IFACE --dev-type tun --rmtun >/dev/null
>      $RUN_SUDO $openvpn --dev $IFACE --dev-type tun --mktun >/dev/null
> +
> +    # it seems that tun devices will settle on NO-CARRIER while not connected to
> +    # any process, but this won't happen immediately. To avoid having the
> +    # NO-CARRIER bit appear in the middle of the tests - which would compromise
> +    # the results - let's wait 1 sec here for it to settle.
> +    sleep 1
> +
>      if [ $? -ne 0 ]; then
>          echo "can't create interface $IFACE"
>          exit 1
> 

This return value check now checks that sleep succeeded instead of the
openvpn --mktun command. I don't think that is what you intended.

-Steffan
Antonio Quartulli Sept. 19, 2019, 2:13 a.m. UTC | #3 
Hi,

On 19/09/2019 11:17, Steffan Karger wrote:
>> diff --git a/tests/t_net.sh b/tests/t_net.sh
>> index 18799d12..97e947ab 100755
>> --- a/tests/t_net.sh
>> +++ b/tests/t_net.sh
>> @@ -34,6 +34,13 @@ reload_dummy()
>>  {
>>      $RUN_SUDO $openvpn --dev $IFACE --dev-type tun --rmtun >/dev/null
>>      $RUN_SUDO $openvpn --dev $IFACE --dev-type tun --mktun >/dev/null
>> +
>> +    # it seems that tun devices will settle on NO-CARRIER while not connected to
>> +    # any process, but this won't happen immediately. To avoid having the
>> +    # NO-CARRIER bit appear in the middle of the tests - which would compromise
>> +    # the results - let's wait 1 sec here for it to settle.
>> +    sleep 1
>> +
>>      if [ $? -ne 0 ]; then
>>          echo "can't create interface $IFACE"
>>          exit 1
>>
> 
> This return value check now checks that sleep succeeded instead of the
> openvpn --mktun command. I don't think that is what you intended.
> 

You're absolutely right.
The sleep should be moved right after the if-check.

Will send a new patch to fix that.

Regards,
diff mbox series

Patch

diff --git a/tests/t_net.sh b/tests/t_net.sh
index 18799d12..97e947ab 100755
--- a/tests/t_net.sh
+++ b/tests/t_net.sh
@@ -34,6 +34,13 @@  reload_dummy()
 {
     $RUN_SUDO $openvpn --dev $IFACE --dev-type tun --rmtun >/dev/null
     $RUN_SUDO $openvpn --dev $IFACE --dev-type tun --mktun >/dev/null
+
+    # it seems that tun devices will settle on NO-CARRIER while not connected to
+    # any process, but this won't happen immediately. To avoid having the
+    # NO-CARRIER bit appear in the middle of the tests - which would compromise
+    # the results - let's wait 1 sec here for it to settle.
+    sleep 1
+
     if [ $? -ne 0 ]; then
         echo "can't create interface $IFACE"
         exit 1