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 2023/01/16 04:28:47 UTC
[GitHub] [nuttx] anchao opened a new pull request, #8129: net/tcp: correct behavior of SO_LINGER
anchao opened a new pull request, #8129:
URL: https://github.com/apache/nuttx/pull/8129
## Summary
net/tcp: correct behavior of SO_LINGER
1. Remove tcp_txdrain() from close() to avoid indefinitely block
2. Send TCP_RST immediately if linger timeout
Signed-off-by: chao an <an...@xiaomi.com>
## Impact
N/A
## Testing
enable CONFIG_NET_SOLINGER
iperf tcp client
```
struct linger ling;
ling.l_onoff = 1;
ling.l_linger = 0; /* timeout is seconds */
if (setsockopt(sockfd, SOL_SOCKET, SO_LINGER,
&ling, sizeof(struct linger)) < 0)
{
printf("server: setsockopt SO_LINGER failure: %d\n", errno);
return -1;
}
```
--
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
[GitHub] [nuttx] acassis commented on a diff in pull request #8129: net/tcp: correct behavior of SO_LINGER
Posted by GitBox <gi...@apache.org>.
acassis commented on code in PR #8129:
URL: https://github.com/apache/nuttx/pull/8129#discussion_r1071347684
##########
net/socket/Kconfig:
##########
@@ -43,7 +43,6 @@ config NET_SOLINGER
bool "SO_LINGER socket option"
default n
depends on NET_TCP_WRITE_BUFFERS || NET_UDP_WRITE_BUFFERS
- select NET_TCP_NOTIFIER if NET_TCP
select NET_UDP_NOTIFIER if NET_UDP
Review Comment:
Why didn't NET_TCP_NOTIFIER need to be selected?
Could same be applied to NET_UDP_NOTIFIER later?
--
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
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8129: net/tcp: correct behavior of SO_LINGER
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8129:
URL: https://github.com/apache/nuttx/pull/8129#discussion_r1071482235
##########
net/tcp/tcp_timer.c:
##########
@@ -352,6 +352,31 @@ void tcp_timer(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn)
return;
}
+#ifdef CONFIG_NET_SOLINGER
+ /* Send reset immediately if linger timeout */
+
+ if (_SO_GETOPT(conn->sconn.s_options, SO_LINGER) &&
+ conn->ltimeout != 0 && conn->ltimeout <= clock_systime_ticks())
Review Comment:
(socktimeo_t)(conn->ltimeout - clock_systime_ticks()) <= 0
##########
net/tcp/tcp_timer.c:
##########
@@ -702,6 +727,29 @@ void tcp_timer(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn)
dev->d_len = 0;
done:
+#ifdef CONFIG_NET_SOLINGER
+ /* Re-update tcp timeout */
+
+ if (_SO_GETOPT(conn->sconn.s_options, SO_LINGER) &&
Review Comment:
move into tcp_update_timer
##########
net/tcp/tcp.h:
##########
@@ -233,7 +233,9 @@ struct tcp_conn_s
uint16_t tx_unacked; /* Number bytes sent but not yet ACKed */
#endif
uint16_t flags; /* Flags of TCP-specific options */
-
+#ifdef CONFIG_NET_SOLINGER
+ sclock_t ltimeout; /* Linger timeout expiration */
Review Comment:
Let's reuse s_linger as the absolute timeout?
##########
net/tcp/tcp_close.c:
##########
@@ -318,6 +289,35 @@ static inline int tcp_close_disconnect(FAR struct socket *psock)
conn->clscb->event = tcp_close_eventhandler;
conn->clscb->priv = conn; /* reference for event handler to free cb */
+#ifdef CONFIG_NET_SOLINGER
+ /* SO_LINGER
+ * Lingers on a close() if data is present. This option controls the
+ * action taken when unsent messages queue on a socket and close() is
+ * performed. If SO_LINGER is set, the system shall block the calling
+ * thread during close() until it can transmit the data or until the
+ * time expires. If SO_LINGER is not specified, and close() is
+ * issued, the system handles the call in a way that allows the
+ * calling thread to continue as quickly as possible. This option
+ * takes a linger structure, as defined in the <sys/socket.h> header,
+ * to specify the state of the option and linger interval.
+ */
+
+ if (_SO_GETOPT(conn->sconn.s_options, SO_LINGER))
+ {
+ sclock_t expire = DSEC2TICK(conn->sconn.s_linger);
+
+ conn->ltimeout = clock_systime_ticks() + expire;
+
+ /* Update RTO timeout if the work exceeds expire */
+
+ if (work_available(&conn->work) ||
Review Comment:
let's call tcp_update_timer
--
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
[GitHub] [nuttx] anchao commented on a diff in pull request #8129: net/tcp: correct behavior of SO_LINGER
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #8129:
URL: https://github.com/apache/nuttx/pull/8129#discussion_r1071731272
##########
net/socket/Kconfig:
##########
@@ -43,7 +43,6 @@ config NET_SOLINGER
bool "SO_LINGER socket option"
default n
depends on NET_TCP_WRITE_BUFFERS || NET_UDP_WRITE_BUFFERS
- select NET_TCP_NOTIFIER if NET_TCP
select NET_UDP_NOTIFIER if NET_UDP
Review Comment:
@acassis ,
now all TCP data transmission is carried out by tcp_timer(), so we can delete the dependence on NET_TCP_NOTIFIER to avoid blocking for user thread.
UDP data transmission does not need to wait for the reply of the ACK confirmation as long as the data can be sent successfully, so we keep the logic of NET_UDP_NOTIFIER(txdrain) when udp close
--
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
[GitHub] [nuttx] anchao commented on a diff in pull request #8129: net/tcp: correct behavior of SO_LINGER
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #8129:
URL: https://github.com/apache/nuttx/pull/8129#discussion_r1071795377
##########
net/tcp/tcp.h:
##########
@@ -233,7 +233,9 @@ struct tcp_conn_s
uint16_t tx_unacked; /* Number bytes sent but not yet ACKed */
#endif
uint16_t flags; /* Flags of TCP-specific options */
-
+#ifdef CONFIG_NET_SOLINGER
+ sclock_t ltimeout; /* Linger timeout expiration */
Review Comment:
Done
##########
net/tcp/tcp_close.c:
##########
@@ -318,6 +289,35 @@ static inline int tcp_close_disconnect(FAR struct socket *psock)
conn->clscb->event = tcp_close_eventhandler;
conn->clscb->priv = conn; /* reference for event handler to free cb */
+#ifdef CONFIG_NET_SOLINGER
+ /* SO_LINGER
+ * Lingers on a close() if data is present. This option controls the
+ * action taken when unsent messages queue on a socket and close() is
+ * performed. If SO_LINGER is set, the system shall block the calling
+ * thread during close() until it can transmit the data or until the
+ * time expires. If SO_LINGER is not specified, and close() is
+ * issued, the system handles the call in a way that allows the
+ * calling thread to continue as quickly as possible. This option
+ * takes a linger structure, as defined in the <sys/socket.h> header,
+ * to specify the state of the option and linger interval.
+ */
+
+ if (_SO_GETOPT(conn->sconn.s_options, SO_LINGER))
+ {
+ sclock_t expire = DSEC2TICK(conn->sconn.s_linger);
+
+ conn->ltimeout = clock_systime_ticks() + expire;
+
+ /* Update RTO timeout if the work exceeds expire */
+
+ if (work_available(&conn->work) ||
Review Comment:
Done
##########
net/tcp/tcp_timer.c:
##########
@@ -352,6 +352,31 @@ void tcp_timer(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn)
return;
}
+#ifdef CONFIG_NET_SOLINGER
+ /* Send reset immediately if linger timeout */
+
+ if (_SO_GETOPT(conn->sconn.s_options, SO_LINGER) &&
+ conn->ltimeout != 0 && conn->ltimeout <= clock_systime_ticks())
Review Comment:
Done
--
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
[GitHub] [nuttx] xiaoxiang781216 commented on a diff in pull request #8129: net/tcp: correct behavior of SO_LINGER
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8129:
URL: https://github.com/apache/nuttx/pull/8129#discussion_r1072273342
##########
net/udp/udp_close.c:
##########
@@ -85,7 +85,8 @@ int udp_close(FAR struct socket *psock)
if (_SO_GETOPT(conn->sconn.s_options, SO_LINGER))
{
- timeout = _SO_TIMEOUT(conn->sconn.s_linger);
+ timeout = (conn->sconn.s_linger == 0) ? 0 :
Review Comment:
merge to previous patch
##########
net/tcp/tcp_timer.c:
##########
@@ -702,6 +748,7 @@ void tcp_timer(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn)
dev->d_len = 0;
done:
+
Review Comment:
revert
##########
net/tcp/tcp_close.c:
##########
@@ -318,6 +289,35 @@ static inline int tcp_close_disconnect(FAR struct socket *psock)
conn->clscb->event = tcp_close_eventhandler;
conn->clscb->priv = conn; /* reference for event handler to free cb */
+#ifdef CONFIG_NET_SOLINGER
+ /* SO_LINGER
+ * Lingers on a close() if data is present. This option controls the
+ * action taken when unsent messages queue on a socket and close() is
+ * performed. If SO_LINGER is set, the system shall block the calling
+ * thread during close() until it can transmit the data or until the
+ * time expires. If SO_LINGER is not specified, and close() is
+ * issued, the system handles the call in a way that allows the
+ * calling thread to continue as quickly as possible. This option
+ * takes a linger structure, as defined in the <sys/socket.h> header,
+ * to specify the state of the option and linger interval.
+ */
+
+ if (_SO_GETOPT(conn->sconn.s_options, SO_LINGER))
+ {
+ sclock_t expire = DSEC2TICK(conn->sconn.s_linger);
+
+ conn->ltimeout = clock_systime_ticks() + expire;
Review Comment:
tcp_update_timer
##########
net/tcp/tcp_timer.c:
##########
@@ -352,6 +372,32 @@ void tcp_timer(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn)
return;
}
+#ifdef CONFIG_NET_SOLINGER
+ /* Send reset immediately if linger timeout */
+
+ if (_SO_GETOPT(conn->sconn.s_options, SO_LINGER) &&
Review Comment:
remove the check
##########
net/tcp/tcp_timer.c:
##########
@@ -180,7 +180,27 @@ static void tcp_update_timer(FAR struct tcp_conn_s *conn)
if (timeout > 0)
{
- if (TICK2HSEC(work_timeleft(&conn->work)) != timeout)
+#ifdef CONFIG_NET_SOLINGER
+ /* Re-update tcp timeout */
+
+ if (_SO_GETOPT(conn->sconn.s_options, SO_LINGER) &&
Review Comment:
remove
##########
net/tcp/tcp_timer.c:
##########
@@ -352,6 +372,32 @@ void tcp_timer(FAR struct net_driver_s *dev, FAR struct tcp_conn_s *conn)
return;
}
+#ifdef CONFIG_NET_SOLINGER
+ /* Send reset immediately if linger timeout */
+
+ if (_SO_GETOPT(conn->sconn.s_options, SO_LINGER) &&
+ conn->ltimeout != 0 &&
+ ((int)(conn->ltimeout - clock_systime_ticks()) <= 0))
Review Comment:
int->sclock_t
--
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
[GitHub] [nuttx] anchao commented on a diff in pull request #8129: net/tcp: correct behavior of SO_LINGER
Posted by GitBox <gi...@apache.org>.
anchao commented on code in PR #8129:
URL: https://github.com/apache/nuttx/pull/8129#discussion_r1071796741
##########
net/socket/Kconfig:
##########
@@ -43,7 +43,6 @@ config NET_SOLINGER
bool "SO_LINGER socket option"
default n
depends on NET_TCP_WRITE_BUFFERS || NET_UDP_WRITE_BUFFERS
- select NET_TCP_NOTIFIER if NET_TCP
select NET_UDP_NOTIFIER if NET_UDP
Review Comment:
@acassis
In this PR I also solved the indefinitely block issue in udp_close, please review this change
https://github.com/apache/nuttx/pull/8129/commits/cb2671f34ee70013b13ecdafb382ebcadb96d551
net/udp: correct linger timeout
UDP linger timeout will be wrongly converted to UINT_MAX by _SO_TIMEOUT() when it is set to 0,
net/socket/socket.h:
```
|
| # define _SO_TIMEOUT(t) ((t) ? (t) * MSEC_PER_DSEC : UINT_MAX)
```
net/udp/udp_close.c:
```
|
| if (_SO_GETOPT(conn->sconn.s_options, SO_LINGER))
| {
| timeout = _SO_TIMEOUT(conn->sconn.s_linger);
| }
```
this change will correct this behavior, if the linger is set to 0, the timeout value should be 0
Signed-off-by: chao an <an...@xiaomi.com>
--
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
[GitHub] [nuttx] xiaoxiang781216 merged pull request #8129: net/tcp: correct behavior of SO_LINGER
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #8129:
URL: https://github.com/apache/nuttx/pull/8129
--
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
[GitHub] [nuttx] fjpanag commented on pull request #8129: net/tcp: correct behavior of SO_LINGER
Posted by "fjpanag (via GitHub)" <gi...@apache.org>.
fjpanag commented on PR #8129:
URL: https://github.com/apache/nuttx/pull/8129#issuecomment-1419076671
I just had the time to properly test this.
It seems to be working nicely, thank 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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [nuttx] yamt commented on pull request #8129: net/tcp: correct behavior of SO_LINGER
Posted by "yamt (via GitHub)" <gi...@apache.org>.
yamt commented on PR #8129:
URL: https://github.com/apache/nuttx/pull/8129#issuecomment-1447812720
> 1. Remove tcp_txdrain() from close() to avoid indefinitely block
after this change, where do we block close() for SO_LINGER?
--
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
[GitHub] [nuttx] anchao commented on pull request #8129: net/tcp: correct behavior of SO_LINGER
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on PR #8129:
URL: https://github.com/apache/nuttx/pull/8129#issuecomment-1448714017
If SO_LINGER is enabled, tcp_close will not release the conn instance immediately, all pending sent data will wait for tcp_timer() to be sent before releasing it in tcp_close_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.
To unsubscribe, e-mail: commits-unsubscribe@nuttx.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org