[{"id":2048,"web_url":"https://patchwork.libcamera.org/comment/2048/","msgid":"<433ee512-2a14-03aa-b55f-f08845eacd62@ideasonboard.com>","date":"2019-06-28T10:17:10","subject":"Re: [libcamera-devel] [PATCH 2/2] test: ipc: unix: Add test for\n\tIPCUnixSocket","submitter":{"id":4,"url":"https://patchwork.libcamera.org/api/people/4/","name":"Kieran Bingham","email":"kieran.bingham@ideasonboard.com"},"content":"Hi Niklas,\n\nOn 27/06/2019 03:09, Niklas Söderlund wrote:\n> Test that the IPC supports sending data and file descriptors over the\n> IPC medium. To be able execute the test two parts are needed, one\n> to drive the test and act as the libcamera (master) and a one to act as\n> the IPA (slave).\n> \n> The master drives the testing posting requests to the slave to process\n> and sometime respond to. A few different tests are preformed.\n> \n> - Master sends an array to the slave which responds with a reversed copy\n>   of the array. The master verifies that a reversed array is returned.\n> \n> - Master sends a list of file descriptors and ask the salve to calculate\n>   and respond with the sum of the size of the files. The master verifies\n>   that the calculate size is correct.\n> \n> - Master send a pre-computed size and a list of file descriptors and\n>   ask the slave to verify that the pre-computed size matches the sum of\n>   the size of the file descriptors.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  test/ipc/meson.build    |  12 ++\n>  test/ipc/unixsocket.cpp | 339 ++++++++++++++++++++++++++++++++++++++++\n>  test/meson.build        |   1 +\n>  3 files changed, 352 insertions(+)\n>  create mode 100644 test/ipc/meson.build\n>  create mode 100644 test/ipc/unixsocket.cpp\n> \n> diff --git a/test/ipc/meson.build b/test/ipc/meson.build\n> new file mode 100644\n> index 0000000000000000..ca8375f35df91731\n> --- /dev/null\n> +++ b/test/ipc/meson.build\n> @@ -0,0 +1,12 @@\n> +ipc_tests = [\n> +    [ 'unixsocket',  'unixsocket.cpp' ],\n> +]\n> +\n> +foreach t : ipc_tests\n> +    exe = executable(t[0], t[1],\n> +                     dependencies : libcamera_dep,\n> +                     link_with : test_libraries,\n> +                     include_directories : test_includes_internal)\n> +\n> +    test(t[0], exe, suite : 'ipc', is_parallel : false)\n> +endforeach\n> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> new file mode 100644\n> index 0000000000000000..c50d7463cee48e75\n> --- /dev/null\n> +++ b/test/ipc/unixsocket.cpp\n> @@ -0,0 +1,339 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * unixsocket.cpp - Unix socket IPC test\n> + */\n> +\n> +#include <algorithm>\n> +#include <atomic>\n> +#include <fcntl.h>\n> +#include <iostream>\n> +#include <string.h>\n> +#include <sys/wait.h>\n> +#include <unistd.h>\n> +\n> +#include <libcamera/camera_manager.h>\n> +#include <libcamera/event_dispatcher.h>\n> +\n> +#include \"ipc_unixsocket.h\"\n> +#include \"test.h\"\n> +\n> +#define CMD_CLOSE 0\n> +#define CMD_REVERSE 1\n> +#define CMD_LEN_CALC 2\n> +#define CMD_LEN_CMP 3\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +int calcLength(int fd)\n> +{\n> +\tlseek(fd, 0, 0);\n> +\tint size = lseek(fd, 0, SEEK_END);\n> +\tlseek(fd, 0, 0);\n> +\n> +\treturn size;\n> +}\n> +\n> +class UnixSocketTestSlave\n> +{\n> +public:\n> +\tUnixSocketTestSlave()\n> +\t\t: exitCode_(EXIT_FAILURE)\n> +\t{\n> +\t\tdispatcher_ = CameraManager::instance()->eventDispatcher();\n> +\t\texit_.store(false, std::memory_order_release);\n> +\t\tipc_.payloadReceived.connect(this, &UnixSocketTestSlave::payloadReceived);\n> +\t}\n> +\n> +\tint run(int fd)\n> +\t{\n> +\t\tif (ipc_.attach(fd)) {\n> +\t\t\tcerr << \"Failed to connect to IPC\" << endl;\n> +\t\t\treturn EXIT_FAILURE;\n> +\t\t}\n> +\n> +\t\twhile (!exit_.load(std::memory_order_acquire))\n> +\t\t\tdispatcher_->processEvents();\n> +\n> +\t\tipc_.close();\n> +\n> +\t\treturn exitCode_;\n> +\t}\n> +\n> +private:\n> +\tvoid payloadReceived(const IPCUnixSocket::Payload &payload)\n> +\t{\n> +\t\tIPCUnixSocket::Payload response;\n> +\t\tint ret;\n> +\n> +\t\tconst uint8_t cmd = payload.data.front();\n> +\n> +\t\tcout << \"Slave received command \" << static_cast<unsigned int>(cmd) << endl;\n> +\n> +\t\tswitch (cmd) {\n> +\t\tcase CMD_CLOSE:\n> +\t\t\tstop(0);\n> +\t\t\tbreak;\n> +\t\tcase CMD_REVERSE: {\n> +\t\t\tstd::vector<uint8_t> raw(payload.data.begin() + 1, payload.data.end());\n> +\t\t\tstd::reverse(raw.begin(), raw.end());\n> +\t\t\traw.insert(raw.begin(), cmd);\n> +\n> +\t\t\tresponse.data = raw;\n> +\t\t\tret = ipc_.send(response);\n> +\t\t\tif (ret < 0) {\n> +\t\t\t\tcerr << \"Reverse fail\" << endl;\n> +\t\t\t\tstop(ret);\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase CMD_LEN_CALC: {\n> +\t\t\tint size = 0;\n> +\t\t\tfor (int fd : payload.fds)\n> +\t\t\t\tsize += calcLength(fd);\n> +\n> +\t\t\tresponse.data.resize(1 + sizeof(size));\n> +\t\t\tresponse.data[0] = cmd;\n> +\t\t\tmemcpy(response.data.data() + 1, &size, sizeof(size));\n> +\n> +\t\t\tret = ipc_.send(response);\n> +\t\t\tif (ret < 0) {\n> +\t\t\t\tcerr << \"Calc fail\" << endl;\n> +\t\t\t\tstop(ret);\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase CMD_LEN_CMP: {\n> +\t\t\tint size = 0;\n> +\t\t\tfor (int fd : payload.fds)\n> +\t\t\t\tsize += calcLength(fd);\n> +\n> +\t\t\tint cmp;\n> +\t\t\tmemcpy(&cmp, payload.data.data() + 1, sizeof(cmp));\n> +\n> +\t\t\tif (cmp != size) {\n> +\t\t\t\tcerr << \"Compare fail\" << endl;\n> +\t\t\t\tstop(-ERANGE);\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tdefault:\n> +\t\t\tcerr << \"Unknown command \" << cmd << endl;\n> +\t\t\tstop(-EINVAL);\n> +\t\t}\n> +\t}\n> +\n> +\tvoid stop(int code)\n> +\t{\n> +\t\texitCode_ = code;\n> +\t\texit_.store(true, std::memory_order_release);\n> +\t}\n> +\n> +\tIPCUnixSocket ipc_;\n> +\tEventDispatcher *dispatcher_;\n> +\tint exitCode_;\n> +\tstd::atomic<bool> exit_;\n> +};\n> +\n> +static const std::vector<std::string> testPaths = {\n> +\t\"/proc/self/exe\",\n> +\t\"/proc/self/exe\",\n> +};\n\nThis looks odd - is an array of two identical strings really the best\nway to make something loop twice? (I think that's the only use of this?)\n\nPerhaps should one of those instances of /proc/self/exe be another file\nin /proc/self ? comm? maps? As far as I understand it-  it just has to\nbe constant when read twice right ?\n\n> +\n> +class UnixSocketTest : public Test\n> +{\n> +protected:\n> +\tint slaveStart(int fd)\n> +\t{\n> +\t\tpid_ = fork();\n> +\n> +\t\tif (pid_ == -1)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (!pid_) {\n> +\t\t\tstd::string arg = std::to_string(fd);\n> +\t\t\texecl(\"/proc/self/exe\", \"/proc/self/exe\",\n> +\t\t\t      arg.c_str(), nullptr);\n> +\n> +\t\t\t/* Only get here if exec fails. */\n> +\t\t\texit(TestFail);\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint slaveStop()\n> +\t{\n> +\t\tint status;\n> +\n> +\t\tif (pid_ < 0)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (waitpid(pid_, &status, 0) < 0)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (!WIFEXITED(status) || WEXITSTATUS(status))\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint testReverse()\n> +\t{\n> +\t\tIPCUnixSocket::Payload reverse;\n> +\n> +\t\treverse.data = { CMD_REVERSE, 1, 2, 3, 4, 5 };\n> +\t\tif (ipc_.send(reverse))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tCameraManager::instance()->eventDispatcher()->processEvents();\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint testCalc()\n> +\t{\n> +\t\tIPCUnixSocket::Payload payload;\n> +\n> +\t\tcalcSize_ = 0;\n> +\t\tfor (const std::string path : testPaths) {\n> +\t\t\tint fd = open(path.c_str(), O_RDONLY);\n> +\t\t\tif (fd < 0)\n> +\t\t\t\treturn TestFail;\n> +\n> +\t\t\tcalcSize_ += calcLength(fd);\n> +\t\t\tpayload.fds.push_back(fd);\n> +\t\t}\n> +\n> +\t\tpayload.data.push_back(CMD_LEN_CALC);\n> +\t\tif (ipc_.send(payload))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tCameraManager::instance()->eventDispatcher()->processEvents();\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint testCmp()\n> +\t{\n> +\t\tIPCUnixSocket::Payload payload;\n> +\n> +\t\tint size = 0;\n> +\t\tfor (const std::string path : testPaths) {\n> +\t\t\tint fd = open(path.c_str(), O_RDONLY);\n> +\t\t\tif (fd < 0)\n> +\t\t\t\treturn TestFail;\n> +\n> +\t\t\tsize += calcLength(fd);\n> +\t\t\tpayload.fds.push_back(fd);\n> +\t\t}\n> +\n> +\t\tpayload.data.resize(1 + sizeof(size));\n> +\t\tpayload.data[0] = CMD_LEN_CMP;\n> +\t\tmemcpy(payload.data.data() + 1, &size, sizeof(size));\n> +\n> +\t\tif (ipc_.send(payload))\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint run()\n> +\t{\n> +\t\tint slavefd = ipc_.create();\n> +\t\tif (slavefd < 0)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (slaveStart(slavefd))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tipc_.payloadReceived.connect(this, &UnixSocketTest::payloadReceived);\n> +\n> +\t\t/* Test reversing a string, this test sending only data. */\n> +\t\tranRev_ = false;\n> +\t\tif (testReverse())\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Test offloading a calculation, this test sending only FDs. */\n> +\t\tranCalc_ = false;\n> +\t\tif (testCalc())\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Test fire and forget, this tests sending data and FDs. */\n> +\t\tif (testCmp())\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Close slave connection. */\n> +\t\tIPCUnixSocket::Payload close;\n> +\t\tclose.data.push_back(CMD_CLOSE);\n> +\t\tif (ipc_.send(close))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tipc_.close();\n> +\t\tif (slaveStop())\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Check that all tests have executed. */\n> +\t\tif (!ranRev_ || !ranCalc_) {\n> +\t\t\tcerr << \"Not all messages responded to\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +private:\n> +\tvoid payloadReceived(const IPCUnixSocket::Payload &payload)\n> +\t{\n> +\t\tconst uint8_t cmd = payload.data.front();\n> +\t\tcout << \"Master received command \" << static_cast<unsigned int>(cmd) << endl;\n> +\n> +\t\tswitch (cmd) {\n> +\t\tcase CMD_REVERSE: {\n> +\t\t\tstd::vector<uint8_t> raw(payload.data.begin() + 1, payload.data.end());\n> +\t\t\tif (raw == std::vector<uint8_t>{ 5, 4, 3, 2, 1 })\n> +\t\t\t\tranRev_ = true;\n> +\t\t\telse\n> +\t\t\t\tcerr << \"Reverse response invalid\" << endl;\n> +\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase CMD_LEN_CALC: {\n> +\t\t\tint output;\n> +\t\t\tmemcpy(&output, payload.data.data() + 1, sizeof(output));\n> +\t\t\tif (output == calcSize_)\n> +\t\t\t\tranCalc_ = true;\n> +\t\t\telse\n> +\t\t\t\tcerr << \"Calc response invalid\" << endl;\n> +\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tdefault:\n> +\t\t\tcerr << \"Unknown command \" << cmd << endl;\n> +\t\t}\n> +\t}\n> +\n> +\tpid_t pid_;\n> +\tIPCUnixSocket ipc_;\n> +\n> +\tbool ranRev_;\n> +\tbool ranCalc_;\n> +\tint calcSize_;\n> +};\n> +\n> +/*\n> + * Can't use TEST_REGISTER() as single binary needs to act as both proxy\n> + * master and slave.\n> + */\n> +int main(int argc, char **argv)\n> +{\n> +\tif (argc == 2) {\n> +\t\tint ipcfd = std::stoi(argv[1]);\n> +\t\tUnixSocketTestSlave slave;\n> +\t\treturn slave.run(ipcfd);\n> +\t}\n> +\n> +\treturn UnixSocketTest().execute();\n> +}\n> diff --git a/test/meson.build b/test/meson.build\n> index c36ac24796367501..3666f6b2385bd4ca 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -2,6 +2,7 @@ subdir('libtest')\n>  \n>  subdir('camera')\n>  subdir('ipa')\n> +subdir('ipc')\n>  subdir('media_device')\n>  subdir('pipeline')\n>  subdir('stream')\n>","headers":{"Return-Path":"<kieran.bingham@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 755D360BC7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 28 Jun 2019 12:17:13 +0200 (CEST)","from [192.168.0.20]\n\t(cpc89242-aztw30-2-0-cust488.18-1.cable.virginm.net [86.31.129.233])\n\tby perceval.ideasonboard.com (Postfix) with ESMTPSA id 9D1CC2C6;\n\tFri, 28 Jun 2019 12:17:12 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561717032;\n\tbh=0oErZI2xnS4op+V5y3p1jtYvJWY0lLseeiLjzHOQiV0=;\n\th=Reply-To:Subject:To:References:From:Date:In-Reply-To:From;\n\tb=CArVV1jvMyFXThpozUoWtId9UxBIycz1oE0CVKjon+IzNdluBaKMzJxPI14f0hVOb\n\tpus4D/En/siMFMAyyFCfvUhT/E1vhMC3Leyu9CCqR6NYsNKMspLfu7f4IZNMbxKauz\n\tagDNFv4F4nKyyaCGpLI6lMrYXjiAWUIqhMyptNpA=","Reply-To":"kieran.bingham@ideasonboard.com","To":"=?utf-8?q?Niklas_S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>,\n\tlibcamera-devel@lists.libcamera.org","References":"<20190627020955.6166-1-niklas.soderlund@ragnatech.se>\n\t<20190627020955.6166-3-niklas.soderlund@ragnatech.se>","From":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Openpgp":"preference=signencrypt","Autocrypt":"addr=kieran.bingham@ideasonboard.com; keydata=\n\tmQINBFYE/WYBEACs1PwjMD9rgCu1hlIiUA1AXR4rv2v+BCLUq//vrX5S5bjzxKAryRf0uHat\n\tV/zwz6hiDrZuHUACDB7X8OaQcwhLaVlq6byfoBr25+hbZG7G3+5EUl9cQ7dQEdvNj6V6y/SC\n\trRanWfelwQThCHckbobWiQJfK9n7rYNcPMq9B8e9F020LFH7Kj6YmO95ewJGgLm+idg1Kb3C\n\tpotzWkXc1xmPzcQ1fvQMOfMwdS+4SNw4rY9f07Xb2K99rjMwZVDgESKIzhsDB5GY465sCsiQ\n\tcSAZRxqE49RTBq2+EQsbrQpIc8XiffAB8qexh5/QPzCmR4kJgCGeHIXBtgRj+nIkCJPZvZtf\n\tKr2EAbc6tgg6DkAEHJb+1okosV09+0+TXywYvtEop/WUOWQ+zo+Y/OBd+8Ptgt1pDRyOBzL8\n\tRXa8ZqRf0Mwg75D+dKntZeJHzPRJyrlfQokngAAs4PaFt6UfS+ypMAF37T6CeDArQC41V3ko\n\tlPn1yMsVD0p+6i3DPvA/GPIksDC4owjnzVX9kM8Zc5Cx+XoAN0w5Eqo4t6qEVbuettxx55gq\n\t8K8FieAjgjMSxngo/HST8TpFeqI5nVeq0/lqtBRQKumuIqDg+Bkr4L1V/PSB6XgQcOdhtd36\n\tOe9X9dXB8YSNt7VjOcO7BTmFn/Z8r92mSAfHXpb07YJWJosQOQARAQABtDBLaWVyYW4gQmlu\n\tZ2hhbSA8a2llcmFuLmJpbmdoYW1AaWRlYXNvbmJvYXJkLmNvbT6JAkAEEwEKACoCGwMFCwkI\n\tBwIGFQgJCgsCBBYCAwECHgECF4ACGQEFAlnDk/gFCQeA/YsACgkQoR5GchCkYf3X5w/9EaZ7\n\tcnUcT6dxjxrcmmMnfFPoQA1iQXr/MXQJBjFWfxRUWYzjvUJb2D/FpA8FY7y+vksoJP7pWDL7\n\tQTbksdwzagUEk7CU45iLWL/CZ/knYhj1I/+5LSLFmvZ/5Gf5xn2ZCsmg7C0MdW/GbJ8IjWA8\n\t/LKJSEYH8tefoiG6+9xSNp1p0Gesu3vhje/GdGX4wDsfAxx1rIYDYVoX4bDM+uBUQh7sQox/\n\tR1bS0AaVJzPNcjeC14MS226mQRUaUPc9250aj44WmDfcg44/kMsoLFEmQo2II9aOlxUDJ+x1\n\txohGbh9mgBoVawMO3RMBihcEjo/8ytW6v7xSF+xP4Oc+HOn7qebAkxhSWcRxQVaQYw3S9iZz\n\t2iA09AXAkbvPKuMSXi4uau5daXStfBnmOfalG0j+9Y6hOFjz5j0XzaoF6Pln0jisDtWltYhP\n\tX9LjFVhhLkTzPZB/xOeWGmsG4gv2V2ExbU3uAmb7t1VSD9+IO3Km4FtnYOKBWlxwEd8qOFpS\n\tjEqMXURKOiJvnw3OXe9MqG19XdeENA1KyhK5rqjpwdvPGfSn2V+SlsdJA0DFsobUScD9qXQw\n\tOvhapHe3XboK2+Rd7L+g/9Ud7ZKLQHAsMBXOVJbufA1AT+IaOt0ugMcFkAR5UbBg5+dZUYJj\n\t1QbPQcGmM3wfvuaWV5+SlJ+WeKIb8ta5Ag0EVgT9ZgEQAM4o5G/kmruIQJ3K9SYzmPishRHV\n\tDcUcvoakyXSX2mIoccmo9BHtD9MxIt+QmxOpYFNFM7YofX4lG0ld8H7FqoNVLd/+a0yru5Cx\n\tadeZBe3qr1eLns10Q90LuMo7/6zJhCW2w+HE7xgmCHejAwuNe3+7yt4QmwlSGUqdxl8cgtS1\n\tPlEK93xXDsgsJj/bw1EfSVdAUqhx8UQ3aVFxNug5OpoX9FdWJLKROUrfNeBE16RLrNrq2ROc\n\tiSFETpVjyC/oZtzRFnwD9Or7EFMi76/xrWzk+/b15RJ9WrpXGMrttHUUcYZEOoiC2lEXMSAF\n\tSSSj4vHbKDJ0vKQdEFtdgB1roqzxdIOg4rlHz5qwOTynueiBpaZI3PHDudZSMR5Fk6QjFooE\n\tXTw3sSl/km/lvUFiv9CYyHOLdygWohvDuMkV/Jpdkfq8XwFSjOle+vT/4VqERnYFDIGBxaRx\n\tkoBLfNDiiuR3lD8tnJ4A1F88K6ojOUs+jndKsOaQpDZV6iNFv8IaNIklTPvPkZsmNDhJMRHH\n\tIu60S7BpzNeQeT4yyY4dX9lC2JL/LOEpw8DGf5BNOP1KgjCvyp1/KcFxDAo89IeqljaRsCdP\n\t7WCIECWYem6pLwaw6IAL7oX+tEqIMPph/G/jwZcdS6Hkyt/esHPuHNwX4guqTbVEuRqbDzDI\n\t2DJO5FbxABEBAAGJAiUEGAEKAA8CGwwFAlnDlGsFCQeA/gIACgkQoR5GchCkYf1yYRAAq+Yo\n\tnbf9DGdK1kTAm2RTFg+w9oOp2Xjqfhds2PAhFFvrHQg1XfQR/UF/SjeUmaOmLSczM0s6XMeO\n\tVcE77UFtJ/+hLo4PRFKm5X1Pcar6g5m4xGqa+Xfzi9tRkwC29KMCoQOag1BhHChgqYaUH3yo\n\tUzaPwT/fY75iVI+yD0ih/e6j8qYvP8pvGwMQfrmN9YB0zB39YzCSdaUaNrWGD3iCBxg6lwSO\n\tLKeRhxxfiXCIYEf3vwOsP3YMx2JkD5doseXmWBGW1U0T/oJF+DVfKB6mv5UfsTzpVhJRgee7\n\t4jkjqFq4qsUGxcvF2xtRkfHFpZDbRgRlVmiWkqDkT4qMA+4q1y/dWwshSKi/uwVZNycuLsz+\n\t+OD8xPNCsMTqeUkAKfbD8xW4LCay3r/dD2ckoxRxtMD9eOAyu5wYzo/ydIPTh1QEj9SYyvp8\n\tO0g6CpxEwyHUQtF5oh15O018z3ZLztFJKR3RD42VKVsrnNDKnoY0f4U0z7eJv2NeF8xHMuiU\n\tRCIzqxX1GVYaNkKTnb/Qja8hnYnkUzY1Lc+OtwiGmXTwYsPZjjAaDX35J/RSKAoy5wGo/YFA\n\tJxB1gWThL4kOTbsqqXj9GLcyOImkW0lJGGR3o/fV91Zh63S5TKnf2YGGGzxki+ADdxVQAm+Q\n\tsbsRB8KNNvVXBOVNwko86rQqF9drZuw=","Organization":"Ideas on Board","Message-ID":"<433ee512-2a14-03aa-b55f-f08845eacd62@ideasonboard.com>","Date":"Fri, 28 Jun 2019 11:17:10 +0100","User-Agent":"Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101\n\tThunderbird/60.7.0","MIME-Version":"1.0","In-Reply-To":"<20190627020955.6166-3-niklas.soderlund@ragnatech.se>","Content-Type":"text/plain; charset=utf-8","Content-Language":"en-GB","Content-Transfer-Encoding":"8bit","Subject":"Re: [libcamera-devel] [PATCH 2/2] test: ipc: unix: Add test for\n\tIPCUnixSocket","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":"Fri, 28 Jun 2019 10:17:13 -0000"}},{"id":2049,"web_url":"https://patchwork.libcamera.org/comment/2049/","msgid":"<20190628105314.GO32581@bigcity.dyn.berto.se>","date":"2019-06-28T10:53:14","subject":"Re: [libcamera-devel] [PATCH 2/2] test: ipc: unix: Add test for\n\tIPCUnixSocket","submitter":{"id":5,"url":"https://patchwork.libcamera.org/api/people/5/","name":"Niklas Söderlund","email":"niklas.soderlund@ragnatech.se"},"content":"Hi Kieran,\n\nThanks for your feedback,\n\nOn 2019-06-28 11:17:10 +0100, Kieran Bingham wrote:\n> Hi Niklas,\n> \n> On 27/06/2019 03:09, Niklas Söderlund wrote:\n> > Test that the IPC supports sending data and file descriptors over the\n> > IPC medium. To be able execute the test two parts are needed, one\n> > to drive the test and act as the libcamera (master) and a one to act as\n> > the IPA (slave).\n> > \n> > The master drives the testing posting requests to the slave to process\n> > and sometime respond to. A few different tests are preformed.\n> > \n> > - Master sends an array to the slave which responds with a reversed copy\n> >   of the array. The master verifies that a reversed array is returned.\n> > \n> > - Master sends a list of file descriptors and ask the salve to calculate\n> >   and respond with the sum of the size of the files. The master verifies\n> >   that the calculate size is correct.\n> > \n> > - Master send a pre-computed size and a list of file descriptors and\n> >   ask the slave to verify that the pre-computed size matches the sum of\n> >   the size of the file descriptors.\n> > \n> > Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> > ---\n> >  test/ipc/meson.build    |  12 ++\n> >  test/ipc/unixsocket.cpp | 339 ++++++++++++++++++++++++++++++++++++++++\n> >  test/meson.build        |   1 +\n> >  3 files changed, 352 insertions(+)\n> >  create mode 100644 test/ipc/meson.build\n> >  create mode 100644 test/ipc/unixsocket.cpp\n> > \n> > diff --git a/test/ipc/meson.build b/test/ipc/meson.build\n> > new file mode 100644\n> > index 0000000000000000..ca8375f35df91731\n> > --- /dev/null\n> > +++ b/test/ipc/meson.build\n> > @@ -0,0 +1,12 @@\n> > +ipc_tests = [\n> > +    [ 'unixsocket',  'unixsocket.cpp' ],\n> > +]\n> > +\n> > +foreach t : ipc_tests\n> > +    exe = executable(t[0], t[1],\n> > +                     dependencies : libcamera_dep,\n> > +                     link_with : test_libraries,\n> > +                     include_directories : test_includes_internal)\n> > +\n> > +    test(t[0], exe, suite : 'ipc', is_parallel : false)\n> > +endforeach\n> > diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> > new file mode 100644\n> > index 0000000000000000..c50d7463cee48e75\n> > --- /dev/null\n> > +++ b/test/ipc/unixsocket.cpp\n> > @@ -0,0 +1,339 @@\n> > +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> > +/*\n> > + * Copyright (C) 2019, Google Inc.\n> > + *\n> > + * unixsocket.cpp - Unix socket IPC test\n> > + */\n> > +\n> > +#include <algorithm>\n> > +#include <atomic>\n> > +#include <fcntl.h>\n> > +#include <iostream>\n> > +#include <string.h>\n> > +#include <sys/wait.h>\n> > +#include <unistd.h>\n> > +\n> > +#include <libcamera/camera_manager.h>\n> > +#include <libcamera/event_dispatcher.h>\n> > +\n> > +#include \"ipc_unixsocket.h\"\n> > +#include \"test.h\"\n> > +\n> > +#define CMD_CLOSE 0\n> > +#define CMD_REVERSE 1\n> > +#define CMD_LEN_CALC 2\n> > +#define CMD_LEN_CMP 3\n> > +\n> > +using namespace std;\n> > +using namespace libcamera;\n> > +\n> > +int calcLength(int fd)\n> > +{\n> > +\tlseek(fd, 0, 0);\n> > +\tint size = lseek(fd, 0, SEEK_END);\n> > +\tlseek(fd, 0, 0);\n> > +\n> > +\treturn size;\n> > +}\n> > +\n> > +class UnixSocketTestSlave\n> > +{\n> > +public:\n> > +\tUnixSocketTestSlave()\n> > +\t\t: exitCode_(EXIT_FAILURE)\n> > +\t{\n> > +\t\tdispatcher_ = CameraManager::instance()->eventDispatcher();\n> > +\t\texit_.store(false, std::memory_order_release);\n> > +\t\tipc_.payloadReceived.connect(this, &UnixSocketTestSlave::payloadReceived);\n> > +\t}\n> > +\n> > +\tint run(int fd)\n> > +\t{\n> > +\t\tif (ipc_.attach(fd)) {\n> > +\t\t\tcerr << \"Failed to connect to IPC\" << endl;\n> > +\t\t\treturn EXIT_FAILURE;\n> > +\t\t}\n> > +\n> > +\t\twhile (!exit_.load(std::memory_order_acquire))\n> > +\t\t\tdispatcher_->processEvents();\n> > +\n> > +\t\tipc_.close();\n> > +\n> > +\t\treturn exitCode_;\n> > +\t}\n> > +\n> > +private:\n> > +\tvoid payloadReceived(const IPCUnixSocket::Payload &payload)\n> > +\t{\n> > +\t\tIPCUnixSocket::Payload response;\n> > +\t\tint ret;\n> > +\n> > +\t\tconst uint8_t cmd = payload.data.front();\n> > +\n> > +\t\tcout << \"Slave received command \" << static_cast<unsigned int>(cmd) << endl;\n> > +\n> > +\t\tswitch (cmd) {\n> > +\t\tcase CMD_CLOSE:\n> > +\t\t\tstop(0);\n> > +\t\t\tbreak;\n> > +\t\tcase CMD_REVERSE: {\n> > +\t\t\tstd::vector<uint8_t> raw(payload.data.begin() + 1, payload.data.end());\n> > +\t\t\tstd::reverse(raw.begin(), raw.end());\n> > +\t\t\traw.insert(raw.begin(), cmd);\n> > +\n> > +\t\t\tresponse.data = raw;\n> > +\t\t\tret = ipc_.send(response);\n> > +\t\t\tif (ret < 0) {\n> > +\t\t\t\tcerr << \"Reverse fail\" << endl;\n> > +\t\t\t\tstop(ret);\n> > +\t\t\t}\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t\tcase CMD_LEN_CALC: {\n> > +\t\t\tint size = 0;\n> > +\t\t\tfor (int fd : payload.fds)\n> > +\t\t\t\tsize += calcLength(fd);\n> > +\n> > +\t\t\tresponse.data.resize(1 + sizeof(size));\n> > +\t\t\tresponse.data[0] = cmd;\n> > +\t\t\tmemcpy(response.data.data() + 1, &size, sizeof(size));\n> > +\n> > +\t\t\tret = ipc_.send(response);\n> > +\t\t\tif (ret < 0) {\n> > +\t\t\t\tcerr << \"Calc fail\" << endl;\n> > +\t\t\t\tstop(ret);\n> > +\t\t\t}\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t\tcase CMD_LEN_CMP: {\n> > +\t\t\tint size = 0;\n> > +\t\t\tfor (int fd : payload.fds)\n> > +\t\t\t\tsize += calcLength(fd);\n> > +\n> > +\t\t\tint cmp;\n> > +\t\t\tmemcpy(&cmp, payload.data.data() + 1, sizeof(cmp));\n> > +\n> > +\t\t\tif (cmp != size) {\n> > +\t\t\t\tcerr << \"Compare fail\" << endl;\n> > +\t\t\t\tstop(-ERANGE);\n> > +\t\t\t}\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t\tdefault:\n> > +\t\t\tcerr << \"Unknown command \" << cmd << endl;\n> > +\t\t\tstop(-EINVAL);\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tvoid stop(int code)\n> > +\t{\n> > +\t\texitCode_ = code;\n> > +\t\texit_.store(true, std::memory_order_release);\n> > +\t}\n> > +\n> > +\tIPCUnixSocket ipc_;\n> > +\tEventDispatcher *dispatcher_;\n> > +\tint exitCode_;\n> > +\tstd::atomic<bool> exit_;\n> > +};\n> > +\n> > +static const std::vector<std::string> testPaths = {\n> > +\t\"/proc/self/exe\",\n> > +\t\"/proc/self/exe\",\n> > +};\n> \n> This looks odd - is an array of two identical strings really the best\n> way to make something loop twice? (I think that's the only use of this?)\n\nI agree it looks odd, but it does the job :-) All that is needed is two \nfile handles (or more) to test that multiple FDs can be transferred.  \nOnce could do with one FD but I feel its better to test with more.\n\n> \n> Perhaps should one of those instances of /proc/self/exe be another file\n> in /proc/self ? comm? maps? As far as I understand it-  it just has to\n> be constant when read twice right ?\n\nI tried finding another file in /proc/self that is a regular file and \nthus have a size. I only managed to find exe and map_files/* where the \nlater provides no stable naming.\n\nI'm open to using a different path here which would be found on every \nsystem, but not sure what that would be, /etc/issue maybe? I'm OK \nkeeping /proc/self/exe multiple times as it does the job and at the same \ntime I'm also open to suggestions of different stable paths to replace \none of them with.\n\n> \n> > +\n> > +class UnixSocketTest : public Test\n> > +{\n> > +protected:\n> > +\tint slaveStart(int fd)\n> > +\t{\n> > +\t\tpid_ = fork();\n> > +\n> > +\t\tif (pid_ == -1)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (!pid_) {\n> > +\t\t\tstd::string arg = std::to_string(fd);\n> > +\t\t\texecl(\"/proc/self/exe\", \"/proc/self/exe\",\n> > +\t\t\t      arg.c_str(), nullptr);\n> > +\n> > +\t\t\t/* Only get here if exec fails. */\n> > +\t\t\texit(TestFail);\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tint slaveStop()\n> > +\t{\n> > +\t\tint status;\n> > +\n> > +\t\tif (pid_ < 0)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (waitpid(pid_, &status, 0) < 0)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (!WIFEXITED(status) || WEXITSTATUS(status))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +\tint testReverse()\n> > +\t{\n> > +\t\tIPCUnixSocket::Payload reverse;\n> > +\n> > +\t\treverse.data = { CMD_REVERSE, 1, 2, 3, 4, 5 };\n> > +\t\tif (ipc_.send(reverse))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tCameraManager::instance()->eventDispatcher()->processEvents();\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tint testCalc()\n> > +\t{\n> > +\t\tIPCUnixSocket::Payload payload;\n> > +\n> > +\t\tcalcSize_ = 0;\n> > +\t\tfor (const std::string path : testPaths) {\n> > +\t\t\tint fd = open(path.c_str(), O_RDONLY);\n> > +\t\t\tif (fd < 0)\n> > +\t\t\t\treturn TestFail;\n> > +\n> > +\t\t\tcalcSize_ += calcLength(fd);\n> > +\t\t\tpayload.fds.push_back(fd);\n> > +\t\t}\n> > +\n> > +\t\tpayload.data.push_back(CMD_LEN_CALC);\n> > +\t\tif (ipc_.send(payload))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tCameraManager::instance()->eventDispatcher()->processEvents();\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tint testCmp()\n> > +\t{\n> > +\t\tIPCUnixSocket::Payload payload;\n> > +\n> > +\t\tint size = 0;\n> > +\t\tfor (const std::string path : testPaths) {\n> > +\t\t\tint fd = open(path.c_str(), O_RDONLY);\n> > +\t\t\tif (fd < 0)\n> > +\t\t\t\treturn TestFail;\n> > +\n> > +\t\t\tsize += calcLength(fd);\n> > +\t\t\tpayload.fds.push_back(fd);\n> > +\t\t}\n> > +\n> > +\t\tpayload.data.resize(1 + sizeof(size));\n> > +\t\tpayload.data[0] = CMD_LEN_CMP;\n> > +\t\tmemcpy(payload.data.data() + 1, &size, sizeof(size));\n> > +\n> > +\t\tif (ipc_.send(payload))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\treturn 0;\n> > +\t}\n> > +\n> > +\tint run()\n> > +\t{\n> > +\t\tint slavefd = ipc_.create();\n> > +\t\tif (slavefd < 0)\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tif (slaveStart(slavefd))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tipc_.payloadReceived.connect(this, &UnixSocketTest::payloadReceived);\n> > +\n> > +\t\t/* Test reversing a string, this test sending only data. */\n> > +\t\tranRev_ = false;\n> > +\t\tif (testReverse())\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/* Test offloading a calculation, this test sending only FDs. */\n> > +\t\tranCalc_ = false;\n> > +\t\tif (testCalc())\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/* Test fire and forget, this tests sending data and FDs. */\n> > +\t\tif (testCmp())\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/* Close slave connection. */\n> > +\t\tIPCUnixSocket::Payload close;\n> > +\t\tclose.data.push_back(CMD_CLOSE);\n> > +\t\tif (ipc_.send(close))\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\tipc_.close();\n> > +\t\tif (slaveStop())\n> > +\t\t\treturn TestFail;\n> > +\n> > +\t\t/* Check that all tests have executed. */\n> > +\t\tif (!ranRev_ || !ranCalc_) {\n> > +\t\t\tcerr << \"Not all messages responded to\" << endl;\n> > +\t\t\treturn TestFail;\n> > +\t\t}\n> > +\n> > +\t\treturn TestPass;\n> > +\t}\n> > +\n> > +private:\n> > +\tvoid payloadReceived(const IPCUnixSocket::Payload &payload)\n> > +\t{\n> > +\t\tconst uint8_t cmd = payload.data.front();\n> > +\t\tcout << \"Master received command \" << static_cast<unsigned int>(cmd) << endl;\n> > +\n> > +\t\tswitch (cmd) {\n> > +\t\tcase CMD_REVERSE: {\n> > +\t\t\tstd::vector<uint8_t> raw(payload.data.begin() + 1, payload.data.end());\n> > +\t\t\tif (raw == std::vector<uint8_t>{ 5, 4, 3, 2, 1 })\n> > +\t\t\t\tranRev_ = true;\n> > +\t\t\telse\n> > +\t\t\t\tcerr << \"Reverse response invalid\" << endl;\n> > +\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t\tcase CMD_LEN_CALC: {\n> > +\t\t\tint output;\n> > +\t\t\tmemcpy(&output, payload.data.data() + 1, sizeof(output));\n> > +\t\t\tif (output == calcSize_)\n> > +\t\t\t\tranCalc_ = true;\n> > +\t\t\telse\n> > +\t\t\t\tcerr << \"Calc response invalid\" << endl;\n> > +\n> > +\t\t\tbreak;\n> > +\t\t}\n> > +\t\tdefault:\n> > +\t\t\tcerr << \"Unknown command \" << cmd << endl;\n> > +\t\t}\n> > +\t}\n> > +\n> > +\tpid_t pid_;\n> > +\tIPCUnixSocket ipc_;\n> > +\n> > +\tbool ranRev_;\n> > +\tbool ranCalc_;\n> > +\tint calcSize_;\n> > +};\n> > +\n> > +/*\n> > + * Can't use TEST_REGISTER() as single binary needs to act as both proxy\n> > + * master and slave.\n> > + */\n> > +int main(int argc, char **argv)\n> > +{\n> > +\tif (argc == 2) {\n> > +\t\tint ipcfd = std::stoi(argv[1]);\n> > +\t\tUnixSocketTestSlave slave;\n> > +\t\treturn slave.run(ipcfd);\n> > +\t}\n> > +\n> > +\treturn UnixSocketTest().execute();\n> > +}\n> > diff --git a/test/meson.build b/test/meson.build\n> > index c36ac24796367501..3666f6b2385bd4ca 100644\n> > --- a/test/meson.build\n> > +++ b/test/meson.build\n> > @@ -2,6 +2,7 @@ subdir('libtest')\n> >  \n> >  subdir('camera')\n> >  subdir('ipa')\n> > +subdir('ipc')\n> >  subdir('media_device')\n> >  subdir('pipeline')\n> >  subdir('stream')\n> > \n> \n> -- \n> Regards\n> --\n> Kieran","headers":{"Return-Path":"<niklas.soderlund@ragnatech.se>","Received":["from mail-lj1-x242.google.com (mail-lj1-x242.google.com\n\t[IPv6:2a00:1450:4864:20::242])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id 3BE2360BC7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 28 Jun 2019 12:53:17 +0200 (CEST)","by mail-lj1-x242.google.com with SMTP id 205so5519670ljj.8\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 28 Jun 2019 03:53:17 -0700 (PDT)","from localhost (customer-145-14-112-32.stosn.net. [145.14.112.32])\n\tby smtp.gmail.com with ESMTPSA id\n\tx22sm494786lfq.20.2019.06.28.03.53.15\n\t(version=TLS1_3 cipher=AEAD-AES256-GCM-SHA384 bits=256/256);\n\tFri, 28 Jun 2019 03:53:15 -0700 (PDT)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=ragnatech-se.20150623.gappssmtp.com; s=20150623;\n\th=date:from:to:cc:subject:message-id:references:mime-version\n\t:content-disposition:content-transfer-encoding:in-reply-to\n\t:user-agent; bh=EiPdL3xObrby+go2sEWgkpCotsmNNVoTJknUM9Lw1f0=;\n\tb=hnZ8FU0V5Dzo/kecvqERrl/CWrcaeXkc6T6cI+AAGc1oBS28fydvTkWvo3PMONg3/Y\n\t9bmFVRadZja3LedNETPfdMWr9LpY4yMNiSRPE+qqH6/0ZJejEbunyiKhcBv9MJzmN97v\n\tNfP5eMtR+4a3XBDol+C6eKz3ow4s3gtiF/oSeIPZfJSTmZu6hNunO8sB761u1DfZMeQ2\n\tRvmK7WagFosRfEalmqTuoq8BVkEVTBR2YxnRSsit1+VJhT1d91oT0nrmFDcYUeBzyW3d\n\tNvm/rgkGmh/fEkzRDcj036h7smqrIl6RW16oLTVvBPb7YIiOkbltcKAFfeMIb2ppMWwQ\n\ttxHQ==","X-Google-DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/relaxed;\n\td=1e100.net; s=20161025;\n\th=x-gm-message-state:date:from:to:cc:subject:message-id:references\n\t:mime-version:content-disposition:content-transfer-encoding\n\t:in-reply-to:user-agent;\n\tbh=EiPdL3xObrby+go2sEWgkpCotsmNNVoTJknUM9Lw1f0=;\n\tb=D6QjrhinNjn3LQlVJ1Jy9Bdl5FuFU6MNazedfTjMgSb8yxXGde3ZX1RSF0mHHQ6oUN\n\tBn29wEdagZ7S0eaeEm5poKhQDY6gMxXT6JRgobqrqduSLeJa5Mx5JAVNXDBDiy1jAFCF\n\t0ce3fSEZHkcx8kus4Nwbwv3dsGZNbTMicfsqBrGiuwxL2IfyFAT2rZE+HdIR78t56O3Y\n\th9UGLQfyRq3R3HH8rZ0NeaonK8bct+MM1zpNW9fS2PZRUlQx91aNsDnqMChxneFVmOJQ\n\td9K9183rzUoT9OcV+i3zvoeJBQNmDW2CLVQ19O7ISXHYf6WsXLdoQDEuU8BJ8WSvms1K\n\t2HIg==","X-Gm-Message-State":"APjAAAWgcjWDiZrD43weGiNYsOyzfDrXNXOTk75OKw6TNWyoAgVGcTqO\n\tqwHw6FatdHoZ/m8D5ORNnsD3Kw==","X-Google-Smtp-Source":"APXvYqwa5Pm8xOkbcHqyRCFa53A7PKdUNyvuRDYi2ns6d9u0YXWp4y3yeCNnq+sdWKYhZHhXkQclaQ==","X-Received":"by 2002:a2e:9857:: with SMTP id\n\te23mr5602952ljj.217.1561719195985; \n\tFri, 28 Jun 2019 03:53:15 -0700 (PDT)","Date":"Fri, 28 Jun 2019 12:53:14 +0200","From":"Niklas =?iso-8859-1?q?S=F6derlund?= <niklas.soderlund@ragnatech.se>","To":"Kieran Bingham <kieran.bingham@ideasonboard.com>","Cc":"libcamera-devel@lists.libcamera.org","Message-ID":"<20190628105314.GO32581@bigcity.dyn.berto.se>","References":"<20190627020955.6166-1-niklas.soderlund@ragnatech.se>\n\t<20190627020955.6166-3-niklas.soderlund@ragnatech.se>\n\t<433ee512-2a14-03aa-b55f-f08845eacd62@ideasonboard.com>","MIME-Version":"1.0","Content-Type":"text/plain; charset=iso-8859-1","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<433ee512-2a14-03aa-b55f-f08845eacd62@ideasonboard.com>","User-Agent":"Mutt/1.12.1 (2019-06-15)","Subject":"Re: [libcamera-devel] [PATCH 2/2] test: ipc: unix: Add test for\n\tIPCUnixSocket","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":"Fri, 28 Jun 2019 10:53:17 -0000"}},{"id":2050,"web_url":"https://patchwork.libcamera.org/comment/2050/","msgid":"<20190628110726.GB4779@pendragon.ideasonboard.com>","date":"2019-06-28T11:07:26","subject":"Re: [libcamera-devel] [PATCH 2/2] test: ipc: unix: Add test for\n\tIPCUnixSocket","submitter":{"id":2,"url":"https://patchwork.libcamera.org/api/people/2/","name":"Laurent Pinchart","email":"laurent.pinchart@ideasonboard.com"},"content":"Hi Niklas,\n\nOn Fri, Jun 28, 2019 at 12:53:14PM +0200, Niklas Söderlund wrote:\n> On 2019-06-28 11:17:10 +0100, Kieran Bingham wrote:\n> > On 27/06/2019 03:09, Niklas Söderlund wrote:\n> >> Test that the IPC supports sending data and file descriptors over the\n> >> IPC medium. To be able execute the test two parts are needed, one\n> >> to drive the test and act as the libcamera (master) and a one to act as\n> >> the IPA (slave).\n> >> \n> >> The master drives the testing posting requests to the slave to process\n> >> and sometime respond to. A few different tests are preformed.\n> >> \n> >> - Master sends an array to the slave which responds with a reversed copy\n> >>   of the array. The master verifies that a reversed array is returned.\n> >> \n> >> - Master sends a list of file descriptors and ask the salve to calculate\n> >>   and respond with the sum of the size of the files. The master verifies\n> >>   that the calculate size is correct.\n> >> \n> >> - Master send a pre-computed size and a list of file descriptors and\n> >>   ask the slave to verify that the pre-computed size matches the sum of\n> >>   the size of the file descriptors.\n> >> \n> >> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> >> ---\n> >>  test/ipc/meson.build    |  12 ++\n> >>  test/ipc/unixsocket.cpp | 339 ++++++++++++++++++++++++++++++++++++++++\n> >>  test/meson.build        |   1 +\n> >>  3 files changed, 352 insertions(+)\n> >>  create mode 100644 test/ipc/meson.build\n> >>  create mode 100644 test/ipc/unixsocket.cpp\n> >> \n> >> diff --git a/test/ipc/meson.build b/test/ipc/meson.build\n> >> new file mode 100644\n> >> index 0000000000000000..ca8375f35df91731\n> >> --- /dev/null\n> >> +++ b/test/ipc/meson.build\n> >> @@ -0,0 +1,12 @@\n> >> +ipc_tests = [\n> >> +    [ 'unixsocket',  'unixsocket.cpp' ],\n> >> +]\n> >> +\n> >> +foreach t : ipc_tests\n> >> +    exe = executable(t[0], t[1],\n> >> +                     dependencies : libcamera_dep,\n> >> +                     link_with : test_libraries,\n> >> +                     include_directories : test_includes_internal)\n> >> +\n> >> +    test(t[0], exe, suite : 'ipc', is_parallel : false)\n> >> +endforeach\n> >> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> >> new file mode 100644\n> >> index 0000000000000000..c50d7463cee48e75\n> >> --- /dev/null\n> >> +++ b/test/ipc/unixsocket.cpp\n> >> @@ -0,0 +1,339 @@\n> >> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> >> +/*\n> >> + * Copyright (C) 2019, Google Inc.\n> >> + *\n> >> + * unixsocket.cpp - Unix socket IPC test\n> >> + */\n> >> +\n> >> +#include <algorithm>\n> >> +#include <atomic>\n> >> +#include <fcntl.h>\n> >> +#include <iostream>\n> >> +#include <string.h>\n> >> +#include <sys/wait.h>\n> >> +#include <unistd.h>\n> >> +\n> >> +#include <libcamera/camera_manager.h>\n> >> +#include <libcamera/event_dispatcher.h>\n> >> +\n> >> +#include \"ipc_unixsocket.h\"\n> >> +#include \"test.h\"\n> >> +\n> >> +#define CMD_CLOSE 0\n> >> +#define CMD_REVERSE 1\n> >> +#define CMD_LEN_CALC 2\n> >> +#define CMD_LEN_CMP 3\n> >> +\n> >> +using namespace std;\n> >> +using namespace libcamera;\n> >> +\n> >> +int calcLength(int fd)\n> >> +{\n> >> +\tlseek(fd, 0, 0);\n> >> +\tint size = lseek(fd, 0, SEEK_END);\n> >> +\tlseek(fd, 0, 0);\n> >> +\n> >> +\treturn size;\n> >> +}\n> >> +\n> >> +class UnixSocketTestSlave\n> >> +{\n> >> +public:\n> >> +\tUnixSocketTestSlave()\n> >> +\t\t: exitCode_(EXIT_FAILURE)\n> >> +\t{\n> >> +\t\tdispatcher_ = CameraManager::instance()->eventDispatcher();\n> >> +\t\texit_.store(false, std::memory_order_release);\n> >> +\t\tipc_.payloadReceived.connect(this, &UnixSocketTestSlave::payloadReceived);\n> >> +\t}\n> >> +\n> >> +\tint run(int fd)\n> >> +\t{\n> >> +\t\tif (ipc_.attach(fd)) {\n> >> +\t\t\tcerr << \"Failed to connect to IPC\" << endl;\n> >> +\t\t\treturn EXIT_FAILURE;\n> >> +\t\t}\n> >> +\n> >> +\t\twhile (!exit_.load(std::memory_order_acquire))\n> >> +\t\t\tdispatcher_->processEvents();\n> >> +\n> >> +\t\tipc_.close();\n> >> +\n> >> +\t\treturn exitCode_;\n> >> +\t}\n> >> +\n> >> +private:\n> >> +\tvoid payloadReceived(const IPCUnixSocket::Payload &payload)\n> >> +\t{\n> >> +\t\tIPCUnixSocket::Payload response;\n> >> +\t\tint ret;\n> >> +\n> >> +\t\tconst uint8_t cmd = payload.data.front();\n> >> +\n> >> +\t\tcout << \"Slave received command \" << static_cast<unsigned int>(cmd) << endl;\n> >> +\n> >> +\t\tswitch (cmd) {\n> >> +\t\tcase CMD_CLOSE:\n> >> +\t\t\tstop(0);\n> >> +\t\t\tbreak;\n> >> +\t\tcase CMD_REVERSE: {\n> >> +\t\t\tstd::vector<uint8_t> raw(payload.data.begin() + 1, payload.data.end());\n> >> +\t\t\tstd::reverse(raw.begin(), raw.end());\n> >> +\t\t\traw.insert(raw.begin(), cmd);\n> >> +\n> >> +\t\t\tresponse.data = raw;\n> >> +\t\t\tret = ipc_.send(response);\n> >> +\t\t\tif (ret < 0) {\n> >> +\t\t\t\tcerr << \"Reverse fail\" << endl;\n> >> +\t\t\t\tstop(ret);\n> >> +\t\t\t}\n> >> +\t\t\tbreak;\n> >> +\t\t}\n> >> +\t\tcase CMD_LEN_CALC: {\n> >> +\t\t\tint size = 0;\n> >> +\t\t\tfor (int fd : payload.fds)\n> >> +\t\t\t\tsize += calcLength(fd);\n> >> +\n> >> +\t\t\tresponse.data.resize(1 + sizeof(size));\n> >> +\t\t\tresponse.data[0] = cmd;\n> >> +\t\t\tmemcpy(response.data.data() + 1, &size, sizeof(size));\n> >> +\n> >> +\t\t\tret = ipc_.send(response);\n> >> +\t\t\tif (ret < 0) {\n> >> +\t\t\t\tcerr << \"Calc fail\" << endl;\n> >> +\t\t\t\tstop(ret);\n> >> +\t\t\t}\n> >> +\t\t\tbreak;\n> >> +\t\t}\n> >> +\t\tcase CMD_LEN_CMP: {\n> >> +\t\t\tint size = 0;\n> >> +\t\t\tfor (int fd : payload.fds)\n> >> +\t\t\t\tsize += calcLength(fd);\n> >> +\n> >> +\t\t\tint cmp;\n> >> +\t\t\tmemcpy(&cmp, payload.data.data() + 1, sizeof(cmp));\n> >> +\n> >> +\t\t\tif (cmp != size) {\n> >> +\t\t\t\tcerr << \"Compare fail\" << endl;\n> >> +\t\t\t\tstop(-ERANGE);\n> >> +\t\t\t}\n> >> +\t\t\tbreak;\n> >> +\t\t}\n> >> +\t\tdefault:\n> >> +\t\t\tcerr << \"Unknown command \" << cmd << endl;\n> >> +\t\t\tstop(-EINVAL);\n> >> +\t\t}\n> >> +\t}\n> >> +\n> >> +\tvoid stop(int code)\n> >> +\t{\n> >> +\t\texitCode_ = code;\n> >> +\t\texit_.store(true, std::memory_order_release);\n> >> +\t}\n> >> +\n> >> +\tIPCUnixSocket ipc_;\n> >> +\tEventDispatcher *dispatcher_;\n> >> +\tint exitCode_;\n> >> +\tstd::atomic<bool> exit_;\n> >> +};\n> >> +\n> >> +static const std::vector<std::string> testPaths = {\n> >> +\t\"/proc/self/exe\",\n> >> +\t\"/proc/self/exe\",\n> >> +};\n> > \n> > This looks odd - is an array of two identical strings really the best\n> > way to make something loop twice? (I think that's the only use of this?)\n> \n> I agree it looks odd, but it does the job :-) All that is needed is two \n> file handles (or more) to test that multiple FDs can be transferred.  \n> Once could do with one FD but I feel its better to test with more.\n> \n> > \n> > Perhaps should one of those instances of /proc/self/exe be another file\n> > in /proc/self ? comm? maps? As far as I understand it-  it just has to\n> > be constant when read twice right ?\n> \n> I tried finding another file in /proc/self that is a regular file and \n> thus have a size. I only managed to find exe and map_files/* where the \n> later provides no stable naming.\n> \n> I'm open to using a different path here which would be found on every \n> system, but not sure what that would be, /etc/issue maybe? I'm OK \n> keeping /proc/self/exe multiple times as it does the job and at the same \n> time I'm also open to suggestions of different stable paths to replace \n> one of them with.\n\nmkstemp() ? This will have the advantage that you can write test data in\nthe files and read it from the other side. pipe() could also be an\noption if you want two fds.\n\n> >> +\n> >> +class UnixSocketTest : public Test\n> >> +{\n> >> +protected:\n> >> +\tint slaveStart(int fd)\n> >> +\t{\n> >> +\t\tpid_ = fork();\n> >> +\n> >> +\t\tif (pid_ == -1)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (!pid_) {\n> >> +\t\t\tstd::string arg = std::to_string(fd);\n> >> +\t\t\texecl(\"/proc/self/exe\", \"/proc/self/exe\",\n> >> +\t\t\t      arg.c_str(), nullptr);\n> >> +\n> >> +\t\t\t/* Only get here if exec fails. */\n> >> +\t\t\texit(TestFail);\n> >> +\t\t}\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +\tint slaveStop()\n> >> +\t{\n> >> +\t\tint status;\n> >> +\n> >> +\t\tif (pid_ < 0)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (waitpid(pid_, &status, 0) < 0)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (!WIFEXITED(status) || WEXITSTATUS(status))\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +\tint testReverse()\n> >> +\t{\n> >> +\t\tIPCUnixSocket::Payload reverse;\n> >> +\n> >> +\t\treverse.data = { CMD_REVERSE, 1, 2, 3, 4, 5 };\n> >> +\t\tif (ipc_.send(reverse))\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tCameraManager::instance()->eventDispatcher()->processEvents();\n> >> +\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +\n> >> +\tint testCalc()\n> >> +\t{\n> >> +\t\tIPCUnixSocket::Payload payload;\n> >> +\n> >> +\t\tcalcSize_ = 0;\n> >> +\t\tfor (const std::string path : testPaths) {\n> >> +\t\t\tint fd = open(path.c_str(), O_RDONLY);\n> >> +\t\t\tif (fd < 0)\n> >> +\t\t\t\treturn TestFail;\n> >> +\n> >> +\t\t\tcalcSize_ += calcLength(fd);\n> >> +\t\t\tpayload.fds.push_back(fd);\n> >> +\t\t}\n> >> +\n> >> +\t\tpayload.data.push_back(CMD_LEN_CALC);\n> >> +\t\tif (ipc_.send(payload))\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tCameraManager::instance()->eventDispatcher()->processEvents();\n> >> +\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +\n> >> +\tint testCmp()\n> >> +\t{\n> >> +\t\tIPCUnixSocket::Payload payload;\n> >> +\n> >> +\t\tint size = 0;\n> >> +\t\tfor (const std::string path : testPaths) {\n> >> +\t\t\tint fd = open(path.c_str(), O_RDONLY);\n> >> +\t\t\tif (fd < 0)\n> >> +\t\t\t\treturn TestFail;\n> >> +\n> >> +\t\t\tsize += calcLength(fd);\n> >> +\t\t\tpayload.fds.push_back(fd);\n> >> +\t\t}\n> >> +\n> >> +\t\tpayload.data.resize(1 + sizeof(size));\n> >> +\t\tpayload.data[0] = CMD_LEN_CMP;\n> >> +\t\tmemcpy(payload.data.data() + 1, &size, sizeof(size));\n> >> +\n> >> +\t\tif (ipc_.send(payload))\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\treturn 0;\n> >> +\t}\n> >> +\n> >> +\tint run()\n> >> +\t{\n> >> +\t\tint slavefd = ipc_.create();\n> >> +\t\tif (slavefd < 0)\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tif (slaveStart(slavefd))\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tipc_.payloadReceived.connect(this, &UnixSocketTest::payloadReceived);\n> >> +\n> >> +\t\t/* Test reversing a string, this test sending only data. */\n> >> +\t\tranRev_ = false;\n> >> +\t\tif (testReverse())\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\t/* Test offloading a calculation, this test sending only FDs. */\n> >> +\t\tranCalc_ = false;\n> >> +\t\tif (testCalc())\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\t/* Test fire and forget, this tests sending data and FDs. */\n> >> +\t\tif (testCmp())\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\t/* Close slave connection. */\n> >> +\t\tIPCUnixSocket::Payload close;\n> >> +\t\tclose.data.push_back(CMD_CLOSE);\n> >> +\t\tif (ipc_.send(close))\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\tipc_.close();\n> >> +\t\tif (slaveStop())\n> >> +\t\t\treturn TestFail;\n> >> +\n> >> +\t\t/* Check that all tests have executed. */\n> >> +\t\tif (!ranRev_ || !ranCalc_) {\n> >> +\t\t\tcerr << \"Not all messages responded to\" << endl;\n> >> +\t\t\treturn TestFail;\n> >> +\t\t}\n> >> +\n> >> +\t\treturn TestPass;\n> >> +\t}\n> >> +\n> >> +private:\n> >> +\tvoid payloadReceived(const IPCUnixSocket::Payload &payload)\n> >> +\t{\n> >> +\t\tconst uint8_t cmd = payload.data.front();\n> >> +\t\tcout << \"Master received command \" << static_cast<unsigned int>(cmd) << endl;\n> >> +\n> >> +\t\tswitch (cmd) {\n> >> +\t\tcase CMD_REVERSE: {\n> >> +\t\t\tstd::vector<uint8_t> raw(payload.data.begin() + 1, payload.data.end());\n> >> +\t\t\tif (raw == std::vector<uint8_t>{ 5, 4, 3, 2, 1 })\n> >> +\t\t\t\tranRev_ = true;\n> >> +\t\t\telse\n> >> +\t\t\t\tcerr << \"Reverse response invalid\" << endl;\n> >> +\n> >> +\t\t\tbreak;\n> >> +\t\t}\n> >> +\t\tcase CMD_LEN_CALC: {\n> >> +\t\t\tint output;\n> >> +\t\t\tmemcpy(&output, payload.data.data() + 1, sizeof(output));\n> >> +\t\t\tif (output == calcSize_)\n> >> +\t\t\t\tranCalc_ = true;\n> >> +\t\t\telse\n> >> +\t\t\t\tcerr << \"Calc response invalid\" << endl;\n> >> +\n> >> +\t\t\tbreak;\n> >> +\t\t}\n> >> +\t\tdefault:\n> >> +\t\t\tcerr << \"Unknown command \" << cmd << endl;\n> >> +\t\t}\n> >> +\t}\n> >> +\n> >> +\tpid_t pid_;\n> >> +\tIPCUnixSocket ipc_;\n> >> +\n> >> +\tbool ranRev_;\n> >> +\tbool ranCalc_;\n> >> +\tint calcSize_;\n> >> +};\n> >> +\n> >> +/*\n> >> + * Can't use TEST_REGISTER() as single binary needs to act as both proxy\n> >> + * master and slave.\n> >> + */\n> >> +int main(int argc, char **argv)\n> >> +{\n> >> +\tif (argc == 2) {\n> >> +\t\tint ipcfd = std::stoi(argv[1]);\n> >> +\t\tUnixSocketTestSlave slave;\n> >> +\t\treturn slave.run(ipcfd);\n> >> +\t}\n> >> +\n> >> +\treturn UnixSocketTest().execute();\n> >> +}\n> >> diff --git a/test/meson.build b/test/meson.build\n> >> index c36ac24796367501..3666f6b2385bd4ca 100644\n> >> --- a/test/meson.build\n> >> +++ b/test/meson.build\n> >> @@ -2,6 +2,7 @@ subdir('libtest')\n> >>  \n> >>  subdir('camera')\n> >>  subdir('ipa')\n> >> +subdir('ipc')\n> >>  subdir('media_device')\n> >>  subdir('pipeline')\n> >>  subdir('stream')","headers":{"Return-Path":"<laurent.pinchart@ideasonboard.com>","Received":["from perceval.ideasonboard.com (perceval.ideasonboard.com\n\t[213.167.242.64])\n\tby lancelot.ideasonboard.com (Postfix) with ESMTPS id E73D660BC7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 28 Jun 2019 13:07:45 +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 5C6992C6;\n\tFri, 28 Jun 2019 13:07:45 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561720065;\n\tbh=fEuH9EeiF62JYv/Jnq4st1l9T0C2hW3MarwkoXUEnG0=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=btRxw5lZuaNfzAGu9r77XQzkl427gUVl8N1mJDuDE8g3tPGwxXXAKlC0XPhDDYnJD\n\tjKkhHIc2W9DxMoOJ33F1mL5Lq0ZQIfpZlGtmnapzXorJgN9N1ikPVxHqOsxJ2sHeSC\n\tv/XMEGVSbTtOP+okzeBxdo1X9IiBD8m294eLKtGc=","Date":"Fri, 28 Jun 2019 14:07:26 +0300","From":"Laurent Pinchart <laurent.pinchart@ideasonboard.com>","To":"Niklas =?utf-8?q?S=C3=B6derlund?= <niklas.soderlund@ragnatech.se>","Cc":"Kieran Bingham <kieran.bingham@ideasonboard.com>,\n\tlibcamera-devel@lists.libcamera.org","Message-ID":"<20190628110726.GB4779@pendragon.ideasonboard.com>","References":"<20190627020955.6166-1-niklas.soderlund@ragnatech.se>\n\t<20190627020955.6166-3-niklas.soderlund@ragnatech.se>\n\t<433ee512-2a14-03aa-b55f-f08845eacd62@ideasonboard.com>\n\t<20190628105314.GO32581@bigcity.dyn.berto.se>","MIME-Version":"1.0","Content-Type":"text/plain; charset=utf-8","Content-Disposition":"inline","Content-Transfer-Encoding":"8bit","In-Reply-To":"<20190628105314.GO32581@bigcity.dyn.berto.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/2] test: ipc: unix: Add test for\n\tIPCUnixSocket","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":"Fri, 28 Jun 2019 11:07:46 -0000"}},{"id":2052,"web_url":"https://patchwork.libcamera.org/comment/2052/","msgid":"<20190628123410.GE4779@pendragon.ideasonboard.com>","date":"2019-06-28T12:34:10","subject":"Re: [libcamera-devel] [PATCH 2/2] test: ipc: unix: Add test for\n\tIPCUnixSocket","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 Thu, Jun 27, 2019 at 04:09:55AM +0200, Niklas Söderlund wrote:\n> Test that the IPC supports sending data and file descriptors over the\n> IPC medium. To be able execute the test two parts are needed, one\n\ns/To be able/To be able to/\n\n> to drive the test and act as the libcamera (master) and a one to act as\n> the IPA (slave).\n> \n> The master drives the testing posting requests to the slave to process\n> and sometime respond to. A few different tests are preformed.\n\ns/sometime/sometimes/\ns/preformed/performed/\n\n> \n> - Master sends an array to the slave which responds with a reversed copy\n>   of the array. The master verifies that a reversed array is returned.\n> \n> - Master sends a list of file descriptors and ask the salve to calculate\n\ns/salve/slave/\n\n>   and respond with the sum of the size of the files. The master verifies\n>   that the calculate size is correct.\n\ns/calculate/calculated/\n\n> \n> - Master send a pre-computed size and a list of file descriptors and\n\ns/send/sends/\n\n>   ask the slave to verify that the pre-computed size matches the sum of\n>   the size of the file descriptors.\n> \n> Signed-off-by: Niklas Söderlund <niklas.soderlund@ragnatech.se>\n> ---\n>  test/ipc/meson.build    |  12 ++\n>  test/ipc/unixsocket.cpp | 339 ++++++++++++++++++++++++++++++++++++++++\n>  test/meson.build        |   1 +\n>  3 files changed, 352 insertions(+)\n>  create mode 100644 test/ipc/meson.build\n>  create mode 100644 test/ipc/unixsocket.cpp\n> \n> diff --git a/test/ipc/meson.build b/test/ipc/meson.build\n> new file mode 100644\n> index 0000000000000000..ca8375f35df91731\n> --- /dev/null\n> +++ b/test/ipc/meson.build\n> @@ -0,0 +1,12 @@\n> +ipc_tests = [\n> +    [ 'unixsocket',  'unixsocket.cpp' ],\n> +]\n> +\n> +foreach t : ipc_tests\n> +    exe = executable(t[0], t[1],\n> +                     dependencies : libcamera_dep,\n> +                     link_with : test_libraries,\n> +                     include_directories : test_includes_internal)\n> +\n> +    test(t[0], exe, suite : 'ipc', is_parallel : false)\n> +endforeach\n> diff --git a/test/ipc/unixsocket.cpp b/test/ipc/unixsocket.cpp\n> new file mode 100644\n> index 0000000000000000..c50d7463cee48e75\n> --- /dev/null\n> +++ b/test/ipc/unixsocket.cpp\n> @@ -0,0 +1,339 @@\n> +/* SPDX-License-Identifier: GPL-2.0-or-later */\n> +/*\n> + * Copyright (C) 2019, Google Inc.\n> + *\n> + * unixsocket.cpp - Unix socket IPC test\n> + */\n> +\n> +#include <algorithm>\n> +#include <atomic>\n> +#include <fcntl.h>\n> +#include <iostream>\n> +#include <string.h>\n> +#include <sys/wait.h>\n> +#include <unistd.h>\n> +\n> +#include <libcamera/camera_manager.h>\n> +#include <libcamera/event_dispatcher.h>\n> +\n> +#include \"ipc_unixsocket.h\"\n> +#include \"test.h\"\n> +\n> +#define CMD_CLOSE 0\n> +#define CMD_REVERSE 1\n> +#define CMD_LEN_CALC 2\n> +#define CMD_LEN_CMP 3\n\ns/ \\([0-9]\\)/\\t\\1/\n\n(tab instead of space to align the numbers)\n\n> +\n> +using namespace std;\n> +using namespace libcamera;\n> +\n> +int calcLength(int fd)\n\ns/calcLength/calculateLength/\n\n> +{\n> +\tlseek(fd, 0, 0);\n> +\tint size = lseek(fd, 0, SEEK_END);\n> +\tlseek(fd, 0, 0);\n> +\n> +\treturn size;\n> +}\n> +\n> +class UnixSocketTestSlave\n> +{\n> +public:\n> +\tUnixSocketTestSlave()\n> +\t\t: exitCode_(EXIT_FAILURE)\n> +\t{\n> +\t\tdispatcher_ = CameraManager::instance()->eventDispatcher();\n> +\t\texit_.store(false, std::memory_order_release);\n\nWith a single process you can use a bool for exit_ and assign it\ndirectly.\n\n> +\t\tipc_.payloadReceived.connect(this, &UnixSocketTestSlave::payloadReceived);\n> +\t}\n> +\n> +\tint run(int fd)\n> +\t{\n> +\t\tif (ipc_.attach(fd)) {\n> +\t\t\tcerr << \"Failed to connect to IPC\" << endl;\n\ns/IPC/IPC channel/\n\n> +\t\t\treturn EXIT_FAILURE;\n> +\t\t}\n> +\n> +\t\twhile (!exit_.load(std::memory_order_acquire))\n> +\t\t\tdispatcher_->processEvents();\n> +\n> +\t\tipc_.close();\n> +\n> +\t\treturn exitCode_;\n> +\t}\n> +\n> +private:\n> +\tvoid payloadReceived(const IPCUnixSocket::Payload &payload)\n> +\t{\n> +\t\tIPCUnixSocket::Payload response;\n> +\t\tint ret;\n> +\n> +\t\tconst uint8_t cmd = payload.data.front();\n\ndata[0] would be more explicit.\n\n> +\n> +\t\tcout << \"Slave received command \" << static_cast<unsigned int>(cmd) << endl;\n> +\n> +\t\tswitch (cmd) {\n> +\t\tcase CMD_CLOSE:\n> +\t\t\tstop(0);\n> +\t\t\tbreak;\n\nA blank line between each case would make it easier to read.\n\n> +\t\tcase CMD_REVERSE: {\n> +\t\t\tstd::vector<uint8_t> raw(payload.data.begin() + 1, payload.data.end());\n> +\t\t\tstd::reverse(raw.begin(), raw.end());\n> +\t\t\traw.insert(raw.begin(), cmd);\n> +\n> +\t\t\tresponse.data = raw;\n\n\t\t\tresponse.data = payload.data;\n\t\t\tstd::reverse(response.data.begin() + 1, response.data.end());\n\n> +\t\t\tret = ipc_.send(response);\n> +\t\t\tif (ret < 0) {\n> +\t\t\t\tcerr << \"Reverse fail\" << endl;\n> +\t\t\t\tstop(ret);\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase CMD_LEN_CALC: {\n> +\t\t\tint size = 0;\n> +\t\t\tfor (int fd : payload.fds)\n> +\t\t\t\tsize += calcLength(fd);\n> +\n> +\t\t\tresponse.data.resize(1 + sizeof(size));\n> +\t\t\tresponse.data[0] = cmd;\n> +\t\t\tmemcpy(response.data.data() + 1, &size, sizeof(size));\n> +\n> +\t\t\tret = ipc_.send(response);\n> +\t\t\tif (ret < 0) {\n> +\t\t\t\tcerr << \"Calc fail\" << endl;\n> +\t\t\t\tstop(ret);\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase CMD_LEN_CMP: {\n> +\t\t\tint size = 0;\n> +\t\t\tfor (int fd : payload.fds)\n> +\t\t\t\tsize += calcLength(fd);\n> +\n> +\t\t\tint cmp;\n> +\t\t\tmemcpy(&cmp, payload.data.data() + 1, sizeof(cmp));\n> +\n> +\t\t\tif (cmp != size) {\n> +\t\t\t\tcerr << \"Compare fail\" << endl;\n> +\t\t\t\tstop(-ERANGE);\n> +\t\t\t}\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tdefault:\n> +\t\t\tcerr << \"Unknown command \" << cmd << endl;\n> +\t\t\tstop(-EINVAL);\n\nMissing break.\n\n> +\t\t}\n> +\t}\n> +\n> +\tvoid stop(int code)\n> +\t{\n> +\t\texitCode_ = code;\n> +\t\texit_.store(true, std::memory_order_release);\n> +\t}\n> +\n> +\tIPCUnixSocket ipc_;\n> +\tEventDispatcher *dispatcher_;\n> +\tint exitCode_;\n> +\tstd::atomic<bool> exit_;\n> +};\n> +\n> +static const std::vector<std::string> testPaths = {\n> +\t\"/proc/self/exe\",\n> +\t\"/proc/self/exe\",\n> +};\n> +\n> +class UnixSocketTest : public Test\n> +{\n> +protected:\n> +\tint slaveStart(int fd)\n> +\t{\n> +\t\tpid_ = fork();\n> +\n> +\t\tif (pid_ == -1)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (!pid_) {\n> +\t\t\tstd::string arg = std::to_string(fd);\n> +\t\t\texecl(\"/proc/self/exe\", \"/proc/self/exe\",\n> +\t\t\t      arg.c_str(), nullptr);\n> +\n> +\t\t\t/* Only get here if exec fails. */\n> +\t\t\texit(TestFail);\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint slaveStop()\n> +\t{\n> +\t\tint status;\n> +\n> +\t\tif (pid_ < 0)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (waitpid(pid_, &status, 0) < 0)\n> +\t\t\treturn TestFail;\n> +\n> +\t\tif (!WIFEXITED(status) || WEXITSTATUS(status))\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +\tint testReverse()\n> +\t{\n> +\t\tIPCUnixSocket::Payload reverse;\n> +\n> +\t\treverse.data = { CMD_REVERSE, 1, 2, 3, 4, 5 };\n> +\t\tif (ipc_.send(reverse))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tCameraManager::instance()->eventDispatcher()->processEvents();\n\nI think you should create a call() helper method that take a payload,\nsends it, calls processEvents() in a loop until a response is received\n(I would add a Timer to make sure we time out), and return the response.\nThat will simplify the logic of the rest of the tests, allowing you to\nmove the checks from payloadReceived() to the individual test methods.\n\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint testCalc()\n> +\t{\n> +\t\tIPCUnixSocket::Payload payload;\n> +\n> +\t\tcalcSize_ = 0;\n> +\t\tfor (const std::string path : testPaths) {\n> +\t\t\tint fd = open(path.c_str(), O_RDONLY);\n> +\t\t\tif (fd < 0)\n> +\t\t\t\treturn TestFail;\n> +\n> +\t\t\tcalcSize_ += calcLength(fd);\n> +\t\t\tpayload.fds.push_back(fd);\n> +\t\t}\n> +\n> +\t\tpayload.data.push_back(CMD_LEN_CALC);\n> +\t\tif (ipc_.send(payload))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tCameraManager::instance()->eventDispatcher()->processEvents();\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint testCmp()\n> +\t{\n> +\t\tIPCUnixSocket::Payload payload;\n> +\n> +\t\tint size = 0;\n> +\t\tfor (const std::string path : testPaths) {\n> +\t\t\tint fd = open(path.c_str(), O_RDONLY);\n> +\t\t\tif (fd < 0)\n> +\t\t\t\treturn TestFail;\n> +\n> +\t\t\tsize += calcLength(fd);\n> +\t\t\tpayload.fds.push_back(fd);\n> +\t\t}\n> +\n> +\t\tpayload.data.resize(1 + sizeof(size));\n> +\t\tpayload.data[0] = CMD_LEN_CMP;\n> +\t\tmemcpy(payload.data.data() + 1, &size, sizeof(size));\n> +\n> +\t\tif (ipc_.send(payload))\n> +\t\t\treturn TestFail;\n> +\n> +\t\treturn 0;\n> +\t}\n> +\n> +\tint run()\n> +\t{\n> +\t\tint slavefd = ipc_.create();\n> +\t\tif (slavefd < 0)\n> +\t\t\treturn TestFail;\n\nPlease print an error message when a failure is returned.\n\n> +\n> +\t\tif (slaveStart(slavefd))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tipc_.payloadReceived.connect(this, &UnixSocketTest::payloadReceived);\n> +\n> +\t\t/* Test reversing a string, this test sending only data. */\n> +\t\tranRev_ = false;\n> +\t\tif (testReverse())\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Test offloading a calculation, this test sending only FDs. */\n> +\t\tranCalc_ = false;\n> +\t\tif (testCalc())\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Test fire and forget, this tests sending data and FDs. */\n> +\t\tif (testCmp())\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Close slave connection. */\n> +\t\tIPCUnixSocket::Payload close;\n> +\t\tclose.data.push_back(CMD_CLOSE);\n> +\t\tif (ipc_.send(close))\n> +\t\t\treturn TestFail;\n> +\n> +\t\tipc_.close();\n> +\t\tif (slaveStop())\n> +\t\t\treturn TestFail;\n> +\n> +\t\t/* Check that all tests have executed. */\n> +\t\tif (!ranRev_ || !ranCalc_) {\n> +\t\t\tcerr << \"Not all messages responded to\" << endl;\n> +\t\t\treturn TestFail;\n> +\t\t}\n> +\n> +\t\treturn TestPass;\n> +\t}\n> +\n> +private:\n> +\tvoid payloadReceived(const IPCUnixSocket::Payload &payload)\n> +\t{\n> +\t\tconst uint8_t cmd = payload.data.front();\n> +\t\tcout << \"Master received command \" << static_cast<unsigned int>(cmd) << endl;\n> +\n> +\t\tswitch (cmd) {\n> +\t\tcase CMD_REVERSE: {\n> +\t\t\tstd::vector<uint8_t> raw(payload.data.begin() + 1, payload.data.end());\n> +\t\t\tif (raw == std::vector<uint8_t>{ 5, 4, 3, 2, 1 })\n> +\t\t\t\tranRev_ = true;\n> +\t\t\telse\n> +\t\t\t\tcerr << \"Reverse response invalid\" << endl;\n> +\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tcase CMD_LEN_CALC: {\n> +\t\t\tint output;\n> +\t\t\tmemcpy(&output, payload.data.data() + 1, sizeof(output));\n> +\t\t\tif (output == calcSize_)\n> +\t\t\t\tranCalc_ = true;\n> +\t\t\telse\n> +\t\t\t\tcerr << \"Calc response invalid\" << endl;\n> +\n> +\t\t\tbreak;\n> +\t\t}\n> +\t\tdefault:\n> +\t\t\tcerr << \"Unknown command \" << cmd << endl;\n> +\t\t}\n> +\t}\n> +\n> +\tpid_t pid_;\n> +\tIPCUnixSocket ipc_;\n> +\n> +\tbool ranRev_;\n> +\tbool ranCalc_;\n> +\tint calcSize_;\n> +};\n> +\n> +/*\n> + * Can't use TEST_REGISTER() as single binary needs to act as both proxy\n> + * master and slave.\n> + */\n> +int main(int argc, char **argv)\n> +{\n> +\tif (argc == 2) {\n> +\t\tint ipcfd = std::stoi(argv[1]);\n> +\t\tUnixSocketTestSlave slave;\n> +\t\treturn slave.run(ipcfd);\n> +\t}\n> +\n> +\treturn UnixSocketTest().execute();\n> +}\n> diff --git a/test/meson.build b/test/meson.build\n> index c36ac24796367501..3666f6b2385bd4ca 100644\n> --- a/test/meson.build\n> +++ b/test/meson.build\n> @@ -2,6 +2,7 @@ subdir('libtest')\n>  \n>  subdir('camera')\n>  subdir('ipa')\n> +subdir('ipc')\n>  subdir('media_device')\n>  subdir('pipeline')\n>  subdir('stream')","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 D137D60BC7\n\tfor <libcamera-devel@lists.libcamera.org>;\n\tFri, 28 Jun 2019 14:34:29 +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 3C52D2C6;\n\tFri, 28 Jun 2019 14:34:29 +0200 (CEST)"],"DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=ideasonboard.com;\n\ts=mail; t=1561725269;\n\tbh=X8uLA+zKNaNTqtEHwARYaAXSEG6kk0MapH+eskpnBhc=;\n\th=Date:From:To:Cc:Subject:References:In-Reply-To:From;\n\tb=BvnlzUrAFTh8KFphv8xJvEtR/j6D+RMKM1q43tJTnLW52Rnj/w+VEc7U/ezbpCfHn\n\tumDFRJ3cJz9GJGBFXwXuUVagKaFYzbB/iub3LTfYqzJHBuevb3quMsaFGqtFdGbKQh\n\tKGQFsJYYIaikciFzUJDMT5USJChghd+VlXa1Tv2A=","Date":"Fri, 28 Jun 2019 15:34:10 +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":"<20190628123410.GE4779@pendragon.ideasonboard.com>","References":"<20190627020955.6166-1-niklas.soderlund@ragnatech.se>\n\t<20190627020955.6166-3-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":"<20190627020955.6166-3-niklas.soderlund@ragnatech.se>","User-Agent":"Mutt/1.10.1 (2018-07-13)","Subject":"Re: [libcamera-devel] [PATCH 2/2] test: ipc: unix: Add test for\n\tIPCUnixSocket","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":"Fri, 28 Jun 2019 12:34:30 -0000"}}]