Skip to content

[dash]: Refactor DASH orch by protobuf format#2722

Merged
lguohan merged 21 commits intosonic-net:dashfrom
Pterosaur:pb_orch
Jul 26, 2023
Merged

[dash]: Refactor DASH orch by protobuf format#2722
lguohan merged 21 commits intosonic-net:dashfrom
Pterosaur:pb_orch

Conversation

@Pterosaur
Copy link
Contributor

@Pterosaur Pterosaur commented Apr 4, 2023

What I did
Refactor orchagent input from text schema to protobuf message. The protobuf message definition is at: https://github.com/sonic-net/sonic-dash-api/tree/master

Why I did it
The original text schema format will take

  1. too much memory in Redis
  2. the performance is not efficient due to large of format conversions.
  3. Weak coding restrictions aren't easy to develop and maintain

So, the binary formation, protobuf, will be needed for better utilization.

How I verified it
Check Azp and test dash vstest in locally.

 sudo pytest --dvsname=vs test_dash_vnet.py
============================================================= test session starts ==============================================================
platform linux -- Python 3.8.10, pytest-7.2.2, pluggy-1.0.0
rootdir: /home/zegan/workspace/sonic-swss/tests
plugins: flaky-3.7.0
collected 8 items

test_dash_vnet.py ........                                                                                                               [100%]

=============================================================== warnings summary ===============================================================
sudo pytest --dvsname=vs test_dash_acl.py
============================================================= test session starts ==============================================================
platform linux -- Python 3.8.10, pytest-7.2.2, pluggy-1.0.0
rootdir: /home/zegan/workspace/sonic-swss/tests
plugins: flaky-3.7.0
collected 6 items

test_dash_acl.py ......                                                                                                                  [100%]

=============================================================== warnings summary ===============================================================

Details if related

Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Pterosaur and others added 8 commits June 18, 2023 10:48
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: zegan <zegan@microsoft.com>
Signed-off-by: zegan <zegan@microsoft.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: zegan <zegan@microsoft.com>
@prsunny prsunny requested a review from prabhataravind July 6, 2023 00:00
Pterosaur added 2 commits July 6, 2023 22:19
Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur Pterosaur marked this pull request as ready for review July 6, 2023 15:29
@Pterosaur Pterosaur requested a review from prsunny as a code owner July 6, 2023 15:29
Signed-off-by: Ze Gan <ganze718@gmail.com>
dash::eni::Eni metadata;
};

struct QosEntry
Copy link
Contributor

Choose a reason for hiding this comment

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

We can possibly remove this too right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it should be removed because it wasn't used anywhere. It's my careless mistake

Copy link
Contributor

@prabhataravind prabhataravind left a comment

Choose a reason for hiding this comment

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

The changes in dash* orchs in general look good to me. Could you please validate your changes with the configurations in these issues? If you could send the sairedis records with these configurations applied, that would be great.

https://github.com/nvidia-sonic/sonic-buildimage/issues/183
https://github.com/nvidia-sonic/sonic-buildimage/issues/178

Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
Signed-off-by: Ze Gan <ganze718@gmail.com>
@Pterosaur
Copy link
Contributor Author

The changes in dash* orchs in general look good to me. Could you please validate your changes with the configurations in these issues? If you could send the sairedis records with these configurations applied, that would be great.

nvidia-sonic/sonic-buildimage#183 nvidia-sonic/sonic-buildimage#178

Check them and add testcases for them.

@Pterosaur Pterosaur requested a review from prabhataravind July 24, 2023 12:24
Copy link
Contributor

@prabhataravind prabhataravind left a comment

Choose a reason for hiding this comment

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

lgtm

@lguohan lguohan merged commit daa1fc7 into sonic-net:dash Jul 26, 2023
theasianpianist pushed a commit to theasianpianist/sonic-swss that referenced this pull request Jul 27, 2023
What I did
Refactor orchagent input from text schema to protobuf message. The protobuf message definition is at: https://github.com/sonic-net/sonic-dash-api/tree/master

Why I did it
The original text schema format will take

too much memory in Redis
the performance is not efficient due to large of format conversions.
Weak coding restrictions aren't easy to develop and maintain
So, the binary formation, protobuf, will be needed for better utilization.

How I verified it
Check Azp and test dash vstest in locally.

Signed-off-by: Ze Gan <ganze718@gmail.com>
theasianpianist pushed a commit to theasianpianist/sonic-swss that referenced this pull request Jul 27, 2023
What I did
Refactor orchagent input from text schema to protobuf message. The protobuf message definition is at: https://github.com/sonic-net/sonic-dash-api/tree/master

Why I did it
The original text schema format will take

too much memory in Redis
the performance is not efficient due to large of format conversions.
Weak coding restrictions aren't easy to develop and maintain
So, the binary formation, protobuf, will be needed for better utilization.

How I verified it
Check Azp and test dash vstest in locally.

Signed-off-by: Ze Gan <ganze718@gmail.com>
Pterosaur added a commit to Pterosaur/sonic-sairedis that referenced this pull request Aug 20, 2023
Swss containter needs dash_api to properly build, based on sonic-net/sonic-swss#2722, attempting to add missing libs

Signed-off-by: Ze Gan <ganze718@gmail.com>
Pterosaur added a commit to Pterosaur/sonic-sairedis that referenced this pull request Aug 20, 2023
Swss containter needs dash_api to properly build, based on sonic-net/sonic-swss#2722, attempting to add missing libs

Signed-off-by: Ze Gan <ganze718@gmail.com>
kcudnik pushed a commit to sonic-net/sonic-sairedis that referenced this pull request Aug 23, 2023
Swss containter needs dash_api to properly build, based on sonic-net/sonic-swss#2722, attempting to add missing libs

Signed-off-by: Ze Gan <ganze718@gmail.com>
Pterosaur added a commit to Pterosaur/sonic-swss-common that referenced this pull request Aug 23, 2023
Swss containter needs dash_api to properly build, based on sonic-net/sonic-swss#2722, attempting to add missing libs

Signed-off-by: Ze Gan <ganze718@gmail.com>
Pterosaur added a commit to sonic-net/sonic-swss-common that referenced this pull request Aug 23, 2023
Swss containter needs dash_api to properly build, based on sonic-net/sonic-swss#2722, attempting to add missing libs

Signed-off-by: Ze Gan <ganze718@gmail.com>
SviatoslavBoichuk pushed a commit to SviatoslavBoichuk/sonic-swss-common that referenced this pull request Sep 7, 2023
Swss containter needs dash_api to properly build, based on sonic-net/sonic-swss#2722, attempting to add missing libs

Signed-off-by: Ze Gan <ganze718@gmail.com>
Janetxxx pushed a commit to Janetxxx/sonic-swss that referenced this pull request Nov 10, 2025
What I did
Refactor orchagent input from text schema to protobuf message. The protobuf message definition is at: https://github.com/sonic-net/sonic-dash-api/tree/master

Why I did it
The original text schema format will take

too much memory in Redis
the performance is not efficient due to large of format conversions.
Weak coding restrictions aren't easy to develop and maintain
So, the binary formation, protobuf, will be needed for better utilization.

How I verified it
Check Azp and test dash vstest in locally.

Signed-off-by: Ze Gan <ganze718@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants