You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "SPRESENSE (via GitHub)" <gi...@apache.org> on 2023/01/31 02:49:46 UTC

[GitHub] [nuttx] SPRESENSE opened a new pull request, #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

SPRESENSE opened a new pull request, #8370:
URL: https://github.com/apache/nuttx/pull/8370

   ## Summary
   
   SOCK_CTRL is added to provide special control over network drivers and daemons. Currently, SOCK_DGRAM and SOCK_STREAM perform this control, but these use socket resources. In the case of usersocket in particular, this is a waste of the device's limited socket resources.
   
   ## Impact
   
   Applications using NET_SOCK_TYPE.
   
   ## Testing
   
   SPRESENSE LTE board.


-- 
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] SPRESENSE commented on pull request #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "SPRESENSE (via GitHub)" <gi...@apache.org>.
SPRESENSE commented on PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#issuecomment-1418888402

   > @SPRESENSE but how to pass the ioctl to usrsock server now?
   
   This fix only add a new Socket Type for requests to Daemons that do not use the device's Socket.
   Therefore, the PASS method to usrsock server will remain the same.


-- 
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] SPRESENSE commented on a diff in pull request #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "SPRESENSE (via GitHub)" <gi...@apache.org>.
SPRESENSE commented on code in PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#discussion_r1099670409


##########
include/nuttx/net/netconfig.h:
##########
@@ -104,52 +104,48 @@
 
 #define NET_SOCK_PROTOCOL  0
 
-/* SOCK_DGRAM is the preferred socket type to use when we just want a
- * socket for performing driver ioctls.  However, we can't use SOCK_DRAM
- * if UDP is disabled.
- *
- * Pick a socket type (and perhaps protocol) compatible with the currently
- * selected address family.
+/* SOCK_CTRL is the preferred socket type to use when we just want a
+ * socket for performing driver ioctls.
  */
 
 #if NET_SOCK_FAMILY == AF_INET
 #  if defined(CONFIG_NET_UDP)
-#    define NET_SOCK_TYPE SOCK_DGRAM
+#    define NET_SOCK_TYPE SOCK_CTRL

Review Comment:
   Sure, I'll do it soon.



-- 
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] SPRESENSE commented on pull request #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "SPRESENSE (via GitHub)" <gi...@apache.org>.
SPRESENSE commented on PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#issuecomment-1420546053

   Dear @xiaoxiang781216 san
   
   Sure. I updated a commit now.


-- 
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] SPRESENSE commented on pull request #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "SPRESENSE (via GitHub)" <gi...@apache.org>.
SPRESENSE commented on PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#issuecomment-1420115057

   Dear @xiaoxiang781216 san
   
   > > Dear @xiaoxiang781216 san
   > > > @SPRESENSE but how to pass the ioctl to usrsock server now?
   > > 
   > > 
   > > This fix only add a new Socket Type for requests to Daemons that do not use the device's Socket. Therefore, the PASS method to usrsock server will remain the same.
   > 
   > Ok, I saw SOCK_CTRL will convert to the real type: https://github.com/apache/nuttx/pull/8370/files#diff-098d0f88e64dc86480d735d351725fce63ff53c8b0d834ca460ba61c2eb9bb13R289-R304 So, what's the benefit to add SOCK_CTRL?
   
   The benefit of this change is that Daemon can determine not to consume Socket resources for devices that should not consume them.(e.g. One of modem as ALT1250 can use 5 internal socket only.)
   By adding the type SOCK_CTRL and specifying that type in the Socket for requests such as the ifup command or ifconfig command, Daemon will know not to open the Socket for the device. So Daemon can receive SOCK_CTRL to prevent the device from consuming Socket resources by not opening the Socket for the device.


-- 
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] masayuki2009 commented on pull request #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#issuecomment-1422617812

   Let me revert 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.

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 #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#discussion_r1099068077


##########
include/nuttx/net/netconfig.h:
##########
@@ -104,52 +104,48 @@
 
 #define NET_SOCK_PROTOCOL  0
 
-/* SOCK_DGRAM is the preferred socket type to use when we just want a
- * socket for performing driver ioctls.  However, we can't use SOCK_DRAM
- * if UDP is disabled.
- *
- * Pick a socket type (and perhaps protocol) compatible with the currently
- * selected address family.
+/* SOCK_CTRL is the preferred socket type to use when we just want a
+ * socket for performing driver ioctls.
  */
 
 #if NET_SOCK_FAMILY == AF_INET
 #  if defined(CONFIG_NET_UDP)
-#    define NET_SOCK_TYPE SOCK_DGRAM
+#    define NET_SOCK_TYPE SOCK_CTRL

Review Comment:
   let's alwasy define
   NET_SOCK_TYPE SOCK_CTRL and simplify the logic to define NET_SOCK_PROTOCOL



-- 
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] SPRESENSE commented on a diff in pull request #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "SPRESENSE (via GitHub)" <gi...@apache.org>.
SPRESENSE commented on code in PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#discussion_r1099676247


##########
include/nuttx/net/netconfig.h:
##########
@@ -104,52 +104,48 @@
 
 #define NET_SOCK_PROTOCOL  0
 
