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

[Mellanox] Support one ingress pool mode #4686

Merged
merged 1 commit into from
Aug 13, 2020

Conversation

stephenxs
Copy link
Collaborator

@stephenxs stephenxs commented Jun 2, 2020

Support one ingress pool mode.

Signed-off-by: Stephen Sun stephens@mellanox.com

- Why I did it
Support one ingress pool mode

- How I did it

- How to verify it

- Description for the changelog

- A picture of a cute animal (not mandatory but encouraged)

@liat-grozovik
Copy link
Collaborator

retest vsimage please

Copy link
Collaborator

@baiwei0427 baiwei0427 left a comment

Choose a reason for hiding this comment

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

5570496 ingress shared buffer size for T1 still seems too small for me

@stephenxs stephenxs marked this pull request as ready for review July 9, 2020 03:43
@neethajohn
Copy link
Contributor

According to the SAI spec, the buffer pool size includes the reserved buffer for all pg/queues. So that would mean we would have the same pool size specified for T1 and T0 templates and internally subtract the reserved space to get the actual shared pool size to allocate.
This is followed by other ASICs which do not include a headroom pool
https://github.com/opencomputeproject/SAI/blob/master/doc/QOS/SAI-Proposal-buffers-Ver4.docx

Copy link
Contributor

@neethajohn neethajohn left a comment

Choose a reason for hiding this comment

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

I see 'ingress_lossy_profile' being defined in the templates but not used anywhere. Is that intentional?

build_image.sh Outdated Show resolved Hide resolved
files/image_config/secureboot/test-certs/signing.cert Outdated Show resolved Hide resolved
files/image_config/secureboot/test-certs/ca.cert Outdated Show resolved Hide resolved
files/image_config/secureboot/test-certs/signing.key Outdated Show resolved Hide resolved
@stephenxs
Copy link
Collaborator Author

ingress_lossy_profile
Hi,
ingress_lossy_profile is referenced in BUFFER_PG|<port>|0. Please check sonic-buildimage/build_templates/buffers_config.j2

@baiwei0427
Copy link
Collaborator

Do you have buffer templates for 2700 C32 sku?

Copy link
Collaborator

@baiwei0427 baiwei0427 left a comment

Choose a reason for hiding this comment

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

Why different SKUs get different Xoff sizes under the same link speed and cable length?

@stephenxs
Copy link
Collaborator Author

stephenxs commented Aug 4, 2020

Why different SKUs get different Xoff sizes under the same link speed and cable length?

If they are based on different ASICs, they can have different xoff sizes because of different cell sizes.
If you're not meaning this, can you elaborate it a bit more?

@stephenxs stephenxs closed this Aug 4, 2020
@stephenxs stephenxs reopened this Aug 4, 2020
@stephenxs
Copy link
Collaborator Author

stephenxs commented Aug 4, 2020

Do you have buffer templates for 2700 C32 sku?

Yes. All the buffer templates of SKUs based 2700 share the same templates.
We can provide the specific template for C32 if you provide the cable lengths of ports for t0/t1.
Are they:

  • t0: 28 * server ports (5m) + 4 leaf ports (40m)
  • t1: 24 * TOR ports (40m) + 8 spine ports (300m)

?

@baiwei0427
Copy link
Collaborator

Why different SKUs get different Xoff sizes under the same link speed and cable length?

If they are based on different ASICs, they can have different xoff sizes because of different cell sizes.
If you're not meaning this, can you elaborate it a bit more?

Got it! What are the cell sizes of 3800 and 4600?

@baiwei0427
Copy link
Collaborator

Do you have buffer templates for 2700 C32 sku?

Yes. All the buffer templates of SKUs based 2700 share the same templates.
We can provide the specific template for C32 if you provide the cable lengths of ports for t0/t1.
Are they:

  • t0: 28 * server ports (5m) + 4 leaf ports (40m)
  • t1: 24 * TOR ports (40m) + 8 spine ports (300m)

?
Let's assume the worst case.
t0: 16 * server ports (100g, 5M) + 16 leaf ports (100g, 40m)
t1: 16 * TOR ports (100g, 40m) + 16 spine ports (100g, 300m)

@stephenxs
Copy link
Collaborator Author

stephenxs commented Aug 4, 2020

Let's assume the worst case.
t0: 16 * server ports (100g, 5M) + 16 leaf ports (100g, 40m)
t1: 16 * TOR ports (100g, 40m) + 16 spine ports (100g, 300m)

Hi @liat-grozovik @lguohan ,
If we all agree on this setup I will calculate the buffer pool size for Mellanox-SN2700 based on this.

…lation

Calculate pool size in t1 as 24 * downlink port + 8 * uplink port

- Take both port and peer MTU into account when calculating headroom
- Worst case factor is decreased to 50%
- Mellanox-SN2700-C28D8 t0, assume 48 * 50G/5m + 8 * 100G/40m ports
- Mellanox-SN2700 (C32)
  - t0: 16 * 100G/5m + 16 * 100G/40m
  - t1: 16 * 100G/40m + 16 * 100G/300m

Signed-off-by: Stephen Sun <stephens@mellanox.com>
@stephenxs
Copy link
Collaborator Author

stephenxs commented Aug 8, 2020

D48C8 and C32 should use different T1 templates

Done. Please review. @baiwei0427 @neethajohn

@liat-grozovik
Copy link
Collaborator

@baiwei0427 are we good to go with these values?

@baiwei0427
Copy link
Collaborator

@baiwei0427 are we good to go with these values?

I will do calculations tomorrow and get back to you as soon as possible,

@stephenxs
Copy link
Collaborator Author

stephenxs commented Aug 10, 2020

@baiwei0427 could you also provide the cable length information for calculating pool size for 3800 as well?
For 3800, we have the following SKUs:

  • Mellanox-SN3800-C64
  • Mellanox-SN3800-D112C8
  • Mellanox-SN3800-D24C52
  • Mellanox-SN3800-D28C50

thank you!

@baiwei0427
Copy link
Collaborator

@baiwei0427 could you also provide the cable length information for calculating pool size for 3800 as well?
For 3800, we have the following SKUs:

  • Mellanox-SN3800-C64
  • Mellanox-SN3800-D112C8
  • Mellanox-SN3800-D24C52
  • Mellanox-SN3800-D28C50

thank you!

Server to T0: 5M
T0 to T1: 40M
T1 to T2: 300M

@stephenxs
Copy link
Collaborator Author

stephenxs commented Aug 12, 2020

@baiwei0427

Server to T0: 5M
T0 to T1: 40M
T1 to T2: 300M

How many ports are connected to servers, TORs, leaf/spine routers respectively in each of the SKU?
Thanks

@baiwei0427
Copy link
Collaborator

@liat-grozovik @stephenxs I did calculations for D48C8 T0, SN2700 T0, and SN2700 T1 and got similar results.

@liat-grozovik
Copy link
Collaborator

Note: Leaving aside new requirements for SN3800. this PR is for updating SN2700 SKUs.

@liat-grozovik liat-grozovik merged commit 54fcdbb into sonic-net:master Aug 13, 2020
@abdosi
Copy link
Contributor

abdosi commented Aug 14, 2020

cherry-pick has conflict. Create PR for 201911

@liat-grozovik @rlhui

@stephenxs stephenxs deleted the single-ingress-pool branch August 14, 2020 23:02
@stephenxs
Copy link
Collaborator Author

cherry-pick has conflict. Create PR for 201911

@liat-grozovik @rlhui

@abdosi Done PR #5194

santhosh-kt pushed a commit to santhosh-kt/sonic-buildimage that referenced this pull request Feb 25, 2021
…lation (sonic-net#4686)

Calculate pool size in t1 as 24 * downlink port + 8 * uplink port

- Take both port and peer MTU into account when calculating headroom
- Worst case factor is decreased to 50%
- Mellanox-SN2700-C28D8 t0, assume 48 * 50G/5m + 8 * 100G/40m ports
- Mellanox-SN2700 (C32)
  - t0: 16 * 100G/5m + 16 * 100G/40m
  - t1: 16 * 100G/40m + 16 * 100G/300m

Signed-off-by: Stephen Sun <stephens@mellanox.com>

Co-authored-by: Stephen Sun <stephens@mellanox.com>
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