+ Modified buffer config template to include internal ASIC with 5m ca… #4959

smaheshm · 2020-07-13T22:38:47Z

- Why I did it

To support buffer config generation for multi ASIC platform. The cable length used to select buffer profile is determined by the role of local and peer switch. In case of multi ASIC platforms cable length needs to determined between ASICs.

- How I did it

Added a new role called "internal" with a cable length of "5m". The role becomes "internal" if one of the ASICs in the pair is defined as 'internal' in minigraph.

- How to verify it

Verified config generation on multi-ASIC and single ASIC platforms. Sent traffic with different priorities, verified the queue counters. Visual confirmation of buffer configuration values.


admin@str-nSonic-acs-2:/etc/sonic$ grep "Ethernet-BP..|3-4" config_db0.json -A2
        "Ethernet-BP12|3-4": {
            "profile": "[BUFFER_PROFILE|pg_lossless_40000_5m_profile]"
        },
--
        "Ethernet-BP16|3-4": {
            "profile": "[BUFFER_PROFILE|pg_lossless_40000_5m_profile]"
        },
--
        "Ethernet-BP20|3-4": {
            "profile": "[BUFFER_PROFILE|pg_lossless_40000_5m_profile]"
        },
--
.
.


admin@str-nSonic-acs-2:~$ sudo ip netns exec asic0 show mmu
Pool: ingress_lossless_pool
----  --------
type  ingress
mode  dynamic
size  12766208
----  --------

Pool: egress_lossless_pool
----  --------
type  egress
mode  static
size  12766208
----  --------

Pool: egress_lossy_pool
----  -------
type  egress
mode  dynamic
size  7326924
----  -------

Profile: pg_lossless_40000_300m_profile
----------  -----------------------------------
xon_offset  2496
dynamic_th  -3
xon         18432
xoff        55120
pool        [BUFFER_POOL|ingress_lossless_pool]
size        56368
----------  -----------------------------------

Profile: pg_lossless_40000_5m_profile
----------  -----------------------------------
xon_offset  2496
dynamic_th  -3
xon         18432
xoff        55120
pool        [BUFFER_POOL|ingress_lossless_pool]
size        56368
----------  -----------------------------------

Profile: egress_lossy_profile
----------  -------------------------------
dynamic_th  3
pool        [BUFFER_POOL|egress_lossy_pool]
size        1518
----------  -------------------------------

Profile: ingress_lossy_profile
----------  -----------------------------------
dynamic_th  3
pool        [BUFFER_POOL|ingress_lossless_pool]
size        0
----------  -----------------------------------

Profile: egress_lossless_profile
---------  ----------------------------------
static_th  12766208
pool       [BUFFER_POOL|egress_lossless_pool]
size       0
---------  ----------------------------------

- Description for the changelog

Modified the 'buffers_config' template to generate cable length between internal ASICs.

Modified sort API to account for internal interfaces that are named as "Ethernet-BPx".

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

…ble length

smaheshm · 2020-07-13T22:40:37Z

recreated this PR to resolve merge conflicts. Addressed comments as mentioned in #4929

arlakshm · 2020-07-14T23:00:21Z

@smaheshm, the sonic-config-engine tests are failing. Please check

smaheshm · 2020-07-15T16:21:16Z

retest mellanox please

smaheshm · 2020-07-15T16:21:26Z

retest broadcom please

smaheshm · 2020-07-16T16:14:52Z

retest mellanox please

smaheshm · 2020-07-17T16:01:59Z

@arlakshm @neethajohn Let me know if I should add any other reviewer.

lguohan · 2020-07-18T11:56:30Z

src/sonic-config-engine/sonic-cfggen

@@ -57,7 +57,9 @@ def sort_by_port_index(value):
    if not value:
        return
    if isinstance(value, list):
-        value.sort(key = lambda k: int(k[8:]))
+        value.sort(
+            key = lambda k: int(k[8:]) if "BP" not in k else int(k[11:]) + 1024


it is difficult to understand the logic here, for example, what does BP mean? what is 1024? why 1024?

Backend ethernet ports are identified as 'Ethernet-BPxy'. The change extracts the port number for the backend ports. '1024' is just to sort the back end ports after the front end ports, assuming number of front end ports will be < 1024.
'+ 1024' has no functional impact other than for cleaner look in config_db. All backend ports are grouped at the end. This can be removed if that is preferred.

Can you add a comment above this sort for future reference?

lguohan · 2020-07-18T12:03:36Z

files/build_templates/buffers_config.j2

-                {%- set roles1 = roles1 | lower %}
-                {%- set roles2 = roles2 | lower %}
+                {%- if 'asic' == neighbor_role | lower %}
+                         {%- set roles1 = 'internal' %}


do we have unit test to cover this change in sonic-config-engine?

Added unit test.

…latforms. All links that include backend ports must be of role 'internal'.

smaheshm · 2020-07-21T18:14:27Z

retest broadcom please

neethajohn · 2020-07-22T16:56:12Z

src/sonic-config-engine/sonic-cfggen

@@ -57,7 +57,9 @@ def sort_by_port_index(value):
    if not value:
        return
    if isinstance(value, list):
-        value.sort(key = lambda k: int(k[8:]))
+        value.sort(
+            key = lambda k: int(k[8:]) if "BP" not in k else int(k[11:]) + 1024


Can you add a comment above this sort for future reference?

smaheshm · 2020-07-24T20:01:02Z

retest vsimage please

#4959) + Modified buffer config template to include internal ASIC with 5m cable length + added unit test to verify the changes.

abdosi · 2020-09-05T14:26:15Z

@smaheshm Please create PR for 201911

smaheshm · 2020-09-08T18:26:46Z

@abdosi This is already in 201911 branch. commit: 42ab5d4

+ Modified buffer config template to include internal ASIC with 5m ca…

04734ba

…ble length

smaheshm mentioned this pull request Jul 13, 2020

[Buffer config template] Modified buffer config template to include cable length between internal ASICs #4929

Closed

smaheshm requested review from arlakshm and neethajohn July 13, 2020 22:40

error in 'if' statement, fixed it

9dd6903

lguohan reviewed Jul 18, 2020

View reviewed changes

+ added unit test to verify cable length calculation for multi ASIC p…

383de5f

…latforms. All links that include backend ports must be of role 'internal'.

neethajohn previously approved these changes Jul 22, 2020

View reviewed changes

+ added comment for the change in 'sort_port_by_index' API

ed754d8

smaheshm dismissed neethajohn’s stale review via ed754d8 July 23, 2020 20:17

lguohan approved these changes Jul 23, 2020

View reviewed changes

neethajohn approved these changes Jul 23, 2020

View reviewed changes

smaheshm merged commit ee4197e into sonic-net:master Jul 27, 2020

smaheshm added the Request for 201911 Branch label Aug 19, 2020

abdosi added the Included in 201911 Branch label Aug 19, 2020

abdosi pushed a commit that referenced this pull request Aug 19, 2020

+ Modified buffer config template to include internal ASIC with 5m ca… (

42ab5d4

#4959) + Modified buffer config template to include internal ASIC with 5m cable length + added unit test to verify the changes.

abdosi removed the Included in 201911 Branch label Sep 5, 2020

abdosi added the Included in 201911 Branch label Sep 19, 2020

smaheshm deleted the pfc2 branch December 15, 2020 22:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

+ Modified buffer config template to include internal ASIC with 5m ca… #4959

+ Modified buffer config template to include internal ASIC with 5m ca… #4959

smaheshm commented Jul 13, 2020

smaheshm commented Jul 13, 2020

arlakshm commented Jul 14, 2020

smaheshm commented Jul 15, 2020

smaheshm commented Jul 15, 2020

smaheshm commented Jul 16, 2020

smaheshm commented Jul 17, 2020

lguohan Jul 18, 2020 •

edited

Loading

smaheshm Jul 21, 2020

neethajohn Jul 22, 2020

lguohan Jul 18, 2020

smaheshm Jul 21, 2020

smaheshm commented Jul 21, 2020

neethajohn Jul 22, 2020

smaheshm commented Jul 24, 2020

abdosi commented Sep 5, 2020

smaheshm commented Sep 8, 2020 •

edited

Loading

+ Modified buffer config template to include internal ASIC with 5m ca… #4959

+ Modified buffer config template to include internal ASIC with 5m ca… #4959

Conversation

smaheshm commented Jul 13, 2020

smaheshm commented Jul 13, 2020

arlakshm commented Jul 14, 2020

smaheshm commented Jul 15, 2020

smaheshm commented Jul 15, 2020

smaheshm commented Jul 16, 2020

smaheshm commented Jul 17, 2020

lguohan Jul 18, 2020 • edited Loading

Choose a reason for hiding this comment

smaheshm Jul 21, 2020

Choose a reason for hiding this comment

neethajohn Jul 22, 2020

Choose a reason for hiding this comment

lguohan Jul 18, 2020

Choose a reason for hiding this comment

smaheshm Jul 21, 2020

Choose a reason for hiding this comment

smaheshm commented Jul 21, 2020

neethajohn Jul 22, 2020

Choose a reason for hiding this comment

smaheshm commented Jul 24, 2020

abdosi commented Sep 5, 2020

smaheshm commented Sep 8, 2020 • edited Loading

lguohan Jul 18, 2020 •

edited

Loading

smaheshm commented Sep 8, 2020 •

edited

Loading