diff --git a/.azure-pipelines/build-sairedis-template.yml b/.azure-pipelines/build-sairedis-template.yml index a88922f2b..d1ae38460 100644 --- a/.azure-pipelines/build-sairedis-template.yml +++ b/.azure-pipelines/build-sairedis-template.yml @@ -99,8 +99,9 @@ jobs: path: $(Build.ArtifactStagingDirectory)/download artifact: common-lib patterns: | - target/debs/${{ parameters.debian_version }}/libyang-*_1.0*.deb - target/debs/${{ parameters.debian_version }}/libyang_1.0*.deb + target/debs/${{ parameters.debian_version }}/libyang3_*.deb + target/debs/${{ parameters.debian_version }}/libyang-dev_3*.deb + target/debs/${{ parameters.debian_version }}/python3-libyang*.deb displayName: "Download libyang from common lib" - script: | set -ex diff --git a/.azure-pipelines/build-swss-template.yml b/.azure-pipelines/build-swss-template.yml index 306629f10..8ec7d10e7 100644 --- a/.azure-pipelines/build-swss-template.yml +++ b/.azure-pipelines/build-swss-template.yml @@ -102,11 +102,13 @@ jobs: target/debs/${{ parameters.debian_version }}/libnl-genl*.deb target/debs/${{ parameters.debian_version }}/libnl-route*.deb target/debs/${{ parameters.debian_version }}/libnl-nf*.deb - target/debs/${{ parameters.debian_version }}/libyang-*_1.0*.deb - target/debs/${{ parameters.debian_version }}/libyang_1.0*.deb + target/debs/${{ parameters.debian_version }}/libyang3_*.deb + target/debs/${{ parameters.debian_version }}/libyang-dev_3*.deb + target/debs/${{ parameters.debian_version }}/python3-libyang*.deb target/debs/${{ parameters.debian_version }}/libprotobuf*.deb target/debs/${{ parameters.debian_version }}/libprotoc*.deb target/debs/${{ parameters.debian_version }}/protobuf-compiler*.deb + target/debs/${{ parameters.debian_version }}/libnexthopgroup*.deb displayName: "Download common libs" - task: DownloadPipelineArtifact@2 inputs: diff --git a/.azure-pipelines/build-template.yml b/.azure-pipelines/build-template.yml index 52a58be1d..0402ea43e 100644 --- a/.azure-pipelines/build-template.yml +++ b/.azure-pipelines/build-template.yml @@ -79,10 +79,9 @@ jobs: ${{ else }}: artifact: common-lib.${{ parameters.arch }} patterns: | - target/debs/${{ parameters.debian_version }}/libyang-*_1.0*.deb - target/debs/${{ parameters.debian_version }}/libyang_1.0*.deb - target/debs/${{ parameters.debian_version }}/libyang-cpp_*.deb - target/debs/${{ parameters.debian_version }}/python3-yang_*.deb + target/debs/${{ parameters.debian_version }}/libyang3_*.deb + target/debs/${{ parameters.debian_version }}/libyang-dev_3*.deb + target/debs/${{ parameters.debian_version }}/python3-libyang*.deb displayName: "Download libyang from ${{ parameters.arch }} common lib" condition: ne('${{ parameters.debian_version }}', 'trixie') - task: DownloadPipelineArtifact@2 @@ -90,18 +89,17 @@ jobs: source: specific project: build pipeline: Azure.sonic-buildimage.common_libs - runVersion: 'specific' - pipelineId: '926659' + runVersion: 'latestFromBranch' + runBranch: 'refs/heads/$(BUILD_BRANCH)' path: $(Build.ArtifactStagingDirectory)/download ${{ if eq(parameters.arch, 'amd64') }}: artifact: common-lib ${{ else }}: artifact: common-lib.${{ parameters.arch }} patterns: | - target/debs/${{ parameters.debian_version }}/libyang-*_1.0*.deb - target/debs/${{ parameters.debian_version }}/libyang_1.0*.deb - target/debs/${{ parameters.debian_version }}/libyang-cpp_*.deb - target/debs/${{ parameters.debian_version }}/python3-yang_*.deb + target/debs/${{ parameters.debian_version }}/libyang3_*.deb + target/debs/${{ parameters.debian_version }}/libyang-dev_3*.deb + target/debs/${{ parameters.debian_version }}/python3-libyang*.deb target/debs/${{ parameters.debian_version }}/libpcre*.deb displayName: "Download libyang from ${{ parameters.arch }} common lib" condition: eq('${{ parameters.debian_version }}', 'trixie') diff --git a/.azure-pipelines/test-docker-sonic-vs-template.yml b/.azure-pipelines/test-docker-sonic-vs-template.yml index 6dc9476b9..859abadb9 100644 --- a/.azure-pipelines/test-docker-sonic-vs-template.yml +++ b/.azure-pipelines/test-docker-sonic-vs-template.yml @@ -53,10 +53,8 @@ jobs: path: $(Build.ArtifactStagingDirectory)/download artifact: common-lib patterns: | - target/debs/${{ parameters.debian_version }}/libyang-*_1.0*.deb - target/debs/${{ parameters.debian_version }}/libyang_1.0*.deb - target/debs/${{ parameters.debian_version }}/libyang-cpp_*.deb - target/debs/${{ parameters.debian_version }}/python3-yang_*.deb + target/debs/${{ parameters.debian_version }}/libyang3_*.deb + target/debs/${{ parameters.debian_version }}/libyang-dev_3*.deb displayName: "Download libyang from common lib" - task: DownloadPipelineArtifact@2 inputs: @@ -93,11 +91,22 @@ jobs: sudo sonic-swss-common/.azure-pipelines/build_and_install_module.sh - # Install libyang packages from downloaded artifacts - sudo dpkg -i $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/libyang-*_1.0*.deb \ - $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/libyang_1.0*.deb \ - $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/libyang-cpp_*.deb \ - $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/python3-yang_*.deb + # Install libyang packages from downloaded artifacts. Use apt rather + # than `dpkg -i` so libyang-dev's libpcre2-dev dep is auto-resolved + # from the host repos. + sudo apt-get install -y \ + $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/libyang3_*.deb \ + $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/libyang-dev_3*.deb + + # python3-libyang's bookworm .deb pins python3 (>= 3.11~, << 3.12) + # and the sonictest pool host runs Ubuntu 22.04 (python 3.10), so the + # .deb won't install regardless of resolver. Build the Python bindings + # from PyPI instead — same fallback as the inline amd64/ubuntu-22.04 + # job. --no-build-isolation + apt's python3-cffi sidesteps a cffi + # version-mismatch Exception that jammy's pip 22.0.2 build-isolation + # env otherwise triggers. + sudo apt-get install -y python3-cffi + sudo pip3 install --no-build-isolation 'libyang==3.3.0' sudo dpkg -i $(Build.ArtifactStagingDirectory)/download/libprotobuf*_amd64.deb $(Build.ArtifactStagingDirectory)/download/libprotobuf-lite*_amd64.deb $(Build.ArtifactStagingDirectory)/download/python3-protobuf*_amd64.deb sudo dpkg -i $(Build.ArtifactStagingDirectory)/download/libdashapi*.deb diff --git a/azure-pipelines.yml b/azure-pipelines.yml index afc94b712..68000c18c 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -52,62 +52,6 @@ stages: - stage: Build jobs: - - job: - displayName: "amd64/ubuntu-20.04" - condition: false - pool: - vmImage: 'ubuntu-20.04' - - steps: - - script: | - sudo apt-get update - sudo apt-get install -y make libtool m4 autoconf dh-exec debhelper cmake pkg-config nlohmann-json3-dev \ - libhiredis-dev libnl-3-dev libnl-genl-3-dev libnl-route-3-dev libnl-nf-3-dev swig3.0 \ - libpython2.7-dev libboost-dev libboost-serialization-dev uuid-dev libzmq3-dev - sudo apt-get install -y sudo - sudo apt-get install -y redis-server redis-tools - sudo apt-get install -y python3-pip - sudo pip3 install pytest - sudo apt-get install -y python - sudo apt-get install cmake libgtest-dev libgmock-dev libyang-dev - cd /usr/src/gtest && sudo cmake . && sudo make - ARCH=$(dpkg --print-architecture) - set -x - sudo curl -fsSL -o /usr/local/bin/bazel \ - https://github.com/bazelbuild/bazelisk/releases/latest/download/bazelisk-linux-${ARCH} - sudo chmod 755 /usr/local/bin/bazel - displayName: "Install dependencies" - - task: DownloadPipelineArtifact@2 - inputs: - source: specific - project: build - pipeline: 142 - artifact: sonic-buildimage.vs - runVersion: 'latestFromBranch' - runBranch: 'refs/heads/$(BUILD_BRANCH)' - path: $(Build.ArtifactStagingDirectory)/download - patterns: | - target/python-wheels/${{ parameters.debian_version }}/sonic_yang_mgmt-1.0-py3-none-any.whl - target/python-wheels/${{ parameters.debian_version }}/sonic_yang_models-1.0-py3-none-any.whl - displayName: "Download yang wheel from latest sonic-buildimage build" - - script: | - set -ex - sudo pip3 install ./download/target/python-wheels/${{ parameters.debian_version }}/sonic_yang_mgmt-1.0-py3-none-any.whl \ - ./download/target/python-wheels/${{ parameters.debian_version }}/sonic_yang_models-1.0-py3-none-any.whl - workingDirectory: $(Build.ArtifactStagingDirectory) - displayName: "Install yang wheel" - - script: | - ./autogen.sh - dpkg-buildpackage -us -uc -b -j$(nproc) && cp ../*.deb . - displayName: "Compile sonic swss common" - - script: | - bazel build //... - bazel test //... - displayName: "Compile and test all Bazel targets" - - publish: $(System.DefaultWorkingDirectory)/ - artifact: sonic-swss-common.amd64.ubuntu20_04 - displayName: "Archive swss common debian packages" - - job: displayName: "amd64/ubuntu-22.04" pool: @@ -142,17 +86,22 @@ stages: path: $(Build.ArtifactStagingDirectory)/download artifact: common-lib patterns: | - target/debs/${{ parameters.debian_version }}/libyang-*_1.0*.deb - target/debs/${{ parameters.debian_version }}/libyang_1.0*.deb - target/debs/${{ parameters.debian_version }}/libyang-cpp_*.deb - target/debs/${{ parameters.debian_version }}/python3-yang_*.deb + target/debs/${{ parameters.debian_version }}/libyang3_*.deb + target/debs/${{ parameters.debian_version }}/libyang-dev_3*.deb displayName: "Download yang deb from amd64 common lib" - script: | set -ex - sudo dpkg -i $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/libyang_1.0*.deb \ - $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/libyang-*_1.0*.deb \ - $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/libyang-cpp_*.deb \ - $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/python3-yang_*.deb + sudo dpkg -i $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/libyang3_*.deb \ + $(Build.ArtifactStagingDirectory)/download/target/debs/${{ parameters.debian_version }}/libyang-dev_3*.deb + # python3-libyang's bookworm .deb pins python3 (>= 3.11~, << 3.12) and + # won't install on Ubuntu 22.04 (python 3.10). Build the Python bindings + # from PyPI instead — they link against the libyang3 we just installed. + # Use --no-build-isolation with apt's python3-cffi to avoid jammy pip + # 22.0.2 picking up cffi 2.0.0 in its build env while Debian's older + # /usr/lib/python3/dist-packages/_cffi_backend.so still wins on sys.path + # (causing a "Version mismatch" Exception inside cffi at build time). + sudo apt-get install -y python3-cffi + sudo pip3 install --no-build-isolation 'libyang==3.3.0' workingDirectory: $(Build.ArtifactStagingDirectory) displayName: "Install yang deb from common lib" - task: DownloadPipelineArtifact@2 diff --git a/common/defaultvalueprovider.cpp b/common/defaultvalueprovider.cpp index ce782fe43..d3f6f7bfe 100644 --- a/common/defaultvalueprovider.cpp +++ b/common/defaultvalueprovider.cpp @@ -16,6 +16,27 @@ using namespace std; using namespace swss; +/* libyang1 vs libyang3 API shim (see defaultvalueprovider.h for the version + * sentinel rationale). Macros below collapse trivial spelling differences; + * structurally divergent code (key extraction, leaf-list defaults, ly_ctx_new, + * module->data layout) still needs explicit #ifdef branches below. */ +#ifdef LY_ARRAY_COUNT +# define SWSS_LYS_NODE_CHILD(n) lysc_node_child(n) +# define SWSS_LYS_DFLT_STR(n) lyd_value_get_canonical((n)->module->ctx, (n)->dflt) +# define SWSS_MODULE_DATA(m) ((m)->compiled ? (m)->compiled->data : nullptr) +# define SWSS_LY_CTX_DESTROY(ctx) ly_ctx_destroy(ctx) +// libyang1 treated revision="" as "load latest"; libyang3 instead requires +// it to match the module's revision exactly, so we have to feed it NULL when +// the caller asked for any revision. +# define SWSS_LY_CTX_LOAD_MODULE(ctx, n, rev) ly_ctx_load_module((ctx), (n), ((rev) && *(rev)) ? (rev) : NULL, NULL) +#else +# define SWSS_LYS_NODE_CHILD(n) ((n)->child) +# define SWSS_LYS_DFLT_STR(n) ((n)->dflt) +# define SWSS_MODULE_DATA(m) ((m)->data) +# define SWSS_LY_CTX_DESTROY(ctx) ly_ctx_destroy(ctx, NULL) +# define SWSS_LY_CTX_LOAD_MODULE(ctx, n, rev) ly_ctx_load_module(ctx, n, rev) +#endif + [[noreturn]] void ThrowRunTimeError(string message) { SWSS_LOG_ERROR("DefaultValueProvider: %s", message.c_str()); @@ -127,7 +148,7 @@ bool TableInfoMultipleList::FoundFieldMappingByKey(const string &key, FieldDefau return keySchema != m_defaultValueMapping.end(); } -shared_ptr DefaultValueHelper::GetKeySchema(struct lys_node* tableChildNode) +shared_ptr DefaultValueHelper::GetKeySchema(SWSS_LYS_NODE* tableChildNode) { SWSS_LOG_DEBUG("DefaultValueHelper::GetKeySchema %s\n",tableChildNode->name); @@ -138,7 +159,23 @@ shared_ptr DefaultValueHelper::GetKeySchema(struct lys_node* tableChi SWSS_LOG_DEBUG("Child list: %s\n",tableChildNode->name); // when a top level container contains list, the key defined by the 'keys' field. - struct lys_node_list *listNode = (struct lys_node_list*)tableChildNode; + SWSS_LYS_NODE_LIST *listNode = (SWSS_LYS_NODE_LIST*)tableChildNode; +#ifdef LY_ARRAY_COUNT + // libyang3 has no keys_str — walk compiled children and flag key nodes. + for (SWSS_LYS_NODE *node = listNode->child; node != NULL; node = node->next) + { + if (!lysc_is_key(node)) + { + continue; + } + if (keyValue.length()) + { + keyValue += " "; + } + keyValue += node->name; + keyFieldCount++; + } +#else if (listNode->keys_str == nullptr) { SWSS_LOG_ERROR("Ignore empty key string on list: %s\n",tableChildNode->name); @@ -147,6 +184,7 @@ shared_ptr DefaultValueHelper::GetKeySchema(struct lys_node* tableChi string key(listNode->keys_str); keyFieldCount = (int)count(key.begin(), key.end(), ' ') + 1; +#endif } else if (tableChildNode->nodetype == LYS_CONTAINER) { @@ -164,16 +202,16 @@ shared_ptr DefaultValueHelper::GetKeySchema(struct lys_node* tableChi return make_shared(keyValue, keyFieldCount); } -void DefaultValueHelper::GetDefaultValueInfoForLeaf(struct lys_node_leaf* leafNode, shared_ptr fieldMapping) +void DefaultValueHelper::GetDefaultValueInfoForLeaf(SWSS_LYS_NODE_LEAF* leafNode, shared_ptr fieldMapping) { if (leafNode->dflt) { - SWSS_LOG_DEBUG("field: %s, default: %s\n",leafNode->name, leafNode->dflt); - fieldMapping->emplace(string(leafNode->name), string(leafNode->dflt)); + SWSS_LOG_DEBUG("field: %s, default: %s\n",leafNode->name, SWSS_LYS_DFLT_STR(leafNode)); + fieldMapping->emplace(string(leafNode->name), string(SWSS_LYS_DFLT_STR(leafNode))); } } -void DefaultValueHelper::GetDefaultValueInfoForChoice(struct lys_node_choice* choiceNode, shared_ptr fieldMapping) +void DefaultValueHelper::GetDefaultValueInfoForChoice(SWSS_LYS_NODE_CHOICE* choiceNode, shared_ptr fieldMapping) { if (choiceNode->dflt == nullptr) { @@ -190,24 +228,46 @@ void DefaultValueHelper::GetDefaultValueInfoForChoice(struct lys_node_choice* ch SWSS_LOG_ERROR("choice case %s is not a leaf node\n",fieldInChoice->name); continue; } - + SWSS_LOG_DEBUG("default choice leaf field: %s\n",fieldInChoice->name); WARNINGS_NO_CAST_ALIGN - struct lys_node_leaf *dfltLeafNode = reinterpret_cast(fieldInChoice); + SWSS_LYS_NODE_LEAF *dfltLeafNode = reinterpret_cast(fieldInChoice); WARNINGS_RESET if (dfltLeafNode->dflt) { - SWSS_LOG_DEBUG("default choice leaf field: %s, default: %s\n",dfltLeafNode->name, dfltLeafNode->dflt); - fieldMapping->emplace(string(fieldInChoice->name), string(dfltLeafNode->dflt)); + SWSS_LOG_DEBUG("default choice leaf field: %s, default: %s\n",dfltLeafNode->name, SWSS_LYS_DFLT_STR(dfltLeafNode)); + fieldMapping->emplace(string(fieldInChoice->name), string(SWSS_LYS_DFLT_STR(dfltLeafNode))); } fieldInChoice = fieldInChoice->next; } } -void DefaultValueHelper::GetDefaultValueInfoForLeaflist(struct lys_node_leaflist *listNode, shared_ptr fieldMapping) +void DefaultValueHelper::GetDefaultValueInfoForLeaflist(SWSS_LYS_NODE_LEAFLIST *listNode, shared_ptr fieldMapping) { // Get leaf-list default value according to:https://www.rfc-editor.org/rfc/rfc7950.html#section-7.7 +#ifdef LY_ARRAY_COUNT + // libyang3: listNode->dflts is an LY_ARRAY of lyd_value* — convert each to + // its canonical string then hand a null-terminated const char ** to JSon. + if (listNode->dflts == nullptr) + { + return; + } + + // LY_ARRAY_COUNT returns uint64_t; cast to size_t for malloc/indexing so + // the implicit narrowing on 32-bit targets (where size_t is uint32_t) + // doesn't trip -Werror=conversion. + size_t count = (size_t)LY_ARRAY_COUNT(listNode->dflts); + const char **dfltValues = (const char **)malloc((count + 1) * sizeof(*dfltValues)); + for (size_t i = 0; i < count; i++) + { + dfltValues[i] = lyd_value_get_canonical(listNode->module->ctx, listNode->dflts[i]); + } + dfltValues[count] = NULL; + + string dfltValueJson = JSon::buildJson(dfltValues); + free(dfltValues); +#else if (listNode->dflt == nullptr) { return; @@ -216,22 +276,23 @@ void DefaultValueHelper::GetDefaultValueInfoForLeaflist(struct lys_node_leaflist const char** dfltValues = listNode->dflt; //convert list default value to json string string dfltValueJson = JSon::buildJson(dfltValues); +#endif SWSS_LOG_DEBUG("list field: %s, default: %s\n",listNode->name, dfltValueJson.c_str()); fieldMapping->emplace(string(listNode->name), dfltValueJson); } -FieldDefaultValueMappingPtr DefaultValueHelper::GetDefaultValueInfo(struct lys_node* tableChildNode) +FieldDefaultValueMappingPtr DefaultValueHelper::GetDefaultValueInfo(SWSS_LYS_NODE* tableChildNode) { SWSS_LOG_DEBUG("DefaultValueHelper::GetDefaultValueInfo %s\n",tableChildNode->name); - auto field = tableChildNode->child; + auto field = SWSS_LYS_NODE_CHILD(tableChildNode); auto fieldMapping = make_shared(); while (field) { if (field->nodetype == LYS_LEAF) { WARNINGS_NO_CAST_ALIGN - struct lys_node_leaf *leafNode = reinterpret_cast(field); + SWSS_LYS_NODE_LEAF *leafNode = reinterpret_cast(field); WARNINGS_RESET SWSS_LOG_DEBUG("leaf field: %s\n",leafNode->name); @@ -239,7 +300,7 @@ FieldDefaultValueMappingPtr DefaultValueHelper::GetDefaultValueInfo(struct lys_n } else if (field->nodetype == LYS_CHOICE) { - struct lys_node_choice *choiceNode = reinterpret_cast(field); + SWSS_LYS_NODE_CHOICE *choiceNode = reinterpret_cast(field); SWSS_LOG_DEBUG("choice field: %s\n",choiceNode->name); GetDefaultValueInfoForChoice(choiceNode, fieldMapping); @@ -247,7 +308,7 @@ FieldDefaultValueMappingPtr DefaultValueHelper::GetDefaultValueInfo(struct lys_n else if (field->nodetype == LYS_LEAFLIST) { WARNINGS_NO_CAST_ALIGN - struct lys_node_leaflist *listNode = reinterpret_cast(field); + SWSS_LYS_NODE_LEAFLIST *listNode = reinterpret_cast(field); WARNINGS_RESET SWSS_LOG_DEBUG("list field: %s\n",listNode->name); @@ -260,10 +321,10 @@ FieldDefaultValueMappingPtr DefaultValueHelper::GetDefaultValueInfo(struct lys_n return fieldMapping; } -int DefaultValueHelper::BuildTableDefaultValueMapping(struct lys_node* table, TableDefaultValueMapping &tableDefaultValueMapping) +int DefaultValueHelper::BuildTableDefaultValueMapping(SWSS_LYS_NODE* table, TableDefaultValueMapping &tableDefaultValueMapping) { int childListCount = 0; - auto nextChild = table->child; + auto nextChild = SWSS_LYS_NODE_CHILD(table); while (nextChild) { // get key from schema @@ -289,7 +350,7 @@ int DefaultValueHelper::BuildTableDefaultValueMapping(struct lys_node* table, Ta } // Load default value info from yang model and append to default value mapping -void DefaultValueProvider::AppendTableInfoToMapping(struct lys_node* table) +void DefaultValueProvider::AppendTableInfoToMapping(SWSS_LYS_NODE* table) { SWSS_LOG_DEBUG("DefaultValueProvider::AppendTableInfoToMapping table name: %s\n",table->name); TableDefaultValueMapping tableDefaultValueMapping; @@ -401,7 +462,7 @@ DefaultValueProvider::~DefaultValueProvider() if (m_context) { // set private_destructor to NULL because no any private data - ly_ctx_destroy(m_context, NULL); + SWSS_LY_CTX_DESTROY(m_context); } } @@ -418,7 +479,15 @@ void DefaultValueProvider::Initialize(const char* modulePath) ThrowRunTimeError("Open Yang model path " + string(modulePath) + " failed"); } +#ifdef LY_ARRAY_COUNT + if (ly_ctx_new(modulePath, LY_CTX_ALL_IMPLEMENTED, &m_context) != LY_SUCCESS) + { + ThrowRunTimeError("ly_ctx_new() failed"); + } +#else m_context = ly_ctx_new(modulePath, LY_CTX_ALLIMPLEMENTED); +#endif + struct dirent *subDir; while ((subDir = readdir(moduleDir)) != nullptr) { @@ -439,7 +508,7 @@ void DefaultValueProvider::Initialize(const char* modulePath) void DefaultValueProvider::LoadModule(const string &name, const string &path, struct ly_ctx *context) { - const struct lys_module *module = ly_ctx_load_module( + const struct lys_module *module = SWSS_LY_CTX_LOAD_MODULE( context, name.c_str(), EMPTY_STR); // Use EMPTY_STR to revision to load the latest revision @@ -450,14 +519,14 @@ void DefaultValueProvider::LoadModule(const string &name, const string &path, st return; } - if (module->data == nullptr) + if (SWSS_MODULE_DATA(module) == nullptr) { // Not every yang file should contains yang model SWSS_LOG_WARN("Yang file %s does not contains model %s.\n", path.c_str(), name.c_str()); return; } - struct lys_node *topLevelNode = module->data; + SWSS_LYS_NODE *topLevelNode = SWSS_MODULE_DATA(module); while (topLevelNode) { if (topLevelNode->nodetype != LYS_CONTAINER) @@ -469,7 +538,7 @@ void DefaultValueProvider::LoadModule(const string &name, const string &path, st } SWSS_LOG_DEBUG("top level container: %s\n",topLevelNode->name); - auto container = topLevelNode->child; + auto container = SWSS_LYS_NODE_CHILD(topLevelNode); while (container) { SWSS_LOG_DEBUG("container name: %s\n",container->name); diff --git a/common/defaultvalueprovider.h b/common/defaultvalueprovider.h index c3a5b2cf6..9913d7569 100644 --- a/common/defaultvalueprovider.h +++ b/common/defaultvalueprovider.h @@ -7,6 +7,24 @@ #include +/* libyang1 vs libyang3 API shim. LY_ARRAY_COUNT is defined only by libyang3 + * (libyang1 has no equivalent macro), so it is used here as the version + * sentinel. libyang3 uses the compiled-schema tree (lysc_node*) with const + * pointers; libyang1 uses the parsed-schema tree (lys_node*). */ +#ifdef LY_ARRAY_COUNT +# define SWSS_LYS_NODE const struct lysc_node +# define SWSS_LYS_NODE_LIST const struct lysc_node_list +# define SWSS_LYS_NODE_LEAF const struct lysc_node_leaf +# define SWSS_LYS_NODE_CHOICE const struct lysc_node_choice +# define SWSS_LYS_NODE_LEAFLIST const struct lysc_node_leaflist +#else +# define SWSS_LYS_NODE struct lys_node +# define SWSS_LYS_NODE_LIST struct lys_node_list +# define SWSS_LYS_NODE_LEAF struct lys_node_leaf +# define SWSS_LYS_NODE_CHOICE struct lys_node_choice +# define SWSS_LYS_NODE_LEAFLIST struct lys_node_leaflist +#endif + #define DEFAULT_YANG_MODULE_PATH "/usr/local/yang-models" #define EMPTY_STR "" @@ -86,17 +104,17 @@ struct TableInfoMultipleList : public TableInfoBase class DefaultValueHelper { public: - static int BuildTableDefaultValueMapping(struct lys_node* table, TableDefaultValueMapping& tableDefaultValueMapping); + static int BuildTableDefaultValueMapping(SWSS_LYS_NODE* table, TableDefaultValueMapping& tableDefaultValueMapping); - static std::shared_ptr GetKeySchema(struct lys_node* table_child_node); + static std::shared_ptr GetKeySchema(SWSS_LYS_NODE* table_child_node); - static FieldDefaultValueMappingPtr GetDefaultValueInfo(struct lys_node* tableChildNode); + static FieldDefaultValueMappingPtr GetDefaultValueInfo(SWSS_LYS_NODE* tableChildNode); - static void GetDefaultValueInfoForChoice(struct lys_node_choice* choiceNode, std::shared_ptr fieldMapping); + static void GetDefaultValueInfoForChoice(SWSS_LYS_NODE_CHOICE* choiceNode, std::shared_ptr fieldMapping); - static void GetDefaultValueInfoForLeaf(struct lys_node_leaf* leafNode, std::shared_ptr fieldMapping); + static void GetDefaultValueInfoForLeaf(SWSS_LYS_NODE_LEAF* leafNode, std::shared_ptr fieldMapping); - static void GetDefaultValueInfoForLeaflist(struct lys_node_leaflist *listNode, std::shared_ptr fieldMapping); + static void GetDefaultValueInfoForLeaflist(SWSS_LYS_NODE_LEAFLIST *listNode, std::shared_ptr fieldMapping); }; class DefaultValueProvider @@ -125,7 +143,7 @@ class DefaultValueProvider void LoadModule(const std::string &name, const std::string &path, struct ly_ctx *context); // Load default value info from yang model and append to default value mapping - void AppendTableInfoToMapping(struct lys_node* table); + void AppendTableInfoToMapping(SWSS_LYS_NODE* table); std::shared_ptr FindDefaultValueInfo(const std::string &table); diff --git a/common/zmqconsumerstatetable.cpp b/common/zmqconsumerstatetable.cpp index 5900927ef..32d2d2178 100644 --- a/common/zmqconsumerstatetable.cpp +++ b/common/zmqconsumerstatetable.cpp @@ -21,7 +21,8 @@ ZmqConsumerStateTable::ZmqConsumerStateTable(DBConnector *db, const std::string : Selectable(pri) , TableBase(tableName, TableBase::getTableSeparator(db->getDbId())) , m_db(db) - , m_zmqServer(zmqServer) + , m_dbName(db->getDbName()) + , m_handlerRegistry(zmqServer.getHandlerRegistry()) { if (popBatchSize > 0) { @@ -44,11 +45,22 @@ ZmqConsumerStateTable::ZmqConsumerStateTable(DBConnector *db, const std::string m_asyncDBUpdater = nullptr; } - m_zmqServer.registerMessageHandler(m_db->getDbName(), tableName, this); + m_handlerRegistry->registerHandler(m_dbName, tableName, this); SWSS_LOG_DEBUG("ZmqConsumerStateTable ctor tableName: %s", tableName.c_str()); } +ZmqConsumerStateTable::~ZmqConsumerStateTable() +{ + // Detach from the registry before any of our members (notably the + // SelectableEvent, whose eventfd we'd write to from handleReceivedData) + // are destroyed. removeHandler() blocks until any in-flight dispatch + // into us returns. The registry is co-owned with the ZmqServer that + // created us, so this is safe even if that ZmqServer has already been + // destroyed — only the shared registry is touched. + m_handlerRegistry->removeHandler(m_dbName, getTableName()); +} + void ZmqConsumerStateTable::handleReceivedData(const std::vector> &kcos) { for (auto kco : kcos) diff --git a/common/zmqconsumerstatetable.h b/common/zmqconsumerstatetable.h index 4136c617e..c9ab8b5dd 100644 --- a/common/zmqconsumerstatetable.h +++ b/common/zmqconsumerstatetable.h @@ -21,6 +21,8 @@ class ZmqConsumerStateTable : public Selectable, public TableBase, public ZmqMes ZmqConsumerStateTable(DBConnector *db, const std::string &tableName, ZmqServer &zmqServer, int popBatchSize = DEFAULT_POP_BATCH_SIZE, int pri = 0, bool dbPersistence = false); + ~ZmqConsumerStateTable() override; + /* Get multiple pop elements */ void pops(std::deque &vkco, const std::string &prefix = EMPTY_PREFIX); @@ -81,7 +83,13 @@ class ZmqConsumerStateTable : public Selectable, public TableBase, public ZmqMes DBConnector *m_db; - ZmqServer& m_zmqServer; + // Cached at construction time so the destructor can call + // m_handlerRegistry->removeHandler() without dereferencing m_db. + std::string m_dbName; + + // Co-owned with the ZmqServer that created us. Lets the destructor + // unregister cleanly even if the ZmqServer has already been destroyed. + std::shared_ptr m_handlerRegistry; std::unique_ptr m_asyncDBUpdater; diff --git a/common/zmqserver.cpp b/common/zmqserver.cpp index fa57ed3ce..61c57afa3 100644 --- a/common/zmqserver.cpp +++ b/common/zmqserver.cpp @@ -12,6 +12,75 @@ using namespace std; namespace swss { +void ZmqHandlerRegistry::registerHandler( + const std::string& dbName, + const std::string& tableName, + ZmqMessageHandler* handler) +{ + std::lock_guard lock(m_mutex); + + auto dbResult = m_handlers.insert(make_pair(dbName, map())); + if (dbResult.second) { + SWSS_LOG_DEBUG("ZmqHandlerRegistry add mapping for db: %s", dbName.c_str()); + } + + auto tableResult = dbResult.first->second.insert(make_pair(tableName, handler)); + if (tableResult.second) { + SWSS_LOG_DEBUG("ZmqHandlerRegistry register handler for db: %s, table: %s", + dbName.c_str(), tableName.c_str()); + } +} + +void ZmqHandlerRegistry::removeHandler( + const std::string& dbName, + const std::string& tableName) +{ + // Take the same mutex that dispatch() holds across the callback. Once we + // acquire it, no callback into this (dbName, tableName) handler is in + // flight and no further one can start — making it safe for the caller to + // destroy the handler object after removeHandler() returns. + std::lock_guard lock(m_mutex); + + auto dbIter = m_handlers.find(dbName); + if (dbIter == m_handlers.end()) { + return; + } + + dbIter->second.erase(tableName); + if (dbIter->second.empty()) { + m_handlers.erase(dbIter); + } + + SWSS_LOG_DEBUG("ZmqHandlerRegistry removed handler for db: %s, table: %s", + dbName.c_str(), tableName.c_str()); +} + +void ZmqHandlerRegistry::dispatch( + const std::string& dbName, + const std::string& tableName, + const std::vector>& kcos) +{ + // Hold the mutex for the duration of the callback. Concurrent + // removeHandler() on this (dbName, tableName) blocks until we return, + // so the handler cannot be destroyed mid-call. + std::lock_guard lock(m_mutex); + + auto dbIter = m_handlers.find(dbName); + if (dbIter == m_handlers.end()) { + SWSS_LOG_DEBUG("ZmqHandlerRegistry can't find any handler for db: %s", dbName.c_str()); + return; + } + + auto tableIter = dbIter->second.find(tableName); + if (tableIter == dbIter->second.end()) { + SWSS_LOG_DEBUG("ZmqHandlerRegistry can't find handler for db: %s, table: %s", + dbName.c_str(), tableName.c_str()); + return; + } + + tableIter->second->handleReceivedData(kcos); +} + ZmqServer::ZmqServer(const std::string& endpoint) : ZmqServer(endpoint, "", false, false) { @@ -34,7 +103,8 @@ ZmqServer::ZmqServer(const std::string& endpoint, const std::string& vrf, bool l m_context(nullptr), m_socket(nullptr), m_oneToOneSync(oneToOneSync), - m_allowZmqPoll(true) + m_allowZmqPoll(true), + m_registry(std::make_shared()) { if (!lazyBind) { @@ -62,6 +132,11 @@ ZmqServer::~ZmqServer() { zmq_ctx_destroy(m_context); } + + // m_registry's refcount drops here. If any registered handler still holds + // a reference, the registry survives until that handler is destroyed; its + // destructor will call removeHandler() safely against the surviving + // registry without touching this (now-gone) ZmqServer. } void ZmqServer::bind() @@ -108,7 +183,7 @@ void ZmqServer::bind() } if (!m_vrf.empty()) - { + { zmq_setsockopt(m_socket, ZMQ_BINDTODEVICE, m_vrf.c_str(), m_vrf.length()); } @@ -130,34 +205,24 @@ void ZmqServer::registerMessageHandler( const std::string tableName, ZmqMessageHandler* handler) { - auto dbResult = m_HandlerMap.insert(pair>(dbName, map())); - if (dbResult.second) { - SWSS_LOG_DEBUG("ZmqServer add handler mapping for db: %s", dbName.c_str()); - } + m_registry->registerHandler(dbName, tableName, handler); +} - auto tableResult = dbResult.first->second.insert(pair(tableName, handler)); - if (tableResult.second) { - SWSS_LOG_DEBUG("ZmqServer register handler for db: %s, table: %s", dbName.c_str(), tableName.c_str()); - } +void ZmqServer::removeMessageHandler( + const std::string& dbName, + const std::string& tableName) +{ + m_registry->removeHandler(dbName, tableName); } ZmqMessageHandler* ZmqServer::findMessageHandler( - const std::string dbName, - const std::string tableName) + const std::string /*dbName*/, + const std::string /*tableName*/) { - auto dbMappingIter = m_HandlerMap.find(dbName); - if (dbMappingIter == m_HandlerMap.end()) { - SWSS_LOG_DEBUG("ZmqServer can't find any handler for db: %s", dbName.c_str()); - return nullptr; - } - - auto tableMappingIter = dbMappingIter->second.find(tableName); - if (tableMappingIter == dbMappingIter->second.end()) { - SWSS_LOG_DEBUG("ZmqServer can't find handler for db: %s, table: %s", dbName.c_str(), tableName.c_str()); - return nullptr; - } - - return tableMappingIter->second; + // Retained as a no-op for source compatibility with external test mocks + // that override this symbol at link time. Real dispatch lives in + // ZmqHandlerRegistry::dispatch(). + return nullptr; } void ZmqServer::handleReceivedData(const char* buffer, const size_t size) @@ -167,14 +232,7 @@ void ZmqServer::handleReceivedData(const char* buffer, const size_t size) std::vector> kcos; BinarySerializer::deserializeBuffer(buffer, size, dbName, tableName, kcos); - // find handler - auto handler = findMessageHandler(dbName, tableName); - if (handler == nullptr) { - SWSS_LOG_WARN("ZmqServer can't find handler for received message: %s", buffer); - return; - } - - handler->handleReceivedData(kcos); + m_registry->dispatch(dbName, tableName, kcos); } void ZmqServer::startMqPollThread() diff --git a/common/zmqserver.h b/common/zmqserver.h index ead5463b0..a0f7090bf 100644 --- a/common/zmqserver.h +++ b/common/zmqserver.h @@ -1,5 +1,8 @@ #pragma once +#include +#include +#include #include #include #include @@ -24,6 +27,32 @@ class ZmqMessageHandler virtual void handleReceivedData(const std::vector>& kcos) = 0; }; +// Shared (db, table) -> handler map. Co-owned by ZmqServer and every +// registered handler so neither party requires the other to outlive it. +// The mutex is held across handler dispatch, so removeHandler() blocks +// until any in-flight callback into the handler being removed has returned +// — making it safe for the caller to destroy the handler immediately after +// removeHandler() returns. Handler callbacks therefore must not themselves +// call registerHandler / removeHandler (would self-deadlock). +class ZmqHandlerRegistry +{ +public: + void registerHandler(const std::string& dbName, + const std::string& tableName, + ZmqMessageHandler* handler); + + void removeHandler(const std::string& dbName, + const std::string& tableName); + + void dispatch(const std::string& dbName, + const std::string& tableName, + const std::vector>& kcos); + +private: + std::mutex m_mutex; + std::map> m_handlers; +}; + class ZmqServer { public: @@ -44,19 +73,37 @@ class ZmqServer const std::string tableName, ZmqMessageHandler* handler); + // Remove a previously-registered handler. Blocks until any in-flight + // dispatch into that handler has returned, so the caller can safely + // destroy the handler object after this call returns. + void removeMessageHandler( + const std::string& dbName, + const std::string& tableName); + // This method should only be used in one-to-one sync mode with the client. void sendMsg(const std::string& dbName, const std::string& tableName, const std::vector& values); void bind(); + // Internal: returns the shared handler registry so a handler implementation + // can co-own it. This lets the handler's destructor call removeHandler() + // without depending on the ZmqServer still being alive (the registry + // survives as long as either party holds a reference). Intended for use + // by ZmqMessageHandler subclasses, not by general callers. + std::shared_ptr getHandlerRegistry() const { return m_registry; } + private: void handleReceivedData(const char* buffer, const size_t size); void startMqPollThread(); void mqPollThread(); - + + // Retained as a no-op stub for source compatibility with external test + // mocks (e.g. sonic-swss's fake_zmqserver.cpp) that override this symbol + // at link time. The internal dispatch path no longer calls it — handler + // lookup happens inside ZmqHandlerRegistry::dispatch(). ZmqMessageHandler* findMessageHandler(const std::string dbName, const std::string tableName); std::vector m_buffer; @@ -77,7 +124,11 @@ class ZmqServer bool m_allowZmqPoll; - std::map> m_HandlerMap; + // Default-initialized in-class so that link-time mocks of ZmqServer + // (which may not initialize this member in their stub constructors) still + // present a valid registry to any real ZmqConsumerStateTable they pair + // with — the real constructor below also sets this explicitly. + std::shared_ptr m_registry = std::make_shared(); }; } diff --git a/pyext/swsscommon.i b/pyext/swsscommon.i index 7dc280952..3fbc07128 100644 --- a/pyext/swsscommon.i +++ b/pyext/swsscommon.i @@ -271,6 +271,13 @@ T castSelectableObj(swss::Selectable *temp) %include "dbconnector.h" #ifdef ENABLE_YANG_MODULES %include "cfg_schema.h" +// DefaultValueHelper exposes libyang schema-node pointers (lys_node* / +// lysc_node*) whose type names differ between libyang1 and libyang3. +// SWIG's preprocessor cannot see , so it always takes +// the libyang1 branch of the version shim — generating bindings that fail +// to compile against libyang3 headers. The helper is internal; no Python +// caller uses it. Skip it so the generated wrappers stay version-agnostic. +%ignore swss::DefaultValueHelper; %include "defaultvalueprovider.h" #endif %include "sonicv2connector.h"