[PATCH v2] net: qrtr: Send HELLO message on endpoint register

Pranav Mahesh Phansalkar <[email protected]>
Newsgroups org.kernel.vger.linux-arm-msm,org.kernel.vger.linux-kernel,org.kernel.vger.netdev
Message-ID <20260807-qrtr-hello-on-ep-register-v2-1-a7f265a42f7e@oss.qualcomm.com>
From: Chris Lew <[email protected]>

HELLO is currently handled entirely by the name server (NS): it is
sent once as a broadcast when the NS initializes, and again as a
reply whenever the NS receives an inbound HELLO from a remote.

In a slave model, neither endpoint initiates a HELLO on its own;
each side only replies to one it receives. Since both sides are waiting
on the other to speak first, no HELLO is ever sent, stalling further
communication.

To fix this:
- Transfer HELLO handshake ownership to the core layer.
- Schedule a work item on endpoint registration to send a HELLO once
  the name server is bound, retrying on failure.
- Enforce HELLO-first ordering by dropping non-HELLO packets until
  the HELLO is confirmed sent, using bool hello_sent guarded by
  ep_lock to make the gate check atomic with xmit().
- Skip nodes with nid == QRTR_EP_NID_AUTO in bcast_enqueue().
- Remove say_hello() from the name server's ctrl_cmd_hello() handler;
  the core layer is now the sole sender of the outbound HELLO.

Signed-off-by: Chris Lew <[email protected]>
Co-developed-by: Deepak Kumar Singh <[email protected]>
Signed-off-by: Deepak Kumar Singh <[email protected]>
Co-developed-by: Pranav Mahesh Phansalkar <[email protected]>
Signed-off-by: Pranav Mahesh Phansalkar <[email protected]>
---
Changes in v2:
- Drop say_hello() from the name server's ctrl_cmd_hello() handler;
  the core layer is now the sole sender of the outbound HELLO, so
  the NS no longer needs to duplicate that logic.
- Fix Signed-off-by/Co-developed-by ordering so the submitter's
  Signed-off-by is last.
- Duplicate-HELLO detection at the name server (hello_rcvd) will be
  handled in a separate patch, so it is dropped from this series.
- Link to v1: https://lore.kernel.org/r/20260731-qrtr-hello-on-ep-register-v1-1-ddee5b1bfaec@oss.qualcomm.com
---
 net/qrtr/af_qrtr.c | 61 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 net/qrtr/ns.c      |  8 +------
 2 files changed, 62 insertions(+), 7 deletions(-)

diff --git a/net/qrtr/af_qrtr.c b/net/qrtr/af_qrtr.c
index d02ef9a74c3ca..7adc0a0531254 100644
--- a/net/qrtr/af_qrtr.c
+++ b/net/qrtr/af_qrtr.c
@@ -9,6 +9,7 @@
 #include <linux/termios.h>	/* For TIOCINQ/OUTQ */
 #include <linux/spinlock.h>
 #include <linux/wait.h>
+#include <linux/workqueue.h>
 
 #include <net/sock.h>
 
@@ -120,8 +121,10 @@ static DEFINE_XARRAY_ALLOC(qrtr_ports);
  * @nid: node id
  * @qrtr_tx_flow: xarray of qrtr_tx_flow, keyed by node << 32 | port
  * @qrtr_tx_lock: lock for qrtr_tx_flow inserts
+ * @hello_sent: hello packet send successful
  * @rx_queue: receive queue
  * @item: list item for broadcast list
+ * @say_hello: scheduled work for sending hello packet
  */
 struct qrtr_node {
 	struct mutex ep_lock;
@@ -132,8 +135,11 @@ struct qrtr_node {
 	struct xarray qrtr_tx_flow;
 	struct mutex qrtr_tx_lock; /* for qrtr_tx_flow */
 
+	bool hello_sent;
+
 	struct sk_buff_head rx_queue;
 	struct list_head item;
+	struct work_struct say_hello;
 };
 
 /**
@@ -187,6 +193,8 @@ static void __qrtr_node_release(struct kref *kref)
 	list_del(&node->item);
 	mutex_unlock(&qrtr_node_lock);
 
+	cancel_work_sync(&node->say_hello);
+
 	skb_queue_purge(&node->rx_queue);
 
 	/* Free tx flow counters */
@@ -341,6 +349,14 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
 	size_t len = skb->len;
 	int rc, confirm_rx;
 
+	mutex_lock(&node->ep_lock);
+	if (!node->hello_sent && type != QRTR_TYPE_HELLO) {
+		mutex_unlock(&node->ep_lock);
+		kfree_skb(skb);
+		return 0;
+	}
+	mutex_unlock(&node->ep_lock);
+
 	confirm_rx = qrtr_tx_wait(node, to->sq_node, to->sq_port, type);
 	if (confirm_rx < 0) {
 		kfree_skb(skb);
@@ -372,6 +388,8 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
 			rc = node->ep->xmit(node->ep, skb);
 		else
 			kfree_skb(skb);
+		if (!rc && type == QRTR_TYPE_HELLO)
+			node->hello_sent = true;
 		mutex_unlock(&node->ep_lock);
 	}
 	/* Need to ensure that a subsequent message carries the otherwise lost
@@ -379,6 +397,9 @@ static int qrtr_node_enqueue(struct qrtr_node *node, struct sk_buff *skb,
 	if (rc && confirm_rx)
 		qrtr_tx_flow_failed(node, to->sq_node, to->sq_port);
 
+	if (rc && type == QRTR_TYPE_HELLO)
+		schedule_work(&node->say_hello);
+
 	return rc;
 }
 
@@ -570,6 +591,35 @@ static struct sk_buff *qrtr_alloc_ctrl_packet(struct qrtr_ctrl_pkt **pkt,
 	return skb;
 }
 
+static void qrtr_hello_work(struct work_struct *work)
+{
+	struct sockaddr_qrtr from = {AF_QIPCRTR, 0, QRTR_PORT_CTRL};
+	struct sockaddr_qrtr to = {AF_QIPCRTR, 0, QRTR_PORT_CTRL};
+	struct qrtr_ctrl_pkt *pkt;
+	struct qrtr_node *node;
+	struct qrtr_sock *ctrl;
+	struct sk_buff *skb;
+
+	node = container_of(work, struct qrtr_node, say_hello);
+
+	/* NS must be bound before we can send */
+	ctrl = qrtr_port_lookup(QRTR_PORT_CTRL);
+	if (!ctrl)
+		return;
+
+	skb = qrtr_alloc_ctrl_packet(&pkt, GFP_KERNEL);
+	if (!skb) {
+		qrtr_port_put(ctrl);
+		return;
+	}
+
+	pkt->cmd = cpu_to_le32(QRTR_TYPE_HELLO);
+	from.sq_node = qrtr_local_nid;
+	to.sq_node = node->nid;
+	qrtr_node_enqueue(node, skb, QRTR_TYPE_HELLO, &from, &to);
+	qrtr_port_put(ctrl);
+}
+
 /**
  * qrtr_endpoint_register() - register a new endpoint
  * @ep: endpoint to register
@@ -595,6 +645,9 @@ int qrtr_endpoint_register(struct qrtr_endpoint *ep, unsigned int nid)
 	node->nid = QRTR_EP_NID_AUTO;
 	node->ep = ep;
 
+	node->hello_sent = false;
+	INIT_WORK(&node->say_hello, qrtr_hello_work);
+
 	xa_init(&node->qrtr_tx_flow);
 	mutex_init(&node->qrtr_tx_lock);
 
@@ -605,6 +658,9 @@ int qrtr_endpoint_register(struct qrtr_endpoint *ep, unsigned int nid)
 	mutex_unlock(&qrtr_node_lock);
 	ep->node = node;
 
+	/* Initiate HELLO handshake from the core layer */
+	schedule_work(&node->say_hello);
+
 	return 0;
 }
 EXPORT_SYMBOL_GPL(qrtr_endpoint_register);
@@ -879,6 +935,11 @@ static int qrtr_bcast_enqueue(struct qrtr_node *node, struct sk_buff *skb,
 
 	mutex_lock(&qrtr_node_lock);
 	list_for_each_entry(node, &qrtr_all_nodes, item) {
+		/* Skip nodes whose node ID is not yet known; they cannot
+		 * receive broadcast packets before the HELLO handshake.
+		 */
+		if (node->nid == QRTR_EP_NID_AUTO)
+			continue;
 		skbn = pskb_copy(skb, GFP_KERNEL);
 		if (!skbn)
 			break;
diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c
index b3f9bbcf9ab9b..14262623d2dd5 100644
--- a/net/qrtr/ns.c
+++ b/net/qrtr/ns.c
@@ -212,6 +212,7 @@ static void lookup_notify(struct sockaddr_qrtr *to, struct qrtr_server *srv,
 		pr_err("failed to send lookup notification\n");
 }
 
+/* Announce the list of servers registered on the local node */
 static int announce_servers(struct sockaddr_qrtr *sq)
 {
 	struct qrtr_server *srv;
@@ -349,15 +350,8 @@ static int say_hello(struct sockaddr_qrtr *dest)
 	return ret;
 }
 
-/* Announce the list of servers registered on the local node */
 static int ctrl_cmd_hello(struct sockaddr_qrtr *sq)
 {
-	int ret;
-
-	ret = say_hello(sq);
-	if (ret < 0)
-		return ret;
-
 	return announce_servers(sq);
 }
 

---
base-commit: 502d801f0ab03e4f32f9a33d203154ce84887921
change-id: 20260623-qrtr-hello-on-ep-register-7064b66ccd70

Best regards,
-- 
Pranav Mahesh Phansalkar <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.