[DASH] Update dash headers to most recent #1834
Conversation
Updates DASH experimental headers to headers generated from https://github.com/sonic-net/DASH repo on origin/main
|
@marian-pritsak, @Pterosaur, @chrispsommers please review |
|
@reshmaintel Could you or your team review? Thanks. |
There was a problem hiding this comment.
Changes seem OK, but it would be helpful to include in the PR description from which commit of DASH, these correspond to. If my understanding is correct, these are hand-copied from generated .h files in DASH repo. There is no way to automatically link back to the code commit. Thanks!
yes, those are hand copied from generated files, i discussed this with Guohan, that we should treat DASH repo like a ground truth for DASH api what description do you have in mind? |
|
It could be as simple as "Copied generated headers from DASH project after building code with commit XYZ." |
| * @brief List matched key sip | ||
| * @brief Optional matched key sip | ||
| * | ||
| * @type sai_ip_prefix_list_t | ||
| * @type sai_ip_address_t | ||
| * @flags MANDATORY_ON_CREATE | CREATE_ONLY | ||
| */ | ||
| SAI_DASH_ACL_RULE_ATTR_SIP, | ||
|
|
||
| /** | ||
| * @brief List matched key protocol | ||
| * @brief Optional matched key protocol | ||
| * | ||
| * @type sai_u8_list_t | ||
| * @type sai_uint8_t | ||
| * @flags MANDATORY_ON_CREATE | CREATE_ONLY | ||
| */ | ||
| SAI_DASH_ACL_RULE_ATTR_PROTOCOL, | ||
|
|
||
| /** | ||
| * @brief Range_list matched key src_port | ||
| * @brief Optional matched key src_port | ||
| * | ||
| * @type sai_u16_range_list_t | ||
| * @type sai_uint16_t | ||
| * @flags MANDATORY_ON_CREATE | CREATE_ONLY | ||
| * @isvlan false | ||
| */ | ||
| SAI_DASH_ACL_RULE_ATTR_SRC_PORT, | ||
|
|
||
| /** | ||
| * @brief Range_list matched key dst_port | ||
| * @brief Optional matched key dst_port | ||
| * | ||
| * @type sai_u16_range_list_t | ||
| * @type sai_uint16_t | ||
| * @flags MANDATORY_ON_CREATE | CREATE_ONLY | ||
| * @isvlan false |
There was a problem hiding this comment.
To the DASH ACL SAI, it's not directly generated from P4 code, you need to unmark the line: https://github.com/sonic-net/DASH/blob/b322bc6e3e4625de9418fdc1615ee7144ca76ecb/dash-pipeline/bmv2/dash_acl.p4#L16 for generating them
Here is a different point between SAI and default P4 of BMv2.
There was a problem hiding this comment.
why this needs to be unmarked ?
There was a problem hiding this comment.
In our design for DASH project, here should be a list, like u16_range_list, u8_list, and ip_prefix_list. But the current implementation of BMv2 doesn't support the list type. Most of the APIs are developed on BMv2 as the prototype. So, if we want to run P4 code on BMv2, this comment should be marked, if we want to generate SAI headers, this comment should be unmarked.
| * @type sai_uint32_t | ||
| * @flags MANDATORY_ON_CREATE | CREATE_ONLY | ||
| */ | ||
| SAI_DASH_ACL_RULE_ATTR_DST_TAG, | ||
|
|
||
| /** | ||
| * @brief Ternary matched mask dst_tag | ||
| * | ||
| * @type sai_uint32_t | ||
| * @flags MANDATORY_ON_CREATE | CREATE_ONLY | ||
| */ | ||
| SAI_DASH_ACL_RULE_ATTR_DST_TAG_MASK, | ||
|
|
||
| /** | ||
| * @brief Ternary matched key src_tag | ||
| * | ||
| * @type sai_uint32_t | ||
| * @flags MANDATORY_ON_CREATE | CREATE_ONLY | ||
| */ | ||
| SAI_DASH_ACL_RULE_ATTR_SRC_TAG, | ||
|
|
||
| /** | ||
| * @brief Ternary matched mask src_tag | ||
| * | ||
| * @type sai_uint32_t | ||
| * @flags MANDATORY_ON_CREATE | CREATE_ONLY | ||
| */ | ||
| SAI_DASH_ACL_RULE_ATTR_SRC_TAG_MASK, | ||
|
|
||
| /** | ||
| * @brief Optional matched key dip | ||
| * | ||
| * @type sai_ip_address_t |
There was a problem hiding this comment.
I have submitted a PR (#1829) for the TAG feature, this design is reviewed by @marian-pritsak
There was a problem hiding this comment.
is that different from auto generated ?
There was a problem hiding this comment.
The TAG feature is still being discussed, so we don't have a final conclusion for it.
There was a problem hiding this comment.
does it matter at this stage? since it's still experimental, does anyone use it at all at that point? it could be updated to keep headers in sync, and changed then at any point
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
Updates DASH experimental headers to headers generated from https://github.com/sonic-net/DASH repo on origin/main
Copied generated headers from DASH project after building code with commit b322bc6 (sonic-net/DASH@b322bc6)