Skip to content
This repository has been archived by the owner on Aug 31, 2022. It is now read-only.

Pfcwd path fix #44

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

TildenWinston
Copy link

Changes made to virtual_db.go to fix the Pfcwd redis data path. I fixed the table name, updated the count used to get port number accordingly, and added if statment to filter out smaller keys like PFC_WD|Global.

Based on the current code in the Sonic-telemetry repo, the original expected path for the PFC_WD keys was PFC_WD_TABLE: https://github.com/Azure/sonic-telemetry/blob/master/sonic_data_client/virtual_db.go#L133. This change was made back when warm boot support was added to SONiC-swss-common, the table name for Pfcwd was changed from PFC_WD_TABLE to PFC_WD. So this changed caused the COUNTERS/Ethernet*/Pfcwd command to look for the ports listed with the PFC_WD_TABLE key which always returned nothing given that it no longer exists. Changing the length then caused line 147 (https://github.com/Azure/sonic-telemetry/blob/master/sonic_data_client/virtual_db.go#L147) to fail to get the proper port number from the keys because the removing the first 14 characters parsed to et## instead of the intended ##.

Copy link
Collaborator

@hui-ma hui-ma left a comment

Choose a reason for hiding this comment

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

Please do a cleaning round. Thansk!

.vscode/launch.json Outdated Show resolved Hide resolved
.vscode/tasks.json Outdated Show resolved Hide resolved
@@ -335,6 +346,7 @@ restart: //Remote server might go down, in that case we restart with next destin
return
}
cs.cMu.Unlock()
log.V(7).Infof("Before switch 346 %v", cs)
Copy link
Collaborator

Choose a reason for hiding this comment

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

debuging line, remove

if clientCfg.TLS != nil {
opts = append(opts, grpc.WithTransportCredentials(credentials.NewTLS(clientCfg.TLS)))
} else {
opts = append(opts, grpc.WithInsecure())
Copy link
Collaborator

Choose a reason for hiding this comment

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

This should not belong to the PR

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants