[PATCH net] net: qrtr: ns: retry service announcements on -EAGAIN

[PATCH net] net: qrtr: ns: retry service announcements on -EAGAIN

[Joonhoe Kim]

Since commit 544d85de4dc2 ("net: qrtr: Send HELLO message on endpoint
register"), qrtr_node_enqueue() returns -EAGAIN for every packet but
HELLO until our HELLO has gone out to the node. The HELLO is retried,
but the NEW_SERVER announcements sent in reply to the remote's HELLO
are not, so a service registered before the remote came up is never
announced to it.

On Kaanapali the modem sends its HELLO before it accepts ours. With
rmtfs started first, the modem never learns about rmtfs and its
watchdog resets the SoC about 40 s later.

Retry each announcement for up to 10 s while it fails with -EAGAIN.
announce_servers() runs from the name service worker, which can sleep.

Fixes: 544d85de4dc2 ("net: qrtr: Send HELLO message on endpoint register")
Cc: stable@vger.kernel.org
Signed-off-by: Joonhoe Kim <26rote@gmail.com>
---
Tested on a Lenovo Legion Tab Y700 Gen 5 (out-of-tree board support)
with rmtfs started before the modem: without this patch the SoC resets;
with it the announcement goes out on the first retry and the modem
stays up. Starting rmtfs after the modem avoids the problem, but
distribution units start rmtfs first and let it boot the modem.

 net/qrtr/ns.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
index bcb090ee79d4..33a9c4449d6f 100644
--- a/net/qrtr/ns.c
+++ b/net/qrtr/ns.c
@@ -5,6 +5,7 @@
  * Copyright (c) 2020, Linaro Ltd.
  */
 
+#include <linux/delay.h>
 #include <linux/module.h>
 #include <linux/qrtr.h>
 #include <linux/workqueue.h>
@@ -226,7 +227,13 @@ static int announce_servers(struct sockaddr_qrtr *sq)
 
 	/* Announce the list of servers registered in this node */
 	xa_for_each(&node->servers, index, srv) {
+		unsigned long timeout = jiffies + 10 * HZ;
+
 		ret = service_announce_new(sq, srv);
+		while (ret == -EAGAIN && time_before(jiffies, timeout)) {
+			msleep(100);
+			ret = service_announce_new(sq, srv);
+		}
 		if (ret < 0) {
 			if (ret == -ENODEV)
 				continue;
-- 
2.43.0

Re: [PATCH net] net: qrtr: ns: retry service announcements on -EAGAIN

[Konrad Dybcio]

On 10/16/26 2:45 PM, Joonhoe Kim wrote:
> Since commit 544d85de4dc2 ("net: qrtr: Send HELLO message on endpoint
> register"), qrtr_node_enqueue() returns -EAGAIN for every packet but
> HELLO until our HELLO has gone out to the node. The HELLO is retried,
> but the NEW_SERVER announcements sent in reply to the remote's HELLO
> are not, so a service registered before the remote came up is never
> announced to it.
> 
> On Kaanapali the modem sends its HELLO before it accepts ours. With
> rmtfs started first, the modem never learns about rmtfs and its
> watchdog resets the SoC about 40 s later.

Please remember about Assisted-by tags if they apply in your case

Konrad

Re: [PATCH net] net: qrtr: ns: retry service announcements on -EAGAIN

[Joonhoe Kim]

On 10/16/26 3:14 PM, Konrad Dybcio wrote:
> Please remember about Assisted-by tags if they apply in your case

It does apply: I used an LLM for the analysis and for writing the
patch. I left the tag off after "docs: remove Assisted-by references"
was queued in docs-next, but I can add it back in v2 or note it below
the "---", whichever you prefer.

Thanks,
Joonhoe Kim

Re: [PATCH net] net: qrtr: ns: retry service announcements on -EAGAIN

[Konrad Dybcio]

On 10/16/26 3:20 PM, Joonhoe Kim wrote:
> On 10/16/26 3:14 PM, Konrad Dybcio wrote:
>> Please remember about Assisted-by tags if they apply in your case
> 
> It does apply: I used an LLM for the analysis and for writing the
> patch. I left the tag off after "docs: remove Assisted-by references"
> was queued in docs-next, but I can add it back in v2 or note it below
> the "---", whichever you prefer.

OK so we went through with removing that, didn't notice. No worries then!

Konrad

Re: [PATCH net] net: qrtr: ns: retry service announcements on -EAGAIN

[Manivannan Sadhasivam]

On Fri, Oct 16, 2026 at 09:45:59PM +0900, Joonhoe Kim wrote:
> Since commit 544d85de4dc2 ("net: qrtr: Send HELLO message on endpoint
> register"), qrtr_node_enqueue() returns -EAGAIN for every packet but
> HELLO until our HELLO has gone out to the node. The HELLO is retried,
> but the NEW_SERVER announcements sent in reply to the remote's HELLO
> are not, so a service registered before the remote came up is never
> announced to it.
> 
> On Kaanapali the modem sends its HELLO before it accepts ours. With
> rmtfs started first, the modem never learns about rmtfs and its
> watchdog resets the SoC about 40 s later.
> 
> Retry each announcement for up to 10 s while it fails with -EAGAIN.
> announce_servers() runs from the name service worker, which can sleep.
> 
> Fixes: 544d85de4dc2 ("net: qrtr: Send HELLO message on endpoint register")
> Cc: stable@vger.kernel.org
> Signed-off-by: Joonhoe Kim <26rote@gmail.com>

Thanks for the fix!

> ---
> Tested on a Lenovo Legion Tab Y700 Gen 5 (out-of-tree board support)
> with rmtfs started before the modem: without this patch the SoC resets;
> with it the announcement goes out on the first retry and the modem
> stays up. Starting rmtfs after the modem avoids the problem, but
> distribution units start rmtfs first and let it boot the modem.
> 
>  net/qrtr/ns.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
> index bcb090ee79d4..33a9c4449d6f 100644
> --- a/net/qrtr/ns.c
> +++ b/net/qrtr/ns.c
> @@ -5,6 +5,7 @@
>   * Copyright (c) 2020, Linaro Ltd.
>   */
>  
> +#include <linux/delay.h>
>  #include <linux/module.h>
>  #include <linux/qrtr.h>
>  #include <linux/workqueue.h>
> @@ -226,7 +227,13 @@ static int announce_servers(struct sockaddr_qrtr *sq)
>  
>  	/* Announce the list of servers registered in this node */
>  	xa_for_each(&node->servers, index, srv) {
> +		unsigned long timeout = jiffies + 10 * HZ;
> +
>  		ret = service_announce_new(sq, srv);
> +		while (ret == -EAGAIN && time_before(jiffies, timeout)) {
> +			msleep(100);
> +			ret = service_announce_new(sq, srv);
> +		}

This loop is going to block other nodes. announce_servers() is called from
ordered workqueue which only executes atmost one work item at any point of time.
So if any other node queues command while this loop is running, it will get
blocked.

What you can do is, ask your LLM to rework this code to schedule a new *delayed*
work in the same qrts_ns.workqueue and just keep on rescheduling it until
-EAGAIN is not received from announce_servers(). While the new work item gets
scheduled on the same workqueue, the delayed work gives other node's a chance
to execute and not just block. You can use a conservative delay of 100ms.

- Mani

-- 
மணிவண்ணன் சதாசிவம்

Re: [PATCH net] net: qrtr: ns: retry service announcements on -EAGAIN

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 3 potential
issue(s) to consider.

Critical: 0 · High: 1 · Medium: 2 (1) · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] The fix only covers one of the two paths that send NEW_SERVER.
- [Medium] The new retry loop in announce_servers() (ns.c:232-236) can
  stall the whole name service for about 10 s per remote HELLO.
- [Medium] The NS side gives up after a fixed 10 s, but the core keeps
  retrying our HELLO with no limit.

Pre-existing issues:
- [Medium] A retry can re-advertise a service that was closed during the
  retry window, and the remote then keeps the stale entry.

--

net: qrtr: ns: retry service announcements on -EAGAIN

This patch makes announce_servers() retry service_announce_new() every
100 ms, for up to 10 s per server, while it returns -EAGAIN. The goal is
that NEW_SERVER replies to a remote's HELLO are not lost before our own
HELLO has gone out. It targets a modem watchdog reset on Kaanapali when
rmtfs registers before the modem comes up.

> diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
> index bcb090ee79d49..33a9c4449d6f8 100644
> --- a/net/qrtr/ns.c
> +++ b/net/qrtr/ns.c

[ ... ]

> @@ -226,7 +227,13 @@ static int announce_servers(struct sockaddr_qrtr *sq)
>  
>  	/* Announce the list of servers registered in this node */
>  	xa_for_each(&node->servers, index, srv) {
> +		unsigned long timeout = jiffies + 10 * HZ;
> +
>  		ret = service_announce_new(sq, srv);

[Severity: High]

This retries the unicast NEW_SERVER replies sent from announce_servers().
Can the broadcast path still lose announcements at the same HELLO gate?

Take a local service that registers after the remote's HELLO has been
handled, but before our HELLO has gone out. It is announced from
ctrl_cmd_new_server():

net/qrtr/ns.c:ctrl_cmd_new_server() {
    ...
	if (srv->node == qrtr_ns.local_node) {
		ret = service_announce_new(&qrtr_ns.bcast_sq, srv);
    ...
}

That reaches qrtr_bcast_enqueue(). It skips only nodes that have no nid
yet, and it ignores the return value of qrtr_node_enqueue():

net/qrtr/af_qrtr.c:qrtr_bcast_enqueue() {
    ...
		if (READ_ONCE(node->nid) == QRTR_EP_NID_AUTO)
			continue;
    ...
		qrtr_node_enqueue(node, skbn, type, from, to);
	}
    ...
	return 0;
}

qrtr_endpoint_post()->qrtr_node_assign() sets the nid when the remote's
first packet (its HELLO) arrives. From then on the node receives
broadcasts, even while hello_sent is still false. qrtr_node_enqueue()
then frees the NEW_SERVER and returns -EAGAIN:

	if (!node->hello_sent && type != QRTR_TYPE_HELLO) {
		mutex_unlock(&node->ep_lock);
		kfree_skb(skb);
		return -EAGAIN;
	}

ctrl_cmd_new_server() sees success. If node->servers was empty when the
remote's HELLO was handled, announce_servers() had nothing to retry.
Nothing calls announce_servers() again once hello_sent becomes true.

The same window opens again after qrtr_endpoint_hello() clears hello_sent
on resume, because the nid stays assigned. DEL_SERVER broadcasts from
server_del() are dropped the same way.

In that case, wouldn't rmtfs (or tqftpserv, pd-mapper) stay unannounced,
leading to the modem watchdog reset described in the commit message?

Could this be handled in the core instead of by polling in one caller?
For example, packets could be queued in qrtr_node_enqueue(), or the
announcements replayed once hello_sent becomes true.

> +		while (ret == -EAGAIN && time_before(jiffies, timeout)) {
> +			msleep(100);

[Severity: Medium]

Can this stall the whole name service for up to 10 s per remote HELLO?

announce_servers() is called from ctrl_cmd_hello() inside
qrtr_ns_worker(). That worker is the only reader of the NS socket, and
it runs as a single work item on an ordered workqueue:

	qrtr_ns.workqueue = alloc_ordered_workqueue("qrtr_ns_handler", 0);

While this loop sleeps, everything else waits behind it: local
NEW_SERVER and NEW_LOOKUP, plus the BYE, DEL_CLIENT and DEL_SERVER
cleanup for every other node and port. Stalls caused by several remotes
add up one after another.

hello_sent can stay false for a long time, or for good. When the remote
rejects an intent request, qcom_glink_request_intent() returns -EAGAIN.
qrtr_node_enqueue() reschedules say_hello only on -EAGAIN:

	if (rc == -EAGAIN && type == QRTR_TYPE_HELLO)
		schedule_delayed_work(&node->say_hello, msecs_to_jiffies(100));

A HELLO that fails with -ETIMEDOUT or -ENOMEM leaves hello_sent false
for good. After that, every later HELLO from that node costs the full
10 s.

Control packets keep arriving through sock_queue_rcv_skb() while the
worker sleeps, from both qrtr_endpoint_post() and qrtr_local_enqueue().
Once sk_rcvbuf is full they are dropped, which can leave stale lookup
and server state. qrtr_ns_remove() also has to wait in
cancel_work_sync() for the sleep to finish.

The commit message only says the worker "can sleep". Could the retry be
moved out of the shared worker, or at least could the commit message
describe these effects?

> +			ret = service_announce_new(sq, srv);

[Severity: Medium]

This is a pre-existing issue, but the retry makes the window wider. Can
this re-advertise a service whose port was closed while the loop was
sleeping?

Sequence:

1. announce_servers() is sleeping in msleep(100), with srv pointing at
   local server S on port P.
2. qrtr_hello_work() succeeds and hello_sent becomes true.
3. Port P is closed. qrtr_port_remove() broadcasts DEL_CLIENT to the
   remote through qrtr_bcast_enqueue(), which now gets through. It also
   queues a local copy for the NS.
4. The NS wakes up and sends NEW_SERVER for P to the remote.
5. Later the NS handles the local DEL_CLIENT. ctrl_cmd_del_client()
   calls server_del(node, port, false), which sends no DEL_SERVER.

The remote is left with an entry for a closed service. This is not a
use-after-free, because only the NS worker frees entries in
node->servers.

The same ordering race already exists whenever the NS worker handles a
remote HELLO before a queued local DEL_CLIENT. The retry adds a window
of up to 100 ms right after hello_sent becomes true.

> +		}
>  		if (ret < 0) {
>  			if (ret == -ENODEV)
>  				continue;

[Severity: Medium]

What happens if the remote accepts our HELLO only after the 10 s
deadline?

When the deadline passes, ret is still -EAGAIN. announce_servers() then
prints "failed to announce new service" and returns, skipping the
remaining servers. qrtr_ns_worker() only logs the error.

The core keeps retrying HELLO every 100 ms through qrtr_hello_work(),
with no limit. So hello_sent can become true after the NS has already
given up:

		if (!rc && type == QRTR_TYPE_HELLO)
			node->hello_sent = true;

At that point nothing in ns.c or af_qrtr.c calls announce_servers()
again, and the remote's HELLO has already been handled. Wouldn't every
service registered before then stay unannounced? That is the same rmtfs
failure this patch is meant to fix.

On glink the firmware controls this delay. qcom_glink_request_intent()
can wait up to 10 * HZ for each intent request while the HELLO xmit
holds node->ep_lock.

The first service_announce_new() blocks on that mutex at the hello_sent
check in qrtr_node_enqueue(). The deadline was set before that call, so
it keeps running, and one slow intent round trip can use up the whole
window. The window also opens again after every qrtr_endpoint_hello()
on resume.

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261016124600.31-1-26rote%40gmail.com

Re: [PATCH net] net: qrtr: ns: retry service announcements on -EAGAIN

[joonhoekim]

From: Joonhoe Kim <26rote@gmail.com>

On Sat, Oct 17, 2026 at 09:24:38AM +0530, Manivannan Sadhasivam wrote:
> This loop is going to block other nodes. announce_servers() is called from
> ordered workqueue which only executes atmost one work item at any point of time.
> So if any other node queues command while this loop is running, it will get
> blocked.

Right, the loop holds the ordered workqueue for up to 10 s.

For v2 I'll drop the loop and retry from a delayed work on
qrtr_ns.workqueue, requeued every 100 ms while the announcement still
gets -EAGAIN. I'm thinking of keeping a time limit like the 10 s in
v1 and cancelling the retry when the node says BYE, but the details
may change once I test it with rmtfs started before the modem. I'll
post it as a new thread.

Thanks,
Joonhoe Kim