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/08/14 08:18:40 UTC
[GitHub] [incubator-nuttx] anchao opened a new pull request #1589: net/tcp: fix tcp socket close timeout if loss wireless connection
anchao opened a new pull request #1589:
URL: https://github.com/apache/incubator-nuttx/pull/1589
## Summary
net/tcp: fix tcp socket close timeout if loss wireless connection
In the current net stack implementation, there is no mechanism
for notifying the loss of the wireless connection, if the network
is disconnected then application sends data packets through tcp,
the tcp_timer will keep retrying fetch the ack for awhile, the
connection status will not be able to be switched timely.
Change-Id: I84d1121527edafc6ee6ad56ba164838694e7e11c
Signed-off-by: chao.an <an...@xiaomi.com>
## Impact
tcp_send / tcp_close
## Testing
carrier off the wireless connection and close the tcp handler
----------------------------------------------------------------
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] v01d commented on pull request #1589: net/tcp: fix tcp socket close timeout if loss wireless connection
Posted by GitBox <gi...@apache.org>.
v01d commented on pull request #1589:
URL: https://github.com/apache/incubator-nuttx/pull/1589#issuecomment-691682171
@patacongo please advise if you feel this is a valid change or not. If you're not sure we can request for someone else's review. I can't comment since I don't know about this part of NuttX.
----------------------------------------------------------------
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] xiaoxiang781216 commented on pull request #1589: net/tcp: fix tcp socket close timeout if loss wireless connection
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on pull request #1589:
URL: https://github.com/apache/incubator-nuttx/pull/1589#issuecomment-691688394
> I would think that a change like this would re-use the same logic as when the PHY detects a loss of the network in a wired connection as in drivers/net/phy_notify.c.
@patacongo let's the already opened socket could close timely when the wire or wireless network lose the conntection(what's this patch do), which doesn't conflict or block to notify the userspace the connection status change through PHY notificaiton. Actually, the fix is required because the kernel space network stack should work without depending on an optional userspace component.
On the other hand, the current PHY notification and recovering desgin isn't general enough because it tightly couple with Ethernet MII/GMII definition and make it's hard to work with the wireless case.
----------------------------------------------------------------
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] patacongo commented on pull request #1589: net/tcp: fix tcp socket close timeout if loss wireless connection
Posted by GitBox <gi...@apache.org>.
patacongo commented on pull request #1589:
URL: https://github.com/apache/incubator-nuttx/pull/1589#issuecomment-674091983
I would think that a change like this would re-use the same logic as when the PHY detects a loss of the network in a wired connection as in drivers/net/phy_notify.c.
----------------------------------------------------------------
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] xiaoxiang781216 edited a comment on pull request #1589: net/tcp: fix tcp socket close timeout if loss wireless connection
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1589:
URL: https://github.com/apache/incubator-nuttx/pull/1589#issuecomment-691688394
> I would think that a change like this would re-use the same logic as when the PHY detects a loss of the network in a wired connection as in drivers/net/phy_notify.c.
@patacongo let's the already opened socket could close timely when the wire or wireless network lose the conntection(what's this patch do), which doesn't conflict or block to notify the userspace the connection status change through PHY notificaiton. Actually, the fix is required because the kernel space network stack should work without depending on an optional userspace component. Do you think so?
On the other hand, the current PHY notification and recovering design isn't general enough because it tightly couple with Ethernet MII/GMII definition and make it's hard to work with the wireless case. https://github.com/apache/incubator-nuttx/issues/1775 is opened to track this issue, @patacongo please give your comment there.
----------------------------------------------------------------
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] xiaoxiang781216 edited a comment on pull request #1589: net/tcp: fix tcp socket close timeout if loss wireless connection
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 edited a comment on pull request #1589:
URL: https://github.com/apache/incubator-nuttx/pull/1589#issuecomment-691688394
> I would think that a change like this would re-use the same logic as when the PHY detects a loss of the network in a wired connection as in drivers/net/phy_notify.c.
@patacongo let's the already opened socket could close timely when the wire or wireless network lose the conntection(what's this patch do), which doesn't conflict or block to notify the userspace the connection status change through PHY notificaiton. Actually, the fix is required because the kernel space network stack should work without depending on an optional userspace component. Do you think so?
On the other hand, the current PHY notification and recovering design isn't general enough because it tightly couple with Ethernet MII/GMII definition and make it's hard to work with the wireless case.
----------------------------------------------------------------
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] anchao commented on pull request #1589: net/tcp: fix tcp socket close timeout if loss wireless connection
Posted by GitBox <gi...@apache.org>.
anchao commented on pull request #1589:
URL: https://github.com/apache/incubator-nuttx/pull/1589#issuecomment-674139608
No, they are completely different problems, PHY notification does not change the behavior of the TCP/IP stack. The stack does not have any interface to determine whether the network is disconnected unexpectedly,
If the wireless network is disconnected after the application completes a socket send(2), then the TCP stack will keep retrying fetch for the ACK, the connection state is remain in TCP_ESTABLISHED
```
void tcp_timer(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn,
int hsec)
{
...
if (conn->tcpstateflags == TCP_TIME_WAIT ||
conn->tcpstateflags == TCP_FIN_WAIT_2) // skip, state remain TCP_ESTABLISHED
{
...
else if (conn->tcpstateflags != TCP_CLOSED)
{
...
if (conn->tx_unacked > 0) // still waiting for the ack but the network is unreachable
{
...
if (conn->tcpstateflags == TCP_SYN_RCVD && // skip, state remain TCP_ESTABLISHED
conn->nrtx >= TCP_MAXSYNRTX)
{
...
else if (
#ifdef CONFIG_NET_TCP_WRITE_BUFFERS
conn->expired > 0 ||
#else
conn->nrtx >= TCP_MAXRTX ||
#endif
(conn->tcpstateflags == TCP_SYN_SENT && // skip, state remain TCP_ESTABLISHED
conn->nrtx >= TCP_MAXSYNRTX)
)
{
....
```
In the process of waiting for ACK, network disconnection will lead to tcp_timer keeps trying to get the ACK, If the application close(2) the TCP connection, the close(2) call will blocked because the TCP stack does not know that the network is unreachable
----------------------------------------------------------------
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] xiaoxiang781216 merged pull request #1589: net/tcp: fix tcp socket close timeout if loss wireless connection
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged pull request #1589:
URL: https://github.com/apache/incubator-nuttx/pull/1589
----------------------------------------------------------------
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