Navigation Menu

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

SAI PTF 1: Proposal #1325

Merged
merged 4 commits into from Dec 9, 2021
Merged

Conversation

pkochmanska
Copy link
Contributor

No description provided.

Signed-off-by: pkochmanska <patrycja.kochmanska@intel.com>
Signed-off-by: pkochmanska <patrycja.kochmanska@intel.com>
@rlhui
Copy link
Collaborator

rlhui commented Oct 28, 2021

@rck-innovium , @richardyu-ms - would you please review?

@rlhui rlhui requested review from rlhui and kcudnik October 28, 2021 16:42
@rlhui
Copy link
Collaborator

rlhui commented Oct 28, 2021

@gechiang - please help review, thanks.

@kcudnik
Copy link
Collaborator

kcudnik commented Oct 28, 2021

not sure if that entire rpc should be generated each time when metadata is generated, should that be a different target in makefile ?

@kcudnik
Copy link
Collaborator

kcudnik commented Oct 28, 2021

also errors:

perl gensairpc.pl
Can't locate Template.pm in @INC (you may need to install the Template module) (@INC contains: /etc/perl /usr/local/lib/x86_64-linux-gnu/perl/5.28.1 /usr/local/share/perl/5.28.1 /usr/lib/x86_64-linux-gnu/perl5/5.28 /usr/share/perl5 /usr/lib/x86_64-linux-gnu/perl/5.28 /usr/share/perl/5.28 /usr/local/lib/site_perl /usr/lib/x86_64-linux-gnu/perl-base) at gensairpc.pl line 41.
BEGIN failed--compilation aborted at gensairpc.pl line 41.

@ravi861
Copy link
Contributor

ravi861 commented Oct 29, 2021

not sure if that entire rpc should be generated each time when metadata is generated, should that be a different target in makefile ?

Would "make rpc" be more appropriate and not built by default?

Signed-off-by: pkochmanska <patrycja.kochmanska@intel.com>
@kcudnik
Copy link
Collaborator

kcudnik commented Oct 29, 2021

not sure if that entire rpc should be generated each time when metadata is generated, should that be a different target in makefile ?

Would "make rpc" be more appropriate and not built by default?

yea seems good to me

@@ -0,0 +1,605 @@
#!/usr/bin/env perl
#
# Copyright (c) 2014 Microsoft Open Technologies, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

2021

