Skip to content

sonic-swss-common: port to libyang3 #973

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
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
82 changes: 50 additions & 32 deletions common/defaultvalueprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -127,7 +127,7 @@ bool TableInfoMultipleList::FoundFieldMappingByKey(const string &key, FieldDefau
return keySchema != m_defaultValueMapping.end();
}

shared_ptr<KeySchema> DefaultValueHelper::GetKeySchema(struct lys_node* tableChildNode)
shared_ptr<KeySchema> DefaultValueHelper::GetKeySchema(const struct lysc_node* tableChildNode)
{
SWSS_LOG_DEBUG("DefaultValueHelper::GetKeySchema %s\n",tableChildNode->name);

Expand All @@ -138,15 +138,20 @@ shared_ptr<KeySchema> 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;
if (listNode->keys_str == nullptr)
const struct lysc_node_list *listNode = (const struct lysc_node_list*)tableChildNode;

for (const struct lysc_node *node = listNode->child; node != NULL; node = node->next)
{
SWSS_LOG_ERROR("Ignore empty key string on list: %s\n",tableChildNode->name);
return nullptr;
if (!lysc_is_key(node)) {
continue;
}
if (keyValue.length())
{
keyValue += " ";
}
keyValue += node->name;
keyFieldCount++;
}

string key(listNode->keys_str);
keyFieldCount = (int)count(key.begin(), key.end(), ' ') + 1;
}
else if (tableChildNode->nodetype == LYS_CONTAINER)
{
Expand All @@ -164,16 +169,16 @@ shared_ptr<KeySchema> DefaultValueHelper::GetKeySchema(struct lys_node* tableChi
return make_shared<KeySchema>(keyValue, keyFieldCount);
}

void DefaultValueHelper::GetDefaultValueInfoForLeaf(struct lys_node_leaf* leafNode, shared_ptr<FieldDefaultValueMapping> fieldMapping)
void DefaultValueHelper::GetDefaultValueInfoForLeaf(const struct lysc_node_leaf* leafNode, shared_ptr<FieldDefaultValueMapping> 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, lyd_value_get_canonical(leafNode->module->ctx, leafNode->dflt));
fieldMapping->emplace(string(leafNode->name), string(lyd_value_get_canonical(leafNode->module->ctx, leafNode->dflt)));
}
}

void DefaultValueHelper::GetDefaultValueInfoForChoice(struct lys_node_choice* choiceNode, shared_ptr<FieldDefaultValueMapping> fieldMapping)
void DefaultValueHelper::GetDefaultValueInfoForChoice(const struct lysc_node_choice* choiceNode, shared_ptr<FieldDefaultValueMapping> fieldMapping)
{
if (choiceNode->dflt == nullptr)
{
Expand All @@ -193,61 +198,69 @@ void DefaultValueHelper::GetDefaultValueInfoForChoice(struct lys_node_choice* ch

SWSS_LOG_DEBUG("default choice leaf field: %s\n",fieldInChoice->name);
WARNINGS_NO_CAST_ALIGN
struct lys_node_leaf *dfltLeafNode = reinterpret_cast<struct lys_node_leaf*>(fieldInChoice);
struct lysc_node_leaf *dfltLeafNode = reinterpret_cast<struct lysc_node_leaf*>(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, lyd_value_get_canonical(dfltLeafNode->module->ctx, dfltLeafNode->dflt));
fieldMapping->emplace(string(fieldInChoice->name), string(lyd_value_get_canonical(dfltLeafNode->module->ctx, dfltLeafNode->dflt)));
}

fieldInChoice = fieldInChoice->next;
}
}

void DefaultValueHelper::GetDefaultValueInfoForLeaflist(struct lys_node_leaflist *listNode, shared_ptr<FieldDefaultValueMapping> fieldMapping)
void DefaultValueHelper::GetDefaultValueInfoForLeaflist(const struct lysc_node_leaflist *listNode, shared_ptr<FieldDefaultValueMapping> fieldMapping)
{
size_t i;
// Get leaf-list default value according to:https://www.rfc-editor.org/rfc/rfc7950.html#section-7.7
if (listNode->dflt == nullptr)
if (listNode->dflts == nullptr)
{
return;
}

const char** dfltValues = listNode->dflt;
/* Convert to null-terminated array of const char * */
const char **dfltValues = (const char **)malloc((LY_ARRAY_COUNT(listNode->dflts)+1) * sizeof(*dfltValues));
for (i=0; i<LY_ARRAY_COUNT(listNode->dflts); i++) {
dfltValues[i] = lyd_value_get_canonical(listNode->module->ctx, listNode->dflts[i]);
}
dfltValues[LY_ARRAY_COUNT(listNode->dflts)] = NULL;

//convert list default value to json string
string dfltValueJson = JSon::buildJson(dfltValues);
free(dfltValues);
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(const struct lysc_node* tableChildNode)
{
SWSS_LOG_DEBUG("DefaultValueHelper::GetDefaultValueInfo %s\n",tableChildNode->name);

auto field = tableChildNode->child;
auto field = lysc_node_child(tableChildNode);
auto fieldMapping = make_shared<FieldDefaultValueMapping>();
while (field)
{
if (field->nodetype == LYS_LEAF)
{
WARNINGS_NO_CAST_ALIGN
struct lys_node_leaf *leafNode = reinterpret_cast<struct lys_node_leaf*>(field);
const struct lysc_node_leaf *leafNode = reinterpret_cast<const struct lysc_node_leaf*>(field);
WARNINGS_RESET

SWSS_LOG_DEBUG("leaf field: %s\n",leafNode->name);
GetDefaultValueInfoForLeaf(leafNode, fieldMapping);
}
else if (field->nodetype == LYS_CHOICE)
{
struct lys_node_choice *choiceNode = reinterpret_cast<struct lys_node_choice*>(field);
const struct lysc_node_choice *choiceNode = reinterpret_cast<const struct lysc_node_choice*>(field);

SWSS_LOG_DEBUG("choice field: %s\n",choiceNode->name);
GetDefaultValueInfoForChoice(choiceNode, fieldMapping);
}
else if (field->nodetype == LYS_LEAFLIST)
{
WARNINGS_NO_CAST_ALIGN
struct lys_node_leaflist *listNode = reinterpret_cast<struct lys_node_leaflist*>(field);
const struct lysc_node_leaflist *listNode = reinterpret_cast<const struct lysc_node_leaflist*>(field);
WARNINGS_RESET

SWSS_LOG_DEBUG("list field: %s\n",listNode->name);
Expand All @@ -260,10 +273,10 @@ FieldDefaultValueMappingPtr DefaultValueHelper::GetDefaultValueInfo(struct lys_n
return fieldMapping;
}

int DefaultValueHelper::BuildTableDefaultValueMapping(struct lys_node* table, TableDefaultValueMapping &tableDefaultValueMapping)
int DefaultValueHelper::BuildTableDefaultValueMapping(const struct lysc_node* table, TableDefaultValueMapping &tableDefaultValueMapping)
{
int childListCount = 0;
auto nextChild = table->child;
auto nextChild = lysc_node_child(table);
while (nextChild)
{
// get key from schema
Expand All @@ -289,7 +302,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(const struct lysc_node* table)
{
SWSS_LOG_DEBUG("DefaultValueProvider::AppendTableInfoToMapping table name: %s\n",table->name);
TableDefaultValueMapping tableDefaultValueMapping;
Expand Down Expand Up @@ -401,7 +414,7 @@ DefaultValueProvider::~DefaultValueProvider()
if (m_context)
{
// set private_destructor to NULL because no any private data
ly_ctx_destroy(m_context, NULL);
ly_ctx_destroy(m_context);
}
}

Expand All @@ -418,7 +431,11 @@ void DefaultValueProvider::Initialize(const char* modulePath)
ThrowRunTimeError("Open Yang model path " + string(modulePath) + " failed");
}

m_context = ly_ctx_new(modulePath, LY_CTX_ALLIMPLEMENTED);
if (ly_ctx_new(modulePath, LY_CTX_ALL_IMPLEMENTED, &m_context) != LY_SUCCESS)
{
ThrowRunTimeError("ly_ctx_new() failed");
}

struct dirent *subDir;
while ((subDir = readdir(moduleDir)) != nullptr)
{
Expand All @@ -442,22 +459,23 @@ void DefaultValueProvider::LoadModule(const string &name, const string &path, st
const struct lys_module *module = ly_ctx_load_module(
context,
name.c_str(),
EMPTY_STR); // Use EMPTY_STR to revision to load the latest revision
EMPTY_STR, // Use EMPTY_STR to revision to load the latest revision
NULL);
if (module == nullptr)
{
const char* err = ly_errmsg(context);
SWSS_LOG_ERROR("Load Yang file %s failed: %s.\n", path.c_str(), err);
return;
}

if (module->data == nullptr)
if (module->compiled == nullptr || module->compiled->data == 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;
struct lysc_node *topLevelNode = module->compiled->data;
while (topLevelNode)
{
if (topLevelNode->nodetype != LYS_CONTAINER)
Expand All @@ -469,7 +487,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 = lysc_node_child(topLevelNode);
while (container)
{
SWSS_LOG_DEBUG("container name: %s\n",container->name);
Expand Down
14 changes: 7 additions & 7 deletions common/defaultvalueprovider.h
Original file line number Diff line number Diff line change
Expand Up @@ -86,17 +86,17 @@ struct TableInfoMultipleList : public TableInfoBase
class DefaultValueHelper
{
public:
static int BuildTableDefaultValueMapping(struct lys_node* table, TableDefaultValueMapping& tableDefaultValueMapping);
static int BuildTableDefaultValueMapping(const struct lysc_node* table, TableDefaultValueMapping& tableDefaultValueMapping);
Copy link
Contributor

Choose a reason for hiding this comment

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

is it possible to use typedef to avoid these changes?

Copy link
Author

@bradh352 bradh352 Feb 17, 2025

Choose a reason for hiding this comment

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

Its not clear to me what you are asking. Libyang1 and libyang3 are not API compatible, lys_node no longer exists. There were significant changes between libyang1 and libyang2, then again between libyang2 and libyang3. SONiC is way behind the curve here.

Are you asking for me to do something to try to make this compile against both libyang1 and libyang3 rather than just a straight port to a new version? If so, what would be the advantage and why would it be worth the effort? There are over a dozen linked PRs to upgrading to libyang3 at this point and all will need to be merged en masse

Copy link
Contributor

Choose a reason for hiding this comment

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

I mean if it's possible to use typedef/macro to encapsulate the data type and function in swss-common, which may avoid such scattered modification, and not sure this gonna be changed again if later upgrade to another version. Thanks for your effort for these PRs.

Copy link
Author

Choose a reason for hiding this comment

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

Being on the outside from libyang development, I can't really predict what changes they may make in the future that are not backwards compatible. But I do know that libyang1 -> libyang2 was supposedly a complete rewrite to differentiate uncompiled vs compiled schema trees hence lys_ was split into lysp_ and lysc_ data types. libyang2 -> libyang3 was a less substantial change. I don't think trying to abstract the changes here would help, it would probably make it harder down the road.

They do maintain transition manuals here:
https://netopeer.liberouter.org/doc/libyang/master/html/transition1_2.html
https://netopeer.liberouter.org/doc/libyang/master/html/transition2_3.html

It "feels" like their API is stabilizing, its just SONiC started with a really early version so there are lots of changes needed.


static std::shared_ptr<KeySchema> GetKeySchema(struct lys_node* table_child_node);
static std::shared_ptr<KeySchema> GetKeySchema(const struct lysc_node* table_child_node);

static FieldDefaultValueMappingPtr GetDefaultValueInfo(struct lys_node* tableChildNode);
static FieldDefaultValueMappingPtr GetDefaultValueInfo(const struct lysc_node* tableChildNode);

static void GetDefaultValueInfoForChoice(struct lys_node_choice* choiceNode, std::shared_ptr<FieldDefaultValueMapping> fieldMapping);
static void GetDefaultValueInfoForChoice(const struct lysc_node_choice* choiceNode, std::shared_ptr<FieldDefaultValueMapping> fieldMapping);

static void GetDefaultValueInfoForLeaf(struct lys_node_leaf* leafNode, std::shared_ptr<FieldDefaultValueMapping> fieldMapping);
static void GetDefaultValueInfoForLeaf(const struct lysc_node_leaf* leafNode, std::shared_ptr<FieldDefaultValueMapping> fieldMapping);

static void GetDefaultValueInfoForLeaflist(struct lys_node_leaflist *listNode, std::shared_ptr<FieldDefaultValueMapping> fieldMapping);
static void GetDefaultValueInfoForLeaflist(const struct lysc_node_leaflist *listNode, std::shared_ptr<FieldDefaultValueMapping> fieldMapping);
};

class DefaultValueProvider
Expand Down Expand Up @@ -125,7 +125,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(const struct lysc_node* table);

std::shared_ptr<TableInfoBase> FindDefaultValueInfo(const std::string &table);

Expand Down
Loading