Handle default VLAN create and delete #929
Conversation
|
|
orchagent/portsorch.h
Outdated
| @@ -110,7 +110,8 @@ class PortsOrch : public Orch, public Subject | |||
| Port m_cpuPort; | |||
| // TODO: Add Bridge/Vlan class | |||
| sai_object_id_t m_default1QBridge; | |||
| sai_object_id_t m_defaultVlan; | |||
| sai_object_id_t m_defaultVlan_ObjId; | |||
There was a problem hiding this comment.
Thanks for submitting the PR. The logic looks valid to me. Could you modify the type of the current m_defaultVlan to type Port? Then you could use m_vlan_info to store both the oid and the id of the VLAN. This would prevent having more private variables.
|
retest this please |
|
@santoshdoke @chenkelly could you check my comments on the pull request? |
| } | ||
|
|
||
| m_defaultVlan.m_vlan_info.vlan_id = attr.value.u16; | ||
|
|
There was a problem hiding this comment.
could you put the default VLAN into the m_portList? Then we don't need to handle special case in the function addVlan.
| @@ -2578,7 +2592,17 @@ bool PortsOrch::addVlan(string vlan_alias) | |||
| sai_attribute_t attr; | |||
| attr.id = SAI_VLAN_ATTR_VLAN_ID; | |||
| attr.value.u16 = vlan_id; | |||
| sai_status_t status = sai_vlan_api->create_vlan(&vlan_oid, gSwitchId, 1, &attr); | |||
| sai_status_t status = SAI_STATUS_SUCCESS; | |||
There was a problem hiding this comment.
If the default VLAN is in the m_portList, there's no need to modify this function.
| sai_status_t status = sai_vlan_api->remove_vlan(vlan.m_vlan_info.vlan_oid); | ||
| if (status != SAI_STATUS_SUCCESS) | ||
| /* Do not delete default VLAN from driver, but clear internal state */ | ||
| if (vlan.m_vlan_info.vlan_id != m_defaultVlan.m_vlan_info.vlan_id) |
There was a problem hiding this comment.
Move this check into the function doVlanTask() before removeVlan() function so that the this function's purpose is not touched. Add add a warning if the to-be-removed VLAN is the default VLAN, since it is not supposed to be removed.
|
retest this please |
|
can you add vs test to verify the app db to asic db translate is protected for vlan 1 case? |
What I did
Handle default VLAN creation/deletion in SWSS orch agent.
Why I did it
The default VLAN is created in SAI/driver during switch initialization. When user tries to create the default VLAN, OrchAgent detects error from SyncD and terminates. Reason is the VLAN already exists. To avoid that, handling the case of default VLAN creation/deletion and skipped the driver call.
How I verified it
Details if related