From: Jakub Kicinski <kuba@kernel.org> Date: 2024-03-26 20:34:13
Hi!
I got a report from a user surprised/displeased that ICMP_TIME_EXCEEDED
breaks connect(), while TCP RFCs say it shouldn't. Even pointing a
finger at Linux, RFC5461:
A number of TCP implementations have modified their reaction to all
ICMP soft errors and treat them as hard errors when they are received
for connections in the SYN-SENT or SYN-RECEIVED states. For example,
this workaround has been implemented in the Linux kernel since
version 2.0.0 (released in 1996) [Linux]. However, it should be
noted that this change violates section 4.2.3.9 of [RFC1122], which
states that these ICMP error messages indicate soft error conditions
and that, therefore, TCP MUST NOT abort the corresponding connection.
Is there any reason we continue with this behavior or is it just that
nobody ever sent a patch?
Somewhat related in tcp_v4_err() we do:
switch (sk->sk_state) {
case TCP_SYN_SENT:
case TCP_SYN_RECV:
[...]
if (!sock_owned_by_user(sk)) {
WRITE_ONCE(sk->sk_err, err);
sk_error_report(sk);
tcp_done(sk);
} else {
WRITE_ONCE(sk->sk_err_soft, err);
}
goto out;
}
So the error is soft if socket is locked, and I can't find anything
in backlog processing which would pay attention. So it seems that
under certain conditions we already ignore it.
Can we ignore it always, or perhaps conditionally based on IP_RECVERR?
On Tue, Mar 26, 2024 at 9:34 PM Jakub Kicinski [off-list ref] wrote:
Hi!
I got a report from a user surprised/displeased that ICMP_TIME_EXCEEDED
breaks connect(), while TCP RFCs say it shouldn't. Even pointing a
finger at Linux, RFC5461:
A number of TCP implementations have modified their reaction to all
ICMP soft errors and treat them as hard errors when they are received
for connections in the SYN-SENT or SYN-RECEIVED states. For example,
this workaround has been implemented in the Linux kernel since
version 2.0.0 (released in 1996) [Linux]. However, it should be
noted that this change violates section 4.2.3.9 of [RFC1122], which
states that these ICMP error messages indicate soft error conditions
and that, therefore, TCP MUST NOT abort the corresponding connection.
Is there any reason we continue with this behavior or is it just that
nobody ever sent a patch?
Back in November of 2023 Eric did merge a patch to bring the
processing in line with section 4.2.3.9 of [RFC1122]:
0a8de364ff7a tcp: no longer abort SYN_SENT when receiving some ICMP
However, the fixed behavior did not meet some expectations of Vagrant
(see the netdev thread "Bug report connect to VM with Vagrant"), so
for now it got reverted:
b59db45d7eba tcp: Revert no longer abort SYN_SENT when receiving some ICMP
I think the hope was to root-cause the Vagrant issue, fix Vagrant's
assumptions, then resubmit Eric's commit. Eric mentioned on Jan 8,
2024: "We will submit the patch again for 6.9, once we get to the root
cause." But I don't think anyone has had time to do that yet.
neal
From: Jakub Kicinski <kuba@kernel.org> Date: 2024-03-26 23:55:55
On Tue, 26 Mar 2024 23:03:26 +0100 Neal Cardwell wrote:
On Tue, Mar 26, 2024 at 9:34 PM Jakub Kicinski [off-list ref] wrote:
quoted
Hi!
I got a report from a user surprised/displeased that ICMP_TIME_EXCEEDED
breaks connect(), while TCP RFCs say it shouldn't. Even pointing a
finger at Linux, RFC5461:
A number of TCP implementations have modified their reaction to all
ICMP soft errors and treat them as hard errors when they are received
for connections in the SYN-SENT or SYN-RECEIVED states. For example,
this workaround has been implemented in the Linux kernel since
version 2.0.0 (released in 1996) [Linux]. However, it should be
noted that this change violates section 4.2.3.9 of [RFC1122], which
states that these ICMP error messages indicate soft error conditions
and that, therefore, TCP MUST NOT abort the corresponding connection.
Is there any reason we continue with this behavior or is it just that
nobody ever sent a patch?
Back in November of 2023 Eric did merge a patch to bring the
processing in line with section 4.2.3.9 of [RFC1122]:
0a8de364ff7a tcp: no longer abort SYN_SENT when receiving some ICMP
However, the fixed behavior did not meet some expectations of Vagrant
(see the netdev thread "Bug report connect to VM with Vagrant"), so
for now it got reverted:
b59db45d7eba tcp: Revert no longer abort SYN_SENT when receiving some ICMP
I think the hope was to root-cause the Vagrant issue, fix Vagrant's
assumptions, then resubmit Eric's commit. Eric mentioned on Jan 8,
2024: "We will submit the patch again for 6.9, once we get to the root
cause." But I don't think anyone has had time to do that yet.
From: Eric Dumazet <edumazet@google.com> Date: 2024-03-27 13:05:32
On Wed, Mar 27, 2024 at 12:55 AM Jakub Kicinski [off-list ref] wrote:
On Tue, 26 Mar 2024 23:03:26 +0100 Neal Cardwell wrote:
quoted
On Tue, Mar 26, 2024 at 9:34 PM Jakub Kicinski [off-list ref] wrote:
quoted
Hi!
I got a report from a user surprised/displeased that ICMP_TIME_EXCEEDED
breaks connect(), while TCP RFCs say it shouldn't. Even pointing a
finger at Linux, RFC5461:
A number of TCP implementations have modified their reaction to all
ICMP soft errors and treat them as hard errors when they are received
for connections in the SYN-SENT or SYN-RECEIVED states. For example,
this workaround has been implemented in the Linux kernel since
version 2.0.0 (released in 1996) [Linux]. However, it should be
noted that this change violates section 4.2.3.9 of [RFC1122], which
states that these ICMP error messages indicate soft error conditions
and that, therefore, TCP MUST NOT abort the corresponding connection.
Is there any reason we continue with this behavior or is it just that
nobody ever sent a patch?
Back in November of 2023 Eric did merge a patch to bring the
processing in line with section 4.2.3.9 of [RFC1122]:
0a8de364ff7a tcp: no longer abort SYN_SENT when receiving some ICMP
However, the fixed behavior did not meet some expectations of Vagrant
(see the netdev thread "Bug report connect to VM with Vagrant"), so
for now it got reverted:
b59db45d7eba tcp: Revert no longer abort SYN_SENT when receiving some ICMP
I think the hope was to root-cause the Vagrant issue, fix Vagrant's
assumptions, then resubmit Eric's commit. Eric mentioned on Jan 8,
2024: "We will submit the patch again for 6.9, once we get to the root
cause." But I don't think anyone has had time to do that yet.
Ah.
Thank you!!
For the record, Leon Romanovsky brought this issue directly to Linus
Torvalds, stating that I broke things.
It tooks weeks before Shachar did some debugging, but with no
conclusion I recall.
This kind of stuff makes me not very eager to work on this point.
From: Leon Romanovsky <leon@kernel.org> Date: 2024-04-02 13:21:42
On Wed, Mar 27, 2024 at 02:05:17PM +0100, Eric Dumazet wrote:
On Wed, Mar 27, 2024 at 12:55 AM Jakub Kicinski [off-list ref] wrote:
quoted
On Tue, 26 Mar 2024 23:03:26 +0100 Neal Cardwell wrote:
quoted
On Tue, Mar 26, 2024 at 9:34 PM Jakub Kicinski [off-list ref] wrote:
quoted
Hi!
I got a report from a user surprised/displeased that ICMP_TIME_EXCEEDED
breaks connect(), while TCP RFCs say it shouldn't. Even pointing a
finger at Linux, RFC5461:
A number of TCP implementations have modified their reaction to all
ICMP soft errors and treat them as hard errors when they are received
for connections in the SYN-SENT or SYN-RECEIVED states. For example,
this workaround has been implemented in the Linux kernel since
version 2.0.0 (released in 1996) [Linux]. However, it should be
noted that this change violates section 4.2.3.9 of [RFC1122], which
states that these ICMP error messages indicate soft error conditions
and that, therefore, TCP MUST NOT abort the corresponding connection.
Is there any reason we continue with this behavior or is it just that
nobody ever sent a patch?
Back in November of 2023 Eric did merge a patch to bring the
processing in line with section 4.2.3.9 of [RFC1122]:
0a8de364ff7a tcp: no longer abort SYN_SENT when receiving some ICMP
However, the fixed behavior did not meet some expectations of Vagrant
(see the netdev thread "Bug report connect to VM with Vagrant"), so
for now it got reverted:
b59db45d7eba tcp: Revert no longer abort SYN_SENT when receiving some ICMP
I think the hope was to root-cause the Vagrant issue, fix Vagrant's
assumptions, then resubmit Eric's commit. Eric mentioned on Jan 8,
2024: "We will submit the patch again for 6.9, once we get to the root
cause." But I don't think anyone has had time to do that yet.
Ah.
Thank you!!
For the record, Leon Romanovsky brought this issue directly to Linus
Torvalds, stating that I broke things.
It tooks weeks before Shachar did some debugging, but with no
conclusion I recall.
Shachar didn't do debugging, she didn't write the bisected patch.
She is verification engineer who was ready to run ANY tests and try
ANY debug patch which you wanted.
This kind of stuff makes me not very eager to work on this point.
From: Eric Dumazet <edumazet@google.com> Date: 2024-04-02 13:32:11
On Tue, Apr 2, 2024 at 3:21 PM Leon Romanovsky [off-list ref] wrote:
On Wed, Mar 27, 2024 at 02:05:17PM +0100, Eric Dumazet wrote:
quoted
On Wed, Mar 27, 2024 at 12:55 AM Jakub Kicinski [off-list ref] wrote:
quoted
On Tue, 26 Mar 2024 23:03:26 +0100 Neal Cardwell wrote:
quoted
On Tue, Mar 26, 2024 at 9:34 PM Jakub Kicinski [off-list ref] wrote:
quoted
Hi!
I got a report from a user surprised/displeased that ICMP_TIME_EXCEEDED
breaks connect(), while TCP RFCs say it shouldn't. Even pointing a
finger at Linux, RFC5461:
A number of TCP implementations have modified their reaction to all
ICMP soft errors and treat them as hard errors when they are received
for connections in the SYN-SENT or SYN-RECEIVED states. For example,
this workaround has been implemented in the Linux kernel since
version 2.0.0 (released in 1996) [Linux]. However, it should be
noted that this change violates section 4.2.3.9 of [RFC1122], which
states that these ICMP error messages indicate soft error conditions
and that, therefore, TCP MUST NOT abort the corresponding connection.
Is there any reason we continue with this behavior or is it just that
nobody ever sent a patch?
Back in November of 2023 Eric did merge a patch to bring the
processing in line with section 4.2.3.9 of [RFC1122]:
0a8de364ff7a tcp: no longer abort SYN_SENT when receiving some ICMP
However, the fixed behavior did not meet some expectations of Vagrant
(see the netdev thread "Bug report connect to VM with Vagrant"), so
for now it got reverted:
b59db45d7eba tcp: Revert no longer abort SYN_SENT when receiving some ICMP
I think the hope was to root-cause the Vagrant issue, fix Vagrant's
assumptions, then resubmit Eric's commit. Eric mentioned on Jan 8,
2024: "We will submit the patch again for 6.9, once we get to the root
cause." But I don't think anyone has had time to do that yet.
Ah.
Thank you!!
For the record, Leon Romanovsky brought this issue directly to Linus
Torvalds, stating that I broke things.
I was waiting input from you. I think you only waited for "revert first"
quoted
It tooks weeks before Shachar did some debugging, but with no
conclusion I recall.
Shachar didn't do debugging, she didn't write the bisected patch.
She is verification engineer who was ready to run ANY tests and try
ANY debug patch which you wanted.
quoted
This kind of stuff makes me not very eager to work on this point.
OK, so it is not important at the end.
I certainly do not want to waste time arguing with you on a valid
patch, which happens to break some buggy user space.
Apparently some people think RFC are not important.
You won.
From: Jason Xing <hidden> Date: 2024-04-02 14:17:55
On Tue, Apr 2, 2024 at 9:32 PM Eric Dumazet [off-list ref] wrote:
On Tue, Apr 2, 2024 at 3:21 PM Leon Romanovsky [off-list ref] wrote:
quoted
On Wed, Mar 27, 2024 at 02:05:17PM +0100, Eric Dumazet wrote:
quoted
On Wed, Mar 27, 2024 at 12:55 AM Jakub Kicinski [off-list ref] wrote:
quoted
On Tue, 26 Mar 2024 23:03:26 +0100 Neal Cardwell wrote:
quoted
On Tue, Mar 26, 2024 at 9:34 PM Jakub Kicinski [off-list ref] wrote:
quoted
Hi!
I got a report from a user surprised/displeased that ICMP_TIME_EXCEEDED
breaks connect(), while TCP RFCs say it shouldn't. Even pointing a
finger at Linux, RFC5461:
A number of TCP implementations have modified their reaction to all
ICMP soft errors and treat them as hard errors when they are received
for connections in the SYN-SENT or SYN-RECEIVED states. For example,
this workaround has been implemented in the Linux kernel since
version 2.0.0 (released in 1996) [Linux]. However, it should be
noted that this change violates section 4.2.3.9 of [RFC1122], which
states that these ICMP error messages indicate soft error conditions
and that, therefore, TCP MUST NOT abort the corresponding connection.
Is there any reason we continue with this behavior or is it just that
nobody ever sent a patch?
Back in November of 2023 Eric did merge a patch to bring the
processing in line with section 4.2.3.9 of [RFC1122]:
0a8de364ff7a tcp: no longer abort SYN_SENT when receiving some ICMP
However, the fixed behavior did not meet some expectations of Vagrant
(see the netdev thread "Bug report connect to VM with Vagrant"), so
for now it got reverted:
b59db45d7eba tcp: Revert no longer abort SYN_SENT when receiving some ICMP
I think the hope was to root-cause the Vagrant issue, fix Vagrant's
assumptions, then resubmit Eric's commit. Eric mentioned on Jan 8,
2024: "We will submit the patch again for 6.9, once we get to the root
cause." But I don't think anyone has had time to do that yet.
Ah.
Thank you!!
For the record, Leon Romanovsky brought this issue directly to Linus
Torvalds, stating that I broke things.
I was waiting input from you. I think you only waited for "revert first"
quoted
quoted
It tooks weeks before Shachar did some debugging, but with no
conclusion I recall.
Shachar didn't do debugging, she didn't write the bisected patch.
She is verification engineer who was ready to run ANY tests and try
ANY debug patch which you wanted.
quoted
This kind of stuff makes me not very eager to work on this point.
OK, so it is not important at the end.
I certainly do not want to waste time arguing with you on a valid
patch, which happens to break some buggy user space.
Apparently some people think RFC are not important.
RFC is important.
Honestly, I read those threads over and over again. Since she provided
some tcpdump logs which do not include ICMP, my question is still the
same as Eric: why does this breakage have a relationship with this
patch??? I get lost. It doesn't make sense really...
If someone is able to more easily reproduce this issue, I'm happy to help debug.
Thanks,
Jason
From: Leon Romanovsky <leon@kernel.org> Date: 2024-04-02 17:32:39
On Tue, Apr 02, 2024 at 10:17:16PM +0800, Jason Xing wrote:
On Tue, Apr 2, 2024 at 9:32 PM Eric Dumazet [off-list ref] wrote:
quoted
On Tue, Apr 2, 2024 at 3:21 PM Leon Romanovsky [off-list ref] wrote:
quoted
On Wed, Mar 27, 2024 at 02:05:17PM +0100, Eric Dumazet wrote:
quoted
On Wed, Mar 27, 2024 at 12:55 AM Jakub Kicinski [off-list ref] wrote:
quoted
On Tue, 26 Mar 2024 23:03:26 +0100 Neal Cardwell wrote:
quoted
On Tue, Mar 26, 2024 at 9:34 PM Jakub Kicinski [off-list ref] wrote:
quoted
Hi!
I got a report from a user surprised/displeased that ICMP_TIME_EXCEEDED
breaks connect(), while TCP RFCs say it shouldn't. Even pointing a
finger at Linux, RFC5461:
A number of TCP implementations have modified their reaction to all
ICMP soft errors and treat them as hard errors when they are received
for connections in the SYN-SENT or SYN-RECEIVED states. For example,
this workaround has been implemented in the Linux kernel since
version 2.0.0 (released in 1996) [Linux]. However, it should be
noted that this change violates section 4.2.3.9 of [RFC1122], which
states that these ICMP error messages indicate soft error conditions
and that, therefore, TCP MUST NOT abort the corresponding connection.
Is there any reason we continue with this behavior or is it just that
nobody ever sent a patch?
Back in November of 2023 Eric did merge a patch to bring the
processing in line with section 4.2.3.9 of [RFC1122]:
0a8de364ff7a tcp: no longer abort SYN_SENT when receiving some ICMP
However, the fixed behavior did not meet some expectations of Vagrant
(see the netdev thread "Bug report connect to VM with Vagrant"), so
for now it got reverted:
b59db45d7eba tcp: Revert no longer abort SYN_SENT when receiving some ICMP
I think the hope was to root-cause the Vagrant issue, fix Vagrant's
assumptions, then resubmit Eric's commit. Eric mentioned on Jan 8,
2024: "We will submit the patch again for 6.9, once we get to the root
cause." But I don't think anyone has had time to do that yet.
Ah.
Thank you!!
For the record, Leon Romanovsky brought this issue directly to Linus
Torvalds, stating that I broke things.
I was waiting input from you. I think you only waited for "revert first"
quoted
quoted
It tooks weeks before Shachar did some debugging, but with no
conclusion I recall.
Shachar didn't do debugging, she didn't write the bisected patch.
She is verification engineer who was ready to run ANY tests and try
ANY debug patch which you wanted.
quoted
This kind of stuff makes me not very eager to work on this point.
OK, so it is not important at the end.
I certainly do not want to waste time arguing with you on a valid
patch, which happens to break some buggy user space.
Apparently some people think RFC are not important.
RFC is important.
Honestly, I read those threads over and over again. Since she provided
some tcpdump logs which do not include ICMP, my question is still the
same as Eric: why does this breakage have a relationship with this
patch??? I get lost. It doesn't make sense really...