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

Pfcwd path fix#44

Open
TildenWinston wants to merge 10 commits intosonic-net:masterfrom
TildenWinston:pfcwd-path-fix
Open

Pfcwd path fix#44
TildenWinston wants to merge 10 commits intosonic-net:masterfrom
TildenWinston:pfcwd-path-fix

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!

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