add Passive Optical Network (PON) to experimental by dpelton-ciena · Pull Request #2350 · opencomputeproject/SAI

dpelton-ciena · 2026-09-09T22:16:52Z

This PR adds API definitions to control Passive Optical Network (PON) hardware components. These are added to the experimental SAI headers.

The changes cover the objects necessary to implement a data center based PON deployment as described in these SONiC PRs:
PON HLD
Dual Homed PON gateway HLD

As the SONiC features are planned for the SONiC 202611 release, we are proposing these changes for inclusion in the 1.19 SAI release.

Signed-off-by: David Pelton <dpelton@ciena.com>

azure-pipelines · 2026-09-09T22:17:01Z

Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run.

kcudnik · 2026-09-10T18:43:57Z

+ */
+typedef enum _sai_pon_action_type_t
+{
+    SAI_PON_ACTION_TYPE_OLT_RESET                   = 0,


no need for expicit numeration if numbers are continous

and add empty line before each enum

kcudnik · 2026-09-10T18:44:40Z

+    sai_bulk_get_pon_onu_statistics_accumulating_olt_pon_omcc_attribute_fn                                                            get_pon_onu_statistics_accumulating_olt_pon_omccs_attribute;
+    sai_bulk_get_pon_onu_statistics_accumulating_olt_pon_service_attribute_fn                                                         get_pon_onu_statistics_accumulating_olt_pon_services_attribute;
+    sai_bulk_get_pon_onu_statistics_accumulating_onu_enhanced_tc_pm_attribute_fn                                                      get_pon_onu_statistics_accumulating_onu_enhanced_tc_pms_attribute;
+    sai_bulk_get_pon_onu_statistics_accumulating_onu_ethernet_frame_extended_pm_attribute_fn                                          get_pon_onu_statistics_accumulating_onu_ethernet_frame_extended_pms_attribute;


why those naming must be so long ?

kcudnik · 2026-09-10T18:44:52Z

+    sai_create_pon_service_config_profile_extended_vlan_tagging_operation_config_data_received_frame_vlan_tagging_operation_fn        create_pon_service_config_profile_extended_vlan_tagging_operation_config_data_received_frame_vlan_tagging_operation;
+    sai_remove_pon_service_config_profile_extended_vlan_tagging_operation_config_data_received_frame_vlan_tagging_operation_fn        remove_pon_service_config_profile_extended_vlan_tagging_operation_config_data_received_frame_vlan_tagging_operation;
+    sai_set_pon_service_config_profile_extended_vlan_tagging_operation_config_data_received_frame_vlan_tagging_operation_attribute_fn set_pon_service_config_profile_extended_vlan_tagging_operation_config_data_received_frame_vlan_tagging_operation_attribute;
+    sai_get_pon_service_config_profile_extended_vlan_tagging_operation_config_data_received_frame_vlan_tagging_operation_attribute_fn get_pon_service_config_profile_extended_vlan_tagging_operation_config_data_received_frame_vlan_tagging_operation_attribute;


this naming is insane !

kcudnik · 2026-09-10T18:53:48Z

+    if ((!defined $type or $type eq "") and $attr eq "SAI_ACL_COUNTER_ATTR_TABLE_ID")
+    {
+        LogWarning "missing type for $attr, defaulting to sai_object_id_t";
+        return "SAI_ATTR_VALUE_TYPE_OBJECT_ID";


why this condition is added?

kcudnik · 2026-09-10T18:57:33Z

-        $APITOOBJMAP{$api} = \@objects;
+        if (defined $APITOOBJMAP{$api})
+        {
+            my %seen = map { $_ => 1 } @{ $APITOOBJMAP{$api} };
+
+            for my $obj (@objects)
+            {
+                next if $seen{$obj};
+                push @{ $APITOOBJMAP{$api} }, $obj;
+                $seen{$obj} = 1;
+            }
+        }
+        else
+        {
+            $APITOOBJMAP{$api} = \@objects;
+        }
+
+        if (defined $API_METHODS{$api})
+        {
+            for my $name (keys %methods)
+            {
+                $API_METHODS{$api}{$name} = 1;
+            }
+        }
+        else
+        {
+            $API_METHODS{$api} = \%methods;


each header should have its own apid and this logic would not be needed, you are breaking header design here !

please follow entire SAI design

Thanks for the feedback. A number of these changes were made such that the header file could be broken up into smaller pieces. We will revert these changes.

you can still break this to smaller modules, just eachmodule should have it's own API
and not all put in single SAI_API_PON

kcudnik · 2026-09-10T18:57:55Z

    return if $fname eq "sai_bulk_object_clear_stats_fn"; # exception

-    if (not $fname =~ /^sai_((get|clear)_(\w+)_stats|get_\w+_stats_ext)_fn$/)
+    if (not $fname =~ /^sai_((get|clear)_(\w+)_stat(s)?|get_\w+_stat(s)?_ext)_fn$/)


why this condition is added?

kcudnik · 2026-09-10T18:58:33Z

    $order =~ s/012/s/g;        # order should be: get_stats,get_stats_ext,clear_stats
    $order =~ s/CR/E/g;         # order should be: bulk_create,bulk_remove
    $order =~ s/SG/T/g;         # order should be: bulk_set,bulk_get
+    $order =~ s/g+/g/g;         # order can include runs of read-only get attribute APIs


why such api exists only ?

kcudnik · 2026-09-10T18:58:51Z

            my $itemname = $2;

-            if ($1 ne $spaces or (length($2) != length($inside) and $struct =~ /_api_t/))
+            if ($1 ne $spaces or (length($2) != length($inside) and $struct =~ /_api_t/ and $struct !~ /sai_pon_api_t/))


you are breaking design here

kcudnik · 2026-09-10T18:58:59Z

            next if $line =~ m![^\\]\\$!;           # macro multiline
            next if $line =~ /^ {4}(\w+);$/;        # union entries
            next if $line =~ /^union _sai_\w+ \{/;  # union entries
+            next if $line =~ /^ {20,}\w+/;          # api struct member lines


why this condition is added?

initial PON HLD + header files

a9d382d

Signed-off-by: David Pelton <dpelton@ciena.com>

kcudnik requested changes Sep 10, 2026

View reviewed changes

Uh oh!

Conversation

dpelton-ciena commented Sep 9, 2026

Uh oh!

azure-pipelines Bot commented Sep 9, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants