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

Submitted by:  Simon Goldschmidt <goldsimon>
Submitted on:  Sun 19 Apr 2009 12:30:34 PM UTC  
 
Category: TCPSeverity: 3 - Normal
Item Group: Faulty BehaviourStatus: Fixed
Privacy: PublicAssigned to: Simon Goldschmidt <goldsimon>
Open/Closed: ClosedPlanned 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>
Project Administrator
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>
Project AdministratorIn charge of this item.
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>
Project AdministratorIn charge of this item.

 

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

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

    Do you think this task is very important?
    If so, you can click here to add your encouragement to it.
    This task has 0 encouragements so far.

    Only logged-in users can vote.

     

    Please enter the title of George Orwell's famous dystopian book (it's a date):

     

     

    Follow 3 latest changes.

    Date Changed By Updated Field Previous Value => Replaced By
    Mon 20 Apr 2009 07:54:15 AM UTCkieranmOpen/ClosedOpen=>Closed
    Mon 20 Apr 2009 07:54:03 AM UTCkieranmStatusReady For Test=>Fixed
    Sun 19 Apr 2009 12:45:34 PM UTCgoldsimonStatusIn Progress=>Ready For Test

    Back to the top


    Powered by Savane 3.1-cleanup1