On 8/11/20 1:50 PM, Mathieu Desnoyers wrote:
> As per RFC4443, the destination address field for ICMPv6 error messages
> is copied from the source address field of the invoking packet.
>
> In configurations with Virtual Routing and Forwarding tables, looking up
> which routing table to use for sending ICMPv6 error messages is
> currently done by using the destination net_device.
>
> If the source and destination interfaces are within separate VRFs, or
> one in the global routing table and the other in a VRF, looking up the
> source address of the invoking packet in the destination interface's
> routing table will fail if the destination interface's routing table
> contains no route to the invoking packet's source address.
>
> One observable effect of this issue is that traceroute6 does not work in
> the following cases:
>
> - Route leaking between global routing table and VRF
> - Route leaking between VRFs
>
> Preferably use the source device routing table when sending ICMPv6 error
> messages. If no source device is set, fall-back on the destination
> device routing table.
>
> Link: https://tools.ietf.org/html/rfc4443
> Signed-off-by: Mathieu Desnoyers <mathieu.desnoy...@efficios.com>
> Cc: David Ahern <dsah...@kernel.org>
> Cc: David S. Miller <da...@davemloft.net>
> Cc: netdev@vger.kernel.org
> ---
> net/ipv6/icmp.c | 15 +++++++++++++--
> net/ipv6/ip6_output.c | 2 --
> 2 files changed, 13 insertions(+), 4 deletions(-)
>
> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> index a4e4912ad607..a971b58b0371 100644
> --- a/net/ipv6/icmp.c
> +++ b/net/ipv6/icmp.c
> @@ -501,8 +501,19 @@ void icmp6_send(struct sk_buff *skb, u8 type, u8 code,
> __u32 info,
> if (__ipv6_addr_needs_scope_id(addr_type)) {
> iif = icmp6_iif(skb);
> } else {
> - dst = skb_dst(skb);
> - iif = l3mdev_master_ifindex(dst ? dst->dev : skb->dev);
> + struct net_device *route_lookup_dev = NULL;
> +
> + /*
> + * The device used for looking up which routing table to use is
> + * preferably the source whenever it is set, which should
> + * ensure the icmp error can be sent to the source host, else
> + * fallback on the destination device.
> + */
> + if (skb->dev)
> + route_lookup_dev = skb->dev;
top of icmp6_send there is a check that skb->dev is set.
> + else if (skb_dst(skb))
> + route_lookup_dev = skb_dst(skb)->dev;
> + iif = l3mdev_master_ifindex(route_lookup_dev);
> }
>
> /*
> diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
> index c78e67d7747f..cd623068de53 100644
> --- a/net/ipv6/ip6_output.c
> +++ b/net/ipv6/ip6_output.c
> @@ -468,8 +468,6 @@ int ip6_forward(struct sk_buff *skb)
> * check and decrement ttl
> */
> if (hdr->hop_limit <= 1) {
> - /* Force OUTPUT device used as source address */
> - skb->dev = dst->dev;
I *think* this ok. Not clear to me why the forward path would change the
skb->dev like that. Goes back to beginning of the git history.
> icmpv6_send(skb, ICMPV6_TIME_EXCEED, ICMPV6_EXC_HOPLIMIT, 0);
> __IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
>
>
