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

PoE design proposal HLD document #1631

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

plvisiondevs
Copy link

Add PoE design proposal HLD document with the following content:

  • Requirements
  • Component interactions
  • DB table definition
  • CLI

In order to isolate PoE functionality and complexity from existing SONiC design, the following main changes are made:

  • Add separate container for PoE control plane routine operations.
  • Introduce separate SYNCD container to support external PoE Abstraction Interface.
  • Add new PoE tables into database container.

SAI community changes #1977

Signed-off-by: Volodymyr Mytnyk <volodymyr.mytnyk@plvision.eu>
Copy link

linux-foundation-easycla bot commented Mar 11, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

@zhangyanzhao
Copy link
Collaborator

@kishorgovind
Copy link
Collaborator

Design is not according to SONiC design philosophy. Without significant requirement or necessity another syncD has been added for POE. Multiple syncD module in SONiC architecture is not a SONiC design philosophy. Also PoE is a hardware feature i.e. feature attached to physical interface and not related to ASIC forwarding. Hence using syncD module for management of this feature is not a right way of feature designing in SONiC. Here we have to follow management POE feature same way how PMON manages fan and power using PDDF framework. In recording team was mentioning about performance impact if we use PDDF framework but I could not get right justification.

- add SWSS cahnges
- include LLDP interaction
- add example PoE config
- mention about warm reboot requirements

Signed-off-by: Volodymyr Mytnyk <volodymyr.mytnyk@plvision.eu>
@kishorgovind
Copy link
Collaborator

My review comment is not addressed yet.

@taraschornyiplv
Copy link

taraschornyiplv commented Apr 3, 2024

Design is not according to SONiC design philosophy. Without significant requirement or necessity another syncD has been added for POE. Multiple syncD module in SONiC architecture is not a SONiC design philosophy. Also PoE is a hardware feature i.e. feature attached to physical interface and not related to ASIC forwarding. Hence using syncD module for management of this feature is not a right way of feature designing in SONiC. Here we have to follow management POE feature same way how PMON manages fan and power using PDDF framework. In recording team was mentioning about performance impact if we use PDDF framework but I could not get right justification.

We want to utilize SAI as the standard API for PoE. This will allow PoE chip vendors to provide SAI library for their PoE devices. Usually, NPU and PoE chips are from different vendors, thus a need for a separate syncd container.
Also in SONiC multiple syncd containers are already available to support PHY chips.

@RogerX87
Copy link

RogerX87 commented Apr 12, 2024

Is there any plan about SNMP? I think SNMP can also control the PoE.

@zhangyanzhao
Copy link
Collaborator

Can we also have the code PRs listed in this HLD PR? Thanks.


#### PoE SYNCD

New PoE SYNCD container componnents:
Copy link
Contributor

Choose a reason for hiding this comment

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

what is the reason to have a poe syncd? why normal syncd won't work. isn't that poe feature is captured with SAI api, then normal syncd can support poe as long as the sai library support poe?

Choose a reason for hiding this comment

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

I believe the motivation is to allow use of an additional instance of SAI. one for NPU and one for PoE. This is assuming that NPU and PoE HW and drivers are developed by different vendors


### PoE Abstraction Interface

The PoE Manager uses a new SAI PoE library that implements the PoE Abstraction Interface.
Copy link
Contributor

Choose a reason for hiding this comment

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

can you share the poe sai library pr? is it already merged in sai?

Choose a reason for hiding this comment

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

SAI PR -opencomputeproject/SAI#1977 (merged)
Implementation will be (PoE HW) vendor specific, we will be providing an example, working implementation for MicroChip PSE (in development)

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