From patchwork Mon Jul 1 23:23:37 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1578 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id EFB696157C for ; Tue, 2 Jul 2019 01:24:03 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 8818F53B for ; Tue, 2 Jul 2019 01:24:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1562023443; bh=eq7m8U769/S/nLcSayJhT2uUk3b3VPmtpd+3mXTHsio=; h=From:To:Subject:Date:In-Reply-To:References:From; b=c6Rfzjs0e8PlCYK8pZsluygugqt83lSeODfP85YKCGhjYP51NYTfU0CKfHqpAOuew aPG92nXE4/ROZQUORzSxFLMHBJJGyfzlzDC328Pw0CpMdfnZlblLwpnZV/dJpzjK4y arAD0sHMvkkBPVEqws4gXrMfT+lJ5NA2ik5vPWFg= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 2 Jul 2019 02:23:37 +0300 Message-Id: <20190701232339.5191-2-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190701232339.5191-1-laurent.pinchart@ideasonboard.com> References: <20190701232339.5191-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 1/3] libcamera: ipc: unix: Add a IPC mechanism based on Unix sockets X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Jul 2019 23:24:04 -0000 From: Niklas Söderlund To be able to isolate an IPA component in a separate process an IPC mechanism is needed to communicate with it. Add an IPC mechanism based on Unix sockets which allows users to pass both data and file descriptors to and from the IPA process. The implementation allows users to send both data and file descriptors in the same message. This allows users to more easily implement serialization and deserialization of objects as all elements belonging to an object can be sent in one message. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart --- src/libcamera/include/ipc_unixsocket.h | 57 +++++ src/libcamera/ipc_unixsocket.cpp | 309 +++++++++++++++++++++++++ src/libcamera/meson.build | 2 + 3 files changed, 368 insertions(+) create mode 100644 src/libcamera/include/ipc_unixsocket.h create mode 100644 src/libcamera/ipc_unixsocket.cpp diff --git a/src/libcamera/include/ipc_unixsocket.h b/src/libcamera/include/ipc_unixsocket.h new file mode 100644 index 000000000000..ef166d742554 --- /dev/null +++ b/src/libcamera/include/ipc_unixsocket.h @@ -0,0 +1,57 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * ipc_unixsocket.h - IPC mechanism based on Unix sockets + */ + +#ifndef __LIBCAMERA_IPC_UNIXSOCKET_H__ +#define __LIBCAMERA_IPC_UNIXSOCKET_H__ + +#include +#include +#include + +#include + +namespace libcamera { + +class IPCUnixSocket +{ +public: + struct Payload { + std::vector data; + std::vector fds; + }; + + IPCUnixSocket(); + ~IPCUnixSocket(); + + int create(); + int bind(int fd); + void close(); + bool isBound() const; + + int send(const Payload &payload); + int receive(Payload *payload); + + Signal readyRead; + +private: + struct Header { + uint32_t data; + uint8_t fds; + }; + + int sendData(const void *buffer, size_t length, const int32_t *fds, unsigned int num); + int recvData(void *buffer, size_t length, int32_t *fds, unsigned int num); + + void dataNotifier(EventNotifier *notifier); + + int fd_; + EventNotifier *notifier_; +}; + +} /* namespace libcamera */ + +#endif /* __LIBCAMERA_IPC_UNIXSOCKET_H__ */ diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp new file mode 100644 index 000000000000..c11f116093c5 --- /dev/null +++ b/src/libcamera/ipc_unixsocket.cpp @@ -0,0 +1,309 @@ +/* SPDX-License-Identifier: LGPL-2.1-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * ipc_unixsocket.cpp - IPC mechanism based on Unix sockets + */ + +#include "ipc_unixsocket.h" + +#include +#include +#include + +#include "log.h" + +/** + * \file ipc_unixsocket.h + * \brief IPC mechanism based on Unix sockets + */ + +namespace libcamera { + +LOG_DEFINE_CATEGORY(IPCUnixSocket) + +/** + * \struct IPCUnixSocket::Payload + * \brief Container for an IPC payload + * + * Holds an array of bytes and an array of file descriptors that can be + * transported across a IPC boundary. + */ + +/** + * \var IPCUnixSocket::Payload::data + * \brief Array of bytes to cross IPC boundary + */ + +/** + * \var IPCUnixSocket::Payload::fds + * \brief Array of file descriptors to cross IPC boundary + */ + +/** + * \class IPCUnixSocket + * \brief IPC mechanism based on Unix sockets + * + * The Unix socket IPC allows bidirectional communication between two processes + * through unnamed Unix sockets. It implements datagram-based communication, + * transporting entire payloads with guaranteed ordering. + * + * The IPC design is asynchronous, a message is queued to a receiver which gets + * notified that a message is ready to be consumed by a signal. The queuer of + * the message gets no notification when a message is delivered nor processed. + * If such interactions are needed a protocol specific to the users use-case + * should be implemented on top of the IPC objects. + * + * Establishment of an IPC channel is asymmetrical. The side that initiates + * communication first instantiates a local side socket and creates the channel + * with create(). The method returns a file descriptor for the remote side of + * the channel, which is passed to the remote process through an out-of-band + * communication method. The remote side then instantiates a socket, and binds + * it to the other side by passing the file descriptor to bind(). At that point + * the channel is operation and communication is bidirectional and symmmetrical. + */ + +IPCUnixSocket::IPCUnixSocket() + : fd_(-1), notifier_(nullptr) +{ +} + +IPCUnixSocket::~IPCUnixSocket() +{ + close(); +} + +/** + * \brief Create an new IPC channel + * + * This method creates a new IPC channel. The socket instance is bound to the + * local side of the channel, and the method returns a file descriptor bound to + * the remote side. The caller is responsible for passing the file descriptor to + * the remote process, where it can be used with IPCUnixSocket::bind() to bind + * the remote side socket. + * + * \return A file descriptor on success, negative error code on failure + */ +int IPCUnixSocket::create() +{ + int sockets[2]; + int ret; + + ret = socketpair(AF_UNIX, SOCK_DGRAM, 0, sockets); + if (ret) { + ret = -errno; + LOG(IPCUnixSocket, Error) + << "Failed to create socket pair: " << strerror(-ret); + return ret; + } + + ret = bind(sockets[0]); + if (ret) + return ret; + + return sockets[1]; +} + +/** + * \brief Bind to an existing IPC channel + * \param[in] fd File descriptor + * + * This method binds the socket instance to an existing IPC channel identified + * by the file descriptor \a fd. The file descriptor is obtained from the + * IPCUnixSocket::create() method. + * + * \return 0 on success or a negative error code otherwise + */ +int IPCUnixSocket::bind(int fd) +{ + if (isBound()) + return -EINVAL; + + fd_ = fd; + notifier_ = new EventNotifier(fd_, EventNotifier::Read); + notifier_->activated.connect(this, &IPCUnixSocket::dataNotifier); + + return 0; +} + +/** + * \brief Close the IPC channel + * + * No communication is possible after close() has been called. + */ +void IPCUnixSocket::close() +{ + if (!isBound()) + return; + + delete notifier_; + notifier_ = nullptr; + + ::close(fd_); + + fd_ = -1; +} + +/** + * \brief Check if the IPC channel is bound + * \return True if the IPC channel is bound, false otherwise + */ +bool IPCUnixSocket::isBound() const +{ + return fd_ != -1; +} + +/** + * \brief Send a message payload + * \param[in] payload Message payload to send + * + * This method queues the message payload for transmission to the other end of + * the IPC channel. It returns immediately, before the message is delivered to + * the remote side. + * + * \return 0 on success or a negative error code otherwise + */ +int IPCUnixSocket::send(const Payload &payload) +{ + int ret; + + if (!isBound()) + return -ENOTCONN; + + Header hdr; + hdr.data = payload.data.size(); + hdr.fds = payload.fds.size(); + + if (!hdr.data && !hdr.fds) + return -EINVAL; + + ret = ::send(fd_, &hdr, sizeof(hdr), 0); + if (ret < 0) { + ret = -errno; + LOG(IPCUnixSocket, Error) + << "Failed to send: " << strerror(-ret); + return ret; + } + + return sendData(payload.data.data(), hdr.data, payload.fds.data(), hdr.fds); +} + +/** + * \brief Receive a message payload + * \param[out] payload Payload where to write the received message + * + * This method receives the message payload from the IPC channel and writes it + * to the \a payload. It blocks until one message is received, if an + * asynchronous behavior is desired this method should be called when the + * readyRead signal is emitted. + * + * \todo Add state machine to make sure we don't block forever and that + * a header is always followed by a payload. + * + * \return 0 on success or a negative error code otherwise + */ +int IPCUnixSocket::receive(Payload *payload) +{ + Header hdr; + int ret; + + if (!isBound()) + return -ENOTCONN; + + if (!payload) + return -EINVAL; + + ret = ::recv(fd_, &hdr, sizeof(hdr), 0); + if (ret < 0) { + ret = -errno; + LOG(IPCUnixSocket, Error) + << "Failed to recv header: " << strerror(-ret); + return ret; + } + + payload->data.resize(hdr.data); + payload->fds.resize(hdr.fds); + + return recvData(payload->data.data(), hdr.data, payload->fds.data(), hdr.fds); +} + +/** + * \var IPCUnixSocket::readyRead + * \brief A Signal emitted when a message is ready to be read + */ + +int IPCUnixSocket::sendData(const void *buffer, size_t length, const int32_t *fds, unsigned int num) +{ + struct iovec iov[1]; + iov[0].iov_base = const_cast(buffer); + iov[0].iov_len = length; + + char buf[CMSG_SPACE(num * sizeof(uint32_t))]; + memset(buf, 0, sizeof(buf)); + + struct cmsghdr *cmsg = (struct cmsghdr *)buf; + cmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t)); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + + struct msghdr msg; + msg.msg_name = nullptr; + msg.msg_namelen = 0; + msg.msg_iov = iov; + msg.msg_iovlen = 1; + msg.msg_control = cmsg; + msg.msg_controllen = cmsg->cmsg_len; + msg.msg_flags = 0; + memcpy(CMSG_DATA(cmsg), fds, num * sizeof(uint32_t)); + + if (sendmsg(fd_, &msg, 0) < 0) { + int ret = -errno; + LOG(IPCUnixSocket, Error) + << "Failed to sendmsg: " << strerror(-ret); + return ret; + } + + return 0; +} + +int IPCUnixSocket::recvData(void *buffer, size_t length, int32_t *fds, unsigned int num) +{ + struct iovec iov[1]; + iov[0].iov_base = buffer; + iov[0].iov_len = length; + + char buf[CMSG_SPACE(num * sizeof(uint32_t))]; + memset(buf, 0, sizeof(buf)); + + struct cmsghdr *cmsg = (struct cmsghdr *)buf; + cmsg->cmsg_len = CMSG_LEN(num * sizeof(uint32_t)); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + + struct msghdr msg; + msg.msg_name = nullptr; + msg.msg_namelen = 0; + msg.msg_iov = iov; + msg.msg_iovlen = 1; + msg.msg_control = cmsg; + msg.msg_controllen = cmsg->cmsg_len; + msg.msg_flags = 0; + + if (recvmsg(fd_, &msg, 0) < 0) { + int ret = -errno; + LOG(IPCUnixSocket, Error) + << "Failed to recvmsg: " << strerror(-ret); + return ret; + } + + memcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t)); + + return 0; +} + +void IPCUnixSocket::dataNotifier(EventNotifier *notifier) +{ + readyRead.emit(this); +} + +} /* namespace libcamera */ diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build index 985aa7e8ab0e..45bd9d1793aa 100644 --- a/src/libcamera/meson.build +++ b/src/libcamera/meson.build @@ -13,6 +13,7 @@ libcamera_sources = files([ 'ipa_interface.cpp', 'ipa_manager.cpp', 'ipa_module.cpp', + 'ipc_unixsocket.cpp', 'log.cpp', 'media_device.cpp', 'media_object.cpp', @@ -38,6 +39,7 @@ libcamera_headers = files([ 'include/formats.h', 'include/ipa_manager.h', 'include/ipa_module.h', + 'include/ipc_unixsocket.h', 'include/log.h', 'include/media_device.h', 'include/media_object.h', From patchwork Mon Jul 1 23:23:38 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1579 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [IPv6:2001:4b98:dc2:55:216:3eff:fef7:d647]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 3A1236157C for ; Tue, 2 Jul 2019 01:24:04 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id DB992524 for ; Tue, 2 Jul 2019 01:24:03 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1562023444; bh=AiaJ4x8KtlvqQe9YHb/2vNKiUFGnRIiCtRcD/1BVXFc=; h=From:To:Subject:Date:In-Reply-To:References:From; b=PVu/LBxRUT+T/xs2wApja8M2FbW6q5qK9Xe2litOIeYRIA5E7qkDuq8tn/spw/s/9 w81iQ/90C94vyrlH5ckWhDnqASesGzZj8HG1X3QFeHSDRxq/a8TEDCcmMwBwNknQxG Du9PTb7TBTUlPAgsYp7vCCe3kPTc9NKw4w2EWWuU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 2 Jul 2019 02:23:38 +0300 Message-Id: <20190701232339.5191-3-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190701232339.5191-1-laurent.pinchart@ideasonboard.com> References: <20190701232339.5191-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 2/3] test: ipc: unix: Add test for IPCUnixSocket X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Jul 2019 23:24:04 -0000 From: Niklas Söderlund Test that the IPC supports sending data and file descriptors over the IPC medium. To be able to execute the test two parts are needed, one to drive the test and act as the libcamera (master) and a one to act as the IPA (slave). The master drives the testing posting requests to the slave to process and sometimes respond to. A few different tests are performed. - Master sends an array to the slave which responds with a reversed copy of the array. The master verifies that a reversed array is returned. - Master ties to sends an empty message making sure that the send call fails. - Master sends a list of file descriptors and ask the slave to calculate and respond with the sum of the size of the files. The master verifies that the calculated size is correct. - Master sends a pre-computed size and a list of file descriptors and asks the slave to verify that the pre-computed size matches the sum of the size of the file descriptors. - Master sends two file descriptors and asks the salve to join the file contents in a new file and respond with its file descriptor. The master then verifies that the content of the returned file descriptor matches the order of the original two files. Signed-off-by: Niklas Söderlund Reviewed-by: Laurent Pinchart Signed-off-by: Laurent Pinchart --- Changes since v3: - Use O_TMPFILE instead of shm_open() - Miscellaneous typo and small fixes --- test/ipc/meson.build | 12 + test/ipc/unixsocket.cpp | 502 ++++++++++++++++++++++++++++++++++++++++ test/meson.build | 1 + 3 files changed, 515 insertions(+) create mode 100644 test/ipc/meson.build create mode 100644 test/ipc/unixsocket.cpp diff --git a/test/ipc/meson.build b/test/ipc/meson.build new file mode 100644 index 000000000000..ca8375f35df9 --- /dev/null +++ b/test/ipc/meson.build @@ -0,0 +1,12 @@ +ipc_tests = [ + [ 'unixsocket', 'unixsocket.cpp' ], +] + +foreach t : ipc_tests + exe = executable(t[0], t[1], + dependencies : libcamera_dep, + link_with : test_libraries, + include_directories : test_includes_internal) + + test(t[0], exe, suite : 'ipc', is_parallel : false) +endforeach diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp new file mode 100644 index 000000000000..eeef64842a75 --- /dev/null +++ b/test/ipc/unixsocket.cpp @@ -0,0 +1,502 @@ +/* SPDX-License-Identifier: GPL-2.0-or-later */ +/* + * Copyright (C) 2019, Google Inc. + * + * unixsocket.cpp - Unix socket IPC test + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include +#include +#include + +#include "ipc_unixsocket.h" +#include "test.h" +#include "utils.h" + +#define CMD_CLOSE 0 +#define CMD_REVERSE 1 +#define CMD_LEN_CALC 2 +#define CMD_LEN_CMP 3 +#define CMD_JOIN 4 + +using namespace std; +using namespace libcamera; + +int calculateLength(int fd) +{ + lseek(fd, 0, 0); + int size = lseek(fd, 0, SEEK_END); + lseek(fd, 0, 0); + + return size; +} + +class UnixSocketTestSlave +{ +public: + UnixSocketTestSlave() + : exitCode_(EXIT_FAILURE), exit_(false) + { + dispatcher_ = CameraManager::instance()->eventDispatcher(); + ipc_.readyRead.connect(this, &UnixSocketTestSlave::readyRead); + } + + int run(int fd) + { + if (ipc_.bind(fd)) { + cerr << "Failed to connect to IPC channel" << endl; + return EXIT_FAILURE; + } + + while (!exit_) + dispatcher_->processEvents(); + + ipc_.close(); + + return exitCode_; + } + +private: + void readyRead(IPCUnixSocket *ipc) + { + IPCUnixSocket::Payload message, response; + int ret; + + ret = ipc->receive(&message); + if (ret) { + cerr << "Receive message failed: " << ret << endl; + return; + } + + const uint8_t cmd = message.data[0]; + + switch (cmd) { + case CMD_CLOSE: + stop(0); + break; + + case CMD_REVERSE: { + response.data = message.data; + std::reverse(response.data.begin() + 1, response.data.end()); + + ret = ipc_.send(response); + if (ret < 0) { + cerr << "Reverse failed" << endl; + stop(ret); + } + break; + } + + case CMD_LEN_CALC: { + int size = 0; + for (int fd : message.fds) + size += calculateLength(fd); + + response.data.resize(1 + sizeof(size)); + response.data[0] = cmd; + memcpy(response.data.data() + 1, &size, sizeof(size)); + + ret = ipc_.send(response); + if (ret < 0) { + cerr << "Calc failed" << endl; + stop(ret); + } + break; + } + + case CMD_LEN_CMP: { + int size = 0; + for (int fd : message.fds) + size += calculateLength(fd); + + int cmp; + memcpy(&cmp, message.data.data() + 1, sizeof(cmp)); + + if (cmp != size) { + cerr << "Compare failed" << endl; + stop(-ERANGE); + } + break; + } + + case CMD_JOIN: { + int outfd = open("/tmp", O_TMPFILE | O_RDWR, + S_IRUSR | S_IWUSR); + if (outfd < 0) { + cerr << "Create out file failed" << endl; + stop(outfd); + return; + } + + for (int fd : message.fds) { + while (true) { + char buf[32]; + ssize_t num = read(fd, &buf, sizeof(buf)); + + if (num < 0) { + cerr << "Read failed" << endl; + stop(-EIO); + return; + } else if (!num) + break; + + if (write(outfd, buf, num) < 0) { + cerr << "Write failed" << endl; + stop(-EIO); + return; + } + } + + close(fd); + } + + lseek(outfd, 0, 0); + response.data.push_back(CMD_JOIN); + response.fds.push_back(outfd); + + ret = ipc_.send(response); + if (ret < 0) { + cerr << "Join failed" << endl; + stop(ret); + } + + close(outfd); + + break; + } + + default: + cerr << "Unknown command " << cmd << endl; + stop(-EINVAL); + break; + } + } + + void stop(int code) + { + exitCode_ = code; + exit_ = true; + } + + IPCUnixSocket ipc_; + EventDispatcher *dispatcher_; + int exitCode_; + bool exit_; +}; + +class UnixSocketTest : public Test +{ +protected: + int slaveStart(int fd) + { + pid_ = fork(); + + if (pid_ == -1) + return TestFail; + + if (!pid_) { + std::string arg = std::to_string(fd); + execl("/proc/self/exe", "/proc/self/exe", + arg.c_str(), nullptr); + + /* Only get here if exec fails. */ + exit(TestFail); + } + + return TestPass; + } + + int slaveStop() + { + int status; + + if (pid_ < 0) + return TestFail; + + if (waitpid(pid_, &status, 0) < 0) + return TestFail; + + if (!WIFEXITED(status) || WEXITSTATUS(status)) + return TestFail; + + return TestPass; + } + + int testReverse() + { + IPCUnixSocket::Payload message, response; + int ret; + + message.data = { CMD_REVERSE, 1, 2, 3, 4, 5 }; + + ret = call(message, &response); + if (ret) + return ret; + + std::reverse(response.data.begin() + 1, response.data.end()); + if (message.data != response.data) + return TestFail; + + return 0; + } + + int testEmptyFail() + { + IPCUnixSocket::Payload message; + + return ipc_.send(message) != -EINVAL; + } + + int testCalc() + { + IPCUnixSocket::Payload message, response; + int sizeOut, sizeIn, ret; + + sizeOut = prepareFDs(&message, 2); + if (sizeOut < 0) + return sizeOut; + + message.data.push_back(CMD_LEN_CALC); + + ret = call(message, &response); + if (ret) + return ret; + + memcpy(&sizeIn, response.data.data() + 1, sizeof(sizeIn)); + if (sizeOut != sizeIn) + return TestFail; + + return 0; + } + + int testCmp() + { + IPCUnixSocket::Payload message; + int size; + + size = prepareFDs(&message, 7); + if (size < 0) + return size; + + message.data.resize(1 + sizeof(size)); + message.data[0] = CMD_LEN_CMP; + memcpy(message.data.data() + 1, &size, sizeof(size)); + + if (ipc_.send(message)) + return TestFail; + + return 0; + } + + int testFdOrder() + { + IPCUnixSocket::Payload message, response; + int ret; + + static const char *strings[2] = { + "Foo", + "Bar", + }; + int fds[2]; + + for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { + unsigned int len = strlen(strings[i]); + + fds[i] = open("/tmp", O_TMPFILE | O_RDWR, + S_IRUSR | S_IWUSR); + if (fds[i] < 0) + return TestFail; + + ret = write(fds[i], strings[i], len); + if (ret < 0) + return TestFail; + + lseek(fds[i], 0, 0); + message.fds.push_back(fds[i]); + } + + message.data.push_back(CMD_JOIN); + + ret = call(message, &response); + if (ret) + return ret; + + for (unsigned int i = 0; i < ARRAY_SIZE(strings); i++) { + unsigned int len = strlen(strings[i]); + char buf[len]; + + close(fds[i]); + + if (read(response.fds[0], &buf, len) <= 0) + return TestFail; + + if (memcmp(buf, strings[i], len)) + return TestFail; + } + + close(response.fds[0]); + + return 0; + } + + int init() + { + callResponse_ = nullptr; + return 0; + } + + int run() + { + int slavefd = ipc_.create(); + if (slavefd < 0) + return TestFail; + + if (slaveStart(slavefd)) { + cerr << "Failed to start slave" << endl; + return TestFail; + } + + ipc_.readyRead.connect(this, &UnixSocketTest::readyRead); + + /* Test reversing a string, this test sending only data. */ + if (testReverse()) { + cerr << "Reveres array test failed" << endl; + return TestFail; + } + + /* Test that an empty message fails. */ + if (testEmptyFail()) { + cerr << "Empty message test failed" << endl; + return TestFail; + } + + /* Test offloading a calculation, this test sending only FDs. */ + if (testCalc()) { + cerr << "Calc test failed" << endl; + return TestFail; + } + + /* Test fire and forget, this tests sending data and FDs. */ + if (testCmp()) { + cerr << "Cmp test failed" << endl; + return TestFail; + } + + /* Test order of file descriptors. */ + if (testFdOrder()) { + cerr << "fd order test failed" << endl; + return TestFail; + } + + /* Close slave connection. */ + IPCUnixSocket::Payload close; + close.data.push_back(CMD_CLOSE); + if (ipc_.send(close)) { + cerr << "Closing IPC channel failed" << endl; + return TestFail; + } + + ipc_.close(); + if (slaveStop()) { + cerr << "Failed to stop slave" << endl; + return TestFail; + } + + return TestPass; + } + +private: + int call(const IPCUnixSocket::Payload &message, IPCUnixSocket::Payload *response) + { + Timer timeout; + int ret; + + callDone_ = false; + callResponse_ = response; + + ret = ipc_.send(message); + if (ret) + return ret; + + timeout.start(200); + while (!callDone_) { + if (!timeout.isRunning()) { + cerr << "Call timeout!" << endl; + callResponse_ = nullptr; + return -ETIMEDOUT; + } + + CameraManager::instance()->eventDispatcher()->processEvents(); + } + + callResponse_ = nullptr; + + return 0; + } + + void readyRead(IPCUnixSocket *ipc) + { + if (!callResponse_) { + cerr << "Read ready without expecting data, fail." << endl; + return; + } + + if (ipc->receive(callResponse_)) { + cerr << "Receive message failed" << endl; + return; + } + + callDone_ = true; + } + + int prepareFDs(IPCUnixSocket::Payload *message, unsigned int num) + { + int fd = open("/proc/self/exe", O_RDONLY); + if (fd < 0) + return fd; + + int size = 0; + for (unsigned int i = 0; i < num; i++) { + int clone = dup(fd); + if (clone < 0) + return clone; + + size += calculateLength(clone); + message->fds.push_back(clone); + } + + close(fd); + + return size; + } + + pid_t pid_; + IPCUnixSocket ipc_; + bool callDone_; + IPCUnixSocket::Payload *callResponse_; +}; + +/* + * Can't use TEST_REGISTER() as single binary needs to act as both proxy + * master and slave. + */ +int main(int argc, char **argv) +{ + if (argc == 2) { + int ipcfd = std::stoi(argv[1]); + UnixSocketTestSlave slave; + return slave.run(ipcfd); + } + + return UnixSocketTest().execute(); +} diff --git a/test/meson.build b/test/meson.build index c36ac2479636..3666f6b2385b 100644 --- a/test/meson.build +++ b/test/meson.build @@ -2,6 +2,7 @@ subdir('libtest') subdir('camera') subdir('ipa') +subdir('ipc') subdir('media_device') subdir('pipeline') subdir('stream') From patchwork Mon Jul 1 23:23:39 2019 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: Laurent Pinchart X-Patchwork-Id: 1580 Return-Path: Received: from perceval.ideasonboard.com (perceval.ideasonboard.com [213.167.242.64]) by lancelot.ideasonboard.com (Postfix) with ESMTPS id 9E8A96157C for ; Tue, 2 Jul 2019 01:24:04 +0200 (CEST) Received: from pendragon.bb.dnainternet.fi (dfj612yhrgyx302h3jwwy-3.rev.dnainternet.fi [IPv6:2001:14ba:21f5:5b00:ce28:277f:58d7:3ca4]) by perceval.ideasonboard.com (Postfix) with ESMTPSA id 3786253B for ; Tue, 2 Jul 2019 01:24:04 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com; s=mail; t=1562023444; bh=TjI7zocRVzBpCg7ppR9TAcE9AoBnfn+yWr7PS48vf9s=; h=From:To:Subject:Date:In-Reply-To:References:From; b=s/RWXt5wZv6/9Gak2hpnYh1DwB3Vtz1eBi3ciyVUmYUFoM6MaAarHWqT9TzrVLnFN ADpYg2iHgLVtme6ToVa5Un12Kf1rRz4j+sUAdkXq1189ddjZGhddlXAMIeKU8y1yIo DRQ40IdwOv6lw9VZTox2xtocq4aKWpIN/Pm99tzU= From: Laurent Pinchart To: libcamera-devel@lists.libcamera.org Date: Tue, 2 Jul 2019 02:23:39 +0300 Message-Id: <20190701232339.5191-4-laurent.pinchart@ideasonboard.com> X-Mailer: git-send-email 2.21.0 In-Reply-To: <20190701232339.5191-1-laurent.pinchart@ideasonboard.com> References: <20190701232339.5191-1-laurent.pinchart@ideasonboard.com> MIME-Version: 1.0 Subject: [libcamera-devel] [PATCH v4 3/3] libcamera: ipc: unix: Make socket operation asynchronous X-BeenThere: libcamera-devel@lists.libcamera.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 01 Jul 2019 23:24:04 -0000 Blocking socket operation when receiving messages may lead to long delays, and possibly a complete deadlock, if the remote side delays sending of the payload after the header, or doesn't send the payload at all. To avoid this, make the socket non-blocking and implement a simple state machine to receive the header synchronously with the socket read notification. The payload read is still synchronous with the receive() method to avoid data copies. Signed-off-by: Laurent Pinchart Reviewed-by: Niklas Söderlund --- src/libcamera/include/ipc_unixsocket.h | 2 + src/libcamera/ipc_unixsocket.cpp | 88 ++++++++++++++++++-------- 2 files changed, 63 insertions(+), 27 deletions(-) diff --git a/src/libcamera/include/ipc_unixsocket.h b/src/libcamera/include/ipc_unixsocket.h index ef166d742554..03e9fe492bde 100644 --- a/src/libcamera/include/ipc_unixsocket.h +++ b/src/libcamera/include/ipc_unixsocket.h @@ -49,6 +49,8 @@ private: void dataNotifier(EventNotifier *notifier); int fd_; + bool headerReceived_; + struct Header header_; EventNotifier *notifier_; }; diff --git a/src/libcamera/ipc_unixsocket.cpp b/src/libcamera/ipc_unixsocket.cpp index c11f116093c5..def08eef00f8 100644 --- a/src/libcamera/ipc_unixsocket.cpp +++ b/src/libcamera/ipc_unixsocket.cpp @@ -7,6 +7,7 @@ #include "ipc_unixsocket.h" +#include #include #include #include @@ -49,10 +50,10 @@ LOG_DEFINE_CATEGORY(IPCUnixSocket) * transporting entire payloads with guaranteed ordering. * * The IPC design is asynchronous, a message is queued to a receiver which gets - * notified that a message is ready to be consumed by a signal. The queuer of - * the message gets no notification when a message is delivered nor processed. - * If such interactions are needed a protocol specific to the users use-case - * should be implemented on top of the IPC objects. + * notified that a message is ready to be consumed by the \ref readyRead + * signal. The sender of the message gets no notification when a message is + * delivered nor processed. If such interactions are needed a protocol specific + * to the users use-case should be implemented on top of the IPC objects. * * Establishment of an IPC channel is asymmetrical. The side that initiates * communication first instantiates a local side socket and creates the channel @@ -64,7 +65,7 @@ LOG_DEFINE_CATEGORY(IPCUnixSocket) */ IPCUnixSocket::IPCUnixSocket() - : fd_(-1), notifier_(nullptr) + : fd_(-1), headerReceived_(false), notifier_(nullptr) { } @@ -89,7 +90,7 @@ int IPCUnixSocket::create() int sockets[2]; int ret; - ret = socketpair(AF_UNIX, SOCK_DGRAM, 0, sockets); + ret = socketpair(AF_UNIX, SOCK_DGRAM | SOCK_NONBLOCK, 0, sockets); if (ret) { ret = -errno; LOG(IPCUnixSocket, Error) @@ -142,6 +143,7 @@ void IPCUnixSocket::close() ::close(fd_); fd_ = -1; + headerReceived_ = false; } /** @@ -193,38 +195,38 @@ int IPCUnixSocket::send(const Payload &payload) * \param[out] payload Payload where to write the received message * * This method receives the message payload from the IPC channel and writes it - * to the \a payload. It blocks until one message is received, if an - * asynchronous behavior is desired this method should be called when the - * readyRead signal is emitted. + * to the \a payload. If no message payload is available, it returns + * immediately with -EAGAIN. The \ref readyRead signal shall be used to receive + * notification of message availability. * * \todo Add state machine to make sure we don't block forever and that * a header is always followed by a payload. * * \return 0 on success or a negative error code otherwise + * \retval -EAGAIN No message payload is available + * \retval -ENOTCONN The socket is not connected (neither create() nor bind() + * has been called) */ int IPCUnixSocket::receive(Payload *payload) { - Header hdr; - int ret; - if (!isBound()) return -ENOTCONN; - if (!payload) - return -EINVAL; + if (!headerReceived_) + return -EAGAIN; - ret = ::recv(fd_, &hdr, sizeof(hdr), 0); - if (ret < 0) { - ret = -errno; - LOG(IPCUnixSocket, Error) - << "Failed to recv header: " << strerror(-ret); + payload->data.resize(header_.data); + payload->fds.resize(header_.fds); + + int ret = recvData(payload->data.data(), header_.data, + payload->fds.data(), header_.fds); + if (ret < 0) return ret; - } - payload->data.resize(hdr.data); - payload->fds.resize(hdr.fds); + headerReceived_ = false; + notifier_->setEnabled(true); - return recvData(payload->data.data(), hdr.data, payload->fds.data(), hdr.fds); + return 0; } /** @@ -232,7 +234,8 @@ int IPCUnixSocket::receive(Payload *payload) * \brief A Signal emitted when a message is ready to be read */ -int IPCUnixSocket::sendData(const void *buffer, size_t length, const int32_t *fds, unsigned int num) +int IPCUnixSocket::sendData(const void *buffer, size_t length, + const int32_t *fds, unsigned int num) { struct iovec iov[1]; iov[0].iov_base = const_cast(buffer); @@ -266,7 +269,8 @@ int IPCUnixSocket::sendData(const void *buffer, size_t length, const int32_t *fd return 0; } -int IPCUnixSocket::recvData(void *buffer, size_t length, int32_t *fds, unsigned int num) +int IPCUnixSocket::recvData(void *buffer, size_t length, + int32_t *fds, unsigned int num) { struct iovec iov[1]; iov[0].iov_base = buffer; @@ -291,8 +295,9 @@ int IPCUnixSocket::recvData(void *buffer, size_t length, int32_t *fds, unsigned if (recvmsg(fd_, &msg, 0) < 0) { int ret = -errno; - LOG(IPCUnixSocket, Error) - << "Failed to recvmsg: " << strerror(-ret); + if (ret != -EAGAIN) + LOG(IPCUnixSocket, Error) + << "Failed to recvmsg: " << strerror(-ret); return ret; } @@ -303,6 +308,35 @@ int IPCUnixSocket::recvData(void *buffer, size_t length, int32_t *fds, unsigned void IPCUnixSocket::dataNotifier(EventNotifier *notifier) { + int ret; + + if (!headerReceived_) { + /* Receive the header. */ + ret = ::recv(fd_, &header_, sizeof(header_), 0); + if (ret < 0) { + ret = -errno; + LOG(IPCUnixSocket, Error) + << "Failed to receive header: " << strerror(-ret); + return; + } + + headerReceived_ = true; + } + + /* + * If the payload has arrived, disable the notifier and emit the + * readyRead signal. The notifier will be reenabled by the receive() + * method. + */ + struct pollfd fds = { fd_, POLLIN, 0 }; + ret = poll(&fds, 1, 0); + if (ret < 0) + return; + + if (!(fds.revents & POLLIN)) + return; + + notifier_->setEnabled(false); readyRead.emit(this); }