[libcamera-devel] [PATCH 2/2] test: ipc: unix: Add test for IPCUnixSocket
Niklas Söderlund
niklas.soderlund at ragnatech.se
Fri Jun 28 12:53:14 CEST 2019
Hi Kieran,
Thanks for your feedback,
On 2019-06-28 11:17:10 +0100, Kieran Bingham wrote:
> Hi Niklas,
>
> On 27/06/2019 03:09, Niklas Söderlund wrote:
> > Test that the IPC supports sending data and file descriptors over the
> > IPC medium. To be able 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 sometime respond to. A few different tests are preformed.
> >
> > - 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 sends a list of file descriptors and ask the salve to calculate
> > and respond with the sum of the size of the files. The master verifies
> > that the calculate size is correct.
> >
> > - Master send a pre-computed size and a list of file descriptors and
> > ask the slave to verify that the pre-computed size matches the sum of
> > the size of the file descriptors.
> >
> > Signed-off-by: Niklas Söderlund <niklas.soderlund at ragnatech.se>
> > ---
> > test/ipc/meson.build | 12 ++
> > test/ipc/unixsocket.cpp | 339 ++++++++++++++++++++++++++++++++++++++++
> > test/meson.build | 1 +
> > 3 files changed, 352 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..ca8375f35df91731
> > --- /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 0000000000000000..c50d7463cee48e75
> > --- /dev/null
> > +++ b/test/ipc/unixsocket.cpp
> > @@ -0,0 +1,339 @@
> > +/* 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 <iostream>
> > +#include <string.h>
> > +#include <sys/wait.h>
> > +#include <unistd.h>
> > +
> > +#include <libcamera/camera_manager.h>
> > +#include <libcamera/event_dispatcher.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
> > +
> > +using namespace std;
> > +using namespace libcamera;
> > +
> > +int calcLength(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)
> > + {
> > + dispatcher_ = CameraManager::instance()->eventDispatcher();
> > + exit_.store(false, std::memory_order_release);
> > + ipc_.payloadReceived.connect(this, &UnixSocketTestSlave::payloadReceived);
> > + }
> > +
> > + int run(int fd)
> > + {
> > + if (ipc_.attach(fd)) {
> > + cerr << "Failed to connect to IPC" << endl;
> > + return EXIT_FAILURE;
> > + }
> > +
> > + while (!exit_.load(std::memory_order_acquire))
> > + dispatcher_->processEvents();
> > +
> > + ipc_.close();
> > +
> > + return exitCode_;
> > + }
> > +
> > +private:
> > + void payloadReceived(const IPCUnixSocket::Payload &payload)
> > + {
> > + IPCUnixSocket::Payload response;
> > + int ret;
> > +
> > + const uint8_t cmd = payload.data.front();
> > +
> > + cout << "Slave received command " << static_cast<unsigned int>(cmd) << endl;
> > +
> > + switch (cmd) {
> > + case CMD_CLOSE:
> > + stop(0);
> > + break;
> > + case CMD_REVERSE: {
> > + std::vector<uint8_t> raw(payload.data.begin() + 1, payload.data.end());
> > + std::reverse(raw.begin(), raw.end());
> > + raw.insert(raw.begin(), cmd);
> > +
> > + response.data = raw;
> > + ret = ipc_.send(response);
> > + if (ret < 0) {
> > + cerr << "Reverse fail" << endl;
> > + stop(ret);
> > + }
> > + break;
> > + }
> > + case CMD_LEN_CALC: {
> > + int size = 0;
> > + for (int fd : payload.fds)
> > + size += calcLength(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 : payload.fds)
> > + size += calcLength(fd);
> > +
> > + int cmp;
> > + memcpy(&cmp, payload.data.data() + 1, sizeof(cmp));
> > +
> > + if (cmp != size) {
> > + cerr << "Compare fail" << endl;
> > + stop(-ERANGE);
> > + }
> > + break;
> > + }
> > + default:
> > + cerr << "Unknown command " << cmd << endl;
> > + stop(-EINVAL);
> > + }
> > + }
> > +
> > + void stop(int code)
> > + {
> > + exitCode_ = code;
> > + exit_.store(true, std::memory_order_release);
> > + }
> > +
> > + IPCUnixSocket ipc_;
> > + EventDispatcher *dispatcher_;
> > + int exitCode_;
> > + std::atomic<bool> exit_;
> > +};
> > +
> > +static const std::vector<std::string> testPaths = {
> > + "/proc/self/exe",
> > + "/proc/self/exe",
> > +};
>
> This looks odd - is an array of two identical strings really the best
> way to make something loop twice? (I think that's the only use of this?)
I agree it looks odd, but it does the job :-) All that is needed is two
file handles (or more) to test that multiple FDs can be transferred.
Once could do with one FD but I feel its better to test with more.
>
> Perhaps should one of those instances of /proc/self/exe be another file
> in /proc/self ? comm? maps? As far as I understand it- it just has to
> be constant when read twice right ?
I tried finding another file in /proc/self that is a regular file and
thus have a size. I only managed to find exe and map_files/* where the
later provides no stable naming.
I'm open to using a different path here which would be found on every
system, but not sure what that would be, /etc/issue maybe? I'm OK
keeping /proc/self/exe multiple times as it does the job and at the same
time I'm also open to suggestions of different stable paths to replace
one of them with.
>
> > +
> > +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 reverse;
> > +
> > + reverse.data = { CMD_REVERSE, 1, 2, 3, 4, 5 };
> > + if (ipc_.send(reverse))
> > + return TestFail;
> > +
> > + CameraManager::instance()->eventDispatcher()->processEvents();
> > +
> > + return 0;
> > + }
> > +
> > + int testCalc()
> > + {
> > + IPCUnixSocket::Payload payload;
> > +
> > + calcSize_ = 0;
> > + for (const std::string path : testPaths) {
> > + int fd = open(path.c_str(), O_RDONLY);
> > + if (fd < 0)
> > + return TestFail;
> > +
> > + calcSize_ += calcLength(fd);
> > + payload.fds.push_back(fd);
> > + }
> > +
> > + payload.data.push_back(CMD_LEN_CALC);
> > + if (ipc_.send(payload))
> > + return TestFail;
> > +
> > + CameraManager::instance()->eventDispatcher()->processEvents();
> > +
> > + return 0;
> > + }
> > +
> > + int testCmp()
> > + {
> > + IPCUnixSocket::Payload payload;
> > +
> > + int size = 0;
> > + for (const std::string path : testPaths) {
> > + int fd = open(path.c_str(), O_RDONLY);
> > + if (fd < 0)
> > + return TestFail;
> > +
> > + size += calcLength(fd);
> > + payload.fds.push_back(fd);
> > + }
> > +
> > + payload.data.resize(1 + sizeof(size));
> > + payload.data[0] = CMD_LEN_CMP;
> > + memcpy(payload.data.data() + 1, &size, sizeof(size));
> > +
> > + if (ipc_.send(payload))
> > + return TestFail;
> > +
> > + return 0;
> > + }
> > +
> > + int run()
> > + {
> > + int slavefd = ipc_.create();
> > + if (slavefd < 0)
> > + return TestFail;
> > +
> > + if (slaveStart(slavefd))
> > + return TestFail;
> > +
> > + ipc_.payloadReceived.connect(this, &UnixSocketTest::payloadReceived);
> > +
> > + /* Test reversing a string, this test sending only data. */
> > + ranRev_ = false;
> > + if (testReverse())
> > + return TestFail;
> > +
> > + /* Test offloading a calculation, this test sending only FDs. */
> > + ranCalc_ = false;
> > + if (testCalc())
> > + return TestFail;
> > +
> > + /* Test fire and forget, this tests sending data and FDs. */
> > + if (testCmp())
> > + return TestFail;
> > +
> > + /* Close slave connection. */
> > + IPCUnixSocket::Payload close;
> > + close.data.push_back(CMD_CLOSE);
> > + if (ipc_.send(close))
> > + return TestFail;
> > +
> > + ipc_.close();
> > + if (slaveStop())
> > + return TestFail;
> > +
> > + /* Check that all tests have executed. */
> > + if (!ranRev_ || !ranCalc_) {
> > + cerr << "Not all messages responded to" << endl;
> > + return TestFail;
> > + }
> > +
> > + return TestPass;
> > + }
> > +
> > +private:
> > + void payloadReceived(const IPCUnixSocket::Payload &payload)
> > + {
> > + const uint8_t cmd = payload.data.front();
> > + cout << "Master received command " << static_cast<unsigned int>(cmd) << endl;
> > +
> > + switch (cmd) {
> > + case CMD_REVERSE: {
> > + std::vector<uint8_t> raw(payload.data.begin() + 1, payload.data.end());
> > + if (raw == std::vector<uint8_t>{ 5, 4, 3, 2, 1 })
> > + ranRev_ = true;
> > + else
> > + cerr << "Reverse response invalid" << endl;
> > +
> > + break;
> > + }
> > + case CMD_LEN_CALC: {
> > + int output;
> > + memcpy(&output, payload.data.data() + 1, sizeof(output));
> > + if (output == calcSize_)
> > + ranCalc_ = true;
> > + else
> > + cerr << "Calc response invalid" << endl;
> > +
> > + break;
> > + }
> > + default:
> > + cerr << "Unknown command " << cmd << endl;
> > + }
> > + }
> > +
> > + pid_t pid_;
> > + IPCUnixSocket ipc_;
> > +
> > + bool ranRev_;
> > + bool ranCalc_;
> > + int calcSize_;
> > +};
> > +
> > +/*
> > + * 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
> --
> Kieran
More information about the libcamera-devel
mailing list