All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthieu Baerts <matthieu.baerts@tessares.net>
To: Paolo Abeni <pabeni@redhat.com>,
	Mat Martineau <mathew.j.martineau@linux.intel.com>
Cc: MPTCP Upstream <mptcp@lists.linux.dev>
Subject: Re: Checksum support: default behaviour
Date: Mon, 14 Jun 2021 17:58:24 +0200	[thread overview]
Message-ID: <83da200b-97bf-2102-ef1d-7b196553e5e0@tessares.net> (raw)
In-Reply-To: <eb41332a032c3c8191ce6b0d0ce2d2aa93ad5081.camel@redhat.com>

Hi Paolo,

Thank you for your reply!

On 14/06/2021 12:39, Paolo Abeni wrote:
> On Fri, 2021-06-11 at 21:05 -0700, Mat Martineau wrote:
>> On Fri, 11 Jun 2021, Matthieu Baerts wrote:
>>
>>> Hello,
>>>
>>> With the current checksum support series from Geliang and Paolo
>>> available in our tree, the default behaviour is not to use this checksum
>>> feature.
>>>
>>> Should we eventually do the opposite and have it enabled by default?
>>>
>>> I do understand this has a cost in terms of performances but this could
>>> help detecting nasty middleboxes, i.e. the ones that modify the TCP
>>> packets without modifying MPTCP options if needed.
>>>
>>> On the other hand, I don't have numbers showing if these middleboxes are
>>> rare or not.
>>>
>>> But also, the main issue I see if we enable the checksum support by
>>> default is that we are no longer able to talk to servers not supporting
>>> it (<5.13), no?
>>>
>>> WDYT?
>>>
>>
>> I lean toward leaving checksums off by default, based on what I've heard 
>> from community members. It sounds like large deployments haven't seen 
>> checksums catch many problems? Some actual data about the frequency of 
>> checksum failures would really help.
> 
> +1 on keeping csum disabled by default until we have at least one of
> - performance data (delta)
> - impact proof (csum corruption in the wild)

I agree: it seems to be a good plan.

But if we decide now to keep it disabled by default, could we later
enable it by default? It is a visible modification for the userspace,
not really an API break but well...

>   (side note: even with no middle boxes at all, no app-layer mangling,
> we can have bad MPTCP csum with correct TCP csum, e.g. due to BER plus
> bad luck causing a stream corruption not catched by TCP csum but
> catched by the MPTCP csum).

I guess that would be quite unlucky, no? Lower layers also checks for
corruption, no?

Cheers,
Matt
-- 
Tessares | Belgium | Hybrid Access Solutions
www.tessares.net

  reply	other threads:[~2021-06-14 15:58 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-11 14:59 Checksum support: default behaviour Matthieu Baerts
2021-06-12  4:05 ` Mat Martineau
2021-06-14 10:39   ` Paolo Abeni
2021-06-14 15:58     ` Matthieu Baerts [this message]
2021-06-14 15:54   ` Matthieu Baerts
2021-06-18 14:23     ` Matthieu Baerts

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=83da200b-97bf-2102-ef1d-7b196553e5e0@tessares.net \
    --to=matthieu.baerts@tessares.net \
    --cc=mathew.j.martineau@linux.intel.com \
    --cc=mptcp@lists.linux.dev \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body. 
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.