From 98f07560121ad2c5d557b921083f27d2855f2637 Mon Sep 17 00:00:00 2001 From: Asha Behera Date: Fri, 20 Sep 2019 15:03:09 +0530 Subject: [PATCH 1/7] Fix for random UT failure --- src/cvl/internal/yparser/yparser.go | 36 +++++++++++++++-------------- 1 file changed, 19 insertions(+), 17 deletions(-) diff --git a/src/cvl/internal/yparser/yparser.go b/src/cvl/internal/yparser/yparser.go index 59d8e87c92..c0576bdc54 100644 --- a/src/cvl/internal/yparser/yparser.go +++ b/src/cvl/internal/yparser/yparser.go @@ -94,28 +94,30 @@ int lyd_data_validate_all(const char *data, const char *depData, const char *oth int lyd_multi_new_leaf(struct lyd_node *parent, const struct lys_module *module, const char *leafVal) { - char s[4048]; - char *name, *val; + char s[4048]; + char *name, *val; + char *saveptr1; - strcpy(s, leafVal); + strcpy(s, leafVal); - name = strtok(s, "#"); + name = strtok_r(s, "#", &saveptr1); - while (name != NULL) - { - val = strtok(NULL, "#"); - if (val != NULL) - { - if (NULL == lyd_new_leaf(parent, module, name, val)) - { - return -1; - } - } + while (name != NULL) + { + val = strtok_r(NULL, "#", &saveptr1); + if (val != NULL) + { + if (NULL == lyd_new_leaf(parent, module, name, val)) + { + return -1; + } + } - name = strtok(NULL, "#"); - } + name = strtok_r(NULL, "#", &saveptr1); + } + + return 0; - return 0; } struct lyd_node *lyd_find_node(struct lyd_node *root, const char *xpath) From 5b782b5df29539e207f3a5c71d905c766e6cb8b6 Mon Sep 17 00:00:00 2001 From: Partha Dutta Date: Tue, 24 Sep 2019 07:40:48 +0530 Subject: [PATCH 2/7] CVL API for fetching dependent and ordered Redis table list --- Makefile | 4 +- src/cvl/cvl.go | 82 +++++++++++++- src/cvl/cvl_api.go | 160 ++++++++++++++++++++++++++++ src/cvl/cvl_test.go | 52 +++++++++ src/cvl/internal/yparser/yparser.go | 4 +- 5 files changed, 295 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index ba9a4c1266..62dbd6d903 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,8 @@ GO_DEPS_LIST = github.com/gorilla/mux \ gopkg.in/go-playground/validator.v9 \ golang.org/x/crypto/ssh \ github.com/antchfx/jsonquery \ - github.com/antchfx/xmlquery + github.com/antchfx/xmlquery \ + github.com/philopon/go-toposort REST_BIN = $(BUILD_DIR)/rest_server/main @@ -76,6 +77,7 @@ cvl: go-deps $(MAKE) -C src/cvl $(MAKE) -C src/cvl/schema $(MAKE) -C src/cvl/testdata/schema + cvl-test: $(MAKE) -C src/cvl gotest diff --git a/src/cvl/cvl.go b/src/cvl/cvl.go index 1f0668fa0c..5d9c6c66d1 100644 --- a/src/cvl/cvl.go +++ b/src/cvl/cvl.go @@ -64,6 +64,11 @@ var dbNameToDbNum map[string]uint8 //map of lua script loaded var luaScripts map[string]*redis.Script +type tblFieldPair struct { + tableName string + field string +} + //var tmpDbCache map[string]interface{} //map of table storing map of key-value pair //m["PORT_TABLE] = {"key" : {"f1": "v1"}} //Important schema information to be loaded at bootup time @@ -79,6 +84,7 @@ type modelTableInfo struct { //multiple leafref possible for union mustExp map[string]string tablesForMustExp map[string]CVLOperation + refFromTables []tblFieldPair //list of table or table/field referring to this table } @@ -198,6 +204,9 @@ func init() { CVL_LOG(ERROR ,"Could not enable notification error %s", err) } + //Build reverse leafref info i.e. which table/field uses one table through leafref + buildRefTableInfo() + dbCacheSet(false, "PORT", 0) } @@ -376,6 +385,75 @@ func storeModelInfo(modelFile string, module *yparser.YParserModule) { //such mo } } +func yangToRedisTblName(yangListName string) string { + if (strings.HasSuffix(yangListName, "_LIST")) { + return yangListName[0:len(yangListName) - len("_LIST")] + } + return yangListName +} + +//This functions build info of depdent table/fields which uses a particulat table through leafref +func buildRefTableInfo() { + for tblName, tblInfo := range modelInfo.tableInfo { + if (len(tblInfo.leafRef) == 0) { + continue + } + + //For each leafref update the table used through leafref + for fieldName, leafRefs := range tblInfo.leafRef { + for _, leafRef := range leafRefs { + matches := reLeafRef.FindStringSubmatch(leafRef) + + //We have the leafref table name + if (matches != nil && len(matches) == 5) { //whole + 4 sub matches + refTable := yangToRedisTblName(matches[2]) + refTblInfo := modelInfo.tableInfo[refTable] + + refFromTables := &refTblInfo.refFromTables + *refFromTables = append(*refFromTables, tblFieldPair{tblName, fieldName}) + modelInfo.tableInfo[refTable] = refTblInfo + } + } + } + + } + + //Now sort list 'refFromTables' under each table based on dependency among them + for tblName, tblInfo := range modelInfo.tableInfo { + if (len(tblInfo.refFromTables) == 0) { + continue + } + + depTableList := []string{} + for i:=0; i < len(tblInfo.refFromTables); i++ { + depTableList = append(depTableList, tblInfo.refFromTables[i].tableName) + } + + sortedTableList, _ := cvg.cv.SortDepTables(depTableList) + if (len(sortedTableList) == 0) { + continue + } + + newRefFromTables := []tblFieldPair{} + + for i:=0; i < len(sortedTableList); i++ { + //Find fieldName + fieldName := "" + for j :=0; j < len(tblInfo.refFromTables); j++ { + if (sortedTableList[i] == tblInfo.refFromTables[j].tableName) { + fieldName = tblInfo.refFromTables[j].field + break + } + } + newRefFromTables = append(newRefFromTables, tblFieldPair{sortedTableList[i], fieldName}) + } + //Update sorted refFromTables + tblInfo.refFromTables = newRefFromTables + modelInfo.tableInfo[tblName] = tblInfo + } + +} + //Find the tables names in must expression, these tables data need to be fetched //during semantic validation func addTableNamesForMustExp() { @@ -932,10 +1010,6 @@ func (c *CVL) updateDeleteDataToCache(tableName string, redisKey string) { // as leafref //Use LUA script to find if table has any entry for this leafref -type tblFieldPair struct { - tableName string - field string -} func (c *CVL) findUsedAsLeafRef(tableName, field string) []tblFieldPair { diff --git a/src/cvl/cvl_api.go b/src/cvl/cvl_api.go index 83b6842519..ddba37ed62 100644 --- a/src/cvl/cvl_api.go +++ b/src/cvl/cvl_api.go @@ -23,9 +23,12 @@ import ( "fmt" "encoding/json" "github.com/go-redis/redis" + //toposort "github.com/stevenle/topsort" + toposort "github.com/philopon/go-toposort" "path/filepath" "cvl/internal/yparser" . "cvl/internal/util" + "strings" ) type CVLValidateType uint @@ -511,3 +514,160 @@ func (c *CVL) ValidateKeyData(key string, data string) CVLRetCode { func (c *CVL) ValidateFields(key string, field string, value string) CVLRetCode { return CVL_NOT_IMPLEMENTED } + +//Sort list of given tables as per their dependency +func (c *CVL) SortDepTables(tableList []string) ([]string, CVLRetCode) { + + //Add all the table names in graph nodes + graph := toposort.NewGraph(len(tableList)) + for ti := 0; ti < len(tableList); ti++ { + graph.AddNodes(tableList[ti]) + } + + //Add all the depedency edges for graph nodes + for ti :=0; ti < len(tableList); ti++ { + for tj :=0; tj < len(tableList); tj++ { + if (tableList[ti] == tableList[tj]) { + continue + } + + for _, leafRefs := range modelInfo.tableInfo[tableList[tj]].leafRef { + for _, leafRef := range leafRefs { + if (strings.Contains(leafRef, tableList[ti])) { + graph.AddEdge(yangToRedisTblName(tableList[tj]), + yangToRedisTblName(tableList[ti])) + } + } + } + } + } + + //Now perform topological sort + result, ret := graph.Toposort() + if ret == false { + return nil, CVL_ERROR + } + + return result, CVL_SUCCESS +} + +//Get the order list(parent then child) of tables in a given YANG module +//within a single model this is obtained using leafref relation +func (c *CVL) GetOrderedTables(yangModule string) ([]string, CVLRetCode) { + tableList := []string{} + + //Get all the table names under this model + for tblName, tblNameInfo := range modelInfo.tableInfo { + if (tblNameInfo.modelName == yangModule) { + tableList = append(tableList, tblName) + } + } + + return c.SortDepTables(tableList) +} + +func (c *CVL) addDepTables(tableMap map[string]bool, tableName string) { + + //Mark it is added in list + tableMap[tableName] = true + + //Now find all tables referred in leafref from this table + for _, leafRefs := range modelInfo.tableInfo[tableName].leafRef { + for _, leafRef := range leafRefs { + matches := reLeafRef.FindStringSubmatch(leafRef) + + //We have the leafref table name + if (matches != nil && len(matches) == 5) { //whole + 4 sub matches + c.addDepTables(tableMap, yangToRedisTblName(matches[2])) //call recursively + } + } + } +} + +//Get the list of dependent tables for a given table in a YANG module +func (c *CVL) GetDepTables(yangModule string, tableName string) ([]string, CVLRetCode) { + tableList := []string{} + tblMap := make(map[string]bool) + + c.addDepTables(tblMap, tableName) + + /* + //Get all the table names under this model + for tblName, _ := range modelInfo.tableInfo { + //if (tblNameInfo.modelName == yangModule) { + // tableList = append(tableList, tblName) + //} + tableList = append(tableList, tblName) + } + */ + + for tblName, _ := range tblMap { + tableList = append(tableList, tblName) + } + + //Add all the table names in graph nodes + graph := toposort.NewGraph(len(tableList)) + for ti := 0; ti < len(tableList); ti++ { + graph.AddNodes(tableList[ti]) + } + + //Add all the depedency edges for graph nodes + for ti :=0; ti < len(tableList); ti++ { + for tj :=0; tj < len(tableList); tj++ { + if (tableList[ti] == tableList[tj]) { + continue + } + + for _, leafRefs := range modelInfo.tableInfo[tableList[tj]].leafRef { + for _, leafRef := range leafRefs { + if (strings.Contains(leafRef, tableList[ti]+"/")) { + graph.AddEdge(yangToRedisTblName(tableList[tj]), + yangToRedisTblName(tableList[ti])) + } + } + } + } + } + + //Now perform topological sort + result, ret := graph.Toposort() + if ret == false { + return nil, CVL_ERROR + } + + return result, CVL_SUCCESS +} + +//Get the dependent data (redis keys) for an entry +func (c *CVL) GetDepDataForDelete(redisKey string) []string { + + tableName, key := splitRedisKey(redisKey) + + mCmd := map[string]*redis.StringSliceCmd{} + pipe := redisClient.Pipeline() + + for _, refTbl := range modelInfo.tableInfo[tableName].refFromTables { + mCmd[refTbl.tableName] = pipe.Keys(fmt.Sprintf("%s|*%s*", refTbl.tableName, key)) //write into pipeline + } + _, err := pipe.Exec() + if err != nil { + CVL_LOG(ERROR, "Failed to fetch dependent key details for table %s", tableName) + } + pipe.Close() + + depKeys := []string{} + for tblName, keys := range mCmd { + res, err := keys.Result() + if (err != nil) { + CVL_LOG(ERROR, "Failed to fetch dependent key details for table %s", tblName) + continue + } + + //Find dependent data for delete recursively + for i :=0; i< len(res); i++ { + depKeys = append(depKeys, c.GetDepDataForDelete(res[i])...) + } + } + + return depKeys +} diff --git a/src/cvl/cvl_test.go b/src/cvl/cvl_test.go index f08394ba34..1cd25ec25d 100644 --- a/src/cvl/cvl_test.go +++ b/src/cvl/cvl_test.go @@ -3442,3 +3442,55 @@ func TestValidateEditConfig_EmptyNode_Positive(t *testing.T) { } } + +func TestSortDepTables(t *testing.T) { + cvSess, _ := cvl.ValidationSessOpen() + + result, _ := cvSess.SortDepTables([]string{"PORT", "ACL_RULE", "ACL_TABLE"}) + + expectedResult := []string{"ACL_RULE", "ACL_TABLE", "PORT"} + + for i := 0; i < len(expectedResult) ; i++ { + if result[i] != expectedResult[i] { + t.Errorf("Validation failed, returned value = %v", result) + break + } + } + + cvl.ValidationSessClose(cvSess) +} + +func TestGetOrderedTables(t *testing.T) { + cvSess, _ := cvl.ValidationSessOpen() + + result, _ := cvSess.GetOrderedTables("sonic-vlan") + + expectedResult := []string{"VLAN_MEMBER", "VLAN"} + + for i := 0; i < len(expectedResult) ; i++ { + if result[i] != expectedResult[i] { + t.Errorf("Validation failed, returned value = %v", result) + break + } + } + + cvl.ValidationSessClose(cvSess) +} + +func TestGetDepTables(t *testing.T) { + cvSess, _ := cvl.ValidationSessOpen() + + result, _ := cvSess.GetDepTables("sonic-acl", "ACL_RULE") + + expectedResult := []string{"ACL_RULE", "ACL_TABLE", "MIRROR_SESSION"} + + for i := 0; i < len(expectedResult) ; i++ { + if result[i] != expectedResult[i] { + t.Errorf("Validation failed, returned value = %v", result) + break + } + } + + cvl.ValidationSessClose(cvSess) + +} diff --git a/src/cvl/internal/yparser/yparser.go b/src/cvl/internal/yparser/yparser.go index 12e0649a45..af16c7edd2 100644 --- a/src/cvl/internal/yparser/yparser.go +++ b/src/cvl/internal/yparser/yparser.go @@ -94,9 +94,9 @@ int lyd_data_validate_all(const char *data, const char *depData, const char *oth int lyd_multi_new_leaf(struct lyd_node *parent, const struct lys_module *module, const char *leafVal) { - char s[4048]; + char s[4048]; char *name, *val; - char *saveptr1; + char *saveptr; strcpy(s, leafVal); From 5d87602730a92d1ac280acacfbc0ba2960c8479b Mon Sep 17 00:00:00 2001 From: Asha Behera Date: Fri, 20 Sep 2019 15:03:09 +0530 Subject: [PATCH 3/7] Fix for random UT failure --- src/cvl/internal/yparser/yparser.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/cvl/internal/yparser/yparser.go b/src/cvl/internal/yparser/yparser.go index 0277088d62..a230105692 100644 --- a/src/cvl/internal/yparser/yparser.go +++ b/src/cvl/internal/yparser/yparser.go @@ -97,7 +97,7 @@ int lyd_multi_new_leaf(struct lyd_node *parent, const struct lys_module *module, char s[4048]; char *name, *val, *saveptr; - strcpy(s, leafVal); + strcpy(s, leafVal); name = strtok_r(s, "#", &saveptr); @@ -114,8 +114,6 @@ int lyd_multi_new_leaf(struct lyd_node *parent, const struct lys_module *module, name = strtok_r(NULL, "#", &saveptr); } - - return 0; } struct lyd_node *lyd_find_node(struct lyd_node *root, const char *xpath) From d6bd39e4e42839209fdd16a87062113cbc5f7d32 Mon Sep 17 00:00:00 2001 From: Partha Dutta Date: Tue, 24 Sep 2019 07:40:48 +0530 Subject: [PATCH 4/7] CVL API for fetching dependent and ordered Redis table list --- Makefile | 3 +- src/cvl/cvl.go | 82 +++++++++++++++++++++-- src/cvl/cvl_api.go | 160 ++++++++++++++++++++++++++++++++++++++++++++ src/cvl/cvl_test.go | 52 ++++++++++++++ 4 files changed, 292 insertions(+), 5 deletions(-) diff --git a/Makefile b/Makefile index f78cee25bb..513bb19e93 100644 --- a/Makefile +++ b/Makefile @@ -48,7 +48,8 @@ GO_DEPS_LIST = github.com/gorilla/mux \ gopkg.in/go-playground/validator.v9 \ golang.org/x/crypto/ssh \ github.com/antchfx/jsonquery \ - github.com/antchfx/xmlquery + github.com/antchfx/xmlquery \ + github.com/philopon/go-toposort REST_BIN = $(BUILD_DIR)/rest_server/main diff --git a/src/cvl/cvl.go b/src/cvl/cvl.go index 2b81dbcaac..77f7d8fda2 100644 --- a/src/cvl/cvl.go +++ b/src/cvl/cvl.go @@ -64,6 +64,11 @@ var dbNameToDbNum map[string]uint8 //map of lua script loaded var luaScripts map[string]*redis.Script +type tblFieldPair struct { + tableName string + field string +} + //var tmpDbCache map[string]interface{} //map of table storing map of key-value pair //m["PORT_TABLE] = {"key" : {"f1": "v1"}} //Important schema information to be loaded at bootup time @@ -80,6 +85,7 @@ type modelTableInfo struct { //multiple leafref possible for union mustExp map[string]string tablesForMustExp map[string]CVLOperation + refFromTables []tblFieldPair //list of table or table/field referring to this table } @@ -200,6 +206,9 @@ func init() { CVL_LOG(ERROR ,"Could not enable notification error %s", err) } + //Build reverse leafref info i.e. which table/field uses one table through leafref + buildRefTableInfo() + dbCacheSet(false, "PORT", 0) } @@ -396,6 +405,75 @@ func storeModelInfo(modelFile string, module *yparser.YParserModule) { //such mo } } +func yangToRedisTblName(yangListName string) string { + if (strings.HasSuffix(yangListName, "_LIST")) { + return yangListName[0:len(yangListName) - len("_LIST")] + } + return yangListName +} + +//This functions build info of depdent table/fields which uses a particulat table through leafref +func buildRefTableInfo() { + for tblName, tblInfo := range modelInfo.tableInfo { + if (len(tblInfo.leafRef) == 0) { + continue + } + + //For each leafref update the table used through leafref + for fieldName, leafRefs := range tblInfo.leafRef { + for _, leafRef := range leafRefs { + matches := reLeafRef.FindStringSubmatch(leafRef) + + //We have the leafref table name + if (matches != nil && len(matches) == 5) { //whole + 4 sub matches + refTable := yangToRedisTblName(matches[2]) + refTblInfo := modelInfo.tableInfo[refTable] + + refFromTables := &refTblInfo.refFromTables + *refFromTables = append(*refFromTables, tblFieldPair{tblName, fieldName}) + modelInfo.tableInfo[refTable] = refTblInfo + } + } + } + + } + + //Now sort list 'refFromTables' under each table based on dependency among them + for tblName, tblInfo := range modelInfo.tableInfo { + if (len(tblInfo.refFromTables) == 0) { + continue + } + + depTableList := []string{} + for i:=0; i < len(tblInfo.refFromTables); i++ { + depTableList = append(depTableList, tblInfo.refFromTables[i].tableName) + } + + sortedTableList, _ := cvg.cv.SortDepTables(depTableList) + if (len(sortedTableList) == 0) { + continue + } + + newRefFromTables := []tblFieldPair{} + + for i:=0; i < len(sortedTableList); i++ { + //Find fieldName + fieldName := "" + for j :=0; j < len(tblInfo.refFromTables); j++ { + if (sortedTableList[i] == tblInfo.refFromTables[j].tableName) { + fieldName = tblInfo.refFromTables[j].field + break + } + } + newRefFromTables = append(newRefFromTables, tblFieldPair{sortedTableList[i], fieldName}) + } + //Update sorted refFromTables + tblInfo.refFromTables = newRefFromTables + modelInfo.tableInfo[tblName] = tblInfo + } + +} + //Find the tables names in must expression, these tables data need to be fetched //during semantic validation func addTableNamesForMustExp() { @@ -994,10 +1072,6 @@ func (c *CVL) updateDeleteDataToCache(tableName string, redisKey string) { // as leafref //Use LUA script to find if table has any entry for this leafref -type tblFieldPair struct { - tableName string - field string -} func (c *CVL) findUsedAsLeafRef(tableName, field string) []tblFieldPair { diff --git a/src/cvl/cvl_api.go b/src/cvl/cvl_api.go index faac5b3481..087b8cdcfe 100644 --- a/src/cvl/cvl_api.go +++ b/src/cvl/cvl_api.go @@ -23,9 +23,12 @@ import ( "fmt" "encoding/json" "github.com/go-redis/redis" + //toposort "github.com/stevenle/topsort" + toposort "github.com/philopon/go-toposort" "path/filepath" "cvl/internal/yparser" . "cvl/internal/util" + "strings" ) type CVLValidateType uint @@ -512,3 +515,160 @@ func (c *CVL) ValidateKeyData(key string, data string) CVLRetCode { func (c *CVL) ValidateFields(key string, field string, value string) CVLRetCode { return CVL_NOT_IMPLEMENTED } + +//Sort list of given tables as per their dependency +func (c *CVL) SortDepTables(tableList []string) ([]string, CVLRetCode) { + + //Add all the table names in graph nodes + graph := toposort.NewGraph(len(tableList)) + for ti := 0; ti < len(tableList); ti++ { + graph.AddNodes(tableList[ti]) + } + + //Add all the depedency edges for graph nodes + for ti :=0; ti < len(tableList); ti++ { + for tj :=0; tj < len(tableList); tj++ { + if (tableList[ti] == tableList[tj]) { + continue + } + + for _, leafRefs := range modelInfo.tableInfo[tableList[tj]].leafRef { + for _, leafRef := range leafRefs { + if (strings.Contains(leafRef, tableList[ti])) { + graph.AddEdge(yangToRedisTblName(tableList[tj]), + yangToRedisTblName(tableList[ti])) + } + } + } + } + } + + //Now perform topological sort + result, ret := graph.Toposort() + if ret == false { + return nil, CVL_ERROR + } + + return result, CVL_SUCCESS +} + +//Get the order list(parent then child) of tables in a given YANG module +//within a single model this is obtained using leafref relation +func (c *CVL) GetOrderedTables(yangModule string) ([]string, CVLRetCode) { + tableList := []string{} + + //Get all the table names under this model + for tblName, tblNameInfo := range modelInfo.tableInfo { + if (tblNameInfo.modelName == yangModule) { + tableList = append(tableList, tblName) + } + } + + return c.SortDepTables(tableList) +} + +func (c *CVL) addDepTables(tableMap map[string]bool, tableName string) { + + //Mark it is added in list + tableMap[tableName] = true + + //Now find all tables referred in leafref from this table + for _, leafRefs := range modelInfo.tableInfo[tableName].leafRef { + for _, leafRef := range leafRefs { + matches := reLeafRef.FindStringSubmatch(leafRef) + + //We have the leafref table name + if (matches != nil && len(matches) == 5) { //whole + 4 sub matches + c.addDepTables(tableMap, yangToRedisTblName(matches[2])) //call recursively + } + } + } +} + +//Get the list of dependent tables for a given table in a YANG module +func (c *CVL) GetDepTables(yangModule string, tableName string) ([]string, CVLRetCode) { + tableList := []string{} + tblMap := make(map[string]bool) + + c.addDepTables(tblMap, tableName) + + /* + //Get all the table names under this model + for tblName, _ := range modelInfo.tableInfo { + //if (tblNameInfo.modelName == yangModule) { + // tableList = append(tableList, tblName) + //} + tableList = append(tableList, tblName) + } + */ + + for tblName, _ := range tblMap { + tableList = append(tableList, tblName) + } + + //Add all the table names in graph nodes + graph := toposort.NewGraph(len(tableList)) + for ti := 0; ti < len(tableList); ti++ { + graph.AddNodes(tableList[ti]) + } + + //Add all the depedency edges for graph nodes + for ti :=0; ti < len(tableList); ti++ { + for tj :=0; tj < len(tableList); tj++ { + if (tableList[ti] == tableList[tj]) { + continue + } + + for _, leafRefs := range modelInfo.tableInfo[tableList[tj]].leafRef { + for _, leafRef := range leafRefs { + if (strings.Contains(leafRef, tableList[ti]+"/")) { + graph.AddEdge(yangToRedisTblName(tableList[tj]), + yangToRedisTblName(tableList[ti])) + } + } + } + } + } + + //Now perform topological sort + result, ret := graph.Toposort() + if ret == false { + return nil, CVL_ERROR + } + + return result, CVL_SUCCESS +} + +//Get the dependent data (redis keys) for an entry +func (c *CVL) GetDepDataForDelete(redisKey string) []string { + + tableName, key := splitRedisKey(redisKey) + + mCmd := map[string]*redis.StringSliceCmd{} + pipe := redisClient.Pipeline() + + for _, refTbl := range modelInfo.tableInfo[tableName].refFromTables { + mCmd[refTbl.tableName] = pipe.Keys(fmt.Sprintf("%s|*%s*", refTbl.tableName, key)) //write into pipeline + } + _, err := pipe.Exec() + if err != nil { + CVL_LOG(ERROR, "Failed to fetch dependent key details for table %s", tableName) + } + pipe.Close() + + depKeys := []string{} + for tblName, keys := range mCmd { + res, err := keys.Result() + if (err != nil) { + CVL_LOG(ERROR, "Failed to fetch dependent key details for table %s", tblName) + continue + } + + //Find dependent data for delete recursively + for i :=0; i< len(res); i++ { + depKeys = append(depKeys, c.GetDepDataForDelete(res[i])...) + } + } + + return depKeys +} diff --git a/src/cvl/cvl_test.go b/src/cvl/cvl_test.go index f08394ba34..1cd25ec25d 100644 --- a/src/cvl/cvl_test.go +++ b/src/cvl/cvl_test.go @@ -3442,3 +3442,55 @@ func TestValidateEditConfig_EmptyNode_Positive(t *testing.T) { } } + +func TestSortDepTables(t *testing.T) { + cvSess, _ := cvl.ValidationSessOpen() + + result, _ := cvSess.SortDepTables([]string{"PORT", "ACL_RULE", "ACL_TABLE"}) + + expectedResult := []string{"ACL_RULE", "ACL_TABLE", "PORT"} + + for i := 0; i < len(expectedResult) ; i++ { + if result[i] != expectedResult[i] { + t.Errorf("Validation failed, returned value = %v", result) + break + } + } + + cvl.ValidationSessClose(cvSess) +} + +func TestGetOrderedTables(t *testing.T) { + cvSess, _ := cvl.ValidationSessOpen() + + result, _ := cvSess.GetOrderedTables("sonic-vlan") + + expectedResult := []string{"VLAN_MEMBER", "VLAN"} + + for i := 0; i < len(expectedResult) ; i++ { + if result[i] != expectedResult[i] { + t.Errorf("Validation failed, returned value = %v", result) + break + } + } + + cvl.ValidationSessClose(cvSess) +} + +func TestGetDepTables(t *testing.T) { + cvSess, _ := cvl.ValidationSessOpen() + + result, _ := cvSess.GetDepTables("sonic-acl", "ACL_RULE") + + expectedResult := []string{"ACL_RULE", "ACL_TABLE", "MIRROR_SESSION"} + + for i := 0; i < len(expectedResult) ; i++ { + if result[i] != expectedResult[i] { + t.Errorf("Validation failed, returned value = %v", result) + break + } + } + + cvl.ValidationSessClose(cvSess) + +} From a699701cea8d7e922a5fd1c636e441b417d63d3b Mon Sep 17 00:00:00 2001 From: Partha Dutta Date: Thu, 10 Oct 2019 12:44:59 +0530 Subject: [PATCH 5/7] Adding max-elements check --- src/cvl/cvl.go | 161 ++++++++++++++++----------------------- src/cvl/cvl_api.go | 7 ++ src/cvl/cvl_luascript.go | 4 + src/cvl/schema/Makefile | 1 - 4 files changed, 78 insertions(+), 95 deletions(-) diff --git a/src/cvl/cvl.go b/src/cvl/cvl.go index 4cf42a1fab..1f6087ff4f 100644 --- a/src/cvl/cvl.go +++ b/src/cvl/cvl.go @@ -22,6 +22,7 @@ import ( "fmt" "os" "strings" + "strconv" "regexp" "time" log "github.com/golang/glog" @@ -80,6 +81,7 @@ type modelTableInfo struct { keys []string redisKeyDelim string redisKeyPattern string + redisTableSize int mapLeaf []string //for 'mapping list' leafRef map[string][]string //for storing all leafrefs for a leaf in a table, //multiple leafref possible for union @@ -284,6 +286,8 @@ func storeModelInfo(modelFile string, module *yparser.YParserModule) { //such mo tableInfo.dbNum = CONFIG_DB //default delim '|' tableInfo.redisKeyDelim = "|" + //Default table size is -1 i.e. size limit + tableInfo.redisTableSize = -1 modelInfo.allKeyDelims[tableInfo.redisKeyDelim] = true fieldCount := 0 @@ -342,34 +346,36 @@ func storeModelInfo(modelFile string, module *yparser.YParserModule) { //such mo } leafRefNodes := xmlquery.Find(listNode, "//type[@name='leafref']") - if (leafRefNodes == nil) { - //Store the tableInfo in global data - modelInfo.tableInfo[tableName] = &tableInfo - - continue - } + if (leafRefNodes != nil) { + tableInfo.leafRef = make(map[string][]string) + for _, leafRefNode := range leafRefNodes { + if (leafRefNode.Parent == nil || leafRefNode.FirstChild == nil) { + continue + } - tableInfo.leafRef = make(map[string][]string) - for _, leafRefNode := range leafRefNodes { - if (leafRefNode.Parent == nil || leafRefNode.FirstChild == nil) { - continue - } + //Get the leaf/leaf-list name holding this leafref + //Note that leaf can have union of leafrefs + leafName := "" + for node := leafRefNode.Parent; node != nil; node = node.Parent { + if (node.Data == "leaf" || node.Data == "leaf-list") { + leafName = getXmlNodeAttr(node, "name") + break + } + } - //Get the leaf/leaf-list name holding this leafref - //Note that leaf can have union of leafrefs - leafName := "" - for node := leafRefNode.Parent; node != nil; node = node.Parent { - if (node.Data == "leaf" || node.Data == "leaf-list") { - leafName = getXmlNodeAttr(node, "name") - break + //Store the leafref path + if (leafName != "") { + tableInfo.leafRef[leafName] = append(tableInfo.leafRef[leafName], + getXmlNodeAttr(leafRefNode.FirstChild, "value")) } } + } - //Store the leafref path - if (leafName != "") { - tableInfo.leafRef[leafName] = append(tableInfo.leafRef[leafName], - getXmlNodeAttr(leafRefNode.FirstChild, "value")) - } + //Find if any 'max-elements' is there, this syntax should be handled as special case + maxElemVal := xmlquery.Find(listNode, "/max-elements/@value") + if maxElemVal != nil { + //Update list size + tableInfo.redisTableSize, _ = strconv.Atoi(maxElemVal[0].Data) } //Find all 'must' expression and store the against its parent node @@ -474,75 +480,6 @@ func buildRefTableInfo() { } -func yangToRedisTblName(yangListName string) string { - if (strings.HasSuffix(yangListName, "_LIST")) { - return yangListName[0:len(yangListName) - len("_LIST")] - } - return yangListName -} - -//This functions build info of depdent table/fields which uses a particulat table through leafref -func buildRefTableInfo() { - for tblName, tblInfo := range modelInfo.tableInfo { - if (len(tblInfo.leafRef) == 0) { - continue - } - - //For each leafref update the table used through leafref - for fieldName, leafRefs := range tblInfo.leafRef { - for _, leafRef := range leafRefs { - matches := reLeafRef.FindStringSubmatch(leafRef) - - //We have the leafref table name - if (matches != nil && len(matches) == 5) { //whole + 4 sub matches - refTable := yangToRedisTblName(matches[2]) - refTblInfo := modelInfo.tableInfo[refTable] - - refFromTables := &refTblInfo.refFromTables - *refFromTables = append(*refFromTables, tblFieldPair{tblName, fieldName}) - modelInfo.tableInfo[refTable] = refTblInfo - } - } - } - - } - - //Now sort list 'refFromTables' under each table based on dependency among them - for tblName, tblInfo := range modelInfo.tableInfo { - if (len(tblInfo.refFromTables) == 0) { - continue - } - - depTableList := []string{} - for i:=0; i < len(tblInfo.refFromTables); i++ { - depTableList = append(depTableList, tblInfo.refFromTables[i].tableName) - } - - sortedTableList, _ := cvg.cv.SortDepTables(depTableList) - if (len(sortedTableList) == 0) { - continue - } - - newRefFromTables := []tblFieldPair{} - - for i:=0; i < len(sortedTableList); i++ { - //Find fieldName - fieldName := "" - for j :=0; j < len(tblInfo.refFromTables); j++ { - if (sortedTableList[i] == tblInfo.refFromTables[j].tableName) { - fieldName = tblInfo.refFromTables[j].field - break - } - } - newRefFromTables = append(newRefFromTables, tblFieldPair{sortedTableList[i], fieldName}) - } - //Update sorted refFromTables - tblInfo.refFromTables = newRefFromTables - modelInfo.tableInfo[tblName] = tblInfo - } - -} - //Find the tables names in must expression, these tables data need to be fetched //during semantic validation func addTableNamesForMustExp() { @@ -1124,6 +1061,44 @@ func (c *CVL) checkDeleteConstraint(cfgData []CVLEditConfigData, return CVL_SUCCESS } +//This function should be called before adding any new entry +//Checks max-elements defined with (current number of entries +//getting added + entries already added and present in request +//cache + entries present in Redis DB +func (c *CVL) checkMaxElemConstraint(tableName string) CVLRetCode { + var nokey []string + + if modelInfo.tableInfo[tableName].redisTableSize == -1 { + //No limit for table size + return CVL_SUCCESS + } + + redisEntries, err := luaScripts["count_entries"].Run(redisClient, nokey, tableName).Result() + curSize := int(redisEntries.(int64)) + + if err != nil { + return CVL_FAILURE + } + + //Count only the entries getting created + for _, cfgDataArr := range c.requestCache[tableName] { + for _, cfgReqData := range cfgDataArr { + if (cfgReqData.VOp != OP_CREATE) { + continue + } + + curSize = curSize + 1 + if (curSize > modelInfo.tableInfo[tableName].redisTableSize) { + //Does not meet the constraint + return CVL_SYNTAX_ERROR + } + } + } + + + return CVL_SUCCESS +} + //Add the data which are referring this key func (c *CVL) updateDeleteDataToCache(tableName string, redisKey string) { if _, existing := c.tmpDbCache[tableName]; existing == false { @@ -1140,8 +1115,6 @@ func (c *CVL) updateDeleteDataToCache(tableName string, redisKey string) { //Find which all tables (and which field) is using given (tableName/field) // as leafref //Use LUA script to find if table has any entry for this leafref - - func (c *CVL) findUsedAsLeafRef(tableName, field string) []tblFieldPair { var tblFieldPairArr []tblFieldPair diff --git a/src/cvl/cvl_api.go b/src/cvl/cvl_api.go index 087b8cdcfe..fa783b368f 100644 --- a/src/cvl/cvl_api.go +++ b/src/cvl/cvl_api.go @@ -322,6 +322,13 @@ func (c *CVL) ValidateEditConfig(cfgData []CVLEditConfigData) (CVLErrorInfo, CVL switch cfgDataItem.VOp { case OP_CREATE: + //Check max-element constraint + if ret := c.checkMaxElemConstraint(tbl); ret != CVL_SUCCESS { + cvlErrObj.ErrCode = CVL_SYNTAX_ERROR + cvlErrObj.CVLErrDetails = cvlErrorMap[cvlErrObj.ErrCode] + return cvlErrObj, CVL_SYNTAX_ERROR + } + if (c.addTableEntryForMustExp(&cfgDataItem, tbl) != CVL_SUCCESS) { c.addTableDataForMustExp(cfgDataItem.VOp, tbl) } diff --git a/src/cvl/cvl_luascript.go b/src/cvl/cvl_luascript.go index fd4c863ad1..41cbe8e7a6 100644 --- a/src/cvl/cvl_luascript.go +++ b/src/cvl/cvl_luascript.go @@ -62,4 +62,8 @@ func loadLuaScript() { return "" `) + //Find current number of entries in a table + luaScripts["count_entries"] = redis.NewScript(` + return #redis.call('KEYS', ARGV[1].."*") + `) } diff --git a/src/cvl/schema/Makefile b/src/cvl/schema/Makefile index 3af437be35..4a1e7360e6 100644 --- a/src/cvl/schema/Makefile +++ b/src/cvl/schema/Makefile @@ -28,7 +28,6 @@ out_common=$(patsubst %.yang, %.yin, $(shell ls -1 $(sonic_yang_common)/*.yang | out_tree=$(patsubst %.yang, %.tree, $(src_files)) search_path=$(sonic_yang):$(sonic_yang_common):$(sonic_yang_common)/ietf -#all: yamlGen allyangs.tree allyangs_tree.html schema all: schema schema: $(out) $(out_common) From 1b120100dca25a05dfe9bb93baae357a83a2272d Mon Sep 17 00:00:00 2001 From: Partha Dutta Date: Thu, 10 Oct 2019 14:54:40 +0530 Subject: [PATCH 6/7] Adding test cases for 'max-elements' --- src/cvl/cvl.go | 2 +- src/cvl/cvl_api.go | 10 -- src/cvl/cvl_test.go | 114 +++++++++++++++++- .../schema/sonic-device-metadata.yang | 1 + 4 files changed, 115 insertions(+), 12 deletions(-) diff --git a/src/cvl/cvl.go b/src/cvl/cvl.go index 1f6087ff4f..2f95d1e024 100644 --- a/src/cvl/cvl.go +++ b/src/cvl/cvl.go @@ -375,7 +375,7 @@ func storeModelInfo(modelFile string, module *yparser.YParserModule) { //such mo maxElemVal := xmlquery.Find(listNode, "/max-elements/@value") if maxElemVal != nil { //Update list size - tableInfo.redisTableSize, _ = strconv.Atoi(maxElemVal[0].Data) + tableInfo.redisTableSize, _ = strconv.Atoi(maxElemVal[0].FirstChild.Data) } //Find all 'must' expression and store the against its parent node diff --git a/src/cvl/cvl_api.go b/src/cvl/cvl_api.go index fa783b368f..89abe8d50d 100644 --- a/src/cvl/cvl_api.go +++ b/src/cvl/cvl_api.go @@ -599,16 +599,6 @@ func (c *CVL) GetDepTables(yangModule string, tableName string) ([]string, CVLRe c.addDepTables(tblMap, tableName) - /* - //Get all the table names under this model - for tblName, _ := range modelInfo.tableInfo { - //if (tblNameInfo.modelName == yangModule) { - // tableList = append(tableList, tblName) - //} - tableList = append(tableList, tblName) - } - */ - for tblName, _ := range tblMap { tableList = append(tableList, tblName) } diff --git a/src/cvl/cvl_test.go b/src/cvl/cvl_test.go index 1cd25ec25d..03d970041c 100644 --- a/src/cvl/cvl_test.go +++ b/src/cvl/cvl_test.go @@ -3450,6 +3450,11 @@ func TestSortDepTables(t *testing.T) { expectedResult := []string{"ACL_RULE", "ACL_TABLE", "PORT"} + if len(expectedResult) != len(result) { + t.Errorf("Validation failed, returned value = %v", result) + return + } + for i := 0; i < len(expectedResult) ; i++ { if result[i] != expectedResult[i] { t.Errorf("Validation failed, returned value = %v", result) @@ -3467,6 +3472,11 @@ func TestGetOrderedTables(t *testing.T) { expectedResult := []string{"VLAN_MEMBER", "VLAN"} + if len(expectedResult) != len(result) { + t.Errorf("Validation failed, returned value = %v", result) + return + } + for i := 0; i < len(expectedResult) ; i++ { if result[i] != expectedResult[i] { t.Errorf("Validation failed, returned value = %v", result) @@ -3482,7 +3492,12 @@ func TestGetDepTables(t *testing.T) { result, _ := cvSess.GetDepTables("sonic-acl", "ACL_RULE") - expectedResult := []string{"ACL_RULE", "ACL_TABLE", "MIRROR_SESSION"} + expectedResult := []string{"ACL_RULE", "ACL_TABLE", "MIRROR_SESSION", "PORT"} + + if len(expectedResult) != len(result) { + t.Errorf("Validation failed, returned value = %v", result) + return + } for i := 0; i < len(expectedResult) ; i++ { if result[i] != expectedResult[i] { @@ -3492,5 +3507,102 @@ func TestGetDepTables(t *testing.T) { } cvl.ValidationSessClose(cvSess) +} + +func TestMaxElements_All_Entries_In_Request(t *testing.T) { + cvSess, _ := cvl.ValidationSessOpen() + + cfgData := []cvl.CVLEditConfigData{ + cvl.CVLEditConfigData{ + cvl.VALIDATE_ALL, + cvl.OP_CREATE, + "DEVICE_METADATA|localhost", + map[string]string{ + "hwsku": "Force10-S6100", + "hostname": "sonic-s6100-01", + "platform": "x86_64-dell_s6100_c2538-r0", + "mac": "4c:76:25:f4:70:82", + "deployment_id": "1", + }, + }, + } + + //Add first element + cvlErrInfo, _ := cvSess.ValidateEditConfig(cfgData) + + if cvlErrInfo.ErrCode != cvl.CVL_SUCCESS { + t.Errorf("Config Validation failed -- error details %v", cvlErrInfo) + } + + cfgData1 := []cvl.CVLEditConfigData{ + cvl.CVLEditConfigData{ + cvl.VALIDATE_ALL, + cvl.OP_CREATE, + "DEVICE_METADATA|localhost1", + map[string]string{ + "hwsku": "Force10-S6101", + "hostname": "sonic-s6100-02", + "platform": "x86_64-dell_s6100_c2538-r0", + "mac": "4c:76:25:f4:70:83", + "deployment_id": "2", + }, + }, + } + + //Try to add second element + cvlErrInfo, _ = cvSess.ValidateEditConfig(cfgData1) + + cvl.ValidationSessClose(cvSess) + + //Should fail as "DEVICE_METADATA" has max-elements as '1' + if cvlErrInfo.ErrCode == cvl.CVL_SUCCESS { + t.Errorf("Config Validation failed -- error details %v", cvlErrInfo) + } + +} + +func TestMaxElements_Entries_In_Redis(t *testing.T) { + depDataMap := map[string]interface{} { + "DEVICE_METADATA" : map[string]interface{} { + "localhost": map[string] interface{} { + "hwsku": "Force10-S6100", + "hostname": "sonic-s6100-01", + "platform": "x86_64-dell_s6100_c2538-r0", + "mac": "4c:76:25:f4:70:82", + "deployment_id": "1", + }, + }, + } + + loadConfigDB(rclient, depDataMap) + + cvSess, _ := cvl.ValidationSessOpen() + + cfgData := []cvl.CVLEditConfigData{ + cvl.CVLEditConfigData{ + cvl.VALIDATE_ALL, + cvl.OP_CREATE, + "DEVICE_METADATA|localhost1", + map[string]string{ + "hwsku": "Force10-S6101", + "hostname": "sonic-s6100-02", + "platform": "x86_64-dell_s6100_c2538-r0", + "mac": "4c:76:25:f4:70:83", + "deployment_id": "2", + }, + }, + } + + //Try to add second element + cvlErrInfo, _ := cvSess.ValidateEditConfig(cfgData) + + unloadConfigDB(rclient, depDataMap) + + cvl.ValidationSessClose(cvSess) + + //Should fail as "DEVICE_METADATA" has max-elements as '1' + if cvlErrInfo.ErrCode == cvl.CVL_SUCCESS { + t.Errorf("Config Validation failed -- error details %v", cvlErrInfo) + } } diff --git a/src/cvl/testdata/schema/sonic-device-metadata.yang b/src/cvl/testdata/schema/sonic-device-metadata.yang index 014f88cd47..217e5d3739 100644 --- a/src/cvl/testdata/schema/sonic-device-metadata.yang +++ b/src/cvl/testdata/schema/sonic-device-metadata.yang @@ -34,6 +34,7 @@ module sonic-device-metadata { list DEVICE_METADATA_LIST { key "name"; + max-elements 1; leaf name{ type string; From 73a8d36d9805b40c81348a895965e49e80a95f3f Mon Sep 17 00:00:00 2001 From: Partha Dutta Date: Thu, 10 Oct 2019 15:30:20 +0530 Subject: [PATCH 7/7] Minor changes --- src/cvl/cvl.go | 4 ++-- src/cvl/cvl_api.go | 1 - 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/cvl/cvl.go b/src/cvl/cvl.go index 2f95d1e024..34eb153170 100644 --- a/src/cvl/cvl.go +++ b/src/cvl/cvl.go @@ -418,7 +418,7 @@ func yangToRedisTblName(yangListName string) string { return yangListName } -//This functions build info of depdent table/fields which uses a particulat table through leafref +//This functions build info of dependent table/fields which uses a particulat table through leafref func buildRefTableInfo() { for tblName, tblInfo := range modelInfo.tableInfo { if (len(tblInfo.leafRef) == 0) { @@ -1064,7 +1064,7 @@ func (c *CVL) checkDeleteConstraint(cfgData []CVLEditConfigData, //This function should be called before adding any new entry //Checks max-elements defined with (current number of entries //getting added + entries already added and present in request -//cache + entries present in Redis DB +//cache + entries present in Redis DB) func (c *CVL) checkMaxElemConstraint(tableName string) CVLRetCode { var nokey []string diff --git a/src/cvl/cvl_api.go b/src/cvl/cvl_api.go index 89abe8d50d..2749933cbc 100644 --- a/src/cvl/cvl_api.go +++ b/src/cvl/cvl_api.go @@ -23,7 +23,6 @@ import ( "fmt" "encoding/json" "github.com/go-redis/redis" - //toposort "github.com/stevenle/topsort" toposort "github.com/philopon/go-toposort" "path/filepath" "cvl/internal/yparser"