[Workaround for XSight SAI] Updated VLAN member task to improve logic for removing VLAN members and handling bridge ports - #145
Conversation
…nd handling bridge ports. Ensured proper handling of FDB entries during bridge port removal.
chenkelly
changed the title
There was a problem hiding this comment.
Pull Request Overview
This PR updates the VLAN member task to improve logic for removing VLAN members and handling bridge ports, specifically for the xsight platform. The changes add platform-specific behavior to handle limitations in XSAI FDB flush notification support.
- Added platform checking to conditionally handle VLAN member removal
- Implemented different bridge port removal logic for xsight platform
- Added platform-specific FDB entry handling during bridge port removal
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| else if (op == DEL_COMMAND) | ||
| { | ||
| if (getPort(vlan_alias, vlan) && vlan.m_members.find(port_alias) != vlan.m_members.end()) | ||
| string platform = getenv("platform") ? getenv("platform") : ""; |
There was a problem hiding this comment.
The platform string is retrieved multiple times in the same method (lines 4684 and 5905). Consider retrieving it once at the beginning of the method and reusing the variable to avoid redundant environment variable lookups.
| if (getPort(vlan_alias, vlan) && vlan.m_members.find(port_alias) != vlan.m_members.end()) | ||
| string platform = getenv("platform") ? getenv("platform") : ""; | ||
|
|
||
| if (platform == XS_PLATFORM_SUBSTRING) |
There was a problem hiding this comment.
The constant XS_PLATFORM_SUBSTRING is used but not defined in this diff. Ensure this constant is properly defined and consider using a more descriptive name that clearly indicates it's checking for the xsight platform.
| if (getPort(vlan_alias, vlan) && vlan.m_members.find(port_alias) != vlan.m_members.end()) | ||
| { | ||
| it++; | ||
| continue; | ||
| if (removeVlanMember(vlan, port)) | ||
| { | ||
| if (m_portVlanMember[port.m_alias].empty()) | ||
| { | ||
| removeBridgePort(port); | ||
| } | ||
| it = consumer.m_toSync.erase(it); | ||
| } | ||
| else | ||
| { | ||
| it++; | ||
| } | ||
| } | ||
| else | ||
| /* Cannot locate the VLAN */ | ||
| it = consumer.m_toSync.erase(it); |
There was a problem hiding this comment.
The platform-specific logic duplicates the existing logic below (lines 4709-4728) with only minor differences. Consider refactoring this into a common function that takes platform-specific behavior as parameters to reduce code duplication.
What I did
Sync 2838438
Perform platform checking because XSAI currently lacks FDB flush notification support.
Ensured proper handling of FDB entries during bridge port removal.