* [PATCHv3 net-next 0/2] route: add support and selftests for directed broadcast forwarding
@ 2018-07-23 11:51 Xin Long
2018-07-23 11:51 ` [PATCHv3 net-next 1/2] route: add support " Xin Long
0 siblings, 1 reply; 8+ messages in thread
From: Xin Long @ 2018-07-23 11:51 UTC (permalink / raw)
To: network dev; +Cc: davem, David Ahern, Davide Caratti, idosch
Patch 1/2 is the feature and 2/2 is the selftest. Check the changelog
on each of them to know the details.
v1->v2:
- fix a typo in changelog.
- fix an uapi break that Davide noticed.
- flush route cache when bc_forwarding is changed.
- add the selftest for this patch as Ido's suggestion.
v2->v3:
- fix an incorrect 'if check' in devinet_conf_proc as David Ahern
noticed.
- extend the selftest after one David Ahern fix for vrf.
Xin Long (2):
route: add support for directed broadcast forwarding
selftests: add a selftest for directed broadcast forwarding
include/linux/inetdevice.h | 1 +
include/uapi/linux/ip.h | 1 +
include/uapi/linux/netconf.h | 1 +
net/ipv4/devinet.c | 11 ++
net/ipv4/route.c | 6 +-
.../selftests/net/forwarding/router_broadcast.sh | 215 +++++++++++++++++++++
6 files changed, 234 insertions(+), 1 deletion(-)
create mode 100755 tools/testing/selftests/net/forwarding/router_broadcast.sh
--
2.1.0
^ permalink raw reply [flat|nested] 8+ messages in thread
* [PATCHv3 net-next 1/2] route: add support for directed broadcast forwarding
2018-07-23 11:51 [PATCHv3 net-next 0/2] route: add support and selftests for directed broadcast forwarding Xin Long
@ 2018-07-23 11:51 ` Xin Long
2018-07-23 11:51 ` [PATCHv3 net-next 2/2] selftests: add a selftest " Xin Long
0 siblings, 1 reply; 8+ messages in thread
From: Xin Long @ 2018-07-23 11:51 UTC (permalink / raw)
To: network dev; +Cc: davem, David Ahern, Davide Caratti, idosch
This patch implements the feature described in rfc1812#section-5.3.5.2
and rfc2644. It allows the router to forward directed broadcast when
sysctl bc_forwarding is enabled.
Note that this feature could be done by iptables -j TEE, but it would
cause some problems:
- target TEE's gateway param has to be set with a specific address,
and it's not flexible especially when the route wants forward all
directed broadcasts.
- this duplicates the directed broadcasts so this may cause side
effects to applications.
Besides, to keep consistent with other os router like BSD, it's also
necessary to implement it in the route rx path.
Note that route cache needs to be flushed when bc_forwarding is
changed.
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
include/linux/inetdevice.h | 1 +
include/uapi/linux/ip.h | 1 +
include/uapi/linux/netconf.h | 1 +
net/ipv4/devinet.c | 11 +++++++++++
net/ipv4/route.c | 6 +++++-
5 files changed, 19 insertions(+), 1 deletion(-)
diff --git a/include/linux/inetdevice.h b/include/linux/inetdevice.h
index 27650f1..c759d1c 100644
--- a/include/linux/inetdevice.h
+++ b/include/linux/inetdevice.h
@@ -93,6 +93,7 @@ static inline void ipv4_devconf_setall(struct in_device *in_dev)
#define IN_DEV_FORWARD(in_dev) IN_DEV_CONF_GET((in_dev), FORWARDING)
#define IN_DEV_MFORWARD(in_dev) IN_DEV_ANDCONF((in_dev), MC_FORWARDING)
+#define IN_DEV_BFORWARD(in_dev) IN_DEV_ANDCONF((in_dev), BC_FORWARDING)
#define IN_DEV_RPFILTER(in_dev) IN_DEV_MAXCONF((in_dev), RP_FILTER)
#define IN_DEV_SRC_VMARK(in_dev) IN_DEV_ORCONF((in_dev), SRC_VMARK)
#define IN_DEV_SOURCE_ROUTE(in_dev) IN_DEV_ANDCONF((in_dev), \
diff --git a/include/uapi/linux/ip.h b/include/uapi/linux/ip.h
index b24a742..e42d13b 100644
--- a/include/uapi/linux/ip.h
+++ b/include/uapi/linux/ip.h
@@ -168,6 +168,7 @@ enum
IPV4_DEVCONF_IGNORE_ROUTES_WITH_LINKDOWN,
IPV4_DEVCONF_DROP_UNICAST_IN_L2_MULTICAST,
IPV4_DEVCONF_DROP_GRATUITOUS_ARP,
+ IPV4_DEVCONF_BC_FORWARDING,
__IPV4_DEVCONF_MAX
};
diff --git a/include/uapi/linux/netconf.h b/include/uapi/linux/netconf.h
index c84fcdf..fac4edd 100644
--- a/include/uapi/linux/netconf.h
+++ b/include/uapi/linux/netconf.h
@@ -18,6 +18,7 @@ enum {
NETCONFA_PROXY_NEIGH,
NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN,
NETCONFA_INPUT,
+ NETCONFA_BC_FORWARDING,
__NETCONFA_MAX
};
#define NETCONFA_MAX (__NETCONFA_MAX - 1)
diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
index d7585ab..ea4bd8a 100644
--- a/net/ipv4/devinet.c
+++ b/net/ipv4/devinet.c
@@ -1827,6 +1827,8 @@ static int inet_netconf_msgsize_devconf(int type)
size += nla_total_size(4);
if (all || type == NETCONFA_MC_FORWARDING)
size += nla_total_size(4);
+ if (all || type == NETCONFA_BC_FORWARDING)
+ size += nla_total_size(4);
if (all || type == NETCONFA_PROXY_NEIGH)
size += nla_total_size(4);
if (all || type == NETCONFA_IGNORE_ROUTES_WITH_LINKDOWN)
@@ -1873,6 +1875,10 @@ static int inet_netconf_fill_devconf(struct sk_buff *skb, int ifindex,
nla_put_s32(skb, NETCONFA_MC_FORWARDING,
IPV4_DEVCONF(*devconf, MC_FORWARDING)) < 0)
goto nla_put_failure;
+ if ((all || type == NETCONFA_BC_FORWARDING) &&
+ nla_put_s32(skb, NETCONFA_BC_FORWARDING,
+ IPV4_DEVCONF(*devconf, BC_FORWARDING)) < 0)
+ goto nla_put_failure;
if ((all || type == NETCONFA_PROXY_NEIGH) &&
nla_put_s32(skb, NETCONFA_PROXY_NEIGH,
IPV4_DEVCONF(*devconf, PROXY_ARP)) < 0)
@@ -2143,6 +2149,10 @@ static int devinet_conf_proc(struct ctl_table *ctl, int write,
if ((new_value == 0) && (old_value != 0))
rt_cache_flush(net);
+ if (i == IPV4_DEVCONF_BC_FORWARDING - 1 &&
+ new_value != old_value)
+ rt_cache_flush(net);
+
if (i == IPV4_DEVCONF_RP_FILTER - 1 &&
new_value != old_value) {
ifindex = devinet_conf_ifindex(net, cnf);
@@ -2259,6 +2269,7 @@ static struct devinet_sysctl_table {
DEVINET_SYSCTL_COMPLEX_ENTRY(FORWARDING, "forwarding",
devinet_sysctl_forward),
DEVINET_SYSCTL_RO_ENTRY(MC_FORWARDING, "mc_forwarding"),
+ DEVINET_SYSCTL_RW_ENTRY(BC_FORWARDING, "bc_forwarding"),
DEVINET_SYSCTL_RW_ENTRY(ACCEPT_REDIRECTS, "accept_redirects"),
DEVINET_SYSCTL_RW_ENTRY(SECURE_REDIRECTS, "secure_redirects"),
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 1df6e97..b678466 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1996,8 +1996,11 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
goto no_route;
}
- if (res->type == RTN_BROADCAST)
+ if (res->type == RTN_BROADCAST) {
+ if (IN_DEV_BFORWARD(in_dev))
+ goto make_route;
goto brd_input;
+ }
if (res->type == RTN_LOCAL) {
err = fib_validate_source(skb, saddr, daddr, tos,
@@ -2014,6 +2017,7 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
if (res->type != RTN_UNICAST)
goto martian_destination;
+make_route:
err = ip_mkroute_input(skb, res, in_dev, daddr, saddr, tos, flkeys);
out: return err;
--
2.1.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* [PATCHv3 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
2018-07-23 11:51 ` [PATCHv3 net-next 1/2] route: add support " Xin Long
@ 2018-07-23 11:51 ` Xin Long
2018-07-23 15:17 ` David Ahern
0 siblings, 1 reply; 8+ messages in thread
From: Xin Long @ 2018-07-23 11:51 UTC (permalink / raw)
To: network dev; +Cc: davem, David Ahern, Davide Caratti, idosch
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
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
--
2.1.0
^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: [PATCHv3 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
2018-07-23 11:51 ` [PATCHv3 net-next 2/2] selftests: add a selftest " Xin Long
@ 2018-07-23 15:17 ` David Ahern
2018-07-24 17:24 ` Xin Long
0 siblings, 1 reply; 8+ messages in thread
From: David Ahern @ 2018-07-23 15:17 UTC (permalink / raw)
To: Xin Long, network dev; +Cc: davem, Davide Caratti, idosch
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCHv3 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
2018-07-23 15:17 ` David Ahern
@ 2018-07-24 17:24 ` Xin Long
2018-07-24 17:37 ` David Ahern
0 siblings, 1 reply; 8+ messages in thread
From: Xin Long @ 2018-07-24 17:24 UTC (permalink / raw)
To: David Ahern; +Cc: network dev, davem, Davide Caratti, Ido Schimmel
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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCHv3 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
2018-07-24 17:24 ` Xin Long
@ 2018-07-24 17:37 ` David Ahern
2018-07-24 17:55 ` Xin Long
0 siblings, 1 reply; 8+ messages in thread
From: David Ahern @ 2018-07-24 17:37 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, davem, Davide Caratti, Ido Schimmel
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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCHv3 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
2018-07-24 17:37 ` David Ahern
@ 2018-07-24 17:55 ` Xin Long
2018-07-24 18:36 ` David Ahern
0 siblings, 1 reply; 8+ messages in thread
From: Xin Long @ 2018-07-24 17:55 UTC (permalink / raw)
To: David Ahern; +Cc: network dev, davem, Davide Caratti, Ido Schimmel
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?
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [PATCHv3 net-next 2/2] selftests: add a selftest for directed broadcast forwarding
2018-07-24 17:55 ` Xin Long
@ 2018-07-24 18:36 ` David Ahern
0 siblings, 0 replies; 8+ messages in thread
From: David Ahern @ 2018-07-24 18:36 UTC (permalink / raw)
To: Xin Long; +Cc: network dev, davem, Davide Caratti, Ido Schimmel
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]
...
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2018-07-24 19:43 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-07-23 11:51 [PATCHv3 net-next 0/2] route: add support and selftests for directed broadcast forwarding Xin Long
2018-07-23 11:51 ` [PATCHv3 net-next 1/2] route: add support " Xin Long
2018-07-23 11:51 ` [PATCHv3 net-next 2/2] selftests: add a selftest " Xin Long
2018-07-23 15:17 ` David Ahern
2018-07-24 17:24 ` Xin Long
2018-07-24 17:37 ` David Ahern
2018-07-24 17:55 ` Xin Long
2018-07-24 18:36 ` David Ahern
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.