Skip to content

deploy updated mt7996e firmware and pin napi/phy CPU affinity on Oran…#1918

Merged
MelvinTo merged 1 commit into
firewalla:masterfrom
jasonlyc:bug_fix
Jun 15, 2026
Merged

deploy updated mt7996e firmware and pin napi/phy CPU affinity on Oran…#1918
MelvinTo merged 1 commit into
firewalla:masterfrom
jasonlyc:bug_fix

Conversation

@jasonlyc

Copy link
Copy Markdown
Contributor

…ge after kernel module is reloaded

@j-sallyjin

Copy link
Copy Markdown

PR Review Summary

✅ What looks good

  • Scope is limited to Orange platform WLAN firmware handling.
  • Firmware files are compared before copy, so reload is avoided when the installed files already match.
  • CPU affinity setup is only attempted after a firmware change and module reload path.

⚠️ Issues found

  • High: sudo rmmod mt7996e; sudo modprobe mt7996e can mask a failed unload. Because ; runs modprobe regardless, exec() may succeed if modprobe exits 0 even when rmmod failed, leaving the old module/firmware active while the code proceeds to pin NAPI threads. Use separate commands with checked results, or &&, and abort affinity setup if unload/reload did not actually happen.
    [platform/orange/OrangePlatform.js:205]

  • Medium: Copy failures can leave a partial firmware bundle installed, but the code still reloads if any earlier file copied successfully. Firmware files should be treated atomically: copy all required files successfully, then reload; otherwise abort and keep the current module running.
    [platform/orange/OrangePlatform.js:196]

  • Medium: NAPI process selection is based on sorting the trailing numeric suffix, then assigning CPU 3/0 to the top two entries. That suffix is not a stable identifier for phy0 vs phy1, so CPU affinity may be applied to the wrong radio after kernel/thread ordering changes. Parse the phy id explicitly from the process name and map affinity by phy0/phy1.
    [platform/orange/OrangePlatform.js:211]

  • Low: Shell commands interpolate paths directly. These filenames are repo-controlled, but using execFile/argument arrays or shell escaping would avoid breakage if paths contain spaces and reduce future command-injection risk.
    [platform/orange/OrangePlatform.js:194]

💡 Suggestions

  • Add a small retry/wait loop after modprobe mt7996e before scanning napi/phy threads; kernel worker creation may race with immediate ps.
  • Consider verifying dstDir exists before copying and logging a clear error if the firmware target path is missing.
  • No linked issue found, so requirement coverage could only be checked against the PR title/body.

Verdict

REQUEST_CHANGES


Repo: firewalla/firerouter
PR: #1918
Head SHA: 34823bd57047158ff79fc3c9a29006d30de319df
Checked at: 2026-06-15 14:58:26 CST

@MelvinTo MelvinTo merged commit 4eb50ac into firewalla:master Jun 15, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants