[RESEND PATCH 0/4] Missing QRTR features

[RESEND PATCH 0/4] Missing QRTR features

[Bjorn Andersson]

The QMUX specification covers packet routing as well as service life cycle and
discovery. The current implementation of qrtr supports the prior part, but in
order to fully implement service management on-top a few more parts are needed.

The first patch in the series serves the purpose of reducing duplication in
patch two and three.

The second and third patch adds two qrtr-level notifications required by the
specification, in order to notify local and remote service controllers about
dying clients.

The last patch serves the purpose of notifying local clients about the presence
of a local service register, allowing them to register services as well as
querying for remote registered services.

Bjorn Andersson (4):
  net: qrtr: Refactor packet allocation
  net: qrtr: Inject BYE on remote termination
  net: qrtr: Broadcast DEL_CLIENT message when endpoint is closed
  net: qrtr: Inform open sockets about new controller

 net/qrtr/qrtr.c | 104 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 98 insertions(+), 6 deletions(-)

-- 
2.12.0

[RESEND PATCH 1/4] net: qrtr: Refactor packet allocation

[Bjorn Andersson]

Extract the allocation and filling in the control message header fields
to a separate function in order to reuse this in subsequent patches.

Cc: Courtney Cavin <ccavin@gmail.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 net/qrtr/qrtr.c | 25 +++++++++++++++++++------
 1 file changed, 19 insertions(+), 6 deletions(-)

diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index a9a8c7d5a4a9..86d35ed50da9 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -245,14 +245,11 @@ int qrtr_endpoint_post(struct qrtr_endpoint *ep, const void *data, size_t len)
 }
 EXPORT_SYMBOL_GPL(qrtr_endpoint_post);
 
-/* Allocate and construct a resume-tx packet. */
-static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node,
-					    u32 dst_node, u32 port)
+static struct sk_buff *qrtr_alloc_ctrl_packet(u32 type, size_t pkt_len,
+					      u32 src_node, u32 dst_node)
 {
-	const int pkt_len = 20;
 	struct qrtr_hdr *hdr;
 	struct sk_buff *skb;
-	__le32 *buf;
 
 	skb = alloc_skb(QRTR_HDR_SIZE + pkt_len, GFP_KERNEL);
 	if (!skb)
@@ -261,7 +258,7 @@ static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node,
 
 	hdr = (struct qrtr_hdr *)skb_put(skb, QRTR_HDR_SIZE);
 	hdr->version = cpu_to_le32(QRTR_PROTO_VER);
-	hdr->type = cpu_to_le32(QRTR_TYPE_RESUME_TX);
+	hdr->type = cpu_to_le32(type);
 	hdr->src_node_id = cpu_to_le32(src_node);
 	hdr->src_port_id = cpu_to_le32(QRTR_PORT_CTRL);
 	hdr->confirm_rx = cpu_to_le32(0);
@@ -269,6 +266,22 @@ static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node,
 	hdr->dst_node_id = cpu_to_le32(dst_node);
 	hdr->dst_port_id = cpu_to_le32(QRTR_PORT_CTRL);
 
+	return skb;
+}
+
+/* Allocate and construct a resume-tx packet. */
+static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node,
+					    u32 dst_node, u32 port)
+{
+	const int pkt_len = 20;
+	struct sk_buff *skb;
+	__le32 *buf;
+
+	skb = qrtr_alloc_ctrl_packet(QRTR_TYPE_RESUME_TX, pkt_len,
+				     src_node, dst_node);
+	if (!skb)
+		return NULL;
+
 	buf = (__le32 *)skb_put(skb, pkt_len);
 	memset(buf, 0, pkt_len);
 	buf[0] = cpu_to_le32(QRTR_TYPE_RESUME_TX);
-- 
2.12.0

[RESEND PATCH 2/4] net: qrtr: Inject BYE on remote termination

[Bjorn Andersson]

Per the QMUX protocol specification a terminating node can send a BYE
control message to signal that the link is going down, upon receiving
this all information about remote services should be discarded and local
clients should be notified.

In the event that the link was brought down abruptly the router is
supposed to act like a BYE message has arrived. As there is no harm in
receiving an extra BYE from the remote this patch implements the latter
by injecting a BYE when the link to the remote is unregistered.

The name service will receive the BYE and can implement the notification
to the local clients.

Cc: Courtney Cavin <ccavin@gmail.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 net/qrtr/qrtr.c | 27 +++++++++++++++++++++++++++
 1 file changed, 27 insertions(+)

diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index 86d35ed50da9..e8cbab23d667 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -111,6 +111,8 @@ struct qrtr_node {
 	struct list_head item;
 };
 
+static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb);
+
 /* Release node resources and free the node.
  *
  * Do not call directly, use qrtr_node_release.  To be used with
@@ -291,6 +293,25 @@ static struct sk_buff *qrtr_alloc_resume_tx(u32 src_node,
 	return skb;
 }
 
+/* Allocate and construct a BYE message to signal remote termination */
+static struct sk_buff *qrtr_alloc_local_bye(u32 src_node)
+{
+	const int pkt_len = 20;
+	struct sk_buff *skb;
+	__le32 *buf;
+
+	skb = qrtr_alloc_ctrl_packet(QRTR_TYPE_BYE, pkt_len,
+				     src_node, qrtr_local_nid);
+	if (!skb)
+		return NULL;
+
+	buf = (__le32 *)skb_put(skb, pkt_len);
+	memset(buf, 0, pkt_len);
+	buf[0] = cpu_to_le32(QRTR_TYPE_BYE);
+
+	return skb;
+}
+
 static struct qrtr_sock *qrtr_port_lookup(int port);
 static void qrtr_port_put(struct qrtr_sock *ipc);
 
