buglwIP - A Lightweight TCP/IP stack - Bugs: bug #37184, tcp_write problem for pcbs in the...

 
 

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

bug #37184: tcp_write problem for pcbs in the SYN_SENT state

Submitter:  Zach Smith <zsmith>
Submitted:  Fri 24 Aug 2012 10:43:58 PM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  goldsimon
Open/Closed:  Closed Planned Release:  None
lwIP version:  1.4.0

Tue 20 May 2014 08:13:23 PM UTC, comment #5: 

Fixed by using pcb->mss if mss_local gets zero.
Thanks for reporting.

Simon Goldschmidt <goldsimon>
Group administrator
Tue 20 May 2014 08:07:08 PM UTC, comment #4: 

I guess this is a simple regression: when making the change of limiting the segment size to half the maximum send window we received (to ensure getting delayed ACKs), we just didn't think pcb->snd_wnd_max would ever be zero.
Using pcb->mss in this case is good enough, I guess.

Simon Goldschmidt <goldsimon>
Group administrator
Thu 13 Sep 2012 08:16:50 PM UTC, comment #3: 

also, it seems that older versions of tcp_write did this

Zach Smith <zsmith>
Thu 13 Sep 2012 08:09:38 PM UTC, comment #2: 

Maybe this is too simple but it seems to me like an easy solution to this would be to use the minimum TCP MSS in this case where we haven't received the other hosts advertised MSS yet.  So, something like this:

instead of this line:
u16_t mss_local = mss_local = LWIP_MIN(pcb->mss, pcb->snd_wnd_max/2);
if pcb->state == SYN_SENT then snd_wnd_max (other hosts advertised mss)is 0

do this:
u16_t mss_local;
if(pcb->state == SYN_SENT)
  mss_local = pcb->mss;     mss has been initialized to 536
else
  mss_local = LWIP_MIN(pcb->mss, pcb->snd_wnd_max/2);

Every host must be able to accept TCP segments of 536 minimum so this would queue up segments of that size and they will be added to the unsent queue and go out when the connection is complete.

Zach Smith <zsmith>
Sun 26 Aug 2012 10:23:16 AM UTC, comment #1: 

I think technically 0 is the right answer for "how much data can we send" in this case, but I'll need to look at the code to see how that mss_local value is being used and work out the best way to improve it.  0 is not the right value for the MSS.

The workaround of not sending until connected should give you something that works but is not a long term solution.  I.e. I would like to fix this.

Kieran Mansley <kieranm>
Group Member
Fri 24 Aug 2012 10:43:58 PM UTC, original submission:  

if the pcb passed to tcp_write is in the state SYN_SENT then this line:
"u16_t mss_local = LWIP_MIN(pcb->mss, pcb->snd_wnd_max/2);"
evaluates to 0 because the pcb->snd_wnd_max has not been set yet (have not yet rcvd SYN_ACK from remote host)

then tcp_write_checks() checks the state of the pcb and allows SYN_SENT as a valid state for data transmission

then down in "phase 3" where the new segments are created in the while (pos < len){} loop the following line:
"u16_t max_len = mss_local - optlen;"
evaluates to 0.  So, the subsequent pbufs and segments are created with a len of 0. pos never increments and we loop around this loop until queuelen > TCP_SND_QUEUELEN at which point execution jumps to memerr.  The data is not added for transmission and returns ERR_MEM.

I don't know what the solution to this should be because a connection in the SYN_SENT state hasn't yet received the advertised MSS from the remote host and can't know what size it will be.  Is it safe to assume the default size of 536 in this case?

My use case is that I have created a tcp client connection by calling tcp_connect() and then just shortly afterwards (while pcb is still in SYN_SENT state) I call tcp_write with some data to go out.  perhaps I should wait until the connection is established to send data but the fact remains that tcp_write (tcp_write_checks) doesn't error on pcbs in the SYN_SENT state.

thank you

Zach Smith <zsmith>

 

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

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by goldsimon (Posted a comment)
  • -email is unavailable- added by kieranm (Posted a comment)
  • -email is unavailable- added by zsmith (Submitted the item)
  • -email is unavailable- added by zsmith
  •  

    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
    2014-05-20 goldsimon StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2014-05-20 goldsimon StatusNone In Progress
        Assigned toNone goldsimon
    2012-08-24 zsmith Carbon-Copy- Added -email is unavailable-

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code