Message ID | 20191101004414.2B2F995C102D@us180.sjc.aristanetworks.com |
---|---|
State | Accepted |
Delegated to: | David Miller |
Headers | show |
Series | [net-next] net: icmp: use input address in traceroute | expand |
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.
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();
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(-)