Skip to contentSkip to conversation

Several t_client fixes and enhancements

PATCH In review [email protected]... 3 participants 18 messages Nov 8, 2016 – Nov 11, 2016

lore
Share & tools
lore
author
#1 lore raw

PATCH 1/4 Fix update_t_client_ips.sh for out of tree builds

From: Samuli Seppänen <[email protected]...>

Signed-off-by Samuli Seppänen <[email protected]...>
---
 tests/t_client.rc-sample | 4 ++--
 tests/t_client.sh.in     | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)
tests/t_client.rc-sample +2 -2
index 59f34c7..31dfafa 100644
@@ -13,8 +13,8 @@ CLIENT_KEY="${top_srcdir}/sample/sample-keys/client.key"
 CLIENT_CERT="${top_srcdir}/sample/sample-keys/client.crt"
 
 # Load EXPECT_IFCONFIG* parameters from cache
-if [ -r "${top_srcdir}/t_client_ips.rc" ]; then
-    . "${top_srcdir}/t_client_ips.rc"
+if [ -r "${top_builddir}/t_client_ips.rc" ]; then
+    . "${top_builddir}/t_client_ips.rc"
 else
     echo "NOTICE: missing t_client_ips.rc will be auto-generated"
 fi
tests/t_client.sh.in +1 -1
index 6c81bc4..408703e 100755
@@ -275,7 +275,7 @@ do
     # If EXCEPT_IFCONFIG* variables for this test are missing, run an --up
     # script to generate them dynamically.
     if [ -z "$expect_ifconfig4" ] || [ -z "$expect_ifconfig6" ]; then
-        up="--setenv TESTNUM $SUF --setenv TOP_BUILDDIR ${top_builddir} --script-security 2 --up ${top_builddir}/tests/update_t_client_ips.sh"
+        up="--setenv TESTNUM $SUF --setenv TOP_BUILDDIR ${top_builddir} --script-security 2 --up ${srcdir}/update_t_client_ips.sh"
     fi
 
     echo -e "\n### test run $SUF: '$test_run_title' ###\n"
Signature · 1 line
2.1.4
#2 lore raw

[Openvpn-devel] [PATCH applied] Re: Fix update_t_client_ips.sh for out of tree builds

ACK. Code change looks good, and verifiably fixes the problem :-)

(Note that there needs to be 5/4 that resets up='' when not needed...)

Your patch has been applied to the master branch.

commit 2af1fa8b19e9723b29f4a4c198f61478ae9e688e

Author: Samuli Seppänen
Date:   Tue Nov 8 14:55:26 2016 +0200
     Fix update_t_client_ips.sh for out of tree builds

     Signed-off-by: Samuli Seppänen <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg12952.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

author
#3 lore raw

PATCH 2/4 Prevent generation of duplicate EXPECT_IFCONFIG entries

In reply to [email protected]...

From: Samuli Seppänen <[email protected]...>

Previously, if t_client.rc did not source t_client_ips.rc, update_t_client_ips.sh would add (the same) EXPECT_IFCONFIG entries to t_client_ips.rc on every run. This patch makes update_t_client_ips.sh check if the entry exists before trying to add it.

Signed-off-by Samuli Seppänen <[email protected]...>
---
 tests/update_t_client_ips.sh | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
tests/update_t_client_ips.sh +11 -2
index e7b58ba..438220d 100755
@@ -3,5 +3,14 @@
 # This --up script caches the IPs handed out by the test VPN server to a file
 # for later use.
 
-echo "EXPECT_IFCONFIG4_$TESTNUM=$ifconfig_local" >> $TOP_BUILDDIR/t_client_ips.rc
-echo "EXPECT_IFCONFIG6_$TESTNUM=$ifconfig_ipv6_local" >> $TOP_BUILDDIR/t_client_ips.rc
+RC="$TOP_BUILDDIR/t_client_ips.rc"
+
+grep EXPECT_IFCONFIG4_$TESTNUM $RC > /dev/null 2>&1
+if [ $? -ne 0 ]; then
+    echo "EXPECT_IFCONFIG4_$TESTNUM=$ifconfig_local" >> $RC
+fi
+
+grep EXPECT_IFCONFIG6_$TESTNUM $RC > /dev/null 2>&1
+if [ $? -ne 0 ]; then
+    echo "EXPECT_IFCONFIG6_$TESTNUM=$ifconfig_ipv6_local" >> $RC
+fi
Signature · 1 line
2.1.4
#4 lore raw

