From: Chris Lew 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 Co-developed-by: Deepak Kumar Singh Signed-off-by: Deepak Kumar Singh Co-developed-by: Pranav Mahesh Phansalkar Signed-off-by: Pranav Mahesh Phansalkar --- 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 /* For TIOCINQ/OUTQ */ #include #include +#include #include @@ -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