[PATCH net-next v2 0/3] fib rule selftest

[PATCH net-next v2 0/3] fib rule selftest

[Roopa Prabhu]

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This series adds a new test to test fib rules.
ip route get is used to test fib rule matches.
This series also extends ip route get to match on
sport and dport to test recent support of sport
and dport fib rule match.

v2 - address ido's commemt to make sport dport
ip route get to work correctly for input route
get. I don't support ip route get on ip-proto match yet.
ip route get creates a udp packet and i have left
it at that. We could extend ip route get to support
a few ip proto matches in followup patches.


Roopa Prabhu (3):
  ipv4: support sport and dport in RTM_GETROUTE
  ipv6: support sport and dport in RTM_GETROUTE
  selftests: net: initial fib rule tests

 include/uapi/linux/rtnetlink.h                |   2 +
 net/ipv4/route.c                              | 152 ++++++++++++-----
 net/ipv6/route.c                              |  25 +++
 tools/testing/selftests/net/Makefile          |   2 +-
 tools/testing/selftests/net/fib_rule_tests.sh | 224 ++++++++++++++++++++++++++
 5 files changed, 366 insertions(+), 39 deletions(-)
 create mode 100644 tools/testing/selftests/net/fib_rule_tests.sh

-- 
2.1.4

[PATCH net-next v2 1/3] ipv4: support sport and dport in RTM_GETROUTE

[Roopa Prabhu]

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This is a followup to fib rules sport, dport match support.
Having them supported in getroute makes it easier to test
fib rule lookups. Used by fib rule self tests. Before this patch
getroute used same skb to pass through the route lookup and
for the netlink getroute reply msg. This patch allocates separate
skb's to keep flow dissector happy.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 include/uapi/linux/rtnetlink.h |   2 +
 net/ipv4/route.c               | 151 ++++++++++++++++++++++++++++++-----------
 2 files changed, 115 insertions(+), 38 deletions(-)

diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
index 9b15005..630ecf4 100644
--- a/include/uapi/linux/rtnetlink.h
+++ b/include/uapi/linux/rtnetlink.h
@@ -327,6 +327,8 @@ enum rtattr_type_t {
 	RTA_PAD,
 	RTA_UID,
 	RTA_TTL_PROPAGATE,
+	RTA_SPORT,
+	RTA_DPORT,
 	__RTA_MAX
 };
 
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 1412a7b..e91ed62 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2568,11 +2568,10 @@ struct rtable *ip_route_output_flow(struct net *net, struct flowi4 *flp4,
 EXPORT_SYMBOL_GPL(ip_route_output_flow);
 
 /* called with rcu_read_lock held */
-static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
-			struct flowi4 *fl4, struct sk_buff *skb, u32 portid,
-			u32 seq)
+static int rt_fill_info(struct net *net, __be32 dst, __be32 src,
+			struct rtable *rt, u32 table_id, struct flowi4 *fl4,
+			struct sk_buff *skb, u32 portid, u32 seq)
 {
-	struct rtable *rt = skb_rtable(skb);
 	struct rtmsg *r;
 	struct nlmsghdr *nlh;
 	unsigned long expires = 0;
@@ -2651,6 +2650,14 @@ static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
 			from_kuid_munged(current_user_ns(), fl4->flowi4_uid)))
 		goto nla_put_failure;
 
+	if (fl4->fl4_sport &&
+	    nla_put_be16(skb, RTA_SPORT, fl4->fl4_sport))
+		goto nla_put_failure;
+
+	if (fl4->fl4_dport &&
+	    nla_put_be16(skb, RTA_DPORT, fl4->fl4_dport))
+		goto nla_put_failure;
+
 	error = rt->dst.error;
 
 	if (rt_is_input_route(rt)) {
@@ -2668,7 +2675,7 @@ static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
 			}
 		} else
 #endif
-			if (nla_put_u32(skb, RTA_IIF, skb->dev->ifindex))
+			if (nla_put_u32(skb, RTA_IIF, fl4->flowi4_iif))
 				goto nla_put_failure;
 	}
 
@@ -2683,35 +2690,86 @@ static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
 	return -EMSGSIZE;
 }
 
+static int nla_get_port(struct nlattr *attr, __be16 *port)
+{
+	int p = nla_get_be16(attr);
+
+	if (p <= 0 || p >= 0xffff)
+		return -EINVAL;
+
+	*port = p;
+	return 0;
+}
+
+static int inet_rtm_getroute_reply(struct sk_buff *in_skb, struct nlmsghdr *nlh,
+				   __be32 dst, __be32 src, struct flowi4 *fl4,
+				   struct rtable *rt, struct fib_result *res)
+{
+	struct net *net = sock_net(in_skb->sk);
+	struct rtmsg *rtm = nlmsg_data(nlh);
+	u32 table_id = RT_TABLE_MAIN;
+	struct sk_buff *skb;
+	int err = 0;
+
+	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
+	if (!skb) {
+		err = -ENOMEM;
+		return err;
+	}
+
+	if (rtm->rtm_flags & RTM_F_LOOKUP_TABLE)
+		table_id = res->table ? res->table->tb_id : 0;
+
+	if (rtm->rtm_flags & RTM_F_FIB_MATCH)
+		err = fib_dump_info(skb, NETLINK_CB(in_skb).portid,
+				    nlh->nlmsg_seq, RTM_NEWROUTE, table_id,
+				    rt->rt_type, res->prefix, res->prefixlen,
+				    fl4->flowi4_tos, res->fi, 0);
+	else
+		err = rt_fill_info(net, dst, src, rt, table_id,
+				   fl4, skb, NETLINK_CB(in_skb).portid,
+				   nlh->nlmsg_seq);
+	if (err < 0)
+		goto errout;
+
+	return rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
+
+errout:
+	kfree_skb(skb);
+	return err;
+}
+
 static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 			     struct netlink_ext_ack *extack)
 {
 	struct net *net = sock_net(in_skb->sk);
-	struct rtmsg *rtm;
 	struct nlattr *tb[RTA_MAX+1];
+	__be16 sport = 0, dport = 0;
 	struct fib_result res = {};
 	struct rtable *rt = NULL;
+	struct sk_buff *skb;
+	struct rtmsg *rtm;
 	struct flowi4 fl4;
+	struct iphdr *iph;
+	struct udphdr *udph;
 	__be32 dst = 0;
 	__be32 src = 0;
+	kuid_t uid;
 	u32 iif;
 	int err;
 	int mark;
-	struct sk_buff *skb;
-	u32 table_id = RT_TABLE_MAIN;
-	kuid_t uid;
 
 	err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv4_policy,
 			  extack);
 	if (err < 0)
