Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
88 changes: 88 additions & 0 deletions doc/SAI-Proposal-Global-PTP-Configuration.md
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
{
Copy link
Copy Markdown
Contributor

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.

/** 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
3 changes: 3 additions & 0 deletions inc/saiport.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment thread
rdsugarmannv marked this conversation as resolved.
* at the port level to a value other than SAI_PORT_PTP_MODE_NONE.
Copy link
Copy Markdown
Contributor

@rck-innovium rck-innovium Mar 20, 2025

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The 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

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The 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

I'll be at the SAI meeting today.

*/
typedef enum _sai_port_ptp_mode_t
{
Expand Down
12 changes: 12 additions & 0 deletions inc/saiswitch.h
Original file line number Diff line number Diff line change
Expand Up @@ -3197,6 +3197,18 @@ typedef enum _sai_switch_attr_t
*/
SAI_SWITCH_ATTR_SHARED_BUFFER_CELL_SIZE,

/**
* @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,

/**
* @brief End of attributes
*/
Expand Down
Loading