Re: [PATCH] net: add Qualcomm IPC router

2015-12-16 Thread David Miller
From: Courtney Cavin 
Date: Wed, 16 Dec 2015 16:01:41 -0800

> We could hardcode the value in kconfig, but that seems like a worse
> solution than a module parameter.
> 
> I'm open to further suggestions.

No module parameters, configure it via netlink or similar at run
time.
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] net: add Qualcomm IPC router

2015-12-16 Thread Courtney Cavin
On Tue, Dec 15, 2015 at 10:01:14PM +0100, David Miller wrote:
> From: Bjorn Andersson 
> Date: Fri, 11 Dec 2015 12:41:59 -0800
> 
> > +static unsigned int qrtr_local_nid = 1;
> > +module_param_named(node_id, qrtr_local_nid, uint, S_IRUGO);
> > +MODULE_PARM_DESC(idVendor, "Local Node Identifier");
> 
> Module parameters suck.
> 
> Allow the user to choose this dynamically.  You have roughtly two choices.
> 
> 1) Subvert the 'protocol' field passed to ->create() and use that, it is
>being ignored otherwise.
> 
> 2) Put it into the socket address for bind().

So each socket can have its own node id?  That doesn't seem right.

The way these node ids are assigned is by a system designer (in this
case Qualcomm).  The ARM, Linux CPU is always node 1, the audio DSP is
always node 5, etc.  Anyone with the knowhow could reassign these
numbers, but there's no reason to have them be dynamic during runtime.
Additionally, allowing dynamic assignment would require code to prevent
id duplication for known remote nodes, as well as to deal with cases in
which remote node discovery happens after local sockets have acquired
that node's id.

Maybe the first socket created needs CAP_NET_ADMIN, and uses the
'protocol' field to set the node id?  Ugh. Gross.

We could hardcode the value in kconfig, but that seems like a worse
solution than a module parameter.

I'm open to further suggestions.

-Courtney
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] net: add Qualcomm IPC router

2015-12-16 Thread Courtney Cavin
On Fri, Dec 11, 2015 at 09:41:59PM +0100, Bjorn Andersson wrote:
> From: Courtney Cavin 
> 
> Add an implementation of Qualcomm's IPC router protocol, used to
> communicate with service providing remote processors.
> 
> Signed-off-by: Courtney Cavin 
> ---
[...]
> +static int qrtr_sendmsg(struct socket *sock, struct msghdr *msg, size_t len)
> +{
> +   DECLARE_SOCKADDR(struct sockaddr_qrtr *, addr, msg->msg_name);
> +   int (*enqueue_fn)(struct qrtr_node *, struct sk_buff *);
> +   struct qrtr_sock *ipc = qrtr_sk(sock->sk);
> +   struct sock *sk = sock->sk;
> +   struct qrtr_node *node;
> +   struct qrtr_hdr *hdr;
> +   struct sk_buff *skb;
> +   size_t plen;
> +   int rc;
> +
> +   if (msg->msg_flags & ~(MSG_DONTWAIT))
> +   return -EINVAL;
> +
> +   if (len > 65535)
> +   return -EMSGSIZE;
> +
> +   lock_sock(sk);
> +
> +   if (addr) {
> +   if (msg->msg_namelen < sizeof(*addr)) {
> +   release_sock(sk);
> +   return -EINVAL;
> +   }
> +
> +   if (addr->sq_family != AF_QIPCRTR) {
> +   release_sock(sk);
> +   return -EINVAL;
> +   }
> +
> +   rc = qrtr_autobind(sock);
> +   if (rc) {
> +   release_sock(sk);
> +   return rc;
> +   }
> +   } else if (sk->sk_state == TCP_ESTABLISHED) {
> +   addr = &ipc->peer;
> +   } else {
> +   release_sock(sk);
> +   return -ENOTCONN;
> +   }
> +
> +   node = NULL;
> +   if (addr->sq_node == QRTR_NODE_BCAST) {
> +   enqueue_fn = qrtr_bcast_enqueue;
> +   } else if (addr->sq_node == 0 || addr->sq_node == ipc->us.sq_node) {

'addr->sq_node == 0' should be removed from this if-condition.  Zero is
a valid node id.  Clients needing the local address can use
getsockname(2).

> +   enqueue_fn = qrtr_local_enqueue;
> +   } else {
> +   enqueue_fn = qrtr_node_enqueue;
> +   node = qrtr_node_lookup(addr->sq_node);
> +   if (!node) {
> +   release_sock(sk);
> +   return -ECONNRESET;
> +   }
> +   }

-Courtney
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] net: add Qualcomm IPC router

