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 2022/08/08 10:22:57 UTC
[GitHub] [incubator-nuttx] XinStellaris opened a new pull request, #6808: net/local:Make local udp send multi-thread safe
XinStellaris opened a new pull request, #6808:
URL: https://github.com/apache/incubator-nuttx/pull/6808
Signed-off-by: 田昕 <ti...@xiaomi.com>
## Summary
When multiple threads sends data through one local udp socket, data may be garbled due to lack of sem.
## Impact
local socket
## Testing
tested on esp32c3
--
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] [incubator-nuttx] XinStellaris commented on a diff in pull request #6808: net/local:Make local udp send multi-thread safe
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on code in PR #6808:
URL: https://github.com/apache/incubator-nuttx/pull/6808#discussion_r940155519
##########
net/local/local_conn.c:
##########
@@ -128,6 +128,16 @@ FAR struct local_conn_s *local_alloc(void)
nxsem_init(&conn->lc_donesem, 0, 0);
nxsem_set_protocol(&conn->lc_donesem, SEM_PRIO_NONE);
+#endif
+
+#ifdef CONFIG_NET_LOCAL_DGRAM
Review Comment:
okay, will fix that
--
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] [incubator-nuttx] XinStellaris commented on a diff in pull request #6808: net/local:Make local udp send multi-thread safe
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on code in PR #6808:
URL: https://github.com/apache/incubator-nuttx/pull/6808#discussion_r940101666
##########
net/local/local_sendmsg.c:
##########
@@ -334,6 +334,16 @@ static ssize_t local_sendto(FAR struct socket *psock,
goto errout_with_halfduplex;
}
+ /* Make sure that dgram is sent safely */
+
+ ret = nxsem_wait_uninterruptible(&conn->lc_sendsem);
+ if (ret < 0)
+ {
+ /* May fail because the task was canceled. */
+
+ return ret;
Review Comment:
ok
##########
net/local/local_conn.c:
##########
@@ -128,6 +128,16 @@ FAR struct local_conn_s *local_alloc(void)
nxsem_init(&conn->lc_donesem, 0, 0);
nxsem_set_protocol(&conn->lc_donesem, SEM_PRIO_NONE);
+#endif
+
+#ifdef CONFIG_NET_LOCAL_DGRAM
+ /* This semaphore is used for sending udp dgram safely in multithread.
+ * local DGRAM consists of preamble,len,buf. Make sure they will
+ * not be garbled when multi-thread sends.
+ */
+
+ nxsem_init(&conn->lc_sendsem, 0, 1);
Review Comment:
will fix that
##########
net/local/local_conn.c:
##########
@@ -128,6 +128,16 @@ FAR struct local_conn_s *local_alloc(void)
nxsem_init(&conn->lc_donesem, 0, 0);
nxsem_set_protocol(&conn->lc_donesem, SEM_PRIO_NONE);
+#endif
+
+#ifdef CONFIG_NET_LOCAL_DGRAM
Review Comment:
I only add lc_sendsem for udp. So if CONFIG_NET_LOCAL_DGRAM is not enabled, the sem does not exist
--
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] [incubator-nuttx] XinStellaris commented on a diff in pull request #6808: net/local:Make local udp send multi-thread safe
Posted by GitBox <gi...@apache.org>.
XinStellaris commented on code in PR #6808:
URL: https://github.com/apache/incubator-nuttx/pull/6808#discussion_r940176427
##########
net/local/local_sendmsg.c:
##########
@@ -196,7 +196,15 @@ static ssize_t local_send(FAR struct socket *psock,
/* Send the packet */
+ ret = nxsem_wait_uninterruptible(&conn->lc_sendsem);
+ if (ret < 0)
+ {
+ /* May fail because the task was canceled. */
+
+ return ret;
+ }
Review Comment:
ok
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6808: net/local:Make local udp send multi-thread safe
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6808:
URL: https://github.com/apache/incubator-nuttx/pull/6808#discussion_r940130894
##########
net/local/local_conn.c:
##########
@@ -128,6 +128,16 @@ FAR struct local_conn_s *local_alloc(void)
nxsem_init(&conn->lc_donesem, 0, 0);
nxsem_set_protocol(&conn->lc_donesem, SEM_PRIO_NONE);
+#endif
+
+#ifdef CONFIG_NET_LOCAL_DGRAM
Review Comment:
As I said TCP has the similar issue regardless whether the data is sent in one or multiple file_write.
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6808: net/local:Make local udp send multi-thread safe
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6808:
URL: https://github.com/apache/incubator-nuttx/pull/6808#discussion_r940084340
##########
net/local/local_sendmsg.c:
##########
@@ -334,6 +334,16 @@ static ssize_t local_sendto(FAR struct socket *psock,
goto errout_with_halfduplex;
}
+ /* Make sure that dgram is sent safely */
+
+ ret = nxsem_wait_uninterruptible(&conn->lc_sendsem);
+ if (ret < 0)
+ {
+ /* May fail because the task was canceled. */
+
+ return ret;
Review Comment:
goto errout_with_sender and close lc_outfile
##########
net/local/local_conn.c:
##########
@@ -128,6 +128,16 @@ FAR struct local_conn_s *local_alloc(void)
nxsem_init(&conn->lc_donesem, 0, 0);
nxsem_set_protocol(&conn->lc_donesem, SEM_PRIO_NONE);
+#endif
+
+#ifdef CONFIG_NET_LOCAL_DGRAM
Review Comment:
why check CONFIG_NET_LOCAL_DGRAM
##########
net/local/local_conn.c:
##########
@@ -128,6 +128,16 @@ FAR struct local_conn_s *local_alloc(void)
nxsem_init(&conn->lc_donesem, 0, 0);
nxsem_set_protocol(&conn->lc_donesem, SEM_PRIO_NONE);
+#endif
+
+#ifdef CONFIG_NET_LOCAL_DGRAM
+ /* This semaphore is used for sending udp dgram safely in multithread.
+ * local DGRAM consists of preamble,len,buf. Make sure they will
+ * not be garbled when multi-thread sends.
+ */
+
+ nxsem_init(&conn->lc_sendsem, 0, 1);
Review Comment:
where destroy sem?
--
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] [incubator-nuttx] xiaoxiang781216 merged pull request #6808: net/local:Make local udp send multi-thread safe
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 merged PR #6808:
URL: https://github.com/apache/incubator-nuttx/pull/6808
--
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] [incubator-nuttx] xiaoxiang781216 commented on a diff in pull request #6808: net/local:Make local udp send multi-thread safe
Posted by GitBox <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #6808:
URL: https://github.com/apache/incubator-nuttx/pull/6808#discussion_r940171364
##########
net/local/local_sendmsg.c:
##########
@@ -196,7 +196,15 @@ static ssize_t local_send(FAR struct socket *psock,
/* Send the packet */
+ ret = nxsem_wait_uninterruptible(&conn->lc_sendsem);
+ if (ret < 0)
+ {
+ /* May fail because the task was canceled. */
+
+ return ret;
+ }
Review Comment:
please fix the style problem
--
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