[{"id":2093,"web_url":"https://patchwork.libcamera.org/comment/2093/","msgid":"<20190701220936.GB24839@pendragon.ideasonboard.com>","date":"2019-07-01T22:09:36","subject":"Re: [libcamera-devel] [PATCH v3 1/2] libcamera: ipc: unix: Add a\n\tIPC mechanism 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 Tue, Jul 02, 2019 at 12:06:11AM +0200, Niklas Söderlund wrote:\n> To be able to isolate an IPA component in a separate process an IPC\n> mechanism is needed to communicate with it. Add an IPC mechanism based\n> on Unix sockets which allows users to pass both data and file descriptors\n> 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> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n\nReviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>\n\n> ---\n>  src/libcamera/include/ipc_unixsocket.h |  57 +++++\n>  src/libcamera/ipc_unixsocket.cpp       | 309 +++++++++++++++++++++++++\n>  src/libcamera/meson.build              |   2 +\n>  3 files changed, 368 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..ef166d7425549102\n> --- /dev/null\n> +++ b/src/libcamera/include/ipc_unixsocket.h\n> @@ -0,0 +1,57 @@\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 <cstdint>\n> +#include <sys/types.h>\n> +#include <vector>\n> +\n> +#include <libcamera/event_notifier.h>\n> +\n> +namespace libcamera {\n> +\n> +class IPCUnixSocket\n> +{\n> +public:\n> +\tstruct Payload {\n> +\t\tstd::vector<uint8_t> data;\n> +\t\tstd::vector<int32_t> fds;\n> +\t};\n> +\n> +\tIPCUnixSocket();\n> +\t~IPCUnixSocket();\n> +\n> +\tint create();\n> +\tint bind(int fd);\n> +\tvoid close();\n> +\tbool isBound() const;\n> +\n> +\tint send(const Payload &payload);\n> +\tint receive(Payload *payload);\n> +\n> +\tSignal<IPCUnixSocket *> readyRead;\n> +\n> +private:\n> +\tstruct Header {\n> +\t\tuint32_t data;\n> +\t\tuint8_t fds;\n> +\t};\n> +\n> +\tint sendData(const void *buffer, size_t length, const int32_t *fds, unsigned int num);\n> +\tint recvData(void *buffer, size_t length, int32_t *fds, unsigned int num);\n> +\n> +\tvoid dataNotifier(EventNotifier *notifier);\n> +\n> +\tint fd_;\n> +\tEventNotifier *notifier_;\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..c11f116093c51b0d\n> --- /dev/null\n> +++ b/src/libcamera/ipc_unixsocket.cpp\n> @@ -0,0 +1,309 @@\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 <string.h>\n> +#include <sys/socket.h>\n> +#include <unistd.h>\n> +\n> +#include \"log.h\"\n> +\n> +/**\n> + * \\file ipc_unixsocket.h\n> + * \\brief IPC mechanism based on Unix sockets\n> + */\n> +\n> +namespace libcamera {\n> +\n> +LOG_DEFINE_CATEGORY(IPCUnixSocket)\n> +\n> +/**\n> + * \\struct IPCUnixSocket::Payload\n> + * \\brief Container for an IPC payload\n> + *\n> + * Holds an array of bytes and an array of file descriptors that can be\n> + * transported across a IPC boundary.\n> + */\n> +\n> +/**\n> + * \\var IPCUnixSocket::Payload::data\n> + * \\brief Array of bytes to cross IPC boundary\n> + */\n> +\n> +/**\n> + * \\var IPCUnixSocket::Payload::fds\n> + * \\brief Array of file descriptors to cross IPC boundary\n> + */\n> +\n> +/**\n> + * \\class IPCUnixSocket\n> + * \\brief IPC mechanism based on Unix sockets\n> + *\n> + * The Unix socket IPC allows bidirectional communication between two processes\n> + * through unnamed Unix sockets. It implements datagram-based communication,\n> + * transporting entire payloads with guaranteed ordering.\n> + *\n> + * The IPC design is asynchronous, a message is queued to a receiver which gets\n> + * notified that a message is ready to be consumed by a signal. The queuer of\n> + * the message gets no notification when a message is delivered nor processed.\n> + * If such interactions are needed a protocol specific to the users use-case\n> + * should be implemented on top of the IPC objects.\n> + *\n> + * Establishment of an IPC channel is asymmetrical. The side that initiates\n> + * communication first instantiates a local side socket and creates the channel\n> + * with create(). The method returns a file descriptor for the remote side of\n> + * the channel, which is passed to the remote process through an out-of-band\n> + * communication method. The remote side then instantiates a socket, and binds\n> + * it to the other side by passing the file descriptor to bind(). At that point\n> + * the channel is operation and communication is bidirectional and symmmetrical.\n> + */\n> +\n> +IPCUnixSocket::IPCUnixSocket()\n> +\t: fd_(-1), notifier_(nullptr)\n> +{\n> +}\n> +\n> +IPCUnixSocket::~IPCUnixSocket()\n> +{\n> +\tclose();\n> +}\n> +\n> +/**\n> + * \\brief Create an new IPC channel\n> + *\n> + * This method creates a new IPC channel. The socket instance is bound to the\n> + * local side of the channel, and the method returns a file descriptor bound to\n> + * the remote side. The caller is responsible for passing the file descriptor to\n> + * the remote process, where it can be used with IPCUnixSocket::bind() to bind\n> + * the remote side socket.\n> + *\n> + * \\return A file descriptor on success, negative error code on failure\n> + */\n> +int IPCUnixSocket::create()\n> +{\n> +\tint sockets[2];\n> +\tint ret;\n> +\n> +\tret = socketpair(AF_UNIX, SOCK_DGRAM, 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> +\tret = bind(sockets[0]);\n> +\tif (ret)\n> +\t\treturn ret;\n> +\n> +\treturn sockets[1];\n> +}\n> +\n> +/**\n> + * \\brief Bind to an existing IPC channel\n> + * \\param[in] fd File descriptor\n> + *\n> + * This method binds the socket instance to an existing IPC channel identified\n> + * by the file descriptor \\a fd. The file descriptor is obtained from the\n> + * IPCUnixSocket::create() method.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int IPCUnixSocket::bind(int fd)\n> +{\n> +\tif (isBound())\n> +\t\treturn -EINVAL;\n> +\n> +\tfd_ = fd;\n> +\tnotifier_ = new EventNotifier(fd_, EventNotifier::Read);\n> +\tnotifier_->activated.connect(this, &IPCUnixSocket::dataNotifier);\n> +\n> +\treturn 0;\n> +}\n> +\n> +/**\n> + * \\brief Close the IPC channel\n> + *\n> + * No communication is possible after close() has been called.\n> + */\n> +void IPCUnixSocket::close()\n> +{\n> +\tif (!isBound())\n> +\t\treturn;\n> +\n> +\tdelete notifier_;\n> +\tnotifier_ = nullptr;\n> +\n> +\t::close(fd_);\n> +\n> +\tfd_ = -1;\n> +}\n> +\n> +/**\n> + * \\brief Check if the IPC channel is bound\n> + * \\return True if the IPC channel is bound, false otherwise\n> + */\n> +bool IPCUnixSocket::isBound() const\n> +{\n> +\treturn fd_ != -1;\n> +}\n> +\n> +/**\n> + * \\brief Send a message payload\n> + * \\param[in] payload Message payload to send\n> + *\n> + * This method queues the message payload for transmission to the other end of\n> + * the IPC channel. It returns immediately, before the message is delivered to\n> + * the remote side.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int IPCUnixSocket::send(const Payload &payload)\n> +{\n> +\tint ret;\n> +\n> +\tif (!isBound())\n> +\t\treturn -ENOTCONN;\n> +\n> +\tHeader hdr;\n> +\thdr.data = payload.data.size();\n> +\thdr.fds = payload.fds.size();\n> +\n> +\tif (!hdr.data && !hdr.fds)\n> +\t\treturn -EINVAL;\n> +\n> +\tret = ::send(fd_, &hdr, sizeof(hdr), 0);\n> +\tif (ret < 0) {\n> +\t\tret = -errno;\n> +\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t<< \"Failed to send: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\treturn sendData(payload.data.data(), hdr.data, payload.fds.data(), hdr.fds);\n> +}\n> +\n> +/**\n> + * \\brief Receive a message payload\n> + * \\param[out] payload Payload where to write the received message\n> + *\n> + * This method receives the message payload from the IPC channel and writes it\n> + * to the \\a payload. It blocks until one message is received, if an\n> + * asynchronous behavior is desired this method should be called when the\n> + * readyRead signal is emitted.\n> + *\n> + * \\todo Add state machine to make sure we don't block forever and that\n> + * a header is always followed by a payload.\n> + *\n> + * \\return 0 on success or a negative error code otherwise\n> + */\n> +int IPCUnixSocket::receive(Payload *payload)\n> +{\n> +\tHeader hdr;\n> +\tint ret;\n> +\n> +\tif (!isBound())\n> +\t\treturn -ENOTCONN;\n> +\n> +\tif (!payload)\n> +\t\treturn -EINVAL;\n> +\n> +\tret = ::recv(fd_, &hdr, sizeof(hdr), 0);\n> +\tif (ret < 0) {\n> +\t\tret = -errno;\n> +\t\tLOG(IPCUnixSocket, Error)\n> +\t\t\t<< \"Failed to recv header: \" << strerror(-ret);\n> +\t\treturn ret;\n> +\t}\n> +\n> +\tpayload->data.resize(hdr.data);\n> +\tpayload->fds.resize(hdr.fds);\n> +\n> +\treturn recvData(payload->data.data(), hdr.data, payload->fds.data(), hdr.fds);\n> +}\n> +\n> +/**\n> + * \\var IPCUnixSocket::readyRead\n> + * \\brief A Signal emitted when a message is ready to be read\n> + */\n> +\n> +int IPCUnixSocket::sendData(const void *buffer, size_t length, const int32_t *fds, unsigned int num)\n> +{\n> +\tstruct iovec iov[1];\n> +\tiov[0].iov_base = const_cast<void *>(buffer);\n> +\tiov[0].iov_len = length;\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::recvData(void *buffer, size_t length, int32_t *fds, unsigned int num)\n> +{\n> +\tstruct iovec iov[1];\n> +\tiov[0].iov_base = buffer;\n> +\tiov[0].iov_len = length;\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> +\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> +\tmemcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t));\n> +\n> +\treturn 0;\n> +}\n> +\n> +void IPCUnixSocket::dataNotifier(EventNotifier *notifier)\n> +{\n> +\treadyRead.emit(this);\n> +}\n> +\n> +} /* namespace libcamera */\n> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build\n> index 985aa7e8ab0eb6ce..45bd9d1793aa0b19 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> @@ -38,6 +39,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":"<laurent.pinchart@ideasonboard.com>","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 366C360BF8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tTue,  2 Jul 2019 00:10:03 +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 B81DA524;\n\tTue,  2 Jul 2019 00:10:02 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1562019002;\n\tbh=+6n/qZEoXBCBMIeW1i6v68vWc3hS1YHy07buq7Q8IDc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=kcwGWYHi5rkqrIgfVjTx/tdZIN7fxsp5cOEuPSOujJut3Zw8Kccr7sZ6c3VXUEZhW\n\tiibJxT3kg2SZLIeNb5W9M3iWEUtjEdO2b6uqri47GeV+xahh8c/ZXb3jbq7t37jqWd\n\tPvMRiKv7YPPubkvZ1qHlpEVuZUTrSRGxQhMIe9nA=","Date":"Tue, 2 Jul 2019 01:09:36 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190701220936.GB24839@pendragon.ideasonboard.com>","References":"<20190701220612.6342-1-niklas.soderlund@ragnatech.se>\n\t<20190701220612.6342-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":"<20190701220612.6342-2-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH v3 1/2] libcamera: ipc: unix: Add a\n\tIPC mechanism based on Unix sockets","X-BeenThere":"libcamera-devel@lists.libcamera.org","X-Mailman-Version":"2.1.23","Precedence":"list","List-Id":"<libcamera-devel.lists.libcamera.org>","List-Unsubscribe":"<https://lists.libcamera.org/options/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=unsubscribe>","List-Archive":"<https://lists.libcamera.org/pipermail/libcamera-devel/>","List-Post":"<mailto:libcamera-devel@lists.libcamera.org>","List-Help":"<mailto:libcamera-devel-request@lists.libcamera.org?subject=help>","List-Subscribe":"<https://lists.libcamera.org/listinfo/libcamera-devel>,\n\t<mailto:libcamera-devel-request@lists.libcamera.org?subject=subscribe>","X-List-Received-Date":"Mon, 01 Jul 2019 22:10:03 -0000"}}]