buglwIP - A Lightweight TCP/IP stack - Bugs: bug #26012, ACKs by SWS avoidance

 
 

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

bug #26012: ACKs by SWS avoidance

Submitter:  Oleg Tyshev <olegreen>
Submitted:  Thu 26 Mar 2009 03:43:57 PM UTC
   
 
Category:  TCP Severity:  3 - Normal
Item Group:  Faulty Behaviour Status:  Fixed
Privacy:  Public Assigned to:  None
Open/Closed:  Closed Planned Release:  None
lwIP version:  1.3.0

Jump to the original submission

Wed 01 Apr 2009 04:10:24 PM UTC, comment #6: 

I have tried to insert time stamp for right announced edge.
(after applying patch for delayed ack)
It doesn't give significant reduction in number of transferred packets.
You can close this bug.

Oleg Tyshev <olegreen>
Mon 30 Mar 2009 05:38:52 PM UTC, comment #5: 

That's right it would be no retransmition.
It is my error.
But this patch can help to avoid zero window probe,
because TCP_WND_UPDATE_THRESHOLD could be relative large.

Oleg Tyshev <olegreen>
Mon 30 Mar 2009 12:04:41 PM UTC, comment #4: 

Thanks for taking a look.

I don't see why the other side would need to retransmit anything.  Can you explain?

The data should be ACKed already by tcp_receive().  All that the tcp_recved() function is doing is working out whether a window update is necessary.  Window updates shouldn't affect retransmissions at all.

Kieran Mansley <kieranm>
Group Member
Fri 27 Mar 2009 05:44:37 PM UTC, comment #3: 

I have seen this patch.
It looks good for me.

It would be nice to add time stamp to pcb
(something like rcv_ann_right_edge_time).
Than it would be possible to send explicit window update from tcp_recved() for case with slow application data read.

if ( old_rcv_ann_wnd < mss &&
     pcb->rcv_ann_wnd > mss &&
     time_now > (pcb->rcv_ann_right_edge_time + 0.5 sec) ) {
  tcp_ack_now(pcb);
}

Without it other side have to retransmit due to retransmission timeout.


Oleg Tyshev <olegreen>
Fri 27 Mar 2009 12:38:36 PM UTC, comment #2: 

I've added a patch here:

https://savannah.nongnu.org/bugs/download.php?file_id=17817

That should solve a lot of these problems.  I would welcome comments and review.

Kieran Mansley <kieranm>
Group Member
Thu 26 Mar 2009 04:09:36 PM UTC, comment #1: 

That patch certainly helps, but there are some other issues outstanding in this area that I'd like to resolve at the same time.  e.g. BUG 20515. 

I want to see lwIP use rcv_ann_wnd in a slightly different way: at the moment it stores the value we might advertise, not the value we did advertise.  I'd like to add another bit of state that tracks the right-hand edge of the window that we advertised (which could be done using the ts_lastacksent field from the timestamps patch I've recently added).  The combination of these would allow us to make sure that window advertisements never shrink the window and only go up in good sized chunks so we're not advertising silly windows and not sending unecessary updates.  I'll try and do some work in this area.

Kieran Mansley <kieranm>
Group Member
Thu 26 Mar 2009 03:43:57 PM UTC, original submission:  

In tcp_recved we increase pcb->rcv_ann_wnd only if receive window >= MSS. That's right.
But tcp_ack(pcb) we call always.
tcp_ack sets TF_ACK_DELAY flag and with next tcp_fasttmr() tick would be duplicate ACK sent (each 250 ms).

If receive buffer is full and application read data relative slow (e.g. 1 byte per 100 millisecond) would be generated 4 unnecessary duplicate ACKs per second

I suppose this patch can help

Oleg Tyshev <olegreen>

 

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

Attached Files
file #17808:  sws_acks.patch added by olegreen (1KiB - text/x-patch)

 

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 olegreen (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-02 kieranm StatusNone Fixed
        Open/ClosedOpen Closed
    2009-03-26 olegreen Attached File- Added sws_acks.patch, #17808

    Back to the top

    Powered by Savane 3.13-d3ae.
    Corresponding source code