@@ -382,11 +403,17 @@ EXPORT_SYMBOL_GPL(qrtr_endpoint_register);
 void qrtr_endpoint_unregister(struct qrtr_endpoint *ep)
 {
 	struct qrtr_node *node = ep->node;
+	struct sk_buff *skb;
 
 	mutex_lock(&node->ep_lock);
 	node->ep = NULL;
 	mutex_unlock(&node->ep_lock);
 
+	/* Notify the local controller about the event */
+	skb = qrtr_alloc_local_bye(node->nid);
+	if (skb)
+		qrtr_local_enqueue(NULL, skb);
+
 	qrtr_node_release(node);
 	ep->node = NULL;
 }
-- 
2.12.0

[RESEND PATCH 3/4] net: qrtr: Broadcast DEL_CLIENT message when endpoint is closed

[Bjorn Andersson]

Per the QMUXv2 protocol specificiation a DEL_CLIENT message should be
broadcasted when an endpoint is disconnected.

The protocol specification does suggest that the router can keep track
of which nodes the endpoint has been communicating with to not wake up
sleeping remotes unecessarily, but implementation of this suggestion is
left for the future.

Cc: Courtney Cavin <ccavin@gmail.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 net/qrtr/qrtr.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index e8cbab23d667..d7516098b5aa 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -112,6 +112,7 @@ struct qrtr_node {
 };
 
 static int qrtr_local_enqueue(struct qrtr_node *node, struct sk_buff *skb);
