-
Notifications
You must be signed in to change notification settings - Fork 583
Add switch-level attribute to control PTP for the entire switch #2148
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,88 @@ | ||
| # Global PTP Configuration in SAI Switch | ||
|
|
||
| Title | Global PTP Configuration in SAI Switch | ||
| ------------|---------------------------------------- | ||
| Authors | Open | ||
| Status | Draft | ||
| Type | Standards track | ||
| Created | 2024-03-03 | ||
| SAI-Version | 1.15 | ||
|
|
||
| ## Overview | ||
| Introducing a PTP (Precision Time Protocol) attribute to the SAI switch object, allowing for switch-wide PTP mode settings that affect all ports on the switch. | ||
|
|
||
| A switch that does not set this attribute will observe no change in behavior. | ||
|
|
||
| ## Motivation | ||
| Currently, PTP configuration in SAI is port-based, and many switches only support configuration of PTP on a global basis. | ||
| In addition, many existing switches have a configuration model which allows global configuration of PTP, and then override on a port level. SAI should follow this flexible example. | ||
|
|
||
| ## Technical Specification | ||
|
|
||
| ### Updated description of existing PORT PTP type - the main comment is new | ||
| ```c | ||
| /** | ||
| * @brief PTP mode | ||
| * | ||
| * These modes can be used at the port and switch level. | ||
| * All ports use the value set at the switch level unless explicitly configured | ||
| * at the port level to a value other than SAI_PORT_PTP_MODE_NONE. | ||
| */ | ||
| typedef enum _sai_port_ptp_mode_t | ||
| { | ||
| /** No special processing for PTP packets */ | ||
| SAI_PORT_PTP_MODE_NONE, | ||
|
|
||
| /** Single-step Timestamp mode for the PTP packets */ | ||
| SAI_PORT_PTP_MODE_SINGLE_STEP_TIMESTAMP, | ||
|
|
||
| /** Two-step Timestamp mode for the PTP packets */ | ||
| SAI_PORT_PTP_MODE_TWO_STEP_TIMESTAMP, | ||
|
|
||
| } sai_port_ptp_mode_t; | ||
|
|
||
|
|
||
| ### New Switch Attribute | ||
| ```c | ||
| typedef enum _sai_switch_attr_t | ||
| { | ||
| // ... existing attributes ... | ||
|
|
||
| /** | ||
| * @brief Global PTP mode configuration | ||
| * | ||
| * Global PTP mode configuration for the switch. | ||
| * Applies to all ports unless overridden by port-specific settings. | ||
| * | ||
| * @type sai_port_ptp_mode_t | ||
| * @flags CREATE_AND_SET | ||
| * @default SAI_PORT_PTP_MODE_NONE | ||
| */ | ||
| SAI_SWITCH_ATTR_PORT_PTP_MODE, | ||
|
|
||
| // ... existing attributes ... | ||
| } sai_switch_attr_t; | ||
| ``` | ||
|
|
||
| ## Usage Example | ||
| ```c | ||
| // Set global PTP configuration at switch level | ||
| sai_attribute_t attr; | ||
| attr.id = SAI_SWITCH_ATTR_PORT_PTP_MODE; | ||
| attr.value.s32 = SAI_PORT_PTP_MODE_SINGLE_STEP_TIMESTAMP; | ||
|
|
||
| sai_status_t status = sai_switch_api->set_switch_attribute( | ||
| switch_id, | ||
| &attr); | ||
|
|
||
| // Get global PTP configuration | ||
| attr.id = SAI_SWITCH_ATTR_PORT_PTP_MODE; | ||
| status = sai_switch_api->get_switch_attribute( | ||
| switch_id, | ||
| 1, | ||
| &attr); | ||
| ``` | ||
|
|
||
| ## References | ||
| 1. IEEE 1588-2008 Standard for a Precision Clock Synchronization Protocol for Networked Measurement and Control Systems | ||
| 2. IEEE 1588-2019 Standard for a Precision Clock Synchronization Protocol for Networked Measurement and Control Systems | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -395,6 +395,9 @@ typedef enum _sai_port_priority_flow_control_mode_t | |
|
|
||
| /** | ||
| * @brief PTP mode | ||
| * These modes can be used at the port and switch level. | ||
| * All ports use the value set at the switch level unless explicitly configured | ||
|
rdsugarmannv marked this conversation as resolved.
|
||
| * at the port level to a value other than SAI_PORT_PTP_MODE_NONE. | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @kcudnik We want SAI_SWITCH_ATTR_PTP_MODE (new) and SAI_PORT_ATTR_PTP_MODE (existing) to use the same enums. Today, SAI_PORT_ATTR_PTP_MODE is using sai_port_ptp_mode_t. Is there a way to deprecate sai_port_ptp_mode_t and instead use a new sai_ptp_mode_t without breaking backward compatibility? The enum values will be the same.
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can change enum in switch attributes in new one, and add in comments that this is deprecated, it will introduce issues in serialization and deserializstion in sonic sairedis where we would need to add manual conversion for both of those enums, we can discuss this on sai community meeting today Is there a reason that ptp mode was introduced in switch if it applies to ports anyway ? Or you just want to have anum without port I name? If values will be the same then it seems like one of those enums is redundant and should not be added on the first place
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And it seems like switch attributes should be named SAI SWITCH ATTR PORT PTP MODE maybe I think description of this attributes explains it, but also it seems like this attributes is redundant, if this only applies to ports, then this attribute will n3ver have effect since each time port ptp mode will be different, port always overrides this, not sure why this was added at the first place, let's discuss on sai meeting g
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I don't see an obvious way to deprecate the old names that doesn't introduce confusion. There are other examples that share enums the way I've done here. For example, see how sai_tunnel_encap_ecn_mode_t is used by both SAI_SWITCH_TUNNEL_ATTR_TUNNEL_ENCAP_ECN_MODE and SAI_TUNNEL_ATTR_ENCAP_ECN_MODE.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I'll be at the SAI meeting today. |
||
| */ | ||
| typedef enum _sai_port_ptp_mode_t | ||
| { | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the comments that are addressed in the C header files needs to be reflect in this MD file as well.