sairedis: Fixing race condition for rif counters #1136

sumanbrcm · 2022-10-13T07:40:21Z

Fixing issue #11621
The cleanup code for stale rif counters are now moved to syncd . Earlier as part of fix for issue #2193 the cleanup for stale rif counters was added.
But it could create a race condition between orchagent removes RIF rate counters from DB and lua script fetching them.
So as a fix all such cleanup has been moved to syncd.

Signed-off-by: Suman Kumar suman.kumar@broadcom.com

All the details are mentioned in :
sonic-net/sonic-swss#2488

linux-foundation-easycla · 2022-10-13T07:40:24Z

The committers listed above are authorized under a signed CLA.

✅ login: sumanbrcm / name: Suman Kumar (d664f98)

sumanbrcm · 2022-10-13T07:45:30Z

Added @prsunny for review

sumanbrcm · 2022-10-13T07:48:18Z

This PR along with (sonic-net/sonic-swss#2488) handles the below issue :
sonic-net/sonic-buildimage#11621

Junchao-Mellanox · 2022-10-14T01:27:26Z

syncd/FlexCounter.cpp

+    return "RATES:" + key + ":RIF";
+}
+
+void FlexCounter::cleanUpRifFromCounterDb(_In_ sai_object_id_t rifId)


Could you please try reuse removeDataFromCountersDB?

@Junchao-Mellanox , Sure , I will use as suggested and check/UT . If it works fine, will submit a patch.

@Junchao-Mellanox , The changes you suggested worked fine , submitting a new patch for this.

kcudnik · 2022-10-14T07:31:33Z

is this backward compatible?

sumanbrcm · 2022-10-17T15:10:21Z

@kcudnik , it is backward compatible. The stale rif counter db table/entries was cleaned up in orchagent in previous version while now it will be cleaned up in syncd.

kcudnik · 2022-10-17T15:42:29Z

can you solve build issues?

sumanbrcm · 2022-10-17T17:06:34Z

sure , will resolve it (Please note the new patch will be submitted if it works fine with @Junchao-Mellanox suggestion)

* Fixing issue #11621 * The cleanup code for stale rif counters are now moved to syncd . Earlier as part of fix for issue #2193 the cleanup for stale rif counters was added. * But it could create a race condition between orchagent removes RIF rate counters from DB and lua script fetching them. * So as a fix all such cleanup has been moved to syncd. Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>

sumanbrcm · 2022-10-28T03:34:12Z

/azpw run Azure.sonic-sairedis

mssonicbld · 2022-10-28T03:34:14Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-10-28T03:34:24Z

Azure Pipelines successfully started running 1 pipeline(s).

sumanbrcm · 2022-10-28T08:45:48Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-10-28T08:45:53Z

Commenter does not have sufficient privileges for PR 1136 in repo sonic-net/sonic-sairedis

sumanbrcm · 2022-10-28T08:46:53Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-10-28T08:46:58Z

Commenter does not have sufficient privileges for PR 1136 in repo sonic-net/sonic-sairedis

sumanbrcm · 2022-10-28T09:06:52Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-10-28T09:06:57Z

Commenter does not have sufficient privileges for PR 1136 in repo sonic-net/sonic-sairedis

sumanbrcm · 2022-10-28T09:44:07Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-10-28T09:44:13Z

Commenter does not have sufficient privileges for PR 1136 in repo sonic-net/sonic-sairedis

sumanbrcm · 2022-10-28T09:56:28Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-10-28T09:56:33Z

Commenter does not have sufficient privileges for PR 1136 in repo sonic-net/sonic-sairedis

sumanbrcm · 2022-10-28T09:57:46Z

/AzurePipelines run

azure-pipelines · 2022-10-28T09:57:58Z

Commenter does not have sufficient privileges for PR 1136 in repo sonic-net/sonic-sairedis

sumanbrcm · 2022-10-28T09:58:15Z

/AzurePipelines run Azure.sonic-sairedis

sumanbrcm · 2022-11-02T07:11:51Z

/AzurePipelines run Azure.sonic-sairedis

azure-pipelines · 2022-11-02T07:11:57Z

