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

[Dynamic Buffer Calc] Don't update pools when ingress_lossless_pool is created during initialization #1685

Merged
merged 1 commit into from
Apr 1, 2021

Conversation

stephenxs
Copy link
Collaborator

@stephenxs stephenxs commented Mar 30, 2021

What I did
Fix bug: Don't update pools when ingress_lossless_pool is created during initialization.

  • This bug increases the converging time of fast reboot for ~5 seconds.
  • It can also cause error logs during system startup:
    Mar 29 11:57:32.640257 mtbc-sonic-03-2700 ERR swss#orchagent: message repeated 19408 times: [ :- resolveFieldRefArray: Failed to parse profile reference:[BUFFER_PROFILE_TABLE:ingress_lossy_profile]]
    

Signed-off-by: Stephen Sun [email protected]

Why I did it
Fix the bug.

How I verified it
Run regression and vs test.

Details if related
The buffer pools will be created after a short-timer and then won't be updated until the initialization has finished. The idea is to call recalculateSharedBufferPool from checkSharedBufferPoolSize only once during initialization. The motivation is to avoid repeatedly updating buffer pools when items in BUFFER_PG have been created for each port. So the flow we expected:

1. Start a short-timer
2. The buffer pools are being configured
3. On timer timeout: create all the buffer pools to APPL_DB.
   The buffer pool size can be inaccurate but it doesn't matter since the system is starting.
   The motivation is to create the buffer pool in APPL_DB and ASIC_DB, which paves the way for other buffer tables to be created.
4. Handle other buffer tables: BUFFER_PROFILE, BUFFER_PG, BUFFER_QUEUE, BUFFER_PORT_INGRESS_PROFILE_LIST_TABLE, and BUFFER_PORT_INGRESS_PROFILE_LIST_TABLE
5. On initialization done, update buffer pools to APPL_DB. Now the buffer pools will be updated with the accurate size

However, when the ingress_lossless_pool is created it will trigger the buffer pools to be updated for the shared headroom pool. If this happens during initialization, it causes the buffer pools created after ingress_lossless_pool unable to be created until the initialization has been done because the buffer pools can only be updated to APPL_DB once during initialization.
For example, assume the buffer pools are created in the following order:

1. egress_lossless_pool
2. egress_lossy_pool
3. ingress_lossless_pool
4. ingress_lossy_pool

In step 3, it won't call recalculateSharedBufferPool because it has been called when handling ingress_lossless_pool. This causes ingress_lossy_pool which follows ingress_lossless_pool can not be created until initialization finished, which in turn causes the entries depending on ingress_lossy_pool can not be created, and there will be error logs reported by bufferorch.
Fix: Don't trigger updating buffer pools when creating ingress_lossless_pool during initialization.

…ialization hasn't finished yet

Bug:
The buffer pools will be created after a short timer and then won't be
updated until the initialization has finished. This is to avoid
repeatedly updating buffer pools after BUFFER_PG and BUFFER_QUEUE have
been created for each port.
However, when the ingress_lossless_pool is created it will trigger the
buffer pools to be updated for shared headroom pool. If this happens
during intialization, it causes all the buffer pools created after
ingress_lossless_pool won't be created until the initialization has been
done.
Fix:
Don't trigger updating buffer pools when creating ingress_lossless_pool
during intialization.

Signed-off-by: Stephen Sun <[email protected]>
@liat-grozovik liat-grozovik merged commit aac71e6 into sonic-net:master Apr 1, 2021
@stephenxs stephenxs deleted the fix-pool-init branch April 1, 2021 10:29
@dprital
Copy link
Collaborator

dprital commented Apr 7, 2021

@daall - Can you please merge into 202012 ?

daall pushed a commit that referenced this pull request Apr 13, 2021
…ialization hasn't finished yet (#1685)

Bug:
The buffer pools will be created after a short timer and then won't be
updated until the initialization has finished. This is to avoid
repeatedly updating buffer pools after BUFFER_PG and BUFFER_QUEUE have
been created for each port.
However, when the ingress_lossless_pool is created it will trigger the
buffer pools to be updated for shared headroom pool. If this happens
during intialization, it causes all the buffer pools created after
ingress_lossless_pool won't be created until the initialization has been
done.
Fix:
Don't trigger updating buffer pools when creating ingress_lossless_pool
during intialization.

Signed-off-by: Stephen Sun <[email protected]>
liat-grozovik pushed a commit that referenced this pull request Apr 27, 2021
- What I did
This is a supplement of #1685. In that PR, most of the cases in which the buffer pool updated during initialization have been avoided but there are still some rare cases not covered. Eg., it will try to recalculate the pool size because the lossless PSs are removed if one port is admin down.

- Why I did it
Make sure that the the buffer pools will be updated only once during initialization.

- How I verified it
Run regression and manual test

Signed-off-by: Stephen Sun <[email protected]>
daall pushed a commit that referenced this pull request Apr 29, 2021
- What I did
This is a supplement of #1685. In that PR, most of the cases in which the buffer pool updated during initialization have been avoided but there are still some rare cases not covered. Eg., it will try to recalculate the pool size because the lossless PSs are removed if one port is admin down.

- Why I did it
Make sure that the the buffer pools will be updated only once during initialization.

- How I verified it
Run regression and manual test

Signed-off-by: Stephen Sun <[email protected]>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
…ialization hasn't finished yet (sonic-net#1685)

Bug:
The buffer pools will be created after a short timer and then won't be
updated until the initialization has finished. This is to avoid
repeatedly updating buffer pools after BUFFER_PG and BUFFER_QUEUE have
been created for each port.
However, when the ingress_lossless_pool is created it will trigger the
buffer pools to be updated for shared headroom pool. If this happens
during intialization, it causes all the buffer pools created after
ingress_lossless_pool won't be created until the initialization has been
done.
Fix:
Don't trigger updating buffer pools when creating ingress_lossless_pool
during intialization.

Signed-off-by: Stephen Sun <[email protected]>
raphaelt-nvidia pushed a commit to raphaelt-nvidia/sonic-swss that referenced this pull request Oct 5, 2021
- What I did
This is a supplement of sonic-net#1685. In that PR, most of the cases in which the buffer pool updated during initialization have been avoided but there are still some rare cases not covered. Eg., it will try to recalculate the pool size because the lossless PSs are removed if one port is admin down.

- Why I did it
Make sure that the the buffer pools will be updated only once during initialization.

- How I verified it
Run regression and manual test

Signed-off-by: Stephen Sun <[email protected]>
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.

5 participants