[net-next] net: icmp: use input address in traceroute

Even with icmp_errors_use_inbound_ifaddr set, traceroute returns the
primary address of the interface the packet was received on, even if
the path goes through a secondary address. In the example:

                    1.0.3.1/24
 ---- 1.0.1.3/24    1.0.1.1/24 ---- 1.0.2.1/24    1.0.2.4/24 ----
 |H1|--------------------------|R1|--------------------------|H2|
 ----            N1            ----            N2            ----

where 1.0.3.1/24 is R1's primary address on N1, traceroute from
H1 to H2 returns:

traceroute to 1.0.2.4 (1.0.2.4), 30 hops max, 60 byte packets
 1  1.0.3.1 (1.0.3.1)  0.018 ms  0.006 ms  0.006 ms
 2  1.0.2.4 (1.0.2.4)  0.021 ms  0.007 ms  0.007 ms

After applying this patch, it returns:

traceroute to 1.0.2.4 (1.0.2.4), 30 hops max, 60 byte packets
 1  1.0.1.1 (1.0.1.1)  0.033 ms  0.007 ms  0.006 ms
 2  1.0.2.4 (1.0.2.4)  0.011 ms  0.007 ms  0.007 ms

Original-patch-by: Bill Fenner <fenner@arista.com>
Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>

---
 net/ipv4/icmp.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

On 10/31/19 6:44 PM, Francesco Ruggeri wrote:
> Even with icmp_errors_use_inbound_ifaddr set, traceroute returns the
> primary address of the interface the packet was received on, even if
> the path goes through a secondary address. In the example:
> 
>                     1.0.3.1/24
>  ---- 1.0.1.3/24    1.0.1.1/24 ---- 1.0.2.1/24    1.0.2.4/24 ----
>  |H1|--------------------------|R1|--------------------------|H2|
>  ----            N1            ----            N2            ----
> 
> where 1.0.3.1/24 is R1's primary address on N1, traceroute from
> H1 to H2 returns:
> 
> traceroute to 1.0.2.4 (1.0.2.4), 30 hops max, 60 byte packets
>  1  1.0.3.1 (1.0.3.1)  0.018 ms  0.006 ms  0.006 ms
>  2  1.0.2.4 (1.0.2.4)  0.021 ms  0.007 ms  0.007 ms
> 
> After applying this patch, it returns:
> 
> traceroute to 1.0.2.4 (1.0.2.4), 30 hops max, 60 byte packets
>  1  1.0.1.1 (1.0.1.1)  0.033 ms  0.007 ms  0.006 ms
>  2  1.0.2.4 (1.0.2.4)  0.011 ms  0.007 ms  0.007 ms
> 
> Original-patch-by: Bill Fenner <fenner@arista.com>
> Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>
> 
> ---
>  net/ipv4/icmp.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index 4298aae74e0e..a72fbdf1fb85 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -682,7 +682,8 @@ void __icmp_send(struct sk_buff *skb_in, int type, int code, __be32 info,
>  			dev = dev_get_by_index_rcu(net, inet_iif(skb_in));
>  
>  		if (dev)
> -			saddr = inet_select_addr(dev, 0, RT_SCOPE_LINK);
> +			saddr = inet_select_addr(dev, iph->saddr,
> +						 RT_SCOPE_LINK);
>  		else
>  			saddr = 0;
>  		rcu_read_unlock();
> 

Change looks good to me, so for that
Reviewed-by: David Ahern <dsahern@gmail.com>

In this case and your ipv6 patch you have a set of commands to show this
problem and verify the fix. Please submit both in a test script under
tools/testing/selftests/net/. Also, veth pairs is a better way to
connect namespaces than macvlan on a dummy device. See any of the fib*
tests in that directory. Those all serve as good templates for a
traceroute test script.

On Thu, Oct 31, 2019 at 8:11 PM David Ahern <dsahern@gmail.com> wrote:
>
> Change looks good to me, so for that
> Reviewed-by: David Ahern <dsahern@gmail.com>
>
> In this case and your ipv6 patch you have a set of commands to show this
> problem and verify the fix. Please submit both in a test script under
> tools/testing/selftests/net/. Also, veth pairs is a better way to
> connect namespaces than macvlan on a dummy device. See any of the fib*
> tests in that directory. Those all serve as good templates for a
> traceroute test script.
>

Sure, will do.
Thanks,
Francesco

From: fruggeri@arista.com (Francesco Ruggeri)
Date: Thu, 31 Oct 2019 17:44:13 -0700

> Even with icmp_errors_use_inbound_ifaddr set, traceroute returns the
> primary address of the interface the packet was received on, even if
> the path goes through a secondary address. In the example:
> 
>                     1.0.3.1/24
>  ---- 1.0.1.3/24    1.0.1.1/24 ---- 1.0.2.1/24    1.0.2.4/24 ----
>  |H1|--------------------------|R1|--------------------------|H2|
>  ----            N1            ----            N2            ----
> 
> where 1.0.3.1/24 is R1's primary address on N1, traceroute from
> H1 to H2 returns:
> 
> traceroute to 1.0.2.4 (1.0.2.4), 30 hops max, 60 byte packets
>  1  1.0.3.1 (1.0.3.1)  0.018 ms  0.006 ms  0.006 ms
>  2  1.0.2.4 (1.0.2.4)  0.021 ms  0.007 ms  0.007 ms
> 
> After applying this patch, it returns:
> 
> traceroute to 1.0.2.4 (1.0.2.4), 30 hops max, 60 byte packets
>  1  1.0.1.1 (1.0.1.1)  0.033 ms  0.007 ms  0.006 ms
>  2  1.0.2.4 (1.0.2.4)  0.011 ms  0.007 ms  0.007 ms
> 
> Original-patch-by: Bill Fenner <fenner@arista.com>
> Signed-off-by: Francesco Ruggeri <fruggeri@arista.com>

Applied.