Skip to content
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

switchorch: fixed unsupported attribute causes skipping of processing the rest of the configurations #3209

Merged
merged 1 commit into from
Jul 2, 2024

Conversation

amazor
Copy link
Contributor

@amazor amazor commented Jun 30, 2024

What I did
Changed it so that if the configuration has an unsupported attribute, it would continue processing the rest of the configuration rather than break out of the loop and end the processing immediately.
Also added syslogs to make it more clear.

Why I did it
Would cause FDB aging time to stay default (0) and never cause aging to occur. Fixes this bug in sonic-mgmt: sonic-net/sonic-mgmt#13375

Caused by this commit https://github.com/sonic-net/sonic-swss/commit/2f8bd9cf6df839abec0ecc9bf31fc9c4b06a6c47

How I verified it
TBD
Details if related

@amazor amazor marked this pull request as ready for review June 30, 2024 14:50
@amazor amazor requested a review from prsunny as a code owner June 30, 2024 14:50
@prsunny prsunny requested a review from kperumalbfn July 2, 2024 17:47
@prsunny
Copy link
Collaborator

prsunny commented Jul 2, 2024

@kperumalbfn , can you review/signoff. This was introduced by #3138

@prsunny prsunny merged commit c9c78dc into sonic-net:master Jul 2, 2024
16 of 17 checks passed
@dgsudharsan
Copy link
Collaborator

@yxieca Can you please prioritize cherry-pick of this to 202311? This also affected ordered_ecmp attribute being skipped.

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202311: #3224

@mssonicbld
Copy link
Collaborator

Cherry-pick PR to 202405: #3225

mssonicbld pushed a commit to mssonicbld/sonic-swss that referenced this pull request Jul 5, 2024
…g the rest of configurations (sonic-net#3209)

What I did
Changed it so that if the configuration has an unsupported attribute, it would continue processing the rest of the configuration rather than break out of the loop and end the processing immediately.
Also added syslogs to make it more clear.
mssonicbld pushed a commit to mssonicbld/sonic-swss that referenced this pull request Jul 5, 2024
…g the rest of configurations (sonic-net#3209)

What I did
Changed it so that if the configuration has an unsupported attribute, it would continue processing the rest of the configuration rather than break out of the loop and end the processing immediately.
Also added syslogs to make it more clear.
@bingwang-ms
Copy link
Contributor

Hi @amazor, from the PR description, FDB aging will fail to work if missing this PR. But I didn't see any similar issue. Can you please help me understand?

mssonicbld pushed a commit that referenced this pull request Jul 5, 2024
…g the rest of configurations (#3209)

What I did
Changed it so that if the configuration has an unsupported attribute, it would continue processing the rest of the configuration rather than break out of the loop and end the processing immediately.
Also added syslogs to make it more clear.
yejianquan pushed a commit to yejianquan/sonic-swss that referenced this pull request Jul 8, 2024
…g the rest of configurations (sonic-net#3209)

What I did
Changed it so that if the configuration has an unsupported attribute, it would continue processing the rest of the configuration rather than break out of the loop and end the processing immediately.
Also added syslogs to make it more clear.
@mssonicbld
Copy link
Collaborator

@amazor cherry pick PR didn't pass PR checker. Please check!!!
#3225

@amazor
Copy link
Contributor Author

amazor commented Jul 9, 2024

@bingwang-ms, This PR fixes an issue when configuring SWITCH_TABLE:switch table in appl_db. If inside the table you attempt to configure ecmp_hash_offset/lag_hash_offset, then the switchorch application will stop processing the rest of the configuration in the table because of the break;. This fix replaces the break; with a continue; if it attempting to process an "unsupported_attribute" so that the fdb aging configuration (and any other valid SWITCH_TABLE:switch configurations) can be processed.

The FDB aging issue only occurs when attempting to use at least one of these two attributes when the ASIC does not support these attributes. These attributes were added in this commit: https://github.com/sonic-net/sonic-buildimage/commit/518c3bc7a9cf5293fcc2a0c76298f83073b1a974

@mssonicbld
Copy link
Collaborator

@amazor cherry pick PR didn't pass PR checker. Please check!!!
#3225

2 similar comments
@mssonicbld
Copy link
Collaborator

@amazor cherry pick PR didn't pass PR checker. Please check!!!
#3225

@mssonicbld
Copy link
Collaborator

@amazor cherry pick PR didn't pass PR checker. Please check!!!
#3225

mssonicbld pushed a commit that referenced this pull request Jul 12, 2024
…g the rest of configurations (#3209)

What I did
Changed it so that if the configuration has an unsupported attribute, it would continue processing the rest of the configuration rather than break out of the loop and end the processing immediately.
Also added syslogs to make it more clear.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants