SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX flags

[kcudnik]

119     /**
120      * @brief Encapsulation Index
121      *
122      * Defines the neighbor's encapsulation index
123      *
124      * @type sai_uint32_t
125      * @flags CREATE_AND_SET
126      * @default 0
127      */
128     SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX,

Based on sonic-net/sonic-sairedis#725, seems like for some cases this attribute is CREATE_AND_SET and for some should be READ_ONLY. When this attribute is set by internal SAI, then default value should be set to "internal" and never change during runtime. And in this case it could actually be READ_ONLY, but for remote neighbors seems like this can be create_and_set.
We need to solve this, for example introducing another RO attribute

@vganesan-nokia, @lguohan

[michaelli10]

@kcudnik , would setting "SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX" to default of "internal" resolve the issue? If no value is provided by the application, SAI will assign an internally generated value. Is there some meta checker restriction that a default "internal" value can never change during runtime?

[kcudnik]

yes, setting default internal will solve the issue, and once assigned ether by user or internal SAI, value should not change during runtime until explicitly changed since value marked as "SET"

[lguohan]

@rlhui , is this resolved?

[kcudnik]

no, seems like still not resolved

[michaelli10]

Hi @kcudnik,
Adding the email thread from Rita with the agreed resolution below. Looks like the changes have not been added yet. I'll circle back with the team on adding the changes ASAP.

We had a sync with @kamil Cudnik , @guohan Lu, @Gen-Hwa Chiang on this.
Fix for this is to change to “@default internal”.
What this means is vendor SAI will assign a value upon neighbor_entry creation, if application does not set the encap_index attribute value at that time.

Application then queries the attribute to find out the actual value assigned.
The difference from before, was application cannot assume its default assigned value is 0.
Secondly, from SAI api point of view, there is no need to have the below flag:
SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_IMPOSE_INDEX
If application wants to set the encap_index attribute, it can go ahead to set the attribute value, then vendor SAI just takes the value.
Suggestion is to deprecate the flag SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_IMPOSE_INDEX in SAI API.

[rlhui]

Hi @kcudnik,
Adding the email thread from Rita with the agreed resolution below. Looks like the changes have not been added yet. I'll circle back with the team on adding the changes ASAP.

We had a sync with @kamil Cudnik , @guohan Lu, @Gen-Hwa Chiang on this.
Fix for this is to change to “@default internal”.
What this means is vendor SAI will assign a value upon neighbor_entry creation, if application does not set the encap_index attribute value at that time.
Application then queries the attribute to find out the actual value assigned.
The difference from before, was application cannot assume its default assigned value is 0.
Secondly, from SAI api point of view, there is no need to have the below flag:
SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_IMPOSE_INDEX
If application wants to set the encap_index attribute, it can go ahead to set the attribute value, then vendor SAI just takes the value.
Suggestion is to deprecate the flag SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_IMPOSE_INDEX in SAI API.

correct. Discussion was closed and resolved but pending Brcm to raise PR for this. Thanks.

[rlhui]

@SarathBug from Brcm will address this. Thanks a lot.

[SarathBug]

@kcudnik @rlhui

I am seeing meta build error after changing default value from 0 to internal

    /**
     * @brief Encapsulation Index
     *
     * Defines the neighbor's encapsulation index
     *
     * @type sai_uint32_t
     * @flags CREATE_AND_SET
     * @default internal
     */
    SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX,

ASSERT FAILED (on line 1188): SAI_NEIGHBOR_ENTRY_ATTR_ENCAP_INDEX: default internal currently can be set only on read only objects
Makefile:79: recipe for target 'all' failed
make: *** [all] Error 1
Build step 'Execute shell' marked build as failure

[kcudnik]

please relax saisanitycheck.c condition on 1178, add case for this scenario