Fix cross-bridge VLAN state corruption in bridge port synchronization

[rtrappman-dev]

Summary

Fix a cross-bridge VLAN state synchronization bug introduced by FireRouter PR #2055.

bridge monitor link reports link events globally across the network namespace. The previous implementation processed VLAN events from other bridges without verifying that the VLAN's underlying physical interface belonged to the BridgePortStateSync instance handling the event.

As a result, an STP state change observed on one bridge could cause VLAN interfaces associated with another bridge to have their port state changed.

Problem

Consider a topology such as:

• br0 → eth0 → eth0.100
• br1 → eth1 → eth1.100

Each bridge has its own BridgePortStateSync instance, but bridge monitor link is namespace-wide.

When an event for eth1.100 was received by the br0 state-sync instance, the code could:

1. Extract eth1 as the physical interface.
2. Read the native bridge state of eth1.
3. Schedule synchronization for eth1.
4. Enumerate and modify VLAN interfaces beginning with eth1..

This allowed one bridge's synchronization logic to operate on VLAN ports belonging to another bridge.

Fix

BridgePortStateSync now records the physical interfaces that belong to its associated native bridge when the monitor starts.

For VLAN events associated with another bridge, synchronization is performed only when the VLAN's underlying physical interface is a member of the current state-sync instance.

This preserves the intended PR #2055 behavior of correcting VLAN port-state drift while preventing cross-bridge interference.

Testing

Added a regression test covering:

• VLAN events for a physical interface outside the monitored bridge are ignored.
• No native bridge state lookup or synchronization is performed for unrelated interfaces.
• VLAN events for a physical interface belonging to the monitored bridge continue to trigger synchronization.

Impact

This prevents unintended modification of VLAN bridge-port state across independent bridges and preserves bridge/VLAN forwarding isolation.

No unrelated networking behavior is changed.

[rtrappman-dev]

@jasonlyc A regression fix for #2055

[j-sallyjin]

PR Review Summary

✅ What looks good

• Correctly scopes namespace-wide bridge monitor link events to the current bridge’s physical members.
• Membership is refreshed whenever monitoring starts, matching configuration re-apply behavior.
• Filtering occurs before asynchronous state lookup or scheduling, avoiding unnecessary work and cross-bridge mutations.
• Regression test covers both rejection of unrelated events and continued handling of valid events.
• Existing CI build passes.
• No linked issue found. PR correct vlan port state if it drifts #2055 is referenced for context but is not an issue.

⚠️ Issues found

No blocking correctness, security, concurrency, or performance issues found.

💡 Suggestions

• Consider testing through startMonitor() or adding a focused assertion that it populates _memberIntfs. The current test directly assigns the private field, so a future regression in the production wiring would not be detected.
• Optionally add coverage for restarting the monitor with a changed member list, confirming stale members are removed.

Verdict

APPROVE

---

Repo: firewalla/firerouter
PR: #2060
Head SHA: ca729d6eb909b9bca65f6ee7107920f45da4928f
Checked at: 2026-09-05 02:02:41 CST

[j-sallyjin]

PR Review Summary

✅ What looks good

• Correctly scopes namespace-wide bridge monitor link events to the current bridge’s member interfaces.
• Membership is refreshed whenever monitoring restarts, avoiding stale bridge configuration.
• The filter runs before asynchronous state lookup and synchronization, preventing cross-bridge side effects and unnecessary I/O.
• Regression coverage verifies both rejection of unrelated VLAN events and continued handling of valid events.
• No security, concurrency, performance, or resource-management regressions found.
• No linked GitHub issue found; PR correct vlan port state if it drifts #2055 is referenced for context but is not an issue.

⚠️ Issues found

None.

💡 Suggestions

• Consider adding coverage through startMonitor(memberIntfs) rather than directly assigning private fields. This would verify that the production initialization path populates _memberIntfs correctly and make the regression test less coupled to implementation details.

Verdict

APPROVE

---

Repo: firewalla/firerouter
PR: #2060
Head SHA: fbc44d4b33abc2556cbf0d22a1ce69d996b63653
Checked at: 2026-09-11 01:26:03 CST

[j-sallyjin]

PR Review Summary

✅ What looks good

• Correctly scopes namespace-wide bridge monitor link VLAN events to the current bridge’s member interfaces.
• Filters unrelated events before asynchronous state lookup and synchronization, preventing cross-bridge mutations and unnecessary I/O.
• Replaces the membership set whenever monitoring restarts, avoiding stale configuration.
• Regression coverage verifies both rejection of unrelated events and continued handling of valid member events.
• No correctness, security, concurrency, performance, or resource-management regressions found.
• No linked GitHub issue found; PR correct vlan port state if it drifts #2055 is a predecessor pull request, not an issue.

⚠️ Issues found

None.

💡 Suggestions

• Consider testing that startMonitor() populates _memberIntfs. The current test assigns this internal field directly, so it would not detect a regression in the production wiring.
• Optionally test restarting the monitor with a changed member list to verify removed members are no longer accepted.

Verdict

APPROVE

---

Repo: firewalla/firerouter
PR: #2060
Head SHA: fbc44d4b33abc2556cbf0d22a1ce69d996b63653
Checked at: 2026-09-11 01:26:05 CST