@querubin thanks for the info; that requirement should not persist, as current work should lessen the overhead. I'll link the task back here when defined.

@querubin Thank you for the detailed results --- firstly, these issues may be overdetermined due to several updates earlier this month; one notable issue is that we had moved to a 5.x series kernel, which showed several problems re QAT support, and an identified kernel bug. We have reverted to 4.19 as of yesterday until the next LTS kernel is available. I would suggest trying the most recent rolling, and then we will diagnose any persistent issues.

This is resolved by T2332; the normalized form is:

Taking a look ...

@querubin please try booting with the vyos-configd service masked: add the kernel boot parameter:

Resolved in PR536.

@thomas-mangin in further tests, I've seen wide variability in timing tests, independent of caching, with the original quote being the high-end. That will need to be investigated, but I think performance should not be considered a road-block for now.

@tux, this has been fixed in the current rolling.

FRR 7.4 has been released, and the default behaviour has been changed, commit 62282e8379. @Viacheslav, when we update to this version, I can work with you to update the migration script.

As discussed in above comment, this is understandable behaviour, but will be re-investigated after the move to fastapi, re T2397.

Addressed in T2568.

This was an early experiment which contributed some ideas towards T2582; closed as superseded by that task.

Discussion updated in PR 513.
https://github.com/vyos/vyos-1x/pull/513

What is here:

This fix remains an open discussion with @zsdc and @Dmitry

This does not currently affect crux (perhaps it did at some point). The crux script logic precludes this bug; confirmed in testing.

https://github.com/vyos/vyos-1x/pull/499

Final tests before PR.

Current version linked below, as an example for further discussion. As these ideas have been circulating for a while, it is important to have examples to debunk/revise/reject. My guess is that one likely wants much less than this, and that @dmbaturin is quite right in his recent comment "we should not rely on diffs unless there's absolutely no other way to do it --- not just about computational expense, but also readability".

@thomas-mangin I'm convinced: for get_child_nodes and get_node, we'll return a dict of respective dicts:

Okay, I like that video already ...

@thomas-mangin Firstly, I needed to write this version before reasonably debating the pros/cons of various approaches --- the important idea developed is that the use of get_sub_dict, and ability to return the sub_dict under the diff'ed nodes will allow a fluid use in writing conf_mode scripts --- beyond that, I am not particularly attached to any details of implementation, yet.

@thomas-mangin and @runar I do like the enum idea, however, this would add boilerplate to the conf_mode scripts, which would quickly become annoying in practice ... rather, common workflow just wants the actionable data (added, deleted) _and_ the ability to then access values under the node in question. Consequently, I'm following @runar 's suggestion of returning a tuple, combined with the return_as_dict arg to allow access to sub-data.

One forgotten point: get_config_diff obtains the config_dicts at root level, so any movement within/between sub-sections are available, with set_level.

It is true that get_config_dict is slow, so it should only be called once (twice for diff) per session. Consequently, the work flow will be:

So, as far as useful helper functions, one certainly wants:
get_child_nodes_changed(... path)
get_value_changed(... path)

@runar in fact, that's all one wants in current use case: has the list of elements, directly _under_ the specified node, changed? For example, (1) change of values (2) added or removed tag node entries.

Yes, I'm expanding all paths under the specified path

Yes, I definitely prefer a return type of tuple ...

Regarding is_value_changed, I was thinking the other way around: get_value_changed returns None if no change, so is_value_changed would be redundant --- put good point: one may/will want both old and new values

I am entirely open to suggestions here; the underlying functions support any such forms. Note however, that we want to distinguish between new/deleted paths and changed values --- one could treat these all as a difference in path, but it will be more convenient for use if we make the distinction ...

Addressed by T2667.