From 0b6c95b1ae36977719558190021ee37738a1922f Mon Sep 17 00:00:00 2001 From: sajilal711 Date: Mon, 24 Aug 2026 11:21:37 +0530 Subject: [PATCH 1/3] initial custom coding guidelines (#460) --- .github/copilot-instructions.md | 13 +++ .github/instructions/General.instructions.md | 98 ++++++++++++++++++++ 2 files changed, 111 insertions(+) create mode 100644 .github/copilot-instructions.md create mode 100644 .github/instructions/General.instructions.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 00000000..25be0634 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,13 @@ +# Dobby Copilot Instructions + +Use the custom instructions in `.github/instructions/*.instructions.md` as the primary coding guidance for this repository. + +## Review Comment Linking + +When leaving review comments based on a rule from `.github/instructions/General.instructions.md`, include a direct link to the section in the same format: + +Refer: https://github.com/rdkcentral/Dobby/blob/develop/.github/instructions/General.instructions.md#critical-logging + +## Scope + +These rules are intended to keep generated code and review comments aligned with Dobby's existing codebase conventions, plugin lifecycle model, build setup, and openspec documents. \ No newline at end of file diff --git a/.github/instructions/General.instructions.md b/.github/instructions/General.instructions.md new file mode 100644 index 00000000..d2c395a6 --- /dev/null +++ b/.github/instructions/General.instructions.md @@ -0,0 +1,98 @@ +--- +applyTo: "**/*.{cpp,h,cc,cxx,hpp},**/CMakeLists.txt,**/*.cmake,**/*.sh" +--- + +# Instruction Summary +1. Critical Logging +2. Recoverable Error Reporting +3. Null Safety and Pointer Style +4. Plugin Lifecycle Discipline +5. RDK Plugin Hook Contract +6. CMake and Plugin Onboarding Compliance +7. Spec and Schema Synchronization + +### Critical Logging + +### Requirement + +Use Dobby logging macros from `AppInfrastructure/Logging/include/Logging.h` for failures: + +```cpp +AI_LOG_ERROR("failed to parse config for plugin '%s'", pluginName.c_str()); +``` + +If the function returns failure, log once with enough context and return the appropriate failure value. + +### Incorrect Example + +```cpp +printf("failed to parse config\n"); +return false; +``` + +### Recoverable Error Reporting + +### Requirement + +Use `AI_LOG_WARN` for recoverable or fallback behavior. + +```cpp +if (missingOptionalField) +{ + AI_LOG_WARN("optional field missing, using default"); +} +``` + +Do not log expected fallback behavior as hard errors. + +### Null Safety and Pointer Style + +### Requirement + +- Use `nullptr` instead of `NULL` in all new code. +- Preserve nearby comparison style (`nullptr == ptr` or `ptr == nullptr`) to avoid style churn inside existing files. +- Validate pointers before dereference and fail with contextual log messages. + +### Plugin Lifecycle Discipline + +### Requirement + +For Dobby RDK plugins and daemon lifecycle-managed components: + +- Keep constructors lightweight. +- Do heavy setup in lifecycle hook methods. +- Release resources in reverse order of allocation. +- Reset internal state after cleanup. + +Where hook/teardown fails, log with enough context to diagnose container and plugin state. + +### RDK Plugin Hook Contract + +### Requirement + +- Implement `IDobbyRdkPlugin` contract correctly. +- Keep `name()` stable. +- Set `hookHints()` to match only implemented hooks. +- Prefer inheriting from `RdkPluginBase` and override only needed hooks. +- Declare inter-plugin ordering requirements through `getDependencies()` when required. + +### CMake and Plugin Onboarding Compliance + +### Requirement + +When introducing a new plugin in top-level `CMakeLists.txt`: + +1. Add matching `PLUGIN_` option and `add_subdirectory(...)`. +2. Update `.github/workflows/L1-tests.yml` plugin build flags. +3. Update `.github/workflows/L2-tests.yml` where integration coverage is needed. +4. Update `cov_build.sh` to include the plugin build flag. + +This keeps CI and static analysis coverage aligned with source registration. + +### Spec and Schema Synchronization + +### Requirement + +- Keep openspec docs in `openspec/specs/` in sync with behavior changes. +- For runtime schema changes under `bundle/runtime-schemas/`, re-run CMake so generated headers are refreshed. +- Do not merge behavior changes that leave specs stale. \ No newline at end of file From af3ac057a189a9b31c3f3ba6869780ebf9e66e55 Mon Sep 17 00:00:00 2001 From: Karthick Swaminathan <85346280+ks734@users.noreply.github.com> Date: Wed, 9 Sep 2026 22:42:14 +0530 Subject: [PATCH 2/3] RDKEMW-17166: Make container memory limits zram-aware (#470) * RDKEMW-17166: make container memory limits zram-aware * RDKEMW-17166: Fix L2 test * RDKEMW-17166: only apply zram-aware mem limit when swapLimit is explicit * Update DobbySpecConfig.cpp * RDKEMW-17166: Fix copilot reviews * RDKEMW-17166: Fix copilot reviews --- README.md | 6 +- bundle/lib/include/DobbySpecConfig.h | 5 + bundle/lib/source/DobbySpecConfig.cpp | 100 ++++++++++++++++-- .../DobbySpecConfigTest.cpp | 23 ++-- .../test_runner/bundle_generation.py | 15 ++- 5 files changed, 130 insertions(+), 19 deletions(-) diff --git a/README.md b/README.md index 2391bb3e..d01eb456 100644 --- a/README.md +++ b/README.md @@ -135,8 +135,8 @@ The table below lists the supported top-level fields. Fields marked **mandatory* | `version` | string | Yes | Spec version. Currently `"1.0"` or `"1.1"`. | | `args` | array | Yes | Command and arguments to run inside the container. | | `user` | object | Yes | `uid` and `gid` the container process runs as. | -| `memLimit` | integer | Yes | Memory limit in bytes (`memory.limit_in_bytes`). Values below 256 KiB are accepted but will only generate a warning and may not be effective. | -| `swapLimit` | integer | No | Swap+memory limit in bytes (`memory.memsw.limit_in_bytes`). Must be ≥ `memLimit`. Defaults to unlimited (-1) when absent. | +| `memLimit` | integer | Yes | Requested memory limit in bytes. When `swapLimit` is present, this value is scaled down according to the host's RAM and active zram swap capacity before being applied to `memory.limit_in_bytes`. Other swap types, such as swapfiles and partitions, are ignored. Values below 256 KiB are accepted but will only generate a warning and may not be effective. | +| `swapLimit` | integer | No | Swap+memory limit in bytes (`memory.memsw.limit_in_bytes`). Must be ≥ the effective scaled memory limit. Defaults to unlimited (-1) when absent. | | `env` | array | No | Environment variables in `"KEY=VALUE"` format. | | `cwd` | string | No | Working directory inside the container. | | `console` | object | No | Console log settings: `path` and `limit` (bytes). | @@ -168,7 +168,7 @@ The table below lists the supported top-level fields. Fields marked **mandatory* } ``` -`swapLimit` sets the combined memory+swap ceiling enforced by the kernel cgroup (`memory.memsw.limit_in_bytes`). When omitted, memory+swap is unlimited (-1), allowing the container to use as much swap as the system provides. +When `swapLimit` is present, `memLimit` is scaled down based on the host's active zram-swap-to-RAM ratio before it is applied to `memory.limit_in_bytes`. Swapfiles and swap partitions are not included in this ratio. `swapLimit` sets the combined memory+swap ceiling enforced by the kernel cgroup (`memory.memsw.limit_in_bytes`). When omitted, `memLimit` is applied directly and memory+swap is unlimited (-1), allowing the container to use as much swap as the system provides. ## DobbyTool This is a simple command line tool that is used for debugging purporses. It connects to the Dobby daemon over dbus and allows for debugging and testing containers. diff --git a/bundle/lib/include/DobbySpecConfig.h b/bundle/lib/include/DobbySpecConfig.h index 8f6bede2..8d176fa1 100644 --- a/bundle/lib/include/DobbySpecConfig.h +++ b/bundle/lib/include/DobbySpecConfig.h @@ -28,6 +28,7 @@ #include #include +#include #include namespace ctemplate { @@ -174,6 +175,9 @@ class DobbySpecConfig : public DobbyConfig static void addGpuDevNodes(const std::shared_ptr &settings, ctemplate::TemplateDictionary *dict); + static int64_t calculatePhysicalMemoryLimit(int64_t memLimit, + double swapToRamRatio); + static void addVpuDevNodes(const std::shared_ptr &settings, ctemplate::TemplateDictionary *dict); @@ -187,6 +191,7 @@ class DobbySpecConfig : public DobbyConfig private: bool mValid; ctemplate::TemplateDictionary* mDictionary; + double mZramSwapToRamRatio; private: Json::Value mSpec; diff --git a/bundle/lib/source/DobbySpecConfig.cpp b/bundle/lib/source/DobbySpecConfig.cpp index b58b99f3..e6fd6167 100644 --- a/bundle/lib/source/DobbySpecConfig.cpp +++ b/bundle/lib/source/DobbySpecConfig.cpp @@ -37,6 +37,7 @@ #include #include #include +#include // Compile time generated strings that (in theory) speeds up the processing // of ctemplate expanding @@ -196,6 +197,75 @@ static const ctemplate::StaticTemplateString SECCOMP_SYSCALLS = int DobbySpecConfig::mNumCores = -1; +int64_t DobbySpecConfig::calculatePhysicalMemoryLimit(int64_t memLimit, + double swapToRamRatio) +{ + return static_cast((1.0 / (1.0 + swapToRamRatio)) * static_cast(memLimit)); +} + +static double getZramSwapToRamRatio() +{ + std::ifstream meminfo("/proc/meminfo"); + if (!meminfo.is_open()) + { + return 0.0; + } + + unsigned long long memTotalKb = 0; + std::string line; + + while (std::getline(meminfo, line)) + { + std::istringstream iss(line); + std::string key; + iss >> key; + + if (key == "MemTotal:") + { + unsigned long long value = 0; + std::string units; + if (!(iss >> value >> units)) + { + return 0.0; + } + memTotalKb = value; + } + } + + if (memTotalKb == 0) + { + return 0.0; + } + + std::ifstream swaps("/proc/swaps"); + if (!swaps.is_open()) + { + return 0.0; + } + + unsigned long long zramTotalKb = 0; + std::string filename; + std::string type; + unsigned long long sizeKb = 0; + unsigned long long usedKb = 0; + int priority = 0; + std::getline(swaps, line); + while (swaps >> filename >> type >> sizeKb >> usedKb >> priority) + { + if (filename.rfind("/dev/zram", 0) == 0) + { + zramTotalKb += sizeKb; + } + } + + if (zramTotalKb == 0) + { + return 0.0; + } + + return static_cast(zramTotalKb) / static_cast(memTotalKb); +} + // TODO: should we only allowed these if a network namespace is enabled ? const std::map DobbySpecConfig::mAllowedCaps = { @@ -225,6 +295,7 @@ DobbySpecConfig::DobbySpecConfig(const std::shared_ptr &utils, , mDefaultPlugins(settings->defaultPlugins()) , mRdkPluginsData(settings->rdkPluginsData()) , mDictionary(nullptr) + , mZramSwapToRamRatio(0.0) , mConf(nullptr) , mSpecVersion(SpecVersion::Unknown) , mUserId(-1) @@ -308,6 +379,7 @@ DobbySpecConfig::DobbySpecConfig(const std::shared_ptr &utils, , mGpuSettings(settings->gpuAccessSettings()) , mVpuSettings(settings->vpuAccessSettings()) , mDictionary(nullptr) + , mZramSwapToRamRatio(0.0) , mConf(nullptr) , mSpecVersion(SpecVersion::Unknown) , mUserId(-1) @@ -532,6 +604,11 @@ bool DobbySpecConfig::parseSpec(ctemplate::TemplateDictionary* dictionary, } } + if (mSpec.isMember("swapLimit") && mSpec["swapLimit"].isIntegral()) + { + mZramSwapToRamRatio = getZramSwapToRamRatio(); + } + // step 2 - get the version number of the spec first, it may determine how // subsequent fields are processed const Json::Value version = mSpec["version"]; @@ -1306,7 +1383,14 @@ bool DobbySpecConfig::processMemLimit(const Json::Value& value, AI_LOG_WARN("memory limit looks dangerously low"); } - dictionary->SetIntValue(MEM_LIMIT, memLimit); + // Only apply the zram-aware adjustment when swapLimit is explicitly + // set; otherwise keep memLimit as-is to match the memory.limit_in_bytes. + unsigned physLimit = memLimit; + if (mSpec.isMember("swapLimit") && mSpec["swapLimit"].isIntegral()) + { + physLimit = static_cast(calculatePhysicalMemoryLimit(memLimit, mZramSwapToRamRatio)); + } + dictionary->SetIntValue(MEM_LIMIT, physLimit); return true; } @@ -1319,8 +1403,9 @@ bool DobbySpecConfig::processMemLimit(const Json::Value& value, * allowing swap to be configured independently of the memory limit. When * absent the swap limit is set to -1 (unlimited). * - * The kernel requires swap >= memLimit, so an error is returned if the - * supplied value is smaller than the memLimit already set. + * The kernel requires swap >= the effective physical memory limit, so an + * error is returned if the supplied value is smaller than the adjusted + * memory.limit_in_bytes value. * * Example json: * @@ -1363,10 +1448,13 @@ bool DobbySpecConfig::processSwapLimit(const Json::Value& value, AI_LOG_ERROR("memLimit is negative; cannot validate swapLimit"); return false; } - if (memSwapSigned < memLimitSigned) + + // swapLimit must be at least the effective zram-adjusted physical limit. + const int64_t physLimit = calculatePhysicalMemoryLimit(memLimitSigned, mZramSwapToRamRatio); + if (memSwapSigned < physLimit) { - AI_LOG_ERROR("swapLimit (%" PRId64 ") must be >= memLimit (%" PRId64 ")", - memSwapSigned, memLimitSigned); + AI_LOG_ERROR("swapLimit (%" PRId64 ") must be >= memory.limit_in_bytes (%" PRId64 ")", + memSwapSigned, physLimit); return false; } } diff --git a/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp b/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp index c7155bf8..e9961589 100644 --- a/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp +++ b/tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp @@ -77,7 +77,7 @@ static const char* kSpecSwapBelowLimit = R"({ "args": ["/bin/true"], "user": { "uid": 1000, "gid": 1000 }, "memLimit": 5996544, - "swapLimit": 2998272 + "swapLimit": 0 })"; // ── Spec with capabilities ──────────────────────────────────────────────────── @@ -208,15 +208,24 @@ class DobbySpecConfigTest : public ::testing::Test // ── Tests ───────────────────────────────────────────────────────────────────── /** - * When 'swapLimit' is absent, MEM_SWAP must default to -1 (unlimited). + * When 'swapLimit' is absent, MEM_SWAP must default to -1 (unlimited) and + * MEM_LIMIT must be left as the raw memLimit (no zram adjustment applied). */ TEST_F(DobbySpecConfigTest, SwapLimit_DefaultsToUnlimited) { auto cfg = makeConfig(kSpecMemOnly); EXPECT_TRUE(cfg->isValid()); + EXPECT_EQ(expandMemTemplate(*cfg), "LIMIT=2998272 SWAP=-1"); } +TEST_F(DobbySpecConfigTest, PhysicalMemoryLimit_UsesRamShareOfTotalCapacity) +{ + EXPECT_EQ(DobbySpecConfig::calculatePhysicalMemoryLimit(1000, 0.0), 1000); + EXPECT_EQ(DobbySpecConfig::calculatePhysicalMemoryLimit(1000, 1.0), 500); + EXPECT_EQ(DobbySpecConfig::calculatePhysicalMemoryLimit(1000, 3.0), 250); +} + /** * When 'swapLimit' is greater than 'memLimit', MEM_SWAP must be set to the * supplied swap limit independently of MEM_LIMIT. @@ -225,7 +234,8 @@ TEST_F(DobbySpecConfigTest, SwapLimit_SetIndependently) { auto cfg = makeConfig(kSpecWithSwap); EXPECT_TRUE(cfg->isValid()); - EXPECT_EQ(expandMemTemplate(*cfg), "LIMIT=2998272 SWAP=5996544"); + + EXPECT_NE(expandMemTemplate(*cfg).find("SWAP=5996544"), std::string::npos); } /** @@ -236,12 +246,13 @@ TEST_F(DobbySpecConfigTest, SwapLimit_EqualToMemLimit_Succeeds) { auto cfg = makeConfig(kSpecSwapEqualsLimit); EXPECT_TRUE(cfg->isValid()); - EXPECT_EQ(expandMemTemplate(*cfg), "LIMIT=2998272 SWAP=2998272"); + + EXPECT_NE(expandMemTemplate(*cfg).find("SWAP=2998272"), std::string::npos); } /** - * When 'swapLimit' < 'memLimit', processSwapLimit must reject the value - * and parsing must fail (kernel requires memsw >= mem). + * When 'swapLimit' is below the effective physical memory limit, + * processSwapLimit must reject the value and parsing must fail. */ TEST_F(DobbySpecConfigTest, SwapLimit_LessThanMemLimit_Fails) { diff --git a/tests/L2_testing/test_runner/bundle_generation.py b/tests/L2_testing/test_runner/bundle_generation.py index b0a480db..25791c33 100755 --- a/tests/L2_testing/test_runner/bundle_generation.py +++ b/tests/L2_testing/test_runner/bundle_generation.py @@ -73,11 +73,18 @@ def _normalise_config(config): if not cpu: resources.pop("cpu", None) - # swap limit is injected by the OCI config template (set equal to - # memory limit to disable swap). Original test bundles pre-date - # this addition, so strip it to keep the comparison stable. + # Swap is generated as -1 when no swapLimit is configured. Preserve the + # stable physical memory limit in that case; only omit it when an + # explicit swap limit caused host-dependent zram scaling. if isinstance(resources, dict) and isinstance(resources.get("memory"), dict): - resources["memory"].pop("swap", None) + memory = resources["memory"] + swap = memory.get("swap") + limit = memory.get("limit") + memory.pop("swap", None) + if swap is not None and swap != -1 and swap != limit: + memory.pop("limit", None) + if not memory: + resources.pop("memory", None) # Runtime may append tmpfs size options at generation time for mount in cfg.get("mounts", []): From 62ac73f2059d738fb7364118fc86c6f1597741b7 Mon Sep 17 00:00:00 2001 From: B-Larsen Date: Thu, 10 Sep 2026 20:42:32 -0400 Subject: [PATCH 3/3] RDKEMW-19866: Update Dobby v3.22.0 --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index b495488d..021f5c6c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -22,12 +22,12 @@ cmake_minimum_required( VERSION 3.7.0 ) include(GNUInstallDirs) # Project setup -project( Dobby VERSION "3.21.0" ) +project( Dobby VERSION "3.22.0" ) # Set the major and minor version numbers of dobby (also used by plugins) set( DOBBY_MAJOR_VERSION 3 ) -set( DOBBY_MINOR_VERSION 21 ) +set( DOBBY_MINOR_VERSION 22 ) set( DOBBY_MICRO_VERSION 0 ) set(INSTALL_CMAKE_DIR lib/cmake/Dobby)