-
Notifications
You must be signed in to change notification settings - Fork 480
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
TPID SAI proposal #1089
TPID SAI proposal #1089
Conversation
inc/saiswitch.h
Outdated
* @brief Up to 4 TPID can be configured on this switch to any port/LAG | ||
* | ||
* @type bool | ||
* @flags READ_ONLY |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
who enforce the limit of 4 ? SAI or Application
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This switch attribute "SAI_SWITCH_ATTR_TPID_CONFIGURABLE" is the way for SAI vendor to tell the application that the SAI vendor is able to handle 4 or more TPID configuration for this switch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Once SAI indicate it is able to handle TPID config, the application will ensure that it will not exceed more than 4 TPID config (default 0x8100 count towards this count) for the given switch. Application will reject any TPID config attempt if this feature is not supported by SAI for the given switch.
doc/TPID/TPID_SAI_proposal.md
Outdated
* @type bool | ||
* @flags READ_ONLY | ||
*/ | ||
SAI_SWITCH_ATTR_TPID_CONFIGURABLE, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is already general attribute capability query, sai_query_attribute_capability()
why not use the general mechanism instead if tpid_configurable ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you referring to
sai_status_t sai_query_attribute_capability(In sai_object_id_t switch_id,
In sai_object_type_t object_type,
In sai_attr_id_t attr_id,
Out sai_attr_capability_t *attr_capability)
just want to clarify my understanding of this API. So for my purpose you are suggesting to call this API at boot up time and pass the switch_id, the object_type set to "SAI_OBJECT_TYPE_PORT" or "SAI_OBJECT_TYPE_LAG", and the attr_id set to the new "SAI_PORT_ATTR_TPID" or "SAI_LAG_ATTR_TPID" and if the response for the corresponding new attribute_id is "implemented", it guarantee that the SAI is capable of handling at least 4 TPIDs per system?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, the response will be a struct saying whether it's implemented on create, set, and get operations
and we can say it's considered implemented only if all the 4 values mentioned are supported
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good. I am ok with this suggested change if rest of SAI community also agree. We can then eliminate the need of this new switch attribute "SAI_SWITCH_ATTR_TPID_CONFIGURABLE"
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gechiang we need to explicit lay out expectattion that if incoming packet TPID is not match Configured TPID packet should not be discarded by switch and should be classified as L2 Packet (Untag) and everything after MAC SA is considered as payload.
I also feel if above we might have to see if we need to SAI attribute to control Discard Vs Untag behaviour if it is clean approach
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have revised the Proposal documentation to reflect what you suggested in regard to mismatched TPID handling. Please review the new revised version.
Thanks!
…estion to use sai_query_attribute_capability() which does not require the new switch attribute
…SAI community PR review
@itaibaz , @JaiOCP , @tushar-ty, thanks a lot for your input, would you please help review/approve this PR? Thanks. |
No description provided.