[PATCHv4 net-next 0/2] route: add support and selftests for directed broadcast forwarding

[PATCHv4 net-next 0/2] route: add support and selftests for directed broadcast forwarding

[Xin Long]

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.

v3->v4:
  - improve the output log in the selftest as David Ahern suggested.

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   | 233 +++++++++++++++++++++
 6 files changed, 252 insertions(+), 1 deletion(-)
 create mode 100755 tools/testing/selftests/net/forwarding/router_broadcast.sh

-- 
2.1.0

[PATCHv4 net-next 1/2] route: add support for directed broadcast forwarding

[Xin Long]

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

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

[Xin Long]

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   | 233 +++++++++++++++++++++
 1 file changed, 233 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..7bd2ebb
--- /dev/null
+++ b/tools/testing/selftests/net/forwarding/router_broadcast.sh
@@ -0,0 +1,233 @@
+#!/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
+
+	log_info "ping $dip, expected reply from $from"
+	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 $?
+}
+
+ping_ipv4()
+{
+	sysctl_set net.ipv4.icmp_echo_ignore_broadcasts 0
+
+	bc_forwarding_disable
+	log_info "bc_forwarding disabled on r1 =>"
+	ping_test_from $h1 198.51.100.255 192.0.2.1
+	log_test "h1 -> net2: reply from r1 (not forwarding)"
+	ping_test_from $h1 198.51.200.255 192.0.2.1
+	log_test "h1 -> net3: reply from r1 (not forwarding)"
+	ping_test_from $h1 192.0.2.255 192.0.2.1
+	log_test "h1 -> net1: reply from r1 (not dropping)"
+	ping_test_from $h1 255.255.255.255 192.0.2.1
+	log_test "h1 -> 255.255.255.255: reply from r1 (not forwarding)"
+
+	ping_test_from $h2 192.0.2.255 198.51.100.1
+	log_test "h2 -> net1: reply from r1 (not forwarding)"
+	ping_test_from $h2 198.51.200.255 198.51.100.1
+	log_test "h2 -> net3: reply from r1 (not forwarding)"
+	ping_test_from $h2 198.51.100.255 198.51.100.1
+	log_test "h2 -> net2: reply from r1 (not dropping)"
+	ping_test_from $h2 255.255.255.255 198.51.100.1
+	log_test "h2 -> 255.255.255.255: reply from r1 (not forwarding)"
+	bc_forwarding_restore
+
+	bc_forwarding_enable
+	log_info "bc_forwarding enabled on r1 =>"
+	ping_test_from $h1 198.51.100.255 198.51.100.2
+	log_test "h1 -> net2: reply from h2 (forwarding)"
+	ping_test_from $h1 198.51.200.255 198.51.200.2
+	log_test "h1 -> net3: reply from h3 (forwarding)"
+	ping_test_from $h1 192.0.2.255 192.0.2.1 1
+	log_test "h1 -> net1: no reply (dropping)"
+	ping_test_from $h1 255.255.255.255 192.0.2.1
+	log_test "h1 -> 255.255.255.255: reply from r1 (not forwarding)"
+
+	ping_test_from $h2 192.0.2.255 192.0.2.2
+	log_test "h2 -> net1: reply from h1 (forwarding)"
+	ping_test_from $h2 198.51.200.255 198.51.200.2
+	log_test "h2 -> net3: reply from h3 (forwarding)"
+	ping_test_from $h2 198.51.100.255 198.51.100.1 1
+	log_test "h2 -> net2: no reply (dropping)"
+	ping_test_from $h2 255.255.255.255 198.51.100.1
+	log_test "h2 -> 255.255.255.255: reply from r1 (not forwarding)"
+	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

Re: [PATCHv4 net-next 0/2] route: add support and selftests for directed broadcast forwarding

[David Miller]

From: Xin Long <lucien.xin@gmail.com>
Date: Fri, 27 Jul 2018 16:37:27 +0800

> 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.
> 
> v3->v4:
>   - improve the output log in the selftest as David Ahern suggested.

Series applied, thanks Xin.