Do basic mutual exclusion check on vlan member and router interface c… #770
Conversation
…onfig for same port Signed-off-by: Jipan Yang <[email protected]>
| if (port.m_bridge_port_id != SAI_NULL_OBJECT_ID) | ||
| { | ||
| SWSS_LOG_ERROR("Router Intfs config on vlan member %s is not allowed", alias.c_str()); | ||
| it = consumer.m_toSync.erase(it); |
There was a problem hiding this comment.
I don't think we should block and discard the configuration. Instead, why don't we retry here?
There was a problem hiding this comment.
The concern is that performing retry here will introduce too many error logs and extra system overhead if the error is persistent for a long time. While lowering the log level will leave the error unnoticed most likely.
| @@ -1972,6 +1972,13 @@ void PortsOrch::doVlanMemberTask(Consumer &consumer) | |||
| continue; | |||
| } | |||
|
|
|||
| if (port.m_rif_id) | |||
| { | |||
| SWSS_LOG_ERROR("Vlan config on router interface %s is not allowed", port.m_alias.c_str()); | |||
|
it is strange that this PR changes vnet test cases. |
|
The test case should be added to check the syslog error message for invalid configuration. I do not see such test cases added. |
…ck in orchagent. Signed-off-by: Jipan Yang <[email protected]>
Before the change "100.102.1.1/24" was configured on "Ethernet4" first, then the second test case tries to add Ethernet4 to Vlan1002. It hit the exact problem of one port being put in both rif and vlan. |
|
retest this please |
sonic-net#770) * A generic JSON file updater, which can add/update-existing attributes. This tool would be used to update /etc/sonic/core_analyzer.rc.json file to add credentials by HW proxy. * Updated per review comments. The option is better named.
Missing MACsec Create Port action in SwitchStateBase::create. This bug will cause MACsec SC Crate action fail. Signed-off-by: Ze Gan <[email protected]>
…onfig for same port
Signed-off-by: Jipan Yang [email protected]
What I did
Eventually cfgmgr should prevent such configuration being pushed to appDB. Here just to do basic preventive check in orchagent.
Why I did it
To avoid wrong configuration from messing up orchagent state handling.
How I verified it
VS
Details if related