[LTP] [PATCH v4] tcp_cmds/ping/ping02: Make it compatible with Busybox

[LTP] [PATCH v4] tcp_cmds/ping/ping02: Make it compatible with Busybox

[Kory Maincent]

The ping from busybox does not have -f parameter, use -i parameter instead.
BusyBox does not accept pattern longer than 2 bytes.
BusyBox support -i option since version 1.30

Signed-off-by: Kory Maincent <kory.maincent@bootlin.com>
---

Changes since v1:
 - Use "-i 0.01 -p aa" parameter instead of "-f"

Changes since v2:
 - Use "-i 0.01 -p aa" only in the case of "-f" parameter is not valid

Changes since v3:
 - Test if -i is a valid option.
 - Move all in do_setup function

 testcases/network/tcp_cmds/ping/ping02.sh | 19 +++++++++++++------
 1 file changed, 13 insertions(+), 6 deletions(-)

diff --git a/testcases/network/tcp_cmds/ping/ping02.sh b/testcases/network/tcp_cmds/ping/ping02.sh
index e0a63c5f6..d4290c873 100755
--- a/testcases/network/tcp_cmds/ping/ping02.sh
+++ b/testcases/network/tcp_cmds/ping/ping02.sh
@@ -16,19 +16,26 @@ do_setup()
 	PING=ping${TST_IPV6}
 
 	tst_require_cmds $PING
+
+	ping_opts="-f -p 000102030405060708090a0b0c0d0e0f"
+	ipaddr=$(tst_ipaddr rhost)
+
+	if ! $PING -c 1 -f $ipaddr >/dev/null 2>&1; then
+		ping_opts="-i 0.01 -p aa"
+		if $PING -i 2>&1 | grep -q "invalid option"; then
+			tst_brk TCONF "unsupported ping version (old busybox?)"
+		fi
+	fi
 }
 
 do_test()
 {
-	local pat="000102030405060708090a0b0c0d0e0f"
-
-	tst_res TINFO "flood $PING: ICMP packets filled with pattern '$pat'"
-
-	local ipaddr=$(tst_ipaddr rhost)
 	local s
 
+	tst_res TINFO "flood $PING: ICMP packets with options '$ping_opts'"
+
 	for s in $PACKETSIZES; do
-		EXPECT_PASS $PING -c $COUNT -f -s $s $ipaddr -p "$pat" \>/dev/null
+		EXPECT_PASS $PING -c $COUNT -s $s $ipaddr $ping_opts \>/dev/null
 	done
 }
 
-- 
2.17.1

[LTP] [PATCH v4] tcp_cmds/ping/ping02: Make it compatible with Busybox

[Petr Vorel]

Hi Kory, Alexey,

> The ping from busybox does not have -f parameter, use -i parameter instead.
> BusyBox does not accept pattern longer than 2 bytes.
> BusyBox support -i option since version 1.30

Reviewed-by: Petr Vorel <pvorel@suse.cz>
Tested-by: Petr Vorel <pvorel@suse.cz>

LGTM.

I suggest to merge with few changes below.

* more precise message "ping from old busybox?"
* use just $(tst_ipaddr rhost) instead of $ipaddr
(some time ago we've started to use function calls for these simple evaluations)
* upper case for global variable (ugly, but easily recognizable

Kind regards,
Petr

diff --git testcases/network/tcp_cmds/ping/ping02.sh testcases/network/tcp_cmds/ping/ping02.sh
index d4290c873..acf2737bf 100755
--- testcases/network/tcp_cmds/ping/ping02.sh
+++ testcases/network/tcp_cmds/ping/ping02.sh
@@ -17,13 +17,12 @@ do_setup()
 
 	tst_require_cmds $PING
 
-	ping_opts="-f -p 000102030405060708090a0b0c0d0e0f"
-	ipaddr=$(tst_ipaddr rhost)
+	PING_OPTS="-f -p 000102030405060708090a0b0c0d0e0f"
 
-	if ! $PING -c 1 -f $ipaddr >/dev/null 2>&1; then
-		ping_opts="-i 0.01 -p aa"
+	if ! $PING -c 1 -f $(tst_ipaddr rhost) >/dev/null 2>&1; then
+		PING_OPTS="-i 0.01 -p aa"
 		if $PING -i 2>&1 | grep -q "invalid option"; then
-			tst_brk TCONF "unsupported ping version (old busybox?)"
+			tst_brk TCONF "unsupported ping version (ping from old busybox?)"
 		fi
 	fi
 }
@@ -32,10 +31,10 @@ do_test()
 {
 	local s
 
-	tst_res TINFO "flood $PING: ICMP packets with options '$ping_opts'"
+	tst_res TINFO "flood $PING: ICMP packets with options '$PING_OPTS'"
 
 	for s in $PACKETSIZES; do
-		EXPECT_PASS $PING -c $COUNT -s $s $ipaddr $ping_opts \>/dev/null
+		EXPECT_PASS $PING -c $COUNT -s $s $(tst_ipaddr rhost) $PING_OPTS \>/dev/null
 	done
 }
 

[LTP] [PATCH v4] tcp_cmds/ping/ping02: Make it compatible with Busybox

[Alexey Kodanev]

On 11.11.2020 17:40, Petr Vorel wrote:
> Hi Kory, Alexey,
> 
>> The ping from busybox does not have -f parameter, use -i parameter instead.
>> BusyBox does not accept pattern longer than 2 bytes.
>> BusyBox support -i option since version 1.30
> 
> Reviewed-by: Petr Vorel <pvorel@suse.cz>
> Tested-by: Petr Vorel <pvorel@suse.cz>
> 
> LGTM.
> 
> I suggest to merge with few changes below.
> 
> * more precise message "ping from old busybox?"
> * use just $(tst_ipaddr rhost) instead of $ipaddr
> (some time ago we've started to use function calls for these simple evaluations)
> * upper case for global variable (ugly, but easily recognizable

Hi Petr,

Feel free to add the style changes, ack. I've already applied the patch.

Thanks Kory!

[LTP] [PATCH v4] tcp_cmds/ping/ping02: Make it compatible with Busybox

[Petr Vorel]

Hi Kory, Alexey,

> >> The ping from busybox does not have -f parameter, use -i parameter instead.
> >> BusyBox does not accept pattern longer than 2 bytes.
> >> BusyBox support -i option since version 1.30

> > Reviewed-by: Petr Vorel <pvorel@suse.cz>
> > Tested-by: Petr Vorel <pvorel@suse.cz>

> > LGTM.

> > I suggest to merge with few changes below.

> > * more precise message "ping from old busybox?"
> > * use just $(tst_ipaddr rhost) instead of $ipaddr
> > (some time ago we've started to use function calls for these simple evaluations)
> > * upper case for global variable (ugly, but easily recognizable

> Hi Petr,

> Feel free to add the style changes, ack. I've already applied the patch.

> Thanks Kory!

Let's leave it as it is, not that important.
Thanks a lot both.

Kind regards,
Petr