From 29e1aa6c12e3590252b102153d8c62155047a676 Mon Sep 17 00:00:00 2001 From: David Sidrane Date: Thu, 28 Jan 2021 13:10:42 -0800 Subject: [PATCH] Reworked file naming and use ROM fs as fall back The server will now attempt to provide files from the primary path RootPath and then the alternate AltRootPath. This alows a layered approch to file service. Setting the RootPath to an SD card and the AltRootPath to a ROM FS will allow a user to provide an updated uavcan firware file that will be used instead of the baked in one without any copying. The path as been removed from the interactions with nodes. There by supporting Non-compliant nodes that could not handle the specified path length. The vitrual pathing has been moved into the FDCache. --- .../include/uavcan/protocol/file_server.hpp | 55 ++- .../protocol/firmware_update_trigger.hpp | 8 +- .../basic_file_server_backend.hpp | 84 ++-- .../uavcan_posix/firmware_version_checker.hpp | 370 +++++++----------- 4 files changed, 250 insertions(+), 267 deletions(-) diff --git a/libuavcan/include/uavcan/protocol/file_server.hpp b/libuavcan/include/uavcan/protocol/file_server.hpp index f1ca877cd8..167be48fa9 100644 --- a/libuavcan/include/uavcan/protocol/file_server.hpp +++ b/libuavcan/include/uavcan/protocol/file_server.hpp @@ -25,11 +25,15 @@ namespace uavcan */ class UAVCAN_EXPORT IFileServerBackend { + public: typedef protocol::file::Path::FieldTypes::path Path; typedef protocol::file::EntryType EntryType; typedef protocol::file::Error Error; + IFileServerBackend::Path root_path_; + IFileServerBackend::Path alt_root_path_; + /** * All read operations must return this number of bytes, unless end of file is reached. */ @@ -40,6 +44,51 @@ public: */ static char getPathSeparator() { return static_cast(protocol::file::Path::SEPARATOR); } + /** + * Set a base path to the files. + */ + void setRootPath(const char * path) + { + if (path) + { + root_path_.clear(); + root_path_ = path; + if (root_path_.back() != getPathSeparator()) + { + root_path_.push_back(getPathSeparator()); + } + } + } + + void setAltRootPath(const char * path) + { + if (path) + { + alt_root_path_.clear(); + alt_root_path_ = path; + if (alt_root_path_.back() != getPathSeparator()) + { + alt_root_path_.push_back(getPathSeparator()); + } + } + } + + /** + * Get a base path to the files. + */ + Path& getRootPath() + { + return root_path_; + } + + /** + * Get a base path to the files. + */ + Path& getAltRootPath() + { + return alt_root_path_; + } + /** * Backend for uavcan.protocol.file.GetInfo. * Refer to uavcan.protocol.file.EntryType for the list of available bit flags. @@ -161,8 +210,11 @@ public: , backend_(backend) { } - int start() + int start(const char* server_root = UAVCAN_NULLPTR, const char* server_alt_root = UAVCAN_NULLPTR) { + backend_.setRootPath(server_root); + backend_.setAltRootPath(server_alt_root); + int res = get_info_srv_.start(GetInfoCallback(this, &BasicFileServer::handleGetInfo)); if (res < 0) { @@ -224,6 +276,7 @@ class FileServer : protected BasicFileServer resp.entry_type, resp.entry_full_path.path); } + public: FileServer(INode& node, IFileServerBackend& backend) : BasicFileServer(node, backend) diff --git a/libuavcan/include/uavcan/protocol/firmware_update_trigger.hpp b/libuavcan/include/uavcan/protocol/firmware_update_trigger.hpp index 5b0d019481..04de3be0d2 100644 --- a/libuavcan/include/uavcan/protocol/firmware_update_trigger.hpp +++ b/libuavcan/include/uavcan/protocol/firmware_update_trigger.hpp @@ -345,12 +345,7 @@ class FirmwareUpdateTrigger : public INodeInfoListener, protocol::file::BeginFirmwareUpdate::Request req; req.source_node_id = getNode().getNodeID().get(); - if (!common_path_prefix_.empty()) - { - req.image_file_remote_path.path += common_path_prefix_.c_str(); - req.image_file_remote_path.path.push_back(protocol::file::Path::SEPARATOR); - } - req.image_file_remote_path.path += path->c_str(); + req.image_file_remote_path.path = path->c_str(); UAVCAN_TRACE("FirmwareUpdateTrigger", "Request to %d with path: %s", int(node_id.get()), req.image_file_remote_path.path.c_str()); @@ -396,6 +391,7 @@ public: */ int start(NodeInfoRetriever& node_info_retriever, const FirmwareFilePath& arg_common_path_prefix = FirmwareFilePath(), + const FirmwareFilePath& arg_alt_path_prefix = FirmwareFilePath(), const TransferPriority priority = TransferPriority::OneHigherThanLowest) { /* diff --git a/libuavcan_drivers/posix/include/uavcan_posix/basic_file_server_backend.hpp b/libuavcan_drivers/posix/include/uavcan_posix/basic_file_server_backend.hpp index eec15f99d3..14f3672030 100644 --- a/libuavcan_drivers/posix/include/uavcan_posix/basic_file_server_backend.hpp +++ b/libuavcan_drivers/posix/include/uavcan_posix/basic_file_server_backend.hpp @@ -1,6 +1,6 @@ /**************************************************************************** * -* Copyright (c) 2015 PX4 Development Team. All rights reserved. +* Copyright (c) 2015, 2021 PX4 Development Team. All rights reserved. * Author: Pavel Kirienko * David Sidrane * @@ -37,6 +37,7 @@ class BasicFileServerBackend : public uavcan::IFileServerBackend enum { FilePermissions = 438 }; ///< 0o666 protected: + class FDCacheBase { public: @@ -71,6 +72,9 @@ protected: /// Rate in Seconds that the cache will be flushed of stale entries. enum { GarbageCollectionSeconds = 60 }; + IFileServerBackend::Path& alt_root_path_; + IFileServerBackend::Path& root_path_; + class FDCacheItem : uavcan::Noncopyable { friend FDCache; @@ -237,8 +241,10 @@ protected: } public: - FDCache(uavcan::INode& node) : + FDCache(uavcan::INode& node, IFileServerBackend::Path& root_path, IFileServerBackend::Path& alt_root_path) : TimerBase(node), + alt_root_path_(alt_root_path), + root_path_(root_path), head_(UAVCAN_NULLPTR) { } @@ -265,7 +271,18 @@ protected: } else { - fd = FDCacheBase::open(path, oflags); + Path vpath = root_path_.c_str(); + vpath += path; + + fd = FDCacheBase::open(vpath.c_str(), oflags); + + if (fd < 0) + { + vpath = alt_root_path_.c_str(); + vpath += path; + fd = FDCacheBase::open(vpath.c_str(), oflags); + } + if (fd < 0) { return fd; @@ -320,7 +337,7 @@ protected: { if (fdcache_ == UAVCAN_NULLPTR) { - fdcache_ = new FDCache(node_); + fdcache_ = new FDCache(node_, getRootPath(), getAltRootPath()); if (fdcache_ == UAVCAN_NULLPTR) { @@ -343,31 +360,41 @@ protected: if (path.size() > 0) { - using namespace std; - struct stat sb; + using namespace std; - rv = stat(path.c_str(), &sb); + struct stat sb; - if (rv < 0) - { - rv = errno; - } - else - { - rv = 0; - out_size = sb.st_size; - out_type.flags = uavcan::protocol::file::EntryType::FLAG_READABLE; - if (S_ISDIR(sb.st_mode)) - { - out_type.flags |= uavcan::protocol::file::EntryType::FLAG_DIRECTORY; - } - else if (S_ISREG(sb.st_mode)) - { - out_type.flags |= uavcan::protocol::file::EntryType::FLAG_FILE; - } - // TODO Using fixed flag FLAG_READABLE until we add file permission checks to return actual value. - } + Path vpath = getRootPath().c_str(); + vpath += path; + + rv = stat(vpath.c_str(), &sb); + if (rv < 0) + { + vpath = getAltRootPath().c_str(); + vpath += path; + rv = stat(vpath.c_str(), &sb); + } + + if (rv < 0) + { + rv = errno; + } + else + { + rv = 0; + out_size = sb.st_size; + out_type.flags = uavcan::protocol::file::EntryType::FLAG_READABLE; + if (S_ISDIR(sb.st_mode)) + { + out_type.flags |= uavcan::protocol::file::EntryType::FLAG_DIRECTORY; + } + else if (S_ISREG(sb.st_mode)) + { + out_type.flags |= uavcan::protocol::file::EntryType::FLAG_FILE; + } + // TODO Using fixed flag FLAG_READABLE until we add file permission checks to return actual value. + } } return rv; } @@ -389,11 +416,12 @@ protected: using namespace std; FDCacheBase& cache = getFDCache(); + int fd = cache.open(path.c_str(), O_RDONLY); if (fd < 0) { - rv = errno; + rv = -errno; } else { @@ -403,7 +431,7 @@ protected: if (rv < 0) { - rv = errno; + rv = -errno; } else { diff --git a/libuavcan_drivers/posix/include/uavcan_posix/firmware_version_checker.hpp b/libuavcan_drivers/posix/include/uavcan_posix/firmware_version_checker.hpp index a7f87b1035..d8ac9009c8 100644 --- a/libuavcan_drivers/posix/include/uavcan_posix/firmware_version_checker.hpp +++ b/libuavcan_drivers/posix/include/uavcan_posix/firmware_version_checker.hpp @@ -23,6 +23,8 @@ # define DIRENT_ISFILE(dtype) ((dtype) == DT_REG) #endif +#define ALT_APD_SIGNATURE 0x40, 0xa2, 0xe4, 0xf1, 0x64, 0x68, 0x91, 0x06 + namespace uavcan_posix { /** @@ -51,12 +53,7 @@ class FirmwareVersionChecker : public uavcan::IFirmwareVersionChecker typedef uavcan::MakeString::Type PathString; BasePathString base_path_; - BasePathString cache_path_; - - /** - * The folder where the files will be copied and read from - */ - static const char* getCacheDir() { return "c"; } + BasePathString alt_base_path_; static void addSlash(BasePathString& path) { @@ -79,147 +76,9 @@ class FirmwareVersionChecker : public uavcan::IFirmwareVersionChecker base_path_ = path; } - void setFirmwareCachePath(const char* path) + void setFirmwareAltBasePath(const char* path) { - cache_path_ = path; - } - - int copyIfNot(const char* srcpath, const char* destpath) - { - using namespace std; - - // Does the file exist - int rv = 0; - int dfd = open(destpath, O_RDONLY, 0); - - if (dfd >= 0) - { - // Close it and exit 0 - (void)close(dfd); - } - else - { - uint8_t buffer[512]; - - dfd = open(destpath, O_WRONLY | O_CREAT, FilePermissions); - if (dfd < 0) - { - rv = -errno; - } - else - { - int sfd = open(srcpath, O_RDONLY, 0); - if (sfd < 0) - { - rv = -errno; - } - else - { - ssize_t size = 0; - do - { - size = ::read(sfd, buffer, sizeof(buffer)); - if (size != 0) - { - if (size < 0) - { - rv = -errno; - } - else - { - rv = 0; - ssize_t remaining = size; - ssize_t total_written = 0; - ssize_t written = 0; - do - { - written = write(dfd, &buffer[total_written], remaining); - if (written < 0) - { - rv = -errno; - } - else - { - total_written += written; - remaining -= written; - } - } - while (written > 0 && remaining > 0); - } - } - } - while (rv == 0 && size != 0); - - (void)close(sfd); - } - (void)close(dfd); - } - } - return rv; - } - - struct AppDescriptor - { - uavcan::uint8_t signature[sizeof(uavcan::uint64_t)]; - uavcan::uint64_t image_crc; - uavcan::uint32_t image_size; - uavcan::uint32_t vcs_commit; - uavcan::uint8_t major_version; - uavcan::uint8_t minor_version; - uavcan::uint8_t reserved[6]; - }; - - static int getFileInfo(const char* path, AppDescriptor& descriptor) - { - using namespace std; - - const unsigned MaxChunk = 512 / sizeof(uint64_t); - - uint64_t signature = 0; - std::memcpy(&signature, "APDesc00", 8); - - int rv = -ENOENT; - uint64_t chunk[MaxChunk]; - int fd = open(path, O_RDONLY); - - if (fd >= 0) - { - AppDescriptor* pdescriptor = UAVCAN_NULLPTR; - - while (pdescriptor == UAVCAN_NULLPTR) - { - int len = read(fd, chunk, sizeof(chunk)); - - if (len == 0) - { - break; - } - - if (len < 0) - { - rv = -errno; - goto out_close; - } - - uint64_t* p = &chunk[0]; - - do - { - if (*p == signature) - { - pdescriptor = reinterpret_cast(p); // FIXME TODO This breaks strict aliasing - descriptor = *pdescriptor; - rv = 0; - break; - } - } - while (p++ <= &chunk[MaxChunk - (sizeof(AppDescriptor) / sizeof(chunk[0]))]); - } - - out_close: - (void)close(fd); - } - return rv; + alt_base_path_ = path; } protected: @@ -249,73 +108,44 @@ protected: * * So for the file: * org.pixhawk.px4cannode-v1-0.1.59efc137.uavcan.bin - * +---fw - * +-c <----------- Files are cashed here. - * +--- px4cannode-v1.59efc137.bin <------ A Firmware file - * +---org.pixhawk.px4cannode-v1 <---------- node_info.name - * +---1.0 <-------------------------------- node_info.name's hardware_version.major,minor - * + - px4cannode-v1.59efc137.bin <---- A well known file must match the name - * in the root fw folder, so if it does not exist - * it is copied up MUST BE < 32 Characters + * +---ufw + * +- board_id.bin */ bool rv = false; + uint16_t board_id = (node_info.hardware_version.major << 8) + node_info.hardware_version.minor; + char bin_file_name[MaxBasePathLength + 1]; + int n = snprintf(bin_file_name, sizeof(bin_file_name), "%u.bin", board_id); - char fname_root[MaxBasePathLength + 1]; - int n = snprintf(fname_root, sizeof(fname_root), "%s%s/%d.%d", - getFirmwareBasePath().c_str(), - node_info.name.c_str(), - node_info.hardware_version.major, - node_info.hardware_version.minor); - - if (n > 0 && n < (int)sizeof(fname_root) - 2) + if (n > 0 && n < (int)sizeof(bin_file_name) - 2) { - DIR* const fwdir = opendir(fname_root); + char bin_file_path[MaxBasePathLength + 1]; - fname_root[n++] = getPathSeparator(); - fname_root[n++] = '\0'; + // Look on Primary location - if (fwdir != UAVCAN_NULLPTR) + snprintf(bin_file_path, sizeof(bin_file_path), "%s%s", + getFirmwareBasePath().c_str(), bin_file_name); + + // We have a file path, is it a valid image + + AppDescriptor descriptor{0}; + + bool found = getFileInfo(bin_file_path, descriptor) == 0; + + if (!found && !getFirmwareAltBasePath().empty()) { - struct dirent* pfile = UAVCAN_NULLPTR; - while ((pfile = readdir(fwdir)) != UAVCAN_NULLPTR) - { - if (DIRENT_ISFILE(pfile->d_type)) - { - // Open any bin file in there. - if (strstr(pfile->d_name, ".bin") != UAVCAN_NULLPTR) - { - PathString full_src_path = fname_root; - full_src_path += pfile->d_name; + snprintf(bin_file_name, sizeof(bin_file_name), "%u.bin", board_id); + snprintf(bin_file_path, sizeof(bin_file_path), "%s/%s", + getFirmwareAltBasePath().c_str(), bin_file_name); - PathString full_dst_path = getFirmwareCachePath().c_str(); - full_dst_path += pfile->d_name; + found = getFileInfo(bin_file_path, descriptor) == 0; + } - // ease the burden on the user - int cr = copyIfNot(full_src_path.c_str(), full_dst_path.c_str()); - - // We have a file, is it a valid image - AppDescriptor descriptor; - - std::memset(&descriptor, 0, sizeof(descriptor)); - - if (cr == 0 && getFileInfo(full_dst_path.c_str(), descriptor) == 0) - { - volatile AppDescriptor descriptorC = descriptor; - descriptorC.reserved[1]++; - - if (node_info.software_version.image_crc == 0 || - (node_info.software_version.major == 0 && node_info.software_version.minor == 0) || - descriptor.image_crc != node_info.software_version.image_crc) - { - rv = true; - out_firmware_file_path = pfile->d_name; - } - break; - } - } - } - } - (void)closedir(fwdir); + if (found && (node_info.software_version.image_crc == 0 || + (node_info.software_version.major == 0 && node_info.software_version.minor == 0) || + descriptor.image_crc != node_info.software_version.image_crc)) + { + rv = true; + out_firmware_file_path = bin_file_name; } } return rv; @@ -349,9 +179,89 @@ protected: } public: + struct AppDescriptor + { + uavcan::uint8_t signature[sizeof(uavcan::uint64_t)]; + union{ + uavcan::uint64_t image_crc; + uavcan::uint32_t crc32_block1; + uavcan::uint32_t crc32_block2; + }; + uavcan::uint32_t image_size; + uavcan::uint32_t vcs_commit; + uavcan::uint8_t major_version; + uavcan::uint8_t minor_version; + uavcan::uint16_t board_id; + uavcan::uint8_t reserved[ 3 + 3 + 2]; + }; + + static int getFileInfo(const char* path, AppDescriptor& descriptor, int limit = 0) + { + using namespace std; + + const unsigned MaxChunk = 512 / sizeof(uint64_t); + + // Make sure this does not present as a valid descriptor + struct { + union { + uavcan::uint64_t l; + uavcan::uint8_t b[sizeof(uint64_t)]{ALT_APD_SIGNATURE}; + } signature; + uavcan::uint8_t zeropad[sizeof(AppDescriptor) - sizeof(uint64_t)]{0}; + } s; + + int rv = -ENOENT; + uint64_t chunk[MaxChunk]; + int fd = open(path, O_RDONLY); + + if (fd >= 0) + { + AppDescriptor* pdescriptor = UAVCAN_NULLPTR; + + while (pdescriptor == UAVCAN_NULLPTR && limit >= 0) + { + int len = read(fd, chunk, sizeof(chunk)); + + if (len == 0) + { + break; + } + + if (len < 0) + { + rv = -errno; + goto out_close; + } + + uint64_t* p = &chunk[0]; + + if (limit > 0) + { + limit -= sizeof(chunk); + } + + do + { + if (*p == s.signature.l) + { + pdescriptor = reinterpret_cast(p); // FIXME TODO This breaks strict aliasing + descriptor = *pdescriptor; + rv = 0; + break; + } + } + while (p++ <= &chunk[MaxChunk - (sizeof(AppDescriptor) / sizeof(chunk[0]))]); + } + + out_close: + (void)close(fd); + } + return rv; + } + const BasePathString& getFirmwareBasePath() const { return base_path_; } - const BasePathString& getFirmwareCachePath() const { return cache_path_; } + const BasePathString& getFirmwareAltBasePath() const { return alt_base_path_; } static char getPathSeparator() { @@ -365,16 +275,29 @@ public: * for the issues that FirmwareFilePath is 40 * * It creates a path structure: - * +---(base_path) - * +-c <----------- Files are cached here. + * +---(base_path) <----------- Files are here. */ - int createFwPaths(const char* base_path) + + int createFwPaths(const char* base_path, const char* alt_base_path = nullptr) { using namespace std; int rv = -uavcan::ErrInvalidParam; - if (base_path) + if (alt_base_path) { + const int len = strlen(alt_base_path); + if (len > 0 && len < base_path_.MaxSize) + { + setFirmwareAltBasePath(alt_base_path); + } + else + { + return rv; + } + } + + if (base_path) + { const int len = strlen(base_path); if (len > 0 && len < base_path_.MaxSize) @@ -383,35 +306,13 @@ public: removeSlash(base_path_); const char* path = getFirmwareBasePath().c_str(); - setFirmwareCachePath(path); - addSlash(cache_path_); - cache_path_ += getCacheDir(); - rv = 0; struct stat sb; if (stat(path, &sb) != 0 || !S_ISDIR(sb.st_mode)) { rv = mkdir(path, S_IRWXU | S_IRWXG | S_IRWXO); } - - path = getFirmwareCachePath().c_str(); - - if (rv == 0 && (stat(path, &sb) != 0 || !S_ISDIR(sb.st_mode))) - { - rv = mkdir(path, S_IRWXU | S_IRWXG | S_IRWXO); - } - addSlash(base_path_); - addSlash(cache_path_); - - if (rv >= 0) - { - if ((getFirmwareCachePath().size() + uavcan::protocol::file::Path::FieldTypes::path::MaxSize) > - MaxPathLength) - { - rv = -uavcan::ErrInvalidConfiguration; - } - } } } return rv; @@ -419,7 +320,12 @@ public: const char* getFirmwarePath() const { - return getFirmwareCachePath().c_str(); + return getFirmwareBasePath().c_str(); + } + + const char* getAltFirmwarePath() const + { + return getFirmwareAltBasePath().c_str(); } }; }