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

Fix VDM freeze and unfreeze needed for PM stats collection #386 #387

Closed
wants to merge 0 commits into from

Conversation

jaganbal-a
Copy link
Contributor

In order to collect PM stats without loss of data following vdm freeze and unfreeze request needs to be followed.

  1. VDM freeze
  2. PM stats collection
  3. VDM unfreeze

Description

Currently incorrect order of freeze and unfreeze request is followed to collect PM statistics, with this the data collected prior PM interval will be lost once the unfreeze request is raised before data collection from PM stats register.

Motivation and Context

fixes #386

How Has This Been Tested?

This is tested by dumping the PM table from state-DB for the interface which holds the collected PM stats for each PM interval.

Additional Information (Optional)

freeze request timestamp is added to PM table which will be used for PM time window based stats collection.
Refer PR sonic-net/SONiC#1258

@longhuan-cisco
Copy link
Contributor

longhuan-cisco commented Jul 19, 2023

please address the test failure in the pre-commit check.

@jarias-lfx
Copy link

/easycla

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Jul 19, 2023

CLA Not Signed

@jaganbal-a
Copy link
Contributor Author

rerun p1

1 similar comment
@jaganbal-a
Copy link
Contributor Author

rerun p1

@jaganbal-a jaganbal-a requested a review from prgeor September 7, 2023 17:31
@prgeor
Copy link
Collaborator

prgeor commented Sep 12, 2023

CLA Not Signed

@jaganbal-a please sign the EasyCLA

sonic_platform_base/sonic_xcvr/api/public/c_cmis.py Outdated Show resolved Hide resolved
if (retry <= 0) and (status != expected_status):
return 1

def freeze_vdm_stats(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jaganbal-a After writing to Freeze, we should wait at max tVDMF Timeout = 10msec and if still VDM_FREEZE_DONE is not SET return failure

Copy link
Contributor Author

Choose a reason for hiding this comment

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

post setting freeze, it waits for tVDMF time '10ms' on each retry, added 3 retry before returning error for the worst case as the delay is 10ms.

#Retry for freeze done status
return self.util_status_check_with_retry(VDM_FREEZE_UNFREEZE_DONE_RETRY, SLEEP_10MS, consts.VDM_FREEZE_DONE, 1)

def unfreeze_vdm_stats(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jaganbal-a After writing to un-Freeze, we should wait at max tVDMF Timeout = 10msec and if still VDM_UNFREEZE_DONE is not SET return failure

@@ -51,6 +51,14 @@ def get_transceiver_pm(self):
api = self.get_xcvr_api()
return api.get_transceiver_pm() if api is not None else None

def freeze_vdm_stats(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jaganbal-a can you add comment for return value status for True(Success) and False(Failure)

api = self.get_xcvr_api()
return api.freeze_vdm_stats() if api is not None else None

def unfreeze_vdm_stats(self):
Copy link
Collaborator

Choose a reason for hiding this comment

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

@jaganbal-a can you add comment for return value status for True(Success) and False(Failure)

@prgeor prgeor changed the title Incorrect vdm freeze and unfreeze is followed on collecting PM stats #386 Fix VDM freeze and unfreeze needed for PM stats collection #386 Sep 12, 2023
@jaganbal-a jaganbal-a closed this Sep 29, 2023
@jaganbal-a
Copy link
Contributor Author

Finding issues in signing the EasyCLA for the commits made in this PR, so created new pull request with PR comment addressed : #402

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.

Incorrect vdm freeze and unfreeze is followed on collecting PM stats
4 participants