Fix GCU bug when backend service modifying config #2295

wen587 · 2022-08-04T09:16:59Z

Modifying PORT admin_status will result in backend service changing the running config. e.g. buffermgrd will modify BUFFER_PG|EthernetXX|3-4 when one of the EthernetXX in PORT table admin down.
This PR is to skip comparison of config that is possiblly being modified by backend service

What I did

Fixes sonic-net/sonic-buildimage#11576

How I did it

Add a workaround to only compare config without backend service impact.

How to verify it

Manual test on specific platform and check operation success.

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

linux-foundation-easycla · 2022-08-04T09:17:02Z

The committers listed above are authorized under a signed CLA.

✅ login: wen587 / name: jingwenxie (4dce058, a2c3abf, 96ba055)

wen587 · 2022-08-05T03:26:35Z

Hi @liuh-80 , could you help review if only these two tables [BUFFER_PG, BUFFER_PROFILE] will be impacted by buffermgrd?

qiluo-msft · 2022-08-05T05:36:32Z

@DavidZagury Could you help review this PR?

qiluo-msft · 2022-08-05T05:37:33Z

generic_config_updater/change_applier.py

+        self.backend_tables = [
+            "BUFFER_PG",
+            "BUFFER_PROFILE",
+            "FLEX_COUNTER_TABLE"


FLEX_COUNTER_TABLE

Which process will modify FLEX_COUNTER_TABLE? #Closed

I found it is being modified after load_minigraph, but not sure if other config changes would modify this table.

Maybe flex_counter_manager https://github.com/sonic-net/sonic-swss/blob/master/orchagent/flex_counter/flex_counter_manager.cpp#L112?

admin@vlab-01:~$ redis-cli monitor | grep " \[4 " | grep 'HSET\|HMSET' ... 1652252491.657340 [4 127.0.0.1:48348] "HMSET" "FLEX_COUNTER_TABLE|PORT" "FLEX_COUNTER_STATUS" "enable" 1652252491.665481 [4 127.0.0.1:48348] "HMSET" "FLEX_COUNTER_TABLE|RIF" "FLEX_COUNTER_STATUS" "enable" 1652252491.673144 [4 127.0.0.1:48348] "HMSET" "FLEX_COUNTER_TABLE|QUEUE" "FLEX_COUNTER_STATUS" "enable" 1652252491.677734 [4 127.0.0.1:48348] "HMSET" "FLEX_COUNTER_TABLE|PFCWD" "FLEX_COUNTER_STATUS" "enable" 1652252491.689264 [4 127.0.0.1:48348] "HMSET" "FLEX_COUNTER_TABLE|PG_WATERMARK" "FLEX_COUNTER_STATUS" "enable" 1652252491.692459 [4 127.0.0.1:48348] "HMSET" "FLEX_COUNTER_TABLE|PG_DROP" "FLEX_COUNTER_STATUS" "enable" 1652252491.695281 [4 127.0.0.1:48348] "HMSET" "FLEX_COUNTER_TABLE|QUEUE_WATERMARK" "FLEX_COUNTER_STATUS" "enable" 1652252491.699036 [4 127.0.0.1:48348] "HMSET" "FLEX_COUNTER_TABLE|BUFFER_POOL_WATERMARK" "FLEX_COUNTER_STATUS" "enable" 1652252491.703090 [4 127.0.0.1:48348] "HMSET" "FLEX_COUNTER_TABLE|PORT_BUFFER_DROP" "FLEX_COUNTER_STATUS" "enable" 1652252491.706964 [4 127.0.0.1:48348] "HMSET" "FLEX_COUNTER_TABLE|ACL" "FLEX_COUNTER_DELAY_STATUS" "false" "FLEX_COUNTER_STATUS" "enable" "POLL_INTERVAL" "10000"

qiluo-msft

LGTM.

generic_config_updater/change_applier.py

DavidZagury · 2022-08-07T07:32:23Z

@qiluo-msft I tested this PR on my setup and it fixed the issue. I did add one comment that I think could have a slight improvement in the performance of this solution .

@stephenxs do you know of any other tables that should be ignored during the GCU validation check?

DavidZagury

LGTM

stephenxs · 2022-08-08T06:58:58Z

@qiluo-msft I tested this PR on my setup and it fixed the issue. I did add one comment that I think could have a slight improvement in the performance of this solution .

@stephenxs do you know of any other tables that should be ignored during the GCU validation check?

Hi
I'm not aware of other tables that can be updated by manager daemons. Now that we have an infra to ignore such tables, we can add them to the list in case we find some new.

wen587 · 2022-08-09T01:37:29Z

/easycla

Update sonic-utilities submodule pointer to include the following: * Fix GCU bug when backend service modifying config ([sonic-net#2295](sonic-net/sonic-utilities#2295)) * Fix issues for sonic_installer upgrade-docker and sonic_installer rollback-docker ([sonic-net#2278](sonic-net/sonic-utilities#2278)) * [crm] add checking for CRM interval range ([sonic-net#2293](sonic-net/sonic-utilities#2293)) * Fix the issue that sonic_platform is not installed on vs image ([sonic-net#2300](sonic-net/sonic-utilities#2300)) * Add FEC correctable and uncorrectable port stats ([sonic-net#2027](sonic-net/sonic-utilities#2027)) * Add CLI to configure YANG config validation ([sonic-net#2147](sonic-net/sonic-utilities#2147)) * Add override testcase to verify removal ([sonic-net#2288](sonic-net/sonic-utilities#2288)) * Fix version in db_migrator for ([sonic-net#2289](sonic-net/sonic-utilities#2289)) * [intfutil] Check whether the FEC mode is supported on the platform before configuring it to CONFIG_DB ([sonic-net#2223](sonic-net/sonic-utilities#2223)) * Transfer organization from Azure to sonic-net ([sonic-net#2284](sonic-net/sonic-utilities#2284)) * [watermarkstat] Fix CLI script for unconfigured PG counters ([sonic-net#2239](sonic-net/sonic-utilities#2239)) * Improve the way to check port type of RJ45 port ([sonic-net#2249](sonic-net/sonic-utilities#2249)) Signed-off-by: dprital <drorp@nvidia.com>

What I did Fixes sonic-net/sonic-buildimage#11576 How I did it Add a workaround to only compare config without backend service impact. How to verify it Manual test on specific platform and check operation success.

Update sonic-utilities submodule pointer to include the following: * Fix test failure in dump table test in 202205 ([sonic-net#2307](sonic-net/sonic-utilities#2307)) * Convert IPv6 addresses to lowercase in apply-patch ([sonic-net#2299](sonic-net/sonic-utilities#2299)) * [config][muxcable] add support to enable/disable ycable telemetry ([sonic-net#2297](sonic-net/sonic-utilities#2297)) * Fix GCU bug when backend service modifying config ([sonic-net#2295](sonic-net/sonic-utilities#2295)) * [intfutil] Check whether the FEC mode is supported on the platform before configuring it to CONFIG_DB ([sonic-net#2223](sonic-net/sonic-utilities#2223)) * Improve the way to check port type of RJ45 port ([sonic-net#2249](sonic-net/sonic-utilities#2249)) * sonic-utils: initial support for link-training ([sonic-net#2071](sonic-net/sonic-utilities#2071)) * Support to enable fips for the command sonic_installer (sonic-net#2154) ([sonic-net#2303](sonic-net/sonic-utilities#2303)) Signed-off-by: dprital <drorp@nvidia.com>

What I did Fixes sonic-net/sonic-buildimage#11576 How I did it Add a workaround to only compare config without backend service impact. How to verify it Manual test on specific platform and check operation success.

Fix GCU bug when backend service modifying config

4dce058

fix unittest

a2c3abf

wen587 requested review from liuh-80 and qiluo-msft August 5, 2022 03:25

wen587 marked this pull request as ready for review August 5, 2022 03:25

qiluo-msft requested review from neethajohn and stephenxs August 5, 2022 05:35

qiluo-msft reviewed Aug 5, 2022

View reviewed changes

qiluo-msft previously approved these changes Aug 5, 2022

View reviewed changes

DavidZagury reviewed Aug 7, 2022

View reviewed changes

generic_config_updater/change_applier.py Outdated Show resolved Hide resolved

resolve comment

96ba055

wen587 dismissed qiluo-msft’s stale review via 96ba055 August 8, 2022 01:49

wen587 added the Request for 202205 Branch label Aug 8, 2022

wen587 requested a review from qiluo-msft August 8, 2022 02:28

DavidZagury approved these changes Aug 8, 2022

View reviewed changes

stephenxs approved these changes Aug 8, 2022

View reviewed changes

neethajohn approved these changes Aug 8, 2022

View reviewed changes

qiluo-msft approved these changes Aug 8, 2022

View reviewed changes

wen587 merged commit be1866f into sonic-net:master Aug 9, 2022

dprital mentioned this pull request Aug 9, 2022

[submodule] Update sonic-utilities submodule sonic-net/sonic-buildimage#11657

Closed

7 tasks

stephenxs mentioned this pull request Aug 9, 2022

[submodule] Advance sonic-swss and sonic-utilities pointer sonic-net/sonic-buildimage#11643

Merged

7 tasks

yxieca added the Included in 202205 Branch label Aug 11, 2022

dprital mentioned this pull request Aug 12, 2022

[202205][submodule] Advance sonic-utilities pointer sonic-net/sonic-buildimage#11721

Closed

7 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix GCU bug when backend service modifying config #2295

Fix GCU bug when backend service modifying config #2295

wen587 commented Aug 4, 2022 •

edited

Loading

linux-foundation-easycla bot commented Aug 4, 2022 •

edited

Loading

wen587 commented Aug 5, 2022

qiluo-msft commented Aug 5, 2022

qiluo-msft Aug 5, 2022 •

edited

Loading

wen587 Aug 5, 2022 •

edited

Loading

qiluo-msft left a comment

DavidZagury commented Aug 7, 2022

DavidZagury left a comment

stephenxs commented Aug 8, 2022

wen587 commented Aug 9, 2022

Fix GCU bug when backend service modifying config #2295

Fix GCU bug when backend service modifying config #2295

Conversation

wen587 commented Aug 4, 2022 • edited Loading

What I did

How I did it

How to verify it

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

linux-foundation-easycla bot commented Aug 4, 2022 • edited Loading

wen587 commented Aug 5, 2022

qiluo-msft commented Aug 5, 2022

qiluo-msft Aug 5, 2022 • edited Loading

Choose a reason for hiding this comment

wen587 Aug 5, 2022 • edited Loading

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

DavidZagury commented Aug 7, 2022

DavidZagury left a comment

Choose a reason for hiding this comment

stephenxs commented Aug 8, 2022

wen587 commented Aug 9, 2022

wen587 commented Aug 4, 2022 •

edited

Loading

linux-foundation-easycla bot commented Aug 4, 2022 •

edited

Loading

qiluo-msft Aug 5, 2022 •

edited

Loading

wen587 Aug 5, 2022 •

edited

Loading