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

Postpone intf task after buffer configuration applied on the port. #539

Merged
merged 1 commit into from
Jul 25, 2018

Conversation

jipanyang
Copy link
Contributor

This also ensures all port setting applied before intf setting.

Signed-off-by: Jipan Yang [email protected]

What I did
Don't apply intf setting on a port before buffer applied on the port.
The order of orches in m_orchList guaranteed that for pending tasks, buffer is processed before port, port before intf.

Why I did it
There is racing condition that, even when "PortInitDone" has been received and m_initDone set true, buffer configuration hasn't been applied.

https://github.com/Azure/sonic-swss/blob/dcf6c905410f08b004e880dbed56823d29e7bd5e/orchagent/portsorch.cpp#L1360

mtu setting for port will be postponed, while IntfsOrch::doTask() only checks gPortsOrch->isInitDone(). For router interface creation, it uses default port mtu which may be different from the one to be configured.

Seen with VS, potentially also may happen on hardware box.

swss.rec, check Ethernet4

2018-07-19.03:38:11.402835|PORT_TABLE:Ethernet4|SET|admin_status:up|mtu:1500
2018-07-19.03:38:11.402840|PORT_TABLE:Ethernet112|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402851|PORT_TABLE:Ethernet100|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402857|PORT_TABLE:Ethernet124|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402862|PORT_TABLE:Ethernet88|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402867|PORT_TABLE:Ethernet76|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402872|PORT_TABLE:Ethernet84|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402877|PORT_TABLE:Ethernet8|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402882|PORT_TABLE:Ethernet96|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402887|PORT_TABLE:Ethernet68|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402892|PORT_TABLE:Ethernet44|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402897|PORT_TABLE:Ethernet48|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402902|PORT_TABLE:Ethernet24|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402907|PORT_TABLE:Ethernet12|SET|admin_status:down|mtu:1500
2018-07-19.03:38:11.402912|PORT_TABLE:Ethernet16|SET|admin_status:down|mtu:1500

2018-07-19.03:38:11.402916|PORT_TABLE:PortInitDone|SET|lanes:0

2018-07-19.03:38:12.450151|INTF_TABLE:Ethernet4:10.0.0.2/31|SET|scope:global|family:IPv4

Also syslog:

....

Jul 19 03:38:11.402958 91fd31fb460f INFO /orchagent: :- doPortTask: Get PortInitDone notification from portsyncd.


Jul 19 03:38:11.403686 91fd31fb460f NOTICE /orchagent: :- addIp2MeRoute: Create IP2me route ip:10.0.0.0
Jul 19 03:38:11.403979 91fd31fb460f NOTICE /orchagent: :- addRouterIntfs: Create router interface for port Ethernet4 mtu 9100          <----- default MTU
....


Jul 19 03:38:11.416971 91fd31fb460f NOTICE /orchagent: :- doPortTask: Set port Ethernet4 MTU to 1500           <---- configured MTU

How I verified it
VS testing.

Details if related

This also ensures all port setting applied before intf setting.

Signed-off-by: Jipan Yang <[email protected]>
@lguohan lguohan merged commit 651bedb into sonic-net:master Jul 25, 2018
lguohan pushed a commit that referenced this pull request Jul 28, 2018
)

This also ensures all port setting applied before intf setting.

Signed-off-by: Jipan Yang <[email protected]>
@jipanyang jipanyang deleted the buffer_before_intf branch February 9, 2019 02:31
oleksandrivantsiv pushed a commit to oleksandrivantsiv/sonic-swss that referenced this pull request Mar 1, 2023
* Move ntf_queue to proper NotificationQueue class

* add extra aspell exception

* Address comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants