From fbf873fc96a99aaf3fb76ac66624a320e18f6c6a Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Sun, 1 Mar 2020 23:03:04 -0800 Subject: [PATCH 1/4] net: qrtr: Respond to HELLO message Lost in the translation from the user space implementation was the detail that HELLO mesages must be exchanged between each node pair. As such the incoming HELLO must be replied to. Similar to the previous implementation no effort is made to prevent two Linux boxes from continuously sending HELLO messages back and forth, this is left to a follow up patch. say_hello() is moved, to facilitate the new call site. Change-Id: I1b1d66c6a24edf5758d05b76fd527b7d2a35c5da Fixes: 0c2204a4ad71 ("net: qrtr: Migrate nameservice to kernel from userspace") Reviewed-by: Manivannan Sadhasivam Tested-by: Manivannan Sadhasivam Signed-off-by: Bjorn Andersson Signed-off-by: David S. Miller Git-commit: a1dc1d6a05a730b62b45828975a088db577d3139 Git-repo: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git Signed-off-by: Chris Lew --- net/qrtr/ns.c | 54 ++++++++++++++++++++++++++++----------------------- 1 file changed, 30 insertions(+), 24 deletions(-) diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c index 413228c4520e..61a58a35cc91 100644 --- a/net/qrtr/ns.c +++ b/net/qrtr/ns.c @@ -286,9 +286,38 @@ static int server_del(struct qrtr_node *node, unsigned int port) return 0; } +static int say_hello(struct sockaddr_qrtr *dest) +{ + struct qrtr_ctrl_pkt pkt; + struct msghdr msg = { }; + struct kvec iv; + int ret; + + iv.iov_base = &pkt; + iv.iov_len = sizeof(pkt); + + memset(&pkt, 0, sizeof(pkt)); + pkt.cmd = cpu_to_le32(QRTR_TYPE_HELLO); + + msg.msg_name = (struct sockaddr *)dest; + msg.msg_namelen = sizeof(*dest); + + ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt)); + if (ret < 0) + pr_err("failed to send hello msg\n"); + + 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); } @@ -566,29 +595,6 @@ static void ctrl_cmd_del_lookup(struct sockaddr_qrtr *from, } } -static int say_hello(void) -{ - struct qrtr_ctrl_pkt pkt; - struct msghdr msg = { }; - struct kvec iv; - int ret; - - iv.iov_base = &pkt; - iv.iov_len = sizeof(pkt); - - memset(&pkt, 0, sizeof(pkt)); - pkt.cmd = cpu_to_le32(QRTR_TYPE_HELLO); - - msg.msg_name = (struct sockaddr *)&qrtr_ns.bcast_sq; - msg.msg_namelen = sizeof(qrtr_ns.bcast_sq); - - ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt)); - if (ret < 0) - pr_err("failed to send hello msg\n"); - - return ret; -} - static void qrtr_ns_worker(struct work_struct *work) { const struct qrtr_ctrl_pkt *pkt; @@ -725,7 +731,7 @@ void qrtr_ns_init(struct work_struct *work) if (!qrtr_ns.workqueue) goto err_sock; - ret = say_hello(); + ret = say_hello(&qrtr_ns.bcast_sq); if (ret < 0) goto err_wq; From efbe72e33181458272060780c2b617019ac89e81 Mon Sep 17 00:00:00 2001 From: Bjorn Andersson Date: Sun, 1 Mar 2020 23:03:05 -0800 Subject: [PATCH 2/4] net: qrtr: Fix FIXME related to qrtr_ns_init() The 2 second delay before calling qrtr_ns_init() meant that the remote processors would register as endpoints in qrtr and the say_hello() call would therefor broadcast the outgoing HELLO to them. With the HELLO handshake corrected this delay is no longer needed. Change-Id: I20aa56cbb6c54cbbd0ccb77e30bcf59661d449cc Reviewed-by: Manivannan Sadhasivam Tested-by: Manivannan Sadhasivam Signed-off-by: Bjorn Andersson Signed-off-by: David S. Miller Git-commit: 71046abfffe9d34ae90c82cf9c8e44355c2e114c Git-repo: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git Signed-off-by: Chris Lew --- net/qrtr/ns.c | 2 +- net/qrtr/qrtr.c | 10 +--------- net/qrtr/qrtr.h | 2 +- 3 files changed, 3 insertions(+), 11 deletions(-) diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c index 61a58a35cc91..e7d0fe3f4330 100644 --- a/net/qrtr/ns.c +++ b/net/qrtr/ns.c @@ -693,7 +693,7 @@ static void qrtr_ns_data_ready(struct sock *sk) queue_work(qrtr_ns.workqueue, &qrtr_ns.work); } -void qrtr_ns_init(struct work_struct *work) +void qrtr_ns_init(void) { struct sockaddr_qrtr sq; int ret; diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c index 8455bc774481..641a1cf9c164 100644 --- a/net/qrtr/qrtr.c +++ b/net/qrtr/qrtr.c @@ -11,7 +11,6 @@ #include #include #include -#include #include #include #include @@ -131,8 +130,6 @@ static DECLARE_RWSEM(qrtr_epts_lock); static DEFINE_IDR(qrtr_ports); static DEFINE_SPINLOCK(qrtr_port_lock); -static struct delayed_work qrtr_ns_work; - /** * struct qrtr_node - endpoint node * @ep_lock: lock for endpoint management and callbacks @@ -1921,11 +1918,7 @@ static int __init qrtr_proto_init(void) return rc; } - /* FIXME: Currently, this 2s delay is required to catch the NEW_SERVER - * messages from routers. But the fix could be somewhere else. - */ - INIT_DELAYED_WORK(&qrtr_ns_work, qrtr_ns_init); - schedule_delayed_work(&qrtr_ns_work, msecs_to_jiffies(2000)); + qrtr_ns_init(); return rc; } @@ -1933,7 +1926,6 @@ postcore_initcall(qrtr_proto_init); static void __exit qrtr_proto_fini(void) { - cancel_delayed_work_sync(&qrtr_ns_work); qrtr_ns_remove(); sock_unregister(qrtr_family.family); proto_unregister(&qrtr_proto); diff --git a/net/qrtr/qrtr.h b/net/qrtr/qrtr.h index 88c15aec411a..fba24c530146 100644 --- a/net/qrtr/qrtr.h +++ b/net/qrtr/qrtr.h @@ -33,7 +33,7 @@ void qrtr_endpoint_unregister(struct qrtr_endpoint *ep); int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len); -void qrtr_ns_init(struct work_struct *work); +void qrtr_ns_init(void); void qrtr_ns_remove(void); From 617ffc4bf429a613918bc390ee3736deddb42b66 Mon Sep 17 00:00:00 2001 From: Chris Lew Date: Tue, 5 May 2020 15:45:35 -0700 Subject: [PATCH 3/4] net: qrtr: Add pr_fmt to ns Add pr_fmt to the ns file for easier error log matching. Change-Id: I711735ee60c1fd9fde25850b8e625bb44c04f4c4 Signed-off-by: Chris Lew --- net/qrtr/ns.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c index e7d0fe3f4330..610755c482dd 100644 --- a/net/qrtr/ns.c +++ b/net/qrtr/ns.c @@ -1,10 +1,12 @@ // SPDX-License-Identifier: GPL-2.0 OR BSD-3-Clause /* * Copyright (c) 2015, Sony Mobile Communications Inc. - * Copyright (c) 2013, The Linux Foundation. All rights reserved. + * Copyright (c) 2013, 2020, The Linux Foundation. All rights reserved. * Copyright (c) 2020, Linaro Ltd. */ +#define pr_fmt(fmt) "qrtr: %s(): " fmt, __func__ + #include #include #include From e5bbd357d99585a2771114309e4339a66224c5f0 Mon Sep 17 00:00:00 2001 From: Chris Lew Date: Tue, 5 May 2020 16:11:44 -0700 Subject: [PATCH 4/4] net: qrtr: Ignore ENODEV failures in ns Ignore the ENODEV failures returned by kernel_sendmsg(). These errors mean either the local port has closed or the remote has gone down. Neither of these scenarios are fatal and will eventually be handled through packets that are later queued on the control port. Also improve the logging messages to print the error code returned by kernel_sendmsg(). Change-Id: Ibcebd1f44a3fbc87febaa84b11e05675663da5a0 Signed-off-by: Chris Lew --- net/qrtr/ns.c | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/net/qrtr/ns.c b/net/qrtr/ns.c index 610755c482dd..600d9f435a46 100644 --- a/net/qrtr/ns.c +++ b/net/qrtr/ns.c @@ -151,8 +151,8 @@ static int service_announce_del(struct sockaddr_qrtr *dest, msg.msg_namelen = sizeof(*dest); ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt)); - if (ret < 0) - pr_err("failed to announce del service\n"); + if (ret < 0 && ret != -ENODEV) + pr_err("failed to announce del service %d\n", ret); return ret; } @@ -182,8 +182,8 @@ static void lookup_notify(struct sockaddr_qrtr *to, struct qrtr_server *srv, msg.msg_namelen = sizeof(*to); ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt)); - if (ret < 0) - pr_err("failed to send lookup notification\n"); + if (ret < 0 && ret != -ENODEV) + pr_err("failed to send lookup notification %d\n", ret); } static int announce_servers(struct sockaddr_qrtr *sq) @@ -204,7 +204,10 @@ static int announce_servers(struct sockaddr_qrtr *sq) ret = service_announce_new(sq, srv); if (ret < 0) { - pr_err("failed to announce new service\n"); + if (ret == -ENODEV) + continue; + + pr_err("failed to announce new service %d\n", ret); return ret; } } @@ -306,7 +309,7 @@ static int say_hello(struct sockaddr_qrtr *dest) ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt)); if (ret < 0) - pr_err("failed to send hello msg\n"); + pr_err("failed to send hello msg %d\n", ret); return ret; } @@ -369,8 +372,8 @@ static int ctrl_cmd_bye(struct sockaddr_qrtr *from) msg.msg_namelen = sizeof(sq); ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt)); - if (ret < 0) { - pr_err("failed to send bye cmd\n"); + if (ret < 0 && ret != -ENODEV) { + pr_err("failed to send bye cmd %d\n", ret); return ret; } } @@ -444,8 +447,8 @@ static int ctrl_cmd_del_client(struct sockaddr_qrtr *from, msg.msg_namelen = sizeof(sq); ret = kernel_sendmsg(qrtr_ns.sock, &msg, &iv, 1, sizeof(pkt)); - if (ret < 0) { - pr_err("failed to send del client cmd\n"); + if (ret < 0 && ret != -ENODEV) { + pr_err("failed to send del client cmd %d\n", ret); return ret; } } @@ -479,7 +482,7 @@ static int ctrl_cmd_new_server(struct sockaddr_qrtr *from, if (srv->node == qrtr_ns.local_node) { ret = service_announce_new(&qrtr_ns.bcast_sq, srv); if (ret < 0) { - pr_err("failed to announce new service\n"); + pr_err("failed to announce new service %d\n", ret); return ret; } }