Enable Pfcwd Queries #332
Enable Pfcwd Queries #332
Conversation
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| @@ -149,7 +149,10 @@ func getPfcwdMap() (map[string]map[string]string, error) { | |||
| } | |||
|
|
|||
| for _, key := range resp { | |||
| name := key[13:] | |||
| if strings.Contains(key, "GLOBAL") || strings.Contains(key, "global") { // ignore PFC_WD|global / PFC_WD|GLOBAL | |||
There was a problem hiding this comment.
Why do we ignore PFC_WD|global?
There was a problem hiding this comment.
No counters associated with global since it is not a port. We are trying to create a mapping of oid from port name and pfc queue index which we will use to retrieve pfc wd counters. We ignore global since it serves no purpose for building the mapping since no port is associated with global
| @@ -119,7 +119,7 @@ func initCountersPfcwdNameMap() error { | |||
| } | |||
|
|
|||
| // Get the mapping between sonic interface name and oids of their PFC-WD enabled queues in COUNTERS_DB | |||
| func getPfcwdMap() (map[string]map[string]string, error) { | |||
| func GetPfcwdMap() (map[string]map[string]string, error) { | |||
There was a problem hiding this comment.
All other function names use camelcase format, should we go back to the previous name?
There was a problem hiding this comment.
Making this function public for testing. Capitalizing the first letter in the function name makes it public.
sonic_data_client/virtual_db.go
Outdated
| if strings.Contains(key, "GLOBAL") || strings.Contains(key, "global") { // ignore PFC_WD|global / PFC_WD|GLOBAL | ||
| continue | ||
| } | ||
| name := key[7:] |
There was a problem hiding this comment.
instead of "7", should we get the length of "PFC_WD|" using a function for future proof?
There was a problem hiding this comment.
Yes, I can make that change.
| nn.TS = time.Unix(0, 200) | ||
| gotNoti = append(gotNoti, nn) | ||
| } | ||
| mutexGotNoti.Unlock() |
There was a problem hiding this comment.
We have a defer statement that will stop the server connection, if there is an error, before the function exits, the defer statement will be called.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
Cherry-pick PR to 202411: #347 |
Why I did it Pfcwd queries currently use PFC_WD_TABLE which is not the correct table name. Use PFC_WD instead. Since Pfcwd are not crucial to other COUNTERS paths, if map creation of Pfcwd fails, we will fail gracefully and return empty json for Pfcwd instead of impacting all COUNTER_DB queries.
Why I did it Pfcwd queries currently use PFC_WD_TABLE which is not the correct table name. Use PFC_WD instead. Since Pfcwd are not crucial to other COUNTERS paths, if map creation of Pfcwd fails, we will fail gracefully and return empty json for Pfcwd instead of impacting all COUNTER_DB queries.
Why I did it
Pfcwd queries currently use PFC_WD_TABLE which is not the correct table name. Use PFC_WD instead.
Since Pfcwd are not crucial to other COUNTERS paths, if map creation of Pfcwd fails, we will fail gracefully and return empty json for Pfcwd instead of impacting all COUNTER_DB queries.
How I did it
Change table name
How to verify it
UT and manual testing
Which release branch to backport (provide reason below if selected)
Description for the changelog
Link to config_db schema for YANG module changes
A picture of a cute animal (not mandatory but encouraged)