-/* SOCK_DGRAM is the preferred socket type to use when we just want a
- * socket for performing driver ioctls.  However, we can't use SOCK_DRAM
- * if UDP is disabled.
- *
- * Pick a socket type (and perhaps protocol) compatible with the currently
- * selected address family.
+/* SOCK_CTRL is the preferred socket type to use when we just want a
+ * socket for performing driver ioctls.
  */
 
 #if NET_SOCK_FAMILY == AF_INET
 #  if defined(CONFIG_NET_UDP)
-#    define NET_SOCK_TYPE SOCK_DGRAM
+#    define NET_SOCK_TYPE SOCK_CTRL

Review Comment:
   @xiaoxiang781216 san
   I updated a changes. Please review again.



-- 
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] masayuki2009 commented on pull request #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#issuecomment-1422614654

   @SPRESENSE @xiaoxiang781216 
   Hmm, l noticed that lm3s6965-ek:discover (QEMU) does not work with this PR.
   
   ```
   NuttShell (NSH) NuttX-10.4.0
   nsh> uname -a
   NuttX  10.4.0 abba05a934 Feb  8 2023 22:35:51 arm lm3s6965-ek
   nsh> ps
     PID GROUP PRI POLICY   TYPE    NPX STATE    EVENT     SIGMASK   STACK   USED  FILLED COMMAND
       0     0   0 FIFO     Kthread N-- Ready              00000000 001000 000464  46.4%  Idle Task
       1     1 224 RR       Kthread --- Waiting  Semaphore 00000000 001992 000276  13.8%  hpwork 0x20001cd0
       2     2  60 RR       Kthread --- Waiting  Semaphore 00000000 001992 000276  13.8%  lpwork 0x20001ce8
       3     3 100 RR       Task    --- Running            00000000 002000 001168  58.4%  nsh_main
       4     4 100 RR       Task    --- Waiting  Semaphore 00000000 002008 000564  28.0%  telnetd
   nsh> free
                      total       used       free    largest  nused  nfree
           Umem:      49616      16560      33056      33056     61      1
   nsh> ifconfig
                IPv4   TCP   UDP  ICMP
   Received     0000  0000  0000  0000
   Dropped      0000  0000  0000  0000
     IPv4        VHL: 0000   Frg: 0000
     Checksum   0000  0000  0000  ----
     TCP         ACK: 0000   SYN: 0000
                 RST: 0000  0000
     Type       0000  ----  ----  0000
   Sent         0000  0000  0000  0000
     Rexmit     ----  0000  ----  ----
   nsh> renew eth0
   ERROR: dhcpc_open() for 'eth0' failed
   nsh> ifconfig
                IPv4   TCP   UDP  ICMP
   Received     0000  0000  0000  0000
   Dropped      0000  0000  0000  0000
     IPv4        VHL: 0000   Frg: 0000
     Checksum   0000  0000  0000  ----
     TCP         ACK: 0000   SYN: 0000
                 RST: 0000  0000
     Type       0000  ----  ----  0000
   Sent         0000  0000  0000  0000
     Rexmit     ----  0000  ----  ----
   ```


-- 
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 pull request #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#issuecomment-1409673772

   @SPRESENSE but how to pass the ioctl to usrsock server now?


-- 
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 pull request #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#issuecomment-1420183766

   > Dear @xiaoxiang781216 san
   > 
   > > > Dear @xiaoxiang781216 san
   > > > > @SPRESENSE but how to pass the ioctl to usrsock server now?
   > > > 
   > > > 
   > > > This fix only add a new Socket Type for requests to Daemons that do not use the device's Socket. Therefore, the PASS method to usrsock server will remain the same.
   > > 
   > > 
   > > Ok, I saw SOCK_CTRL will convert to the real type: https://github.com/apache/nuttx/pull/8370/files#diff-098d0f88e64dc86480d735d351725fce63ff53c8b0d834ca460ba61c2eb9bb13R289-R304 So, what's the benefit to add SOCK_CTRL?
   > 
   > The benefit of this change is that Daemon can determine not to consume Socket resources for devices that should not consume them.(e.g. One of modem as ALT1250 can use 5 internal socket only.) By adding the type SOCK_CTRL and specifying that type in the Socket for requests such as the ifup command or ifconfig command, Daemon will know not to open the Socket for the device. So Daemon can receive SOCK_CTRL to prevent the device from consuming Socket resources by not opening the Socket for the device.
   
   Ok, so the patch tries to reduce the driver consumption, but the nework stack self. Look reasonable, thanks.


-- 
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 pull request #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#issuecomment-1418961143

   > Dear @xiaoxiang781216 san
   > 
   > > @SPRESENSE but how to pass the ioctl to usrsock server now?
   > 
   > This fix only add a new Socket Type for requests to Daemons that do not use the device's Socket. Therefore, the PASS method to usrsock server will remain the same.
   
   Ok, I saw SOCK_CTRL will convert to the real type:
   https://github.com/apache/nuttx/pull/8370/files#diff-098d0f88e64dc86480d735d351725fce63ff53c8b0d834ca460ba61c2eb9bb13R289-R304
   So, what's the benefit to add SOCK_CTRL?


-- 
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 pull request #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on PR #8370:
URL: https://github.com/apache/nuttx/pull/8370#issuecomment-1420531759

   @SPRESENSE please merge the intermediate change into the first patch.


-- 
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 #8370: include/sys/socket.h: Add SOCK_CTRL to socket type

Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #8370:
URL: https://github.com/apache/nuttx/pull/8370


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