-		goto errout;
+		return err;
 
 	rtm = nlmsg_data(nlh);
 
 	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
 	if (!skb) {
 		err = -ENOBUFS;
-		goto errout;
+		return err;
 	}
 
 	/* Reserve room for dummy headers, this skb can pass
@@ -2728,15 +2786,36 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 		uid = make_kuid(current_user_ns(), nla_get_u32(tb[RTA_UID]));
 	else
 		uid = (iif ? INVALID_UID : current_uid());
+	if (tb[RTA_SPORT]) {
+		err = nla_get_port(tb[RTA_SPORT], &sport);
+		if (err)
+			goto errout_free;
+	}
 
-	/* Bugfix: need to give ip_route_input enough of an IP header to
-	 * not gag.
-	 */
-	ip_hdr(skb)->protocol = IPPROTO_UDP;
-	ip_hdr(skb)->saddr = src;
-	ip_hdr(skb)->daddr = dst;
+	if (tb[RTA_DPORT]) {
+		err = nla_get_port(tb[RTA_DPORT], &dport);
+		if (err)
+			goto errout_free;
+	}
 
-	skb_reserve(skb, MAX_HEADER + sizeof(struct iphdr));
+	skb->protocol = htons(ETH_P_IP);
+	iph = skb_put(skb, sizeof(struct iphdr));
+	iph->protocol = IPPROTO_UDP;
+	iph->saddr = src;
+	iph->daddr = dst;
+	iph->version = 0x4;
+	iph->frag_off = 0;
+
+	if (sport || dport) {
+		iph->ihl = 0x5;
+		skb_set_transport_header(skb, skb->len);
+
+		udph = skb_put(skb, sizeof(struct udphdr));
+		udph->dest = dport;
+		udph->source = sport;
+		udph->len = sizeof(struct udphdr);
+		udph->check = 0;
+	}
 
 	memset(&fl4, 0, sizeof(fl4));
 	fl4.daddr = dst;
@@ -2745,6 +2824,11 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 	fl4.flowi4_oif = tb[RTA_OIF] ? nla_get_u32(tb[RTA_OIF]) : 0;
 	fl4.flowi4_mark = mark;
 	fl4.flowi4_uid = uid;
+	if (sport)
+		fl4.fl4_sport = sport;
+	if (dport)
+		fl4.fl4_dport = dport;
+	fl4.flowi4_proto = IPPROTO_UDP;
 
 	rcu_read_lock();
 
@@ -2754,9 +2838,10 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 		dev = dev_get_by_index_rcu(net, iif);
 		if (!dev) {
 			err = -ENODEV;
-			goto errout_free;
+			goto errout_rcu;
 		}
 
+		fl4.flowi4_iif = iif; /* for rt_fill_info */
 		skb->protocol	= htons(ETH_P_IP);
 		skb->dev	= dev;
 		skb->mark	= mark;
@@ -2777,42 +2862,32 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 	}
 
 	if (err)
-		goto errout_free;
+		goto errout_rcu;
 
 	if (rtm->rtm_flags & RTM_F_NOTIFY)
 		rt->rt_flags |= RTCF_NOTIFY;
 
-	if (rtm->rtm_flags & RTM_F_LOOKUP_TABLE)
-		table_id = res.table ? res.table->tb_id : 0;
-
 	if (rtm->rtm_flags & RTM_F_FIB_MATCH) {
 		if (!res.fi) {
 			err = fib_props[res.type].error;
 			if (!err)
 				err = -EHOSTUNREACH;
-			goto errout_free;
+			goto errout_rcu;
 		}
-		err = fib_dump_info(skb, NETLINK_CB(in_skb).portid,
-				    nlh->nlmsg_seq, RTM_NEWROUTE, table_id,
-				    rt->rt_type, res.prefix, res.prefixlen,
-				    fl4.flowi4_tos, res.fi, 0);
-	} else {
-		err = rt_fill_info(net, dst, src, table_id, &fl4, skb,
-				   NETLINK_CB(in_skb).portid, nlh->nlmsg_seq);
 	}
+
+	err = inet_rtm_getroute_reply(in_skb, nlh, dst, src, &fl4, rt, &res);
 	if (err < 0)
-		goto errout_free;
+		goto errout_rcu;
 
 	rcu_read_unlock();
 
-	err = rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
-errout:
-	return err;
-
 errout_free:
-	rcu_read_unlock();
 	kfree_skb(skb);
-	goto errout;
+	return err;
+errout_rcu:
+	rcu_read_unlock();
+	goto errout_free;
 }
 
 void ip_rt_multicast_event(struct in_device *in_dev)
-- 
2.1.4

[PATCH net-next v2 2/3] ipv6: support sport and dport in RTM_GETROUTE

[Roopa Prabhu]

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This is a followup to fib6 rules sport and dport
match support. Having them supported in getroute
makes it easier to test fib6 rule lookups. Used by fib6 rule
self tests.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 net/ipv6/route.c | 25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 8ed1b51..bcdc056 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -4071,6 +4071,8 @@ static const struct nla_policy rtm_ipv6_policy[RTA_MAX+1] = {
 	[RTA_UID]		= { .type = NLA_U32 },
 	[RTA_MARK]		= { .type = NLA_U32 },
 	[RTA_TABLE]		= { .type = NLA_U32 },
+	[RTA_SPORT]		= { .type = NLA_U16 },
+	[RTA_DPORT]		= { .type = NLA_U16 },
 };
 
 static int rtm_to_fib6_config(struct sk_buff *skb, struct nlmsghdr *nlh,
@@ -4728,6 +4730,17 @@ int rt6_dump_route(struct fib6_info *rt, void *p_arg)
 			     arg->cb->nlh->nlmsg_seq, NLM_F_MULTI);
 }
 
