sonic-net / sonic-swss

SONiC Switch State Service (SwSS)
https://azure.github.io/SONiC
Other
170 stars 503 forks source link

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

Closed amazor closed 1 month ago

amazor commented 1 month ago

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: https://github.com/sonic-net/sonic-mgmt/issues/13375

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

How I verified it TBD Details if related

prsunny commented 1 month ago

@kperumalbfn , can you review/signoff. This was introduced by https://github.com/sonic-net/sonic-swss/pull/3138

dgsudharsan commented 1 month ago

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

mssonicbld commented 1 month ago

Cherry-pick PR to 202311: https://github.com/sonic-net/sonic-swss/pull/3224

mssonicbld commented 1 month ago

Cherry-pick PR to 202405: https://github.com/sonic-net/sonic-swss/pull/3225

bingwang-ms commented 1 month ago

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 commented 1 month ago

@amazor cherry pick PR didn't pass PR checker. Please check!!!
https://github.com/sonic-net/sonic-swss/pull/3225

amazor commented 1 month ago

@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 commented 1 month ago

@amazor cherry pick PR didn't pass PR checker. Please check!!!
https://github.com/sonic-net/sonic-swss/pull/3225

mssonicbld commented 1 month ago

@amazor cherry pick PR didn't pass PR checker. Please check!!!
https://github.com/sonic-net/sonic-swss/pull/3225

mssonicbld commented 1 month ago

@amazor cherry pick PR didn't pass PR checker. Please check!!!
https://github.com/sonic-net/sonic-swss/pull/3225