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

Enable Pfcwd Queries #332

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
100 changes: 100 additions & 0 deletions gnmi_server/server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,106 @@ func createKeepAliveServer(t *testing.T, port int64) *Server {
return s
}

func TestPFCWDErrors(t *testing.T) {
s := createServer(t, 8081)
go runServer(t, s)
defer s.ForceStop()

mock := gomonkey.ApplyFunc(sdc.GetPfcwdMap, func() (map[string]map[string]string, error) {
return nil, fmt.Errorf("Mock error")
})
defer mock.Reset()

fileName := "../testdata/COUNTERS:Ethernet_wildcard_alias.txt"
countersEthernetWildcardByte, err := ioutil.ReadFile(fileName)
if err != nil {
t.Fatalf("read file %v err: %v", fileName, err)
}
var countersEthernetWildcardJson interface{}
json.Unmarshal(countersEthernetWildcardByte, &countersEthernetWildcardJson)

tests := []struct {
desc string
q client.Query
wantNoti []client.Notification
poll int
}{
{
desc: "query COUNTERS/Ethernet*",
poll: 1,
q: client.Query{
Target: "COUNTERS_DB",
Type: client.Poll,
Queries: []client.Path{{"COUNTERS", "Ethernet*"}},
TLS: &tls.Config{InsecureSkipVerify: true},
},
wantNoti: []client.Notification{
client.Update{Path: []string{"COUNTERS", "Ethernet*"}, TS: time.Unix(0, 200), Val: countersEthernetWildcardJson},
client.Update{Path: []string{"COUNTERS", "Ethernet*"}, TS: time.Unix(0, 200), Val: countersEthernetWildcardJson},
},
},
}

ns, _ := sdcfg.GetDbDefaultNamespace()
prepareDb(t, ns)
var mutexGotNoti sync.Mutex

for _, tt := range tests {
t.Run(tt.desc, func(t *testing.T) {
q := tt.q
q.Addrs = []string{"127.0.0.1:8081"}
c := client.New()
var gotNoti []client.Notification

q.NotificationHandler = func(n client.Notification) error {
mutexGotNoti.Lock()
if nn, ok := n.(client.Update); ok {
nn.TS = time.Unix(0, 200)
gotNoti = append(gotNoti, nn)
}
mutexGotNoti.Unlock()

Choose a reason for hiding this comment

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

connection close on error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

return nil
}

wg := new(sync.WaitGroup)
wg.Add(1)

go func() {
defer wg.Done()
if err := c.Subscribe(context.Background(), q); err != nil {
t.Errorf("c.Subscribe(): got error %v, expected nil", err)
}
}()

wg.Wait()

for i := 0; i < tt.poll; i++ {
err := c.Poll()
if err != nil {
t.Errorf("c.Poll(): got error %v, expected nil", err)
}
}

mutexGotNoti.Lock()

if len(gotNoti) == 0 {
t.Errorf("Expected non zero length of notifications")
}

if diff := pretty.Compare(tt.wantNoti, gotNoti); diff != "" {
t.Log("\n Want: \n", tt.wantNoti)
t.Log("\n Got : \n", gotNoti)
t.Errorf("unexpected updates:\n%s", diff)
}

mutexGotNoti.Unlock()

c.Close()
})
}
}


// runTestGet requests a path from the server by Get grpc call, and compares if
// the return code and response value are expected.
func runTestGet(t *testing.T, ctx context.Context, gClient pb.GNMIClient, pathTarget string,
Expand Down
2 changes: 1 addition & 1 deletion sonic_data_client/db_client.go
Original file line number Diff line number Diff line change
Expand Up @@ -607,7 +607,7 @@ func populateDbtablePath(prefix, path *gnmipb.Path, pathG2S *map[*gnmipb.Path][]
}
err = initCountersPfcwdNameMap()
if err != nil {
return err
log.Errorf("Could not create CountersPfcwdNameMap: %v", err)
}
}

Expand Down
24 changes: 18 additions & 6 deletions sonic_data_client/virtual_db.go
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ func initAliasMap() error {
func initCountersPfcwdNameMap() error {
var err error
if len(countersPfcwdNameMap) == 0 {
countersPfcwdNameMap, err = getPfcwdMap()
countersPfcwdNameMap, err = GetPfcwdMap()
if err != nil {
return err
}
Expand All @@ -119,10 +119,11 @@ 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) {

Choose a reason for hiding this comment

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

All other function names use camelcase format, should we go back to the previous name?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making this function public for testing. Capitalizing the first letter in the function name makes it public.

var pfcwdName_map = make(map[string]map[string]string)

dbName := "CONFIG_DB"
pfcwdTableName := "PFC_WD"
redis_client_map, err := GetRedisClientsForDb(dbName)
if err != nil {
return nil, err
Expand All @@ -134,8 +135,8 @@ func getPfcwdMap() (map[string]map[string]string, error) {
log.V(1).Infof("Can not connect to %v in namsespace %v, err: %v", dbName, namespace, err)
return nil, err
}

keyName := fmt.Sprintf("PFC_WD_TABLE%v*", separator)
keyName := fmt.Sprintf("%s%v*", pfcwdTableName, separator)
resp, err := redisDb.Keys(keyName).Result()
if err != nil {
log.V(1).Infof("redis get keys failed for %v in namsepace %v, key = %v, err: %v", dbName, namespace, keyName, err)
Expand All @@ -149,7 +150,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
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we ignore PFC_WD|global?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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

continue
}
name := key[len(keyName) - 1:]
pfcwdName_map[name] = make(map[string]string)
}

Expand All @@ -164,7 +168,15 @@ func getPfcwdMap() (map[string]map[string]string, error) {
log.V(1).Infof("PFC WD not enabled on device")
return nil, nil
}
qos_key := resp[0]

var qos_key string
for _, key := range resp {
if strings.Contains(key, "GLOBAL") || strings.Contains(key, "global") { // ignore PORT_QOS_MAP|global / PORT_QOS_MAP|GLOBAL
continue
}
qos_key = key
break
}

fieldName := "pfc_enable"
priorities, err := redisDb.HGet(qos_key, fieldName).Result()
Expand Down
Loading
Loading