+static int nla_get_port(struct nlattr *attr, __be16 *port)
+{
+	int p = nla_get_be16(attr);
+
+	if (p <= 0 || p >= 0xffff)
+		return -EINVAL;
+
+	*port = p;
+	return 0;
+}
+
 static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 			      struct netlink_ext_ack *extack)
 {
@@ -4782,6 +4795,18 @@ static int inet6_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 	else
 		fl6.flowi6_uid = iif ? INVALID_UID : current_uid();
 
+	if (tb[RTA_SPORT]) {
+		err = nla_get_port(tb[RTA_SPORT], &fl6.fl6_sport);
+		if (err)
+			goto errout;
+	}
+
+	if (tb[RTA_DPORT]) {
+		err = nla_get_port(tb[RTA_DPORT], &fl6.fl6_dport);
+		if (err)
+			goto errout;
+	}
+
 	if (iif) {
 		struct net_device *dev;
 		int flags = 0;
-- 
2.1.4

[PATCH net-next v2 3/3] selftests: net: initial fib rule tests

[Roopa Prabhu]

From: Roopa Prabhu <roopa@cumulusnetworks.com>

This adds a first set of tests for fib rule match/action for
ipv4 and ipv6. Initial tests only cover action lookup table.
can be extended to cover other actions in the future.
Uses ip route get to validate the rule lookup.

Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
---
 tools/testing/selftests/net/Makefile          |   2 +-
 tools/testing/selftests/net/fib_rule_tests.sh | 224 ++++++++++++++++++++++++++
 2 files changed, 225 insertions(+), 1 deletion(-)
 create mode 100644 tools/testing/selftests/net/fib_rule_tests.sh

diff --git a/tools/testing/selftests/net/Makefile b/tools/testing/selftests/net/Makefile
index 902820d..9a8f9b0 100644
--- a/tools/testing/selftests/net/Makefile
+++ b/tools/testing/selftests/net/Makefile
@@ -6,7 +6,7 @@ CFLAGS += -I../../../../usr/include/
 
 TEST_PROGS := run_netsocktests run_afpackettests test_bpf.sh netdevice.sh rtnetlink.sh
 TEST_PROGS += fib_tests.sh fib-onlink-tests.sh in_netns.sh pmtu.sh udpgso.sh
-TEST_PROGS += udpgso_bench.sh
+TEST_PROGS += udpgso_bench.sh fib_rule_tests.sh
 TEST_GEN_PROGS_EXTENDED := in_netns.sh
 TEST_GEN_FILES =  socket
 TEST_GEN_FILES += psock_fanout psock_tpacket msg_zerocopy
diff --git a/tools/testing/selftests/net/fib_rule_tests.sh b/tools/testing/selftests/net/fib_rule_tests.sh
new file mode 100644
index 0000000..01a250f
--- /dev/null
+++ b/tools/testing/selftests/net/fib_rule_tests.sh
@@ -0,0 +1,224 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+
+# This test is for checking IPv4 and IPv6 FIB rules API
+
+ret=0
+
+PAUSE_ON_FAIL=${PAUSE_ON_FAIL:=no}
+IP="ip -netns testns"
+
+RTABLE=100
+GW_IP4=192.51.100.2
+SRC_IP=192.51.100.3
+GW_IP6=2001:db8:1::2
+SRC_IP6=2001:db8:1::3
+
+DEV_ADDR=192.51.100.1
+DEV=dummy0
+
+log_test()
+{
+	local rc=$1
+	local expected=$2
+	local msg="$3"
+
+	if [ ${rc} -eq ${expected} ]; then
+		nsuccess=$((nsuccess+1))
+		printf "\n    TEST: %-50s  [ OK ]\n" "${msg}"
+	else
+		nfail=$((nfail+1))
+		printf "\n    TEST: %-50s  [FAIL]\n" "${msg}"
+		if [ "${PAUSE_ON_FAIL}" = "yes" ]; then
+			echo
+			echo "hit enter to continue, 'q' to quit"
+			read a
+			[ "$a" = "q" ] && exit 1
+		fi
+	fi
+}
+
+log_section()
+{
+	echo
+	echo "######################################################################"
+	echo "TEST SECTION: $*"
+	echo "######################################################################"
+}
+
+setup()
+{
+	set -e
+	ip netns add testns
+	$IP link set dev lo up
+
+	$IP link add dummy0 type dummy
+	$IP link set dev dummy0 up
+	$IP address add 198.51.100.1/24 dev dummy0
+	$IP -6 address add 2001:db8:1::1/64 dev dummy0
+
+	set +e
+}
+
+cleanup()
+{
+	$IP link del dev dummy0 &> /dev/null
+	ip netns del testns
+}
+
+fib_check_iproute_support()
+{
+	ip rule help 2>&1 | grep -q $1
+	if [ $? -ne 0 ]; then
+		echo "SKIP: iproute2 iprule too old, missing $1 match"
+		return 1
+	fi
+
+	ip route get help 2>&1 | grep -q $2
+	if [ $? -ne 0 ]; then
+		echo "SKIP: iproute2 get route too old, missing $2 match"
+		return 1
+	fi
+
+	return 0
+}
+
+fib_rule6_del()
+{
+	$IP -6 rule del $1
+	log_test $? 0 "rule6 del $1"
+}
+
+fib_rule6_del_by_pref()
+{
+	pref=$($IP -6 rule show | grep "$1 lookup $TABLE" | cut -d ":" -f 1)
+	$IP -6 rule del pref $pref
+}
+
+fib_rule6_test_match_n_redirect()
+{
+	local match="$1"
+	local getmatch="$2"
+
+	$IP -6 rule add $match table $RTABLE
+	$IP -6 route get $GW_IP6 $getmatch | grep -q "table $RTABLE"
+	log_test $? 0 "rule6 check: $1"
+
+	fib_rule6_del_by_pref "$match"
+	log_test $? 0 "rule6 del by pref: $match"
+}
+
+fib_rule6_test()
+{
+	# setup the fib rule redirect route
+	$IP -6 route add table $RTABLE default via $GW_IP6 dev $DEV onlink
+
+	match="oif $DEV"
+	fib_rule6_test_match_n_redirect "$match" "$match" "oif redirect to table"
+
+	match="from $SRC_IP6 iif $DEV"
+	fib_rule6_test_match_n_redirect "$match" "$match" "iif redirect to table"
+
+	match="tos 0x10"
+	fib_rule6_test_match_n_redirect "$match" "$match" "tos redirect to table"
+
+	match="fwmark 0x64"
+	getmatch="mark 0x64"
+	fib_rule6_test_match_n_redirect "$match" "$getmatch" "fwmark redirect to table"
+
+	fib_check_iproute_support "uidrange" "uid"
+	if [ $? -eq 0 ]; then
+		match="uidrange 100-100"
+		getmatch="uid 100"
+		fib_rule6_test_match_n_redirect "$match" "$getmatch" "uid redirect to table"
+	fi
+
+	fib_check_iproute_support "sport" "sport"
+	if [ $? -eq 0 ]; then
+		match="sport 666 dport 777"
+		fib_rule6_test_match_n_redirect "$match" "$match" "sport and dport redirect to table"
+	fi
+}
+
+fib_rule4_del()
+{
+	$IP rule del $1
+	log_test $? 0 "del $1"
+}
+
+fib_rule4_del_by_pref()
+{
+	pref=$($IP rule show | grep "$1 lookup $TABLE" | cut -d ":" -f 1)
+	$IP rule del pref $pref
+}
+
+fib_rule4_test_match_n_redirect()
+{
+	local match="$1"
+	local getmatch="$2"
+
+	$IP rule add $match table $RTABLE
+	$IP route get $GW_IP4 $getmatch | grep -q "table $RTABLE"
+	log_test $? 0 "rule4 check: $1"
+
+	fib_rule4_del_by_pref "$match"
+	log_test $? 0 "rule4 del by pref: $match"
+}
+
+fib_rule4_test()
+{
+	# setup the fib rule redirect route
+	$IP route add table $RTABLE default via $GW_IP4 dev $DEV onlink
+
+	match="oif $DEV"
+	fib_rule4_test_match_n_redirect "$match" "$match" "oif redirect to table"
+
+	match="from $SRC_IP iif $DEV"
+	fib_rule4_test_match_n_redirect "$match" "$match" "iif redirect to table"
+
+	match="tos 0x10"
+	fib_rule4_test_match_n_redirect "$match" "$match" "tos redirect to table"
+
+	match="fwmark 0x64"
+	getmatch="mark 0x64"
+	fib_rule4_test_match_n_redirect "$match" "$getmatch" "fwmark redirect to table"
+
+	fib_check_iproute_support "uidrange" "uid"
+	if [ $? -eq 0 ]; then
+		match="uidrange 100-100"
+		getmatch="uid 100"
+		fib_rule4_test_match_n_redirect "$match" "$getmatch" "uid redirect to table"
+	fi
+
+	fib_check_iproute_support "sport" "sport"
+	if [ $? -eq 0 ]; then
+		match="sport 666 dport 777"
+		fib_rule4_test_match_n_redirect "$match" "$match" "sport and dport redirect to table"
+	fi
+}
+
+run_fibrule_tests()
+{
+	log_section "IPv4 fib rule"
+	fib_rule4_test
+	log_section "IPv6 fib rule"
+	fib_rule6_test
+}
+
+if [ "$(id -u)" -ne 0 ];then
+	echo "SKIP: Need root privileges"
+	exit 0
+fi
+
+if [ ! -x "$(command -v ip)" ]; then
+	echo "SKIP: Could not run test without ip tool"
+	exit 0
+fi
+
+# start clean
+cleanup &> /dev/null
+setup
+run_fibrule_tests
+cleanup
+
+exit $ret
-- 
2.1.4

Re: [PATCH net-next v2 1/3] ipv4: support sport and dport in RTM_GETROUTE

[David Ahern]

On 5/6/18 6:59 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> This is a followup to fib rules sport, dport match support.
> Having them supported in getroute makes it easier to test
> fib rule lookups. Used by fib rule self tests. Before this patch
> getroute used same skb to pass through the route lookup and
> for the netlink getroute reply msg. This patch allocates separate
> skb's to keep flow dissector happy.
> 
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
>  include/uapi/linux/rtnetlink.h |   2 +
>  net/ipv4/route.c               | 151 ++++++++++++++++++++++++++++++-----------
>  2 files changed, 115 insertions(+), 38 deletions(-)
> 
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index 9b15005..630ecf4 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -327,6 +327,8 @@ enum rtattr_type_t {
>  	RTA_PAD,
>  	RTA_UID,
>  	RTA_TTL_PROPAGATE,
> +	RTA_SPORT,
> +	RTA_DPORT,

If you are going to add sport and dport because of the potential for FIB
rules, you need to add ip-proto as well. I realize existing code assumed
UDP, but the FIB rules cover any IP proto. Yes, I know this makes the
change much larger to generate tcp, udp as well as iphdr options; the
joys of new features. ;-)

I also suggest a comment that these new RTA attributes are used for
GETROUTE only.

And you need to add the new entries to rtm_ipv4_policy.


>  	__RTA_MAX
>  };
>  
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 1412a7b..e91ed62 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -2568,11 +2568,10 @@ struct rtable *ip_route_output_flow(struct net *net, struct flowi4 *flp4,
>  EXPORT_SYMBOL_GPL(ip_route_output_flow);
>  
>  /* called with rcu_read_lock held */
> -static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
> -			struct flowi4 *fl4, struct sk_buff *skb, u32 portid,
> -			u32 seq)
> +static int rt_fill_info(struct net *net, __be32 dst, __be32 src,
> +			struct rtable *rt, u32 table_id, struct flowi4 *fl4,
> +			struct sk_buff *skb, u32 portid, u32 seq)
>  {
> -	struct rtable *rt = skb_rtable(skb);
>  	struct rtmsg *r;
>  	struct nlmsghdr *nlh;
>  	unsigned long expires = 0;
> @@ -2651,6 +2650,14 @@ static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
>  			from_kuid_munged(current_user_ns(), fl4->flowi4_uid)))
>  		goto nla_put_failure;
>  
> +	if (fl4->fl4_sport &&
> +	    nla_put_be16(skb, RTA_SPORT, fl4->fl4_sport))
> +		goto nla_put_failure;
> +
> +	if (fl4->fl4_dport &&
> +	    nla_put_be16(skb, RTA_DPORT, fl4->fl4_dport))
> +		goto nla_put_failure;

Why return the attributes to the user? I can't see any value in that.
UID option is not returned either so there is precedence.


> +
>  	error = rt->dst.error;
>  
>  	if (rt_is_input_route(rt)) {
> @@ -2668,7 +2675,7 @@ static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
>  			}
>  		} else
>  #endif
> -			if (nla_put_u32(skb, RTA_IIF, skb->dev->ifindex))
> +			if (nla_put_u32(skb, RTA_IIF, fl4->flowi4_iif))
>  				goto nla_put_failure;
>  	}
>  
> @@ -2683,35 +2690,86 @@ static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
>  	return -EMSGSIZE;
>  }
>  
> +static int nla_get_port(struct nlattr *attr, __be16 *port)
> +{
> +	int p = nla_get_be16(attr);

__be16 p;

> +
> +	if (p <= 0 || p >= 0xffff)
> +		return -EINVAL;

This check is not needed by definition of be16.

> +
> +	*port = p;
> +	return 0;
> +}
> +
> +static int inet_rtm_getroute_reply(struct sk_buff *in_skb, struct nlmsghdr *nlh,
> +				   __be32 dst, __be32 src, struct flowi4 *fl4,
> +				   struct rtable *rt, struct fib_result *res)
> +{
> +	struct net *net = sock_net(in_skb->sk);
> +	struct rtmsg *rtm = nlmsg_data(nlh);
> +	u32 table_id = RT_TABLE_MAIN;
> +	struct sk_buff *skb;
> +	int err = 0;
> +
> +	skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
> +	if (!skb) {
> +		err = -ENOMEM;
> +		return err;
> +	}

just 'return -ENOMEM' and without the {}.


> +
> +	if (rtm->rtm_flags & RTM_F_LOOKUP_TABLE)
> +		table_id = res->table ? res->table->tb_id : 0;
> +
> +	if (rtm->rtm_flags & RTM_F_FIB_MATCH)
> +		err = fib_dump_info(skb, NETLINK_CB(in_skb).portid,
> +				    nlh->nlmsg_seq, RTM_NEWROUTE, table_id,
> +				    rt->rt_type, res->prefix, res->prefixlen,
> +				    fl4->flowi4_tos, res->fi, 0);
> +	else
> +		err = rt_fill_info(net, dst, src, rt, table_id,
> +				   fl4, skb, NETLINK_CB(in_skb).portid,
> +				   nlh->nlmsg_seq);
> +	if (err < 0)
> +		goto errout;
> +
> +	return rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
> +
> +errout:
> +	kfree_skb(skb);
> +	return err;
> +}
> +
>  static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
>  			     struct netlink_ext_ack *extack)
>  {
>  	struct net *net = sock_net(in_skb->sk);
> -	struct rtmsg *rtm;
>  	struct nlattr *tb[RTA_MAX+1];
> +	__be16 sport = 0, dport = 0;
>  	struct fib_result res = {};
>  	struct rtable *rt = NULL;
> +	struct sk_buff *skb;
> +	struct rtmsg *rtm;
>  	struct flowi4 fl4;
> +	struct iphdr *iph;
> +	struct udphdr *udph;
>  	__be32 dst = 0;
>  	__be32 src = 0;
> +	kuid_t uid;
>  	u32 iif;
>  	int err;
>  	int mark;
> -	struct sk_buff *skb;
> -	u32 table_id = RT_TABLE_MAIN;
> -	kuid_t uid;
>  
>  	err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv4_policy,
>  			  extack);
>  	if (err < 0)
> -		goto errout;
> +		return err;
>  
>  	rtm = nlmsg_data(nlh);
>  
>  	skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
>  	if (!skb) {
>  		err = -ENOBUFS;
> -		goto errout;
> +		return err;

just return -ENOBUFS


>  	}
>  
>  	/* Reserve room for dummy headers, this skb can pass

Re: [PATCH net-next v2 2/3] ipv6: support sport and dport in RTM_GETROUTE

[David Ahern]

On 5/6/18 6:59 PM, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@cumulusnetworks.com>
> 
> This is a followup to fib6 rules sport and dport
> match support. Having them supported in getroute
> makes it easier to test fib6 rule lookups. Used by fib6 rule
> self tests.
> 
> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
> ---
>  net/ipv6/route.c | 25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)

similar comments as IPv4 patch.

Re: [PATCH net-next v2 1/3] ipv4: support sport and dport in RTM_GETROUTE

[kbuild test robot]

Hi Roopa,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Roopa-Prabhu/fib-rule-selftest/20180507-094538
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   net/ipv4/route.c:1271:31: sparse: expression using sizeof(void)
   net/ipv4/route.c:1271:31: sparse: expression using sizeof(void)
   net/ipv4/route.c:1274:16: sparse: expression using sizeof(void)
   net/ipv4/route.c:1274:16: sparse: expression using sizeof(void)
   net/ipv4/route.c:1295:15: sparse: expression using sizeof(void)
   net/ipv4/route.c:688:38: sparse: expression using sizeof(void)
   net/ipv4/route.c:712:38: sparse: expression using sizeof(void)
   net/ipv4/route.c:782:46: sparse: incorrect type in argument 2 (different base types) @@    expected unsigned int [unsigned] [usertype] key @@    got ed int [unsigned] [usertype] key @@
   net/ipv4/route.c:782:46:    expected unsigned int [unsigned] [usertype] key
   net/ipv4/route.c:782:46:    got restricted __be32 [usertype] new_gw
>> net/ipv4/route.c:2695:29: sparse: incorrect type in initializer (different base types) @@    expected int [signed] p @@    got restint [signed] p @@
   net/ipv4/route.c:2695:29:    expected int [signed] p
   net/ipv4/route.c:2695:29:    got restricted __be16
>> net/ipv4/route.c:2700:15: sparse: incorrect type in assignment (different base types) @@    expected restricted __be16 [usertype] <noident> @@    got 6 [usertype] <noident> @@
   net/ipv4/route.c:2700:15:    expected restricted __be16 [usertype] <noident>
   net/ipv4/route.c:2700:15:    got int [signed] p
>> net/ipv4/route.c:2816:27: sparse: incorrect type in assignment (different base types) @@    expected restricted __be16 [usertype] len @@    got 6 [usertype] len @@
   net/ipv4/route.c:2816:27:    expected restricted __be16 [usertype] len
   net/ipv4/route.c:2816:27:    got unsigned long

vim +2695 net/ipv4/route.c

  2692	
  2693	static int nla_get_port(struct nlattr *attr, __be16 *port)
  2694	{
> 2695		int p = nla_get_be16(attr);
  2696	
  2697		if (p <= 0 || p >= 0xffff)
  2698			return -EINVAL;
  2699	
> 2700		*port = p;
  2701		return 0;
  2702	}
  2703	
  2704	static int inet_rtm_getroute_reply(struct sk_buff *in_skb, struct nlmsghdr *nlh,
  2705					   __be32 dst, __be32 src, struct flowi4 *fl4,
  2706					   struct rtable *rt, struct fib_result *res)
  2707	{
  2708		struct net *net = sock_net(in_skb->sk);
  2709		struct rtmsg *rtm = nlmsg_data(nlh);
  2710		u32 table_id = RT_TABLE_MAIN;
  2711		struct sk_buff *skb;
  2712		int err = 0;
  2713	
  2714		skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
  2715		if (!skb) {
  2716			err = -ENOMEM;
  2717			return err;
  2718		}
  2719	
  2720		if (rtm->rtm_flags & RTM_F_LOOKUP_TABLE)
  2721			table_id = res->table ? res->table->tb_id : 0;
  2722	
  2723		if (rtm->rtm_flags & RTM_F_FIB_MATCH)
  2724			err = fib_dump_info(skb, NETLINK_CB(in_skb).portid,
  2725					    nlh->nlmsg_seq, RTM_NEWROUTE, table_id,
  2726					    rt->rt_type, res->prefix, res->prefixlen,
  2727					    fl4->flowi4_tos, res->fi, 0);
  2728		else
  2729			err = rt_fill_info(net, dst, src, rt, table_id,
  2730					   fl4, skb, NETLINK_CB(in_skb).portid,
  2731					   nlh->nlmsg_seq);
  2732		if (err < 0)
  2733			goto errout;
  2734	
  2735		return rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
  2736	
  2737	errout:
  2738		kfree_skb(skb);
  2739		return err;
  2740	}
  2741	
  2742	static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
  2743				     struct netlink_ext_ack *extack)
  2744	{
  2745		struct net *net = sock_net(in_skb->sk);
  2746		struct nlattr *tb[RTA_MAX+1];
  2747		__be16 sport = 0, dport = 0;
  2748		struct fib_result res = {};
  2749		struct rtable *rt = NULL;
  2750		struct sk_buff *skb;
  2751		struct rtmsg *rtm;
  2752		struct flowi4 fl4;
  2753		struct iphdr *iph;
  2754		struct udphdr *udph;
  2755		__be32 dst = 0;
  2756		__be32 src = 0;
  2757		kuid_t uid;
  2758		u32 iif;
  2759		int err;
  2760		int mark;
  2761	
  2762		err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv4_policy,
  2763				  extack);
  2764		if (err < 0)
  2765			return err;
  2766	
  2767		rtm = nlmsg_data(nlh);
  2768	
  2769		skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
  2770		if (!skb) {
  2771			err = -ENOBUFS;
  2772			return err;
  2773		}
  2774	
  2775		/* Reserve room for dummy headers, this skb can pass
  2776		   through good chunk of routing engine.
  2777		 */
  2778		skb_reset_mac_header(skb);
  2779		skb_reset_network_header(skb);
  2780	
  2781		src = tb[RTA_SRC] ? nla_get_in_addr(tb[RTA_SRC]) : 0;
  2782		dst = tb[RTA_DST] ? nla_get_in_addr(tb[RTA_DST]) : 0;
  2783		iif = tb[RTA_IIF] ? nla_get_u32(tb[RTA_IIF]) : 0;
  2784		mark = tb[RTA_MARK] ? nla_get_u32(tb[RTA_MARK]) : 0;
  2785		if (tb[RTA_UID])
  2786			uid = make_kuid(current_user_ns(), nla_get_u32(tb[RTA_UID]));
  2787		else
  2788			uid = (iif ? INVALID_UID : current_uid());
  2789		if (tb[RTA_SPORT]) {
  2790			err = nla_get_port(tb[RTA_SPORT], &sport);
  2791			if (err)
  2792				goto errout_free;
  2793		}
  2794	
  2795		if (tb[RTA_DPORT]) {
  2796			err = nla_get_port(tb[RTA_DPORT], &dport);
  2797			if (err)
  2798				goto errout_free;
  2799		}
  2800	
  2801		skb->protocol = htons(ETH_P_IP);
  2802		iph = skb_put(skb, sizeof(struct iphdr));
  2803		iph->protocol = IPPROTO_UDP;
  2804		iph->saddr = src;
  2805		iph->daddr = dst;
  2806		iph->version = 0x4;
  2807		iph->frag_off = 0;
  2808	
  2809		if (sport || dport) {
  2810			iph->ihl = 0x5;
  2811			skb_set_transport_header(skb, skb->len);
  2812	
  2813			udph = skb_put(skb, sizeof(struct udphdr));
  2814			udph->dest = dport;
  2815			udph->source = sport;
> 2816			udph->len = sizeof(struct udphdr);
  2817			udph->check = 0;
  2818		}
  2819	
  2820		memset(&fl4, 0, sizeof(fl4));
  2821		fl4.daddr = dst;
  2822		fl4.saddr = src;
  2823		fl4.flowi4_tos = rtm->rtm_tos;
  2824		fl4.flowi4_oif = tb[RTA_OIF] ? nla_get_u32(tb[RTA_OIF]) : 0;
  2825		fl4.flowi4_mark = mark;
  2826		fl4.flowi4_uid = uid;
  2827		if (sport)
  2828			fl4.fl4_sport = sport;
  2829		if (dport)
  2830			fl4.fl4_dport = dport;
  2831		fl4.flowi4_proto = IPPROTO_UDP;
  2832	
  2833		rcu_read_lock();
  2834	
  2835		if (iif) {
  2836			struct net_device *dev;
  2837	
  2838			dev = dev_get_by_index_rcu(net, iif);
  2839			if (!dev) {
  2840				err = -ENODEV;
  2841				goto errout_rcu;
  2842			}
  2843	
  2844			fl4.flowi4_iif = iif; /* for rt_fill_info */
  2845			skb->protocol	= htons(ETH_P_IP);
  2846			skb->dev	= dev;
  2847			skb->mark	= mark;
  2848			err = ip_route_input_rcu(skb, dst, src, rtm->rtm_tos,
  2849						 dev, &res);
  2850	
  2851			rt = skb_rtable(skb);
  2852			if (err == 0 && rt->dst.error)
  2853				err = -rt->dst.error;
  2854		} else {
  2855			fl4.flowi4_iif = LOOPBACK_IFINDEX;
  2856			rt = ip_route_output_key_hash_rcu(net, &fl4, &res, skb);
  2857			err = 0;
  2858			if (IS_ERR(rt))
  2859				err = PTR_ERR(rt);
  2860			else
  2861				skb_dst_set(skb, &rt->dst);
  2862		}
  2863	
  2864		if (err)
  2865			goto errout_rcu;
  2866	
  2867		if (rtm->rtm_flags & RTM_F_NOTIFY)
  2868			rt->rt_flags |= RTCF_NOTIFY;
  2869	
  2870		if (rtm->rtm_flags & RTM_F_FIB_MATCH) {
  2871			if (!res.fi) {
  2872				err = fib_props[res.type].error;
  2873				if (!err)
  2874					err = -EHOSTUNREACH;
  2875				goto errout_rcu;
  2876			}
  2877		}
  2878	
  2879		err = inet_rtm_getroute_reply(in_skb, nlh, dst, src, &fl4, rt, &res);
  2880		if (err < 0)
  2881			goto errout_rcu;
  2882	
  2883		rcu_read_unlock();
  2884	
  2885	errout_free:
  2886		kfree_skb(skb);
  2887		return err;
  2888	errout_rcu:
  2889		rcu_read_unlock();
  2890		goto errout_free;
  2891	}
  2892	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH net-next v2 2/3] ipv6: support sport and dport in RTM_GETROUTE

[kbuild test robot]

Hi Roopa,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on net-next/master]

url:    https://github.com/0day-ci/linux/commits/Roopa-Prabhu/fib-rule-selftest/20180507-094538
reproduce:
        # apt-get install sparse
        make ARCH=x86_64 allmodconfig
        make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   net/ipv6/route.c:2591:15: sparse: expression using sizeof(void)
   net/ipv6/route.c:1391:15: sparse: expression using sizeof(void)
   net/ipv6/route.c:1590:36: sparse: incorrect type in initializer (different address spaces) @@    expected struct fib6_info *from @@    got struct fib6_info struct fib6_info *from @@
   net/ipv6/route.c:1590:36:    expected struct fib6_info *from
   net/ipv6/route.c:1590:36:    got struct fib6_info [noderef] <asn:4>*from
   net/ipv6/route.c:1968:39: sparse: incorrect type in assignment (different base types) @@    expected unsigned int [unsigned] [usertype] flow_label @@    got gned] [usertype] flow_label @@
   net/ipv6/route.c:1968:39:    expected unsigned int [unsigned] [usertype] flow_label
   net/ipv6/route.c:1968:39:    got restricted __be32
   net/ipv6/route.c:2311:15: sparse: expression using sizeof(void)
>> net/ipv6/route.c:4735:29: sparse: incorrect type in initializer (different base types) @@    expected int [signed] p @@    got restint [signed] p @@
   net/ipv6/route.c:4735:29:    expected int [signed] p
   net/ipv6/route.c:4735:29:    got restricted __be16
>> net/ipv6/route.c:4740:15: sparse: incorrect type in assignment (different base types) @@    expected restricted __be16 [usertype] <noident> @@    got 6 [usertype] <noident> @@
   net/ipv6/route.c:4740:15:    expected restricted __be16 [usertype] <noident>
   net/ipv6/route.c:4740:15:    got int [signed] p
   net/ipv6/route.c:3196:12: sparse: context imbalance in 'ip6_route_del' - wrong count at exit

vim +4735 net/ipv6/route.c

  4732	
  4733	static int nla_get_port(struct nlattr *attr, __be16 *port)
  4734	{
> 4735		int p = nla_get_be16(attr);
  4736	
  4737		if (p <= 0 || p >= 0xffff)
  4738			return -EINVAL;
  4739	
> 4740		*port = p;
  4741		return 0;
  4742	}
  4743	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

Re: [PATCH net-next v2 1/3] ipv4: support sport and dport in RTM_GETROUTE

[Roopa Prabhu]

On Sun, May 6, 2018 at 6:46 PM, David Ahern <dsa@cumulusnetworks.com> wrote:
> On 5/6/18 6:59 PM, Roopa Prabhu wrote:
>> From: Roopa Prabhu <roopa@cumulusnetworks.com>
>>
>> This is a followup to fib rules sport, dport match support.
>> Having them supported in getroute makes it easier to test
>> fib rule lookups. Used by fib rule self tests. Before this patch
>> getroute used same skb to pass through the route lookup and
>> for the netlink getroute reply msg. This patch allocates separate
>> skb's to keep flow dissector happy.
>>
>> Signed-off-by: Roopa Prabhu <roopa@cumulusnetworks.com>
>> ---
>>  include/uapi/linux/rtnetlink.h |   2 +
>>  net/ipv4/route.c               | 151 ++++++++++++++++++++++++++++++-----------
>>  2 files changed, 115 insertions(+), 38 deletions(-)
>>
>> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
>> index 9b15005..630ecf4 100644
>> --- a/include/uapi/linux/rtnetlink.h
>> +++ b/include/uapi/linux/rtnetlink.h
>> @@ -327,6 +327,8 @@ enum rtattr_type_t {
>>       RTA_PAD,
>>       RTA_UID,
>>       RTA_TTL_PROPAGATE,
>> +     RTA_SPORT,
>> +     RTA_DPORT,
>
> If you are going to add sport and dport because of the potential for FIB
> rules, you need to add ip-proto as well. I realize existing code assumed
> UDP, but the FIB rules cover any IP proto. Yes, I know this makes the
> change much larger to generate tcp, udp as well as iphdr options; the
> joys of new features. ;-)


:) sure..like i mentioned in the cover letter..., i was thinking of
submitting follow up patches for more ip_proto.
since i will be spinning v3, let me see if i can include that too.



>
> I also suggest a comment that these new RTA attributes are used for
> GETROUTE only.


sure

>
> And you need to add the new entries to rtm_ipv4_policy.
>
>
>>       __RTA_MAX
>>  };

ack,

>>
>> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
>> index 1412a7b..e91ed62 100644
>> --- a/net/ipv4/route.c
>> +++ b/net/ipv4/route.c
>> @@ -2568,11 +2568,10 @@ struct rtable *ip_route_output_flow(struct net *net, struct flowi4 *flp4,
>>  EXPORT_SYMBOL_GPL(ip_route_output_flow);
>>
>>  /* called with rcu_read_lock held */
>> -static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
>> -                     struct flowi4 *fl4, struct sk_buff *skb, u32 portid,
>> -                     u32 seq)
>> +static int rt_fill_info(struct net *net, __be32 dst, __be32 src,
>> +                     struct rtable *rt, u32 table_id, struct flowi4 *fl4,
>> +                     struct sk_buff *skb, u32 portid, u32 seq)
>>  {
>> -     struct rtable *rt = skb_rtable(skb);
>>       struct rtmsg *r;
>>       struct nlmsghdr *nlh;
>>       unsigned long expires = 0;
>> @@ -2651,6 +2650,14 @@ static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
>>                       from_kuid_munged(current_user_ns(), fl4->flowi4_uid)))
>>               goto nla_put_failure;
>>
>> +     if (fl4->fl4_sport &&
>> +         nla_put_be16(skb, RTA_SPORT, fl4->fl4_sport))
>> +             goto nla_put_failure;
>> +
>> +     if (fl4->fl4_dport &&
>> +         nla_put_be16(skb, RTA_DPORT, fl4->fl4_dport))
>> +             goto nla_put_failure;
>
> Why return the attributes to the user? I can't see any value in that.
> UID option is not returned either so there is precedence.

hmm..i do see UID returned just 2 lines above. :)

In the least i think it will confirm that the kernel did see your attributes :).


>
>
>> +
>>       error = rt->dst.error;
>>
>>       if (rt_is_input_route(rt)) {
>> @@ -2668,7 +2675,7 @@ static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
>>                       }
>>               } else
>>  #endif
>> -                     if (nla_put_u32(skb, RTA_IIF, skb->dev->ifindex))
>> +                     if (nla_put_u32(skb, RTA_IIF, fl4->flowi4_iif))
>>                               goto nla_put_failure;
>>       }
>>
>> @@ -2683,35 +2690,86 @@ static int rt_fill_info(struct net *net,  __be32 dst, __be32 src, u32 table_id,
>>       return -EMSGSIZE;
>>  }
>>
>> +static int nla_get_port(struct nlattr *attr, __be16 *port)
>> +{
>> +     int p = nla_get_be16(attr);
>
> __be16 p;
>
>> +
>> +     if (p <= 0 || p >= 0xffff)
>> +             return -EINVAL;
>
> This check is not needed by definition of be16.

ack, will fix the kbuild sparse warning also for both v4 and v6.


>
>> +
>> +     *port = p;
>> +     return 0;
>> +}
>> +
>> +static int inet_rtm_getroute_reply(struct sk_buff *in_skb, struct nlmsghdr *nlh,
>> +                                __be32 dst, __be32 src, struct flowi4 *fl4,
>> +                                struct rtable *rt, struct fib_result *res)
>> +{
>> +     struct net *net = sock_net(in_skb->sk);
>> +     struct rtmsg *rtm = nlmsg_data(nlh);
>> +     u32 table_id = RT_TABLE_MAIN;
>> +     struct sk_buff *skb;
>> +     int err = 0;
>> +
>> +     skb = nlmsg_new(NLMSG_DEFAULT_SIZE, GFP_ATOMIC);
>> +     if (!skb) {
>> +             err = -ENOMEM;
>> +             return err;
>> +     }
>
> just 'return -ENOMEM' and without the {}.

yep

>
>
>> +
>> +     if (rtm->rtm_flags & RTM_F_LOOKUP_TABLE)
>> +             table_id = res->table ? res->table->tb_id : 0;
>> +
>> +     if (rtm->rtm_flags & RTM_F_FIB_MATCH)
>> +             err = fib_dump_info(skb, NETLINK_CB(in_skb).portid,
>> +                                 nlh->nlmsg_seq, RTM_NEWROUTE, table_id,
>> +                                 rt->rt_type, res->prefix, res->prefixlen,
>> +                                 fl4->flowi4_tos, res->fi, 0);
>> +     else
>> +             err = rt_fill_info(net, dst, src, rt, table_id,
>> +                                fl4, skb, NETLINK_CB(in_skb).portid,
>> +                                nlh->nlmsg_seq);
>> +     if (err < 0)
>> +             goto errout;
>> +
>> +     return rtnl_unicast(skb, net, NETLINK_CB(in_skb).portid);
>> +
>> +errout:
>> +     kfree_skb(skb);
>> +     return err;
>> +}
>> +
>>  static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
>>                            struct netlink_ext_ack *extack)
>>  {
>>       struct net *net = sock_net(in_skb->sk);
>> -     struct rtmsg *rtm;
>>       struct nlattr *tb[RTA_MAX+1];
>> +     __be16 sport = 0, dport = 0;
>>       struct fib_result res = {};
>>       struct rtable *rt = NULL;
>> +     struct sk_buff *skb;
>> +     struct rtmsg *rtm;
>>       struct flowi4 fl4;
>> +     struct iphdr *iph;
>> +     struct udphdr *udph;
>>       __be32 dst = 0;
>>       __be32 src = 0;
>> +     kuid_t uid;
>>       u32 iif;
>>       int err;
>>       int mark;
>> -     struct sk_buff *skb;
>> -     u32 table_id = RT_TABLE_MAIN;
>> -     kuid_t uid;
>>
>>       err = nlmsg_parse(nlh, sizeof(*rtm), tb, RTA_MAX, rtm_ipv4_policy,
>>                         extack);
>>       if (err < 0)
>> -             goto errout;
>> +             return err;
>>
>>       rtm = nlmsg_data(nlh);
>>
>>       skb = alloc_skb(NLMSG_GOODSIZE, GFP_KERNEL);
>>       if (!skb) {
>>               err = -ENOBUFS;
>> -             goto errout;
>> +             return err;
>
> just return -ENOBUFS
>

yes

>
>>       }
>>
>>       /* Reserve room for dummy headers, this skb can pass
>