Re: [Openvpn-devel] [PATCH 2/4] Prevent generation of duplicate EXPECT_IFCONFIG entries

Hi,

Quoted 1 line · On Tue, Nov 08, 2016 at 02:55:27PM +0200, [email protected]... wrote:

+grep EXPECT_IFCONFIG4_$TESTNUM $RC > /dev/null 2>&1

NAK. This will grep for stuff like "EXPECT_IFCONFIG4_1", and if the RC file has

EXPECT_IFCONFIG4_1a=10.194.1.54

the grep will be successful, and "EXPECT_IFCONFIG4_1" will never be added.

So this needs to be something like

grep EXPECT_IFCONFIG4_$TESTNUM= $RC > /dev/null 2>&1

                             ^^

and

grep EXPECT_IFCONFIG6_$TESTNUM= $RC > /dev/null 2>&1

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#5 lore raw

PATCH v2 2/4 Prevent generation of duplicate EXPECT_IFCONFIG entries

In reply to [email protected]...

From: Samuli Seppänen <[email protected]...>

Previously, if t_client.rc did not source t_client_ips.rc, update_t_client_ips.sh would add (the same) EXPECT_IFCONFIG entries to t_client_ips.rc on every run. This patch makes update_t_client_ips.sh check if the entry exists before trying to add it.

v2: prevent partial matches of the EXCEPT_IFCONFIG variable name

Signed-off-by Samuli Seppänen <[email protected]...>
---
 tests/update_t_client_ips.sh | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
tests/update_t_client_ips.sh +11 -2
index e7b58ba..96e3826 100755
@@ -3,5 +3,14 @@
 # This --up script caches the IPs handed out by the test VPN server to a file
 # for later use.
 
-echo "EXPECT_IFCONFIG4_$TESTNUM=$ifconfig_local" >> $TOP_BUILDDIR/t_client_ips.rc
-echo "EXPECT_IFCONFIG6_$TESTNUM=$ifconfig_ipv6_local" >> $TOP_BUILDDIR/t_client_ips.rc
+RC="$TOP_BUILDDIR/t_client_ips.rc"
+
+grep EXPECT_IFCONFIG4_$TESTNUM= $RC > /dev/null 2>&1
+if [ $? -ne 0 ]; then
+    echo "EXPECT_IFCONFIG4_$TESTNUM=$ifconfig_local" >> $RC
+fi
+
+grep EXPECT_IFCONFIG6_$TESTNUM= $RC > /dev/null 2>&1
+if [ $? -ne 0 ]; then
+    echo "EXPECT_IFCONFIG6_$TESTNUM=$ifconfig_ipv6_local" >> $RC
+fi
Signature · 1 line
2.1.4
#6 lore raw

[Openvpn-devel] [PATCH applied] Re: Prevent generation of duplicate EXPECT_IFCONFIG entries

ACK. Does what it says on the lid (and no more) :-)

Your patch has been applied to the master branch.

commit 82a601f1e23d1c37bc2120dca12cadc4dbf2b4fc

Author: Samuli Seppänen
Date:   Tue Nov 8 16:06:03 2016 +0200
     Prevent generation of duplicate EXPECT_IFCONFIG entries

     Signed-off-by: Samuli Seppänen <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg12965.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

author
#7 lore raw

PATCH 3/4 Make sure that all relevant files under test go to release tarballs

In reply to [email protected]...

From: Samuli Seppänen <[email protected]...>

Signed-off-by Samuli Seppänen <[email protected]...>
---
 tests/Makefile.am | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)
tests/Makefile.am +5 -1
index 235cd13..e55928b 100644
@@ -24,4 +24,8 @@ TESTS = $(test_scripts)
 
 dist_noinst_SCRIPTS = \
 	$(test_scripts) \
