casaroli commented on code in PR #20333:
URL: https://github.com/apache/nuttx/pull/20333#discussion_r4434996514
##########
net/devif/devif_poll.c:
##########
@@ -316,7 +317,31 @@ devif_poll_can_connections(FAR struct net_driver_s *dev,
if (dev->d_len > 0)
{
- bstop = callback(dev);
+ if (!IFF_IS_LOOPBACK(dev->d_flags)
+#ifdef CONFIG_NET_CANPROTO_OPTIONS
+ && _SO_GETOPT(can_conn->sconn.s_options, CAN_RAW_LOOPBACK)
+#endif
+ )
+ {
+ FAR struct iob_s *iob = can_iob_clone(dev);
+
+ bstop = callback(dev);
+
+ if (bstop && iob != NULL)
Review Comment:
`bstop` does not tell that the frame was sent. netdev_upper_txpoll() returns
a negative errno on a transmit failure after netpkt_put() puts the frame back
in d_iob, so this code loops back a frame that was not sent and overwrites
d_iob (the frame leaks). Legacy drivers return 0 after a successful send (for
example s32k1xx_txpoll()), so they never loop back.
Fix: decide the loopback from the transmit result, not from the poll stop
flag.
##########
net/devif/devif_poll.c:
##########
@@ -316,7 +317,31 @@ devif_poll_can_connections(FAR struct net_driver_s *dev,
if (dev->d_len > 0)
{
- bstop = callback(dev);
+ if (!IFF_IS_LOOPBACK(dev->d_flags)
Review Comment:
Without CONFIG_NET_CANPROTO_OPTIONS this condition is only
`!IFF_IS_LOOPBACK()`, so every sent frame is cloned and delivered to the other
local sockets and the user cannot turn it off. The PR body says the loopback is
under CONFIG_NET_CANPROTO_OPTIONS; it is not, so the default behaviour of every
SocketCAN configuration changes.
Fix: put the loopback under CONFIG_NET_CANPROTO_OPTIONS as the body says.
##########
net/can/can_recvmsg.c:
##########
@@ -161,6 +161,11 @@ static size_t can_recvfrom_newdata(FAR struct net_driver_s
*dev,
pstate->pr_conn->sconn.s_options);
#endif
+ if (dev->d_iob->io_conn == &pstate->pr_conn->sconn)
+ {
+ pstate->pr_msg->msg_flags |= MSG_CONFIRM;
Review Comment:
MSG_CONFIRM is only set, never cleared, and psock_recvmsg() does not clear
msg_flags. An application that reuses its msghdr sees MSG_CONFIRM on every
frame after the first own frame.
Fix: clear MSG_CONFIRM in can_recvmsg() before the receive.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