@pkochmanska pkochmanska changed the title Create SAI-Proposal-SAI_PTF.md SAI PTF 1: Proposal Nov 3, 2021
```
cd <root>/meta
make
./gensairpc.pl --clean --no-meta-build
Copy link
Collaborator

Choose a reason for hiding this comment

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

seems this parameter doesn't exist in perl


[%- BLOCK catch_exception -%]
except sai_thrift_exception as e:
status = e.status
Copy link
Collaborator

Choose a reason for hiding this comment

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

how about generating those exception handlers into a utils method that can be used by the user?

Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure I understand your point of view. This block simply surround each RPC server function call in sai_adapter with try-except clause. How would you like to turn it into a method? Secondly, do you mean to put this into sai_utils file that we want to add here: https://github.com/opencomputeproject/SAI/pull/1335/files#diff-7a25e711c6b68d0df0743a168d6efcdb1258b18e7062942eefd507eec767d9cb or some other? sai_utils file is not intended to be autogenerated. It is written manually.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see, there is a condition 'if CATCH_EXCEPTIONS:'.
LGTM

Copy link
Collaborator

Choose a reason for hiding this comment

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

but still, have some concerns about this try-except, first, it is not thread-safe. secondly, an exception should be happened as unexpected. why do we need to wrap that, and also request a manual check for the status, but not throw it directly and let users to handle them as their needs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the design we established and according to that we have written all the tests, this change would mean we have to modify all the test cases we have to handle exceptions.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ok, i see

./gensairpc.pl --help

DEPENDENCIES
Ubuntu packages:
Copy link
Collaborator

Choose a reason for hiding this comment

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

is there any place that mentioned 'graphviz' as a dependency for 'dot' which required in Makefile 'BINS = doxygen perl dot'
Not sure, if libmoose-perl needed

Copy link
Contributor

Choose a reason for hiding this comment

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

'BINS = doxygen perl dot' is an old part of Makefile. We don't introduce 'dot' with the generator. That's why its dependencies are not listed here.

Copy link
Contributor

Choose a reason for hiding this comment

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

+ IMO we could add libmoose-perl to the list, but it is a dependency of libmoosex-aliases-perl so they should be installed together.

SaiHelper inherits from SaiHelperBase but provides additional configuration.
The base configuration created by SaiHelper looks like this:

| Name | Vlan ID | Ports | Tagging |
Copy link
Contributor

Choose a reason for hiding this comment

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

Please update the table according to the new one we have in sai_base_test file that is to be upstreamed (https://github.com/aljer/SAI/blob/sai_ptf_infra/ptf/sai_base_test.py#L490)

[%- # The body of the file: -%]
// AUTOGENERATED FILE! DO NOT EDIT

namespace py sai
Copy link
Collaborator

Choose a reason for hiding this comment

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

current namespace is switch_sai.
saiservee.cpp depends on those generated files, like switch_sai_rpc, do we plan to use a new saiserver.cpp?
saiserver is referenced by syncd-rpc, then we need to upgrade the syncd-rpc to use the new saiserver if we want to use the new structure?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes. The 2nd PR for SAI PTF has a new sai_rpc_frontend.cpp which uses the new namespace. I am going to create a 3rd PR which will include the Makefile.

Signed-off-by: pkochmanska <patrycja.kochmanska@intel.com>
@ravi861
Copy link
Contributor

ravi861 commented Nov 10, 2021

@lguohan @rlhui Should we give more time for reviews or can we go ahead with merge?

@KrisNey-MSFT
Copy link

Hi team, per our SONiC-DASH-Test-Workgroup community meeting this morning, may we ask to accommodate SONiC-DASH (NIC) as we move forward? @chrispsommers @rlhui @ravi861
We are hoping to leverage SAI-Thrift for test purposes.
Thank you, @KrisNey-MSFT

@ravi861
Copy link
Contributor

ravi861 commented Nov 16, 2021

@KrisNey-MSFT Can you please reach out to me over email ravi.vantipalli@intel.com for what you are looking for?

@KrisNey-MSFT
Copy link

KrisNey-MSFT commented Nov 16, 2021 via email

@KrisNey-MSFT
Copy link

KrisNey-MSFT commented Nov 16, 2021 via email

@rlhui
Copy link
Collaborator

rlhui commented Nov 16, 2021

Adding @smaheshm to review as well.

@reshmaintel
Copy link
Contributor

@KrisNey-MSFT Hi Kristina, would you please include me in the SAI PTF call with Ravi/Intel. Alternately I can arrange a meeting for all of us to discuss. Thanks,

@richardyu-ms
Copy link
Collaborator

@KrisNey-MSFT Hi Kristina, would you please include me in the SAI PTF call with Ravi/Intel. Alternately I can arrange a meeting for all of us to discuss. Thanks,

Me too, thanks.

@rlhui rlhui merged commit 05648bc into opencomputeproject:master Dec 9, 2021
richardyu-ms pushed a commit to richardyu-ms/SAI that referenced this pull request Dec 18, 2021
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup
richardyu-ms pushed a commit to richardyu-ms/SAI that referenced this pull request Dec 18, 2021
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup
richardyu-ms pushed a commit to richardyu-ms/SAI that referenced this pull request Dec 19, 2021
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup
richardyu-ms pushed a commit to richardyu-ms/SAI that referenced this pull request Dec 19, 2021
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup

Signed-off-by: Richard Yu <richard.yu@microsoft.com>
RyoYang pushed a commit to RyoYang/SAI that referenced this pull request Dec 23, 2021
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup
RyoYang pushed a commit to RyoYang/SAI that referenced this pull request Dec 24, 2021
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup
RyoYang pushed a commit to RyoYang/SAI that referenced this pull request Dec 24, 2021
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup
RyoYang pushed a commit to RyoYang/SAI that referenced this pull request Dec 24, 2021
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup
RyoYang pushed a commit to RyoYang/SAI that referenced this pull request Dec 24, 2021
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup
richardyu-ms added a commit that referenced this pull request Jan 6, 2022
RyoYang pushed a commit to RyoYang/SAI that referenced this pull request Mar 14, 2022
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup

Signed-off-by: Richard Yu <richard.yu@microsoft.com>
RyoYang pushed a commit to RyoYang/SAI that referenced this pull request Mar 14, 2022
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup
RyoYang pushed a commit to RyoYang/SAI that referenced this pull request Mar 14, 2022
This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup

Signed-off-by: Yang Wang <yangwang1@microsoft.com>
richardyu-ms added a commit that referenced this pull request Mar 14, 2022
* SAI PTF 1: Proposal (#1325)

This is an autogeneration framework for PTF based SAI tests.  Before in order to add a new test it is necessary to:

1. Add an entry in switch_sai.thrift.
2. Add RPC server method for new entry in switch_sai_rpc_server.cpp.
3. Add a python wrapper in switch.py (if applicable).
4. Write a new test

The main goal of autogeneration framework is to generate first three steps based on SAI headers to facilitate the process of writing new tests.

* Create SAI-Proposal-SAI_PTF.md
* Adding generator script
* New makefile target and cleanup

Signed-off-by: Yang Wang <yangwang1@microsoft.com>

* SAI PTF 2: sai_base_test infra (#1335)

The changes introduce SAI PTF framework base tests infrastructure. Following files have been added:

sai_rpc_frontend.cpp - RPC server frontend file - there is a set of manually written helper functions that converts/parses SAI and thrift attributes and also here we have autogenerated SAI RPC server functions included
sai_base_test.py - python module with base test classes with common tests configuration
sai_utils.py - python module with some helper function used in SAI PTF tests
saitest.py - first test module with simple framework test

Signed-off-by: Aleksandra Jereczek <aleksandra.jereczek@intel.com>
Signed-off-by: Yang Wang <yangwang1@microsoft.com>

* Proposal for init switch and destroy switch from PRC (#1387)

keep the switch id in creat_switch and remove it from remove_switch

1. support to create the create_switch only once before the test run and use the same switch id within multi-round of testing
2. support remove the switch id

Signed-off-by: Richard Yu <richard.yu@microsoft.com>
Signed-off-by: Yang Wang <yangwang1@microsoft.com>

* [SAIServer]Add new SaiServer to support ptf-SAI test structure (#1388)

* [SAIServer]Add new SaiServer to support ptf-SAI test structure

Add build script to generate the necessary package as saiserverv2
Add new saiserver to support new structure - without switch init and with thrift 0.13.0
Add makefile
add sai_rpc_frontend
simplify saiserver and remove init method
add permission for copy_saithriftv2.s
copy saiserver at dh builddeb

ctypesgen for sai-ptf and install python-saithrift for two version

Signed-off-by: richardyu-ms <richard.yu@microsoft.com>

* add a package name for sai_adapter

Signed-off-by: richardyu-ms <richard.yu@microsoft.com>

* address review comments

Signed-off-by: richardyu-ms <richard.yu@microsoft.com>

* add more comments for the command in the Makefile

Signed-off-by: richardyu-ms <richard.yu@microsoft.com>
Signed-off-by: Yang Wang <yangwang1@microsoft.com>

* Add platform support and a sanity test sample (#1390) (#1415)

* Add a middle layer (strategy and factory) under the test cases which can

no more if-else for platform selecting
no more code injection for different platforms, just need to abstract to a method on the differences
auto select the platform by the parameters in the test starter (shell, happed in run_p4_tests, not published here)
easy-distinct structure for the difference from platforms
prepared for future statistic
Add a sanity sample, it can

configure across all the ports (base on port's configuration file)
simple FDB and VLAN naming according to port's number
make a basic check on the status of the devices
use as a sample for how to use the new middle layer

fix and suppress LGTM alters

Signed-off-by: Richard Yu <richard.yu@microsoft.com>

* code refactor part I

- add comments for class and method
- reformat code
- move platform related method to corresponding class

code refactor Part II

optimize the class hirerarchy
simplify the import items
add more docs

Signed-off-by: richardyu-ms <richard.yu@microsoft.com>

* suppress lgtm warnings

fix lgtm warning

Signed-off-by: richardyu-ms <richard.yu@microsoft.com>
Signed-off-by: Yang Wang <yangwang1@microsoft.com>

* [FIX]Fix the circular reference issue when build sai header py (#1427)

Fix circular reference issue.

Test done:
build the saiserver and check the sai_headers.py

Signed-off-by: richardyu-ms <richard.yu@microsoft.com>
Signed-off-by: Yang Wang <yangwang1@microsoft.com>

* Skip brcm teardown assertion (#1423) (#1428)

Signed-off-by: Yang Wang <yangwang1@microsoft.com>

* Add two more API as RPC for warmboot testing (#1421) (#1437)

try to expose the sai api object_type_query and switch_id_query for
warmboot testing
Modify the template file for generate code from meta.

Signed-off-by: richardyu-ms <richard.yu@microsoft.com>
Signed-off-by: Yang Wang <yangwang1@microsoft.com>

Co-authored-by: Patrycja Kochmanska <patrycja.kochmanska@intel.com>
Co-authored-by: Aleksandra Jereczek <aleksandra.jereczek@intel.com>
Co-authored-by: Richard.Yu <richard.yu@microsoft.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.

None yet

8 participants