[GitHub] [incubator-nuttx] a-lunev commented on pull request #6562: net/tcp: fix regression of invalid update the rexmit_seq in buffer mode

[GitBox <gi...@apache.org>]

a-lunev commented on PR #6562:
URL: https://github.com/apache/incubator-nuttx/pull/6562#issuecomment-1173227327

   > Hi,
   > 
   > I re-reviewed the previous commit, rexmit_seq should only work properly in nonbuffer mode or SNDFILE is enabled, right? 
   
   If you mean the upstream state before February 23, 2022, then yes, it is right (rexmit_seq was enabled by conditional compilation only for tcp_send_unbuffered and tcp_sendfile). After that time I have not yet analyzed what changes have been done in the upstream, so I do not know.
   
   > NET_TCP_WRITE_BUFFERS is used in our project, It seems that this issue is caused by my previous pull request:
   > 
   > #6451
   > 
   > The reason for submitting this PR is because we found that the rexmit_seq is not updated every time in buffer mode. rexmit_seq in an invalid value being used when packet loss triggers fast retransmission
   > 
   > In PR #6451, I adopted @a-lunev 's suggestion ( according to RFC 6298 (5.4) ). When triggering fast retransmission, only retransmit one segment that is not currently acknowledged, so rexmit_seq should not be used instead of sndseq in this scenario, because sndseq of retransmission in the packet does not need to be re-updated
   > 
   > Since I haven't done enough verification in the nonbuffer and SNDFILE mode, so rexmit_seq should be disabled if NET_TCP_WRITE_BUFFERS is enabled as before, I will update this PR soon
   
   For the buffered mode I enabled using rexmit_seq only in PR #5589 (it is not yet merged into the upstream). So far I have not yet analyzed the further history of changes in the upstream repo since my last other PR #5598 (it is also not yet merged into the usptream).
   Could you perform the following test:
   -- git checkout 1ded8bbabbe0b23e3242be17fb87d418966778b2 (PR #5589 was based on the commit)
   -- apply the patch of PR #5589
   -- apply the patch of PR #5598 (over the applied #5589)
   -- check / test if there are some issues on your side (The time I created PR #5589 and #5598, I tested them on my side and everything worked as expected, I wrote about this in PR #5589).


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org