buglwIP - A Lightweight TCP/IP stack - Bugs: bug #26236, TCP options (timestamp) don't work...

 
 

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

bug #26236: TCP options (timestamp) don't work with no-copy-tcpwrite

Submitter:  Simon Goldschmidt <goldsimon>
Submitted:  Sun 19 Apr 2009 12:30:34 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:  CVS Head

Mon 20 Apr 2009 07:54:03 AM UTC, comment #2: 

Thanks for this (and all the other fixes to the timestamp code that you've made this week).  From a quick glance at the code it looks good to me, and what you say makes sense.  I'll try and find some time to test it, but I'm happy to resolve this fixed in the mean time.

Kieran Mansley <kieranm>
Group Member
Sun 19 Apr 2009 12:45:34 PM UTC, comment #1: 

Kieran, I've checked in a change that deallocates the space reserved for options and only concatenates segments with the same flags. I think this fixes it, but I'd be happy to know your thoughts about it before closing this entry as fixed (since the timestamp/option code is from you).

Simon Goldschmidt <goldsimon>
Group administrator
Sun 19 Apr 2009 12:30:34 PM UTC, original submission:  

When calling tcp_write (on a pcb with the TF_TIMESTAMP flag turned on) without the COPY flag, it allocates 2 pbufs (RAM for headers + options, ROM for data).

If the last unsent segment has room, the header will be deallocated but the space reserved for the data will not be deallocated. Also, the flags are lost (not copied to the unsent segment).

The obvious solution is to change

    if(pbuf_header(queue->p, -TCP_HLEN)) {

to

    if(pbuf_header(queue->p, -(TCP_HLEN + optlen))) {

which would effectively deallocate the space reserved for options as well.

However, we would lose the options (from @param optflags): copying the flags to the unsent segment does not work since we cannot be sure there is enough space reserved in the header pbuf of that segment to allocate the options.

I'll check in the above code change since it's obvious, but the cleanest solution for not copying the options would be to not concatenate the two segments if the options are different.

Any comments?

(I've set this planned for 1.3.1 since it's a bug introduced after 1.3.0)

Simon Goldschmidt <goldsimon>
Group administrator

 

(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 kieranm (Posted a comment)
  • -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 3 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2009-04-20 kieranm Open/ClosedOpen Closed
    2009-04-20 kieranm StatusReady For Test Fixed
    2009-04-19 goldsimon StatusIn Progress Ready For Test

    Back to the top

    Powered by Savane 3.13-02a9.
    Corresponding source code