[ppi]: Implement port bulk comparison logic #2564

nazariig · 2022-12-09T14:43:28Z

Signed-off-by: Nazarii Hnydyn [email protected]

DEPENDS:

HLD: sonic-net/SONiC#1084

What I did

Implemented port bulk add/remove comparison logic
Refactored Port OA to facilitate port config operations

Why I did it

To improve switch Fast Boot startup time

How I verified it

UT tests
VS tests

Details if related

N/A

bingwang-ms · 2022-12-16T01:50:26Z

/azp run

azure-pipelines · 2022-12-16T01:50:35Z

Azure Pipelines successfully started running 1 pipeline(s).

nazariig · 2023-01-04T20:52:28Z

/azpw run

mssonicbld · 2023-01-04T20:52:30Z

/AzurePipelines run

azure-pipelines · 2023-01-04T20:52:40Z

Azure Pipelines successfully started running 1 pipeline(s).

nazariig · 2023-01-05T17:57:33Z

/azpw run

mssonicbld · 2023-01-05T17:57:34Z

/AzurePipelines run

azure-pipelines · 2023-01-05T17:57:45Z

Azure Pipelines successfully started running 1 pipeline(s).

nazariig · 2023-01-05T22:46:28Z

/azpw run

mssonicbld · 2023-01-05T22:46:30Z

/AzurePipelines run

azure-pipelines · 2023-01-05T22:46:40Z

Azure Pipelines successfully started running 1 pipeline(s).

prgeor · 2023-01-08T07:28:42Z

@nazariig can you attach the test results to the PR?

orchagent/portsorch.cpp

prgeor · 2023-01-09T17:12:23Z

@nazariig can you provide more detailed test results. Have you tested AN/LT? FEC?

nazariig · 2023-01-18T15:52:31Z

@nazariig can you provide more detailed test results. Have you tested AN/LT? FEC?

@prgeor done. All VS tests are passed. BTW, we don't have a way to test LT. Maybe you can help with that?

liat-grozovik · 2023-01-19T15:32:26Z

@nazariig please review the gap in the coverage report.
@prgeor is the change approved from your pov?

prgeor · 2023-01-23T03:08:57Z

@nazariig please share the performance data for ports create as part of create_switch (SAI profile file) vs bulk ports create from portsorch for your platform

prgeor · 2023-01-23T03:09:28Z

@nazariig can you provide more detailed test results. Have you tested AN/LT? FEC?

@prgeor done. All VS tests are passed. BTW, we don't have a way to test LT. Maybe you can help with that?

@nazariig can you test AN?

prgeor · 2023-01-23T03:10:26Z

@nazariig can you provide more detailed test results. Have you tested AN/LT? FEC?

@nazariig can you test setting of FEC?

prgeor · 2023-01-23T03:22:23Z

orchagent/portsorch.cpp

