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/12/16 02:50:30 UTC
[GitHub] [incubator-nuttx] anchao opened a new pull request #2548: net/tcp: Optimize TCP handshake performance
anchao opened a new pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548
## Summary
arch/netdev: try tcp timer in every txavail call
net/tcp/handshake: send the SYN immediately
net/tcp: send the ack on nonblock mode
In the current implementation, the first transmission of the new
connection handshake is depends entirely by tcp_timer(), which will
caused 0.5s - 1s delay each time in connect().
This patch is mainly to improve the performance of TCP handshake.
Original:
nsh> tcp_client
[ 1.536100] TCP connect start.
[ 2.000200] TCP connect end. DIFF: tick: 4641, 464ms.
[ 3.000300] TCP connect start.
[ 4.000400] TCP connect end. DIFF: tick: 10001, 1000ms.
[ 5.000500] TCP connect start.
[ 6.000600] TCP connect end. DIFF: tick: 10001, 1000ms.
[ 7.000700] TCP connect start.
[ 8.000800] TCP connect end. DIFF: tick: 10001, 1000ms.
Optimized:
nsh> tcp_client
[ 3.263600] TCP connect start.
[ 3.263700] TCP connect end. DIFF: tick: 1, 0ms.
[ 4.263800] TCP connect start.
[ 4.263800] TCP connect end. DIFF: tick: 0, 0ms.
[ 5.263900] TCP connect start.
[ 5.263900] TCP connect end. DIFF: tick: 0, 0ms.
[ 6.264000] TCP connect start.
[ 6.264000] TCP connect end. DIFF: tick: 0, 0ms.
[ 7.264100] TCP connect start.
[ 7.264100] TCP connect end. DIFF: tick: 0, 0ms.
Signed-off-by: chao.an <an...@xiaomi.com>
## Impact
TCP connect()
## Testing
tcp_client.c (Run in NuttX):
```
#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
int main(int argc, char **argv)
{
struct sockaddr_in serveraddr = { 0 };
int len = sizeof(serveraddr);
int start_ts;
int end_ts;
int ret;
int fd;
while (1) {
fd = socket(AF_INET, SOCK_STREAM, 0);
if (fd < 0)
return fd;
serveraddr.sin_family = AF_INET;
serveraddr.sin_addr.s_addr = inet_addr("192.168.31.28");
serveraddr.sin_port = htons(atoi("8888"));
start_ts = (int)clock_systime_ticks();
syslog(LOG_INFO, "TCP connect start.\n");
ret = connect(fd, (struct sockaddr*)&serveraddr, len);
if (ret < 0)
goto bail;
end_ts = (int)clock_systime_ticks();
syslog(LOG_INFO, "TCP connect end. DIFF: tick: %d, %dms.\n",
end_ts - start_ts, TICK2MSEC(end_ts - start_ts));
sleep(1);
close(fd);
}
bail:
close(fd);
return 0;
}
```
tcp_server.c (another host):
```
#include <stdio.h>
#include <sys/types.h>
#include <sys/socket.h>
#include <sys/socket.h>
#include <netinet/in.h>
#include <arpa/inet.h>
int main()
{
struct sockaddr_in serveraddr = {0}, clientaddr = {0};
int len = sizeof(serveraddr);
int clientfd;
int ret;
int fd;
fd = socket(AF_INET, SOCK_STREAM, 0);
if (fd < 0)
return fd;
serveraddr.sin_family = AF_INET;
serveraddr.sin_addr.s_addr = inet_addr("0.0.0.0");
serveraddr.sin_port = htons(8888);
ret = bind(fd, (struct sockaddr*)&serveraddr, len);
if (ret < 0)
goto bail;
listen(fd, 10);
while (1) {
clientfd = accept(fd, (struct sockaddr*)&clientaddr, &len);
usleep(500*1000);
close(clientfd);
}
bail:
close(fd);
return 0;
}
```
Result:
Original:
nsh> tcp_client
[ 1.536100] TCP connect start.
[ 2.000200] TCP connect end. DIFF: tick: 4641, 464ms.
[ 3.000300] TCP connect start.
[ 4.000400] TCP connect end. DIFF: tick: 10001, 1000ms.
[ 5.000500] TCP connect start.
[ 6.000600] TCP connect end. DIFF: tick: 10001, 1000ms.
[ 7.000700] TCP connect start.
[ 8.000800] TCP connect end. DIFF: tick: 10001, 1000ms.
[ 9.000900] TCP connect start.
[ 10.001000] TCP connect end. DIFF: tick: 10001, 1000ms.
[ 11.001100] TCP connect start.
[ 12.001200] TCP connect end. DIFF: tick: 10001, 1000ms.
Optimized:
nsh> tcp_client
[ 3.263600] TCP connect start.
[ 3.263700] TCP connect end. DIFF: tick: 1, 0ms.
[ 4.263800] TCP connect start.
[ 4.263800] TCP connect end. DIFF: tick: 0, 0ms.
[ 5.263900] TCP connect start.
[ 5.263900] TCP connect end. DIFF: tick: 0, 0ms.
[ 6.264000] TCP connect start.
[ 6.264000] TCP connect end. DIFF: tick: 0, 0ms.
[ 7.264100] TCP connect start.
[ 7.264100] TCP connect end. DIFF: tick: 0, 0ms.
[ 8.264200] TCP connect start.
[ 8.264200] TCP connect end. DIFF: tick: 0, 0ms.
[ 9.264300] TCP connect start.
[ 9.264300] TCP connect end. DIFF: tick: 0, 0ms.
[ 10.264400] TCP connect start.
[ 10.264400] TCP connect end. DIFF: tick: 0, 0ms.
----------------------------------------------------------------
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] masayuki2009 commented on pull request #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747828790
> I will investigate this side effect further, my local test program is also using http audio streaming, we use ffmpeg built-in http container, Could you please help share more details of your test case? or is it possible to reproduce this problem through other samples(wget ? curl ?)?
@anchao
I'm not sure if they can stop downloading manually (i.e. active close case)
>Yes, if I create a tag with a similar name locally, the processing here will be messy.
>I noticed your new pull request #2561. I picked it and seems works as expected. It may need review by other PMCs further.
Thanks and could you put your comments to the PR?
----------------------------------------------------------------
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 #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
anchao commented on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747836622
> @anchao
> I'm not sure if they can stop downloading manually (i.e. active close case)
Hi @masayuki2009 san,
Could you please check the behavior if revert related commit or just restore the changes of devif_timer() back to devif_poll() under the configuration of spresense:rndi?
----------------------------------------------------------------
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 #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
anchao commented on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747825295
> Hi @anchao
>
> I tested this PR with spresense:rndi configuration.
> It works but found a side effect.
>
> For example, if nxplayer starts HTTP audio streaming and then stops manually.
> Sometimes, a deadlock happened.
> In this case, I noticed that the FIN packet was not sent.
> Could you confirm if this kind of side effect happens on your side?
Hi @masayuki2009 san,
I will investigate this side effect further, my local test program is also using http audio streaming, we use ffmpeg built-in http container, Could you please help share more details of your test case? or is it possible to reproduce this problem through other samples(wget ? curl ?)?
>
> Also, I had trouble with #2557
> So I had to revert it locally to test this PR.
> Is the PR works on your side?
Yes, if I create a tag with a similar name locally, the processing here will be messy.
I noticed your new pull request #2561. I picked it and seems works as expected. It may need review by other PMCs further.
----------------------------------------------------------------
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 edited a comment on pull request #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
anchao edited a comment on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747637195
> Hi @anchao,
>
> Nice performance improvements!
> I think we should apply this change to the following network drivers too because they are using devif_poll().
> What do you think?
>
> * usbdev: cdcecm.c, rndis.c
> * net: rpmsgdrv.c, loopback.c, slip.c, enc28j60.c, encx24j600.c lan91c111.c, tun.c, ftmac100.c, skeleton.c, dm90x0.c
> * usbhost: usbhost_cdcmbim.c
> * wireless: bcm_netdev.c, spirit_netdev.c, xbee_netdev.c
Hi @masayuki2009 san,
Thanks for your review and reminder !
Yes, I have omitted the network driver part, please review the new commit, which has added driver changes.
----------------------------------------------------------------
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 #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
anchao commented on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747637195
> Hi @anchao,
>
> Nice performance improvements!
> I think we should apply this change to the following network drivers too because they are using devif_poll().
> What do you think?
>
> * usbdev: cdcecm.c, rndis.c
> * net: rpmsgdrv.c, loopback.c, slip.c, enc28j60.c, encx24j600.c lan91c111.c, tun.c, ftmac100.c, skeleton.c, dm90x0.c
> * usbhost: usbhost_cdcmbim.c
> * wireless: bcm_netdev.c, spirit_netdev.c, xbee_netdev.c
Hi @masayuki2009 san,
Hi masayuki san,
Thanks for your review and reminder !
Yes, I have omitted the network driver part, please review the new commit, which has added driver changes.
----------------------------------------------------------------
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] masayuki2009 edited a comment on pull request #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
masayuki2009 edited a comment on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747393440
Hi @anchao,
Nice performance improvements!
However, should we apply this change to the following network drivers too? because they are using devif_poll().
- usbdev: cdcecm.c, rndis.c
- net: rpmsgdrv.c, loopback.c, slip.c, enc28j60.c, encx24j600.c lan91c111.c, tun.c, ftmac100.c, skeleton.c, dm90x0.c
- usbhost: usbhost_cdcmbim.c
- wireless: bcm_netdev.c, spirit_netdev.c, xbee_netdev.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] masayuki2009 commented on pull request #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747857868
> > @anchao
> > I'm not sure if they can stop downloading manually (i.e. active close case)
>
> Hi @masayuki2009 san,
>
> Could you please check the behavior if revert related commit or just restore the changes of devif_timer() back to devif_poll() under the configuration of spresense:rndi?
@anchao
Firstly, I reverted the following commit but sometimes the deadlock still happens.
```
commit 36869733f9f26866e9fd803af62b4f5ea444f13e (HEAD -> 20121601)
Author: Masayuki Ishikawa <Ma...@jp.sony.com>
Date: Fri Dec 18 12:25:36 2020 +0900
Revert "drivers/netdev: try tcp timer in every txavail call"
This reverts commit de7fd51718d85959c08402413810f7f31e7ffd03.
```
Next, I reverted this PR but sometimes the deadlock still happens.
So there might be another networking issue.
----------------------------------------------------------------
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] masayuki2009 merged pull request #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
masayuki2009 merged pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548
----------------------------------------------------------------
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] masayuki2009 commented on pull request #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747920239
> > Next, I reverted this PR but sometimes the deadlock still happens.
> > So there might be another networking issue.
>
> @anchao
> I noticed that there was a deadlock in the Spresense audio driver
> when `ioctl(pplayer->dev_fd, AUDIOIOC_STOP, 0); ` was called.
>
> So the issue does not relate to this PR. Sorry for confusing you.
Hi @anchao,
I noticed that I forgot to update the apps repository. :(
Actually, the below PR fixed the spresense:rndis issue.
https://github.com/apache/incubator-nuttx-apps/pull/522
I've been testing this PR with spresense:rndis and working well so far.
----------------------------------------------------------------
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] masayuki2009 commented on pull request #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747773261
Hi @anchao
I tested this PR with spresense:rndi configuration.
It works but found a side effect.
For example, if nxplayer starts HTTP audio streaming and then stops manually.
Sometimes, a deadlock happened.
In this case, I noticed that the FIN packet was not sent.
Could you confirm if this kind of side effect happens on your side?
Also, I had trouble with https://github.com/apache/incubator-nuttx/pull/2557
So I had to revert it locally to test this PR.
Is the PR works on your side?
----------------------------------------------------------------
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] masayuki2009 commented on pull request #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747393440
Hi @anchao,
Nice performance improvements!
However, should we apply this change to the following network drivers too? because they are using devif_poll().
- usbdev: cdcecm.c, rndis.c
- net: rpmsgdrv.c, loopback.c, slip.c, enc28j60.c, encx24j600.c lan91c111.c, tun.c, ftmac100.c, skeleton.c, dm90x0.c,
- usbhost: usbhost_cdcmbim.c
- wireless: bcm_netdev.c, spirit_netdev.c, xbee_netdev.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] masayuki2009 commented on pull request #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747867387
>Next, I reverted this PR but sometimes the deadlock still happens.
>So there might be another networking issue.
@anchao
I noticed that there was a deadlock in the Spresense audio driver
when ```ioctl(pplayer->dev_fd, AUDIOIOC_STOP, 0); ``` was called.
So the issue does not relate to this PR. Sorry for confusing you.
----------------------------------------------------------------
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] masayuki2009 commented on pull request #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
masayuki2009 commented on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747873007
Let me merge this PR.
----------------------------------------------------------------
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 #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
anchao commented on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747875907
> Let me merge this PR.
> > Next, I reverted this PR but sometimes the deadlock still happens.
> > So there might be another networking issue.
>
> @anchao
> I noticed that there was a deadlock in the Spresense audio driver
> when `ioctl(pplayer->dev_fd, AUDIOIOC_STOP, 0); ` was called.
>
> So the issue does not relate to this PR. Sorry for confusing you.
You are welcome, thanks for your rigorous work!
----------------------------------------------------------------
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] masayuki2009 edited a comment on pull request #2548: net/tcp: Optimize TCP handshake performance
Posted by GitBox <gi...@apache.org>.
masayuki2009 edited a comment on pull request #2548:
URL: https://github.com/apache/incubator-nuttx/pull/2548#issuecomment-747393440
Hi @anchao,
Nice performance improvements!
I think we should apply this change to the following network drivers too because they are using devif_poll().
What do you think?
- usbdev: cdcecm.c, rndis.c
- net: rpmsgdrv.c, loopback.c, slip.c, enc28j60.c, encx24j600.c lan91c111.c, tun.c, ftmac100.c, skeleton.c, dm90x0.c
- usbhost: usbhost_cdcmbim.c
- wireless: bcm_netdev.c, spirit_netdev.c, xbee_netdev.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