diff mbox series

[PATCHv3,net-next,2/2] selftests: add a selftest for directed broadcast forwarding

Message ID a3c46ade85539e450f037e62984f750c7d56084d.1532346504.git.lucien.xin@gmail.com
State Changes Requested, archived
Delegated to: David Miller
Headers show
Series route: add support and selftests for directed broadcast forwarding | expand

Commit Message

Xin Long July 23, 2018, 11:51 a.m. UTC
As Ido's suggestion, this patch is to add a selftest for directed
broadcast forwarding with vrf. It does the assertion by checking
the src IP of the echo-reply packet in ping_test_from.

Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 .../selftests/net/forwarding/router_broadcast.sh   | 215 +++++++++++++++++++++
 1 file changed, 215 insertions(+)
 create mode 100755 tools/testing/selftests/net/forwarding/router_broadcast.sh

Comments

David Ahern July 23, 2018, 3:17 p.m. UTC | #1
On 7/23/18 5:51 AM, Xin Long wrote:
> +ping_ipv4()
> +{
> +	sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0
> +
> +	bc_forwarding_disable
> +	ping_test_from $h1 198.51.100.255 192.0.2.1
> +	ping_test_from $h1 198.51.200.255 192.0.2.1
> +	ping_test_from $h1 192.0.2.255 192.0.2.1
> +	ping_test_from $h1 255.255.255.255 192.0.2.1
> +
> +	ping_test_from $h2 192.0.2.255 198.51.100.1
> +	ping_test_from $h2 198.51.200.255 198.51.100.1
> +	ping_test_from $h2 198.51.100.255 198.51.100.1
> +	ping_test_from $h2 255.255.255.255 198.51.100.1
> +	bc_forwarding_restore
> +
> +	bc_forwarding_enable
> +	ping_test_from $h1 198.51.100.255 198.51.100.2
> +	ping_test_from $h1 198.51.200.255 198.51.200.2
> +	ping_test_from $h1 192.0.2.255 192.0.2.1 1
> +	ping_test_from $h1 255.255.255.255 192.0.2.1
> +
> +	ping_test_from $h2 192.0.2.255 192.0.2.2
> +	ping_test_from $h2 198.51.200.255 198.51.200.2
> +	ping_test_from $h2 198.51.100.255 198.51.100.1 1
> +	ping_test_from $h2 255.255.255.255 198.51.100.1
> +	bc_forwarding_restore
> +
> +	sysctl_restore net.ipv4.icmp_echo_ignore_broadcasts

You need a better description for each test. This output:
TEST: ping_test_from                            [PASS]
TEST: ping_test_from                            [PASS]
TEST: ping_test_from                            [PASS]
TEST: ping_test_from                            [PASS]
...

does not help in understanding which cases are working and which are not.
Xin Long July 24, 2018, 5:24 p.m. UTC | #2
On Mon, Jul 23, 2018 at 11:17 PM, David Ahern <dsahern@gmail.com> wrote:
> On 7/23/18 5:51 AM, Xin Long wrote:
>> +ping_ipv4()
>> +{
>> +     sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0
>> +
>> +     bc_forwarding_disable
>> +     ping_test_from $h1 198.51.100.255 192.0.2.1
>> +     ping_test_from $h1 198.51.200.255 192.0.2.1
>> +     ping_test_from $h1 192.0.2.255 192.0.2.1
>> +     ping_test_from $h1 255.255.255.255 192.0.2.1
>> +
>> +     ping_test_from $h2 192.0.2.255 198.51.100.1
>> +     ping_test_from $h2 198.51.200.255 198.51.100.1
>> +     ping_test_from $h2 198.51.100.255 198.51.100.1
>> +     ping_test_from $h2 255.255.255.255 198.51.100.1
>> +     bc_forwarding_restore
>> +
>> +     bc_forwarding_enable
>> +     ping_test_from $h1 198.51.100.255 198.51.100.2
>> +     ping_test_from $h1 198.51.200.255 198.51.200.2
>> +     ping_test_from $h1 192.0.2.255 192.0.2.1 1
>> +     ping_test_from $h1 255.255.255.255 192.0.2.1
>> +
>> +     ping_test_from $h2 192.0.2.255 192.0.2.2
>> +     ping_test_from $h2 198.51.200.255 198.51.200.2
>> +     ping_test_from $h2 198.51.100.255 198.51.100.1 1
>> +     ping_test_from $h2 255.255.255.255 198.51.100.1
>> +     bc_forwarding_restore
>> +
>> +     sysctl_restore net.ipv4.icmp_echo_ignore_broadcasts
>
> You need a better description for each test. This output:
> TEST: ping_test_from                            [PASS]
> TEST: ping_test_from                            [PASS]
> TEST: ping_test_from                            [PASS]
> TEST: ping_test_from                            [PASS]
> ...
>
> does not help in understanding which cases are working and which are not.
# ./router_broadcast.sh
INFO: bc_forwarding disabled on r1=>
INFO: h1 -> net2: reply from r1 (not forwarding)
TEST: ping_test_from                                                [PASS]
INFO: h1 -> net3: reply from r1 (not forwarding)
TEST: ping_test_from                                                [PASS]
INFO: h1 -> net1: reply from r1 (not dropping)
TEST: ping_test_from                                                [PASS]
INFO: h1 -> 255.255.255.255: reply from r1 (not forwarding)
TEST: ping_test_from                                                [PASS]
INFO: h2 -> net1: reply from r1 (not forwarding)
TEST: ping_test_from                                                [PASS]
INFO: h2 -> net3: reply from r1 (not forwarding)
TEST: ping_test_from                                                [PASS]
INFO: h2 -> net2: reply from r1 (not dropping)
TEST: ping_test_from                                                [PASS]
INFO: h2 -> 255.255.255.255: reply from r1 (not forwarding)
TEST: ping_test_from                                                [PASS]
INFO: bc_forwarding enabled on r1 =>
INFO: h1 -> net2: reply from h2 (forwarding)
TEST: ping_test_from                                                [PASS]
INFO: h1 -> net3: reply from h3 (forwarding)
TEST: ping_test_from                                                [PASS]
INFO: h1 -> net1: no reply (dropping)
TEST: ping_test_from                                                [PASS]
INFO: h1 -> 255.255.255.255: reply from r1 (not forwarding)
TEST: ping_test_from                                                [PASS]
INFO: h2 -> net1: reply from h3 (forwarding)
TEST: ping_test_from                                                [PASS]
INFO: h2 -> net3: reply from h1 (forwarding)
TEST: ping_test_from                                                [PASS]
INFO: h2 -> net2: no reply (dropping)
TEST: ping_test_from                                                [PASS]
INFO: h2 -> 255.255.255.255: reply from r1 (not forwarding)
TEST: ping_test_from                                                [PASS]

I hope this log looks good to you?
David Ahern July 24, 2018, 5:37 p.m. UTC | #3
On 7/24/18 11:24 AM, Xin Long wrote:
> On Mon, Jul 23, 2018 at 11:17 PM, David Ahern <dsahern@gmail.com> wrote:
>> On 7/23/18 5:51 AM, Xin Long wrote:
>>> +ping_ipv4()
>>> +{
>>> +     sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0
>>> +
>>> +     bc_forwarding_disable
>>> +     ping_test_from $h1 198.51.100.255 192.0.2.1
>>> +     ping_test_from $h1 198.51.200.255 192.0.2.1
>>> +     ping_test_from $h1 192.0.2.255 192.0.2.1
>>> +     ping_test_from $h1 255.255.255.255 192.0.2.1
>>> +
>>> +     ping_test_from $h2 192.0.2.255 198.51.100.1
>>> +     ping_test_from $h2 198.51.200.255 198.51.100.1
>>> +     ping_test_from $h2 198.51.100.255 198.51.100.1
>>> +     ping_test_from $h2 255.255.255.255 198.51.100.1
>>> +     bc_forwarding_restore
>>> +
>>> +     bc_forwarding_enable
>>> +     ping_test_from $h1 198.51.100.255 198.51.100.2
>>> +     ping_test_from $h1 198.51.200.255 198.51.200.2
>>> +     ping_test_from $h1 192.0.2.255 192.0.2.1 1
>>> +     ping_test_from $h1 255.255.255.255 192.0.2.1
>>> +
>>> +     ping_test_from $h2 192.0.2.255 192.0.2.2
>>> +     ping_test_from $h2 198.51.200.255 198.51.200.2
>>> +     ping_test_from $h2 198.51.100.255 198.51.100.1 1
>>> +     ping_test_from $h2 255.255.255.255 198.51.100.1
>>> +     bc_forwarding_restore
>>> +
>>> +     sysctl_restore net.ipv4.icmp_echo_ignore_broadcasts
>>
>> You need a better description for each test. This output:
>> TEST: ping_test_from                            [PASS]
>> TEST: ping_test_from                            [PASS]
>> TEST: ping_test_from                            [PASS]
>> TEST: ping_test_from                            [PASS]
>> ...
>>
>> does not help in understanding which cases are working and which are not.
> # ./router_broadcast.sh
> INFO: bc_forwarding disabled on r1=>
> INFO: h1 -> net2: reply from r1 (not forwarding)
> TEST: ping_test_from                                                [PASS]
> INFO: h1 -> net3: reply from r1 (not forwarding)
> TEST: ping_test_from                                                [PASS]
> INFO: h1 -> net1: reply from r1 (not dropping)
> TEST: ping_test_from                                                [PASS]
> INFO: h1 -> 255.255.255.255: reply from r1 (not forwarding)
> TEST: ping_test_from                                                [PASS]
> INFO: h2 -> net1: reply from r1 (not forwarding)
> TEST: ping_test_from                                                [PASS]
> INFO: h2 -> net3: reply from r1 (not forwarding)
> TEST: ping_test_from                                                [PASS]
> INFO: h2 -> net2: reply from r1 (not dropping)
> TEST: ping_test_from                                                [PASS]
> INFO: h2 -> 255.255.255.255: reply from r1 (not forwarding)
> TEST: ping_test_from                                                [PASS]
> INFO: bc_forwarding enabled on r1 =>
> INFO: h1 -> net2: reply from h2 (forwarding)
> TEST: ping_test_from                                                [PASS]
> INFO: h1 -> net3: reply from h3 (forwarding)
> TEST: ping_test_from                                                [PASS]
> INFO: h1 -> net1: no reply (dropping)
> TEST: ping_test_from                                                [PASS]
> INFO: h1 -> 255.255.255.255: reply from r1 (not forwarding)
> TEST: ping_test_from                                                [PASS]
> INFO: h2 -> net1: reply from h3 (forwarding)
> TEST: ping_test_from                                                [PASS]
> INFO: h2 -> net3: reply from h1 (forwarding)
> TEST: ping_test_from                                                [PASS]
> INFO: h2 -> net2: no reply (dropping)
> TEST: ping_test_from                                                [PASS]
> INFO: h2 -> 255.255.255.255: reply from r1 (not forwarding)
> TEST: ping_test_from                                                [PASS]
> 
> I hope this log looks good to you?
> 

The extra INFO is good, but the TEST line needs a better description.
Xin Long July 24, 2018, 5:55 p.m. UTC | #4
On Wed, Jul 25, 2018 at 1:37 AM, David Ahern <dsahern@gmail.com> wrote:
> On 7/24/18 11:24 AM, Xin Long wrote:
>> On Mon, Jul 23, 2018 at 11:17 PM, David Ahern <dsahern@gmail.com> wrote:
>>> On 7/23/18 5:51 AM, Xin Long wrote:
>>>> +ping_ipv4()
>>>> +{
>>>> +     sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0
>>>> +
>>>> +     bc_forwarding_disable
>>>> +     ping_test_from $h1 198.51.100.255 192.0.2.1
>>>> +     ping_test_from $h1 198.51.200.255 192.0.2.1
>>>> +     ping_test_from $h1 192.0.2.255 192.0.2.1
>>>> +     ping_test_from $h1 255.255.255.255 192.0.2.1
>>>> +
>>>> +     ping_test_from $h2 192.0.2.255 198.51.100.1
>>>> +     ping_test_from $h2 198.51.200.255 198.51.100.1
>>>> +     ping_test_from $h2 198.51.100.255 198.51.100.1
>>>> +     ping_test_from $h2 255.255.255.255 198.51.100.1
>>>> +     bc_forwarding_restore
>>>> +
>>>> +     bc_forwarding_enable
>>>> +     ping_test_from $h1 198.51.100.255 198.51.100.2
>>>> +     ping_test_from $h1 198.51.200.255 198.51.200.2
>>>> +     ping_test_from $h1 192.0.2.255 192.0.2.1 1
>>>> +     ping_test_from $h1 255.255.255.255 192.0.2.1
>>>> +
>>>> +     ping_test_from $h2 192.0.2.255 192.0.2.2
>>>> +     ping_test_from $h2 198.51.200.255 198.51.200.2
>>>> +     ping_test_from $h2 198.51.100.255 198.51.100.1 1
>>>> +     ping_test_from $h2 255.255.255.255 198.51.100.1
>>>> +     bc_forwarding_restore
>>>> +
>>>> +     sysctl_restore net.ipv4.icmp_echo_ignore_broadcasts
>>>
>>> You need a better description for each test. This output:
>>> TEST: ping_test_from                            [PASS]
>>> TEST: ping_test_from                            [PASS]
>>> TEST: ping_test_from                            [PASS]
>>> TEST: ping_test_from                            [PASS]
>>> ...
>>>
>>> does not help in understanding which cases are working and which are not.
>> # ./router_broadcast.sh
>> INFO: bc_forwarding disabled on r1=>
>> INFO: h1 -> net2: reply from r1 (not forwarding)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h1 -> net3: reply from r1 (not forwarding)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h1 -> net1: reply from r1 (not dropping)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h1 -> 255.255.255.255: reply from r1 (not forwarding)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h2 -> net1: reply from r1 (not forwarding)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h2 -> net3: reply from r1 (not forwarding)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h2 -> net2: reply from r1 (not dropping)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h2 -> 255.255.255.255: reply from r1 (not forwarding)
>> TEST: ping_test_from                                                [PASS]
>> INFO: bc_forwarding enabled on r1 =>
>> INFO: h1 -> net2: reply from h2 (forwarding)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h1 -> net3: reply from h3 (forwarding)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h1 -> net1: no reply (dropping)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h1 -> 255.255.255.255: reply from r1 (not forwarding)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h2 -> net1: reply from h3 (forwarding)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h2 -> net3: reply from h1 (forwarding)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h2 -> net2: no reply (dropping)
>> TEST: ping_test_from                                                [PASS]
>> INFO: h2 -> 255.255.255.255: reply from r1 (not forwarding)
>> TEST: ping_test_from                                                [PASS]
>>
>> I hope this log looks good to you?
>>
>
> The extra INFO is good, but the TEST line needs a better description.
>
INFO: bc_forwarding disabled on r1 =>
INFO: h1 -> net2: reply from r1 (not forwarding)
TEST: ping 198.51.100.255, expected reply from 192.0.2.1            [PASS]
INFO: h1 -> net3: reply from r1 (not forwarding)
TEST: ping 198.51.200.255, expected reply from 192.0.2.1            [PASS]
INFO: h1 -> net1: reply from r1 (not dropping)
TEST: ping 192.0.2.255, expected reply from 192.0.2.1               [PASS]
....
how about this?
David Ahern July 24, 2018, 6:36 p.m. UTC | #5
On 7/24/18 11:55 AM, Xin Long wrote:
> INFO: bc_forwarding disabled on r1 =>
> INFO: h1 -> net2: reply from r1 (not forwarding)
> TEST: ping 198.51.100.255, expected reply from 192.0.2.1            [PASS]
> INFO: h1 -> net3: reply from r1 (not forwarding)
> TEST: ping 198.51.200.255, expected reply from 192.0.2.1            [PASS]
> INFO: h1 -> net1: reply from r1 (not dropping)
> TEST: ping 192.0.2.255, expected reply from 192.0.2.1               [PASS]
> ....
> how about this?

Personally, I think that is backwards. So like this:

INFO: bc_forwarding disabled on r1 =>
INFO: ping 198.51.100.255, expected reply from 192.0.2.1
TEST: h1 -> net2: reply from r1 (not forwarding)              [PASS]
INFO: ping 198.51.200.255, expected reply from 192.0.2.1
TEST: h1 -> net3: reply from r1 (not forwarding)              [PASS]
INFO: ping 192.0.2.255, expected reply from 192.0.2.1
TEST: h1 -> net1: reply from r1 (not dropping)                [PASS]
...
diff mbox series

Patch

diff --git a/tools/testing/selftests/net/forwarding/router_broadcast.sh b/tools/testing/selftests/net/forwarding/router_broadcast.sh
new file mode 100755
index 0000000..f2a5a51
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/router_broadcast.sh
@@ -0,0 +1,215 @@ 
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+ALL_TESTS="ping_ipv4"
+NUM_NETIFS=6
+source lib.sh
+
+h1_create()
+{
+	vrf_create "vrf-h1"
+	ip link set dev $h1 master vrf-h1
+
+	ip link set dev vrf-h1 up
+	ip link set dev $h1 up
+
+	ip address add 192.0.2.2/24 dev $h1
+
+	ip route add 198.51.100.0/24 vrf vrf-h1 nexthop via 192.0.2.1
+	ip route add 198.51.200.0/24 vrf vrf-h1 nexthop via 192.0.2.1
+}
+
+h1_destroy()
+{
+	ip route del 198.51.200.0/24 vrf vrf-h1
+	ip route del 198.51.100.0/24 vrf vrf-h1
+
+	ip address del 192.0.2.2/24 dev $h1
+
+	ip link set dev $h1 down
+	vrf_destroy "vrf-h1"
+}
+
+h2_create()
+{
+	vrf_create "vrf-h2"
+	ip link set dev $h2 master vrf-h2
+
+	ip link set dev vrf-h2 up
+	ip link set dev $h2 up
+
+	ip address add 198.51.100.2/24 dev $h2
+
+	ip route add 192.0.2.0/24 vrf vrf-h2 nexthop via 198.51.100.1
+	ip route add 198.51.200.0/24 vrf vrf-h2 nexthop via 198.51.100.1
+}
+
+h2_destroy()
+{
+	ip route del 198.51.200.0/24 vrf vrf-h2
+	ip route del 192.0.2.0/24 vrf vrf-h2
+
+	ip address del 198.51.100.2/24 dev $h2
+
+	ip link set dev $h2 down
+	vrf_destroy "vrf-h2"
+}
+
+h3_create()
+{
+	vrf_create "vrf-h3"
+	ip link set dev $h3 master vrf-h3
+
+	ip link set dev vrf-h3 up
+	ip link set dev $h3 up
+
+	ip address add 198.51.200.2/24 dev $h3
+
+	ip route add 192.0.2.0/24 vrf vrf-h3 nexthop via 198.51.200.1
+	ip route add 198.51.100.0/24 vrf vrf-h3 nexthop via 198.51.200.1
+}
+
+h3_destroy()
+{
+	ip route del 198.51.100.0/24 vrf vrf-h3
+	ip route del 192.0.2.0/24 vrf vrf-h3
+
+	ip address del 198.51.200.2/24 dev $h3
+
+	ip link set dev $h3 down
+	vrf_destroy "vrf-h3"
+}
+
+router_create()
+{
+	ip link set dev $rp1 up
+	ip link set dev $rp2 up
+	ip link set dev $rp3 up
+
+	ip address add 192.0.2.1/24 dev $rp1
+
+	ip address add 198.51.100.1/24 dev $rp2
+	ip address add 198.51.200.1/24 dev $rp3
+}
+
+router_destroy()
+{
+	ip address del 198.51.200.1/24 dev $rp3
+	ip address del 198.51.100.1/24 dev $rp2
+
+	ip address del 192.0.2.1/24 dev $rp1
+
+	ip link set dev $rp3 down
+	ip link set dev $rp2 down
+	ip link set dev $rp1 down
+}
+
+setup_prepare()
+{
+	h1=${NETIFS[p1]}
+	rp1=${NETIFS[p2]}
+
+	rp2=${NETIFS[p3]}
+	h2=${NETIFS[p4]}
+
+	rp3=${NETIFS[p5]}
+	h3=${NETIFS[p6]}
+
+	vrf_prepare
+
+	h1_create
+	h2_create
+	h3_create
+
+	router_create
+
+	forwarding_enable
+}
+
+cleanup()
+{
+	pre_cleanup
+
+	forwarding_restore
+
+	router_destroy
+
+	h3_destroy
+	h2_destroy
+	h1_destroy
+
+	vrf_cleanup
+}
+
+bc_forwarding_disable()
+{
+	sysctl_set net.ipv4.conf.all.bc_forwarding 0
+	sysctl_set net.ipv4.conf.$rp1.bc_forwarding 0
+}
+
+bc_forwarding_enable()
+{
+	sysctl_set net.ipv4.conf.all.bc_forwarding 1
+	sysctl_set net.ipv4.conf.$rp1.bc_forwarding 1
+}
+
+bc_forwarding_restore()
+{
+	sysctl_restore net.ipv4.conf.$rp1.bc_forwarding
+	sysctl_restore net.ipv4.conf.all.bc_forwarding
+}
+
+ping_test_from()
+{
+	local oif=$1
+	local dip=$2
+	local from=$3
+	local fail=${4:-0}
+
+	RET=0
+
+	ip vrf exec $(master_name_get $oif) \
+	$PING -I $oif $dip -c 10 -i 0.1 -w 2 -b 2>&1 | grep $from &> /dev/null
+	check_err_fail $fail $?
+	log_test "ping_test_from"
+}
+
+ping_ipv4()
+{
+	sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0
+
+	bc_forwarding_disable
+	ping_test_from $h1 198.51.100.255 192.0.2.1
+	ping_test_from $h1 198.51.200.255 192.0.2.1
+	ping_test_from $h1 192.0.2.255 192.0.2.1
+	ping_test_from $h1 255.255.255.255 192.0.2.1
+
+	ping_test_from $h2 192.0.2.255 198.51.100.1
+	ping_test_from $h2 198.51.200.255 198.51.100.1
+	ping_test_from $h2 198.51.100.255 198.51.100.1
+	ping_test_from $h2 255.255.255.255 198.51.100.1
+	bc_forwarding_restore
+
+	bc_forwarding_enable
+	ping_test_from $h1 198.51.100.255 198.51.100.2
+	ping_test_from $h1 198.51.200.255 198.51.200.2
+	ping_test_from $h1 192.0.2.255 192.0.2.1 1
+	ping_test_from $h1 255.255.255.255 192.0.2.1
+
+	ping_test_from $h2 192.0.2.255 192.0.2.2
+	ping_test_from $h2 198.51.200.255 198.51.200.2
+	ping_test_from $h2 198.51.100.255 198.51.100.1 1
+	ping_test_from $h2 255.255.255.255 198.51.100.1
+	bc_forwarding_restore
+
+	sysctl_restore net.ipv4.icmp_echo_ignore_broadcasts
+}
+
+trap cleanup EXIT
+
+setup_prepare
+setup_wait
+
+tests_run
+
+exit $EXIT_STATUS