Make container swap limit configurable independently of memory limit

[goruklu]

Summary

Closes #424

The OCI config templates previously hard-coded memory.memsw.limit_in_bytes (swap) to the same value as memory.limit_in_bytes, making it impossible to configure swap independently of the memory limit.

Changes

New optional swapLimit spec field

A new top-level swapLimit field has been added to the Dobby JSON spec. When present it sets the cgroup swap limit independently of memLimit. When absent the behaviour is unchanged — swap defaults to the same value as memLimit (i.e. no additional swap beyond the memory limit).

{
    "memLimit": 2998272,
    "swapLimit": 5996544
}

The kernel requires memory.memsw.limit_in_bytes >= memory.limit_in_bytes, so an error is returned if swapLimit is set lower than memLimit.

Implementation

File	Change
bundle/lib/source/DobbySpecConfig.cpp	New MEM_SWAP ctemplate constant; JSON_FLAG_SWAPLIMIT flag (bit 23); new processSwapLimit() processor; step-5 default sets MEM_SWAP = memLimit when swapLimit is absent
bundle/lib/include/DobbySpecConfig.h	Declaration of processSwapLimit()
bundle/lib/source/templates/OciConfigJson1.0.2-dobby.template	"swap": {{MEM_SWAP}} (was {{MEM_LIMIT}})
bundle/lib/source/templates/OciConfigJsonVM1.0.2-dobby.template	Same template fix
README.md	New Dobby Spec Format section documenting all fields including memLimit and swapLimit

Tests

L1 (unit) — tests/L1_testing/tests/DobbySpecConfigTest/

Five GTest cases using #define private public to directly exercise processSwapLimit and verify the MEM_SWAP ctemplate dictionary value:

• SwapLimit_DefaultsToMemLimit — absent swapLimit → MEM_SWAP == MEM_LIMIT
• SwapLimit_SetIndependently — swapLimit > memLimit → MEM_SWAP reflects the supplied value
• SwapLimit_EqualToMemLimit_Succeeds — boundary: swapLimit == memLimit is accepted
• SwapLimit_LessThanMemLimit_Fails — swapLimit < memLimit → processSwapLimit returns false
• SwapLimit_NonIntegral_Fails — non-integer swapLimit → processSwapLimit returns false

L2 (integration) — tests/L2_testing/test_runner/swap_limit_tests.py

Two end-to-end tests that launch real containers via DobbyTool and read the cgroup value from the container log. Gracefully skips (not fails) on platforms where swapaccount=1 is not set on the kernel cmdline.

• Swap limit default — only memLimit set → memory.memsw.limit_in_bytes == memLimit
• Swap limit override — swapLimit > memLimit → memory.memsw.limit_in_bytes == swapLimit

Backward compatibility

Fully backward compatible. Existing specs that only specify memLimit produce identical OCI output to before.

[Copilot]

Pull request overview

Adds support for configuring container swap limits independently of the memory limit in Dobby specs, updating the OCI template expansion and adding both unit and integration tests to validate the behavior.

Changes:

• Add optional top-level swapLimit to the Dobby JSON spec and plumb it through DobbySpecConfig into a new MEM_SWAP template value.
• Update OCI config templates to use {{MEM_SWAP}} for "memory.swap" while preserving backward-compatible defaulting to memLimit when swapLimit is absent.
• Add L1 GTest coverage for swap-limit parsing/defaulting and L2 end-to-end tests that validate the written cgroup swap limit.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 7 comments.

Show a summary per file

File	Description
bundle/lib/source/DobbySpecConfig.cpp	Adds MEM_SWAP, swapLimit parsing, and defaulting behavior to keep legacy behavior unchanged.
bundle/lib/include/DobbySpecConfig.h	Declares the new processSwapLimit() JSON processor.
bundle/lib/source/templates/OciConfigJson1.0.2-dobby.template	Switches OCI "memory.swap" to use MEM_SWAP.
bundle/lib/source/templates/OciConfigJsonVM1.0.2-dobby.template	Same template change for VM template.
tests/L1_testing/tests/DobbySpecConfigTest/DobbySpecConfigTest.cpp	New unit tests validating default/override and invalid swap-limit cases.
tests/L1_testing/tests/DobbySpecConfigTest/CMakeLists.txt	Adds build configuration for the new L1 test target.
tests/L2_testing/test_runner/swap_limit_tests.py	Adds integration tests for default swap and overridden swap behavior.
tests/L2_testing/dobby_specs/swap_limit.json	New container spec used by the L2 swap limit override test.
README.md	Documents the new swapLimit field in the Dobby spec format section.
rdkPlugins/Networking/source/NetworkingPlugin.cpp	Additional networking behavior changes (bridge check optimization, concurrent iptables apply, interface caching).
rdkPlugins/Networking/include/NetworkingPlugin.h	Adds cached external-interface member.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