Commenter does not have sufficient privileges for PR 1136 in repo sonic-net/sonic-sairedis

prsunny · 2022-11-04T02:16:59Z

Please try with "/azpw run"

sumanbrcm · 2022-11-06T09:22:03Z

/azpw run

mssonicbld · 2022-11-06T09:22:05Z

/AzurePipelines run

azure-pipelines · 2022-11-06T09:22:17Z

Azure Pipelines successfully started running 1 pipeline(s).

sumanbrcm · 2022-11-07T08:42:43Z

/azpw run

mssonicbld · 2022-11-07T08:42:45Z

/AzurePipelines run

azure-pipelines · 2022-11-07T08:42:55Z

Azure Pipelines successfully started running 1 pipeline(s).

sumanbrcm · 2022-11-08T11:27:00Z

@prsunny @liat-grozovik @kcudnik
This is request for merge/related-approval.

* Fixing issue #11621 * The cleanup code for stale rif counters are now moved to syncd . Earlier as part of fix for issue #2193 the cleanup for stale rif counters was added. * But it could create a race condition between orchagent removes RIF rate counters from DB and lua script fetching them. * So as a fix all such cleanup has been moved to syncd. Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>

stepanblyschak · 2022-11-22T10:20:16Z

@kcudnik @yxieca Could you please help to assign label "Request for 202205"? This fix is neccessary for 202205

yxieca · 2022-11-29T00:17:53Z

@sumanbrcm this change cannot be cherry-picked cleanly, can you help enter an new PR for 202205 branch?

* Fixing issue #11621 * The cleanup code for stale rif counters are now moved to syncd . Earlier as part of fix for issue #2193 the cleanup for stale rif counters was added. * But it could create a race condition between orchagent removes RIF rate counters from DB and lua script fetching them. * So as a fix all such cleanup has been moved to syncd. Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>

yxieca · 2023-02-02T21:35:44Z

@adyeung can you help ping the author? This change cannot be cherry-picked cleanly to 202205 branch. a separate PR is needed.

adyeung · 2023-02-02T21:47:05Z

Requested submitter @sumanbrcm to prioritize next wk

Changes for 202205 branch Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>

sumanbrcm · 2023-02-06T14:04:28Z

@yxieca , @adyeung
The PR for 202205 branch : -
#1202

Changes for 202205 branch Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>

Changes for 202205 branch Fixing issue #11621 All the details of the fix in master branch is in the PR : - sairedis: Fixing race condition for rif counters #1136 swss: Fixing race condition for rif counters sonic-swss#2488 This change is already merged to master branch , but sonic-sairedis change (sairedis: Fixing race condition for rif counters #1136) could not be cherry picked to 202205 branch as the base file FlexCounter.cpp differs in both the branches. Also, the API removeDataFromCountersDB is not available in 202205 branch for the cleanup. Hence the current fix takes care of cleaning up the stale rif counters issue through newly added API cleanUpRifFromCounterDb . Unit Tests:- The steps followed are same as in sonic-net/sonic-buildimage#11621 Create RIF in SONiC, wait till RIF rates are populated in COUNTERS DB Remove RIF Repeat the steps multiple times and check if any error syslog is seen (No error syslog is seen) Also checked cleanup for rif counters. After RIF creation derived info of oid for RIF from "COUNTERS_RIF_NAME_MAP" 127) "Vlan100" 128) "oid:0x6000000000aa5" Checked all the tabled in COUNTER_DB which has same OID in keys 127.0.0.1:6379[2]> keys 6000000000aa5 "RATES:oid:0x6000000000aa5:RIF" "COUNTERS:oid:0x6000000000aa5" "RATES:oid:0x6000000000aa5" 127.0.0.1:6379[2]> Deleted the RIF by removing the ip on the intf. Checked COUNTER_DB again with same OID if there are stale entries or not. No stale entries exist now. 127.0.0.1:6379[2]> keys 6000000000aa5 (empty array) 127.0.0.1:6379[2]> Signed-off-by: Suman Kumar suman.kumar@broadcom.com Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>

prsunny requested review from Junchao-Mellanox and dgsudharsan October 14, 2022 01:14