+        attrPtrList.push_back(attrDataList.back().data());
+    }
+
+    auto status = sai_port_api->create_ports(


From HLD, what if create_ports is NULL or not supported?

@prgeor these statements are not relevant:

Checking for NULL doesn't make any sense - the bulk API (sairedis) is implemented and currently available for all the vendors

The current syncd implementation doesn't propagate SAI_STATUS_NOT_SUPPORTED and SAI_STATUS_NOT_IMPLEMENTED codes to upper layer. Instead, it automatically falls back to the legacy one-by-one processing of the requested bulk operation objects. Taking this into consideration, there is no point to have a capabilities check in swss

prgeor · 2023-01-23T03:24:13Z

@nazariig can you simulate a scenario where your changes are tested on your platform where it does not support bulk port creation?

nazariig · 2023-01-23T13:28:56Z

@nazariig please share the performance data for ports create as part of create_switch (SAI profile file) vs bulk ports create from portsorch for your platform

it is not relevant in context of this feature, since create_switch part hasn't been touched.

Signed-off-by: Nazarii Hnydyn <[email protected]>

nazariig · 2023-07-20T14:59:53Z

/azpw run

mssonicbld · 2023-07-20T14:59:55Z

/AzurePipelines run

azure-pipelines · 2023-07-20T15:00:01Z

No commit pushedDate could be found for PR 2564 in repo sonic-net/sonic-swss

nazariig · 2023-07-20T21:28:52Z

/azpw run Azure.sonic-swss

mssonicbld · 2023-07-20T21:28:53Z

/AzurePipelines run Azure.sonic-swss

azure-pipelines · 2023-07-20T21:28:58Z

No commit pushedDate could be found for PR 2564 in repo sonic-net/sonic-swss

xumia · 2023-07-22T05:32:15Z

@nazariig , could you please try the command "/azpw run Azure.sonic-swss" again?

nazariig · 2023-07-22T20:25:41Z

/azpw run Azure.sonic-swss

mssonicbld · 2023-07-22T20:25:43Z

/AzurePipelines run Azure.sonic-swss

azure-pipelines · 2023-07-22T20:25:52Z

Azure Pipelines successfully started running 1 pipeline(s).

nazariig · 2023-07-22T20:26:52Z

@nazariig , could you please try the command "/azpw run Azure.sonic-swss" again?

@xumia done. It works! Did you fix the issue?

liat-grozovik · 2023-07-23T08:14:02Z

@prgeor @lguohan please be reminded this is critical feature for 202305 and 202211. please close the review ASAP.

- What I did Implemented port bulk add/remove comparison logic Refactored Port OA to facilitate port config operations - Why I did it To improve switch Fast Boot startup time - How I verified it UT tests VS tests Backport from master: [ppi]: Implement port bulk comparison logic #2564 Signed-off-by: Nazarii Hnydyn <[email protected]>

liat-grozovik · 2023-07-23T08:20:09Z

Backported PR was approved and merged. moving forward with this PR as well.

DEPENDS: [202211][ppi]: Implement port bulk comparison logic (#2564) #2821 HLD: sonic-net/SONiC#1084 What I did Removed unused code Why I did it To complete code cleanup How I verified it UT tests VS tests Details if related Backport from master: [ppi]: General code cleanup: remove unused methods #2867

- What I did Implemented port bulk add/remove comparison logic Refactored Port OA to facilitate port config operations - Why I did it To improve switch Fast Boot startup time - How I verified it UT tests VS tests Signed-off-by: Nazarii Hnydyn <[email protected]>

nazariig requested a review from prsunny as a code owner December 9, 2022 14:43

nazariig force-pushed the master-ppi-rel branch from 310808c to 032efa2 Compare December 9, 2022 14:56

nazariig requested review from liat-grozovik and volodymyrsamotiy December 9, 2022 17:58

liat-grozovik mentioned this pull request Dec 13, 2022

Port Profile Init HLD sonic-net/SONiC#1084

Merged

prsunny requested a review from prgeor December 16, 2022 18:40

nazariig force-pushed the master-ppi-rel branch from 032efa2 to d7741a0 Compare January 6, 2023 10:13

prgeor reviewed Jan 8, 2023

View reviewed changes

orchagent/portsorch.cpp Show resolved Hide resolved

nazariig force-pushed the master-ppi-rel branch from d7741a0 to cea1516 Compare January 17, 2023 09:47

prgeor reviewed Jan 23, 2023

View reviewed changes

nazariig added 6 commits July 20, 2023 12:43

[ppi]: Implement port bulk comparison logic.

c3d1bb6

Signed-off-by: Nazarii Hnydyn <[email protected]>

[ppi]: Limit number of attributes on port create.

726bd1d

Signed-off-by: Nazarii Hnydyn <[email protected]>

[ppi]: Port serdes parser improvements.

d13b00f

Signed-off-by: Nazarii Hnydyn <[email protected]>

[ppi]: Backport sonic-net#2607.

ea5077a

Signed-off-by: Nazarii Hnydyn <[email protected]>

[ppi]: Handle review comments.

4d8db39

Signed-off-by: Nazarii Hnydyn <[email protected]>

[ppi]: Improve code coverage.

ee984cc

Signed-off-by: Nazarii Hnydyn <[email protected]>

nazariig force-pushed the master-ppi-rel branch from f01539d to ee984cc Compare July 20, 2023 09:43

liat-grozovik approved these changes Jul 23, 2023

View reviewed changes

liat-grozovik merged commit d54c767 into sonic-net:master Jul 23, 2023
14 checks passed

nazariig mentioned this pull request Jul 27, 2023

[ppi]: General code cleanup: remove unused methods #2867

Merged

mssonicbld mentioned this pull request Sep 3, 2023

[action] [PR:15866] [Mellanox] [PPI] Enable global port late create for SN5600 sonic-net/sonic-buildimage#16402

Merged

11 tasks

nazariig added the Cherry Pick Conflict_202305 label Oct 5, 2023

nazariig mentioned this pull request Oct 6, 2023

[202305][ppi]: Implement port bulk comparison logic #2921

Merged

dgsudharsan added the Included in 202305 Branch label Oct 17, 2023

[ppi]: Implement port bulk comparison logic #2564

[ppi]: Implement port bulk comparison logic #2564

Conversation

nazariig commented Dec 9, 2022 • edited Loading

bingwang-ms commented Dec 16, 2022

azure-pipelines bot commented Dec 16, 2022

nazariig commented Jan 4, 2023

mssonicbld commented Jan 4, 2023

azure-pipelines bot commented Jan 4, 2023

nazariig commented Jan 5, 2023

mssonicbld commented Jan 5, 2023

azure-pipelines bot commented Jan 5, 2023

nazariig commented Jan 5, 2023

mssonicbld commented Jan 5, 2023

azure-pipelines bot commented Jan 5, 2023

prgeor commented Jan 8, 2023

prgeor commented Jan 9, 2023

nazariig commented Jan 18, 2023

liat-grozovik commented Jan 19, 2023

prgeor commented Jan 23, 2023

prgeor commented Jan 23, 2023

prgeor commented Jan 23, 2023

prgeor Jan 23, 2023

Choose a reason for hiding this comment

nazariig Jan 23, 2023

Choose a reason for hiding this comment

prgeor commented Jan 23, 2023

nazariig commented Jan 23, 2023

nazariig commented Jul 20, 2023

mssonicbld commented Jul 20, 2023

azure-pipelines bot commented Jul 20, 2023

nazariig commented Jul 20, 2023

mssonicbld commented Jul 20, 2023

azure-pipelines bot commented Jul 20, 2023

xumia commented Jul 22, 2023

nazariig commented Jul 22, 2023

mssonicbld commented Jul 22, 2023

azure-pipelines bot commented Jul 22, 2023

nazariig commented Jul 22, 2023 • edited Loading

liat-grozovik commented Jul 23, 2023

liat-grozovik commented Jul 23, 2023

nazariig commented Dec 9, 2022 •

edited

Loading

nazariig commented Jul 22, 2023 •

edited

Loading