Re: [PATCH net-next 5/5] ipv6: Compute multipath hash for forwarded ICMP errors from offending packet

[Hannes Frederic Sowa]

Hello,

On 01.11.2016 17:39, Tom Herbert wrote:
> On Tue, Nov 1, 2016 at 9:25 AM, Hannes Frederic Sowa
> <hannes@xxxxxxxxxxxxxxxxxxx> wrote:
>> On 31.10.2016 20:25, Tom Herbert wrote:
>>> The normal hash for TCP or UDP using ECMP is over <protocol, srcIP,
>>> dstIP, srcPort, dstPort>. For an ICMP packet ECMP would most likely be
>>> done over <protocol, srcIP, dstIP>. There really is no way to ensure
>>> that an ICMP packet will follow the same path as TCP or any other
>>> protocol. Fortunately, this is really isn't so terrible. The Internet
>>> has worked this way ever since routers started using ports as input to
>>> ECMP and that hasn't caused any major meltdown.
>>
>> The normal hash for forwarding is without srcPort or dstPort, so the
>> same as ICMP and especially also because of fragmentation problematic I
>> don't think a lot of routers use L4 port information for ECMP either.
>>
> I don't think we can define a "normal hash". There is no requirement
> that routers do ECMP a certain way, or that they do ECMP, or that for
> that matter that they even consistently route packets for the same
> flow. All of this is optimization, not something we can rely on
> operationally. So in the general case, regardless of anything we do in
> the stack, either ICMP packets will follow the same path as the flow
> are they won't. If they don't things still need to to work. So I still
> don't see what material benefit this patch gives us.

There certainly is no standard ECMP hash algorithm. ;)

Even Linux IPv6 ECMP behaved like that for a long time (very bad). It
just routed put packets on different links without consulting any hash.
That exactly was the reason why it was unusable and was upgraded some
while ago.

I do remember a lot of IPv6 PMTU blackholes in the past, so every patch
that improves connectivity here seems valuable to me, even if it does
not fix the problem completely in the end.

Bye,
Hannes