All of lore.kernel.org
 help / color / mirror / Atom feed
* [LTP] [PATCH 1/1] net/traceroute01: Check also -T flag
@ 2020-11-12 17:36 Petr Vorel
  2020-11-19 14:23 ` =?unknown-8bit?q?K=C3=B6ry?= Maincent
  0 siblings, 1 reply; 7+ messages in thread
From: Petr Vorel @ 2020-11-12 17:36 UTC (permalink / raw)
  To: ltp

From: Petr Vorel <pvorel@suse.cz>

and move checks to run_trace()

There are 3 traceroute versions:

* Dmitry Butskoy (http://traceroute.sourceforge.net/)
* busybox
* iputils (only tracepath6; deprecated, but still used (e.g. OpenWrt Project)

-I is supported by Dmitry Butskoy's and busybox implementation
-T is supported only by Dmitry Butskoy's implementation

Signed-off-by: Petr Vorel <pvorel@suse.cz>
---
Hi Alexey, Kory,

follow up to Kory's fix (there are some problems with suse.cz
mailserver, thus I haven't reply about pushing your patch).

Kind regards,
Petr

 testcases/network/traceroute/traceroute01.sh | 30 +++++++++++---------
 1 file changed, 17 insertions(+), 13 deletions(-)

diff --git a/testcases/network/traceroute/traceroute01.sh b/testcases/network/traceroute/traceroute01.sh
index 38f4d3b85..90030af39 100755
--- a/testcases/network/traceroute/traceroute01.sh
+++ b/testcases/network/traceroute/traceroute01.sh
@@ -13,9 +13,12 @@ TST_NEEDS_TMPDIR=1
 
 setup()
 {
-	tst_res TINFO "traceroute version:"
-	tst_res TINFO $(traceroute --version 2>&1)
-	[ "$TST_IPV6" ] && tst_res TINFO "NOTE: tracepath6 from iputils is not supported"
+
+	TRACEROUTE=traceroute${TST_IPV6}
+	tst_require_cmds $TRACEROUTE
+
+	tst_res TINFO "$TRACEROUTE version:"
+	tst_res TINFO $($TRACEROUTE --version 2>&1)
 }
 
 run_trace()
@@ -24,18 +27,23 @@ run_trace()
 	local ip=$(tst_ipaddr rhost)
 	local pattern="^[ ]+1[ ]+$ip([ ]+[0-9]+[.][0-9]+ ms){3}"
 
+	if $TRACEROUTE $opts 2>&1 | grep -q "invalid option"; then
+		tst_res TCONF "$opts flag not supported"
+		return
+	fi
+
 	# According to man pages, default sizes:
 	local bytes=60
 	[ "$TST_IPV6" ] && bytes=80
 
-	EXPECT_PASS traceroute $ip $bytes -n -m 2 $opts \>out.log 2>&1
+	EXPECT_PASS $TRACEROUTE $ip $bytes -n -m 2 $opts \>out.log 2>&1
 
 	grep -q "$bytes byte" out.log
 	if [ $? -ne 0 ]; then
 		cat out.log
 		tst_res TFAIL "'$bytes byte' not found"
 	else
-		tst_res TPASS "traceroute use $bytes bytes"
+		tst_res TPASS "$TRACEROUTE use $bytes bytes"
 	fi
 
 	tail -1 out.log | grep -qE "$pattern"
@@ -43,24 +51,20 @@ run_trace()
 		cat out.log
 		tst_res TFAIL "pattern '$pattern' not found in log"
 	else
-		tst_res TPASS "traceroute test completed with 1 hop"
+		tst_res TPASS "$TRACEROUTE test completed with 1 hop"
 	fi
 }
 
 test1()
 {
-	tst_res TINFO "run traceroute with ICMP ECHO"
+	tst_res TINFO "run $TRACEROUTE with ICMP ECHO"
 	run_trace -I
 }
 
 test2()
 {
-	tst_res TINFO "run traceroute with TCP SYN"
-	if traceroute -T 2>&1 | grep -q "invalid option"; then
-		tst_res TCONF "-T flag (TCP SYN) not supported"
-	else
-		run_trace -T
-	fi
+	tst_res TINFO "run $TRACEROUTE with TCP SYN"
+	run_trace -T
 }
 
 tst_run
-- 
2.29.2


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [LTP] [PATCH 1/1] net/traceroute01: Check also -T flag
  2020-11-12 17:36 [LTP] [PATCH 1/1] net/traceroute01: Check also -T flag Petr Vorel
@ 2020-11-19 14:23 ` =?unknown-8bit?q?K=C3=B6ry?= Maincent
  2020-11-19 15:41   ` Petr Vorel
  0 siblings, 1 reply; 7+ messages in thread
From: =?unknown-8bit?q?K=C3=B6ry?= Maincent @ 2020-11-19 14:23 UTC (permalink / raw)
  To: ltp

Hello Petr,

Just find out you didn't merge your patch which makes the code cleaner. :)

Regards,


On Thu, 12 Nov 2020 18:36:09 +0100
Petr Vorel <petr.vorel@suse.com> wrote:

> From: Petr Vorel <pvorel@suse.cz>
> 
> and move checks to run_trace()
> 
> There are 3 traceroute versions:
> 
> * Dmitry Butskoy (http://traceroute.sourceforge.net/)
> * busybox
> * iputils (only tracepath6; deprecated, but still used (e.g. OpenWrt Project)
> 
> -I is supported by Dmitry Butskoy's and busybox implementation
> -T is supported only by Dmitry Butskoy's implementation
> 
> Signed-off-by: Petr Vorel <pvorel@suse.cz>
> ---
> Hi Alexey, Kory,
> 
> follow up to Kory's fix (there are some problems with suse.cz
> mailserver, thus I haven't reply about pushing your patch).
> 
> Kind regards,
> Petr
> 
>  testcases/network/traceroute/traceroute01.sh | 30 +++++++++++---------
>  1 file changed, 17 insertions(+), 13 deletions(-)
> 
> diff --git a/testcases/network/traceroute/traceroute01.sh
> b/testcases/network/traceroute/traceroute01.sh index 38f4d3b85..90030af39
> 100755 --- a/testcases/network/traceroute/traceroute01.sh
> +++ b/testcases/network/traceroute/traceroute01.sh
> @@ -13,9 +13,12 @@ TST_NEEDS_TMPDIR=1
>  
>  setup()
>  {
> -	tst_res TINFO "traceroute version:"
> -	tst_res TINFO $(traceroute --version 2>&1)
> -	[ "$TST_IPV6" ] && tst_res TINFO "NOTE: tracepath6 from iputils is
> not supported" +
> +	TRACEROUTE=traceroute${TST_IPV6}
> +	tst_require_cmds $TRACEROUTE
> +
> +	tst_res TINFO "$TRACEROUTE version:"
> +	tst_res TINFO $($TRACEROUTE --version 2>&1)
>  }
>  
>  run_trace()
> @@ -24,18 +27,23 @@ run_trace()
>  	local ip=$(tst_ipaddr rhost)
>  	local pattern="^[ ]+1[ ]+$ip([ ]+[0-9]+[.][0-9]+ ms){3}"
>  
> +	if $TRACEROUTE $opts 2>&1 | grep -q "invalid option"; then
> +		tst_res TCONF "$opts flag not supported"
> +		return
> +	fi
> +
>  	# According to man pages, default sizes:
>  	local bytes=60
>  	[ "$TST_IPV6" ] && bytes=80
>  
> -	EXPECT_PASS traceroute $ip $bytes -n -m 2 $opts \>out.log 2>&1
> +	EXPECT_PASS $TRACEROUTE $ip $bytes -n -m 2 $opts \>out.log 2>&1
>  
>  	grep -q "$bytes byte" out.log
>  	if [ $? -ne 0 ]; then
>  		cat out.log
>  		tst_res TFAIL "'$bytes byte' not found"
>  	else
> -		tst_res TPASS "traceroute use $bytes bytes"
> +		tst_res TPASS "$TRACEROUTE use $bytes bytes"
>  	fi
>  
>  	tail -1 out.log | grep -qE "$pattern"
> @@ -43,24 +51,20 @@ run_trace()
>  		cat out.log
>  		tst_res TFAIL "pattern '$pattern' not found in log"
>  	else
> -		tst_res TPASS "traceroute test completed with 1 hop"
> +		tst_res TPASS "$TRACEROUTE test completed with 1 hop"
>  	fi
>  }
>  
>  test1()
>  {
> -	tst_res TINFO "run traceroute with ICMP ECHO"
> +	tst_res TINFO "run $TRACEROUTE with ICMP ECHO"
>  	run_trace -I
>  }
>  
>  test2()
>  {
> -	tst_res TINFO "run traceroute with TCP SYN"
> -	if traceroute -T 2>&1 | grep -q "invalid option"; then
> -		tst_res TCONF "-T flag (TCP SYN) not supported"
> -	else
> -		run_trace -T
> -	fi
> +	tst_res TINFO "run $TRACEROUTE with TCP SYN"
> +	run_trace -T
>  }
>  
>  tst_run



-- 
K?ry Maincent, Bootlin
Embedded Linux and kernel engineering
https://bootlin.com

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [LTP] [PATCH 1/1] net/traceroute01: Check also -T flag
  2020-11-19 14:23 ` =?unknown-8bit?q?K=C3=B6ry?= Maincent
