[libcamera-devel] [RFC 1/2] libcamera: ipc: unix: Add a IPC mechanism based on Unix sockets
Kieran Bingham
kieran.bingham at ideasonboard.com
Mon Jun 24 09:44:08 CEST 2019
Hi Niklas,
Great I'm very pleased to see this RFC!
- thanks for tackling this topic!
On 21/06/2019 05:15, Niklas Söderlund wrote:
> To be able to isolate an IPA component in a separate process and IPC
> mechanism is needed to communicate with it. Add a IPC mechanism based on
> Unix sockets which allows users to pass both data and file descriptors
> to 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,
>
> The implementation guarantees that a whole object is transmitted and
> received over IPC before it's handed of. This allows IPC users to not
> have to deal with buffering and may depend on that it only needs to deal
> with serialization/deserialization of complete object blobs.
>
> The implementation also provides a priv field in the IPC message header
> which is a 32 bit integer that can be used by IPA implementations that
> do not require a complex protocol header to describe what type of
> message is transmitted.
>
> Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> ---
> src/libcamera/include/ipc_unixsocket.h | 58 +++++
> src/libcamera/ipc_unixsocket.cpp | 330 +++++++++++++++++++++++++
> src/libcamera/meson.build | 2 +
> 3 files changed, 390 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 0000000000000000..864fa93b1f190fb7
> --- /dev/null
> +++ b/src/libcamera/include/ipc_unixsocket.h
> @@ -0,0 +1,58 @@
> +/* 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 <cstdint>
> +#include <sys/types.h>
> +#include <vector>
> +
> +namespace libcamera {
> +
> +class IPCUnixSocket
> +{
> +public:
> + struct Payload {
> + uint32_t priv;
> + std::vector<uint8_t> data;
> + std::vector<int32_t> fds;
> + };
> +
> + IPCUnixSocket();
> + IPCUnixSocket(int fd);
> +
> + int create();
> + int connect();
> + void close();
> +
> + int send(const Payload &payload);
> + int recv(Payload *payload, int timeout);
> + int call(const Payload &payload, Payload *response, int timeout);
> +
> +private:
> + struct Header {
> + uint32_t priv;
> + uint32_t data;
> + uint8_t fds;
> + };
> +
> + int poll(int timeout);
> +
> + int sendData(const void *buffer, ssize_t length);
> + int recvData(void *buffer, ssize_t length, int timeout);
> +
> + int sendFds(const int32_t *fds, unsigned int num);
> + int recvFds(int32_t *fds, unsigned int num, int timeout);
> +
> + int fd_;
> + bool master_;
> +};
> +
> +} /* 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 0000000000000000..b34fa0317a18b37c
> --- /dev/null
> +++ b/src/libcamera/ipc_unixsocket.cpp
> @@ -0,0 +1,330 @@
> +/* 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 <errno.h>
> +#include <poll.h>
> +#include <stdio.h>
> +#include <string.h>
> +#include <sys/socket.h>
> +#include <unistd.h>
> +
> +#include "log.h"
> +
> +/**
> + * \file ipc_unixsocket.h
> + * \brief IPC mechanism based on Unix sockets
> + */
> +
> +/*
> + * Markers to use in IPC protocol, there is no specific meaning to the values,
> + * but they should be unique.
> + */
> +#define CMD_PING 0x1f
> +#define CMD_PONG 0xf1
> +#define CMD_FD 0x77
> +
> +namespace libcamera {
> +
> +LOG_DEFINE_CATEGORY(IPCUnixSocket)
> +
> +IPCUnixSocket::IPCUnixSocket()
> + : fd_(-1), master_(false)
> +{
> +}
> +
> +IPCUnixSocket::IPCUnixSocket(int fd)
> + : fd_(fd), master_(false)
> +{
> +}
Trivial - but I think you could merge those two constructors with a
default value for "int fd=-1" on the function prototype.
> +
> +int IPCUnixSocket::create()
> +{
> + int sockets[2];
> + int ret;
> +
> + ret = socketpair(AF_UNIX, SOCK_STREAM, 0, sockets);
> + if (ret) {
> + ret = -errno;
> + LOG(IPCUnixSocket, Error)
> + << "Failed to create socket pair: " << strerror(-ret);
> + return ret;
> + }
> +
> + fd_ = sockets[0];
> + master_ = true;
> +
> + return sockets[1];
> +}
> +
> +int IPCUnixSocket::connect()
> +{
> + Payload payload = {};
I'd call this 'Payload message', as the one below is 'Payload response'.
Otherwise we're doing Int int.
> + Payload response = {};
> +
> + if (master_) {
> + payload.data.push_back(CMD_PING);
> +
> + if (call(payload, &response, 500))
> + return -1;
> +
> + if (response.data[0] != CMD_PONG)
> + return -1;
> + } else {
> + if (recv(&payload, 500))
> + return -1;
> +
> + if (payload.data[0] != CMD_PING)
> + return -1;
> +
> + response.data.push_back(CMD_PONG);
> +
> + if (send(response))
> + return -1;
> + }
This doesn't really seem like a 'connect()' call?
It's more of a ping test?
Is it required to establish communication between the sockets?
I think the SocketPair does everything necessary to establish the link
between the two sockets - so perhaps this is just to start to define a
base IPC API ?
(If so -that should really be in the pure-virtual base class header, and
this function implemented as a noop or such).
> +
> + return 0;
> +}
> +
> +void IPCUnixSocket::close()
> +{
> + if (fd_ == -1)
> + return;
> +
> + ::close(fd_);
> +
> + fd_ = -1;
> +}
> +
> +int IPCUnixSocket::send(const Payload &payload)
> +{
> + Header hdr;
> + int ret;
> +
> + if (fd_ < 0)
> + return -ENOTCONN;
> +
> + hdr.priv = payload.priv;
> + hdr.data = payload.data.size();
> + hdr.fds = payload.fds.size();
> +
> + ret = sendData(&hdr, sizeof(hdr));
> + if (ret)
> + return ret;
> +
> + if (hdr.data) {
> + ret = sendData(payload.data.data(), hdr.data);
> + if (ret)
> + return ret;
> + }
> +
> + if (hdr.fds) {
> + ret = sendFds(payload.fds.data(), hdr.fds);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +int IPCUnixSocket::recv(Payload *payload, int timeout)
s/recv/receive/ ?
> +{
> + Header hdr;
> + int ret;
> +
> + if (fd_ < 0)
> + return -ENOTCONN;
> +
> + ret = recvData(&hdr, sizeof(hdr), timeout);
> + if (ret)
> + return ret;
> +
> + payload->priv = hdr.priv;
> + payload->data.resize(hdr.data);
> + payload->fds.resize(hdr.fds);
> +
> + if (hdr.data) {
> + ret = recvData(payload->data.data(), hdr.data, timeout);
> + if (ret)
> + return ret;
> + }
> +
> + if (hdr.fds) {
> + ret = recvFds(payload->fds.data(), hdr.fds, timeout);
> + if (ret)
> + return ret;
> + }
> +
> + return 0;
> +}
> +
> +int IPCUnixSocket::call(const Payload &payload, Payload *response, int timeout)
> +{
> + int ret = send(payload);
> + if (ret)
> + return ret;
> +
> + return recv(response, timeout);
> +}
> +
> +int IPCUnixSocket::poll(int timeout)
> +{
> + struct pollfd pollfd = { fd_, POLLIN, 0 };
> +
> + int ret = ::poll(&pollfd, 1, timeout);
> + if (ret < 0) {
> + ret = -errno;
> + LOG(IPCUnixSocket, Error)
> + << "Failed to poll: " << strerror(-ret);
> + return ret;
> + } else if (ret == 0) {
> + return -ETIMEDOUT;
> + }
> +
> + return 0;
> +}
> +
> +int IPCUnixSocket::sendData(const void *buffer, ssize_t length)
> +{
> + ssize_t len, sent;
> + const uint8_t *pos;
> +
> + if (fd_ < 0)
> + return -ENOTCONN;
> +
> + pos = static_cast<const uint8_t *>(buffer);
> + len = length;
> +
> + while (len) {
> + sent = ::send(fd_, pos, len, 0);
> + if (sent < 0) {
> + sent = -errno;
> + LOG(IPCUnixSocket, Error)
> + << "Failed to send: " << strerror(-sent);
> + return sent;
> + }
> +
> + pos += sent;
> + len -= sent;
> + }
> +
> + return 0;
> +}If it's P
> +
> +int IPCUnixSocket::recvData(void *buffer, ssize_t length, int timeout)
> +{
> + ssize_t len, recived;
> + uint8_t *pos;
> +
> + if (fd_ < 0)
> + return -ENOTCONN;
> +
> + pos = static_cast<uint8_t *>(buffer);
> + len = length;
> +
> + while (len) {
> + int ret = poll(timeout);
> + if (ret < 0)
> + return ret;
> +
> + recived = ::recv(fd_, pos, len, 0);
> + if (recived < 0) {
> + recived = -errno;
> + LOG(IPCUnixSocket, Error)
> + << "Failed to recv: " << strerror(-recived);
> + return recived;
> + }
> +
> + pos += recived;
> + len -= recived;
> + }
> +
> + return 0;
> +}
> +
> +int IPCUnixSocket::sendFds(const int32_t *fds, unsigned int num)
> +{
> + char cmd = CMD_FD;
> + struct iovec iov[1];
> + iov[0].iov_base = &cmd;
> + iov[0].iov_len = sizeof(cmd);
> +
> + 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::recvFds(int32_t *fds, unsigned int num, int timeout)
> +{
> + char cmd;
> + struct iovec iov[1];
> + iov[0].iov_base = &cmd;
> + iov[0].iov_len = sizeof(cmd);
> +
> + 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;
> +
> + int ret = poll(timeout);
> + if (ret < 0)
> + return ret;
> +
> + if (recvmsg(fd_, &msg, 0) < 0) {
> + int ret = -errno;
> + LOG(IPCUnixSocket, Error)
> + << "Failed to recvmsg: " << strerror(-ret);
> + return ret;
> + }
> +
> + if (cmd != CMD_FD) {
> + LOG(IPCUnixSocket, Error) << "FD marker wrong";
> + return -EINVAL;
> + }
> +
> + memcpy(fds, CMSG_DATA(cmsg), num * sizeof(uint32_t));
> +
> + return 0;
> +}
> +
> +} /* namespace libcamera */
> diff --git a/src/libcamera/meson.build b/src/libcamera/meson.build
> index f26ad5b2dc57c014..1158825fa5b0702d 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',
> @@ -37,6 +38,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',
>
--
Regards
--
Kieran
More information about the libcamera-devel
mailing list