[{"id":1970,"web_url":"https://patchwork.libcamera.org/comment/1970/","msgid":"<20190622214628.GI8156@pendragon.ideasonboard.com>","date":"2019-06-22T21:46:28","subject":"Re: [libcamera-devel] [RFC 1/2] libcamera: ipc: unix: Add a IPC\n\tmechanism based on Unix sockets","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Fri, Jun 21, 2019 at 06:15:18AM +0200, Niklas Söderlund wrote:\n> To be able to isolate an IPA component in a separate process and IPC\n\ns/and/an/\n\n> mechanism is needed to communicate with it. Add a IPC mechanism based on\n\ns/a IPC/an IPC/\n\n> Unix sockets which allows users to pass both data and file descriptors\n> to to and from the IPA process.\n\ns/to to/to/\n\n> \n> The implementation allows users to send both data and file descriptors\n> in the same message. This allows users to more easily implement\n> serialization and deserialization of objects as all elements belonging\n> to an object can be sent in one message,\n\ns/,/./\n\n> \n> The implementation guarantees that a whole object is transmitted and\n> received over IPC before it's handed of. This allows IPC users to not\n\nWhat do you mean by handed of here ?\n\n> have to deal with buffering and may depend on that it only needs to deal\n> with serialization/deserialization of complete object blobs.\n> \n> The implementation also provides a priv field in the IPC message header\n> which is a 32 bit integer that can be used by IPA implementations that\n> do not require a complex protocol header to describe what type of\n> message is transmitted.\n\nI'm not sure I would do that, wouldn't it make more sense to completely\nseparate the IPC transport from the protocol ? Otherwise, where will we\ndraw the line ?\n\n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/include/ipc_unixsocket.h | 58 +++++\n> src/libcamera/ipc_unixsocket.cpp | 330 +++++++++++++++++++++++++\n> src/libcamera/meson.build | 2 +\n> 3 files changed, 390 insertions(+)\n> create mode 100644 src/libcamera/include/ipc_unixsocket.h\n> create mode 100644 src/libcamera/ipc_unixsocket.cpp\n> \n> diff --git a/src/libcamera/include/ipc_unixsocket.h b/src/libcamera/include/ipc_unixsocket.h\n> new file mode 100644\n> index 0000000000000000..864fa93b1f190fb7\n> --- /dev/null\n> +++ b/src/libcamera/include/ipc_unixsocket.h\n> @@ -0,0 +1,58 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * ipc_unixsocket.h - IPC mechanism based on Unix sockets\n> + */\n> +\n> +#ifndef __LIBCAMERA_IPC_UNIXSOCKET_H__\n> +#define __LIBCAMERA_IPC_UNIXSOCKET_H__\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class IPCUnixSocket\n> +{\n> +public:\n> +\tstruct Payload {\n> +\t\tuint32_t priv;\n> +\t\tstd::vector data;\n> +\t\tstd::vector fds;\n> +\t};\n> +\n> +\tIPCUnixSocket();\n> +\tIPCUnixSocket(int fd);\n> +\n> +\tint create();\n> +\tint connect();\n> +\tvoid close();\n> +\n> +\tint send(const Payload &payload);\n> +\tint recv(Payload *payload, int timeout);\n> +\tint call(const Payload &payload, Payload *response, int timeout);\n> +\n> +private:\n> +\tstruct Header {\n> +\t\tuint32_t priv;\n> +\t\tuint32_t data;\n> +\t\tuint8_t fds;\n> +\t};\n> +\n> +\tint poll(int timeout);\n> +\n> +\tint sendData(const void *buffer, ssize_t length);\n> +\tint recvData(void *buffer, ssize_t length, int timeout);\n\nDoes length ever need to be negative ? If not, I would make it a size_t.\n\n> +\n> +\tint sendFds(const int32_t *fds, unsigned int num);\n> +\tint recvFds(int32_t *fds, unsigned int num, int timeout);\n> +\n> +\tint fd_;\n> +\tbool master_;\n> +};\n\nI think it could make sense to create an abstract IPC class, with\nIPCUnixSocket being a particular implementation.\n\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_IPC_UNIXSOCKET_H__ */\n> diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> new file mode 100644\n> index 0000000000000000..b34fa0317a18b37c\n> --- /dev/null\n> +++ b/src/libcamera/ipc_unixsocket.cpp\n> @@ -0,0 +1,330 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * ipc_unixsocket.cpp - IPC mechanism based on Unix sockets\n> + */\n> +\n> +#include \"ipc_unixsocket.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \"log.h\"\n> +\n> +/**\n> + * \\file ipc_unixsocket.h\n> + * \\brief IPC mechanism based on Unix sockets\n> + */\n> +\n> +/*\n> + * Markers to use in IPC protocol, there is no specific meaning to the values,\n> + * but they should be unique.\n> + */\n> +#define CMD_PING 0x1f\n> +#define CMD_PONG 0xf1\n> +#define CMD_FD 0x77\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPCUnixSocket)\n> +\n> +IPCUnixSocket::IPCUnixSocket()\n> +\t: fd_(-1), master_(false)\n> +{\n> +}\n> +\n> +IPCUnixSocket::IPCUnixSocket(int fd)\n> +\t: fd_(fd), master_(false)\n> +{\n> +}\n> +\n> +int IPCUnixSocket::create()\n> +{\n> +\tint sockets[2];\n> +\tint ret;\n> +\n> +\tret = socketpair(AF_UNIX, SOCK_STREAM, 0, sockets);\n> +\tif (ret) {\n> +\t\tret = -errno;\n> +\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t<< \"Failed to create socket pair: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tfd_ = sockets[0];\n> +\tmaster_ = true;\n> +\n> +\treturn sockets[1];\n> +}\n> +\n> +int IPCUnixSocket::connect()\n> +{\n> +\tPayload payload = {};\n> +\tPayload response = {};\n> +\n> +\tif (master_) {\n> +\t\tpayload.data.push_back(CMD_PING);\n> +\n> +\t\tif (call(payload, &response, 500))\n> +\t\t\treturn -1;\n> +\n> +\t\tif (response.data[0] != CMD_PONG)\n> +\t\t\treturn -1;\n> +\t} else {\n> +\t\tif (recv(&payload, 500))\n> +\t\t\treturn -1;\n> +\n> +\t\tif (payload.data[0] != CMD_PING)\n> +\t\t\treturn -1;\n> +\n> +\t\tresponse.data.push_back(CMD_PONG);\n> +\n> +\t\tif (send(response))\n> +\t\t\treturn -1;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +void IPCUnixSocket::close()\n> +{\n> +\tif (fd_ == -1)\n> +\t\treturn;\n> +\n> +\t::close(fd_);\n> +\n> +\tfd_ = -1;\n> +}\n> +\n> +int IPCUnixSocket::send(const Payload &payload)\n> +{\n> +\tHeader hdr;\n> +\tint ret;\n> +\n> +\tif (fd_ < 0)\n> +\t\treturn -ENOTCONN;\n> +\n> +\thdr.priv = payload.priv;\n> +\thdr.data = payload.data.size();\n> +\thdr.fds = payload.fds.size();\n> +\n> +\tret = sendData(&hdr, sizeof(hdr));\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tif (hdr.data) {\n> +\t\tret = sendData(payload.data.data(), hdr.data);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\tif (hdr.fds) {\n> +\t\tret = sendFds(payload.fds.data(), hdr.fds);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int IPCUnixSocket::recv(Payload *payload, int timeout)\n> +{\n> +\tHeader hdr;\n> +\tint ret;\n> +\n> +\tif (fd_ < 0)\n> +\t\treturn -ENOTCONN;\n> +\n> +\tret = recvData(&hdr, sizeof(hdr), timeout);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tpayload->priv = hdr.priv;\n> +\tpayload->data.resize(hdr.data);\n> +\tpayload->fds.resize(hdr.fds);\n> +\n> +\tif (hdr.data) {\n> +\t\tret = recvData(payload->data.data(), hdr.data, timeout);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\tif (hdr.fds) {\n> +\t\tret = recvFds(payload->fds.data(), hdr.fds, timeout);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int IPCUnixSocket::call(const Payload &payload, Payload *response, int timeout)\n> +{\n> +\tint ret = send(payload);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn recv(response, timeout);\n> +}\n> +\n> +int IPCUnixSocket::poll(int timeout)\n> +{\n> +\tstruct pollfd pollfd = { fd_, POLLIN, 0 };\n> +\n> +\tint ret = ::poll(&pollfd, 1, timeout);\n> +\tif (ret < 0) {\n> +\t\tret = -errno;\n> +\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t<< \"Failed to poll: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t} else if (ret == 0) {\n> +\t\treturn -ETIMEDOUT;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int IPCUnixSocket::sendData(const void *buffer, ssize_t length)\n> +{\n> +\tssize_t len, sent;\n> +\tconst uint8_t *pos;\n> +\n> +\tif (fd_ < 0)\n> +\t\treturn -ENOTCONN;\n> +\n\nThe caller already checks for this, so I think you can skip this check.\n\n> +\tpos = static_cast(buffer);\n> +\tlen = length;\n> +\n> +\twhile (len) {\n> +\t\tsent = ::send(fd_, pos, len, 0);\n> +\t\tif (sent < 0) {\n> +\t\t\tsent = -errno;\n> +\t\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t\t<< \"Failed to send: \" << strerror(-sent);\n> +\t\t\treturn sent;\n> +\t\t}\n> +\n> +\t\tpos += sent;\n> +\t\tlen -= sent;\n> +\t}\n\nIs the loop needed, can a message be fragmented over a UNIX socket ?\nAccording to the manpage of send(), \"If the message is too long to pass\natomically through the underlying protocol, the error EMSGSIZE is\nreturned, and the message is not transmitted.\"\n\n> +\n> +\treturn 0;\n> +}\n> +\n> +int IPCUnixSocket::recvData(void *buffer, ssize_t length, int timeout)\n> +{\n> +\tssize_t len, recived;\n\ns/recived/received/\n\n> +\tuint8_t *pos;\n> +\n> +\tif (fd_ < 0)\n> +\t\treturn -ENOTCONN;\n> +\n> +\tpos = static_cast(buffer);\n> +\tlen = length;\n> +\n> +\twhile (len) {\n> +\t\tint ret = poll(timeout);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\n\nPlease don't make blocking calls. You should use an event notifier to be\nnotified of incoming messages, and should then emit a signal with the\nreceived payload.\n\nThis will make the call() method more difficult to implement, but I\nthink we should be completely event-driven to avoid blocking event\nloops. The alternative would be threads, which we will likely end up\nusing eventually, but that's compatible with an event-driven approach.\n\n> +\t\trecived = ::recv(fd_, pos, len, 0);\n> +\t\tif (recived < 0) {\n> +\t\t\trecived = -errno;\n> +\t\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t\t<< \"Failed to recv: \" << strerror(-recived);\n> +\t\t\treturn recived;\n> +\t\t}\n> +\n> +\t\tpos += recived;\n> +\t\tlen -= recived;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int IPCUnixSocket::sendFds(const int32_t *fds, unsigned int num)\n> +{\n> +\tchar cmd = CMD_FD;\n> +\tstruct iovec iov[1];\n> +\tiov[0].iov_base = &cmd;\n> +\tiov[0].iov_len = sizeof(cmd);\n> +\n> +\tchar buf[CMSG_SPACE(num * sizeof(uint32_t))];\n> +\tmemset(buf, 0, sizeof(buf));\n> +\n> +\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> +\tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> +\tcmsg->cmsg_level = SOL_SOCKET;\n> +\tcmsg->cmsg_type = SCM_RIGHTS;\n> +\n> +\tstruct msghdr msg;\n> +\tmsg.msg_name = nullptr;\n> +\tmsg.msg_namelen = 0;\n> +\tmsg.msg_iov = iov;\n> +\tmsg.msg_iovlen = 1;\n> +\tmsg.msg_control = cmsg;\n> +\tmsg.msg_controllen = cmsg->cmsg_len;\n> +\tmsg.msg_flags = 0;\n> +\tmemcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t));\n> +\n> +\tif (sendmsg(fd_, &msg, 0) < 0) {\n> +\t\tint ret = -errno;\n> +\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t<< \"Failed to sendmsg: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n\nCouldn't you use sendmsg() to send both the payload data and the file\ndescriptors in a single message ? You would send a first datagram with\nthe header containing the command, data size and number of file\ndescriptors, and then a second datagram with the payload.\n\n> +\n> +\treturn 0;\n> +}\n> +\n> +int IPCUnixSocket::recvFds(int32_t *fds, unsigned int num, int timeout)\n> +{\n> +\tchar cmd;\n> +\tstruct iovec iov[1];\n> +\tiov[0].iov_base = &cmd;\n> +\tiov[0].iov_len = sizeof(cmd);\n> +\n> +\tchar buf[CMSG_SPACE(num * sizeof(uint32_t))];\n> +\tmemset(buf, 0, sizeof(buf));\n> +\n> +\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> +\tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> +\tcmsg->cmsg_level = SOL_SOCKET;\n> +\tcmsg->cmsg_type = SCM_RIGHTS;\n> +\n> +\tstruct msghdr msg;\n> +\tmsg.msg_name = nullptr;\n> +\tmsg.msg_namelen = 0;\n> +\tmsg.msg_iov = iov;\n> +\tmsg.msg_iovlen = 1;\n> +\tmsg.msg_control = cmsg;\n> +\tmsg.msg_controllen = cmsg->cmsg_len;\n> +\tmsg.msg_flags = 0;\n> +\n> +\tint ret = poll(timeout);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tif (recvmsg(fd_, &msg, 0) < 0) {\n> +\t\tint ret = -errno;\n> +\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t<< \"Failed to recvmsg: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tif (cmd != CMD_FD) {\n> +\t\tLOG(IPCUnixSocket, Error) << \"FD marker wrong\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tmemcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t));\n> +\n> +\treturn 0;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index f26ad5b2dc57c014..1158825fa5b0702d 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -13,6 +13,7 @@ libcamera_sources = files([\n> 'ipa_interface.cpp',\n> 'ipa_manager.cpp',\n> 'ipa_module.cpp',\n> + 'ipc_unixsocket.cpp',\n> 'log.cpp',\n> 'media_device.cpp',\n> 'media_object.cpp',\n> @@ -37,6 +38,7 @@ libcamera_headers = files([\n> 'include/formats.h',\n> 'include/ipa_manager.h',\n> 'include/ipa_module.h',\n> + 'include/ipc_unixsocket.h',\n> 'include/log.h',\n> 'include/media_device.h',\n> 'include/media_object.h',","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id AD2CB6002E\n\tfor ;\n\tSat, 22 Jun 2019 23:46:46 +0200 (CEST)","from pendragon.ideasonboard.com\n\t(dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi\n\t[IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2D4BC67;\n\tSat, 22 Jun 2019 23:46:46 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561240006;\n\tbh=8BXsN9O01UWXZaNAUWIRaZNqcBJIVNpvfi39eIM6u0I=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=PgQB3uyawbSbwVETV9NDr0VWi+esnxkgoV3nHehdTwCJXoXmnOrVNJ5/n4XumpZTn\n\t93OVdpLXVZier53+KPax3V9EOdCuTQ4kx9aglh+eIDTL5cFhstBRVCIEwRD07kaE4f\n\t2e9gCLW4meYdg9uCoSnTeVOORNosv/a/v/qARxpY=","Date":"Sun, 23 Jun 2019 00:46:28 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190622214628.GI8156@pendragon.ideasonboard.com>","References":"<20190621041519.29689-1-niklas.soderlund@ragnatech.se>\n\t<20190621041519.29689-2-niklas.soderlund@ragnatech.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190621041519.29689-2-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC 1/2] libcamera: ipc: unix: Add a IPC\n\tmechanism based on Unix sockets","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sat, 22 Jun 2019 21:46:46 -0000"}},{"id":1973,"web_url":"https://patchwork.libcamera.org/comment/1973/","msgid":"<20190623145303.GA32581@bigcity.dyn.berto.se>","date":"2019-06-23T14:53:03","subject":"Re: [libcamera-devel] [RFC 1/2] libcamera: ipc: unix: Add a IPC\n\tmechanism based on Unix sockets","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Laurent,\n\nThanks for your feedback.\n\nOn 2019-06-23 00:46:28 +0300, Laurent Pinchart wrote:\n> Hi Niklas,\n> \n> Thank you for the patch.\n> \n> On Fri, Jun 21, 2019 at 06:15:18AM +0200, Niklas Söderlund wrote:\n> > To be able to isolate an IPA component in a separate process and IPC\n> \n> s/and/an/\n> \n> > mechanism is needed to communicate with it. Add a IPC mechanism based on\n> \n> s/a IPC/an IPC/\n> \n> > Unix sockets which allows users to pass both data and file descriptors\n> > to to and from the IPA process.\n> \n> s/to to/to/\n> \n> > \n> > The implementation allows users to send both data and file descriptors\n> > in the same message. This allows users to more easily implement\n> > serialization and deserialization of objects as all elements belonging\n> > to an object can be sent in one message,\n> \n> s/,/./\n> \n> > \n> > The implementation guarantees that a whole object is transmitted and\n> > received over IPC before it's handed of. This allows IPC users to not\n> \n> What do you mean by handed of here ?\n\nWhat I tried to express is that messages are sent and received as a \nwhole unit and that users of the IPC class do not need to care about \nassembling partly transmitted messages.\n\n> \n> > have to deal with buffering and may depend on that it only needs to deal\n> > with serialization/deserialization of complete object blobs.\n> > \n> > The implementation also provides a priv field in the IPC message header\n> > which is a 32 bit integer that can be used by IPA implementations that\n> > do not require a complex protocol header to describe what type of\n> > message is transmitted.\n> \n> I'm not sure I would do that, wouldn't it make more sense to completely\n> separate the IPC transport from the protocol ? Otherwise, where will we\n> draw the line ?\n\nI don't feel strongly about this, and you might be right that providing \nthis one filed it becomes hard to draw the line. Will drop this field \nfor next version.\n\n> \n> > Signed-off-by: Niklas Söderlund \n> > ---\n> > src/libcamera/include/ipc_unixsocket.h | 58 +++++\n> > src/libcamera/ipc_unixsocket.cpp | 330 +++++++++++++++++++++++++\n> > src/libcamera/meson.build | 2 +\n> > 3 files changed, 390 insertions(+)\n> > create mode 100644 src/libcamera/include/ipc_unixsocket.h\n> > create mode 100644 src/libcamera/ipc_unixsocket.cpp\n> > \n> > diff --git a/src/libcamera/include/ipc_unixsocket.h b/src/libcamera/include/ipc_unixsocket.h\n> > new file mode 100644\n> > index 0000000000000000..864fa93b1f190fb7\n> > --- /dev/null\n> > +++ b/src/libcamera/include/ipc_unixsocket.h\n> > @@ -0,0 +1,58 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * ipc_unixsocket.h - IPC mechanism based on Unix sockets\n> > + */\n> > +\n> > +#ifndef __LIBCAMERA_IPC_UNIXSOCKET_H__\n> > +#define __LIBCAMERA_IPC_UNIXSOCKET_H__\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +namespace libcamera {\n> > +\n> > +class IPCUnixSocket\n> > +{\n> > +public:\n> > +\tstruct Payload {\n> > +\t\tuint32_t priv;\n> > +\t\tstd::vector data;\n> > +\t\tstd::vector fds;\n> > +\t};\n> > +\n> > +\tIPCUnixSocket();\n> > +\tIPCUnixSocket(int fd);\n> > +\n> > +\tint create();\n> > +\tint connect();\n> > +\tvoid close();\n> > +\n> > +\tint send(const Payload &payload);\n> > +\tint recv(Payload *payload, int timeout);\n> > +\tint call(const Payload &payload, Payload *response, int timeout);\n> > +\n> > +private:\n> > +\tstruct Header {\n> > +\t\tuint32_t priv;\n> > +\t\tuint32_t data;\n> > +\t\tuint8_t fds;\n> > +\t};\n> > +\n> > +\tint poll(int timeout);\n> > +\n> > +\tint sendData(const void *buffer, ssize_t length);\n> > +\tint recvData(void *buffer, ssize_t length, int timeout);\n> \n> Does length ever need to be negative ? If not, I would make it a size_t.\n> \n> > +\n> > +\tint sendFds(const int32_t *fds, unsigned int num);\n> > +\tint recvFds(int32_t *fds, unsigned int num, int timeout);\n> > +\n> > +\tint fd_;\n> > +\tbool master_;\n> > +};\n> \n> I think it could make sense to create an abstract IPC class, with\n> IPCUnixSocket being a particular implementation.\n\nI wondered about this when designing the class and my view is that this \nis not needed. I don't think designing IPA proxies that could work with \nany IPC mechanism is a good idea. Is there any other users of IPC then \nIPA proxies you can think of that could benefit from a baseclass \nimplementation?\n\n> \n> > +\n> > +} /* namespace libcamera */\n> > +\n> > +#endif /* __LIBCAMERA_IPC_UNIXSOCKET_H__ */\n> > diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> > new file mode 100644\n> > index 0000000000000000..b34fa0317a18b37c\n> > --- /dev/null\n> > +++ b/src/libcamera/ipc_unixsocket.cpp\n> > @@ -0,0 +1,330 @@\n> > +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * ipc_unixsocket.cpp - IPC mechanism based on Unix sockets\n> > + */\n> > +\n> > +#include \"ipc_unixsocket.h\"\n> > +\n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +#include \n> > +\n> > +#include \"log.h\"\n> > +\n> > +/**\n> > + * \\file ipc_unixsocket.h\n> > + * \\brief IPC mechanism based on Unix sockets\n> > + */\n> > +\n> > +/*\n> > + * Markers to use in IPC protocol, there is no specific meaning to the values,\n> > + * but they should be unique.\n> > + */\n> > +#define CMD_PING 0x1f\n> > +#define CMD_PONG 0xf1\n> > +#define CMD_FD 0x77\n> > +\n> > +namespace libcamera {\n> > +\n> > +LOG_DEFINE_CATEGORY(IPCUnixSocket)\n> > +\n> > +IPCUnixSocket::IPCUnixSocket()\n> > +\t: fd_(-1), master_(false)\n> > +{\n> > +}\n> > +\n> > +IPCUnixSocket::IPCUnixSocket(int fd)\n> > +\t: fd_(fd), master_(false)\n> > +{\n> > +}\n> > +\n> > +int IPCUnixSocket::create()\n> > +{\n> > +\tint sockets[2];\n> > +\tint ret;\n> > +\n> > +\tret = socketpair(AF_UNIX, SOCK_STREAM, 0, sockets);\n> > +\tif (ret) {\n> > +\t\tret = -errno;\n> > +\t\tLOG(IPCUnixSocket, Error)\n> > +\t\t\t<< \"Failed to create socket pair: \" << strerror(-ret);\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tfd_ = sockets[0];\n> > +\tmaster_ = true;\n> > +\n> > +\treturn sockets[1];\n> > +}\n> > +\n> > +int IPCUnixSocket::connect()\n> > +{\n> > +\tPayload payload = {};\n> > +\tPayload response = {};\n> > +\n> > +\tif (master_) {\n> > +\t\tpayload.data.push_back(CMD_PING);\n> > +\n> > +\t\tif (call(payload, &response, 500))\n> > +\t\t\treturn -1;\n> > +\n> > +\t\tif (response.data[0] != CMD_PONG)\n> > +\t\t\treturn -1;\n> > +\t} else {\n> > +\t\tif (recv(&payload, 500))\n> > +\t\t\treturn -1;\n> > +\n> > +\t\tif (payload.data[0] != CMD_PING)\n> > +\t\t\treturn -1;\n> > +\n> > +\t\tresponse.data.push_back(CMD_PONG);\n> > +\n> > +\t\tif (send(response))\n> > +\t\t\treturn -1;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +void IPCUnixSocket::close()\n> > +{\n> > +\tif (fd_ == -1)\n> > +\t\treturn;\n> > +\n> > +\t::close(fd_);\n> > +\n> > +\tfd_ = -1;\n> > +}\n> > +\n> > +int IPCUnixSocket::send(const Payload &payload)\n> > +{\n> > +\tHeader hdr;\n> > +\tint ret;\n> > +\n> > +\tif (fd_ < 0)\n> > +\t\treturn -ENOTCONN;\n> > +\n> > +\thdr.priv = payload.priv;\n> > +\thdr.data = payload.data.size();\n> > +\thdr.fds = payload.fds.size();\n> > +\n> > +\tret = sendData(&hdr, sizeof(hdr));\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tif (hdr.data) {\n> > +\t\tret = sendData(payload.data.data(), hdr.data);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\t}\n> > +\n> > +\tif (hdr.fds) {\n> > +\t\tret = sendFds(payload.fds.data(), hdr.fds);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int IPCUnixSocket::recv(Payload *payload, int timeout)\n> > +{\n> > +\tHeader hdr;\n> > +\tint ret;\n> > +\n> > +\tif (fd_ < 0)\n> > +\t\treturn -ENOTCONN;\n> > +\n> > +\tret = recvData(&hdr, sizeof(hdr), timeout);\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\tpayload->priv = hdr.priv;\n> > +\tpayload->data.resize(hdr.data);\n> > +\tpayload->fds.resize(hdr.fds);\n> > +\n> > +\tif (hdr.data) {\n> > +\t\tret = recvData(payload->data.data(), hdr.data, timeout);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\t}\n> > +\n> > +\tif (hdr.fds) {\n> > +\t\tret = recvFds(payload->fds.data(), hdr.fds, timeout);\n> > +\t\tif (ret)\n> > +\t\t\treturn ret;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int IPCUnixSocket::call(const Payload &payload, Payload *response, int timeout)\n> > +{\n> > +\tint ret = send(payload);\n> > +\tif (ret)\n> > +\t\treturn ret;\n> > +\n> > +\treturn recv(response, timeout);\n> > +}\n> > +\n> > +int IPCUnixSocket::poll(int timeout)\n> > +{\n> > +\tstruct pollfd pollfd = { fd_, POLLIN, 0 };\n> > +\n> > +\tint ret = ::poll(&pollfd, 1, timeout);\n> > +\tif (ret < 0) {\n> > +\t\tret = -errno;\n> > +\t\tLOG(IPCUnixSocket, Error)\n> > +\t\t\t<< \"Failed to poll: \" << strerror(-ret);\n> > +\t\treturn ret;\n> > +\t} else if (ret == 0) {\n> > +\t\treturn -ETIMEDOUT;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int IPCUnixSocket::sendData(const void *buffer, ssize_t length)\n> > +{\n> > +\tssize_t len, sent;\n> > +\tconst uint8_t *pos;\n> > +\n> > +\tif (fd_ < 0)\n> > +\t\treturn -ENOTCONN;\n> > +\n> \n> The caller already checks for this, so I think you can skip this check.\n> \n> > +\tpos = static_cast(buffer);\n> > +\tlen = length;\n> > +\n> > +\twhile (len) {\n> > +\t\tsent = ::send(fd_, pos, len, 0);\n> > +\t\tif (sent < 0) {\n> > +\t\t\tsent = -errno;\n> > +\t\t\tLOG(IPCUnixSocket, Error)\n> > +\t\t\t\t<< \"Failed to send: \" << strerror(-sent);\n> > +\t\t\treturn sent;\n> > +\t\t}\n> > +\n> > +\t\tpos += sent;\n> > +\t\tlen -= sent;\n> > +\t}\n> \n> Is the loop needed, can a message be fragmented over a UNIX socket ?\n> According to the manpage of send(), \"If the message is too long to pass\n> atomically through the underlying protocol, the error EMSGSIZE is\n> returned, and the message is not transmitted.\"\n\nYou are correct. I even checked this when writing the code but I must \nhave forgotten to remove the loop. Will drop form next version. Do you \nthink it's worth the effort to handle a EMSGSIZE error by breaking the \nmessage apart or shall we make this error Fatal and handle it if we \nendup trying to send very large messages?\n\n> \n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int IPCUnixSocket::recvData(void *buffer, ssize_t length, int timeout)\n> > +{\n> > +\tssize_t len, recived;\n> \n> s/recived/received/\n> \n> > +\tuint8_t *pos;\n> > +\n> > +\tif (fd_ < 0)\n> > +\t\treturn -ENOTCONN;\n> > +\n> > +\tpos = static_cast(buffer);\n> > +\tlen = length;\n> > +\n> > +\twhile (len) {\n> > +\t\tint ret = poll(timeout);\n> > +\t\tif (ret < 0)\n> > +\t\t\treturn ret;\n> > +\n> \n> Please don't make blocking calls. You should use an event notifier to be\n> notified of incoming messages, and should then emit a signal with the\n> received payload.\n> \n> This will make the call() method more difficult to implement, but I\n> think we should be completely event-driven to avoid blocking event\n> loops. The alternative would be threads, which we will likely end up\n> using eventually, but that's compatible with an event-driven approach.\n\nI toyed with event notifiers when designing this but once I started to \nmock implement the real world use-cases as I see them they felt not that \nuseful. I'm happy to be convinced otherwise. The way I see it there are \nonly two use-cases for our IPC.\n\n- Send statistics to IPA from pipeline handler\n\n Once a buffer with statistics are completed in the pipeline handler \n context it's event complete notifier should send() the data to the \n IPA. This is a fire and forget operation from the pipelines point of \n view and should not block.\n\n- Ask IPA for controls to apply when applying a request to hardware.\n\n Before a request is applied to hardware the IPA should be asked about \n how to translate libcamera controls to device controls. In this case \n the pipeline handler should call() the IPA to do this translation. In \n my head it makes sens that this would be a blocking call as the \n pipeline can't really do much before it gets the information from the \n IPA.\n\n Only thing that really could happen is that a new statistic buffers \n completes while the call is blocking. But I reasoned that this is not \n a problem as the IPA would need to have a lock to not update \n statistics while in the middle of translating libcamera controls to \n device controls.\n\nThat being said I'm not against making this event driven and as I write \nthis I could see there being some merit to it. But first lets hash out \nthe use-cases, maybe it will lead to the insight that moving to threads \ndirectly is the best solution.\n\n> \n> > +\t\trecived = ::recv(fd_, pos, len, 0);\n> > +\t\tif (recived < 0) {\n> > +\t\t\trecived = -errno;\n> > +\t\t\tLOG(IPCUnixSocket, Error)\n> > +\t\t\t\t<< \"Failed to recv: \" << strerror(-recived);\n> > +\t\t\treturn recived;\n> > +\t\t}\n> > +\n> > +\t\tpos += recived;\n> > +\t\tlen -= recived;\n> > +\t}\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int IPCUnixSocket::sendFds(const int32_t *fds, unsigned int num)\n> > +{\n> > +\tchar cmd = CMD_FD;\n> > +\tstruct iovec iov[1];\n> > +\tiov[0].iov_base = &cmd;\n> > +\tiov[0].iov_len = sizeof(cmd);\n> > +\n> > +\tchar buf[CMSG_SPACE(num * sizeof(uint32_t))];\n> > +\tmemset(buf, 0, sizeof(buf));\n> > +\n> > +\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> > +\tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> > +\tcmsg->cmsg_level = SOL_SOCKET;\n> > +\tcmsg->cmsg_type = SCM_RIGHTS;\n> > +\n> > +\tstruct msghdr msg;\n> > +\tmsg.msg_name = nullptr;\n> > +\tmsg.msg_namelen = 0;\n> > +\tmsg.msg_iov = iov;\n> > +\tmsg.msg_iovlen = 1;\n> > +\tmsg.msg_control = cmsg;\n> > +\tmsg.msg_controllen = cmsg->cmsg_len;\n> > +\tmsg.msg_flags = 0;\n> > +\tmemcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t));\n> > +\n> > +\tif (sendmsg(fd_, &msg, 0) < 0) {\n> > +\t\tint ret = -errno;\n> > +\t\tLOG(IPCUnixSocket, Error)\n> > +\t\t\t<< \"Failed to sendmsg: \" << strerror(-ret);\n> > +\t\treturn ret;\n> > +\t}\n> \n> Couldn't you use sendmsg() to send both the payload data and the file\n> descriptors in a single message ? You would send a first datagram with\n> the header containing the command, data size and number of file\n> descriptors, and then a second datagram with the payload.\n\nOne could do that. The reason I did not do this is two fold. \n\n- I have a notion that send()/recv() is faster then sendmsg()/recvmsg(), \n but I have no prof of this.\n- If we want to deal with messages that are too big to fit inside a \n single message I think it would be easier to handle that with \n send()/recv().\n\nDepending on what we wish to do with the EMSGSIZE question above I will \nact on this suggestion.\n\n> \n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +int IPCUnixSocket::recvFds(int32_t *fds, unsigned int num, int timeout)\n> > +{\n> > +\tchar cmd;\n> > +\tstruct iovec iov[1];\n> > +\tiov[0].iov_base = &cmd;\n> > +\tiov[0].iov_len = sizeof(cmd);\n> > +\n> > +\tchar buf[CMSG_SPACE(num * sizeof(uint32_t))];\n> > +\tmemset(buf, 0, sizeof(buf));\n> > +\n> > +\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> > +\tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> > +\tcmsg->cmsg_level = SOL_SOCKET;\n> > +\tcmsg->cmsg_type = SCM_RIGHTS;\n> > +\n> > +\tstruct msghdr msg;\n> > +\tmsg.msg_name = nullptr;\n> > +\tmsg.msg_namelen = 0;\n> > +\tmsg.msg_iov = iov;\n> > +\tmsg.msg_iovlen = 1;\n> > +\tmsg.msg_control = cmsg;\n> > +\tmsg.msg_controllen = cmsg->cmsg_len;\n> > +\tmsg.msg_flags = 0;\n> > +\n> > +\tint ret = poll(timeout);\n> > +\tif (ret < 0)\n> > +\t\treturn ret;\n> > +\n> > +\tif (recvmsg(fd_, &msg, 0) < 0) {\n> > +\t\tint ret = -errno;\n> > +\t\tLOG(IPCUnixSocket, Error)\n> > +\t\t\t<< \"Failed to recvmsg: \" << strerror(-ret);\n> > +\t\treturn ret;\n> > +\t}\n> > +\n> > +\tif (cmd != CMD_FD) {\n> > +\t\tLOG(IPCUnixSocket, Error) << \"FD marker wrong\";\n> > +\t\treturn -EINVAL;\n> > +\t}\n> > +\n> > +\tmemcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t));\n> > +\n> > +\treturn 0;\n> > +}\n> > +\n> > +} /* namespace libcamera */\n> > diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> > index f26ad5b2dc57c014..1158825fa5b0702d 100644\n> > --- a/src/libcamera/meson.build\n> > +++ b/src/libcamera/meson.build\n> > @@ -13,6 +13,7 @@ libcamera_sources = files([\n> > 'ipa_interface.cpp',\n> > 'ipa_manager.cpp',\n> > 'ipa_module.cpp',\n> > + 'ipc_unixsocket.cpp',\n> > 'log.cpp',\n> > 'media_device.cpp',\n> > 'media_object.cpp',\n> > @@ -37,6 +38,7 @@ libcamera_headers = files([\n> > 'include/formats.h',\n> > 'include/ipa_manager.h',\n> > 'include/ipa_module.h',\n> > + 'include/ipc_unixsocket.h',\n> > 'include/log.h',\n> > 'include/media_device.h',\n> > 'include/media_object.h',\n> \n> -- \n> Regards,\n> \n> Laurent Pinchart","headers":{"Return-Path":"","Received":["from mail-lj1-x243.google.com (mail-lj1-x243.google.com\n\t[IPv6:2a00:1450:4864:20::243])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 9B1AA6157E\n\tfor ;\n\tSun, 23 Jun 2019 16:53:06 +0200 (CEST)","by mail-lj1-x243.google.com with SMTP id p17so10178493ljg.1\n\tfor ;\n\tSun, 23 Jun 2019 07:53:06 -0700 (PDT)","from localhost (customer-145-14-112-32.stosn.net. [145.14.112.32])\n\tby smtp.gmail.com with ESMTPSA id\n\tx194sm1178714lfa.64.2019.06.23.07.53.04\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tSun, 23 Jun 2019 07:53:04 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=+dypnEp5in5CZVcPs0HD/x+r5bNw3vACUIOhQYXNIM8=;\n\tb=Z2HWlbF+jd1i61Jh4qaQQDpUGk1I3mUd+FwmYJhQpwEpb1R6djx4vmFpRNYKvbndhp\n\tL1vCS83PhBGkqriWQeqAb4CO6S6DaGlpovHiF2iO/HitWK7kJMnpe9iPT7QNca21qcoX\n\t7oxKGGCJXYNFKoQ25UbIB6SFpibsJDoAb8m1o6FgIHyoNkfhfBDuRKuJUpbzvlfO/l0L\n\ttqvwR/+O1LvUCcuVfwJ/1Wu3x0JHtX9XQllOBiLhjoloDJtVeMKkMUDHCRnhEKWFWr+Z\n\t/BWFQjK4vyFwntg2jA/LBtS8v9+AMEFwEd3+i3nSaHBslAnFLx9VRzGjMIm0p6ZI1fuk\n\t+X2A==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=+dypnEp5in5CZVcPs0HD/x+r5bNw3vACUIOhQYXNIM8=;\n\tb=n17na9LCle/xL+cPAhOJmpvXJquxeAhjMAPqQvrOzKjSrVmg5LDFTfSHxRsx546X9J\n\t0cpJInzqSaGyjhZen7/ekCE7H/BG8oA5hMT7XMoAknfkAITLGBOH/ect2/HmuHCuGc8l\n\tnuBOsDD4YkgkcCCtsER313YXpNqJTRiWK8IxkKI7jLBPI2ksrVrSpcc19tgYpa01Kr02\n\tTP8+aFzSswDZdsRUl49OzRebDUxhwATtfPyYJaYPoElp+mZa1Qq4CYrddkjJGu0S3bHO\n\tqp7IhO8CPt71fmNYnNYV19WLBmcUtzzYHd/DENmgVN51bm6tHjBdiE4SJXLpeuJ9XIW3\n\tc2yg==","X-Gm-Message-State":"APjAAAUbzq9/oemho8IhVtOlMqd/pVr6fwxKqOso7CWKr47w/u+tb29Y\n\tnobiMlGq65tvxzRQfRixSsyfjYjLnNs=","X-Google-Smtp-Source":"APXvYqwC9fh3Nb68JC4meuiHEZ5Mt9K7YRUJeRfH8U5s0TaOfWvdVaDLXfnKyr/Fq4+c8K/egctawQ==","X-Received":"by 2002:a2e:5d83:: with SMTP id\n\tv3mr11039989lje.196.1561301585490; \n\tSun, 23 Jun 2019 07:53:05 -0700 (PDT)","Date":"Sun, 23 Jun 2019 16:53:03 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= ","To":"Laurent Pinchart ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190623145303.GA32581@bigcity.dyn.berto.se>","References":"<20190621041519.29689-1-niklas.soderlund@ragnatech.se>\n\t<20190621041519.29689-2-niklas.soderlund@ragnatech.se>\n\t<20190622214628.GI8156@pendragon.ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190622214628.GI8156@pendragon.ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [RFC 1/2] libcamera: ipc: unix: Add a IPC\n\tmechanism based on Unix sockets","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sun, 23 Jun 2019 14:53:07 -0000"}},{"id":1985,"web_url":"https://patchwork.libcamera.org/comment/1985/","msgid":"<20190623230756.GH6124@pendragon.ideasonboard.com>","date":"2019-06-23T23:07:56","subject":"Re: [libcamera-devel] [RFC 1/2] libcamera: ipc: unix: Add a IPC\n\tmechanism based on Unix sockets","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nThank you for the patch.\n\nOn Sun, Jun 23, 2019 at 04:53:03PM +0200, Niklas Söderlund wrote:\n> On 2019-06-23 00:46:28 +0300, Laurent Pinchart wrote:\n> > On Fri, Jun 21, 2019 at 06:15:18AM +0200, Niklas Söderlund wrote:\n> >> To be able to isolate an IPA component in a separate process and IPC\n> > \n> > s/and/an/\n> > \n> >> mechanism is needed to communicate with it. Add a IPC mechanism based on\n> > \n> > s/a IPC/an IPC/\n> > \n> >> Unix sockets which allows users to pass both data and file descriptors\n> >> to to and from the IPA process.\n> > \n> > s/to to/to/\n> > \n> >> \n> >> The implementation allows users to send both data and file descriptors\n> >> in the same message. This allows users to more easily implement\n> >> serialization and deserialization of objects as all elements belonging\n> >> to an object can be sent in one message,\n> > \n> > s/,/./\n> > \n> >> \n> >> The implementation guarantees that a whole object is transmitted and\n> >> received over IPC before it's handed of. This allows IPC users to not\n> > \n> > What do you mean by handed of here ?\n> \n> What I tried to express is that messages are sent and received as a \n> whole unit and that users of the IPC class do not need to care about \n> assembling partly transmitted messages.\n> \n> >> have to deal with buffering and may depend on that it only needs to deal\n> >> with serialization/deserialization of complete object blobs.\n> >> \n> >> The implementation also provides a priv field in the IPC message header\n> >> which is a 32 bit integer that can be used by IPA implementations that\n> >> do not require a complex protocol header to describe what type of\n> >> message is transmitted.\n> > \n> > I'm not sure I would do that, wouldn't it make more sense to completely\n> > separate the IPC transport from the protocol ? Otherwise, where will we\n> > draw the line ?\n> \n> I don't feel strongly about this, and you might be right that providing \n> this one filed it becomes hard to draw the line. Will drop this field \n> for next version.\n> \n> >> Signed-off-by: Niklas Söderlund \n> >> ---\n> >> src/libcamera/include/ipc_unixsocket.h | 58 +++++\n> >> src/libcamera/ipc_unixsocket.cpp | 330 +++++++++++++++++++++++++\n> >> src/libcamera/meson.build | 2 +\n> >> 3 files changed, 390 insertions(+)\n> >> create mode 100644 src/libcamera/include/ipc_unixsocket.h\n> >> create mode 100644 src/libcamera/ipc_unixsocket.cpp\n> >> \n> >> diff --git a/src/libcamera/include/ipc_unixsocket.h b/src/libcamera/include/ipc_unixsocket.h\n> >> new file mode 100644\n> >> index 0000000000000000..864fa93b1f190fb7\n> >> --- /dev/null\n> >> +++ b/src/libcamera/include/ipc_unixsocket.h\n> >> @@ -0,0 +1,58 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2019, Google Inc.\n> >> + *\n> >> + * ipc_unixsocket.h - IPC mechanism based on Unix sockets\n> >> + */\n> >> +\n> >> +#ifndef __LIBCAMERA_IPC_UNIXSOCKET_H__\n> >> +#define __LIBCAMERA_IPC_UNIXSOCKET_H__\n> >> +\n> >> +#include \n> >> +#include \n> >> +#include \n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +class IPCUnixSocket\n> >> +{\n> >> +public:\n> >> +\tstruct Payload {\n> >> +\t\tuint32_t priv;\n> >> +\t\tstd::vector data;\n> >> +\t\tstd::vector fds;\n> >> +\t};\n> >> +\n> >> +\tIPCUnixSocket();\n> >> +\tIPCUnixSocket(int fd);\n> >> +\n> >> +\tint create();\n> >> +\tint connect();\n> >> +\tvoid close();\n> >> +\n> >> +\tint send(const Payload &payload);\n> >> +\tint recv(Payload *payload, int timeout);\n> >> +\tint call(const Payload &payload, Payload *response, int timeout);\n> >> +\n> >> +private:\n> >> +\tstruct Header {\n> >> +\t\tuint32_t priv;\n> >> +\t\tuint32_t data;\n> >> +\t\tuint8_t fds;\n> >> +\t};\n> >> +\n> >> +\tint poll(int timeout);\n> >> +\n> >> +\tint sendData(const void *buffer, ssize_t length);\n> >> +\tint recvData(void *buffer, ssize_t length, int timeout);\n> > \n> > Does length ever need to be negative ? If not, I would make it a size_t.\n> > \n> >> +\n> >> +\tint sendFds(const int32_t *fds, unsigned int num);\n> >> +\tint recvFds(int32_t *fds, unsigned int num, int timeout);\n> >> +\n> >> +\tint fd_;\n> >> +\tbool master_;\n> >> +};\n> > \n> > I think it could make sense to create an abstract IPC class, with\n> > IPCUnixSocket being a particular implementation.\n> \n> I wondered about this when designing the class and my view is that this \n> is not needed. I don't think designing IPA proxies that could work with \n> any IPC mechanism is a good idea. Is there any other users of IPC then \n> IPA proxies you can think of that could benefit from a baseclass \n> implementation?\n\nI can imagine proxies differing on both their IPC method and their\nprocess isolation method. We may end up with platform-specific proxies\n(Linux, ChromeOS, Android, ...) that each bundle a particular IPC with a\nparticular isolation method, but we may also realise that those proxies\nwould be identical in the remaining of their code. In that latter case\nhaving an abstract interface for the IPC could allow us to write a\nsingle proxy implementation that would select the IPC method at runtime\nbased on the platform (and possibly on a configuration file).\n\nI'm not saying that an abstract API is a requirement at this moment, but\nif it could be done, it may help. It would be easier to start with an\nabstract API and then decide to drop it and split the implementations\nthan starting with several IPC implementations and then having to\nrefactor them with a single API.\n\n> >> +\n> >> +} /* namespace libcamera */\n> >> +\n> >> +#endif /* __LIBCAMERA_IPC_UNIXSOCKET_H__ */\n> >> diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> >> new file mode 100644\n> >> index 0000000000000000..b34fa0317a18b37c\n> >> --- /dev/null\n> >> +++ b/src/libcamera/ipc_unixsocket.cpp\n> >> @@ -0,0 +1,330 @@\n> >> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> >> +/*\n> >> + * Copyright (C) 2019, Google Inc.\n> >> + *\n> >> + * ipc_unixsocket.cpp - IPC mechanism based on Unix sockets\n> >> + */\n> >> +\n> >> +#include \"ipc_unixsocket.h\"\n> >> +\n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +#include \n> >> +\n> >> +#include \"log.h\"\n> >> +\n> >> +/**\n> >> + * \\file ipc_unixsocket.h\n> >> + * \\brief IPC mechanism based on Unix sockets\n> >> + */\n> >> +\n> >> +/*\n> >> + * Markers to use in IPC protocol, there is no specific meaning to the values,\n> >> + * but they should be unique.\n> >> + */\n> >> +#define CMD_PING 0x1f\n> >> +#define CMD_PONG 0xf1\n> >> +#define CMD_FD 0x77\n> >> +\n> >> +namespace libcamera {\n> >> +\n> >> +LOG_DEFINE_CATEGORY(IPCUnixSocket)\n> >> +\n> >> +IPCUnixSocket::IPCUnixSocket()\n> >> +\t: fd_(-1), master_(false)\n> >> +{\n> >> +}\n> >> +\n> >> +IPCUnixSocket::IPCUnixSocket(int fd)\n> >> +\t: fd_(fd), master_(false)\n> >> +{\n> >> +}\n> >> +\n> >> +int IPCUnixSocket::create()\n> >> +{\n> >> +\tint sockets[2];\n> >> +\tint ret;\n> >> +\n> >> +\tret = socketpair(AF_UNIX, SOCK_STREAM, 0, sockets);\n> >> +\tif (ret) {\n> >> +\t\tret = -errno;\n> >> +\t\tLOG(IPCUnixSocket, Error)\n> >> +\t\t\t<< \"Failed to create socket pair: \" << strerror(-ret);\n> >> +\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\tfd_ = sockets[0];\n> >> +\tmaster_ = true;\n> >> +\n> >> +\treturn sockets[1];\n> >> +}\n> >> +\n> >> +int IPCUnixSocket::connect()\n> >> +{\n> >> +\tPayload payload = {};\n> >> +\tPayload response = {};\n> >> +\n> >> +\tif (master_) {\n> >> +\t\tpayload.data.push_back(CMD_PING);\n> >> +\n> >> +\t\tif (call(payload, &response, 500))\n> >> +\t\t\treturn -1;\n> >> +\n> >> +\t\tif (response.data[0] != CMD_PONG)\n> >> +\t\t\treturn -1;\n> >> +\t} else {\n> >> +\t\tif (recv(&payload, 500))\n> >> +\t\t\treturn -1;\n> >> +\n> >> +\t\tif (payload.data[0] != CMD_PING)\n> >> +\t\t\treturn -1;\n> >> +\n> >> +\t\tresponse.data.push_back(CMD_PONG);\n> >> +\n> >> +\t\tif (send(response))\n> >> +\t\t\treturn -1;\n> >> +\t}\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +void IPCUnixSocket::close()\n> >> +{\n> >> +\tif (fd_ == -1)\n> >> +\t\treturn;\n> >> +\n> >> +\t::close(fd_);\n> >> +\n> >> +\tfd_ = -1;\n> >> +}\n> >> +\n> >> +int IPCUnixSocket::send(const Payload &payload)\n> >> +{\n> >> +\tHeader hdr;\n> >> +\tint ret;\n> >> +\n> >> +\tif (fd_ < 0)\n> >> +\t\treturn -ENOTCONN;\n> >> +\n> >> +\thdr.priv = payload.priv;\n> >> +\thdr.data = payload.data.size();\n> >> +\thdr.fds = payload.fds.size();\n> >> +\n> >> +\tret = sendData(&hdr, sizeof(hdr));\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\tif (hdr.data) {\n> >> +\t\tret = sendData(payload.data.data(), hdr.data);\n> >> +\t\tif (ret)\n> >> +\t\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\tif (hdr.fds) {\n> >> +\t\tret = sendFds(payload.fds.data(), hdr.fds);\n> >> +\t\tif (ret)\n> >> +\t\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +int IPCUnixSocket::recv(Payload *payload, int timeout)\n> >> +{\n> >> +\tHeader hdr;\n> >> +\tint ret;\n> >> +\n> >> +\tif (fd_ < 0)\n> >> +\t\treturn -ENOTCONN;\n> >> +\n> >> +\tret = recvData(&hdr, sizeof(hdr), timeout);\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\tpayload->priv = hdr.priv;\n> >> +\tpayload->data.resize(hdr.data);\n> >> +\tpayload->fds.resize(hdr.fds);\n> >> +\n> >> +\tif (hdr.data) {\n> >> +\t\tret = recvData(payload->data.data(), hdr.data, timeout);\n> >> +\t\tif (ret)\n> >> +\t\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\tif (hdr.fds) {\n> >> +\t\tret = recvFds(payload->fds.data(), hdr.fds, timeout);\n> >> +\t\tif (ret)\n> >> +\t\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +int IPCUnixSocket::call(const Payload &payload, Payload *response, int timeout)\n> >> +{\n> >> +\tint ret = send(payload);\n> >> +\tif (ret)\n> >> +\t\treturn ret;\n> >> +\n> >> +\treturn recv(response, timeout);\n> >> +}\n> >> +\n> >> +int IPCUnixSocket::poll(int timeout)\n> >> +{\n> >> +\tstruct pollfd pollfd = { fd_, POLLIN, 0 };\n> >> +\n> >> +\tint ret = ::poll(&pollfd, 1, timeout);\n> >> +\tif (ret < 0) {\n> >> +\t\tret = -errno;\n> >> +\t\tLOG(IPCUnixSocket, Error)\n> >> +\t\t\t<< \"Failed to poll: \" << strerror(-ret);\n> >> +\t\treturn ret;\n> >> +\t} else if (ret == 0) {\n> >> +\t\treturn -ETIMEDOUT;\n> >> +\t}\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +int IPCUnixSocket::sendData(const void *buffer, ssize_t length)\n> >> +{\n> >> +\tssize_t len, sent;\n> >> +\tconst uint8_t *pos;\n> >> +\n> >> +\tif (fd_ < 0)\n> >> +\t\treturn -ENOTCONN;\n> >> +\n> > \n> > The caller already checks for this, so I think you can skip this check.\n> > \n> >> +\tpos = static_cast(buffer);\n> >> +\tlen = length;\n> >> +\n> >> +\twhile (len) {\n> >> +\t\tsent = ::send(fd_, pos, len, 0);\n> >> +\t\tif (sent < 0) {\n> >> +\t\t\tsent = -errno;\n> >> +\t\t\tLOG(IPCUnixSocket, Error)\n> >> +\t\t\t\t<< \"Failed to send: \" << strerror(-sent);\n> >> +\t\t\treturn sent;\n> >> +\t\t}\n> >> +\n> >> +\t\tpos += sent;\n> >> +\t\tlen -= sent;\n> >> +\t}\n> > \n> > Is the loop needed, can a message be fragmented over a UNIX socket ?\n> > According to the manpage of send(), \"If the message is too long to pass\n> > atomically through the underlying protocol, the error EMSGSIZE is\n> > returned, and the message is not transmitted.\"\n> \n> You are correct. I even checked this when writing the code but I must \n> have forgotten to remove the loop. Will drop form next version. Do you \n> think it's worth the effort to handle a EMSGSIZE error by breaking the \n> message apart or shall we make this error Fatal and handle it if we \n> endup trying to send very large messages?\n\nI'm tempted to handle fragmentation in the upper layer, but it's a good\nquestion. There are pros and cons in both cases I suppose. \n\nNow that I've written this, I realise you create sockets of type\nSOCK_STREAM, which allow fragmentation. Shouldn't we use SOCK_DGRAM\ninstead, as that should be more efficient ? Or maybe SOCK_SEQPACKET ?\n\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +int IPCUnixSocket::recvData(void *buffer, ssize_t length, int timeout)\n> >> +{\n> >> +\tssize_t len, recived;\n> > \n> > s/recived/received/\n> > \n> >> +\tuint8_t *pos;\n> >> +\n> >> +\tif (fd_ < 0)\n> >> +\t\treturn -ENOTCONN;\n> >> +\n> >> +\tpos = static_cast(buffer);\n> >> +\tlen = length;\n> >> +\n> >> +\twhile (len) {\n> >> +\t\tint ret = poll(timeout);\n> >> +\t\tif (ret < 0)\n> >> +\t\t\treturn ret;\n> >> +\n> > \n> > Please don't make blocking calls. You should use an event notifier to be\n> > notified of incoming messages, and should then emit a signal with the\n> > received payload.\n> > \n> > This will make the call() method more difficult to implement, but I\n> > think we should be completely event-driven to avoid blocking event\n> > loops. The alternative would be threads, which we will likely end up\n> > using eventually, but that's compatible with an event-driven approach.\n> \n> I toyed with event notifiers when designing this but once I started to \n> mock implement the real world use-cases as I see them they felt not that \n> useful. I'm happy to be convinced otherwise. The way I see it there are \n> only two use-cases for our IPC.\n\nPlease note that we shouldn't hardcode the use cases in the IPC API if\npossible, to make the class as reusable as possible. As discussed\nbefore, there's a risk we'll find out that isolation should be handled\non top of the Camera object API, which would require IPC usage with\nother use cases.\n\n> - Send statistics to IPA from pipeline handler\n> \n> Once a buffer with statistics are completed in the pipeline handler \n> context it's event complete notifier should send() the data to the \n> IPA. This is a fire and forget operation from the pipelines point of \n> view and should not block.\n\nNote that on the other side you should use an EventNotifier to receive\nthe statistics.\n\n> - Ask IPA for controls to apply when applying a request to hardware.\n> \n> Before a request is applied to hardware the IPA should be asked about \n> how to translate libcamera controls to device controls. In this case \n> the pipeline handler should call() the IPA to do this translation. In \n> my head it makes sens that this would be a blocking call as the \n> pipeline can't really do much before it gets the information from the \n> IPA.\n> \n> Only thing that really could happen is that a new statistic buffers \n> completes while the call is blocking. But I reasoned that this is not \n> a problem as the IPA would need to have a lock to not update \n> statistics while in the middle of translating libcamera controls to \n> device controls.\n\nIt's not entirely clear yet exactly how the pipeline handler and the IPA\nwill communicate. I do agree that synchronous blocking calls are\ntempting as they're easy, but they will also introduce delays that are\npotentially harmful. We will need to handle timeouts (which should be\nquite short), and I think an event-driven API would be safer.\n\n> That being said I'm not against making this event driven and as I write \n> this I could see there being some merit to it. But first lets hash out \n> the use-cases, maybe it will lead to the insight that moving to threads \n> directly is the best solution.\n> \n> >> +\t\trecived = ::recv(fd_, pos, len, 0);\n> >> +\t\tif (recived < 0) {\n> >> +\t\t\trecived = -errno;\n> >> +\t\t\tLOG(IPCUnixSocket, Error)\n> >> +\t\t\t\t<< \"Failed to recv: \" << strerror(-recived);\n> >> +\t\t\treturn recived;\n> >> +\t\t}\n> >> +\n> >> +\t\tpos += recived;\n> >> +\t\tlen -= recived;\n> >> +\t}\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +int IPCUnixSocket::sendFds(const int32_t *fds, unsigned int num)\n> >> +{\n> >> +\tchar cmd = CMD_FD;\n> >> +\tstruct iovec iov[1];\n> >> +\tiov[0].iov_base = &cmd;\n> >> +\tiov[0].iov_len = sizeof(cmd);\n> >> +\n> >> +\tchar buf[CMSG_SPACE(num * sizeof(uint32_t))];\n> >> +\tmemset(buf, 0, sizeof(buf));\n> >> +\n> >> +\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> >> +\tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> >> +\tcmsg->cmsg_level = SOL_SOCKET;\n> >> +\tcmsg->cmsg_type = SCM_RIGHTS;\n> >> +\n> >> +\tstruct msghdr msg;\n> >> +\tmsg.msg_name = nullptr;\n> >> +\tmsg.msg_namelen = 0;\n> >> +\tmsg.msg_iov = iov;\n> >> +\tmsg.msg_iovlen = 1;\n> >> +\tmsg.msg_control = cmsg;\n> >> +\tmsg.msg_controllen = cmsg->cmsg_len;\n> >> +\tmsg.msg_flags = 0;\n> >> +\tmemcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t));\n> >> +\n> >> +\tif (sendmsg(fd_, &msg, 0) < 0) {\n> >> +\t\tint ret = -errno;\n> >> +\t\tLOG(IPCUnixSocket, Error)\n> >> +\t\t\t<< \"Failed to sendmsg: \" << strerror(-ret);\n> >> +\t\treturn ret;\n> >> +\t}\n> > \n> > Couldn't you use sendmsg() to send both the payload data and the file\n> > descriptors in a single message ? You would send a first datagram with\n> > the header containing the command, data size and number of file\n> > descriptors, and then a second datagram with the payload.\n> \n> One could do that. The reason I did not do this is two fold. \n> \n> - I have a notion that send()/recv() is faster then sendmsg()/recvmsg(), \n> but I have no prof of this.\n\nI wouldn't expect that. If it's true we can take the issue in\nconsideration, but I think that send(header) + send(data) + sendmsg(fds)\nwill always be slower than send(header) + sendmsg(data + fds). And if\nyou care about performances, sendmmsg() may be interesting.\n\n> - If we want to deal with messages that are too big to fit inside a \n> single message I think it would be easier to handle that with \n> send()/recv().\n> \n> Depending on what we wish to do with the EMSGSIZE question above I will \n> act on this suggestion.\n\nWhat's the size limit for messages with SOCK_DGRAM or SOCK_SEQPACKET ?\n\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +int IPCUnixSocket::recvFds(int32_t *fds, unsigned int num, int timeout)\n> >> +{\n> >> +\tchar cmd;\n> >> +\tstruct iovec iov[1];\n> >> +\tiov[0].iov_base = &cmd;\n> >> +\tiov[0].iov_len = sizeof(cmd);\n> >> +\n> >> +\tchar buf[CMSG_SPACE(num * sizeof(uint32_t))];\n> >> +\tmemset(buf, 0, sizeof(buf));\n> >> +\n> >> +\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> >> +\tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> >> +\tcmsg->cmsg_level = SOL_SOCKET;\n> >> +\tcmsg->cmsg_type = SCM_RIGHTS;\n> >> +\n> >> +\tstruct msghdr msg;\n> >> +\tmsg.msg_name = nullptr;\n> >> +\tmsg.msg_namelen = 0;\n> >> +\tmsg.msg_iov = iov;\n> >> +\tmsg.msg_iovlen = 1;\n> >> +\tmsg.msg_control = cmsg;\n> >> +\tmsg.msg_controllen = cmsg->cmsg_len;\n> >> +\tmsg.msg_flags = 0;\n> >> +\n> >> +\tint ret = poll(timeout);\n> >> +\tif (ret < 0)\n> >> +\t\treturn ret;\n> >> +\n> >> +\tif (recvmsg(fd_, &msg, 0) < 0) {\n> >> +\t\tint ret = -errno;\n> >> +\t\tLOG(IPCUnixSocket, Error)\n> >> +\t\t\t<< \"Failed to recvmsg: \" << strerror(-ret);\n> >> +\t\treturn ret;\n> >> +\t}\n> >> +\n> >> +\tif (cmd != CMD_FD) {\n> >> +\t\tLOG(IPCUnixSocket, Error) << \"FD marker wrong\";\n> >> +\t\treturn -EINVAL;\n> >> +\t}\n> >> +\n> >> +\tmemcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t));\n> >> +\n> >> +\treturn 0;\n> >> +}\n> >> +\n> >> +} /* namespace libcamera */\n> >> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> >> index f26ad5b2dc57c014..1158825fa5b0702d 100644\n> >> --- a/src/libcamera/meson.build\n> >> +++ b/src/libcamera/meson.build\n> >> @@ -13,6 +13,7 @@ libcamera_sources = files([\n> >> 'ipa_interface.cpp',\n> >> 'ipa_manager.cpp',\n> >> 'ipa_module.cpp',\n> >> + 'ipc_unixsocket.cpp',\n> >> 'log.cpp',\n> >> 'media_device.cpp',\n> >> 'media_object.cpp',\n> >> @@ -37,6 +38,7 @@ libcamera_headers = files([\n> >> 'include/formats.h',\n> >> 'include/ipa_manager.h',\n> >> 'include/ipa_module.h',\n> >> + 'include/ipc_unixsocket.h',\n> >> 'include/log.h',\n> >> 'include/media_device.h',\n> >> 'include/media_object.h',","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 264326157E\n\tfor ;\n\tMon, 24 Jun 2019 01:10:27 +0200 (CEST)","from pendragon.ideasonboard.com (81-175-216-236.bb.dnainternet.fi\n\t[81.175.216.236])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 8C24C2E7;\n\tMon, 24 Jun 2019 01:10:26 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561331426;\n\tbh=5Xiyavxau2ILU8vNCDvMmmpCFse9ZlFaqGO9anhfHg8=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=FmMmFbPYvVWdjPhhx19xht2P4sPDfP8koqk90Ef8iu4oWW3QQNUEg1C0H5WZ+PS7O\n\tp61XrUfcllqNL8VIbXLHXiV6WYQEyTryFjRbSLSHchF25PywzZRolAHWIZSbG+u1ga\n\tr64ioNuowqj8ri4hFiNl0UBkMtppdQkIutbV8/00=","Date":"Mon, 24 Jun 2019 02:07:56 +0300","From":"Laurent Pinchart ","To":"Niklas =?utf-8?q?S=C3=B6derlund?= ","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190623230756.GH6124@pendragon.ideasonboard.com>","References":"<20190621041519.29689-1-niklas.soderlund@ragnatech.se>\n\t<20190621041519.29689-2-niklas.soderlund@ragnatech.se>\n\t<20190622214628.GI8156@pendragon.ideasonboard.com>\n\t<20190623145303.GA32581@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190623145303.GA32581@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [RFC 1/2] libcamera: ipc: unix: Add a IPC\n\tmechanism based on Unix sockets","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Sun, 23 Jun 2019 23:10:27 -0000"}},{"id":1986,"web_url":"https://patchwork.libcamera.org/comment/1986/","msgid":"","date":"2019-06-24T07:44:08","subject":"Re: [libcamera-devel] [RFC 1/2] libcamera: ipc: unix: Add a IPC\n\tmechanism based on Unix sockets","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nGreat I'm very pleased to see this RFC!\n - thanks for tackling this topic!\n\nOn 21/06/2019 05:15, Niklas Söderlund wrote:\n> To be able to isolate an IPA component in a separate process and IPC\n> mechanism is needed to communicate with it. Add a IPC mechanism based on\n> Unix sockets which allows users to pass both data and file descriptors\n> to to and from the IPA process.\n> \n> The implementation allows users to send both data and file descriptors\n> in the same message. This allows users to more easily implement\n> serialization and deserialization of objects as all elements belonging\n> to an object can be sent in one message,\n> \n> The implementation guarantees that a whole object is transmitted and\n> received over IPC before it's handed of. This allows IPC users to not\n> have to deal with buffering and may depend on that it only needs to deal\n> with serialization/deserialization of complete object blobs.\n> \n> The implementation also provides a priv field in the IPC message header\n> which is a 32 bit integer that can be used by IPA implementations that\n> do not require a complex protocol header to describe what type of\n> message is transmitted.\n> \n> Signed-off-by: Niklas Söderlund \n> ---\n> src/libcamera/include/ipc_unixsocket.h | 58 +++++\n> src/libcamera/ipc_unixsocket.cpp | 330 +++++++++++++++++++++++++\n> src/libcamera/meson.build | 2 +\n> 3 files changed, 390 insertions(+)\n> create mode 100644 src/libcamera/include/ipc_unixsocket.h\n> create mode 100644 src/libcamera/ipc_unixsocket.cpp\n> \n> diff --git a/src/libcamera/include/ipc_unixsocket.h b/src/libcamera/include/ipc_unixsocket.h\n> new file mode 100644\n> index 0000000000000000..864fa93b1f190fb7\n> --- /dev/null\n> +++ b/src/libcamera/include/ipc_unixsocket.h\n> @@ -0,0 +1,58 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * ipc_unixsocket.h - IPC mechanism based on Unix sockets\n> + */\n> +\n> +#ifndef __LIBCAMERA_IPC_UNIXSOCKET_H__\n> +#define __LIBCAMERA_IPC_UNIXSOCKET_H__\n> +\n> +#include \n> +#include \n> +#include \n> +\n> +namespace libcamera {\n> +\n> +class IPCUnixSocket\n> +{\n> +public:\n> +\tstruct Payload {\n> +\t\tuint32_t priv;\n> +\t\tstd::vector data;\n> +\t\tstd::vector fds;\n> +\t};\n> +\n> +\tIPCUnixSocket();\n> +\tIPCUnixSocket(int fd);\n> +\n> +\tint create();\n> +\tint connect();\n> +\tvoid close();\n> +\n> +\tint send(const Payload &payload);\n> +\tint recv(Payload *payload, int timeout);\n> +\tint call(const Payload &payload, Payload *response, int timeout);\n> +\n> +private:\n> +\tstruct Header {\n> +\t\tuint32_t priv;\n> +\t\tuint32_t data;\n> +\t\tuint8_t fds;\n> +\t};\n> +\n> +\tint poll(int timeout);\n> +\n> +\tint sendData(const void *buffer, ssize_t length);\n> +\tint recvData(void *buffer, ssize_t length, int timeout);\n> +\n> +\tint sendFds(const int32_t *fds, unsigned int num);\n> +\tint recvFds(int32_t *fds, unsigned int num, int timeout);\n> +\n> +\tint fd_;\n> +\tbool master_;\n> +};\n> +\n> +} /* namespace libcamera */\n> +\n> +#endif /* __LIBCAMERA_IPC_UNIXSOCKET_H__ */\n> diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp\n> new file mode 100644\n> index 0000000000000000..b34fa0317a18b37c\n> --- /dev/null\n> +++ b/src/libcamera/ipc_unixsocket.cpp\n> @@ -0,0 +1,330 @@\n> +/* SPDX-License-Identifier: LGPL-2.1-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * ipc_unixsocket.cpp - IPC mechanism based on Unix sockets\n> + */\n> +\n> +#include \"ipc_unixsocket.h\"\n> +\n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +#include \n> +\n> +#include \"log.h\"\n> +\n> +/**\n> + * \\file ipc_unixsocket.h\n> + * \\brief IPC mechanism based on Unix sockets\n> + */\n> +\n> +/*\n> + * Markers to use in IPC protocol, there is no specific meaning to the values,\n> + * but they should be unique.\n> + */\n> +#define CMD_PING 0x1f\n> +#define CMD_PONG 0xf1\n> +#define CMD_FD 0x77\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPCUnixSocket)\n> +\n> +IPCUnixSocket::IPCUnixSocket()\n> +\t: fd_(-1), master_(false)\n> +{\n> +}\n> +\n> +IPCUnixSocket::IPCUnixSocket(int fd)\n> +\t: fd_(fd), master_(false)\n> +{\n> +}\n\nTrivial - but I think you could merge those two constructors with a\ndefault value for \"int fd=-1\" on the function prototype.\n\n> +\n> +int IPCUnixSocket::create()\n> +{\n> +\tint sockets[2];\n> +\tint ret;\n> +\n> +\tret = socketpair(AF_UNIX, SOCK_STREAM, 0, sockets);\n> +\tif (ret) {\n> +\t\tret = -errno;\n> +\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t<< \"Failed to create socket pair: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tfd_ = sockets[0];\n> +\tmaster_ = true;\n> +\n> +\treturn sockets[1];\n> +}\n> +\n> +int IPCUnixSocket::connect()\n> +{\n> +\tPayload payload = {};\n\nI'd call this 'Payload message', as the one below is 'Payload response'.\nOtherwise we're doing Int int.\n\n\n> +\tPayload response = {};\n> +\n> +\tif (master_) {\n> +\t\tpayload.data.push_back(CMD_PING);\n> +\n> +\t\tif (call(payload, &response, 500))\n> +\t\t\treturn -1;\n> +\n> +\t\tif (response.data[0] != CMD_PONG)\n> +\t\t\treturn -1;\n> +\t} else {\n> +\t\tif (recv(&payload, 500))\n> +\t\t\treturn -1;\n> +\n> +\t\tif (payload.data[0] != CMD_PING)\n> +\t\t\treturn -1;\n> +\n> +\t\tresponse.data.push_back(CMD_PONG);\n> +\n> +\t\tif (send(response))\n> +\t\t\treturn -1;\n> +\t}\n\nThis doesn't really seem like a 'connect()' call?\nIt's more of a ping test?\n\nIs it required to establish communication between the sockets?\n\nI think the SocketPair does everything necessary to establish the link\nbetween the two sockets - so perhaps this is just to start to define a\nbase IPC API ?\n\n(If so -that should really be in the pure-virtual base class header, and\nthis function implemented as a noop or such).\n\n\n> +\n> +\treturn 0;\n> +}\n> +\n> +void IPCUnixSocket::close()\n> +{\n> +\tif (fd_ == -1)\n> +\t\treturn;\n> +\n> +\t::close(fd_);\n> +\n> +\tfd_ = -1;\n> +}\n> +\n> +int IPCUnixSocket::send(const Payload &payload)\n> +{\n> +\tHeader hdr;\n> +\tint ret;\n> +\n> +\tif (fd_ < 0)\n> +\t\treturn -ENOTCONN;\n> +\n> +\thdr.priv = payload.priv;\n> +\thdr.data = payload.data.size();\n> +\thdr.fds = payload.fds.size();\n> +\n> +\tret = sendData(&hdr, sizeof(hdr));\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tif (hdr.data) {\n> +\t\tret = sendData(payload.data.data(), hdr.data);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\tif (hdr.fds) {\n> +\t\tret = sendFds(payload.fds.data(), hdr.fds);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int IPCUnixSocket::recv(Payload *payload, int timeout)\n\ns/recv/receive/ ?\n\n> +{\n> +\tHeader hdr;\n> +\tint ret;\n> +\n> +\tif (fd_ < 0)\n> +\t\treturn -ENOTCONN;\n> +\n> +\tret = recvData(&hdr, sizeof(hdr), timeout);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\tpayload->priv = hdr.priv;\n> +\tpayload->data.resize(hdr.data);\n> +\tpayload->fds.resize(hdr.fds);\n> +\n> +\tif (hdr.data) {\n> +\t\tret = recvData(payload->data.data(), hdr.data, timeout);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\tif (hdr.fds) {\n> +\t\tret = recvFds(payload->fds.data(), hdr.fds, timeout);\n> +\t\tif (ret)\n> +\t\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int IPCUnixSocket::call(const Payload &payload, Payload *response, int timeout)\n> +{\n> +\tint ret = send(payload);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn recv(response, timeout);\n> +}\n> +\n> +int IPCUnixSocket::poll(int timeout)\n> +{\n> +\tstruct pollfd pollfd = { fd_, POLLIN, 0 };\n> +\n> +\tint ret = ::poll(&pollfd, 1, timeout);\n> +\tif (ret < 0) {\n> +\t\tret = -errno;\n> +\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t<< \"Failed to poll: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t} else if (ret == 0) {\n> +\t\treturn -ETIMEDOUT;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int IPCUnixSocket::sendData(const void *buffer, ssize_t length)\n> +{\n> +\tssize_t len, sent;\n> +\tconst uint8_t *pos;\n> +\n> +\tif (fd_ < 0)\n> +\t\treturn -ENOTCONN;\n> +\n> +\tpos = static_cast(buffer);\n> +\tlen = length;\n> +\n> +\twhile (len) {\n> +\t\tsent = ::send(fd_, pos, len, 0);\n> +\t\tif (sent < 0) {\n> +\t\t\tsent = -errno;\n> +\t\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t\t<< \"Failed to send: \" << strerror(-sent);\n> +\t\t\treturn sent;\n> +\t\t}\n> +\n> +\t\tpos += sent;\n> +\t\tlen -= sent;\n> +\t}\n> +\n> +\treturn 0;\n> +}If it's P\n> +\n> +int IPCUnixSocket::recvData(void *buffer, ssize_t length, int timeout)\n> +{\n> +\tssize_t len, recived;\n> +\tuint8_t *pos;\n> +\n> +\tif (fd_ < 0)\n> +\t\treturn -ENOTCONN;\n> +\n> +\tpos = static_cast(buffer);\n> +\tlen = length;\n> +\n> +\twhile (len) {\n> +\t\tint ret = poll(timeout);\n> +\t\tif (ret < 0)\n> +\t\t\treturn ret;\n> +\n> +\t\trecived = ::recv(fd_, pos, len, 0);\n> +\t\tif (recived < 0) {\n> +\t\t\trecived = -errno;\n> +\t\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t\t<< \"Failed to recv: \" << strerror(-recived);\n> +\t\t\treturn recived;\n> +\t\t}\n> +\n> +\t\tpos += recived;\n> +\t\tlen -= recived;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int IPCUnixSocket::sendFds(const int32_t *fds, unsigned int num)\n> +{\n> +\tchar cmd = CMD_FD;\n> +\tstruct iovec iov[1];\n> +\tiov[0].iov_base = &cmd;\n> +\tiov[0].iov_len = sizeof(cmd);\n> +\n> +\tchar buf[CMSG_SPACE(num * sizeof(uint32_t))];\n> +\tmemset(buf, 0, sizeof(buf));\n> +\n> +\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> +\tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> +\tcmsg->cmsg_level = SOL_SOCKET;\n> +\tcmsg->cmsg_type = SCM_RIGHTS;\n> +\n> +\tstruct msghdr msg;\n> +\tmsg.msg_name = nullptr;\n> +\tmsg.msg_namelen = 0;\n> +\tmsg.msg_iov = iov;\n> +\tmsg.msg_iovlen = 1;\n> +\tmsg.msg_control = cmsg;\n> +\tmsg.msg_controllen = cmsg->cmsg_len;\n> +\tmsg.msg_flags = 0;\n> +\tmemcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t));\n> +\n> +\tif (sendmsg(fd_, &msg, 0) < 0) {\n> +\t\tint ret = -errno;\n> +\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t<< \"Failed to sendmsg: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn 0;\n> +}\n> +\n> +int IPCUnixSocket::recvFds(int32_t *fds, unsigned int num, int timeout)\n> +{\n> +\tchar cmd;\n> +\tstruct iovec iov[1];\n> +\tiov[0].iov_base = &cmd;\n> +\tiov[0].iov_len = sizeof(cmd);\n> +\n> +\tchar buf[CMSG_SPACE(num * sizeof(uint32_t))];\n> +\tmemset(buf, 0, sizeof(buf));\n> +\n> +\tstruct cmsghdr *cmsg = (struct cmsghdr *)buf;\n> +\tcmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t));\n> +\tcmsg->cmsg_level = SOL_SOCKET;\n> +\tcmsg->cmsg_type = SCM_RIGHTS;\n> +\n> +\tstruct msghdr msg;\n> +\tmsg.msg_name = nullptr;\n> +\tmsg.msg_namelen = 0;\n> +\tmsg.msg_iov = iov;\n> +\tmsg.msg_iovlen = 1;\n> +\tmsg.msg_control = cmsg;\n> +\tmsg.msg_controllen = cmsg->cmsg_len;\n> +\tmsg.msg_flags = 0;\n> +\n> +\tint ret = poll(timeout);\n> +\tif (ret < 0)\n> +\t\treturn ret;\n> +\n> +\tif (recvmsg(fd_, &msg, 0) < 0) {\n> +\t\tint ret = -errno;\n> +\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t<< \"Failed to recvmsg: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tif (cmd != CMD_FD) {\n> +\t\tLOG(IPCUnixSocket, Error) << \"FD marker wrong\";\n> +\t\treturn -EINVAL;\n> +\t}\n> +\n> +\tmemcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t));\n> +\n> +\treturn 0;\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index f26ad5b2dc57c014..1158825fa5b0702d 100644\n> --- a/src/libcamera/meson.build\n> +++ b/src/libcamera/meson.build\n> @@ -13,6 +13,7 @@ libcamera_sources = files([\n> 'ipa_interface.cpp',\n> 'ipa_manager.cpp',\n> 'ipa_module.cpp',\n> + 'ipc_unixsocket.cpp',\n> 'log.cpp',\n> 'media_device.cpp',\n> 'media_object.cpp',\n> @@ -37,6 +38,7 @@ libcamera_headers = files([\n> 'include/formats.h',\n> 'include/ipa_manager.h',\n> 'include/ipa_module.h',\n> + 'include/ipc_unixsocket.h',\n> 'include/log.h',\n> 'include/media_device.h',\n> 'include/media_object.h',\n>","headers":{"Return-Path":"","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id CF56D60C29\n\tfor ;\n\tMon, 24 Jun 2019 09:44:12 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 2B5FD2E7;\n\tMon, 24 Jun 2019 09:44:12 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561362252;\n\tbh=UD4t2cJPQNfP024rPsyuCaLBLG371uknyfKV+u5gAiQ=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=WEvonZKOuouCkIKViaPB3N7ZTHtvhIJaWaekcK8uR+7Qcha1s8ZRK3jj2QDWLW8/6\n\t+phy1kLU0TLEficgs+24s/kyyslL+H4sCXkssTpzayCSx52bZnm0VcRY1SfFdRqjv+\n\tD9YM24z0GyOTEmtkDY9vOn1VJDCN96+gWrcAk+Ko=","Reply-To":"kieran.bingham@ideasonboard.com","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= ,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190621041519.29689-1-niklas.soderlund@ragnatech.se>\n\t<20190621041519.29689-2-niklas.soderlund@ragnatech.se>","From":"Kieran Bingham ","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"","Date":"Mon, 24 Jun 2019 08:44:08 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<20190621041519.29689-2-niklas.soderlund@ragnatech.se>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [RFC 1/2] libcamera: ipc: unix: Add a IPC\n\tmechanism based on Unix sockets","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"","List-Unsubscribe":",\n\t","List-Archive":"","List-Post":"","List-Help":"","List-Subscribe":",\n\t","X-List-Received-Date":"Mon, 24 Jun 2019 07:44:13 -0000"}}]