-	t_cltsrv-down.sh
+	t_cltsrv-down.sh \
+	update_t_client_ips.sh
+
+dist_noinst_DATA = \
+	t_client.rc-sample
Signature · 1 line
2.1.4
#8 lore raw

[Openvpn-devel] [PATCH applied] Re: Make sure that all relevant files under test go to release tarballs

ACK, thanks for catching this :-)

Your patch has been applied to the master branch.

commit e81023f00bbc503c41cea0a988a0ac00edae754a

Author: Samuli Seppänen
Date:   Tue Nov 8 14:55:28 2016 +0200
     Make sure that all relevant files under test go to release tarballs

     Signed-off-by: Samuli Seppänen <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg12955.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

author
#9 lore raw

PATCH 4/4 Allow passing extra arguments to fping/fping6 in t_client.rc

In reply to [email protected]...

From: Samuli Seppänen <[email protected]...>

This can be useful, for example, in preventing fping failures caused by external network issues.

Signed-off-by Samuli Seppänen <[email protected]...>
---
 tests/t_client.rc-sample | 1 +
 tests/t_client.sh.in     | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)
tests/t_client.rc-sample +1 -0
index 31dfafa..4fdea48 100644
@@ -11,6 +11,7 @@ top_srcdir="${top_srcdir:-..}"
 CA_CERT="${top_srcdir}/sample/sample-keys/ca.crt"
 CLIENT_KEY="${top_srcdir}/sample/sample-keys/client.key"
 CLIENT_CERT="${top_srcdir}/sample/sample-keys/client.crt"
+#FPING_EXTRA_ARGS="-t 1000"
 
 # Load EXPECT_IFCONFIG* parameters from cache
 if [ -r "${top_builddir}/t_client_ips.rc" ]; then
tests/t_client.sh.in +2 -2
index 408703e..9b980cf 100755
@@ -231,8 +231,8 @@ run_ping_tests()
     do
 	echo "run IPv$proto ping tests ($want), $bytes byte packets..."
 
-	echo "$cmd -b $bytes -C 20 -p 250 -q $targetlist" >>$LOGDIR/$SUF:fping.out
-	$cmd -b $bytes -C 20 -p 250 -q $targetlist >>$LOGDIR/$SUF:fping.out 2>&1
+	echo "$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist" >>$LOGDIR/$SUF:fping.out
+	$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist >>$LOGDIR/$SUF:fping.out 2>&1
 
 	# while OpenVPN is running, pings must succeed (want='want_ok')
 	# before OpenVPN is up, pings must NOT succeed (want='want_fail')
Signature · 1 line
2.1.4
#10 lore raw

Re: [Openvpn-devel] [PATCH 4/4] Allow passing extra arguments to fping/fping6 in t_client.rc

Hi,

tests/t_client.sh.in run_ping_tests() · line 235

Quoted 4 lines · On Tue, Nov 08, 2016 at 02:55:29PM +0200, [email protected]... wrote:
tests/t_client.sh.in +2 -2
-	echo "$cmd -b $bytes -C 20 -p 250 -q $targetlist" >>$LOGDIR/$SUF:fping.out
-	$cmd -b $bytes -C 20 -p 250 -q $targetlist >>$LOGDIR/$SUF:fping.out 2>&1
+	echo "$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist" >>$LOGDIR/$SUF:fping.out
+	$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist >>$LOGDIR/$SUF:fping.out 2>&1

I would put the args at the end, so if needed, "-p 250" or "-C 20" could be overriden.

Thus, feature-NAK.

Code-Change is OK :-)

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
#11 lore raw

Re: [Openvpn-devel] [PATCH 4/4] Allow passing extra arguments to fping/fping6 in t_client.rc

Hi,

Quoted 5 lines · Gert Doering

tests/t_client.sh.in run_ping_tests() · line 235

Quoted 2 lines
tests/t_client.sh.in +2 -0
+	echo "$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist" >>$LOGDIR/$SUF:fping.out
+	$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist >>$LOGDIR/$SUF:fping.out 2>&1

I would put the args at the end, so if needed, "-p 250" or "-C 20" could be overriden.

