[libcamera-devel] [PATCH v3 2/2] test: ipc: unix: Add test for IPCUnixSocket
Laurent Pinchart
laurent.pinchart at ideasonboard.com
Tue Jul 2 00:37:07 CEST 2019
Hi Niklas,
Thank you for the patch.
On Tue, Jul 02, 2019 at 12:06:12AM +0200, Niklas Söderlund wrote:
> 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.
s/ties to sends/tries to send/
We may allow sending empty messages later, but for now this makes sense.
> - 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
s/salve/slave/
> 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 <niklas.soderlund at ragnatech.se>
> ---
> test/ipc/meson.build | 17 ++
> test/ipc/unixsocket.cpp | 510 ++++++++++++++++++++++++++++++++++++++++
> test/meson.build | 1 +
> 3 files changed, 528 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 0000000000000000..b557698d4b264ca5
> --- /dev/null
> +++ b/test/ipc/meson.build
> @@ -0,0 +1,17 @@
> +ipc_tests = [
> + [ 'unixsocket', 'unixsocket.cpp' ],
> +]
> +
> +ipc_tests_dep = [
> + cc.find_library('rt'),
> + libcamera_dep,
4 spaces for indentation.
> +]
> +
> +foreach t : ipc_tests
> + exe = executable(t[0], t[1],
> + dependencies : ipc_tests_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 0000000000000000..d3e1d9227c537e66
> --- /dev/null
> +++ b/test/ipc/unixsocket.cpp
> @@ -0,0 +1,510 @@
> +/* SPDX-License-Identifier: GPL-2.0-or-later */
> +/*
> + * Copyright (C) 2019, Google Inc.
> + *
> + * unixsocket.cpp - Unix socket IPC test
> + */
> +
> +#include <algorithm>
> +#include <atomic>
> +#include <fcntl.h>
> +#include <fcntl.h>
This is why I always insist on sorting headers alphabetically :-)
> +#include <iostream>
> +#include <string.h>
> +#include <sys/mman.h>
> +#include <sys/stat.h>
> +#include <sys/wait.h>
> +#include <unistd.h>
> +#include <string.h>
> +
> +#include <libcamera/camera_manager.h>
> +#include <libcamera/event_dispatcher.h>
> +#include <libcamera/timer.h>
> +
> +#include "ipc_unixsocket.h"
> +#include "test.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;
> +
> + if (ipc->receive(&message)) {
> + cerr << "Receive message failed" << 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 fail" << 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 fail" << 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 fail" << endl;
> + stop(-ERANGE);
> + }
> + break;
> + }
> +
> + case CMD_JOIN: {
> + int outfd = shm_open("/libcamera.ipc.unixsocket.fdorder.out",
> + O_RDWR | O_CREAT | O_TRUNC,
> + S_IRUSR | S_IWUSR);
> +
> + if (outfd < 0) {
> + cerr << "Create out file fail" << endl;
s/fail/failed/
> + stop(outfd);
> + return;
> + }
> +
> + for (int fd : message.fds) {
> + while (true) {
> + char buf[32];
> + int num = read(fd, &buf, sizeof(buf));
s/int/ssize_t/
> +
> + if (num < 0) {
> + cerr << "Read fail" << endl;
s/fail/failed/
> + stop(-EIO);
> + return;
> + } else if (!num)
> + break;
> +
> + if (write(outfd, buf, num) < 0) {
> + cerr << "Write fail" << endl;
s/fail/failed/
> + stop(-EIO);
> + return;
> + }
> + }
> +
> + close(fd);
This will very likely become a cause of fd leaks in the future. Should
we create a File object that will wrap an fd, and store a
std::vector<std::unique_ptr<File>> in the Payload ? That should help
preventing leaks. It can be added on top of this series.
> + }
> +
> + lseek(outfd, 0, 0);
> + response.data.push_back(CMD_JOIN);
> + response.fds.push_back(outfd);
> +
> + ret = ipc_.send(response);
> + if (ret < 0) {
> + cerr << "Join fail" << endl;
s/fail/failed/
> + 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;
> +
> + struct {
> + const char *name;
> + const char *string;
> + int fd;
> + } data[] = {
> + { .name = "/libcamera.ipc.unixsocket.fdorder.0", .string = "Foo", .fd = -1 },
> + { .name = "/libcamera.ipc.unixsocket.fdorder.1", .string = "Bar", .fd = -1 },
> + { .name = nullptr, .string = nullptr, .fd = -1 },
It's a bit annoying that shm_open() requires a name :-S I had a look for
possible replacements, memfd_create() seems nice but is quite recent and
doesn't seem available in all libc implementations. open() with
O_TMPFILE could come to the rescue. As I don't want to ask you to respin
yet another version of this patch, do you mind if I take over to
experiment ?
> + };
> +
> + for (unsigned int i = 0; data[i].name && data[i].string; i++) {
for (unsigned int i = 0; i < ARRAY_SIZE(data); ++i) {
and you could remove the empty entry in data.
> + unsigned int len = strlen(data[i].string);
> +
> + data[i].fd = shm_open(data[i].name,
> + O_RDWR | O_CREAT | O_TRUNC,
> + S_IRUSR | S_IWUSR);
> +
No need for this blank line.
> + if (data[i].fd < 0)
> + return TestFail;
> +
> + ret = write(data[i].fd, data[i].string, len);
> + if (ret < 0)
> + return TestFail;
> +
> + lseek(data[i].fd, 0, 0);
> + message.fds.push_back(data[i].fd);
> + }
> +
> + message.data.push_back(CMD_JOIN);
> +
> + ret = call(message, &response);
> + if (ret)
> + return ret;
> +
> + for (unsigned int i = 0; data[i].name && data[i].string; i++) {
> + unsigned int len = strlen(data[i].string);
> + char buf[len];
> +
> + close(data[i].fd);
> +
> + if (read(response.fds[0], &buf, len) <= 0)
> + return TestFail;
> +
> + if (strncmp(buf, data[i].string, 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 empty message fails. */
s/Test/Test that an/
> + if (testEmptyFail()) {
> + cerr << "empty test failed" << endl;
s/empty/Empty message/
> + 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 c36ac24796367501..3666f6b2385bd4ca 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')
--
Regards,
Laurent Pinchart
More information about the libcamera-devel
mailing list