On Mon, Mar 28, 2016 at 6:54 PM, Jesse Gross [off-list ref] wrote:

On Mon, Mar 28, 2016 at 6:24 PM, Tom Herbert [off-list ref] wrote:

quoted

On Mon, Mar 28, 2016 at 4:58 PM, Alexander Duyck [off-list ref] wrote:

quoted

This patch should fix the issues seen with a recent fix to prevent
tunnel-in-tunnel frames from being generated with GRO.  The fix itself is
correct for now as long as we do not add any devices that support
NETIF_F_GSO_GRE_CSUM.  When such a device is added it could have the
potential to mess things up due to the fact that the outer transport header
points to the outer UDP header and not the GRE header as would be expected.

Fixes: fac8e0f579695 ("tunnels: Don't apply GRO to multiple layers of encapsulation.")

This could only fix FOU/GUE. It is very possible someone else could
happily be doing some other layered encapsulation and never had a
problem before, so the decision to start enforcing only a single layer
of encapsulation for GRO would still break them. I still think we
should revert the patch, and for next version fixes things to that any
combination/nesting of encapsulation is supported, and if there are
exceptions to that support they need be clearly documented.

It was pointed out to me that prior to my patch, it was also possible
to remotely cause a stack overflow by filling up a packet with tunnel
headers and letting GRO descend through them over and over again.

Then the fix would be set set a reasonable limit on the number of
encapsulation levels.

Tom, I'm sorry that you don't like how I fixed this issue but there
really, truly is a bug here. I gave you a specific example to be clear
but that doesn't mean that is the only case. I am aware that the bug
is not encountered in all situations and that the fix removes an
optimization in some of those but I think that ensuring correct
behavior must come first.

The example you gave results in packet loss, this is not
incorrectness. Actually reproduce a real issue that leads to
incorrectness and then we can talk about a solution.

Tom