Junchao-Mellanox suggested changes Oct 14, 2022

View reviewed changes

kcudnik approved these changes Oct 27, 2022

View reviewed changes

Junchao-Mellanox approved these changes Oct 28, 2022

View reviewed changes

sumanbrcm mentioned this pull request Oct 28, 2022

swss: Fixing race condition for rif counters sonic-net/sonic-swss#2488

Merged

prsunny merged commit 2f2698c into sonic-net:master Nov 10, 2022

richardyu-ms mentioned this pull request Nov 15, 2022

[cherry-pick]Align 202211 with master for 202211 release (#1129)(#1116)(#1141) (#1136) (#1140) #1151

Merged

yxieca added the Request for 202205 Branch label Nov 27, 2022

yxieca added the Cherry Pick Conflict_202205 label Nov 29, 2022

sumanbrcm added a commit to sumanbrcm/sonic-sairedis that referenced this pull request Feb 6, 2023

Fixing race condition for rif counters sonic-net#1136

54bf9e7

Changes for 202205 branch Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>

sumanbrcm mentioned this pull request Feb 6, 2023

Fixing race condition for rif counters #1136 (202205 branch) #1202

Merged

sumanbrcm added a commit to sumanbrcm/sonic-sairedis that referenced this pull request Feb 7, 2023

Fixing race condition for rif counters sonic-net#1136

090a376

Changes for 202205 branch Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>

sumanbrcm added a commit to sumanbrcm/sonic-sairedis that referenced this pull request Feb 7, 2023

Fixing race condition for rif counters sonic-net#1136

979b623

Changes for 202205 branch Signed-off-by: Suman Kumar <suman.kumar@broadcom.com>

yxieca added the Included in 202205 Branch label Feb 9, 2023

sairedis: Fixing race condition for rif counters #1136

sairedis: Fixing race condition for rif counters #1136

Conversation

sumanbrcm commented Oct 13, 2022

linux-foundation-easycla bot commented Oct 13, 2022 • edited Loading

sumanbrcm commented Oct 13, 2022

sumanbrcm commented Oct 13, 2022

Junchao-Mellanox Oct 14, 2022

Choose a reason for hiding this comment

sumanbrcm Oct 17, 2022

Choose a reason for hiding this comment

sumanbrcm Oct 27, 2022

Choose a reason for hiding this comment

kcudnik commented Oct 14, 2022

sumanbrcm commented Oct 17, 2022

kcudnik commented Oct 17, 2022

sumanbrcm commented Oct 17, 2022

sumanbrcm commented Oct 28, 2022

mssonicbld commented Oct 28, 2022

azure-pipelines bot commented Oct 28, 2022

sumanbrcm commented Oct 28, 2022

azure-pipelines bot commented Oct 28, 2022

sumanbrcm commented Oct 28, 2022

azure-pipelines bot commented Oct 28, 2022

sumanbrcm commented Oct 28, 2022

azure-pipelines bot commented Oct 28, 2022

sumanbrcm commented Oct 28, 2022

azure-pipelines bot commented Oct 28, 2022

sumanbrcm commented Oct 28, 2022

azure-pipelines bot commented Oct 28, 2022

sumanbrcm commented Oct 28, 2022

azure-pipelines bot commented Oct 28, 2022

sumanbrcm commented Oct 28, 2022

sumanbrcm commented Nov 2, 2022

azure-pipelines bot commented Nov 2, 2022

prsunny commented Nov 4, 2022

sumanbrcm commented Nov 6, 2022

mssonicbld commented Nov 6, 2022

azure-pipelines bot commented Nov 6, 2022

sumanbrcm commented Nov 7, 2022

mssonicbld commented Nov 7, 2022

azure-pipelines bot commented Nov 7, 2022

sumanbrcm commented Nov 8, 2022

stepanblyschak commented Nov 22, 2022

yxieca commented Nov 29, 2022

yxieca commented Feb 2, 2023

adyeung commented Feb 2, 2023

sumanbrcm commented Feb 6, 2023

linux-foundation-easycla bot commented Oct 13, 2022 •

edited

Loading