Skip to content

RDKEMW-15176: set swap memory limit for containers #419

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
goruklu wants to merge 1 commit into
base: release/v3.14
Choose a base branch
from
Open

RDKEMW-15176: set swap memory limit for containers #419

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions bundle/lib/source/DobbySpecConfig.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,10 @@ static const ctemplate::StaticTemplateString USERNS_DISABLED =

static const ctemplate::StaticTemplateString MEM_LIMIT =
STS_INIT(MEM_LIMIT, "MEM_LIMIT");
static const ctemplate::StaticTemplateString MEM_SWAP_LIMIT =
STS_INIT(MEM_SWAP_LIMIT, "MEM_SWAP_LIMIT");

static constexpr unsigned MEM_SWAP_LIMIT_EXTRA_BYTES = 200u * 1024u * 1024u;
Comment on lines +67 to +68
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEM_SWAP_LIMIT_EXTRA_BYTES introduces a hard-coded 200MB policy with no rationale or configurability; this makes the runtime behavior difficult to tune across devices. Consider documenting why 200MB is required (and what it represents) or making it a configurable setting tied to platform defaults.

Suggested change
static constexpr unsigned MEM_SWAP_LIMIT_EXTRA_BYTES = 200u * 1024u * 1024u;
// Additional swap limit headroom (in bytes) added on top of MEM_LIMIT.
// The default of 200MB has historically been used as a conservative value
// to account for kernel overhead and transient memory spikes on typical
// target devices. If porting to hardware with significantly different
// memory characteristics, this value can be overridden at build time by
// defining MEM_SWAP_LIMIT_EXTRA_BYTES (for example via
// -DMEM_SWAP_LIMIT_EXTRA_BYTES=<bytes> in CXXFLAGS).
#ifndef MEM_SWAP_LIMIT_EXTRA_BYTES
static constexpr unsigned MEM_SWAP_LIMIT_EXTRA_BYTES = 200u * 1024u * 1024u;
#endif

Copilot uses AI. Check for mistakes.

static const ctemplate::StaticTemplateString CPU_SHARES_ENABLED =
STS_INIT(CPU_SHARES_ENABLED, "CPU_SHARES_ENABLED");
Expand Down Expand Up @@ -1274,6 +1278,7 @@ bool DobbySpecConfig::processMemLimit(const Json::Value& value,
}

Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MEM_SWAP_LIMIT is computed as memLimit + MEM_SWAP_LIMIT_EXTRA_BYTES with unsigned arithmetic; if a caller supplies a large memLimit this can overflow and silently wrap, producing an invalid/too-small swap limit. Consider validating an upper bound (e.g., reject values where memLimit > UINT_MAX - MEM_SWAP_LIMIT_EXTRA_BYTES) and/or switching to a string/64-bit safe path when writing to the template dictionary.

Suggested change
// Prevent unsigned overflow when computing the swap limit
if (memLimit > (UINT_MAX - MEM_SWAP_LIMIT_EXTRA_BYTES))
{
AI_LOG_ERROR("memory limit too large, would overflow swap limit");
return false;
}

Copilot uses AI. Check for mistakes.
dictionary->SetIntValue(MEM_LIMIT, memLimit);
dictionary->SetIntValue(MEM_SWAP_LIMIT, memLimit + MEM_SWAP_LIMIT_EXTRA_BYTES);

return true;
}
Expand Down
4 changes: 3 additions & 1 deletion bundle/lib/source/templates/OciConfigJson1.0.2-dobby.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -327,7 +327,9 @@ static const char* ociJsonTemplate = R"JSON(
{{/DEV_WHITELIST_SECTION}}
],
"memory": {
"limit": {{MEM_LIMIT}}
"limit": {{MEM_LIMIT}},
"swap": {{MEM_SWAP_LIMIT}},
"swappiness": 60
Comment on lines +330 to +332
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This template sets swap greater than limit (MEM_SWAP_LIMIT is derived from MEM_LIMIT plus extra bytes). That means the container can exceed the memory limit by swapping, so it will not be OOM-killed at the point it reaches the memory limit (which conflicts with the PR title/description). If the goal is to trigger OOM at the configured memory limit when host swap is enabled, consider setting swap equal to the memory limit (cgroup v1 semantics) or otherwise aligning the swap value with the intended kill threshold.

Copilot uses AI. Check for mistakes.
Comment on lines +331 to +332
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"swappiness": 60 is a new, hard-coded behavioral change that will affect memory reclamation/swap behavior for all containers using this template. If this value is required for the OOM-kill fix, consider making it configurable (template placeholder) or omitting it to keep existing default behavior, and document the intended impact.

Suggested change
"swap": {{MEM_SWAP_LIMIT}},
"swappiness": 60
"swap": {{MEM_SWAP_LIMIT}}{{#MEM_SWAPPINESS_SECTION}},
"swappiness": {{MEM_SWAPPINESS}}{{/MEM_SWAPPINESS_SECTION}}

Copilot uses AI. Check for mistakes.
},
"cpu": {
{{#CPU_SHARES_ENABLED}}
Expand Down
4 changes: 3 additions & 1 deletion bundle/lib/source/templates/OciConfigJsonVM1.0.2-dobby.template
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,9 @@ static const char* ociJsonTemplate = R"JSON(
{{/DEV_WHITELIST_SECTION}}
],
"memory": {
"limit": {{MEM_LIMIT}}
"limit": {{MEM_LIMIT}},
"swap": {{MEM_SWAP_LIMIT}},
"swappiness": 60
Comment on lines +341 to +343
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This template sets swap greater than limit (MEM_SWAP_LIMIT is derived from MEM_LIMIT plus extra bytes). That means the container can exceed the memory limit by swapping, so it will not be OOM-killed at the point it reaches the memory limit (which conflicts with the PR title/description). If the goal is to trigger OOM at the configured memory limit when host swap is enabled, consider setting swap equal to the memory limit (cgroup v1 semantics) or otherwise aligning the swap value with the intended kill threshold.

Copilot uses AI. Check for mistakes.
Comment on lines +342 to +343
Copy link

Copilot AI Mar 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"swappiness": 60 is a new, hard-coded behavioral change that will affect memory reclamation/swap behavior for all containers using this template. If this value is required for the OOM-kill fix, consider making it configurable (template placeholder) or omitting it to keep existing default behavior, and document the intended impact.

Suggested change
"swap": {{MEM_SWAP_LIMIT}},
"swappiness": 60
"swap": {{MEM_SWAP_LIMIT}}{{#MEM_SWAPPINESS_ENABLED}},
"swappiness": {{MEM_SWAPPINESS}}{{/MEM_SWAPPINESS_ENABLED}}

Copilot uses AI. Check for mistakes.
},
"cpu": {
{{#CPU_SHARES_ENABLED}}
Expand Down
Loading