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

PINS SONiC HLD for Generic SAI Extensions #1088

Merged
merged 11 commits into from
Dec 3, 2022
Merged

PINS SONiC HLD for Generic SAI Extensions #1088

merged 11 commits into from
Dec 3, 2022

Conversation

svshah-intel
Copy link
Contributor

@svshah-intel svshah-intel commented Sep 26, 2022

PINS is an architecture to implement a modelled P4 program, in the SONiC framework, on top of the SAI pipeline. SAI pipeline is a well-documented forwarding behavior of a data-plane.
In this document, we describe how the modeling can be taken a step further, allowing PINS/SONiC to impelement extensions to the SAI pipeline.

Repo PR title State
sonic-swss-common PINS Extension tables support GitHub issue/pull request detail
sonic-pins PINS Extension tables support GitHub issue/pull request detail
sonic-swss [p4orch]: PINS Extension tables support GitHub issue/pull request detail


Current P4Orch implementation of PINS SAI path maintains mapping from FIXED P4RT table name to the manager responsible for processing respective table entries to relevant SAI Objects. With added support of the PINS SAI Extension tables, control flow for FIXED P4RT tables remains as before, where-as EXT P4RT table entries will be directed to the newly introduced SAI Extension manager.

#### SAI Extension Manager <a name="SAI-Extension-Manager"></a>
Copy link
Collaborator

Choose a reason for hiding this comment

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

the SAI extension manager looks very much like the SAI-DB itself, except it got pulled up another level higher.

Choose a reason for hiding this comment

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

Hi Lihua, thank you for your comment. I have some response below. Please see, and we can add more details here as well.

The SAI Extension manager in the P4Orch, which caters to serving extension tables, maintains table entries separately per table. Each of these tables are tagged with an algorithmically determined precedence value based on dependency graph. At the time of draining table entries, The SAI Extension manager may be called multiple times, each time with a different precedence value and so allowing the SAI Extension manager process table entries only from those tables matching the precedence value.

#### Cross Table dependencies <a name="Cross-Table-dependencies"></a>
Once the Table Definition database is built, the state created for each table would have cross-reference table/match-fields mapped from eg. an action parameter if that action parameter is defined to have a cross table dependency. As stated earlier, this document focuses on dependencies from PINS SAI Extension table entries to another PINS SAI Extension table entry, and from PINS SAI Extension table entris to PINS SAI table entry. Thus, only those scenarios are considered in the design [This framework can be leveraged, with some enhancements, for other scenarios as well like for PINS object dependencies on non-PINS SAI objects, PINS SAI table entries referencing to entry from PINS SAI Extension table].
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems to me that this is actually the key weakness of the SAI extension manager design. SAI DB already has all the objects so building cross table design would have been straightforward, and as a day-1 scenario. However, with the SAI extension manager design, the objects basically need to have a copy in the SAI extension manager, which is missing for objects from other parts of the system at this point

Choose a reason for hiding this comment

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

Hi Lihua, Since Sai API calls require OID to be passed as well, all the Orchs within SWSS store the OID mappings. Example being mirrororch, routeorch, vrforch. Similarly P4orch also maintains p4 entries/key to oid mapping as added last year and does ref counting as well.
The new SAI-Extensions manager within P4Orch is extending the same functionality for handling the non-SAI objects and have the corresponding OID mappings for dependencies. Since the SAI objects are “known” it is possible for us to query the corresponding manager in OrchAgent to retrieve the OID info, but for SAI Generic Extensions to handle the same for new non-SAI objects we needed to add a generic way to retrieve OID given a key-value and here the dependency graph helps. Please let us know and we can add more details.
Thank you.
Reshma


Table prefix for PINS SAI Path tables is **P4RT_TABLE:FIXED_<TableName>**
Table prefix for PINS SAI Extension tables would be **P4RT_TABLE:EXT_<TableName>**

Copy link

Choose a reason for hiding this comment

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

If the motivation to add prefix is to distinguish between "standard" vs "extensions", then why call it 'fixed_" vs "ext_". Wouldn't it better to use "STD_" prefix for PINS SAI path tables for clarity sake?

Choose a reason for hiding this comment

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

Hi Hanif, The prefix 'fixed_' exists in PINS upstreamed in 202111 release, indicates it is as defined in SAI. We can look into changing fixed_ to STD_ after assessing impact of test automations and further discussions with Google.

rck-innovium
rck-innovium previously approved these changes Oct 20, 2022
@zhangyanzhao
Copy link
Collaborator

@svshah-intel can you please help to add the code PRs to this HLD by referring to PR #806 ? We need get all feature code PRs tracked in HLD. Thanks.

@svshah-intel
Copy link
Contributor Author

@svshah-intel can you please help to add the code PRs to this HLD by referring to PR #806 ? We need get all feature code PRs tracked in HLD. Thanks.

done!

@svshah-intel svshah-intel changed the title PINS SONiC HLD for SAI Generic Extensions PINS SONiC HLD for Generic SAI Extensions Nov 16, 2022
@lihuay
Copy link
Collaborator

lihuay commented Nov 21, 2022

The current P4Orch keeps separate/duplicated tables for fixed objects -- e.g. route objects. This can potentially cause conflicts with existing orch agents, and makes it hard for other orch agents from leveraging the SAI extension objects. But I think it's reasonable to address this in future iterations.

Copy link
Collaborator

@lihuay lihuay left a comment

Choose a reason for hiding this comment

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

The current P4Orch keeps separate/duplicated tables for fixed objects -- e.g. route objects. This can potentially cause conflicts with existing orch agents, and makes it hard for other orch agents from leveraging the SAI extension objects. But I think it's reasonable to address this in future iterations.

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.

7 participants