patchlwIP - A Lightweight TCP/IP stack - Patches: patch #9039, tcp: Avoid 3-second delay for a...

 
 

You are not allowed to post comments on this tracker with your current authentication level.

patch #9039: tcp: Avoid 3-second delay for a half-open connection

Submitter:  Simon Goldschmidt <goldsimon>
Submitted:  Mon 04 Jul 2016 08:19:01 AM UTC
   
 
Category:  TCP Priority:  5 - Normal
Status:  Done Privacy:  Public
Assigned to:  goldsimon Open/Closed:  Closed
Planned Release:  2.0.0 Beta2

Jump to the original submission

Tue 19 Jul 2016 08:40:05 PM UTC, comment #9: 

Definitively worth mentioning. Event worth fixing if it's a tiny fix... OTOH, it doesn't seem to be a real-world-problem ;-)

Simon Goldschmidt <goldsimon>
Group administrator
Tue 19 Jul 2016 08:32:49 PM UTC, comment #8: 

Simon,

I assume you mean my list of checks for SYN-SENT?  I just summarized the steps listed in RFC 793.  The only deviation I see in LwIP (with the fix applied from this patch) is that in the case we receive ONLY the ACK flag (and it's acceptable) we are still sending an RST.  RFC 793 says this should be silently dropped.

I can't think of a case where this would happen, but I thought the deviation from RFC 793 was worth mentioning

Joel Cunningham <jcunningham>
Group Member
Tue 19 Jul 2016 07:22:24 PM UTC, comment #7: 

Sorry, I've completely lost your comments #4 and #5.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 19 Jul 2016 07:10:09 PM UTC, comment #6: 

Open new bug (bug #48563) for support of simultaneous opens

Joel Cunningham <jcunningham>
Group Member
Tue 05 Jul 2016 03:30:54 PM UTC, comment #5: 

I reviewed RFC 793 and I think the only check we aren't doing is if the ACK is acceptable and no SYN is contained, then the packet is silently ignored.

I honestly can't think of a situation where the remote TCP would only ACK our SYN, but regardless we aren't exactly following the steps listed:

https://tools.ietf.org/html/rfc793#page-66

If in SYN-SENT:
1. Check for ACK
2. Check for RST
3. Check security precedence
4. Check for SYN
5. If neither SYN or RST, drop

I also noticed during this analysis that it looks like we don't handle simultaneous opens.  This is when both TCPs perform an active open at the same time, going from SYN-SENT to SYN-RCVD by receiving only a SYN packet.  This can be seen on page 32: https://tools.ietf.org/html/rfc793#page-32.  In our handling of SYN-SENT we only check for both SYN and ACK flags.  If my static analysis is correct, I can open a new bug

Joel Cunningham <jcunningham>
Group Member
Mon 04 Jul 2016 10:26:26 AM UTC, comment #4: 

I've had fear of creating and endless ping-pong loop when directly answering, but that's probably low risk and shouldn't happen...

Applied, thanks for the patch.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 04 Jul 2016 08:39:41 AM UTC, comment #3: 

I don't think that any check is needed.  An alternative would be to substantially reduce the retransmission timeout (which seems excessively high at three seconds).  This patch is intended to be a less intrusive change.

Michael Brown <mcb30>
Mon 04 Jul 2016 08:31:44 AM UTC, comment #2: 

Looks good, but do we need a check on ackno of the incoming ACK segment to prevent sending too much SYN segments? I think we don't, but I'm not absolutely sure...

Simon Goldschmidt <goldsimon>
Group administrator
Mon 04 Jul 2016 08:20:14 AM UTC, comment #1: 

Lost part of that message:

If lwIP encounters a half-open connection (e.g. due to a restarted
application reusing the same port numbers) it will correctly send a
RST but will not resend the SYN until one retransmission timeout later
(approximately three seconds). This can increase the time taken by
lpxelinux.0 to fetch its configuration file from a few milliseconds to
around 30 seconds.

Simon Goldschmidt <goldsimon>
Group administrator
Mon 04 Jul 2016 08:19:01 AM UTC, original submission:  

Received on lwip-devel, 2016-07-04:

Fix by immediately retransmitting the SYN whenever a half-open
connection is detected.

Signed-off-by: Michael Brown <mcb30@ipxe.org>
---
src/core/tcp_in.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/src/core/tcp_in.c b/src/core/tcp_in.c
index f291887..5e7f2cd 100644
--- a/src/core/tcp_in.c
+++ b/src/core/tcp_in.c
@@ -812,6 +812,9 @@ tcp_process(struct tcp_pcb *pcb)
/* send a RST to bring the other side in a non-synchronized state. */
tcp_rst(ackno, seqno + tcplen, ip_current_dest_addr(),
ip_current_src_addr(), tcphdr->dest, tcphdr->src);
+ /* Resend SYN immediately to establish connection */
+ pcb->rtime = 0;
+ tcp_rexmit_rto(pcb);
}
break;
case SYN_RCVD:
--
2.3.8

Simon Goldschmidt <goldsimon>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by jcunningham (Posted a comment)
  • -email is unavailable- added by gradator (Updated the item)
  • -email is unavailable- added by mcb30 (Posted a comment)
  • -email is unavailable- added by goldsimon
  • -email is unavailable- added by goldsimon (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2016-07-04 goldsimon StatusNone Done
        Assigned toNone goldsimon
        Open/ClosedOpen Closed
    2016-07-04 gradator Attached File- Added 0001-tcp-Avoid-3-second-delay-for-a-half-open-connection.patch, #37699
    2016-07-04 goldsimon Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code