2015-12-15 Thread Dan Williams
On Tue, 2015-12-15 at 15:13 -0600, Dan Williams wrote:
> On Tue, 2015-12-15 at 16:01 -0500, David Miller wrote:
> > From: Bjorn Andersson 
> > Date: Fri, 11 Dec 2015 12:41:59 -0800
> > 
> > > +static unsigned int qrtr_local_nid = 1;
> > > +module_param_named(node_id, qrtr_local_nid, uint, S_IRUGO);
> > > +MODULE_PARM_DESC(idVendor, "Local Node Identifier");
> 
> Also s/idVendor/node_id?

Well, not that it matters if the module parameter gets removed...

Dan

> 
> Dan
> 
> > Module parameters suck.
> > 
> > Allow the user to choose this dynamically.  You have roughtly two
> > choices.
> > 
> > 1) Subvert the 'protocol' field passed to ->create() and use that,
> > it
> > is
> >being ignored otherwise.
> > 
> > 2) Put it into the socket address for bind().
> > --
> > To unsubscribe from this list: send the line "unsubscribe netdev"
> > in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] net: add Qualcomm IPC router

2015-12-15 Thread Dan Williams
On Tue, 2015-12-15 at 16:01 -0500, David Miller wrote:
> From: Bjorn Andersson 
> Date: Fri, 11 Dec 2015 12:41:59 -0800
> 
> > +static unsigned int qrtr_local_nid = 1;
> > +module_param_named(node_id, qrtr_local_nid, uint, S_IRUGO);
> > +MODULE_PARM_DESC(idVendor, "Local Node Identifier");

Also s/idVendor/node_id?

Dan

> Module parameters suck.
> 
> Allow the user to choose this dynamically.  You have roughtly two
> choices.
> 
> 1) Subvert the 'protocol' field passed to ->create() and use that, it
> is
>being ignored otherwise.
> 
> 2) Put it into the socket address for bind().
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] net: add Qualcomm IPC router

2015-12-15 Thread David Miller
From: Bjorn Andersson 
Date: Fri, 11 Dec 2015 12:41:59 -0800

> +static unsigned int qrtr_local_nid = 1;
> +module_param_named(node_id, qrtr_local_nid, uint, S_IRUGO);
> +MODULE_PARM_DESC(idVendor, "Local Node Identifier");

Module parameters suck.

Allow the user to choose this dynamically.  You have roughtly two choices.

1) Subvert the 'protocol' field passed to ->create() and use that, it is
   being ignored otherwise.

2) Put it into the socket address for bind().
--
To unsubscribe from this list: send the line "unsubscribe linux-arm-msm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] net: add Qualcomm IPC router

2015-12-11 Thread Bjorn Andersson
From: Courtney Cavin 

Add an implementation of Qualcomm's IPC router protocol, used to
communicate with service providing remote processors.

Signed-off-by: Courtney Cavin 
---

Downstream this code also includes a large part of a service discovery
mechanism, controlled through a set of custom ioctls. This functionality is
instead moved to user space, a reference implementation for this can be found
at:

  https://github.com/sonyxperiadev/qrtr

 include/linux/socket.h|   4 +-
 include/uapi/linux/qrtr.h |  12 +
 net/Kconfig   |   1 +
 net/Makefile  |   1 +
 net/qrtr/Kconfig  |  24 ++
 net/qrtr/Makefile |   2 +
 net/qrtr/qrtr.c   | 976 ++
 net/qrtr/qrtr.h   |  31 ++
 net/qrtr/smd.c| 119 ++
 9 files changed, 1169 insertions(+), 1 deletion(-)
 create mode 100644 include/uapi/linux/qrtr.h
 create mode 100644 net/qrtr/Kconfig
 create mode 100644 net/qrtr/Makefile
 create mode 100644 net/qrtr/qrtr.c
 create mode 100644 net/qrtr/qrtr.h
 create mode 100644 net/qrtr/smd.c

diff --git a/include/linux/socket.h b/include/linux/socket.h
index 5bf59c8493b7..10e600db06f1 100644
--- a/include/linux/socket.h
+++ b/include/linux/socket.h
@@ -200,7 +200,8 @@ struct ucred {
 #define AF_ALG 38  /* Algorithm sockets*/
 #define AF_NFC 39  /* NFC sockets  */
 #define AF_VSOCK   40  /* vSockets */
-#define AF_MAX 41  /* For now.. */
+#define AF_QIPCRTR 41  /* Qualcomm IPC Router  */
+#define AF_MAX 42  /* For now.. */
 
 /* Protocol families, same as address families. */
 #define PF_UNSPEC  AF_UNSPEC
@@ -246,6 +247,7 @@ struct ucred {
 #define PF_ALG AF_ALG
 #define PF_NFC AF_NFC
 #define PF_VSOCK   AF_VSOCK
+#define PF_QIPCRTR AF_QIPCRTR
 #define PF_MAX AF_MAX
 
 /* Maximum queue length specifiable by listen.  */
diff --git a/include/uapi/linux/qrtr.h b/include/uapi/linux/qrtr.h
new file mode 100644
index ..66c0748d26e2
--- /dev/null
+++ b/include/uapi/linux/qrtr.h
@@ -0,0 +1,12 @@
+#ifndef _LINUX_QRTR_H
+#define _LINUX_QRTR_H
+
+#include 
+
+struct sockaddr_qrtr {
+   __kernel_sa_family_t sq_family;
+   __u32 sq_node;
+   __u32 sq_port;
+};
+
+#endif /* _LINUX_QRTR_H */
diff --git a/net/Kconfig b/net/Kconfig
index 11f8c22af34d..5d9a52ef9a55 100644
--- a/net/Kconfig
+++ b/net/Kconfig
@@ -233,6 +233,7 @@ source "net/mpls/Kconfig"
 source "net/hsr/Kconfig"
 source "net/switchdev/Kconfig"
 source "net/l3mdev/Kconfig"
+source "net/qrtr/Kconfig"
 
 config RPS
bool
diff --git a/net/Makefile b/net/Makefile
index a5d04098dfce..007deb2f17c2 100644
--- a/net/Makefile
+++ b/net/Makefile
@@ -77,3 +77,4 @@ endif
 ifneq ($(CONFIG_NET_L3_MASTER_DEV),)
 obj-y  += l3mdev/
 endif
+obj-$(CONFIG_QRTR) += qrtr/
diff --git a/net/qrtr/Kconfig b/net/qrtr/Kconfig
new file mode 100644
index ..f053cc25f621
--- /dev/null
+++ b/net/qrtr/Kconfig
@@ -0,0 +1,24 @@
+# Qualcomm IPC Router configuration
+#
+
+config QRTR
+   bool "Qualcomm IPC Router support"
+   depends on ARCH_QCOM || COMPILE_TEST
+   ---help---
+ Say Y if you intend to use Qualcomm IPC router protocol.  The
+ protocol is used to communicate with services provided by other
+ hardware blocks in the system.
+
+ In order to do service lookups, a userspace daemon is required to
+ maintain a service listing.
+
+if QRTR
+
+config QRTR_SMD
+   tristate "SMD IPC Router channels"
+   depends on QRTR && QCOM_SMD && OF
+   ---help---
+ Say Y here to support SMD based ipcrouter channels.  SMD is the
+ most common transport for IPC Router.
+
+endif # QRTR
diff --git a/net/qrtr/Makefile b/net/qrtr/Makefile
new file mode 100644
index ..e282a84ffc5c
--- /dev/null
+++ b/net/qrtr/Makefile
@@ -0,0 +1,2 @@
+obj-y := qrtr.o
+obj-$(CONFIG_QRTR_SMD) += smd.o
diff --git a/net/qrtr/qrtr.c b/net/qrtr/qrtr.c
new file mode 100644
index ..c96dfb6bfbaf
--- /dev/null
+++ b/net/qrtr/qrtr.c
@@ -0,0 +1,976 @@
+/*
+ * Copyright (c) 2015, Sony Mobile Communications Inc.
+ * Copyright (c) 2013, The Linux Foundation. All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 and
+ * only version 2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ */
+#include 
+#include 
+#include  /* For TIOCINQ/OUTQ */
+
+#include 
+
+#include "qrtr.h"
+
+#define QRTR_PROTO_VER 1
+
+/* auto-bind range */
+#define QRTR_MIN_EPH_SOCKET 0x4000
+