add_library(DobbySpecConfigTestLib SHARED STATIC ...) is invalid CMake (a target can't be both SHARED and STATIC). This will fail to configure/build; choose exactly one library type (typically STATIC for unit-test helper libs) and remove the other keyword.

[Copilot]

#define private public is left defined for the rest of the translation unit, which can unintentionally rewrite private to public in subsequently included headers (gtest/gmock/stdlib) and cause hard-to-diagnose build issues. Add #undef private immediately after including DobbySpecConfig.h so the macro’s scope is limited to that include.

[Copilot]

The caching logic can never cache an empty result because it uses if (!mExtIfaces.empty()) as the “already computed” check, and only assigns mExtIfaces when externalIfaces is non-empty. This contradicts the comment (“compute this once”) and means the expensive settings parse + /sys/class/net scan repeats on every call when no interfaces are available. Consider adding a separate mExtIfacesComputed flag (or caching even an empty vector) so the result is memoized regardless of size.

[Copilot]

Using std::async(std::launch::async, ...) here can throw std::system_error (e.g., failure to start a new thread). Since this isn’t caught, the plugin could terminate via exception instead of returning false. Consider wrapping the async creation / future.get() in a try/catch that logs and returns false, or avoid std::async and apply rules sequentially if thread creation fails.

[Copilot]

This “skip” path returns True, which the L2 framework counts as a passed test (count_successes sums passed is True). That makes CI summaries report a pass even when swap accounting is unavailable. If the intent is a real skip, consider extending test_utils.TestResult / output helpers to support an explicit skipped state (so it doesn’t increment passed), or at least adjust the runner’s accounting to treat this case separately.

[Copilot]

The README says memLimit “Must be ≥ 256 KiB”, but the implementation only logs a warning when it’s below 256 KiB and still accepts the value (see processMemLimit in bundle/lib/source/DobbySpecConfig.cpp). Either update the documentation to reflect the warning-only behavior, or enforce the minimum in code so the doc is accurate.

Suggested change

	| `memLimit` | integer | Yes | Memory limit in bytes (`memory.limit_in_bytes`). Must be ≥ 256 KiB. |
	| `memLimit` | integer | Yes | Memory limit in bytes (`memory.limit_in_bytes`). Values below 256 KiB are accepted but only generate a warning and may not be effective. |

[Copilot]

The PR title/description are focused on making swap limits configurable, but this PR also changes NetworkingPlugin behavior (bridge existence check, concurrent iptables apply, and interface caching). Consider moving the networking changes into a separate PR so the swap-limit feature can be reviewed/reverted independently.

[Copilot]

Pull request overview

Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The swapLimit defaulting logic is keyed off the processed flags. If the JSON contains swapLimit but its processor returns false (e.g. non-integral or < memLimit), the flag never gets set and this block will still populate MEM_SWAP from memLimit, producing a seemingly-valid config.json despite a rejected swapLimit. Prefer basing the default on JSON presence (e.g. !mSpec.isMember("swapLimit")) and/or only applying this default when parsing is still successful, so invalid swapLimit values don’t get silently replaced by the default in the generated dictionary.

[Copilot]

This new L2 test group won’t run unless it’s registered by the L2 runner (tests/L2_testing/test_runner/runner.py currently lists supported test modules explicitly). Please ensure swap_limit_tests is imported and added to supported_tests, otherwise these swap-limit integration checks won’t execute in CI.

[Copilot]

The markdown table is using || at the start of each row, which introduces an unintended empty first column and renders oddly on GitHub. Use a single leading | for the header/separator/rows to format the table correctly.

[Copilot]

Pull request overview

Copilot reviewed 16 out of 16 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

tests/L2_testing/test_runner/basic_sanity_tests.py:126

• The new thread-based wait_for_string loop can spin at 100% CPU if the daemon process exits and proc.stderr.readline() starts returning "" (EOF) immediately; since the thread is daemon=True and never terminates on timeout, it can leak/spin for the remainder of the test run. Consider adding EOF/backoff handling and a proper signalling mechanism (e.g., threading.Event) so the reader can stop cleanly on timeout without busy-looping.

    def wait_for_string():
        while True:
            # notice that all data are in stderr not in stdout, this is DobbyDaemon design
            output = proc.stderr.readline()
            if string_to_find in output:
                test_utils.print_log("Found string \"%s\"" % string_to_find, test_utils.Severity.debug)
                return

    found = False
    reader = threading.Thread(target=wait_for_string, daemon=True)
    test_utils.print_log("Starting async read thread", test_utils.Severity.debug)
    reader.start()
    reader.join(timeout)
    # if thread still running
    if reader.is_alive():
        test_utils.print_log("Reader still exists, closing", test_utils.Severity.debug)
        # daemon=True: thread is abandoned and reaped when the process exits
        test_utils.print_log("Not found string \"%s\"" % string_to_find, test_utils.Severity.error)
    else:
        found = True
    return found

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

add_library is declared with both SHARED and STATIC, which is not a valid CMake signature and will fail configuration. Pick exactly one library type (or omit the type if you just need an object library / standard target).

[Copilot]

file(GLOB TESTS *.cpp) will include DobbySpecConfigLinkStubs.cpp, which is already compiled into DobbySpecConfigTestLib. This will produce duplicate symbol/link errors. Exclude the stubs from the glob (or list test sources explicitly).

Suggested change

	file(GLOB TESTS *.cpp)
	file(GLOB TESTS *.cpp)
	list(REMOVE_ITEM TESTS DobbySpecConfigLinkStubs.cpp)

[Copilot]

This new test group is not referenced by tests/L2_testing/test_runner/runner.py (it isn't in supported_tests), so it won't run as part of the normal L2 suite. If these are intended to be CI-enforced, add this module to the runner’s supported_tests list (or whichever discovery mechanism CI uses).

[Copilot]

processSwapLimit accepts any integral JSON value and then uses asUInt(). With JsonCpp, a negative integer is still “integral” and will wrap to a huge unsigned value, bypassing the swap >= memLimit check and setting an unintended cgroup limit. Reject negative values explicitly (e.g., require value.isUInt() / memLimitVal.isUInt()), or validate value.asInt64() >= 0 before converting.

Suggested change

	unsigned memSwap = value.asUInt();
	// the kernel requires memory.memsw.limit_in_bytes >= memory.limit_in_bytes
	const Json::Value& memLimitVal = mSpec["memLimit"];
	if (memLimitVal.isIntegral() && (memSwap < memLimitVal.asUInt()))
	{
	AI_LOG_ERROR("swapLimit (%u) must be >= memLimit (%u)",
	memSwap, memLimitVal.asUInt());
	return false;
	}
	// Validate that swapLimit is not negative before converting to unsigned
	const int64_t memSwapSigned = value.asInt64();
	if (memSwapSigned < 0)
	{
	AI_LOG_ERROR("swapLimit must be non-negative");
	return false;
	}
	// the kernel requires memory.memsw.limit_in_bytes >= memory.limit_in_bytes
	const Json::Value& memLimitVal = mSpec["memLimit"];
	if (memLimitVal.isIntegral())
	{
	const int64_t memLimitSigned = memLimitVal.asInt64();
	if (memLimitSigned < 0)
	{
	AI_LOG_ERROR("memLimit must be non-negative when swapLimit is specified");
	return false;
	}
	if (memSwapSigned < memLimitSigned)
	{
	AI_LOG_ERROR("swapLimit (%lld) must be >= memLimit (%lld)",
	static_cast<long long>(memSwapSigned),
	static_cast<long long>(memLimitSigned));
	return false;
	}
	}
	unsigned memSwap = static_cast<unsigned>(memSwapSigned);