* Checksum support: default behaviour @ 2021-06-11 14:59 Matthieu Baerts 2021-06-12 4:05 ` Mat Martineau 0 siblings, 1 reply; 6+ messages in thread From: Matthieu Baerts @ 2021-06-11 14:59 UTC (permalink / raw) To: MPTCP Upstream 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? Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Checksum support: default behaviour 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:54 ` Matthieu Baerts 0 siblings, 2 replies; 6+ messages in thread From: Mat Martineau @ 2021-06-12 4:05 UTC (permalink / raw) To: Matthieu Baerts; +Cc: MPTCP Upstream 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. Your last point about connecting to older upstream kernels is also an important one. I'd rather keep it possible to connect to those kernels using default configuration options unless it's too risky to do so. -- Mat Martineau Intel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Checksum support: default behaviour 2021-06-12 4:05 ` Mat Martineau @ 2021-06-14 10:39 ` Paolo Abeni 2021-06-14 15:58 ` Matthieu Baerts 2021-06-14 15:54 ` Matthieu Baerts 1 sibling, 1 reply; 6+ messages in thread From: Paolo Abeni @ 2021-06-14 10:39 UTC (permalink / raw) To: Mat Martineau, Matthieu Baerts; +Cc: MPTCP Upstream 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) (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). /P ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Checksum support: default behaviour 2021-06-14 10:39 ` Paolo Abeni @ 2021-06-14 15:58 ` Matthieu Baerts 0 siblings, 0 replies; 6+ messages in thread From: Matthieu Baerts @ 2021-06-14 15:58 UTC (permalink / raw) To: Paolo Abeni, Mat Martineau; +Cc: MPTCP Upstream 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 ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Checksum support: default behaviour 2021-06-12 4:05 ` Mat Martineau 2021-06-14 10:39 ` Paolo Abeni @ 2021-06-14 15:54 ` Matthieu Baerts 2021-06-18 14:23 ` Matthieu Baerts 1 sibling, 1 reply; 6+ messages in thread From: Matthieu Baerts @ 2021-06-14 15:54 UTC (permalink / raw) To: Mat Martineau; +Cc: MPTCP Upstream Hi Mat, Thank you for your reply! On 12/06/2021 06:05, 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. I don't have data but I asked around and the checksum support has been added for middleboxes like Application-level gateway (ALG) or the ones modifying HTTP to add more content, inject JS, etc. For the ALG, I know that they are needed for NAT, e.g. for FTP to change the IP/port inside the payload. But in this case, I don't think it is supposed to change the size of the packet, only bytes inside. (except if we switch from v4 to v6?). So in theory, MPTCP DSS doesn't need to be updated. For the other cases, hopefully these middleboxes should inject quite a bit of data and that should be somehow visible for MPTCP to fallback to TCP. But again, I didn't do any experiment on that nor have data. At Tessares, we usually control the network between the client and the server but the situation is different using the "wild Internet" :) > Your last point about connecting to older upstream kernels is also an > important one. I'd rather keep it possible to connect to those kernels > using default configuration options unless it's too risky to do so. Yes, that seems to force us to keep it disabled by default. At least for the moment. Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: Checksum support: default behaviour 2021-06-14 15:54 ` Matthieu Baerts @ 2021-06-18 14:23 ` Matthieu Baerts 0 siblings, 0 replies; 6+ messages in thread From: Matthieu Baerts @ 2021-06-18 14:23 UTC (permalink / raw) To: Mat Martineau; +Cc: MPTCP Upstream Hi Mat, On 14/06/2021 17:54, Matthieu Baerts wrote: > Hi Mat, > > Thank you for your reply! > > On 12/06/2021 06:05, 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. > > I don't have data but I asked around and the checksum support has been > added for middleboxes like Application-level gateway (ALG) or the ones > modifying HTTP to add more content, inject JS, etc. > > For the ALG, I know that they are needed for NAT, e.g. for FTP to change > the IP/port inside the payload. But in this case, I don't think it is > supposed to change the size of the packet, only bytes inside. (except if > we switch from v4 to v6?). So in theory, MPTCP DSS doesn't need to be > updated. There might be issues with protocol like FTP: the IP address is represented in plain text which means it will likely change the size of the packet. DSS mapping will no longer be correct. If we reduce the size, we might take a bit of time to realise we will never get the missing bit. But we should detect it at some points and yeah, that's FTP... Cheers, Matt -- Tessares | Belgium | Hybrid Access Solutions www.tessares.net ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2021-06-18 14:23 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 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 2021-06-14 15:54 ` Matthieu Baerts 2021-06-18 14:23 ` Matthieu Baerts
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).