[GitHub] [incubator-nuttx] a-lunev commented on a change in pull request #5138: net/tcp(unbuffered): advance sndseq by +1 because SYN and FIN occupy one sequence number

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

a-lunev commented on a change in pull request #5138:
URL: https://github.com/apache/incubator-nuttx/pull/5138#discussion_r777877997



##########
File path: net/tcp/tcp_input.c
##########
@@ -870,6 +870,11 @@ static void tcp_input(FAR struct net_driver_s *dev, uint8_t domain,
 
         if ((tcp->flags & TCP_CTL) == TCP_SYN)
           {
+#if !defined(CONFIG_NET_TCP_WRITE_BUFFERS)
+            tcp_setsequence(conn->sndseq, conn->rexmit_seq);
+#else
+            /* REVISIT for the buffered mode */

Review comment:
       PRs #5102 and #5138 changed only the unbuffered mode. The buffered mode was not modified (isolated by conditional compilation in those patches).
   Considering the unbuffered mode, previously sndseq was updated / overwritten in multiple places. In PR #5102 I removed excessive sndseq overwrites, including sndseq overwrite in tcp_input.c. However, that exposed an issue (sndseq did not advance by +1 on sending SYN packet). In PR #5138 I added advancing sndseq by +1 on sending SYN and FIN packets as it should be according to RFC 793 (previously the issue was hidden / masked because sndseq was overwritten by acknowledge numbers from received packets of the remote party. As I can see, that algorithm came from uIP).
   I added "REVISIT for the buffered mode" comments because the same correction may be reasonable for the buffered mode. I suppose there is the same situation as for the unbuffered mode (I have not yet analyzed the buffered mode code thoroughly).
   However, in the buffered mode I see other issues. When I tested the buffered mode some time ago I noticed that TCP window size advertised by NuttX stack jumped up and down very strange.
   One more issue with the buffered mode I found recently based on my netcat testing with enabled NETUTILS_NETCAT_SENDFILE option (I implemented the option here: https://github.com/apache/incubator-nuttx-apps/pull/952). After sending a file via netcat (over tcp_sendfile) the TCP connection is not finalized properly. Preliminary I think it may be similar issue with incorrect unackseq calculation that was with the unbuffered mode (#4656). I will investigate the issue.
   Anyway, I am going to prepare several new patches concerning tcp_sendfile similar to what I did for tcp_send_unbuffered. As I can see, tcp_sendfile implementation is very similar to tcp_send_unbuffered.




-- 
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