Skip to content

Add fabric port isolation attribute#1498

Merged
rlhui merged 6 commits intoopencomputeproject:masterfrom
jfeng-arista:add_fabric_port_isolation_api
Sep 19, 2022
Merged

Add fabric port isolation attribute#1498
rlhui merged 6 commits intoopencomputeproject:masterfrom
jfeng-arista:add_fabric_port_isolation_api

Conversation

@jfeng-arista
Copy link
Copy Markdown
Contributor

Add fabric port isolation attribute

@jfeng-arista jfeng-arista force-pushed the add_fabric_port_isolation_api branch from 45afea2 to 74a6a05 Compare June 17, 2022 18:59
@kcudnik
Copy link
Copy Markdown
Collaborator

kcudnik commented Jun 20, 2022

WARNING: line ends in whitespace saiport.h 1888: * @brief Fabric port isolation setting

@jfeng-arista jfeng-arista force-pushed the add_fabric_port_isolation_api branch from d8646a9 to cffac35 Compare June 23, 2022 21:43
@kcudnik
Copy link
Copy Markdown
Collaborator

kcudnik commented Jun 24, 2022

still some issues with build, please fix

@jfeng-arista jfeng-arista force-pushed the add_fabric_port_isolation_api branch 2 times, most recently from ff38010 to 7f85b00 Compare June 24, 2022 23:57
@jfeng-arista jfeng-arista force-pushed the add_fabric_port_isolation_api branch 2 times, most recently from c1a76a7 to 457edbf Compare July 11, 2022 16:45
* MAC level, but the link partner will not use
* it for traffic distribution.
* false: Undo the isolation operation.
* This attribute is for fabric links only.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add something like this in the comment section.
Refer sairouterinterface.h

@condition SAI_PORT_ATTR_TYPE == SAI_PORT_TYPE_FABRIC

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi,

SAI_PORT_ATTR_TYPE is a read only attribute , and as far as I know that @condition not works for read only attribute. So checking with you again about if we need to make this change ?

thank you

Jie

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried add the @condition, and the metadata check failed,

it looks like based on the following 3 checks, we can not use @condition check for read only attribute. thank you

switch ((int)md->flags)

switch ((int)md->flags)

META_MD_ASSERT_FAIL(md, "read only attribute can't be valid only");

Copy link
Copy Markdown
Collaborator

@abdosi abdosi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please check above comment.

@jfeng-arista jfeng-arista force-pushed the add_fabric_port_isolation_api branch 3 times, most recently from 7dc052f to 1c3ecd5 Compare July 18, 2022 21:40
inc/saiport.h Outdated
* @type bool
* @flags CREATE_AND_SET
* @default false
*/
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add a comment that this attribute is valid only for
SAI_PORT_ATTR_TYPE == SAI_PORT_TYPE_FABRIC

@kcudnik
I believe since SAI_PORT_ATTR_TYPE is a read only attribute, tooling will now allow validonly check.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kcudnik I agree, thats why request for adding a text to clarify the intent

@rlhui
Copy link
Copy Markdown
Collaborator

rlhui commented Aug 25, 2022

@jfeng-arista - would you please resolve conflicts?

@jfeng-arista jfeng-arista force-pushed the add_fabric_port_isolation_api branch from 8db00a4 to 1c3ecd5 Compare August 25, 2022 18:50
@jfeng-arista
Copy link
Copy Markdown
Contributor Author

/azpw run

@jfeng-arista
Copy link
Copy Markdown
Contributor Author

Hi,
Can any one help me re-trigger the build for my branch and I could not trigger it by my last comment.
thank you
Jie

@jfeng-arista jfeng-arista force-pushed the add_fabric_port_isolation_api branch from 94abf9f to a5e365a Compare August 30, 2022 18:31
Signed-off-by: Jie Feng <[email protected]>

remove extra line end space.

Signed-off-by: Jie Feng <[email protected]>

Move the new attribute to the end of the list

Signed-off-by: Jie Feng <[email protected]>
Signed-off-by: Jie Feng <[email protected]>
@jfeng-arista jfeng-arista force-pushed the add_fabric_port_isolation_api branch from a5e365a to 6e067bc Compare August 30, 2022 19:11
@rlhui rlhui merged commit b582263 into opencomputeproject:master Sep 19, 2022
@jfeng-arista
Copy link
Copy Markdown
Contributor Author

jfeng-arista commented Sep 19, 2022 via email

@rlhui rlhui mentioned this pull request Mar 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants