[PATCH net-next 0/7] net: ip: add drop reasons to input route

[PATCH net-next 0/7] net: ip: add drop reasons to input route

[Menglong Dong]

In this series, we mainly add some skb drop reasons to the input path of
ip routing.

The function ip_route_input_noref() is used commonly, and its return value
is used by the caller sometimes. So, it's not easy to make it return skb
drop reasons. Instead, we add the pointer of the drop reason to the
function arguments of it. And we do the same things to
ip_route_input_rcu() and ip_route_input_slow().

The errno from fib_validate_source() is -EINVAL or -EXDEV, and -EXDEV is
used in ip_rcv_finish_core() to increase the LINUX_MIB_IPRPFILTER. For
this case, we can check it by
"drop_reason == SKB_DROP_REASON_IP_RPFILTER" instead. Therefore, we can
make fib_validate_source() return -reason. Meanwhile, we make
ip_route_input_mc() and ip_mc_validate_source() return drop reason.

Following new skb drop reasons are added:

  SKB_DROP_REASON_IP_LOCAL_SOURCE
  SKB_DROP_REASON_IP_INVALID_SOURCE
  SKB_DROP_REASON_IP_INVALID_DEST
  SKB_DROP_REASON_IP_LOCALNET

Menglong Dong (7):
  net: ip: add drop reason to ip_route_input_noref()
  net: ip: add drop reason to ip_route_input_rcu()
  net: ip: add drop reason to ip_route_input_slow()
  net: ip: make fib_validate_source() return drop reason
  net: ip: make ip_route_input_mc() return drop reason
  net: ip: make ip_mc_validate_source() return drop reason
  net: ip: fix typo in the doc of SKB_DROP_REASON_IP_INNOROUTES

 drivers/net/ipvlan/ipvlan_l3s.c |   2 +-
 include/net/dropreason-core.h   |  21 +++++-
 include/net/route.h             |  12 ++--
 net/core/lwt_bpf.c              |   2 +-
 net/ipv4/arp.c                  |   2 +-
 net/ipv4/fib_frontend.c         |  19 ++++--
 net/ipv4/ip_fragment.c          |   2 +-
 net/ipv4/ip_input.c             |  11 ++--
 net/ipv4/route.c                | 111 +++++++++++++++++++++-----------
 net/ipv4/xfrm4_input.c          |   2 +-
 net/ipv4/xfrm4_protocol.c       |   2 +-
 11 files changed, 122 insertions(+), 64 deletions(-)

-- 
2.39.5

[PATCH net-next 1/7] net: ip: add drop reason to ip_route_input_noref()

[Menglong Dong]

The errno which ip_route_input_noref() returns can be used and checked by
the caller, so it's complex to make ip_route_input_noref() return drop
reason.

Instead, we add the pointer of the skb drop reason to the function
arguments of ip_route_input_noref, and adjust all the callers of it.
Then, we can pass the skb drop reasons to the caller.

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
 drivers/net/ipvlan/ipvlan_l3s.c | 2 +-
 include/net/route.h             | 5 +++--
 net/core/lwt_bpf.c              | 2 +-
 net/ipv4/arp.c                  | 2 +-
 net/ipv4/ip_fragment.c          | 2 +-
 net/ipv4/ip_input.c             | 7 +++----
 net/ipv4/route.c                | 3 ++-
 net/ipv4/xfrm4_input.c          | 2 +-
 net/ipv4/xfrm4_protocol.c       | 2 +-
 9 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/drivers/net/ipvlan/ipvlan_l3s.c b/drivers/net/ipvlan/ipvlan_l3s.c
index d5b05e803219..fbfdd8c00056 100644
--- a/drivers/net/ipvlan/ipvlan_l3s.c
+++ b/drivers/net/ipvlan/ipvlan_l3s.c
@@ -52,7 +52,7 @@ static struct sk_buff *ipvlan_l3_rcv(struct net_device *dev,
 		int err;
 
 		err = ip_route_input_noref(skb, ip4h->daddr, ip4h->saddr,
-					   ip4h->tos, sdev);
+					   ip4h->tos, sdev, NULL);
 		if (unlikely(err))
 			goto out;
 		break;
diff --git a/include/net/route.h b/include/net/route.h
index 1789f1e6640b..cb9f31080517 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -202,7 +202,8 @@ int ip_mc_validate_source(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 			  u8 tos, struct net_device *dev,
 			  struct in_device *in_dev, u32 *itag);
 int ip_route_input_noref(struct sk_buff *skb, __be32 dst, __be32 src,
-			 u8 tos, struct net_device *devin);
+			 u8 tos, struct net_device *devin,
+			 enum skb_drop_reason *reason);
 int ip_route_use_hint(struct sk_buff *skb, __be32 dst, __be32 src,
 		      u8 tos, struct net_device *devin,
 		      const struct sk_buff *hint);
@@ -213,7 +214,7 @@ static inline int ip_route_input(struct sk_buff *skb, __be32 dst, __be32 src,
 	int err;
 
 	rcu_read_lock();
-	err = ip_route_input_noref(skb, dst, src, tos, devin);
+	err = ip_route_input_noref(skb, dst, src, tos, devin, NULL);
 	if (!err) {
 		skb_dst_force(skb);
 		if (!skb_dst(skb))
diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index 1a14f915b7a4..df50f2977c90 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -96,7 +96,7 @@ static int bpf_lwt_input_reroute(struct sk_buff *skb)
 		dev_hold(dev);
 		skb_dst_drop(skb);
 		err = ip_route_input_noref(skb, iph->daddr, iph->saddr,
-					   iph->tos, dev);
+					   iph->tos, dev, NULL);
 		dev_put(dev);
 	} else if (skb->protocol == htons(ETH_P_IPV6)) {
 		skb_dst_drop(skb);
diff --git a/net/ipv4/arp.c b/net/ipv4/arp.c
index 11c1519b3699..a9dac0ef2be6 100644
--- a/net/ipv4/arp.c
+++ b/net/ipv4/arp.c
@@ -835,7 +835,7 @@ static int arp_process(struct net *net, struct sock *sk, struct sk_buff *skb)
 	}
 
 	if (arp->ar_op == htons(ARPOP_REQUEST) &&
-	    ip_route_input_noref(skb, tip, sip, 0, dev) == 0) {
+	    ip_route_input_noref(skb, tip, sip, 0, dev, NULL) == 0) {
 
 		rt = skb_rtable(skb);
 		addr_type = rt->rt_type;
diff --git a/net/ipv4/ip_fragment.c b/net/ipv4/ip_fragment.c
index a92664a5ef2e..cdc75cfc1826 100644
--- a/net/ipv4/ip_fragment.c
+++ b/net/ipv4/ip_fragment.c
@@ -176,7 +176,7 @@ static void ip_expire(struct timer_list *t)
 	/* skb has no dst, perform route lookup again */
 	iph = ip_hdr(head);
 	err = ip_route_input_noref(head, iph->daddr, iph->saddr,
-					   iph->tos, head->dev);
+					   iph->tos, head->dev, NULL);
 	if (err)
 		goto out;
 
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index b6e7d4921309..dc062ae49137 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -318,12 +318,11 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
 			      struct sk_buff *skb, struct net_device *dev,
 			      const struct sk_buff *hint)
 {
+	enum skb_drop_reason drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
 	const struct iphdr *iph = ip_hdr(skb);
-	int err, drop_reason;
+	int err;
 	struct rtable *rt;
 
-	drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
-
 	if (ip_can_use_hint(skb, iph, hint)) {
 		err = ip_route_use_hint(skb, iph->daddr, iph->saddr, iph->tos,
 					dev, hint);
@@ -363,7 +362,7 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
 	 */
 	if (!skb_valid_dst(skb)) {
 		err = ip_route_input_noref(skb, iph->daddr, iph->saddr,
-					   iph->tos, dev);
+					   iph->tos, dev, &drop_reason);
 		if (unlikely(err))
 			goto drop_error;
 	} else {
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 723ac9181558..f1767e0cc9d9 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2465,7 +2465,8 @@ static int ip_route_input_rcu(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 }
 
 int ip_route_input_noref(struct sk_buff *skb, __be32 daddr, __be32 saddr,
-			 u8 tos, struct net_device *dev)
+			 u8 tos, struct net_device *dev,
+			 enum skb_drop_reason *reason)
 {
 	struct fib_result res;
 	int err;
diff --git a/net/ipv4/xfrm4_input.c b/net/ipv4/xfrm4_input.c
index a620618cc568..14990cc30c68 100644
--- a/net/ipv4/xfrm4_input.c
+++ b/net/ipv4/xfrm4_input.c
@@ -33,7 +33,7 @@ static inline int xfrm4_rcv_encap_finish(struct net *net, struct sock *sk,
 		const struct iphdr *iph = ip_hdr(skb);
 
 		if (ip_route_input_noref(skb, iph->daddr, iph->saddr,
-					 iph->tos, skb->dev))
+					 iph->tos, skb->dev, NULL))
 			goto drop;
 	}
 
diff --git a/net/ipv4/xfrm4_protocol.c b/net/ipv4/xfrm4_protocol.c
index b146ce88c5d0..9678ff876169 100644
--- a/net/ipv4/xfrm4_protocol.c
+++ b/net/ipv4/xfrm4_protocol.c
@@ -76,7 +76,7 @@ int xfrm4_rcv_encap(struct sk_buff *skb, int nexthdr, __be32 spi,
 		const struct iphdr *iph = ip_hdr(skb);
 
 		if (ip_route_input_noref(skb, iph->daddr, iph->saddr,
-					 iph->tos, skb->dev))
+					 iph->tos, skb->dev, NULL))
 			goto drop;
 	}
 
-- 
2.39.5

[PATCH net-next 2/7] net: ip: add drop reason to ip_route_input_rcu()

[Menglong Dong]

Add the pointer of the skb drop reason to the function arguments of
ip_route_input_rcu().

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
 net/ipv4/route.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index f1767e0cc9d9..385efe6d71a7 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2415,7 +2415,8 @@ out:	return err;
 
 /* called with rcu_read_lock held */
 static int ip_route_input_rcu(struct sk_buff *skb, __be32 daddr, __be32 saddr,
-			      u8 tos, struct net_device *dev, struct fib_result *res)
+			      u8 tos, struct net_device *dev, struct fib_result *res,
+			      enum skb_drop_reason *reason)
 {
 	/* Multicast recognition logic is moved from route cache to here.
 	 * The problem was that too many Ethernet cards have broken/missing
@@ -2473,7 +2474,7 @@ int ip_route_input_noref(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 
 	tos &= INET_DSCP_MASK;
 	rcu_read_lock();
-	err = ip_route_input_rcu(skb, daddr, saddr, tos, dev, &res);
+	err = ip_route_input_rcu(skb, daddr, saddr, tos, dev, &res, reason);
 	rcu_read_unlock();
 
 	return err;
@@ -3288,7 +3289,7 @@ static int inet_rtm_getroute(struct sk_buff *in_skb, struct nlmsghdr *nlh,
 		skb->mark	= mark;
 		err = ip_route_input_rcu(skb, dst, src,
 					 rtm->rtm_tos & INET_DSCP_MASK, dev,
-					 &res);
+					 &res, NULL);
 
 		rt = skb_rtable(skb);
 		if (err == 0 && rt->dst.error)
-- 
2.39.5

[PATCH net-next 3/7] net: ip: add drop reason to ip_route_input_slow()

[Menglong Dong]

In this commit, we make ip_route_input_slow() support skb drop reason by
adding the pointer of drop reason to its functions aeguments.

Following new skb drop reasons are added:

  SKB_DROP_REASON_IP_LOCAL_SOURCE
  SKB_DROP_REASON_IP_INVALID_SOURCE
  SKB_DROP_REASON_IP_INVALID_DEST
  SKB_DROP_REASON_IP_LOCALNET

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
 include/net/dropreason-core.h | 19 +++++++++++++++++++
 net/ipv4/route.c              | 32 ++++++++++++++++++++++++--------
 2 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 4748680e8c88..3d1b09f70bbd 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -76,6 +76,10 @@
 	FN(INVALID_PROTO)		\
 	FN(IP_INADDRERRORS)		\
 	FN(IP_INNOROUTES)		\
+	FN(IP_LOCAL_SOURCE)		\
+	FN(IP_INVALID_SOURCE)		\
+	FN(IP_INVALID_DEST)		\
+	FN(IP_LOCALNET)			\
 	FN(PKT_TOO_BIG)			\
 	FN(DUP_FRAG)			\
 	FN(FRAG_REASM_TIMEOUT)		\
@@ -365,6 +369,21 @@ enum skb_drop_reason {
 	 * IPSTATS_MIB_INADDRERRORS
 	 */
 	SKB_DROP_REASON_IP_INNOROUTES,
+	/** @SKB_DROP_REASON_IP_LOCAL_SOURCE: the source ip is local */
+	SKB_DROP_REASON_IP_LOCAL_SOURCE,
+	/**
+	 * @SKB_DROP_REASON_IP_INVALID_SOURCE: the source ip is invalid:
+	 * 1) source ip is multicast or limited broadcast
+	 * 2) source ip is zero and not IGMP
+	 */
+	SKB_DROP_REASON_IP_INVALID_SOURCE,
+	/**
+	 * @SKB_DROP_REASON_IP_INVALID_DEST: the dest ip is invalid:
+	 * 1) dest ip is 0
+	 */
+	SKB_DROP_REASON_IP_INVALID_DEST,
+	/** @SKB_DROP_REASON_IP_LOCALNET: source or dest ip is local net */
+	SKB_DROP_REASON_IP_LOCALNET,
 	/**
 	 * @SKB_DROP_REASON_PKT_TOO_BIG: packet size is too big (maybe exceed the
 	 * MTU)
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 385efe6d71a7..ab70917c62e5 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2202,8 +2202,10 @@ static struct net_device *ip_rt_get_dev(struct net *net,
 
 static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 			       u8 tos, struct net_device *dev,
-			       struct fib_result *res)
+			       struct fib_result *res,
+			       enum skb_drop_reason *reason)
 {
+	enum skb_drop_reason __reason = SKB_DROP_REASON_NOT_SPECIFIED;
 	struct in_device *in_dev = __in_dev_get_rcu(dev);
 	struct flow_keys *flkeys = NULL, _flkeys;
 	struct net    *net = dev_net(dev);
@@ -2231,8 +2233,10 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 		fl4.flowi4_tun_key.tun_id = 0;
 	skb_dst_drop(skb);
 
-	if (ipv4_is_multicast(saddr) || ipv4_is_lbcast(saddr))
+	if (ipv4_is_multicast(saddr) || ipv4_is_lbcast(saddr)) {
+		__reason = SKB_DROP_REASON_IP_INVALID_SOURCE;
 		goto martian_source;
+	}
 
 	res->fi = NULL;
 	res->table = NULL;
@@ -2242,21 +2246,29 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 	/* Accept zero addresses only to limited broadcast;
 	 * I even do not know to fix it or not. Waiting for complains :-)
 	 */
-	if (ipv4_is_zeronet(saddr))
+	if (ipv4_is_zeronet(saddr)) {
+		__reason = SKB_DROP_REASON_IP_INVALID_SOURCE;
 		goto martian_source;
+	}
 
-	if (ipv4_is_zeronet(daddr))
+	if (ipv4_is_zeronet(daddr)) {
+		__reason = SKB_DROP_REASON_IP_INVALID_DEST;
 		goto martian_destination;
+	}
 
 	/* Following code try to avoid calling IN_DEV_NET_ROUTE_LOCALNET(),
 	 * and call it once if daddr or/and saddr are loopback addresses
 	 */
 	if (ipv4_is_loopback(daddr)) {
-		if (!IN_DEV_NET_ROUTE_LOCALNET(in_dev, net))
+		if (!IN_DEV_NET_ROUTE_LOCALNET(in_dev, net)) {
+			__reason = SKB_DROP_REASON_IP_LOCALNET;
 			goto martian_destination;
+		}
 	} else if (ipv4_is_loopback(saddr)) {
-		if (!IN_DEV_NET_ROUTE_LOCALNET(in_dev, net))
+		if (!IN_DEV_NET_ROUTE_LOCALNET(in_dev, net)) {
+			__reason = SKB_DROP_REASON_IP_LOCALNET;
 			goto martian_source;
+		}
 	}
 
 	/*
@@ -2315,7 +2327,10 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 
 make_route:
 	err = ip_mkroute_input(skb, res, in_dev, daddr, saddr, tos, flkeys);
-out:	return err;
+out:
+	if (reason && err)
+		*reason = __reason;
+	return err;
 
 brd_input:
 	if (skb->protocol != htons(ETH_P_IP))
@@ -2406,6 +2421,7 @@ out:	return err;
 
 e_nobufs:
 	err = -ENOBUFS;
+	__reason = SKB_DROP_REASON_NOMEM;
 	goto out;
 
 martian_source:
@@ -2462,7 +2478,7 @@ static int ip_route_input_rcu(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 		return err;
 	}
 
-	return ip_route_input_slow(skb, daddr, saddr, tos, dev, res);
+	return ip_route_input_slow(skb, daddr, saddr, tos, dev, res, reason);
 }
 
 int ip_route_input_noref(struct sk_buff *skb, __be32 daddr, __be32 saddr,
-- 
2.39.5

[PATCH net-next 4/7] net: ip: make fib_validate_source() return drop reason

[Menglong Dong]

In this commit, we make fib_validate_source/__fib_validate_source return
-reason instead of errno on error. As the return value of them can be
-errno, 0, and 1, we can't make it return enum skb_drop_reason directly.

In the origin logic, if __fib_validate_source() return -EXDEV,
LINUX_MIB_IPRPFILTER will be counted. And now, we need to adjust it by
checking "reason == SKB_DROP_REASON_IP_RPFILTER".

We set the errno to -EINVAL when fib_validate_source() is called and the
validation fails, as the errno can be checked in the caller and now its
value is -reason, which can lead misunderstand.

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
 net/ipv4/fib_frontend.c | 19 +++++++++++++------
 net/ipv4/ip_input.c     |  4 +---
 net/ipv4/route.c        | 17 +++++++++++++----
 3 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
index 793e6781399a..779c90de3a54 100644
--- a/net/ipv4/fib_frontend.c
+++ b/net/ipv4/fib_frontend.c
@@ -346,6 +346,7 @@ static int __fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
 				 int rpf, struct in_device *idev, u32 *itag)
 {
 	struct net *net = dev_net(dev);
+	enum skb_drop_reason reason;
 	struct flow_keys flkeys;
 	int ret, no_addr;
 	struct fib_result res;
@@ -377,9 +378,15 @@ static int __fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
 
 	if (fib_lookup(net, &fl4, &res, 0))
 		goto last_resort;
-	if (res.type != RTN_UNICAST &&
-	    (res.type != RTN_LOCAL || !IN_DEV_ACCEPT_LOCAL(idev)))
-		goto e_inval;
+	if (res.type != RTN_UNICAST) {
+		if (res.type != RTN_LOCAL) {
+			reason = SKB_DROP_REASON_IP_INVALID_SOURCE;
+			goto e_inval;
+		} else if (!IN_DEV_ACCEPT_LOCAL(idev)) {
+			reason = SKB_DROP_REASON_IP_LOCAL_SOURCE;
+			goto e_inval;
+		}
+	}
 	fib_combine_itag(itag, &res);
 
 	dev_match = fib_info_nh_uses_dev(res.fi, dev);
@@ -412,9 +419,9 @@ static int __fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
 	return 0;
 
 e_inval:
-	return -EINVAL;
+	return -reason;
 e_rpf:
-	return -EXDEV;
+	return -SKB_DROP_REASON_IP_RPFILTER;
 }
 
 /* Ignore rp_filter for packets protected by IPsec. */
@@ -440,7 +447,7 @@ int fib_validate_source(struct sk_buff *skb, __be32 src, __be32 dst,
 		 * and the same host but different containers are not.
 		 */
 		if (inet_lookup_ifaddr_rcu(net, src))
-			return -EINVAL;
+			return -SKB_DROP_REASON_IP_LOCAL_SOURCE;
 
 ok:
 		*itag = 0;
diff --git a/net/ipv4/ip_input.c b/net/ipv4/ip_input.c
index dc062ae49137..aac0575bb1a4 100644
--- a/net/ipv4/ip_input.c
+++ b/net/ipv4/ip_input.c
@@ -424,10 +424,8 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
 	return NET_RX_DROP;
 
 drop_error:
-	if (err == -EXDEV) {
-		drop_reason = SKB_DROP_REASON_IP_RPFILTER;
+	if (drop_reason == SKB_DROP_REASON_IP_RPFILTER)
 		__NET_INC_STATS(net, LINUX_MIB_IPRPFILTER);
-	}
 	goto drop;
 }
 
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index ab70917c62e5..9de85051463b 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1690,7 +1690,7 @@ int ip_mc_validate_source(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 		err = fib_validate_source(skb, saddr, 0, tos, 0, dev,
 					  in_dev, itag);
 		if (err < 0)
-			return err;
+			return -EINVAL;
 	}
 	return 0;
 }
@@ -1788,6 +1788,7 @@ static int __mkroute_input(struct sk_buff *skb,
 	err = fib_validate_source(skb, saddr, daddr, tos, FIB_RES_OIF(*res),
 				  in_dev->dev, in_dev, &itag);
 	if (err < 0) {
+		err = -EINVAL;
 		ip_handle_martian_source(in_dev->dev, in_dev, skb, daddr,
 					 saddr);
 
@@ -2162,8 +2163,10 @@ int ip_route_use_hint(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 
 	tos &= INET_DSCP_MASK;
 	err = fib_validate_source(skb, saddr, daddr, tos, 0, dev, in_dev, &tag);
-	if (err < 0)
+	if (err < 0) {
+		err = -EINVAL;
 		goto martian_source;
+	}
 
 skip_validate_source:
 	skb_dst_copy(skb, hint);
@@ -2313,8 +2316,11 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 	if (res->type == RTN_LOCAL) {
 		err = fib_validate_source(skb, saddr, daddr, tos,
 					  0, dev, in_dev, &itag);
-		if (err < 0)
+		if (err < 0) {
+			__reason = -err;
+			err = -EINVAL;
 			goto martian_source;
+		}
 		goto local_input;
 	}
 
@@ -2339,8 +2345,11 @@ static int ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 	if (!ipv4_is_zeronet(saddr)) {
 		err = fib_validate_source(skb, saddr, 0, tos, 0, dev,
 					  in_dev, &itag);
-		if (err < 0)
+		if (err < 0) {
+			__reason = -err;
+			err = -EINVAL;
 			goto martian_source;
+		}
 	}
 	flags |= RTCF_BROADCAST;
 	res->type = RTN_BROADCAST;
-- 
2.39.5

[PATCH net-next 5/7] net: ip: make ip_route_input_mc() return drop reason

[Menglong Dong]

Make ip_route_input_mc() return drop reason, and adjust the call of it
in ip_route_input_rcu().

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
 net/ipv4/route.c | 23 +++++++++++++----------
 1 file changed, 13 insertions(+), 10 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 9de85051463b..f577012985c5 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1696,8 +1696,9 @@ int ip_mc_validate_source(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 }
 
 /* called in rcu_read_lock() section */
-static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
-			     u8 tos, struct net_device *dev, int our)
+static enum skb_drop_reason
+ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
+		  u8 tos, struct net_device *dev, int our)
 {
 	struct in_device *in_dev = __in_dev_get_rcu(dev);
 	unsigned int flags = RTCF_MULTICAST;
@@ -1707,7 +1708,7 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 
 	err = ip_mc_validate_source(skb, daddr, saddr, tos, dev, in_dev, &itag);
 	if (err)
-		return err;
+		return SKB_DROP_REASON_NOT_SPECIFIED;
 
 	if (our)
 		flags |= RTCF_LOCAL;
@@ -1718,7 +1719,7 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 	rth = rt_dst_alloc(dev_net(dev)->loopback_dev, flags, RTN_MULTICAST,
 			   false);
 	if (!rth)
-		return -ENOBUFS;
+		return SKB_DROP_REASON_NOMEM;
 
 #ifdef CONFIG_IP_ROUTE_CLASSID
 	rth->dst.tclassid = itag;
@@ -1734,7 +1735,7 @@ static int ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 
 	skb_dst_drop(skb);
 	skb_dst_set(skb, &rth->dst);
-	return 0;
+	return SKB_NOT_DROPPED_YET;
 }
 
 
@@ -2455,12 +2456,12 @@ static int ip_route_input_rcu(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 	 * route cache entry is created eventually.
 	 */
 	if (ipv4_is_multicast(daddr)) {
+		enum skb_drop_reason __reason = SKB_DROP_REASON_NOT_SPECIFIED;
 		struct in_device *in_dev = __in_dev_get_rcu(dev);
 		int our = 0;
-		int err = -EINVAL;
 
 		if (!in_dev)
-			return err;
+			return -EINVAL;
 		our = ip_check_mc_rcu(in_dev, daddr, saddr,
 				      ip_hdr(skb)->protocol);
 
@@ -2481,10 +2482,12 @@ static int ip_route_input_rcu(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 		     IN_DEV_MFORWARD(in_dev))
 #endif
 		   ) {
-			err = ip_route_input_mc(skb, daddr, saddr,
-						tos, dev, our);
+			__reason = ip_route_input_mc(skb, daddr, saddr,
+						     tos, dev, our);
 		}
-		return err;
+		if (reason && __reason)
+			*reason = __reason;
+		return __reason ? -EINVAL : 0;
 	}
 
 	return ip_route_input_slow(skb, daddr, saddr, tos, dev, res, reason);
-- 
2.39.5

[PATCH net-next 6/7] net: ip: make ip_mc_validate_source() return drop reason

[Menglong Dong]

Make ip_mc_validate_source() return drop reason, and adjust the call of
it in ip_route_input_mc().

Another caller of it is ip_rcv_finish_core->udp_v4_early_demux, and the
errno is not checked in detail, so we don't do more adjustment for it.

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
 include/net/route.h |  7 ++++---
 net/ipv4/route.c    | 33 ++++++++++++++++++---------------
 2 files changed, 22 insertions(+), 18 deletions(-)

diff --git a/include/net/route.h b/include/net/route.h
index cb9f31080517..cd0f585dacf0 100644
--- a/include/net/route.h
+++ b/include/net/route.h
@@ -198,9 +198,10 @@ static inline struct rtable *ip_route_output_gre(struct net *net, struct flowi4
 	fl4->fl4_gre_key = gre_key;
 	return ip_route_output_key(net, fl4);
 }
-int ip_mc_validate_source(struct sk_buff *skb, __be32 daddr, __be32 saddr,
-			  u8 tos, struct net_device *dev,
-			  struct in_device *in_dev, u32 *itag);
+enum skb_drop_reason
+ip_mc_validate_source(struct sk_buff *skb, __be32 daddr, __be32 saddr,
+		      u8 tos, struct net_device *dev,
+		      struct in_device *in_dev, u32 *itag);
 int ip_route_input_noref(struct sk_buff *skb, __be32 dst, __be32 src,
 			 u8 tos, struct net_device *devin,
 			 enum skb_drop_reason *reason);
diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index f577012985c5..89f97637af20 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1665,34 +1665,37 @@ struct rtable *rt_dst_clone(struct net_device *dev, struct rtable *rt)
 EXPORT_SYMBOL(rt_dst_clone);
 
 /* called in rcu_read_lock() section */
-int ip_mc_validate_source(struct sk_buff *skb, __be32 daddr, __be32 saddr,
-			  u8 tos, struct net_device *dev,
-			  struct in_device *in_dev, u32 *itag)
+enum skb_drop_reason
+ip_mc_validate_source(struct sk_buff *skb, __be32 daddr, __be32 saddr,
+		      u8 tos, struct net_device *dev,
+		      struct in_device *in_dev, u32 *itag)
 {
 	int err;
 
 	/* Primary sanity checks. */
 	if (!in_dev)
-		return -EINVAL;
+		return SKB_DROP_REASON_NOT_SPECIFIED;
 
-	if (ipv4_is_multicast(saddr) || ipv4_is_lbcast(saddr) ||
-	    skb->protocol != htons(ETH_P_IP))
-		return -EINVAL;
+	if (ipv4_is_multicast(saddr) || ipv4_is_lbcast(saddr))
+		return SKB_DROP_REASON_IP_INVALID_SOURCE;
+
+	if (skb->protocol != htons(ETH_P_IP))
+		return SKB_DROP_REASON_INVALID_PROTO;
 
 	if (ipv4_is_loopback(saddr) && !IN_DEV_ROUTE_LOCALNET(in_dev))
-		return -EINVAL;
+		return SKB_DROP_REASON_IP_LOCALNET;
 
 	if (ipv4_is_zeronet(saddr)) {
 		if (!ipv4_is_local_multicast(daddr) &&
 		    ip_hdr(skb)->protocol != IPPROTO_IGMP)
-			return -EINVAL;
+			return SKB_DROP_REASON_IP_INVALID_SOURCE;
 	} else {
 		err = fib_validate_source(skb, saddr, 0, tos, 0, dev,
 					  in_dev, itag);
 		if (err < 0)
-			return -EINVAL;
+			return -err;
 	}
-	return 0;
+	return SKB_NOT_DROPPED_YET;
 }
 
 /* called in rcu_read_lock() section */
@@ -1702,13 +1705,13 @@ ip_route_input_mc(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 {
 	struct in_device *in_dev = __in_dev_get_rcu(dev);
 	unsigned int flags = RTCF_MULTICAST;
+	enum skb_drop_reason reason;
 	struct rtable *rth;
 	u32 itag = 0;
-	int err;
 
-	err = ip_mc_validate_source(skb, daddr, saddr, tos, dev, in_dev, &itag);
-	if (err)
-		return SKB_DROP_REASON_NOT_SPECIFIED;
+	reason = ip_mc_validate_source(skb, daddr, saddr, tos, dev, in_dev, &itag);
+	if (reason)
+		return reason;
 
 	if (our)
 		flags |= RTCF_LOCAL;
-- 
2.39.5

[PATCH net-next 7/7] net: ip: fix typo in the doc of SKB_DROP_REASON_IP_INNOROUTES

[Menglong Dong]

This is a copy error, and SKB_DROP_REASON_IP_INNOROUTES should correspond
to IPSTATS_MIB_INADDRERRORS in the comment. Just fix it

Signed-off-by: Menglong Dong <dongml2@chinatelecom.cn>
---
 include/net/dropreason-core.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/net/dropreason-core.h b/include/net/dropreason-core.h
index 3d1b09f70bbd..a68235240f6a 100644
--- a/include/net/dropreason-core.h
+++ b/include/net/dropreason-core.h
@@ -366,7 +366,7 @@ enum skb_drop_reason {
 	SKB_DROP_REASON_IP_INADDRERRORS,
 	/**
 	 * @SKB_DROP_REASON_IP_INNOROUTES: network unreachable, corresponding to
-	 * IPSTATS_MIB_INADDRERRORS
+	 * IPSTATS_MIB_INNOROUTES
 	 */
 	SKB_DROP_REASON_IP_INNOROUTES,
 	/** @SKB_DROP_REASON_IP_LOCAL_SOURCE: the source ip is local */
-- 
2.39.5

Re: [PATCH net-next 1/7] net: ip: add drop reason to ip_route_input_noref()

[Jakub Kicinski]

no longer applies, please respin

On Tue,  1 Oct 2024 13:59:59 +0800 Menglong Dong wrote:
> +	enum skb_drop_reason drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
>  	const struct iphdr *iph = ip_hdr(skb);
> -	int err, drop_reason;
> +	int err;
>  	struct rtable *rt;

reverse xmas tree

>  
> -	drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
> -
>  	if (ip_can_use_hint(skb, iph, hint)) {
>  		err = ip_route_use_hint(skb, iph->daddr, iph->saddr, iph->tos,
>  					dev, hint);
> @@ -363,7 +362,7 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
>  	 */
>  	if (!skb_valid_dst(skb)) {
>  		err = ip_route_input_noref(skb, iph->daddr, iph->saddr,
> -					   iph->tos, dev);
> +					   iph->tos, dev, &drop_reason);

I find the extra output argument quite ugly.
I can't apply this now to try to suggest something better, perhaps you
can come up with a better solution..
-- 
pw-bot: cr

Re: [PATCH net-next 1/7] net: ip: add drop reason to ip_route_input_noref()

[Eric Dumazet]

On Fri, Oct 4, 2024 at 6:36 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> no longer applies, please respin
>
> On Tue,  1 Oct 2024 13:59:59 +0800 Menglong Dong wrote:
> > +     enum skb_drop_reason drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
> >       const struct iphdr *iph = ip_hdr(skb);
> > -     int err, drop_reason;
> > +     int err;
> >       struct rtable *rt;
>
> reverse xmas tree
>
> >
> > -     drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
> > -
> >       if (ip_can_use_hint(skb, iph, hint)) {
> >               err = ip_route_use_hint(skb, iph->daddr, iph->saddr, iph->tos,
> >                                       dev, hint);
> > @@ -363,7 +362,7 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
> >        */
> >       if (!skb_valid_dst(skb)) {
> >               err = ip_route_input_noref(skb, iph->daddr, iph->saddr,
> > -                                        iph->tos, dev);
> > +                                        iph->tos, dev, &drop_reason);
>
> I find the extra output argument quite ugly.
> I can't apply this now to try to suggest something better, perhaps you
> can come up with a better solution..

Also, passing a local variable by address forces the compiler to emit
more canary checks in more
networking core functions.


See :


config STACKPROTECTOR_STRONG
bool "Strong Stack Protector"
depends on STACKPROTECTOR
depends on $(cc-option,-fstack-protector-strong)
default y
help
  Functions will have the stack-protector canary logic added in any
  of the following conditions:

  - local variable's address used as part of the right hand side of an
    assignment or function argument
  - local variable is an array (or union containing an array),
    regardless of array type or length
  - uses register local variables

  This feature requires gcc version 4.9 or above, or a distribution
  gcc with the feature backported ("-fstack-protector-strong").

  On an x86 "defconfig" build, this feature adds canary checks to
  about 20% of all kernel functions, which increases the kernel code
  size by about 2%.

Re: [PATCH net-next 1/7] net: ip: add drop reason to ip_route_input_noref()

[Menglong Dong]

On Sat, Oct 5, 2024 at 12:54 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Fri, Oct 4, 2024 at 6:36 PM Jakub Kicinski <kuba@kernel.org> wrote:
> >
> > no longer applies, please respin
> >
> > On Tue,  1 Oct 2024 13:59:59 +0800 Menglong Dong wrote:
> > > +     enum skb_drop_reason drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
> > >       const struct iphdr *iph = ip_hdr(skb);
> > > -     int err, drop_reason;
> > > +     int err;
> > >       struct rtable *rt;
> >
> > reverse xmas tree
> >
> > >
> > > -     drop_reason = SKB_DROP_REASON_NOT_SPECIFIED;
> > > -
> > >       if (ip_can_use_hint(skb, iph, hint)) {
> > >               err = ip_route_use_hint(skb, iph->daddr, iph->saddr, iph->tos,
> > >                                       dev, hint);
> > > @@ -363,7 +362,7 @@ static int ip_rcv_finish_core(struct net *net, struct sock *sk,
> > >        */
> > >       if (!skb_valid_dst(skb)) {
> > >               err = ip_route_input_noref(skb, iph->daddr, iph->saddr,
> > > -                                        iph->tos, dev);
> > > +                                        iph->tos, dev, &drop_reason);
> >
> > I find the extra output argument quite ugly.
> > I can't apply this now to try to suggest something better, perhaps you
> > can come up with a better solution..
>
> Also, passing a local variable by address forces the compiler to emit
> more canary checks in more
> networking core functions.
>

Yeah, passing the address of the drop reasons to a function
looks not nice. The first glance for me is to make
ip_route_input_noref() return drop reasons, but I'm afraid that
the errno it returns is used by the caller.

Let me dig it deeper, and make the functions in this series
return drop reasons, instead of passing a local variable.

Thanks!
Menglong Dong


>
> See :
>
>
> config STACKPROTECTOR_STRONG
> bool "Strong Stack Protector"
> depends on STACKPROTECTOR
> depends on $(cc-option,-fstack-protector-strong)
> default y
> help
>   Functions will have the stack-protector canary logic added in any
>   of the following conditions:
>
>   - local variable's address used as part of the right hand side of an
>     assignment or function argument
>   - local variable is an array (or union containing an array),
>     regardless of array type or length
>   - uses register local variables
>
>   This feature requires gcc version 4.9 or above, or a distribution
>   gcc with the feature backported ("-fstack-protector-strong").
>
>   On an x86 "defconfig" build, this feature adds canary checks to
>   about 20% of all kernel functions, which increases the kernel code
>   size by about 2%.