@ 2020-11-19 15:41   ` Petr Vorel
  2020-11-19 17:48     ` Alexey Kodanev
  0 siblings, 1 reply; 7+ messages in thread
From: Petr Vorel @ 2020-11-19 15:41 UTC (permalink / raw)
  To: ltp

Hi Kory, Alexey,

> Hello Petr,

> Just find out you didn't merge your patch which makes the code cleaner. :)
Waiting for Alexey's review.
Also hesitate about tracepath6 symlink to traceroute being always installed
on all distros (hopefully yes).

Kind regards,
Petr

> Regards,


> On Thu, 12 Nov 2020 18:36:09 +0100
> Petr Vorel <petr.vorel@suse.com> wrote:

> > From: Petr Vorel <pvorel@suse.cz>

> > and move checks to run_trace()

> > There are 3 traceroute versions:

> > * Dmitry Butskoy (http://traceroute.sourceforge.net/)
> > * busybox
> > * iputils (only tracepath6; deprecated, but still used (e.g. OpenWrt Project)

> > -I is supported by Dmitry Butskoy's and busybox implementation
> > -T is supported only by Dmitry Butskoy's implementation

> > Signed-off-by: Petr Vorel <pvorel@suse.cz>
> > ---
> > Hi Alexey, Kory,

> > follow up to Kory's fix (there are some problems with suse.cz
> > mailserver, thus I haven't reply about pushing your patch).

> > Kind regards,
> > Petr

> >  testcases/network/traceroute/traceroute01.sh | 30 +++++++++++---------
> >  1 file changed, 17 insertions(+), 13 deletions(-)

> > diff --git a/testcases/network/traceroute/traceroute01.sh
> > b/testcases/network/traceroute/traceroute01.sh index 38f4d3b85..90030af39
> > 100755 --- a/testcases/network/traceroute/traceroute01.sh
> > +++ b/testcases/network/traceroute/traceroute01.sh
> > @@ -13,9 +13,12 @@ TST_NEEDS_TMPDIR=1

> >  setup()
> >  {
> > -	tst_res TINFO "traceroute version:"
> > -	tst_res TINFO $(traceroute --version 2>&1)
> > -	[ "$TST_IPV6" ] && tst_res TINFO "NOTE: tracepath6 from iputils is
> > not supported" +
> > +	TRACEROUTE=traceroute${TST_IPV6}
> > +	tst_require_cmds $TRACEROUTE
> > +
> > +	tst_res TINFO "$TRACEROUTE version:"
> > +	tst_res TINFO $($TRACEROUTE --version 2>&1)
> >  }

> >  run_trace()
> > @@ -24,18 +27,23 @@ run_trace()
> >  	local ip=$(tst_ipaddr rhost)
> >  	local pattern="^[ ]+1[ ]+$ip([ ]+[0-9]+[.][0-9]+ ms){3}"

> > +	if $TRACEROUTE $opts 2>&1 | grep -q "invalid option"; then
> > +		tst_res TCONF "$opts flag not supported"
> > +		return
> > +	fi
> > +
> >  	# According to man pages, default sizes:
> >  	local bytes=60
> >  	[ "$TST_IPV6" ] && bytes=80

> > -	EXPECT_PASS traceroute $ip $bytes -n -m 2 $opts \>out.log 2>&1
> > +	EXPECT_PASS $TRACEROUTE $ip $bytes -n -m 2 $opts \>out.log 2>&1

> >  	grep -q "$bytes byte" out.log
> >  	if [ $? -ne 0 ]; then
> >  		cat out.log
> >  		tst_res TFAIL "'$bytes byte' not found"
> >  	else
> > -		tst_res TPASS "traceroute use $bytes bytes"
> > +		tst_res TPASS "$TRACEROUTE use $bytes bytes"
> >  	fi

> >  	tail -1 out.log | grep -qE "$pattern"
> > @@ -43,24 +51,20 @@ run_trace()
> >  		cat out.log
> >  		tst_res TFAIL "pattern '$pattern' not found in log"
> >  	else
> > -		tst_res TPASS "traceroute test completed with 1 hop"
> > +		tst_res TPASS "$TRACEROUTE test completed with 1 hop"
> >  	fi
> >  }

> >  test1()
> >  {
> > -	tst_res TINFO "run traceroute with ICMP ECHO"
> > +	tst_res TINFO "run $TRACEROUTE with ICMP ECHO"
> >  	run_trace -I
> >  }

> >  test2()
> >  {
> > -	tst_res TINFO "run traceroute with TCP SYN"
> > -	if traceroute -T 2>&1 | grep -q "invalid option"; then
> > -		tst_res TCONF "-T flag (TCP SYN) not supported"
> > -	else
> > -		run_trace -T
> > -	fi
> > +	tst_res TINFO "run $TRACEROUTE with TCP SYN"
> > +	run_trace -T
> >  }

> >  tst_run

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [LTP] [PATCH 1/1] net/traceroute01: Check also -T flag
  2020-11-19 15:41   ` Petr Vorel
@ 2020-11-19 17:48     ` Alexey Kodanev
  2020-11-19 19:56       ` Petr Vorel
  0 siblings, 1 reply; 7+ messages in thread
From: Alexey Kodanev @ 2020-11-19 17:48 UTC (permalink / raw)
  To: ltp

On 19.11.2020 18:41, Petr Vorel wrote:
> Hi Kory, Alexey,
> 
>> Hello Petr,
> 
>> Just find out you didn't merge your patch which makes the code cleaner. :)
> Waiting for Alexey's review.
> Also hesitate about tracepath6 symlink to traceroute being always installed
> on all distros (hopefully yes).

Hi Petr,

From the subject it seems you are adding a new check for -T flag
but it is actually for -I option?

I think it's ok using symlink or -4/-6, it is also adding an extra
check for ipv6 that $(tst_ipaddr rhost) is indeed ipv6.

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [LTP] [PATCH 1/1] net/traceroute01: Check also -T flag
  2020-11-19 17:48     ` Alexey Kodanev
@ 2020-11-19 19:56       ` Petr Vorel
  2020-11-20  5:59         ` Alexey Kodanev
  0 siblings, 1 reply; 7+ messages in thread
From: Petr Vorel @ 2020-11-19 19:56 UTC (permalink / raw)
  To: ltp

Hi Alexey,

> On 19.11.2020 18:41, Petr Vorel wrote:
> > Hi Kory, Alexey,

> >> Hello Petr,

> >> Just find out you didn't merge your patch which makes the code cleaner. :)
> > Waiting for Alexey's review.
> > Also hesitate about tracepath6 symlink to traceroute being always installed
I meant here traceroute6 symlink to traceroute.
> > on all distros (hopefully yes).

> Hi Petr,

> From the subject it seems you are adding a new check for -T flag
> but it is actually for -I option?
Good catch, thanks!

> I think it's ok using symlink or -4/-6,
If nobody complains I'd keep symlink version as it can check
traceroute from iputils. -4/-6 is more portable. If anybody complains and we
revert it back, I'd also put back iputils related warning.

> it is also adding an extra
> check for ipv6 that $(tst_ipaddr rhost) is indeed ipv6.
Sorry, I didn't get this.

BTW ack this change?

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [LTP] [PATCH 1/1] net/traceroute01: Check also -T flag
  2020-11-19 19:56       ` Petr Vorel
@ 2020-11-20  5:59         ` Alexey Kodanev
  2020-11-20  8:45           ` Petr Vorel
  0 siblings, 1 reply; 7+ messages in thread
From: Alexey Kodanev @ 2020-11-20  5:59 UTC (permalink / raw)
  To: ltp

On 19.11.2020 22:56, Petr Vorel wrote:
> Hi Alexey,
> 
>> On 19.11.2020 18:41, Petr Vorel wrote:
>>> Hi Kory, Alexey,
> 
>>>> Hello Petr,
> 
>>>> Just find out you didn't merge your patch which makes the code cleaner. :)
>>> Waiting for Alexey's review.
>>> Also hesitate about tracepath6 symlink to traceroute being always installed
> I meant here traceroute6 symlink to traceroute.
>>> on all distros (hopefully yes).
> 
>> Hi Petr,
> 
>> From the subject it seems you are adding a new check for -T flag
>> but it is actually for -I option?
> Good catch, thanks!
> 
>> I think it's ok using symlink or -4/-6,
> If nobody complains I'd keep symlink version as it can check
> traceroute from iputils. -4/-6 is more portable. If anybody complains and we
> revert it back, I'd also put back iputils related warning.
> 
>> it is also adding an extra
>> check for ipv6 that $(tst_ipaddr rhost) is indeed ipv6.
> Sorry, I didn't get this.
> 

Hi Petr,

traceroute6 127.0.0.1
traceroute6: 127.0.0.1: Address family for hostname not supported

And both ::1 and 127.0.0.1 are ok for traceroute without the address
family option.

> BTW ack this change?
> 

Acked-by: Alexey Kodanev <alexey.kodanev@oracle.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [LTP] [PATCH 1/1] net/traceroute01: Check also -T flag
  2020-11-20  5:59         ` Alexey Kodanev
@ 2020-11-20  8:45           ` Petr Vorel
  0 siblings, 0 replies; 7+ messages in thread
From: Petr Vorel @ 2020-11-20  8:45 UTC (permalink / raw)
  To: ltp

Hi Alexey,

...
> Hi Petr,

> traceroute6 127.0.0.1
> traceroute6: 127.0.0.1: Address family for hostname not supported

> And both ::1 and 127.0.0.1 are ok for traceroute without the address
> family option.

Thanks for an explanation!

> > BTW ack this change?


> Acked-by: Alexey Kodanev <alexey.kodanev@oracle.com>

Thanks! Merged.

Kind regards,
Petr

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2020-11-20  8:45 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-12 17:36 [LTP] [PATCH 1/1] net/traceroute01: Check also -T flag Petr Vorel
2020-11-19 14:23 ` =?unknown-8bit?q?K=C3=B6ry?= Maincent
2020-11-19 15:41   ` Petr Vorel
2020-11-19 17:48     ` Alexey Kodanev
2020-11-19 19:56       ` Petr Vorel
2020-11-20  5:59         ` Alexey Kodanev
2020-11-20  8:45           ` Petr Vorel

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.