rndis.c: move ifup in setconfig to the work queue by zhhyu7 · Pull Request #18428 · apache/nuttx

zhhyu7 · 2026-02-24T08:19:46Z

Summary

Move network interface up operation (ifup) to work queue in RNDIS USB device driver to avoid calling netdev_carrier_on API in interrupt context.

This PR addresses a threading context issue in the RNDIS USB device driver where network interface initialization was being performed in interrupt context.

Impact

Modified usbclass_setconfig() to schedule ifup operation via work_queue() instead of calling it directly;
Reused existing pollwork work structure since the network card is not yet in RUNNING state at this point.

Testing

sim:usbdev
NuttX test log:

NuttShell (NSH)
nsh> ifconfig
eth0	Link encap:Ethernet HWaddr 42:73:ca:8b:35:04 at RUNNING mtu 1500
	inet addr:10.0.1.2 DRaddr:10.0.1.1 Mask:255.255.255.0
	inet6 addr: fe80::4073:caff:fe8b:3504/64
	inet6 DRaddr: ::

nsh> conn 0[New Thread 0x7ffff3bff640 (LWP 926963)]

conn_main: Performing architecture-specific initialization
conn_main: Exiting
nsh> [New Thread 0x7ffff33fe640 (LWP 926964)]
[New Thread 0x7ffff2bfd640 (LWP 926965)]

nsh> ifconfig
eth0	Link encap:Ethernet HWaddr 42:73:ca:8b:35:04 at RUNNING mtu 1500
	inet addr:10.0.1.2 DRaddr:10.0.1.1 Mask:255.255.255.0
	inet6 addr: fe80::4073:caff:fe8b:3504/64
	inet6 DRaddr: ::

eth1	Link encap:Ethernet HWaddr 00:00:00:00:00:00 at RUNNING mtu 1504
	inet addr:0.0.0.0 DRaddr:0.0.0.0 Mask:0.0.0.0
	inet6 addr: ::/0
	inet6 DRaddr: ::

nsh> ifconfig eth1 100.0.0.2/24
nsh> ping 100.0.0.1
PING 100.0.0.1 56 bytes of data
56 bytes from 100.0.0.1: icmp_seq=0 time=10.0 ms
56 bytes from 100.0.0.1: icmp_seq=1 time=10.0 ms
56 bytes from 100.0.0.1: icmp_seq=2 time=10.0 ms
56 bytes from 100.0.0.1: icmp_seq=3 time=10.0 ms
56 bytes from 100.0.0.1: icmp_seq=4 time=10.0 ms
56 bytes from 100.0.0.1: icmp_seq=5 time=10.0 ms
56 bytes from 100.0.0.1: icmp_seq=6 time=10.0 ms
56 bytes from 100.0.0.1: icmp_seq=7 time=10.0 ms
56 bytes from 100.0.0.1: icmp_seq=8 time=10.0 ms
56 bytes from 100.0.0.1: icmp_seq=9 time=10.0 ms
10 packets transmitted, 10 received, 0% packet loss, time 10100 ms
rtt min/avg/max/mdev = 10.000/10.000/10.000/0.000 ms
nsh>

xiaoxiang781216 · 2026-02-24T08:27:07Z

drivers/usbdev/rndis.c

-      priv->netdev.d_flags |= IFF_UP;
-    }
+
+  /* Schedule to work queue because netdev_carrier_on API can't be used in


but where rndis call netdev_carrier_on?

but where rndis call netdev_carrier_on?

@xiaoxiang781216 The call chain is as follows:
usbclass_setconfig -> rndis_ifup_work -> priv->netdev.d_ifup -> rndis_ifup -> netdev_carrier_on

it's better to run priv->netdev.d_flags |= IFF_UP here, but move notify to work queue

it's better to run priv->netdev.d_flags |= IFF_UP here, but move notify to work queue

@xiaoxiang781216 Done.

xiaoxiang781216 · 2026-02-24T08:28:26Z

drivers/usbdev/rndis.c

+ * Name: rndis_ifup_work
+ *
+ * Description:
+ *   Schedule to work queue because netdev_carrier_on API can't be used in


how about move the interrupt context check and queue wwork inisde netdev_carrier_on/netdev_carrier_off

how about move the interrupt context check and queue wwork inisde netdev_carrier_on/netdev_carrier_off

@xiaoxiang781216 Add an additional struct work specifically for netdev_carrier_on/netdev_carrier_off in the net_driver_s structure?

xiaoxiang781216 · 2026-02-24T09:45:29Z

drivers/usbdev/rndis.c

+   * reused.
+   */
+
+  work_queue(LPWORK, &priv->pollwork, rndis_carrier_on_work, priv, 0);


let's call work_queue inside rndis_ifup/rndis_ifdown instead

anchao · 2026-02-24T10:09:28Z

drivers/usbdev/rndis.c

+
+static void rndis_carrier_on_work(FAR void *arg)
+{
+  FAR struct rndis_dev_s *priv = (FAR struct rndis_dev_s *)arg;


Suggested change

FAR struct rndis_dev_s *priv = (FAR struct rndis_dev_s *)arg;

FAR struct rndis_dev_s *priv = arg;

I don't understand this PR. Is it because netlink can't be used in interrupt context?

@anchao Done. Yes, netlink_lock() will be called in netlink_device_notify, If trigger the wait of sem_wait will causes a crash.

how about change netlink_device_notify() ISR safe?

diff --git a/include/nuttx/net/netdev.h b/include/nuttx/net/netdev.h index 23a3f2f5e5..25394f48a0 100644 --- a/include/nuttx/net/netdev.h +++ b/include/nuttx/net/netdev.h @@ -535,6 +535,10 @@ struct net_driver_s struct timespec d_rxtime; #endif +#ifndef CONFIG_NETLINK_DISABLE_GETLINK + struct work_s d_nlwork; +#endif + /* Application callbacks: * * Network device event handlers are retained in a 'list' and are called diff --git a/net/netlink/netlink_route.c b/net/netlink/netlink_route.c index b8f3293a84..d264496937 100644 --- a/net/netlink/netlink_route.c +++ b/net/netlink/netlink_route.c @@ -1510,11 +1510,10 @@ ssize_t netlink_route_sendto(NETLINK_HANDLE handle, ****************************************************************************/ #ifndef CONFIG_NETLINK_DISABLE_GETLINK -void netlink_device_notify(FAR struct net_driver_s *dev) +static void netlink_device_notify_internal(FAR void *arg) { FAR struct netlink_response_s *resp; - - DEBUGASSERT(dev != NULL); + FAR struct net_driver_s *dev = arg; resp = netlink_get_device(dev, NULL); if (resp != NULL) @@ -1523,6 +1522,21 @@ void netlink_device_notify(FAR struct net_driver_s *dev) netlink_add_terminator(NULL, NULL, RTNLGRP_LINK); } } + +void netlink_device_notify(FAR struct net_driver_s *dev) +{ + DEBUGASSERT(dev != NULL); + + if (up_interrupt_context()) + { + work_queue(LPWORK, &dev->d_nlwork, + netlink_device_notify_internal, dev, 0); + } + else + { + netlink_device_notify_internal(dev); + } +} #endif /****************************************************************************

how about change netlink_device_notify() ISR safe?

@anchao Okay, if we make this change, I won't need to submit this PR. Will you directly submit this modification?

@anchao Isn't this solution more chaotic? netdev_carrier API was never intended to run in interrupt context, and all other netdevs follow this convention (or at least they should).
This PR is a bugfix for a previous PR that misused the API. The correct fix is to use the API correctly, not to change the API behavior.

@raiden00pl
My intention is to fix this issue at the most critical point. Because in RNDIS, we have modified netdev_carrier_on() to be invoked from a workqueue. If other network drivers also need to call netdev_carrier_on() in interrupt context, developers would have to make similar changes.

how about change netlink_device_notify() ISR safe?

@anchao Okay, if we make this change, I won't need to submit this PR. Will you directly submit this modification?

@zhhyu7 You may still need to submit the patch. I’m only suggesting this change, but I can’t verify it.

Move network interface up operation (ifup) to work queue in RNDIS USB device driver to avoid calling netdev_carrier_on API in interrupt context. Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>

zhhyu7 requested a review from xiaoxiang781216 as a code owner February 24, 2026 08:19

github-actions bot added Size: S The size of the change in this PR is small Area: USB labels Feb 24, 2026

zhhyu7 mentioned this pull request Feb 24, 2026

[BUG] assert in semaphore/sem_wait.c due to netdev_carrier_on called in interrupt context. #18421

Open

1 task

xiaoxiang781216 linked an issue Feb 24, 2026 that may be closed by this pull request

[BUG] assert in semaphore/sem_wait.c due to netdev_carrier_on called in interrupt context. #18421

Open

1 task

xiaoxiang781216 reviewed Feb 24, 2026

View reviewed changes

zhhyu7 force-pushed the upstream-11 branch from 536769e to 1992030 Compare February 24, 2026 09:26

simbit18 requested review from acassis, anchao, jerpelea, linguini1, michallenc and raiden00pl February 24, 2026 09:31

xiaoxiang781216 reviewed Feb 24, 2026

View reviewed changes

anchao reviewed Feb 24, 2026

View reviewed changes

rndis.c: move ifup in setconfig to the work queue

139b05d

Move network interface up operation (ifup) to work queue in RNDIS USB device driver to avoid calling netdev_carrier_on API in interrupt context. Signed-off-by: zhanghongyu <zhanghongyu@xiaomi.com>

zhhyu7 force-pushed the upstream-11 branch from 1992030 to 139b05d Compare February 24, 2026 11:14

xiaoxiang781216 approved these changes Feb 25, 2026

View reviewed changes

	FAR struct rndis_dev_s priv = (FAR struct rndis_dev_s )arg;
	FAR struct rndis_dev_s *priv = arg;

Conversation

zhhyu7 commented Feb 24, 2026

Summary

Impact

Testing

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants