Introduce new extended port oper status notification

[kcudnik]

Bring back compatibility with old structure.

So we are actually in luck, since sizeof(sai_port_oper_status_notification_t) == 16, because of port_id being 64 bit aligned, also as sizeof(sai_extended_port_oper_status_notification_t) == 16, which means we maybe don't need this PR

[kcudnik]

Ok below is explanation with some examples:

before SAI version v1.15, sai_port_oper_status_notification_t structure was containing 2 members, port_id and port_state.

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:

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:

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;

what could be used like this:

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
    data.port_error_status = zzz; // new bitmap field

   fn(1, &data); // callback
}

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 let's use new extended notification type like this:

// 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; // oif 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
}

[kcudnik]

@JaiOCP please take a look

[JaiOCP]

@JaiOCP please take a look

Looks good to me

[JaiOCP]

Do we still need this field. Can it be removed and we just the attr count and attr list?

[kcudnik]

We cna leave it or remove it both ways works fine

[kcudnik]

@JaiOCP are you ok to leave this port_error_status here ? since it was main reason to introduce new port status ? and all new future extended attributes will be added in attr count ? or should we start alrady doint it from attr/list ?

[JaiOCP]

yes, I am ok to leave it here as long as there is an example workflow captured in md doc for future additions.

…

On Mon, Nov 18, 2024 at 1:52 AM Kamil Cudnik ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In inc/saiport.h <#2087 (comment)> : > + * 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; @JaiOCP <https://github.com/JaiOCP> are you ok to leave this port_error_status here ? since it was main reason to introduce new port status ? and all new future extended attributes will be added in attr count ? — Reply to this email directly, view it on GitHub <#2087 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AKCSHLLNTN7XBVKXHXGZIQT2BG2HBAVCNFSM6AAAAABPUSNRDCVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDINBSGAZTSMRWGU> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- This electronic communication and the information and any files transmitted with it, or attached to it, are confidential and are intended solely for the use of the individual or entity to whom it is addressed and may contain information that is confidential, legally privileged, protected by privacy laws, or otherwise restricted from disclosure to anyone else. If you are not the intended recipient or the person responsible for delivering the e-mail to the intended recipient, you are hereby notified that any use, copying, distributing, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited. If you received this e-mail in error, please return the e-mail to the sender, delete it from your computer, and destroy any printed copy of it.

[kcudnik]

i will create documentation for this

[kcudnik]

yes, I am ok to leave it here as long as there is an example workflow captured in md doc for future additions.

@JaiOCP please check, i added documentation md file to this PR, basically it's the same as ahttps://github.com//pull/2087#issuecomment-2450236051 and i preserved port_error_status field in extended structure, so by default attr_count passed should be zero

[JaiOCP]

yes, I am ok to leave it here as long as there is an example workflow captured in md doc for future additions.

@JaiOCP please check, i added documentation md file to this PR, basically it's the same as ahttps://github.com//pull/2087#issuecomment-2450236051 and i preserved port_error_status field in extended structure, so by default attr_count passed should be zero

Thank You. This looks good