[Mellanox] Avoid attaching lossless buffer profiles for internal ports #18978

vivekrnv · 2024-05-16T03:25:19Z

What I did

Internal ports of smartswitch will not have RDMA traffic and so we need not apply lossless buffer profiles and need not enable PFC. Refer vivekrnv@6accbf5 for a better visualization of the changes in buffer_default_objects.j2
Update the Mellanox-SN4700-O28 SKU buffer profiles to match the following config. Topology inferred from t1-28-lag topology (Add t1-28-lag topology sonic-mgmt#9837) .

Port configuration	Value
Breakout mode for each port	Defined in port mapping
Speed of the port	Defined in Port mapping
Internal Ports	Defined in Port mapping

Buffer configuration	Value
Shared headroom	Enabled
Shared headroom pool factor	2
Dynamic Buffer	Disable
In static buffer scenario how many uplinks and downlinks?	8 1x400G uplinks and 20 1x400G downlinks

Port Mapping

Ports	Mode
1-8	1x400G
9-28	1x400G
28-32	1x200G (Internal Ports connected to DPU)

Number of Uplinks / Downlinks:

T1 topology:
Length of downlink: 40m
Length of uplink: 300m

Work item tracking

Microsoft ADO (number only):

How I did it

How to verify it

Unit Tests
Verify IP traffic between DPU and NPU through internal ports after applying buffer profiles

root@r-leopard-79:/home/admin# show int status
      Interface                            Lanes    Speed    MTU    FEC    Alias             Vlan    Oper    Admin                                             Type    Asym PFC
---------------  -------------------------------  -------  -----  -----  -------  ---------------  ------  -------  -----------------------------------------------  ----------
    Ethernet224  224,225,226,227,228,229,230,231     200G   9100    N/A    etp29           routed      up       up                                DPU-NPU Data Port         off
    Ethernet232  232,233,234,235,236,237,238,239     200G   9100    N/A    etp30           routed      up       up                                DPU-NPU Data Port         off
    Ethernet240  240,241,242,243,244,245,246,247     200G   9100    N/A    etp31           routed      up       up                                DPU-NPU Data Port         off
    Ethernet248  248,249,250,251,252,253,254,255     200G   9100    N/A    etp32           routed      up       up                                DPU-NPU Data Port         off

Run config qos reload and verify Buffer tables:

"BUFFER_PG": {
       ....................
        "Ethernet224|3-4": {
            "profile": "ingress_lossy_profile"
        },
        "Ethernet232|3-4": {
            "profile": "ingress_lossy_profile"
        },
        "Ethernet240|3-4": {
            "profile": "ingress_lossy_profile"
        },
        "Ethernet248|3-4": {
            "profile": "ingress_lossy_profile"
        }
    },

"BUFFER_QUEUE": {
       "Ethernet224|3-4": {
            "profile": "q_lossy_profile"
        },
        "Ethernet232|3-4": {
            "profile": "q_lossy_profile"
        },
        "Ethernet240|3-4": {
            "profile": "q_lossy_profile"
        },
        "Ethernet248|3-4": {
            "profile": "q_lossy_profile"
        },
    },

"BUFFER_PORT_INGRESS_PROFILE_LIST": {
        "Ethernet224": {
            "profile_list": "ingress_lossy_profile"
        },
        "Ethernet232": {
            "profile_list": "ingress_lossy_profile"
        },
        "Ethernet240": {
            "profile_list": "ingress_lossy_profile"
        },
        "Ethernet248": {
            "profile_list": "ingress_lossy_profile"
        }
    },

"BUFFER_PORT_EGRESS_PROFILE_LIST": {
        "Ethernet224": {
            "profile_list": "egress_lossy_profile"
        },
        "Ethernet232": {
            "profile_list": "egress_lossy_profile"
        },
        "Ethernet240": {
            "profile_list": "egress_lossy_profile"
        },
        "Ethernet248": {
            "profile_list": "egress_lossy_profile"
        }
}

"PORT_QOS_MAP": {
       "Ethernet224": {
            "dscp_to_tc_map": "AZURE",
            "pfc_to_pg_map": "AZURE",
            "pfc_to_queue_map": "AZURE",
            "tc_to_pg_map": "AZURE",
            "tc_to_queue_map": "AZURE"
        },
}

QUEUE: {
     "Ethernet224|3": {
            "scheduler": "scheduler.0"
      },
     "Ethernet224|4": {
            "scheduler": "scheduler.0"
     },
}

swss.rec output for traditional mode

2024-04-30.21:51:29.193211|BUFFER_POOL_TABLE:egress_lossless_pool|SET|mode:dynamic|size:60817392|type:egress                                                                                                                           
2024-04-30.21:51:29.193238|BUFFER_POOL_TABLE:egress_lossy_pool|SET|mode:dynamic|size:50270208|type:egress                                                                                                                              
2024-04-30.21:51:29.193258|BUFFER_POOL_TABLE:ingress_lossless_pool|SET|mode:dynamic|size:50270208|type:ingress|xoff:5611520                                                                                                            
2024-04-30.21:51:29.223414|BUFFER_PROFILE_TABLE:pg_lossless_400000_300m_profile|SET|dynamic_th:0|pool:ingress_lossless_pool|size:38912|xoff:358400|xon:38912                                                                           
2024-04-30.21:51:29.223454|BUFFER_PROFILE_TABLE:egress_lossless_profile|SET|dynamic_th:7|pool:egress_lossless_pool|size:0                                                                                                              
2024-04-30.21:51:29.223474|BUFFER_PROFILE_TABLE:pg_lossless_400000_40m_profile|SET|dynamic_th:0|pool:ingress_lossless_pool|size:38912|xoff:144384|xon:38912                                                                            
2024-04-30.21:51:29.223490|BUFFER_PROFILE_TABLE:ingress_lossless_profile|SET|dynamic_th:7|pool:ingress_lossless_pool|size:0                                                                                                            
2024-04-30.21:51:29.223524|BUFFER_PROFILE_TABLE:ingress_lossy_profile|SET|dynamic_th:3|pool:ingress_lossless_pool|size:0                                                                                                               
2024-04-30.21:51:29.223546|BUFFER_PROFILE_TABLE:egress_lossy_profile|SET|dynamic_th:7|pool:egress_lossy_pool|size:9216                                                                                                                 
2024-04-30.21:51:29.223564|BUFFER_PROFILE_TABLE:q_lossy_profile|SET|dynamic_th:3|pool:egress_lossy_pool|size:0

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

vivekrnv · 2024-05-17T03:39:14Z

/azpw ms_conflict

vivekrnv · 2024-05-17T23:14:35Z

/azpw ms_conflict

lguohan · 2024-05-19T06:49:03Z

device/mellanox/x86_64-mlnx_msn4700-r0/Mellanox-SN4700-O28/buffers_defaults_objects.j2

why this cannot be unified with ACS-MSN2700?

it is hard to maintain a separate file.

same for buffers_defaults_t0/t1 j2.

hard to maintain a separate file.

We already maintain a few different buffer_defaults_objects.j2 files for each scenario, like the file under 2700/D48C8 supports shared headroom and extra queues

the one under 2700/ACS-2700 does not support shared headroom.

So, we already maintain more than one file based on use case. Thus i'd prefer not coupling SmartSwitch changes with existing ones. It easy to maintain this way

buffers_defaults_t0/t1 j2. is anyway specific per SKU so it'll be different.

@lguohan
The main logic of buffer templates is implemented in buffer_defaults_objects.j2.
we have had 2 different buffer_defaults_objects.j2s. One for single ingress pool mode + shared headroom and the other for double ingress pool without shared headroom pool.
We do it in this way because it will make the template very difficult to understand and maintain if we combine them into one.
Now it's a similar scenario and we have another buffer_defaults_objects.j2 for the smart switch.
Putting them all together we have only 3 buffer_defaults_objects.j2s.

As for the buffer_defaults_t0/t1.j2, they are very simple.
The main logic is to define the pool sizes and to invoke macros to define other buffer objects, like PGs, queues, etc.

So, it doesn't look like a challenge to maintain.

Hi @lguohan, let me know what you think?

Hi @lguohan, A gentle reminder

liat-grozovik · 2024-05-22T15:58:26Z

@lguohan let me know if you are ok to to merge following Vivek feedback.
moving forward i think we need to have some general files which can be overrides by specific devices. this should be a new feature on master i hope to have in next release

liat-grozovik · 2024-05-22T16:00:18Z

@vivekrnv is this needed for 202311 and should be considered as bug fix?

vivekrnv · 2024-05-22T16:15:02Z

@vivekrnv is this needed for 202311 and should be considered as bug fix?

hi @liat-grozovik , not required for 202311

lguohan · 2024-05-24T15:08:35Z

@liat-grozovik , i really concerns on duplicating the templates, this makes the maintaining future maintaining efforts very challenging.

prsunny · 2024-07-09T16:46:50Z

@lguohan to signoff

kperumalbfn · 2024-07-12T22:39:29Z

files/build_templates/qos_config.j2

-            "pfcwd_sw_enable" : "3,4"
+            "pfcwd_sw_enable" : "3,4",
+{% endif %}
+            "tc_to_pg_map"    : "AZURE",


tc_to_pg_map and pfc_to_queue_map is also not required for internal ports. Is this excluded?

tc_to_pg_map is required for internal ports.

You mean pfc_to_pg_map and pfc_to_queue_map? Then yes, these two are not required but however having them wouldn't cause any issues IMO. We are configuring the map without enabling pfc on any of the pg's. Let me know if you think i should remove these two as well

I agree with @vivekrnv. It's harmless to keep these mappings the same between internal ports and normal ports but the logic will be much simpler.

@vivekrnv @stephenxs lossless buffer profiles are not attached to internal ports, so lossless functionality is not possible on these internal ports.

If it is to keep the config uniform across all the ports, this is good.

Yes, the idea is to keep the maps uniform across all the ports

Hi @kperumalbfn, Can we sign this off?

kperumalbfn · 2024-07-31T15:15:17Z

LGTM

sonic-net#18978) [Mellanox] Avoid attaching lossless buffer profiles for internal ports (sonic-net#18978) Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

mssonicbld · 2024-08-02T00:52:12Z

Cherry-pick PR to 202405: #19768

#18978) [Mellanox] Avoid attaching lossless buffer profiles for internal ports (#18978) Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

vivekrnv and others added 10 commits April 19, 2024 18:49

Don't apply lossless profiles on Dpc ports

37b386c

Fix Buffer PG profile

46749cf

Fix updating wrong sku

2ca447a

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

Update the original SKU

1a869dc

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

Set the default topo to T1

3d7fdaa

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

Fix buffer profile ingress list

ee4a443

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

Adjust pool sizes and xoff

2dfeccf

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

Add Unit Tests for template change

75d02f4

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

Fix pool sizes and xoff

27230b7

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

Update headers

c6ada06

Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

vivekrnv requested a review from lguohan as a code owner May 16, 2024 03:25

vivekrnv requested review from stephenxs and dgsudharsan and removed request for lguohan May 16, 2024 03:25

stephenxs approved these changes May 16, 2024

View reviewed changes

Merge branch 'sonic-net:master' into fix_buffer_dpc

205d49b

lguohan reviewed May 19, 2024

View reviewed changes

liat-grozovik approved these changes May 22, 2024

View reviewed changes

liat-grozovik changed the title ~~Avoid attaching lossless buffer profiles for internal ports~~ [Mellanox] Avoid attaching lossless buffer profiles for internal ports May 22, 2024

vivekrnv added the Request for 202405 Branch label Jun 6, 2024

dgsudharsan requested a review from lguohan July 1, 2024 04:09

oleksandrivantsiv requested a review from prsunny July 8, 2024 22:55

prsunny requested a review from kperumalbfn July 9, 2024 16:46

kperumalbfn reviewed Jul 12, 2024

View reviewed changes

dgsudharsan requested a review from kperumalbfn July 30, 2024 22:43

kperumalbfn approved these changes Jul 31, 2024

View reviewed changes

kperumalbfn merged commit 4e78b11 into sonic-net:master Jul 31, 2024
19 checks passed

bingwang-ms added the Approved for 202405 Branch label Aug 2, 2024

mssonicbld mentioned this pull request Aug 2, 2024

[action] [PR:18978] [Mellanox] Avoid attaching lossless buffer profiles for internal ports (#18978) #19768

Merged

mssonicbld added the Created PR to 202405 Branch label Aug 2, 2024

mssonicbld pushed a commit that referenced this pull request Aug 2, 2024

[Mellanox] Avoid attaching lossless buffer profiles for internal ports (

2eb1380

#18978) [Mellanox] Avoid attaching lossless buffer profiles for internal ports (#18978) Signed-off-by: Vivek Reddy <vkarri@nvidia.com>

mssonicbld added Included in 202405 Branch and removed Created PR to 202405 Branch labels Aug 2, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Mellanox] Avoid attaching lossless buffer profiles for internal ports #18978

[Mellanox] Avoid attaching lossless buffer profiles for internal ports #18978

vivekrnv commented May 16, 2024

vivekrnv commented May 17, 2024

vivekrnv commented May 17, 2024

lguohan May 19, 2024

lguohan May 19, 2024

vivekrnv May 21, 2024

stephenxs May 25, 2024

vivekrnv May 28, 2024

vivekrnv Jun 7, 2024

liat-grozovik commented May 22, 2024

liat-grozovik commented May 22, 2024

vivekrnv commented May 22, 2024

lguohan commented May 24, 2024

prsunny commented Jul 9, 2024

kperumalbfn Jul 12, 2024

vivekrnv Jul 15, 2024 •

edited

Loading

stephenxs Jul 16, 2024

kperumalbfn Jul 19, 2024

vivekrnv Jul 19, 2024

vivekrnv Jul 23, 2024

kperumalbfn commented Jul 31, 2024

mssonicbld commented Aug 2, 2024

[Mellanox] Avoid attaching lossless buffer profiles for internal ports #18978

[Mellanox] Avoid attaching lossless buffer profiles for internal ports #18978

Conversation

vivekrnv commented May 16, 2024

What I did

Work item tracking

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

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

vivekrnv commented May 17, 2024

vivekrnv commented May 17, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liat-grozovik commented May 22, 2024

liat-grozovik commented May 22, 2024

vivekrnv commented May 22, 2024

lguohan commented May 24, 2024

prsunny commented Jul 9, 2024

Choose a reason for hiding this comment

vivekrnv Jul 15, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kperumalbfn commented Jul 31, 2024

mssonicbld commented Aug 2, 2024

vivekrnv Jul 15, 2024 •

edited

Loading