[PATCH net,v2] vrf: Use orig netdev to count Ip6InNoRoutes and a fresh route lookup when sending dest unreach

[PATCH net,v2] vrf: Use orig netdev to count Ip6InNoRoutes and a fresh route lookup when sending dest unreach

[Stephen Suryaputra]

When there is no route to an IPv6 dest addr, skb_dst(skb) points
to loopback dev in the case of that the IP6CB(skb)->iif is
enslaved to a vrf. This causes Ip6InNoRoutes to be incremented on the
loopback dev. This also causes the lookup to fail on icmpv6_send() and
the dest unreachable to not sent and Ip6OutNoRoutes gets incremented on
the loopback dev.

To reproduce:
* Gateway configuration:
	ip link add dev vrf_258 type vrf table 258
	ip link set dev enp0s9 master vrf_258
	ip addr add 66:1/64 dev enp0s9
	ip -6 route add unreachable default metric 8192 table 258
	sysctl -w net.ipv6.conf.all.forwarding=1
	sysctl -w net.ipv6.conf.enp0s9.forwarding=1
* Sender configuration:
	ip addr add 66::2/64 dev enp0s9
	ip -6 route add default via 66::1
and ping 67::1 for example from the sender.

Fix this by counting on the original netdev and reset the skb dst to
force a fresh lookup.

v2: Fix typo of destination address in the repro steps.
Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
---
 net/ipv6/route.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index e8c73b7782cd..3b026a310b3e 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3772,23 +3772,33 @@ int ipv6_route_ioctl(struct net *net, unsigned int cmd, void __user *arg)
 
 static int ip6_pkt_drop(struct sk_buff *skb, u8 code, int ipstats_mib_noroutes)
 {
-	int type;
 	struct dst_entry *dst = skb_dst(skb);
+	struct inet6_dev *idev = ip6_dst_idev(dst);
+	struct net *net = dev_net(dst->dev);
+	int type;
+
+	if (netif_is_l3_master(skb->dev) &&
+	    idev == __in6_dev_get(net->loopback_dev)) {
+		idev = __in6_dev_get_safely(dev_get_by_index_rcu(net, IP6CB(skb)->iif));  
+	}
+
 	switch (ipstats_mib_noroutes) {
 	case IPSTATS_MIB_INNOROUTES:
 		type = ipv6_addr_type(&ipv6_hdr(skb)->daddr);
 		if (type == IPV6_ADDR_ANY) {
-			IP6_INC_STATS(dev_net(dst->dev),
-				      __in6_dev_get_safely(skb->dev),
-				      IPSTATS_MIB_INADDRERRORS);
+			IP6_INC_STATS(net, idev, IPSTATS_MIB_INADDRERRORS);
 			break;
 		}
 		/* FALLTHROUGH */
 	case IPSTATS_MIB_OUTNOROUTES:
-		IP6_INC_STATS(dev_net(dst->dev), ip6_dst_idev(dst),
-			      ipstats_mib_noroutes);
+		IP6_INC_STATS(net, idev, ipstats_mib_noroutes);
 		break;
 	}
+
+	/* Start over by dropping the dst for l3mdev case */
+	if (netif_is_l3_master(skb->dev)) 
+		skb_dst_drop(skb);
+
 	icmpv6_send(skb, ICMPV6_DEST_UNREACH, code, 0);
 	kfree_skb(skb);
 	return 0;
-- 
2.17.1

Re: [PATCH net,v2] vrf: Use orig netdev to count Ip6InNoRoutes and a fresh route lookup when sending dest unreach

[David Ahern]

On 4/24/19 12:54 PM, Stephen Suryaputra wrote:
> When there is no route to an IPv6 dest addr, skb_dst(skb) points
> to loopback dev in the case of that the IP6CB(skb)->iif is
> enslaved to a vrf. This causes Ip6InNoRoutes to be incremented on the
> loopback dev. This also causes the lookup to fail on icmpv6_send() and
> the dest unreachable to not sent and Ip6OutNoRoutes gets incremented on
> the loopback dev.

The root cause is the 'special' dst entries ipv6 has. I want to convert
those to generated ones like IPv4 has in which case the vrf device is
used over the loopback. The changes for nexthops move the code in the
right direction for this to happen. But that is a future change.

> 
> To reproduce:
> * Gateway configuration:
> 	ip link add dev vrf_258 type vrf table 258
> 	ip link set dev enp0s9 master vrf_258
> 	ip addr add 66:1/64 dev enp0s9
> 	ip -6 route add unreachable default metric 8192 table 258
> 	sysctl -w net.ipv6.conf.all.forwarding=1
> 	sysctl -w net.ipv6.conf.enp0s9.forwarding=1
> * Sender configuration:
> 	ip addr add 66::2/64 dev enp0s9
> 	ip -6 route add default via 66::1
> and ping 67::1 for example from the sender.

here's another self contained test case:

ip netns add foo
ip -netns foo li set lo up
ip -netns foo link add dev red up type vrf table 258
ip -6 -netns foo ro add vrf red unreachable default

ip link add veth1 type veth peer name veth2
ip link set veth2 netns foo
ip -netns foo li set veth2 vrf red up
ip li set veth1 up

ip -6 addr add 2001:d8:66::1/64 dev veth1
ip -6 -netns foo addr add 2001:d8:66::2/64 dev veth2
ip -6 ro add 2001:db8:67::64/128 via 2001:d8:66::2
ip netns exec foo sysctl -w net.ipv6.conf.veth2.forwarding=1
ping6 -c1 -w1 -I veth1 2001:db8:67::64
# fails  Destination unreachable: No route

ip -netns foo -6 ru add pref 66 to 2001:db8:67::64 prohibit
ping6 -c1 -w1 -I veth1 2001:db8:67::64
# Destination unreachable: Administratively prohibited

I am adding the above to my vrf tests.

> 
> Fix this by counting on the original netdev and reset the skb dst to
> force a fresh lookup.
> 
> v2: Fix typo of destination address in the repro steps.
> Signed-off-by: Stephen Suryaputra <ssuryaextr@gmail.com>
> ---
>  net/ipv6/route.c | 22 ++++++++++++++++------
>  1 file changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/net/ipv6/route.c b/net/ipv6/route.c
> index e8c73b7782cd..3b026a310b3e 100644
> --- a/net/ipv6/route.c
> +++ b/net/ipv6/route.c
> @@ -3772,23 +3772,33 @@ int ipv6_route_ioctl(struct net *net, unsigned int cmd, void __user *arg)
>  
>  static int ip6_pkt_drop(struct sk_buff *skb, u8 code, int ipstats_mib_noroutes)
>  {
> -	int type;
>  	struct dst_entry *dst = skb_dst(skb);
> +	struct inet6_dev *idev = ip6_dst_idev(dst);
> +	struct net *net = dev_net(dst->dev);
> +	int type;
> +
> +	if (netif_is_l3_master(skb->dev) &&
> +	    idev == __in6_dev_get(net->loopback_dev)) {

dst->dev == net->loopback_dev
should work as well.

> +		idev = __in6_dev_get_safely(dev_get_by_index_rcu(net, IP6CB(skb)->iif));  

theoretically this function is called for the output path (dst->output)
as well in which case iif is not set. But, I was not able to create a
test case that hit this function for it; local traffic is failing much
sooner (connect, sendmsg).

Reviewed-by: David Ahern <dsahern@gmail.com>
Tested-by: David Ahern <dsahern@gmail.com>

Re: [PATCH net,v2] vrf: Use orig netdev to count Ip6InNoRoutes and a fresh route lookup when sending dest unreach

[David Miller]

From: Stephen Suryaputra <ssuryaextr@gmail.com>
Date: Wed, 24 Apr 2019 14:54:44 -0400

> @@ -3772,23 +3772,33 @@ int ipv6_route_ioctl(struct net *net, unsigned int cmd, void __user *arg)
>  
>  static int ip6_pkt_drop(struct sk_buff *skb, u8 code, int ipstats_mib_noroutes)
>  {
> -	int type;
>  	struct dst_entry *dst = skb_dst(skb);
> +	struct inet6_dev *idev = ip6_dst_idev(dst);
> +	struct net *net = dev_net(dst->dev);
> +	int type;

Please reverse-christmas tree here.

Thank you.