Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Do not allow to add port to .1Q bridge while router port deletion is not completed #2669

Merged
merged 5 commits into from
Feb 22, 2023

Conversation

liorghub
Copy link
Contributor

@liorghub liorghub commented Feb 16, 2023

What I did
Do not allow to add port to .1Q bridge while router port deletion is not completed.

Why I did it
Adding router port to .1Q bridge is not allowed and will cause to errors in the driver.

How I verified it

Create PortChannel102 and set it as router port.
Make sure PortChannel102 has active neighbors.
Now run the following commands using a shell script (not one by one).

  1. config interface ip remove PortChannel102 10.0.0.0/31
  2. config vlan add 1000
  3. config vlan member add 1000 PortChannel102

Follow the logs and verify addBridgePort is not perfomed until removeRouterIntfs is completed.
Unitest added as well.

This PR fixes issue sonic-net/sonic-buildimage#13714

  • 201811
  • 201911
  • 202006
  • 202012
  • 202106
  • 202111
  • 202205
  • 202211
    Details if related

stepanblyschak
stepanblyschak previously approved these changes Feb 16, 2023
@@ -4879,6 +4879,12 @@ bool PortsOrch::addBridgePort(Port &port)
return true;
}

if (port.m_rif_id != 0)
{
SWSS_LOG_NOTICE("Interface %s is a router port", port.m_alias.c_str());
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add more context in the log message and change it to warning since we are returning here in a failure scenario?
"Cannot create bridge port. Interface %s is a router port"

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liorghub I recommend changing it to "WARN" log instead of notice since here are returning here in a failure scenario.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dgsudharsan this is actually a retry mechanism (not failure).
When we print this message we also return false in the next line, DB change is not consumed and we will try again till success.

@liat-grozovik
Copy link
Collaborator

@prsunny any objection to merge?

@prsunny prsunny merged commit baa302e into sonic-net:master Feb 22, 2023
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Feb 23, 2023
Update sonic-swss submodule pointer to include the following:
* baa302e Do not allow to add port to .1Q bridge while router port deletion is not completed  ([sonic-net#2669](sonic-net/sonic-swss#2669))
* f66abed Support for tc-dot1p and tc-dscp qosmap ([sonic-net#2559](sonic-net/sonic-swss#2559))
* 35385ad [RouteOrch] Record ROUTE_TABLE entry programming status to APPL_STATE_DB ([sonic-net#2512](sonic-net/sonic-swss#2512))
* 0704f78 [Workaround] EvpnRemoteVnip2pOrch warmboot check failure ([sonic-net#2626](sonic-net/sonic-swss#2626))
* 4df5cab [ResponsePublisher] add pipeline support  ([sonic-net#2511](sonic-net/sonic-swss#2511))

Signed-off-by: dprital <[email protected]>
prsunny pushed a commit to sonic-net/sonic-buildimage that referenced this pull request Feb 23, 2023
Update sonic-swss submodule pointer to include the following:
* baa302e Do not allow to add port to .1Q bridge while router port deletion is not completed  ([#2669](sonic-net/sonic-swss#2669))
* f66abed Support for tc-dot1p and tc-dscp qosmap ([#2559](sonic-net/sonic-swss#2559))
* 35385ad [RouteOrch] Record ROUTE_TABLE entry programming status to APPL_STATE_DB ([#2512](sonic-net/sonic-swss#2512))
* 0704f78 [Workaround] EvpnRemoteVnip2pOrch warmboot check failure ([#2626](sonic-net/sonic-swss#2626))
* 4df5cab [ResponsePublisher] add pipeline support  ([#2511](sonic-net/sonic-swss#2511))
StormLiangMS pushed a commit that referenced this pull request Mar 7, 2023
…not completed (#2669)

* Do not set port to be a bridge port while it is still a router port
yxieca pushed a commit that referenced this pull request Mar 8, 2023
…not completed (#2669)

* Do not set port to be a bridge port while it is still a router port
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants