Introduce new extended port oper status notification

doc/SAI-Proposal-Extended-Port-Notification.md

@@ -0,0 +1,148 @@
+Before SAI version v1.15, *sai_port_oper_status_notification_t* structure was containing 2 members, port_id and port_state.
+
+```C
+typedef struct _sai_port_oper_status_notification_t { // v1.14.0
+    sai_object_id_t port_id;
+    sai_port_oper_status_t port_state;
+} sai_port_oper_status_notification_t;
+```
+
+In vendor use case it could be used like this, when notificaiton was created:
+
+```C
+sai_port_state_change_notification_fn fn = ... ; // pointer obtained from SAI_SWITCH_ATTR_PORT_STATE_CHANGE_NOTIFY
+
+if (fn != NULL)
+{
+    sai_port_oper_status_notification_t data; // sizeof(data) == 16
+
+    data.port_id = xxx; // oif of affected port
+    data.port_state = yyy; // actual port status
+
+    fn(1, &data); // callback
+}
+```
+
+In SAI version v1.15 breaking change was introduced:
+
+```C
+typedef struct _sai_port_oper_status_notification_t { // v1.15.0
+    sai_object_id_t port_id;
+    sai_port_oper_status_t port_state;
+    sai_port_error_status_t port_error_status;
+} sai_port_oper_status_notification_t;
+```
+
+Which added 3rd field to notification (port_error_status).
+
+Which could be used like this:
+
+```C
+sai_port_state_change_notification_fn fn = ... ; // pointer obtained from SAI_SWITCH_ATTR_PORT_STATE_CHANGE_NOTIFY
+
+if (fn != NULL)
+{
+    sai_port_oper_status_notification_t data; // sizeof(data) == 16
+
+    data.port_id = xxx; // oid of affected port
+    data.port_state = yyy; // actual port status
+    data.port_error_status = zzz; // new bitmap field
+
+    fn(1, &data); // callback
+}
+```
+
+Fortunetly that change didn't change sizeof(sai_port_oper_status_notification_t) == 16, which is not impatcing
+sai_port_state_change_notification_fn data pointer, which would cause troubles when iterating over notification
+data using next indexes, since allignment of structures would be off.
+
+Now this is my proposal, we bring back structure sai_port_oper_status_notification_t to as it was in v1.14
+by removing port_error_status field, and introduce attr_count and attr_list fields:
+
+```C
+typedef struct _sai_extended_port_oper_status_notification_t
+{
+    /**
+     * @brief Port id.
+     *
+     * @objects SAI_OBJECT_TYPE_PORT, SAI_OBJECT_TYPE_BRIDGE_PORT, SAI_OBJECT_TYPE_LAG
+     */
+    sai_object_id_t port_id;
+
+    /** Port operational status */
+    sai_port_oper_status_t port_state;
+
+    /** Bitmap of various port error or fault status */
+    sai_port_error_status_t port_error_status;
+
+    /** Attributes count */
+    uint32_t attr_count;
+
+    /**
+     * @brief Attributes
+     *
+     * Object type NULL specifies that attribute list is for object type
+     * specified in port_id field. For example if port_id field contains LAG
+     * object then list of attributes contains SAI_LAG_ATTR_* attributes.
+     *
+     * @objects SAI_OBJECT_TYPE_NULL
+     */
+    sai_attribute_t *attr_list;
+
+} sai_extended_port_oper_status_notification_t;
+```
+
+This could be strucutre used from SAI v1.16.0.
+Let's use new extended notification type like this:
+
+```C
+// still support previous notification
+
+sai_port_state_change_notification_fn fn = ... ; // obtained from SAI_SWITCH_ATTR_PORT_STATE_CHANGE_NOTIFY
+
+if (fn != NULL)
+{
+    sai_port_oper_status_notification_t data; // sizeof(data) == 16
+
+    data.port_id = xxx; // oif of affected port
+    data.port_state = yyy; // actual port status
+
+    fn(1, &data); // callback
+}
+
+// here support new notification
+
+sai_extended_port_state_change_notification_fn extfn = ...; // pointer obtained from SAI_SWITCH_ATTR_EXTENDED_PORT_STATE_CHANGE_NOTIFY
+
+if (extfn != NULL)
+{
+    sai_extended_port_oper_status_notification_t data;
+
+    data.port_id = xxx; // oid of affected port
+    data.port_state = yyy; // actual port status
+    data.port_error_status = zzz; // bitmap value of port error status
+
+    sai_attribute_t list[1];
+
+    // example attribute with value at the time of the event for port_id fired
+
+    list[0].id = SAI_PORT_ATTR_LINK_TRAINING_RX_STATUS;
+    list[0].value.s32 = SAI_PORT_LINK_TRAINING_FAILURE_STATUS_NO_ERROR;
+
+    data.attr_count = 1;
+    data.attr_list = list;
+
+    fn(1, &data); // callback for extended notification
+}
+```
+
+With this solution we are still backward compatible with old notification and we also can support new one.
+Advantage is that we can add as many new members to the attr_list as we want not breaking compatibility in the
+future.
+
+The only hiccup is that we have 2 breaking changes:
+* SAI version v1.14.0 sai_port_oper_status_notification_t have 2 fields (sizeof == 16)
+* SAI version v1.15.0 sai_port_oper_status_notification_t have 3 fields (sizeof == 16) - breaking changea - adding port_error_status
+* SAI version v1.16.0 sai_port_oper_status_notification_t have 2 fields (sizeof == 16) - breaking change - removing port_error_status
+
+but we will keep compatibility over all previous and future version for this strucute, and all new extensions would be added in new notification.

inc/saiport.h

@@ -141,9 +141,48 @@ typedef struct _sai_port_oper_status_notification_t
     /** Port operational status */
     sai_port_oper_status_t port_state;
 
+} sai_port_oper_status_notification_t;
+
+/**
+ * @brief Defines the extended operational status of the port
+ *
+ * Any additional data will must be passed on attributes list. Usually that
+ * will be port attributes that are READ_ONLY and the value will represent the
+ * state of given attribute for port_id object at the time that notification
+ * was generated.
+ *
+ * @count attr_list[attr_count]
+ */
+typedef struct _sai_extended_port_oper_status_notification_t
+{
+    /**
+     * @brief Port id.
+     *
+     * @objects SAI_OBJECT_TYPE_PORT, SAI_OBJECT_TYPE_BRIDGE_PORT, SAI_OBJECT_TYPE_LAG
+     */
+    sai_object_id_t port_id;
+
+    /** Port operational status */
+    sai_port_oper_status_t port_state;
+
     /** Bitmap of various port error or fault status */
     sai_port_error_status_t port_error_status;
-} sai_port_oper_status_notification_t;
+
+    /** Attributes count */
+    uint32_t attr_count;
+
+    /**
+     * @brief Attributes
+     *
+     * Object type NULL specifies that attribute list is for object type
+     * specified in port_id field. For example if port_id field contains LAG
+     * object then list of attributes contains SAI_LAG_ATTR_* attributes.
+     *
+     * @objects SAI_OBJECT_TYPE_NULL
+     */
+    sai_attribute_t *attr_list;
+
+} sai_extended_port_oper_status_notification_t;
 
 /**
  * @brief Attribute data for #SAI_PORT_ATTR_GLOBAL_FLOW_CONTROL_MODE
@@ -3502,6 +3541,20 @@ typedef void (*sai_port_state_change_notification_fn)(
         _In_ uint32_t count,
         _In_ const sai_port_oper_status_notification_t *data);
 
+/**
+ * @brief Extended port state change notification
+ *
+ * Passed as a parameter into sai_initialize_switch()
+ *
+ * @count data[count]
+ *
+ * @param[in] count Number of notifications
+ * @param[in] data Array of port operational status
+ */
+typedef void (*sai_extended_port_state_change_notification_fn)(
+        _In_ uint32_t count,
+        _In_ const sai_extended_port_oper_status_notification_t *data);
+
 /**
  * @brief Port host tx ready notification
  *

inc/saiswitch.h

@@ -3070,6 +3070,20 @@ typedef enum _sai_switch_attr_t
      */
     SAI_SWITCH_ATTR_SELECTIVE_COUNTER_LIST,
 
+    /**
+     * @brief Extended port state change notification callback function passed to the adapter.
+     *
+     * In case driver does not support this attribute, The Host adapter should poll
+     * port status by SAI_PORT_ATTR_OPER_STATUS.
+     *
+     * Use sai_extended_port_state_change_notification_fn as notification function.
+     *
+     * @type sai_pointer_t sai_extended_port_state_change_notification_fn
+     * @flags CREATE_AND_SET
+     * @default NULL
+     */
+    SAI_SWITCH_ATTR_EXTENDED_PORT_STATE_CHANGE_NOTIFY,
+
     /**
      * @brief End of attributes
      */

meta/saisanitycheck.c

@@ -6005,6 +6005,7 @@ void check_struct_and_union_size()
     CHECK_STRUCT_SIZE(sai_port_snr_list_t, 16);
     CHECK_STRUCT_SIZE(sai_port_snr_values_t, 8);
     CHECK_STRUCT_SIZE(sai_port_oper_status_notification_t, 16);
+    CHECK_STRUCT_SIZE(sai_extended_port_oper_status_notification_t, 32);
     CHECK_STRUCT_SIZE(sai_prbs_rx_state_t, 8);
     CHECK_STRUCT_SIZE(sai_qos_map_list_t, 16);
     CHECK_STRUCT_SIZE(sai_qos_map_params_t, 16);

meta/saiserializetest.c

@@ -1412,9 +1412,16 @@ void test_serialize_notifications()
     memset(&data1, 0, sizeof(data1));
 
     res = sai_serialize_port_state_change_notification(buf, 1, &data1);
-    ret = "{\"count\":1,\"data\":[{\"port_id\":\"oid:0x0\",\"port_state\":\"SAI_PORT_OPER_STATUS_UNKNOWN\",\"port_error_status\":\"SAI_PORT_ERROR_STATUS_CLEAR\"}]}";
+    ret = "{\"count\":1,\"data\":[{\"port_id\":\"oid:0x0\",\"port_state\":\"SAI_PORT_OPER_STATUS_UNKNOWN\"}]}";
     ASSERT_STR_EQ(buf, ret , res);
 
+    sai_extended_port_oper_status_notification_t data1e;
+    memset(&data1e, 0, sizeof(data1e));
+
+    res = sai_serialize_extended_port_state_change_notification(buf, 1, &data1e);
+    ret = "{\"count\":1,\"data\":[{\"port_id\":\"oid:0x0\",\"port_state\":\"SAI_PORT_OPER_STATUS_UNKNOWN\",\"port_error_status\":\"SAI_PORT_ERROR_STATUS_CLEAR\",\"attr_count\":0,\"attr_list\":null}]}";
+    ASSERT_STR_EQ(buf, ret, res);
+
     sai_queue_deadlock_notification_data_t data2;
     memset(&data2, 0, sizeof(data2));
 

meta/structs.pl

@@ -173,8 +173,15 @@ sub BuildCommitHistory
 
         if ($histCount > $currCount)
         {
-            LogError "FATAL: $structTypeName members were removed on commit $commit, NOT ALLOWED!";
-            exit 1;
+            if ($structTypeName eq "sai_port_oper_status_notification_t")
+            {
+                # we allow this to change back backward compatibility
+            }
+            else
+            {
+                LogError "FATAL: $structTypeName members were removed on commit $commit, NOT ALLOWED!";
+                exit 1;
+            }
         }
 
         my $minCount = ($histCount > $currCount) ? $currCount : $histCount;