no stp on vlan

[melo-crypto]

https://github.com/firewalla/firecommit/issues/8198

[j-sallyjin]

PR Review Summary

✅ What looks good

• The change is small and targets the right area: bridge creation/STP enablement in plugins/interface/bridge_intf_plugin.js.
• For pure VLAN bridges like eth0.10 + eth1.10, the PR prevents enabling MSTP/STP on that bridge, which addresses the “do not send STP packet with VLAN tag” part of the linked issue.
• Existing native untagged bridges with multiple physical interfaces still keep the previous STP behavior.

⚠️ Issues found

• Medium: linked issue is only partially covered.
  The linked issue firewalla/firecommit#8198 requires:

  • STP packets should not be sent with VLAN tags: Met for pure VLAN bridges
  • STP should only be sent on native untagged LAN: Mostly met
  • All LAN port status should stay consistent with native LAN: Not addressed

  The PR disables STP on VLAN bridges, but it does not mirror native bridge/STP port state into the corresponding VLAN bridge ports. If the native LAN blocks or transitions a port, VLAN bridge members may still remain forwarding unless another part of the system synchronizes those states.

• Medium: VLAN detection is too broad.
  i.includes('.') treats any interface name containing a dot as VLAN. That likely matches current VLAN naming, but it is not a strict VLAN-subinterface check. A helper such as isVlanSubInterface() or checking known interface plugin metadata would make this less brittle.

• Low: mixed native/VLAN bridge behavior is unclear.
  If a bridge ever contains both untagged and VLAN subinterfaces, every(i => i.includes('.')) returns false and STP remains enabled for the whole bridge. That could still produce tagged STP on VLAN members.

💡 Suggestions

• Add explicit handling for VLAN bridge port state consistency with the native LAN bridge, or point to the existing mechanism if it already exists elsewhere.
• Replace the inline includes('.') heuristic with a named helper so the intended VLAN interface convention is documented and reusable.
• Consider adding a regression test or config-level test for:
  • pure native bridge: STP enabled
  • pure VLAN bridge: STP not enabled
  • mixed interface list: expected behavior defined explicitly

Verdict

REQUEST_CHANGES

---

Repo: firewalla/firerouter
PR: #1907
Head SHA: d7d07b3dfbe1b8655107cd457b2eb977e54a3efe
Checked at: 2026-06-09 11:00:54 CST