Re: apropos https://github.com/multipath-tcp/mptcp_net-next/issues/265

[Mat Martineau <mathew.j.martineau@linux.intel.com>]

On Wed, 4 May 2022, Maxim Galaganov wrote:

> On 04.05.2022 12:52, Paolo Abeni wrote:
>> 
>> Could you please additionally test the following patch? Kernel for both
>> arches must be rebuilt.
>> 
>> This should cause old x86 build with csum enabled to fallback to TCP
>> while interoperating with new ones, which is ugly and bad, but - given
>> that csum is disable by default - possibly still acceptable?!?
>
>
> The results for me are as follows:
>
> old x86_64 (5.17.5) + old mips (5.15.35) => reset
> new x86_64 (5.18-rc5+patch) vs old mips (5.15.35) => works
> new x86_64 (5.18-rc5+patch) vs new mips (5.15.35+patch) => works
> old x86_64 (5.17.2) + new mips (5.15.35+patch) => reset
> old x86_64 (5.17.2) + new x86_64 (5.18-rc5+patch) => reset
> new x86_64 (5.18-rc5+patch) talking to itself => works
>
> I edited the patch so that it applies to my kernels, and this might be the 
> reason I don't see fallback but see reset instead -- I don't have "mptcp: TCP 
> fallback for established connections" patches in my tree as they are only in 
> net-next yet. Or I may be doing something stupid.
>
> Is there a way for an old box without infinite mapping support to fallback an 
> established connection in case of csum failure?
>

In some cases, I think we can do that!

Both peers do not consider MPTCP "fully established" until they receive a 
valid DATA_ACK, and the old box will fall back if the new box sends an 
infinite mapping in response to the first data. So, if as part of the fix 
we make sure the new box responds to a bad checksum in the initial DSS (or 
MPC+DATA) with an infinite mapping instead of a reset, fallback should 
work. And I think what's needed to do that is to only set subflow->mp_fail 
at the end of validate_data_csum() if subflow->fully_established is 
'true'.

However, if the new box is the first to send data with a checksum, the old 
box decides whether to reset or fallback. It looks like unpatched 5.17, 
5.16, and 5.15 would send a reset in this "patched host sends data first" 
scenario. 5.14 looks like it would fall back? Older MPTCP kernels don't 
support DSS checksums.

This means that if the use case is a client/server setup where the client 
sends data first, and the server is running a new/patched kernel, we get 
the fallback behavior. That's good!


This still leaves that "patched host sends data first" scenario that 
should be addressed. The RFC requires checksums to be enabled if one peer 
requests it in the SYN/SYNACK handshake. Is it too much of a kludge to use 
a sysctl that would force TCP fallback if a peer requests checksums? This 
would be just like the behavior we had in v5.13 and earlier. We could add 
another state to the existing MPTCP checksum sysctl ("force fallback if 
checksums requested").


--
Mat Martineau
Intel