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

Initial version of PINS HLD #809

Merged
merged 4 commits into from
Mar 1, 2022
Merged

Initial version of PINS HLD #809

merged 4 commits into from
Mar 1, 2022

Conversation

bocon13
Copy link
Contributor

@bocon13 bocon13 commented Jun 25, 2021

High Level Design document for PINS: P4 Integrated Network Stack

@ghost
Copy link

ghost commented Jun 25, 2021

CLA assistant check
All CLA requirements met.

@bocon13 bocon13 marked this pull request as draft June 25, 2021 03:08
## SAI API
There are no changes to SAI.

For programmable hardware targets, PINS introduces an additional SAI header, saip4ext.h, to map user-defined private P4 extensions to vendor SAI implementations. More details on saip4ext.h can be found in the supplemental document [P4 Extensions for SAI](#broken-todo).
Copy link
Contributor

Choose a reason for hiding this comment

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

where can we find saip4ext.h?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link

@ravi861 ravi861 Aug 18, 2021

Choose a reason for hiding this comment

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

Here is the SAI HLD.
opencomputeproject/SAI#1293

doc/pins/pins_hld.md Outdated Show resolved Hide resolved
doc/pins/pins_hld.md Outdated Show resolved Hide resolved


## Testing Requirements/Design
The entire PINS code, including the P4RT Application, P4 orchagent, etc is well unit tested. The coverage goal is at least 90% which is already achieved. Integration tests will be added in the following release.
Copy link
Contributor

Choose a reason for hiding this comment

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

do we have sonic-mgmt intergation test planned?

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, in the second release.

doc/pins/pins_hld.md Outdated Show resolved Hide resolved

SONiC relied on basic packet I/O support that uses netdev ports to send and receive packets. P4Runtime applications require additional packet attributes like the target egress port which are not available via netdev. Similarly directed Tx from application level is not currently supported in the SONiC framework. This is required to support scenarios where the remote SDN controller needs to send a packet via one of the front panel ports of the switch.

To support the above requirements, a model similar to sFlow is used. The receive solution creates a netlink type host interface. It programs a user defined trap for packets that are interesting to the controller and maps them to the netlink host interface. A generic ASIC independent model is defined for passing parameters such as “target egress port” to the application container. A detailed description of the receive path is present in the supplementary document on [Packet IO](#broken-todo).
Copy link
Contributor

Choose a reason for hiding this comment

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

where can i find the sai spec of the new netlink device interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The genetlink socket isn't modeled in SAI. It's something that is explicitly added to the vendor kernel module where packets are received from the ASIC. We use standard SAI ACLs to program the punt rules to collect packets from the data plane.

doc/pins/pins_hld.md Outdated Show resolved Hide resolved
@bocon13
Copy link
Contributor Author

bocon13 commented Aug 12, 2021

HLD has been updated with the latest from our internal document and address some of the review comments.

@bocon13 bocon13 marked this pull request as ready for review August 12, 2021 02:24
Includes updates to address Microsoft's comments
* P4RT: An application that receives P4 programming requests from the controller and programs the requests to the APPL DB.
* P4RT tables: a new APPL DB table that defines the P4 programming requests.
* P4Orch: A new orch that programs the P4RT table from APPL DB to ASIC DB. It also sends response notifications to P4RT and manages the APPL STATE DB.
* APPL STATE DB: A new DB for applications to query the system status. It has the same schema of the APPL DB.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please change the name of this new DB? RESPONSE DB? or any other better name.

Copy link
Contributor

Choose a reason for hiding this comment

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

Will change to "APPL Response DB". Having the APPL keyword as part of the DB is to indicate that the schema of the DB reflects the APPL DB and hence it is easy for an application to compare the entry present in this DB with the intent to get an understanding of which parts of the intent have been satisfied. An intent might be partially met and that would be reflected in this DB.


### Application Level Responses

PINS introduces the concept of application level responses. SONiC currently supports synchronous communication between the SWSS and Syncd containers. PINS extends synchronized communication to applications.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Hope we'll have a generic design as much as possible for native SONiC apps as well.

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 goal is to build a response path for all applications - local and remote. The response path is a key missing feature in the current architecture and it would be advantageous for both local and remote applications.

@zhangyanzhao zhangyanzhao merged commit 505135e into sonic-net:master Mar 1, 2022
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.

6 participants