You are viewing a plain text version of this content. The canonical link for it is here.

Posted to commits@nuttx.apache.org by GitBox <gi...@apache.org> on 2020/11/18 09:38:12 UTC

[GitHub] [incubator-nuttx] retogaeh opened a new pull request #2332: net/tcp: Rectified keepalive fix

retogaeh opened a new pull request #2332:
URL: https://github.com/apache/incubator-nuttx/pull/2332


   ## Summary
   Simplification of previous fix. Also fixes the case when new data should
   be sent while a keepalive is not yet ACKed.
   
   _Previous fix did not work properly_
   
   ## Impact
   Improves TCP stability if keepalive is used.
   
   ## Testing
   
   


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

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

[GitHub] [incubator-nuttx] acassis commented on pull request #2332: net/tcp: Rectified keepalive fix

Posted by GitBox <gi...@apache.org>.

acassis commented on pull request #2332:
URL: https://github.com/apache/incubator-nuttx/pull/2332#issuecomment-729737936


   Hi @retogaeh why #ifdef CONFIG_NET_TCP_KEEPALIVE is not used anymore? Is it modification able to handle the case where ACK is not received? I noticed conn->tx_unacked is not touched anymore.


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

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

[GitHub] [incubator-nuttx] retogaeh commented on pull request #2332: net/tcp: Rectified keepalive fix

Posted by GitBox <gi...@apache.org>.

retogaeh commented on pull request #2332:
URL: https://github.com/apache/incubator-nuttx/pull/2332#issuecomment-729753010

> Hi @retogaeh why #ifdef CONFIG_NET_TCP_KEEPALIVE is not used anymore? Is it modification able to handle the case where ACK is not received? I noticed conn->tx_unacked is not touched anymore.

Hello acassis, regarding your questions:
Both sections with CONFIG_NET_TCP_KEEPALIVE were introduced by us with commit 3 weeks ago. They are only required if you access members which are only created if the same flag is defined. Since those accesses were removed, it is not needed any longer. The one in tcp_send.c was added to revert what was done in tcp_timer.c to send an keepalive so that things should not be messed up if application data follows shortly after. However, that was the last piece we added before committing and sort of didn't test anymore. This actually broke our fix.
The code which added to the tcp_timer.c back then would have worked on its own but not in combination with tcp_send.c This basically made it possible that keepalive works.
However, as it turned out the only thing required is the removal of that tx_unacked increment. I am far away of fully understand this tcp stack, but in general a keepalive does not require a dummy payload.

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

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

[GitHub] [incubator-nuttx] acassis commented on pull request #2332: net/tcp: Rectified keepalive fix

Posted by GitBox <gi...@apache.org>.

acassis commented on pull request #2332:
URL: https://github.com/apache/incubator-nuttx/pull/2332#issuecomment-729940826


   Thank you @retogaeh !


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

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

[GitHub] [incubator-nuttx] acassis merged pull request #2332: net/tcp: Rectified keepalive fix

Posted by GitBox <gi...@apache.org>.

acassis merged pull request #2332:
URL: https://github.com/apache/incubator-nuttx/pull/2332


   


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

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