Well, "at the end" would be "before $targetlist", obviously :-)

Thus:

+ $cmd -b $bytes -C 20 -p 250 -q $FPING_EXTRA_ARGS $targetlist >>$LOGDIR/$SUF:fping.out 2>&1

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#12 lore raw

PATCH v2 4/4 Allow passing extra arguments to fping/fping6 in t_client.rc

In reply to [email protected]...

From: Samuli Seppänen <[email protected]...>

This can be useful, for example, in preventing fping failures caused by external network issues.

v2:
- Allow override of the default parameters

Signed-off-by Samuli Seppänen <[email protected]...>
---
 tests/t_client.rc-sample | 1 +
 tests/t_client.sh.in     | 4 ++--
 2 files changed, 3 insertions(+), 2 deletions(-)
tests/t_client.rc-sample +1 -0
index 31dfafa..4fdea48 100644
@@ -11,6 +11,7 @@ top_srcdir="${top_srcdir:-..}"
 CA_CERT="${top_srcdir}/sample/sample-keys/ca.crt"
 CLIENT_KEY="${top_srcdir}/sample/sample-keys/client.key"
 CLIENT_CERT="${top_srcdir}/sample/sample-keys/client.crt"
+#FPING_EXTRA_ARGS="-t 1000"
 
 # Load EXPECT_IFCONFIG* parameters from cache
 if [ -r "${top_builddir}/t_client_ips.rc" ]; then
tests/t_client.sh.in +2 -2
index 408703e..076f6bd 100755
@@ -231,8 +231,8 @@ run_ping_tests()
     do
 	echo "run IPv$proto ping tests ($want), $bytes byte packets..."
 
-	echo "$cmd -b $bytes -C 20 -p 250 -q $targetlist" >>$LOGDIR/$SUF:fping.out
-	$cmd -b $bytes -C 20 -p 250 -q $targetlist >>$LOGDIR/$SUF:fping.out 2>&1
+	echo "$cmd -b $bytes -C 20 -p 250 -q $FPING_EXTRA_ARGS $targetlist" >>$LOGDIR/$SUF:fping.out
+	$cmd -b $bytes -C 20 -p 250 -q $FPING_EXTRA_ARGS $targetlist >>$LOGDIR/$SUF:fping.out 2>&1
 
 	# while OpenVPN is running, pings must succeed (want='want_ok')
 	# before OpenVPN is up, pings must NOT succeed (want='want_fail')
Signature · 1 line
2.1.4
#13 lore raw

[Openvpn-devel] [PATCH applied] Re: Allow passing extra arguments to fping/fping6 in t_client.rc

ACK. Tested with FPING_EXTRA_ARGS="-C 5" as "proof of concept".

Your patch has been applied to the master branch.

commit 6f17e18be0d6ab801704400abcc6b17d4fed9650

Author: Samuli Seppänen
Date:   Tue Nov 8 15:50:43 2016 +0200
     Allow passing extra arguments to fping/fping6 in t_client.rc

     Signed-off-by: Samuli Seppänen <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: https://www.mail-archive.com/[email protected]/msg12962.html
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering

#14 lore raw

Re: [Openvpn-devel] [PATCHv2 4/4] Allow passing extra arguments to fping/fping6 in t_client.rc

In reply to [email protected]...

tests/t_client.sh.in run_ping_tests() · line 235

Quoted 3 lines · On 08/11/16 13:50, [email protected]... wrote:

From: Samuli Seppänen <[email protected]...>

tests/t_client.sh.in +1 -0
+	$cmd -b $bytes -C 20 -p 250 -q $FPING_EXTRA_ARGS $targetlist >>$LOGDIR/$SUF:fping.out 2>&1

This allows $FPING_EXTRA_ARGS to take precedence over previous arguments

eg: $FPING_EXTRA_ARGS = "-C 1000 -p 1000 -i 1000"

This does not:
$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist $LOGDIR/$SUF:fping.out 2>&1

Sorry I have not diff'd this.

Regards

#15 lore raw

Re: [Openvpn-devel] [PATCHv2 4/4] Allow passing extra arguments to fping/fping6 in t_client.rc

