| Message ID | 20200925200452.2080-1-mathieu.desnoyers@efficios.com |
|---|---|
| Headers | show |
| Series | l3mdev icmp error route lookup fixes | expand |
On 9/25/20 1:04 PM, Mathieu Desnoyers wrote: > Hi, > > Here is an updated series of fixes for ipv4 and ipv6 which which ensure > the route lookup is performed on the right routing table in VRF > configurations when sending TTL expired icmp errors (useful for > traceroute). > > It includes tests for both ipv4 and ipv6. > > These fixes address specifically address the code paths involved in > sending TTL expired icmp errors. As detailed in the individual commit > messages, those fixes do not address similar icmp errors related to > network namespaces and unreachable / fragmentation needed messages, > which appear to use different code paths. > > The main changes since the last round are updates to the selftests. > This looks fine to me. I noticed the IPv6 large packet test case is failing; the fib6 tracepoint is showing the loopback as the iif which is wrong: ping6 8488 [004] 502.015817: fib6:fib6_table_lookup: table 255 oif 0 iif 1 proto 58 ::/0 -> 2001:db8:16:1::1/0 tos 0 scope 0 flags 0 ==> dev lo gw :: err -113 I will dig into it later this week.
On 10/5/20 9:30 AM, David Ahern wrote: > On 9/25/20 1:04 PM, Mathieu Desnoyers wrote: >> Hi, >> >> Here is an updated series of fixes for ipv4 and ipv6 which which ensure >> the route lookup is performed on the right routing table in VRF >> configurations when sending TTL expired icmp errors (useful for >> traceroute). >> >> It includes tests for both ipv4 and ipv6. >> >> These fixes address specifically address the code paths involved in >> sending TTL expired icmp errors. As detailed in the individual commit >> messages, those fixes do not address similar icmp errors related to >> network namespaces and unreachable / fragmentation needed messages, >> which appear to use different code paths. >> >> The main changes since the last round are updates to the selftests. >> > > This looks fine to me. I noticed the IPv6 large packet test case is > failing; the fib6 tracepoint is showing the loopback as the iif which is > wrong: > > ping6 8488 [004] 502.015817: fib6:fib6_table_lookup: table 255 oif 0 > iif 1 proto 58 ::/0 -> 2001:db8:16:1::1/0 tos 0 scope 0 flags 0 ==> dev > lo gw :: err -113 > > I will dig into it later this week. > I see the problem here -- source address selection is picking ::1. I do not have a solution to the problem yet, but its resolution is independent of the change in this set so I think this one is good to go.
----- On Oct 11, 2020, at 7:56 PM, David Ahern dsahern@gmail.com wrote: > On 10/5/20 9:30 AM, David Ahern wrote: >> On 9/25/20 1:04 PM, Mathieu Desnoyers wrote: >>> Hi, >>> >>> Here is an updated series of fixes for ipv4 and ipv6 which which ensure >>> the route lookup is performed on the right routing table in VRF >>> configurations when sending TTL expired icmp errors (useful for >>> traceroute). >>> >>> It includes tests for both ipv4 and ipv6. >>> >>> These fixes address specifically address the code paths involved in >>> sending TTL expired icmp errors. As detailed in the individual commit >>> messages, those fixes do not address similar icmp errors related to >>> network namespaces and unreachable / fragmentation needed messages, >>> which appear to use different code paths. >>> >>> The main changes since the last round are updates to the selftests. >>> >> >> This looks fine to me. I noticed the IPv6 large packet test case is >> failing; the fib6 tracepoint is showing the loopback as the iif which is >> wrong: >> >> ping6 8488 [004] 502.015817: fib6:fib6_table_lookup: table 255 oif 0 >> iif 1 proto 58 ::/0 -> 2001:db8:16:1::1/0 tos 0 scope 0 flags 0 ==> dev >> lo gw :: err -113 >> >> I will dig into it later this week. >> > > I see the problem here -- source address selection is picking ::1. I do > not have a solution to the problem yet, but its resolution is > independent of the change in this set so I think this one is good to go. OK, do you want to pick up the RFC patch series, or should I re-send it without RFC tag ? Thanks, Mathieu
On 10/12/20 5:57 AM, Mathieu Desnoyers wrote: > OK, do you want to pick up the RFC patch series, or should I re-send it > without RFC tag ? you need to re-send for Dave or Jakub to pick them up via patchworks
----- On Oct 12, 2020, at 9:45 AM, David Ahern dsahern@gmail.com wrote: > On 10/12/20 5:57 AM, Mathieu Desnoyers wrote: >> OK, do you want to pick up the RFC patch series, or should I re-send it >> without RFC tag ? > > you need to re-send for Dave or Jakub to pick them up via patchworks OK. Can I have your Acked-by or Reviewed-by for all three patches ? Thanks, Mathieu
On 10/12/20 7:10 AM, Mathieu Desnoyers wrote: > ----- On Oct 12, 2020, at 9:45 AM, David Ahern dsahern@gmail.com wrote: > >> On 10/12/20 5:57 AM, Mathieu Desnoyers wrote: >>> OK, do you want to pick up the RFC patch series, or should I re-send it >>> without RFC tag ? >> >> you need to re-send for Dave or Jakub to pick them up via patchworks > > OK. Can I have your Acked-by or Reviewed-by for all three patches ? > sure. Reviewed-by: David Ahern <dsahern@gmail.com>
Hi, Here is an updated series of fixes for ipv4 and ipv6 which which ensure the route lookup is performed on the right routing table in VRF configurations when sending TTL expired icmp errors (useful for traceroute). It includes tests for both ipv4 and ipv6. These fixes address specifically address the code paths involved in sending TTL expired icmp errors. As detailed in the individual commit messages, those fixes do not address similar icmp errors related to network namespaces and unreachable / fragmentation needed messages, which appear to use different code paths. The main changes since the last round are updates to the selftests. Thanks, Mathieu Mathieu Desnoyers (2): ipv4/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2) ipv6/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2) Michael Jeanson (1): selftests: Add VRF route leaking tests net/ipv4/icmp.c | 23 +- net/ipv6/icmp.c | 7 +- net/ipv6/ip6_output.c | 2 - tools/testing/selftests/net/Makefile | 1 + .../selftests/net/vrf_route_leaking.sh | 626 ++++++++++++++++++ 5 files changed, 653 insertions(+), 6 deletions(-) create mode 100755 tools/testing/selftests/net/vrf_route_leaking.sh