Skip to content

fix(saiserver): preserve hybrid port VLAN memberships across tagging modes - #755

Open
mohankarthik wants to merge 1 commit into
openconfig:mainfrom
mohankarthik:fix-hybrid-vlan-member
Open

mohankarthik wants to merge 1 commit into
openconfig:mainfrom
mohankarthik:fix-hybrid-vlan-member

Conversation

@mohankarthik

@mohankarthik mohankarthik commented Oct 13, 2026 •

Copy link
Copy Markdown
Contributor

Summary

Follow-up to #750 fixing hybrid and multi-VLAN port handling, RemoveVlanMember cleanup, and vlan map thread safety on main:

  1. Synchronize vlan Map Access (vlan.mu):
    • Protect all reads and writes to vlan.members and vlan.vlans (vidByOid, oidByVid, memberByOid, memberByPortId, untaggedMemberByPortId, hasTaggedMember, CreateVlan, CreateVlanMember, RemoveVlanMember, Reset) with vlan.mu (sync.RWMutex).
  2. Preserve Existing VLAN Memberships on Hybrid & Multi-VLAN Trunk Ports (untaggedMemberByPortId):
    • Previously, CreateVlanMember called memberByPortId(portID) and deleted any existing VLAN membership on the port before adding the new one.
    • Replace memberByPortId eviction with untaggedMemberByPortId(portID) and only evict when the new membership is UNTAGGED / PRIORITY_TAGGED, allowing multiple TAGGED VLANs (trunk port) and one UNTAGGED native VLAN + TAGGED VLANs (hybrid port) to coexist regardless of creation order.
  3. Update RemoveVlanMember Lifecycle:
    • Remove the deleted member OID from the parent VLAN's MemberList attribute in attrmgr.
    • When memberships remain on the port after removal, reprogram VlanTable according to the remaining memberships (restoring untagged Encap(ETHERNET_VLAN) + Update(VLAN_TAG) actions when the last TAGGED membership is removed from a hybrid port); when no memberships remain on the port, remove its VlanTable entry via TableEntryRemove.
  4. Unit Tests (vlan_test.go):
    • Fix && -> || assertion bug in TestVlanOperations.
    • Add 5 subtests in TestCreateVlanMemberHybridPort covering UNTAGGED->TAGGED, TAGGED->UNTAGGED, multi-VLAN TAGGED trunk ports, untagged VLAN replacement on a hybrid port, and tagged/untagged member removal.

@mohankarthik
mohankarthik requested a review from a team as a code owner October 13, 2026 15:56
@mohankarthik
mohankarthik force-pushed the fix-hybrid-vlan-member branch from 9188d3b to dce6113 Compare October 13, 2026 17:55
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.

1 participant