Support SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION

[longhuan-cisco]

Note

This PR has dependency on latest OCP SAI attribute SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION, which hasn't been released to sonic-sairedis/SAI. Thus, before its release, it's expected to see build error on this attribute in pre-commit pipeline.

sonic-platform-daemon side change: sonic-net/sonic-platform-daemons#643

What I did
Support SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION

Why I did it

How I verified it
Tested end-to-end xcvrd->OA->SAI/SDK

Details if related

[mssonicbld]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[longhuan-cisco]

@prgeor @mihirpat1 could you please review, thanks

[prgeor]

@longhuan-cisco I am not sure why the attribute count was previously +1, but do you see any harm in maintaining the same this time as well?

[longhuan-cisco]

The previous code was using serdes_attr's size +1 (not attr_list's size + 1).
serdes_attr doesn't contain SAI_PORT_SERDES_ATTR_PORT_ID attribute (i.e. attr_list's first attribute), that's why +1.

It would better just simply use attr_list.size() as attribute count, which is less bug prone.
https://github.com/opencomputeproject/SAI/blob/b925fc0f0fc8c620eea7d9dead7fc99e33b7b44e/inc/saiport.h#L4200-L4214

/**
 * @brief Create port serdes
 *
 * @param[out] port_serdes_id Port serdes id
 * @param[in] switch_id Switch id
 * @param[in] attr_count Number of attributes
 * @param[in] attr_list Array of attributes
 *
 * @return #SAI_STATUS_SUCCESS on success, failure status code on error
 */
typedef sai_status_t (*sai_create_port_serdes_fn)(
        _Out_ sai_object_id_t *port_serdes_id,
        _In_ sai_object_id_t switch_id,
        _In_ uint32_t attr_count,
        _In_ const sai_attribute_t *attr_list);

Besides, here, if we maintain serdes_attr.size()+1 as attribute count, issue will happen because it could end up truncating the last attribute of attr_list (i.e. SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION), which is not in serdes_attr since argument serdes_attr with type std::map<sai_port_serdes_attr_t, std::vector<uint32_t>> can only contain IDRIVER / PREEMPHASIS / etc whose value type is uint32_t list, SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION is passed via argument custom_serdes_attrs_str separately.

[prgeor]

@longhuan-cisco please introduce setPortCustomSerdesAttribute() so that it does not affect old API usage and limits your change to custom API only

[longhuan-cisco]

enum SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION is the same sai_port_serdes_attr_t type as today's IDRIVER / PREEMPHASIS / etc.
And the same SAI API sai_port_api->create_port_serdes is used for all of them.

typedef enum _sai_port_serdes_attr_t
{
    SAI_PORT_SERDES_ATTR_START,

    SAI_PORT_SERDES_ATTR_PORT_ID = SAI_PORT_SERDES_ATTR_START,

    SAI_PORT_SERDES_ATTR_PREEMPHASIS,

    SAI_PORT_SERDES_ATTR_IDRIVER,

...
    SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION,
...
} sai_port_serdes_attr_t;

The logic/code setting IDRIVER / PREEMPHASIS / etc is exactly the same as the one setting CUSTOM_COLLECTION. The only difference is their value type: the value of IDRIVER / PREEMPHASIS / etc is u32list, the value of CUSTOM_COLLECTION is json.
If we have two separate set functions: one setPortSerdesAttribute() for IDRIVER / PREEMPHASIS / etc, another setPortCustomSerdesAttribute() for CUSTOM_COLLECTION. We will end up having lots of duplicate code in these functions (almost the entire function code will be copy/paste), which doesn't look good.

I feel the effect to old API usage is already fairly self-contained:

    if (!custom_serdes_attrs_str.empty())
    {
        port_serdes_attr.id = SAI_PORT_SERDES_ATTR_CUSTOM_COLLECTION;
        port_serdes_attr.value.json.json.count = static_cast<uint32_t>(custom_serdes_attrs_str.size());
        port_serdes_attr.value.json.json.list = reinterpret_cast<int8_t*>(const_cast<char*>(custom_serdes_attrs_str.data()));
        attr_list.emplace_back(port_serdes_attr);
    }

Only when custom_serdes_attrs_str is not empty, the above added code will get exercised.