Re: [PATCH AUTOSEL 5.9 035/111] ipv6/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2)

2020-10-19 Thread Sasha Levin

On Mon, Oct 19, 2020 at 09:19:33AM -0400, Mathieu Desnoyers wrote:

- On Oct 18, 2020, at 9:40 PM, David Ahern dsah...@gmail.com wrote:


On 10/18/20 1:40 PM, Jakub Kicinski wrote:

This one got applied a few days ago, and the urgency is low so it may be
worth letting it see at least one -rc release ;)


agreed


Likewise, I agree there is no need to hurry. Letting those patches live through
a few -rc releases before picking them into stable is a wise course of action.


That's fair, and I've moved this patch into a different queue to make it
go out later.

--
Thanks,
Sasha


Re: [PATCH AUTOSEL 5.9 035/111] ipv6/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2)

2020-10-19 Thread Sasha Levin

On Mon, Oct 19, 2020 at 08:33:27AM -0700, Jakub Kicinski wrote:

On Mon, 19 Oct 2020 07:52:36 -0400 Sasha Levin wrote:

On Sun, Oct 18, 2020 at 07:40:12PM -0600, David Ahern wrote:
>On 10/18/20 1:40 PM, Jakub Kicinski wrote:
>> This one got applied a few days ago, and the urgency is low so it may be
>> worth letting it see at least one -rc release ;)
>
>agreed

Definitely - AUTOSEL patches get extra soaking time before getting
queued up. This is more of a request to make sure it's not doing
anything silly.


Could you put a number on "extra soaking time"?

I'm asking mostly out of curiosity :)


The AUTOSEL process adds at least another week into the flow, this means
that the fastest this patch will go in into a released kernel is about 2
weeks from now.

--
Thanks,
Sasha


Re: [PATCH AUTOSEL 5.9 035/111] ipv6/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2)

2020-10-19 Thread Jakub Kicinski
On Mon, 19 Oct 2020 07:52:36 -0400 Sasha Levin wrote:
> On Sun, Oct 18, 2020 at 07:40:12PM -0600, David Ahern wrote:
> >On 10/18/20 1:40 PM, Jakub Kicinski wrote:  
> >> This one got applied a few days ago, and the urgency is low so it may be
> >> worth letting it see at least one -rc release ;)  
> >
> >agreed  
> 
> Definitely - AUTOSEL patches get extra soaking time before getting
> queued up. This is more of a request to make sure it's not doing
> anything silly.

Could you put a number on "extra soaking time"? 

I'm asking mostly out of curiosity :)


Re: [PATCH AUTOSEL 5.9 035/111] ipv6/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2)

2020-10-19 Thread Mathieu Desnoyers
- On Oct 18, 2020, at 9:40 PM, David Ahern dsah...@gmail.com wrote:

> On 10/18/20 1:40 PM, Jakub Kicinski wrote:
>> This one got applied a few days ago, and the urgency is low so it may be
>> worth letting it see at least one -rc release ;)
> 
> agreed

Likewise, I agree there is no need to hurry. Letting those patches live through
a few -rc releases before picking them into stable is a wise course of action.

Thanks,

Mathieu


-- 
Mathieu Desnoyers
EfficiOS Inc.
http://www.efficios.com


Re: [PATCH AUTOSEL 5.9 035/111] ipv6/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2)

2020-10-19 Thread Sasha Levin

On Sun, Oct 18, 2020 at 07:40:12PM -0600, David Ahern wrote:

On 10/18/20 1:40 PM, Jakub Kicinski wrote:

This one got applied a few days ago, and the urgency is low so it may be
worth letting it see at least one -rc release ;)


agreed


Definitely - AUTOSEL patches get extra soaking time before getting
queued up. This is more of a request to make sure it's not doing
anything silly.

--
Thanks,
Sasha


Re: [PATCH AUTOSEL 5.9 035/111] ipv6/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2)

2020-10-18 Thread David Ahern
On 10/18/20 1:40 PM, Jakub Kicinski wrote:
> This one got applied a few days ago, and the urgency is low so it may be
> worth letting it see at least one -rc release ;)

agreed


[PATCH AUTOSEL 5.9 035/111] ipv6/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2)

2020-10-18 Thread Sasha Levin
From: Mathieu Desnoyers 

[ Upstream commit 272928d1cdacfc3b55f605cb0e9115832ecfb20c ]

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

Use the source device routing table when sending ICMPv6 error
messages.

[ In the context of ipv4, it has been pointed out that a similar issue
  may exist with ICMP errors triggered when forwarding between network
  namespaces. It would be worthwhile to investigate whether ipv6 has
  similar issues, but is outside of the scope of this investigation. ]

[ Testing shows that similar issues exist with ipv6 unreachable /
  fragmentation needed messages.  However, investigation of this
  additional failure mode is beyond this investigation's scope. ]

Link: https://tools.ietf.org/html/rfc4443
Signed-off-by: Mathieu Desnoyers 
Reviewed-by: David Ahern 
Signed-off-by: Jakub Kicinski 
Signed-off-by: Sasha Levin 
---
 net/ipv6/icmp.c   | 7 +--
 net/ipv6/ip6_output.c | 2 --
 2 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index a4e4912ad607b..91209a2760aa5 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -501,8 +501,11 @@ 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);
+   /*
+* The source device is used for looking up which routing table
+* to use for sending an ICMP error.
+*/
+   iif = l3mdev_master_ifindex(skb->dev);
}
 
/*
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index c78e67d7747fb..cd623068de536 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;
icmpv6_send(skb, ICMPV6_TIME_EXCEED, ICMPV6_EXC_HOPLIMIT, 0);
__IP6_INC_STATS(net, idev, IPSTATS_MIB_INHDRERRORS);
 
-- 
2.25.1



Re: [PATCH AUTOSEL 5.9 035/111] ipv6/icmp: l3mdev: Perform icmp error route lookup on source device routing table (v2)

2020-10-18 Thread Jakub Kicinski
On Sun, 18 Oct 2020 15:16:51 -0400 Sasha Levin wrote:
> From: Mathieu Desnoyers 
> 
> [ Upstream commit 272928d1cdacfc3b55f605cb0e9115832ecfb20c ]
> 
> 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.

This one got applied a few days ago, and the urgency is low so it may be
worth letting it see at least one -rc release ;)

Maybe Mathieu & David feel differently.