Hi,

Quoted 8 lines · On Fri, Nov 11, 2016 at 12:25:49AM +0000, debbie10t wrote:

tests/t_client.sh.in run_ping_tests() · line 235

Quoted 3 lines · On 08/11/16 13:50, [email protected]... wrote:

From: Samuli Seppänen <[email protected]...>

tests/t_client.sh.in +1 -0
+	$cmd -b $bytes -C 20 -p 250 -q $FPING_EXTRA_ARGS $targetlist >>$LOGDIR/$SUF:fping.out 2>&1

This allows $FPING_EXTRA_ARGS to take precedence over previous arguments

eg: $FPING_EXTRA_ARGS = "-C 1000 -p 1000 -i 1000"

Right, which was the point of "v2" :-) - it's a good way to shoot your own foot, but this is an "expert mode knob".

Quoted 3 lines

This does not:
$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist $LOGDIR/$SUF:fping.out 2>&1

Right, which was v1.

gert

Signature · 4 lines
USENET is *not* the non-clickable part of WWW!
                                                           //www.muc.de/~gert/
Gert Doering - Munich, Germany                             [email protected]...
fax: +49-89-35655025                        [email protected]...
author
#16 lore raw

Re: [Openvpn-devel] [PATCHv2 4/4] Allow passing extra arguments to fping/fping6 in t_client.rc

Il 11/11/2016 09:58, Gert Doering ha scritto:

Quoted 15 lines

Hi,

Quoted 8 lines · On Fri, Nov 11, 2016 at 12:25:49AM +0000, debbie10t wrote:

tests/t_client.sh.in run_ping_tests() · line 235

Quoted 3 lines · On 08/11/16 13:50, [email protected]... wrote:

From: Samuli Seppänen <[email protected]...>

tests/t_client.sh.in +1 -0
+	$cmd -b $bytes -C 20 -p 250 -q $FPING_EXTRA_ARGS $targetlist >>$LOGDIR/$SUF:fping.out 2>&1

This allows $FPING_EXTRA_ARGS to take precedence over previous arguments

eg: $FPING_EXTRA_ARGS = "-C 1000 -p 1000 -i 1000"

Right, which was the point of "v2" :-) - it's a good way to shoot your own foot, but this is an "expert mode knob".

Quoted 7 lines
Quoted 3 lines

This does not:
$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist $LOGDIR/$SUF:fping.out 2>&1

Right, which was v1.

gert

In any case, thanks for the review to both of you!

Signature · 5 lines
Samuli Seppänen
Community Manager
OpenVPN Technologies, Inc

irc freenode net: mattock
#17 lore raw

Re: [Openvpn-devel] [PATCHv2 4/4] Allow passing extra arguments to fping/fping6 in t_client.rc

Quoted 16 lines · Samuli Seppänen

Il 11/11/2016 09:58, Gert Doering ha scritto:

Quoted 15 lines

Hi,

Quoted 9 lines · On Fri, Nov 11, 2016 at 12:25:49AM +0000, debbie10t wrote:
Quoted 4 lines · On 08/11/16 13:50, [email protected]... wrote:

From: Samuli Seppänen <[email protected]...>

+ $cmd -b $bytes -C 20 -p 250 -q $FPING_EXTRA_ARGS $targetlist

Quoted 1 line
Quoted 1 line

$LOGDIR/$SUF:fping.out 2>&1

This allows $FPING_EXTRA_ARGS to take precedence over previous arguments

eg: $FPING_EXTRA_ARGS = "-C 1000 -p 1000 -i 1000"

Right, which was the point of "v2" :-) - it's a good way to shoot your own foot, but this is an "expert mode knob".

OK, I misunderstood the requirement but now that is clear.

Quoted 10 lines
Quoted 8 lines
Quoted 4 lines

This does not:
$cmd $FPING_EXTRA_ARGS -b $bytes -C 20 -p 250 -q $targetlist $LOGDIR/$SUF:fping.out 2>&1

Right, which was v1.

gert

In any case, thanks for the review to both of you!

Doing my best :-)