+static int qrtr_bcast_enqueue(struct qrtr_node *node, struct sk_buff *skb);
 
 /* Release node resources and free the node.
  *
@@ -312,6 +313,26 @@ static struct sk_buff *qrtr_alloc_local_bye(u32 src_node)
 	return skb;
 }
 
+static struct sk_buff *qrtr_alloc_del_client(struct sockaddr_qrtr *sq)
+{
+	const int pkt_len = 20;
+	struct sk_buff *skb;
+	__le32 *buf;
+
+	skb = qrtr_alloc_ctrl_packet(QRTR_TYPE_DEL_CLIENT, pkt_len,
+				     sq->sq_node, QRTR_NODE_BCAST);
+	if (!skb)
+		return NULL;
+
+	buf = (__le32 *)skb_put(skb, pkt_len);
+	memset(buf, 0, pkt_len);
+	buf[0] = cpu_to_le32(QRTR_TYPE_DEL_CLIENT);
+	buf[1] = cpu_to_le32(sq->sq_node);
+	buf[2] = cpu_to_le32(sq->sq_port);
+
+	return skb;
+}
+
 static struct qrtr_sock *qrtr_port_lookup(int port);
 static void qrtr_port_put(struct qrtr_sock *ipc);
 
@@ -448,8 +469,15 @@ static void qrtr_port_put(struct qrtr_sock *ipc)
 /* Remove port assignment. */
 static void qrtr_port_remove(struct qrtr_sock *ipc)
 {
+	struct sk_buff *skb;
 	int port = ipc->us.sq_port;
 
+	skb = qrtr_alloc_del_client(&ipc->us);
+	if (skb) {
+		skb_set_owner_w(skb, &ipc->sk);
+		qrtr_bcast_enqueue(NULL, skb);
+	}
+
 	if (port == QRTR_PORT_CTRL)
 		port = 0;
 
-- 
2.12.0

[RESEND PATCH 4/4] net: qrtr: Inform open sockets about new controller

[Bjorn Andersson]

As the higher level communication only deals with "services" the
a service directory is required to keep track of local and remote
services. In order for qrtr clients to be informed about when the
service directory implementation is available some event needs to be
passed to them.

Rather than introducing support for broadcasting such a message in-band
to all open local sockets we flag each socket with ENETRESET, as there
are no other expected operations that would benefit from having support
from locally broadcasting messages.

Cc: Courtney Cavin <ccavin@gmail.com>
Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org>
---
 net/qrtr/qrtr.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
index d7516098b5aa..c7a5d861906b 100644
--- a/net/qrtr/qrtr.c
+++ b/net/qrtr/qrtr.c
@@ -530,6 +530,26 @@ static int qrtr_port_assign(struct qrtr_sock *ipc, int *port)
 	return 0;
 }
 
+/* Reset all non-control ports */
+static void qrtr_reset_ports(void)
+{
+	struct qrtr_sock *ipc;
+	int id;
+
+	mutex_lock(&qrtr_port_lock);
+	idr_for_each_entry(&qrtr_ports, ipc, id) {
+		/* Don't reset control port */
+		if (id == 0)
+			continue;
+
+		sock_hold(&ipc->sk);
+		ipc->sk.sk_err = ENETRESET;
+		wake_up_interruptible(sk_sleep(&ipc->sk));
+		sock_put(&ipc->sk);
+	}
+	mutex_unlock(&qrtr_port_lock);
+}
+
 /* Bind socket to address.
  *
  * Socket should be locked upon call.
@@ -558,6 +578,10 @@ static int __qrtr_bind(struct socket *sock,
 
 	sock_reset_flag(sk, SOCK_ZAPPED);
 
+	/* Notify all open ports about the new controller */
+	if (port == QRTR_PORT_CTRL)
+		qrtr_reset_ports();
+
 	return 0;
 }
 
-- 
2.12.0

Re: [RESEND PATCH 0/4] Missing QRTR features

[David Miller]

From: Bjorn Andersson <bjorn.andersson@linaro.org>
Date: Wed,  7 Jun 2017 14:07:35 -0700

> The QMUX specification covers packet routing as well as service life cycle and
> discovery. The current implementation of qrtr supports the prior part, but in
> order to fully implement service management on-top a few more parts are needed.
> 
> The first patch in the series serves the purpose of reducing duplication in
> patch two and three.
> 
> The second and third patch adds two qrtr-level notifications required by the
> specification, in order to notify local and remote service controllers about
> dying clients.
> 
> The last patch serves the purpose of notifying local clients about the presence
> of a local service register, allowing them to register services as well as
> querying for remote registered services.

Looks good, series applied to net-next, thank you.