You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@nuttx.apache.org by "anchao (via GitHub)" <gi...@apache.org> on 2023/01/30 14:00:45 UTC
[GitHub] [nuttx] anchao opened a new pull request, #8361: net/devif: check the net device before use
anchao opened a new pull request, #8361:
URL: https://github.com/apache/nuttx/pull/8361
## Summary
net/devif: check the net device before use
Signed-off-by: chao an <an...@xiaomi.com>
## Impact
N/A
## Testing
tcp/udp test
--
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 #8361: net/devif: check the net device before use
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #8361:
URL: https://github.com/apache/nuttx/pull/8361#issuecomment-1410201734
@anchao
I noticed that the renew command with lm3s6965-ek:discover on QEMU-7.1 does not work with this PR.
```
nsh> uname -a
NuttX 10.4.0 7fce145b30 Jan 31 2023 20:35:26 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 000472 47.2% 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 001160 58.0% nsh_main
4 4 100 RR Task --- Waiting Semaphore 00000000 002008 000564 28.0% telnetd
nsh> ifconfig
eth0 Link encap:Ethernet HWaddr 00:e0:de:ad:be:ef at UP
inet addr:10.0.0.2 DRaddr:10.0.0.1 Mask:255.255.255.0
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_request() failed
```
--
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 #8361: net/devif: check the net device before use
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 commented on code in PR #8361:
URL: https://github.com/apache/nuttx/pull/8361#discussion_r1091589847
##########
net/devif/devif_send.c:
##########
@@ -68,47 +69,50 @@
void devif_send(FAR struct net_driver_s *dev, FAR const void *buf,
int len, unsigned int offset)
{
-#ifndef CONFIG_NET_IPFRAG
- unsigned int limit = NETDEV_PKTSIZE(dev) -
- NET_LL_HDRLEN(dev) - offset;
+ int ret;
- if (dev == NULL || len == 0 || len > limit)
+ if (dev == NULL)
{
- nerr("ERROR: devif_send fail: %p, sndlen: %u, pktlen: %u\n",
- dev, len, limit);
- return;
+ ret = -ENODEV;
+ goto errout;
}
-#else
- if (dev == NULL || len == 0)
+
+ if (len == 0)
+ {
+ ret = -EINVAL;
+ goto errout;
+ }
+
+#ifndef CONFIG_NET_IPFRAG
+ if (len > NETDEV_PKTSIZE(dev) - NET_LL_HDRLEN(dev) - offset)
{
- nerr("ERROR: devif_send fail: %p, sndlen: %u\n", dev, len);
- return;
+ ret = -EMSGSIZE;
+ goto errout;
}
#endif
- /* Copy in iob to target device buffer */
+ /* Append the send buffer after device buffer */
- if (len <= iob_navail(false) * CONFIG_IOB_BUFSIZE)
+ if (len > iob_navail(false) * CONFIG_IOB_BUFSIZE &&
+ netdev_iob_prepare(dev, false, 0) != OK)
{
- /* Prepare device buffer before poll callback */
+ ret = -ENOMEM;
+ goto errout;
+ }
- if (netdev_iob_prepare(dev, false, 0) != OK)
- {
- return;
- }
+ /* Prepare device buffer before poll callback */
- iob_update_pktlen(dev->d_iob, offset);
+ iob_update_pktlen(dev->d_iob, offset);
- dev->d_sndlen = iob_trycopyin(dev->d_iob, buf, len, offset, false);
- }
- else
+ ret = iob_trycopyin(dev->d_iob, buf, len, offset, false);
+ if (ret != len)
{
- dev->d_sndlen = 0;
+ netdev_iob_release(dev);
}
- if (dev->d_sndlen != len)
Review Comment:
let's return here to avoid the check(ret < 0) after errout
##########
net/devif/devif_iobsend.c:
##########
@@ -56,67 +57,59 @@ void devif_iob_send(FAR struct net_driver_s *dev, FAR struct iob_s *iob,
unsigned int len, unsigned int offset,
unsigned int target_offset)
{
-#ifndef CONFIG_NET_IPFRAG
- unsigned int limit = NETDEV_PKTSIZE(dev) -
- NET_LL_HDRLEN(dev) - target_offset;
+ int ret;
- if (dev == NULL || len == 0 || len > limit)
-#else
- if (dev == NULL || len == 0)
-#endif
+ if (dev == NULL)
{
- if (dev->d_iob == NULL)
- {
- iob_free_chain(iob);
- }
+ ret = -ENODEV;
+ goto errout;
+ }
+
+ if (len == 0)
+ {
+ ret = -EINVAL;
+ goto errout;
+ }
#ifndef CONFIG_NET_IPFRAG
- nerr("devif_iob_send error, %p, send len: %u, limit len: %u\n",
- dev, len, limit);
-#else
- nerr("devif_iob_send error, %p, send len: %u\n", dev, len);
-#endif
- return;
+ if (len > NETDEV_PKTSIZE(dev) - NET_LL_HDRLEN(dev) - target_offset)
+ {
+ ret = -EMSGSIZE;
+ goto errout;
}
+#endif
/* Append the send buffer after device buffer */
- if (dev->d_iob != NULL)
+ if (len > iob_navail(false) * CONFIG_IOB_BUFSIZE)
{
- dev->d_sndlen = 0;
-
- if (len > iob_navail(false) * CONFIG_IOB_BUFSIZE)
- {
- return;
- }
-
- /* Clone the iob to target device buffer */
+ ret = -ENOMEM;
+ goto errout;
+ }
- if (iob_clone_partial(iob, len, offset, dev->d_iob,
- target_offset, false, false) != OK)
- {
- netdev_iob_release(dev);
- nerr("devif_iob_send error, not enough iob entries, "
- "send len: %u\n", len);
- return;
- }
+ /* Clone the iob to target device buffer */
- dev->d_sndlen = len;
- }
- else
+ ret = iob_clone_partial(iob, len, offset, dev->d_iob,
+ target_offset, false, false);
+ if (ret != OK)
{
- /* Send the iob directly if no device buffer */
-
- dev->d_iob = iob;
- dev->d_sndlen = len;
- dev->d_buf = NETLLBUF;
+ netdev_iob_release(dev);
+ goto errout;
}
+ dev->d_sndlen = len;
+
#ifdef CONFIG_NET_TCP_WRBUFFER_DUMP
/* Dump the outgoing device buffer */
lib_dumpbuffer("devif_iob_send", dev->d_appdata, len);
#endif
+
+errout:
Review Comment:
let's return here to avoid the check(ret < 0) after errout
--
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 #8361: net/devif: check the net device before use
Posted by "xiaoxiang781216 (via GitHub)" <gi...@apache.org>.
xiaoxiang781216 merged PR #8361:
URL: https://github.com/apache/nuttx/pull/8361
--
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 #8361: net/devif: check the net device before use
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on PR #8361:
URL: https://github.com/apache/nuttx/pull/8361#issuecomment-1410303196
> @anchao
>
> > Could you please share the qemu command line?
>
> ```
> /home/ishikawa/opensource/QEMU/qemu-7.1/build/arm-softmmu/qemu-system-arm -net nic,model=stellaris -net user,hostfwd=tcp::20023-:23,hostfwd=tcp::20021-:21,hostfwd=tcp::25001-:5001 -M lm3s6965evb -kernel ./nuttx -nographic
> ```
@masayuki2009 thank you, please review https://github.com/apache/nuttx/pull/8380
--
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 #8361: net/devif: check the net device before use
Posted by "masayuki2009 (via GitHub)" <gi...@apache.org>.
masayuki2009 commented on PR #8361:
URL: https://github.com/apache/nuttx/pull/8361#issuecomment-1410260377
@anchao
>Could you please share the qemu command line?
```
/home/ishikawa/opensource/QEMU/qemu-7.1/build/arm-softmmu/qemu-system-arm -net nic,model=stellaris -net user,hostfwd=tcp::20023-:23,hostfwd=tcp::20021-:21,hostfwd=tcp::25001-:5001 -M lm3s6965evb -kernel ./nuttx -nographic
```
--
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 #8361: net/devif: check the net device before use
Posted by "acassis (via GitHub)" <gi...@apache.org>.
acassis commented on code in PR #8361:
URL: https://github.com/apache/nuttx/pull/8361#discussion_r1090830195
##########
net/devif/devif_iobsend.c:
##########
@@ -56,67 +57,53 @@ void devif_iob_send(FAR struct net_driver_s *dev, FAR struct iob_s *iob,
unsigned int len, unsigned int offset,
unsigned int target_offset)
{
-#ifndef CONFIG_NET_IPFRAG
- unsigned int limit = NETDEV_PKTSIZE(dev) -
- NET_LL_HDRLEN(dev) - target_offset;
+ int ret;
- if (dev == NULL || len == 0 || len > limit)
-#else
if (dev == NULL || len == 0)
-#endif
{
- if (dev->d_iob == NULL)
- {
- iob_free_chain(iob);
- }
+ ret = -EINVAL;
Review Comment:
Case dev==NULL shouldn't it return -ENODEV instead of -EINVAL?
--
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 #8361: net/devif: check the net device before use
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on PR #8361:
URL: https://github.com/apache/nuttx/pull/8361#issuecomment-1410239540
> @anchao
>
> I noticed that the renew command with lm3s6965-ek:discover on QEMU-7.1 does not work with this PR.
Could you please share the qemu command line?
--
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 #8361: net/devif: check the net device before use
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #8361:
URL: https://github.com/apache/nuttx/pull/8361#discussion_r1091462986
##########
net/devif/devif_iobsend.c:
##########
@@ -56,67 +57,53 @@ void devif_iob_send(FAR struct net_driver_s *dev, FAR struct iob_s *iob,
unsigned int len, unsigned int offset,
unsigned int target_offset)
{
-#ifndef CONFIG_NET_IPFRAG
- unsigned int limit = NETDEV_PKTSIZE(dev) -
- NET_LL_HDRLEN(dev) - target_offset;
+ int ret;
- if (dev == NULL || len == 0 || len > limit)
-#else
if (dev == NULL || len == 0)
-#endif
{
- if (dev->d_iob == NULL)
- {
- iob_free_chain(iob);
- }
+ ret = -EINVAL;
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] anchao commented on a diff in pull request #8361: net/devif: check the net device before use
Posted by "anchao (via GitHub)" <gi...@apache.org>.
anchao commented on code in PR #8361:
URL: https://github.com/apache/nuttx/pull/8361#discussion_r1091591820
##########
net/devif/devif_send.c:
##########
@@ -68,47 +69,50 @@
void devif_send(FAR struct net_driver_s *dev, FAR const void *buf,
int len, unsigned int offset)
{
-#ifndef CONFIG_NET_IPFRAG
- unsigned int limit = NETDEV_PKTSIZE(dev) -
- NET_LL_HDRLEN(dev) - offset;
+ int ret;
- if (dev == NULL || len == 0 || len > limit)
+ if (dev == NULL)
{
- nerr("ERROR: devif_send fail: %p, sndlen: %u, pktlen: %u\n",
- dev, len, limit);
- return;
+ ret = -ENODEV;
+ goto errout;
}
-#else
- if (dev == NULL || len == 0)
+
+ if (len == 0)
+ {
+ ret = -EINVAL;
+ goto errout;
+ }
+
+#ifndef CONFIG_NET_IPFRAG
+ if (len > NETDEV_PKTSIZE(dev) - NET_LL_HDRLEN(dev) - offset)
{
- nerr("ERROR: devif_send fail: %p, sndlen: %u\n", dev, len);
- return;
+ ret = -EMSGSIZE;
+ goto errout;
}
#endif
- /* Copy in iob to target device buffer */
+ /* Append the send buffer after device buffer */
- if (len <= iob_navail(false) * CONFIG_IOB_BUFSIZE)
+ if (len > iob_navail(false) * CONFIG_IOB_BUFSIZE &&
+ netdev_iob_prepare(dev, false, 0) != OK)
{
- /* Prepare device buffer before poll callback */
+ ret = -ENOMEM;
+ goto errout;
+ }
- if (netdev_iob_prepare(dev, false, 0) != OK)
- {
- return;
- }
+ /* Prepare device buffer before poll callback */
- iob_update_pktlen(dev->d_iob, offset);
+ iob_update_pktlen(dev->d_iob, offset);
- dev->d_sndlen = iob_trycopyin(dev->d_iob, buf, len, offset, false);
- }
- else
+ ret = iob_trycopyin(dev->d_iob, buf, len, offset, false);
+ if (ret != len)
{
- dev->d_sndlen = 0;
+ netdev_iob_release(dev);
}
- if (dev->d_sndlen != len)
Review Comment:
Done
##########
net/devif/devif_iobsend.c:
##########
@@ -56,67 +57,59 @@ void devif_iob_send(FAR struct net_driver_s *dev, FAR struct iob_s *iob,
unsigned int len, unsigned int offset,
unsigned int target_offset)
{
-#ifndef CONFIG_NET_IPFRAG
- unsigned int limit = NETDEV_PKTSIZE(dev) -
- NET_LL_HDRLEN(dev) - target_offset;
+ int ret;
- if (dev == NULL || len == 0 || len > limit)
-#else
- if (dev == NULL || len == 0)
-#endif
+ if (dev == NULL)
{
- if (dev->d_iob == NULL)
- {
- iob_free_chain(iob);
- }
+ ret = -ENODEV;
+ goto errout;
+ }
+
+ if (len == 0)
+ {
+ ret = -EINVAL;
+ goto errout;
+ }
#ifndef CONFIG_NET_IPFRAG
- nerr("devif_iob_send error, %p, send len: %u, limit len: %u\n",
- dev, len, limit);
-#else
- nerr("devif_iob_send error, %p, send len: %u\n", dev, len);
-#endif
- return;
+ if (len > NETDEV_PKTSIZE(dev) - NET_LL_HDRLEN(dev) - target_offset)
+ {
+ ret = -EMSGSIZE;
+ goto errout;
}
+#endif
/* Append the send buffer after device buffer */
- if (dev->d_iob != NULL)
+ if (len > iob_navail(false) * CONFIG_IOB_BUFSIZE)
{
- dev->d_sndlen = 0;
-
- if (len > iob_navail(false) * CONFIG_IOB_BUFSIZE)
- {
- return;
- }
-
- /* Clone the iob to target device buffer */
+ ret = -ENOMEM;
+ goto errout;
+ }
- if (iob_clone_partial(iob, len, offset, dev->d_iob,
- target_offset, false, false) != OK)
- {
- netdev_iob_release(dev);
- nerr("devif_iob_send error, not enough iob entries, "
- "send len: %u\n", len);
- return;
- }
+ /* Clone the iob to target device buffer */
- dev->d_sndlen = len;
- }
- else
+ ret = iob_clone_partial(iob, len, offset, dev->d_iob,
+ target_offset, false, false);
+ if (ret != OK)
{
- /* Send the iob directly if no device buffer */
-
- dev->d_iob = iob;
- dev->d_sndlen = len;
- dev->d_buf = NETLLBUF;
+ netdev_iob_release(dev);
+ goto errout;
}
+ dev->d_sndlen = len;
+
#ifdef CONFIG_NET_TCP_WRBUFFER_DUMP
/* Dump the outgoing device buffer */
lib_dumpbuffer("devif_iob_send", dev->d_appdata, len);
#endif
+
+errout:
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