Regarding (1) which is about changing the default. No issues, I was trying to break any code until someone had more time than me to test it, as the devil is always in the details.

From John on github:

Yes the GCC preprocessors is limited (but works for the current use case) and m4 is indeed more powerful but not a user-friendly tool IMHO.

I will have a look as this was not supported by vyatta and therefore not added to the code when converted to python

Coming with a syntax which is not ultimately going to be as complex as the cli may be an impossible challenge. Changing the API to include in the XML what is path vs payload may indeed lead to indeed a better API tho. The example given use the word create in the path when REST would use POST.

I would have expected the output generated to be an OR of the validators or regexes and allow the output if any would have passed it

Thank you for writing some testing code using the smoketest repository. It may take a few working days for anyone to come back to you.

Thank you for the feedback, even if it was not what I was hoping for !

I would not use cpu<numbe> but just <number> like is used for taskset for example, but it would be a real improvement for users.
An op command should be provided giving the affinity of CPU ( from lscpu ) and the documentation should warn about interrupt crossing numa domains (for the case where a network card is providing multiple interfaces).

And there are some PR pending on that exact code ..

If anything this code could/should be extracted in an internal library and then a tool created to replace what we have so the behaviour is consistent in both cases.

There is now some python XML code to parse the XML. m4 is not a nice tool. If better pre-processing is required, which I would not argue for, please explain the issue you are trying to solve.

related to T1699

It may be a good idea to also have an option to hook debug logging to syslog.

Yes, it would solve the issue ... but ... currently, we re-apply the whole interface setting, so there is no change to have the vyos and live configuration not sync'ed.
This would be lost. It is a trade-off, but it could be done. It would be however the only sub-system working that way.

Ideally which interface is master/slave should be recorded and handled by VyOS so that users do not have to put some workaround like this one to know.

I do not have a lab to reproduce this ATM.

Sorry on behalf of the team. The process of PR review is not ideal. I appreciate that you have asked for merge, and would be probably expecting some feedback As it was not, and that you got neither yet. I know it can be a bit frustrating .. I also have PRs waiting! 🥱
Hopefully there will be more regular review going forward, in the meanwhile please be assured that your contribution is appreciated and again sorry for the wait.

@efficiosoft understood. We tend to all hangout there to help each other. If you have another way I/we can relate to you to support you (but here) let us know.

@efficiosoft are you on our slack channel ?

Well, it would be trivial to add an --auto-reload switch to the daemon for development purposes which could enable an inotify watcher over the XML files.

@efficiosoft asking users to fire a daemon is not really user-friendly :-) Once a daemon is loaded, whatever change is done to any initialisation data on the disk will not affect it. Should a tool exist loading the same data each time, then changes can be tested without affecting the running daemon.

Thank you for the offer, having read your patch it seems you are indeed fluent in Python and as there is lots to do so I am sure you will be able to help :-)
As @dmbaturin started this, I will let him come back to you about what could be done.

@SrividyaA the task is assigned to you, if you are going to fix it, all good, otherwise happy to do so.

https://lwn.net/Articles/822744/

As I click "sail", I just realise that as there is only one config per router, ConfigDict could be a subclass of Config which could be itself singleton. So even if it is subclassed, all instances could/would share the same underlying data and could be used inter-exchangeably for the content of the config.

@jestabro I would like to hear why you advocate this API and why you believe it is better than the one I have suggested.

Regarding the API proposed, most of these functions are also syntactic sugar for the same operations. Looking at the use cases, there is two: getting information about a leaf value, or getting information about tagNode changes.

Ok, I will look at how we can use the current configuration to insert the tagNode name when we generate the default configuration

Re-reading the entry, I am now unsure what you believe should be different.

Yes, I should parse the tagNode and insert them into the default data.

Thank you for this explaining what is happening. I would indeed rather tag were used as keys.

Yes, so there is no way to know at parsing time (unless you have two elements or check the XML) that this is a multi-element. Something "computer-friendly" would generate:

test {
    something [one]
}

The data returned by get_config_dict should always have the same format to prevent any function using it to have to check if one element is a list or a string.

Agreeing and defining the name of the keys to be used in the dict passed to verify/generate/apply, would be very beneficial for the project.
The current code does this by calling a number of helper function (so that all interface have the same keys) but this is not defined or/and enforced.

Should we provide the script which perform the change and define what action can be performed in the xml?

The work to perform the get_config/verify/generate/apply in python is already available in this tree:
https://github.com/thomas-mangin/vyos-1x/tree/T2522

@cpo, you can change the defaults from True to False in the patch if this is what you want to do.

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

I propose to provide alternatives is_ functions using the new XML code. I will provide a patch for review.

I agree that this kind of changes are better done at the start of a development cycle.

There is really no difference between calling Config() and using it functions in the code - as is done by every module - vs having the class inherit from it as I do not use any private ( _ prefixed ) functions. I can modify the code to have self.config as an object of the class instead but IMO this is cosmetic and does not change anything with the API and coding guideline.

not willing to take the lead on this task but happy to help.

@jestabro could you please clarify how interfaces-tunnel.py is not following the guideline. The class it uses to generate the dict is internal to the get_config() function and the dict API is respected.

The problem also probably exists with vif_s .. it needs to be investigated.

https://github.com/vyos/vyos-1x/pull/474/files

was done part of T2633

could have the range 68-65536 but it may be a bit on the extreme side.

https://github.com/vyos/vyos-1x/pull/473 was merged so now need to agree sane limits for the XML.

I have a PR for this (not changing the XML limiting range) for review ATM.

related to T2630

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

Breaking user existing configs should be a no-no. If the options can be used that way under Linux, then we should not restrict it if it is not invalid. If we intend to prevent it then we would need a way to warn users clearly and we have no framework for this ATM.

Need to add max MTU to operational mode and create a new validator using it and applying it to the xml. The only question being if the information is always available.

I see no issue with the proposed solution.

LOL - I could have commited :-) Thanks !

@Dmitry correct same bug - thank you. resolved.

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

The patch was merged and the issue should be resolved with the next ISO.

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

Thank you very much for the POC. Very useful to understand the proposed design.

hello Fabio, could you please show me how the vti interfaces are presented under Linux so I can fix the code. I thought I had properly ported the code from Perl to Python must I must have misunderstood something.

Yes, we need to try/except the apply section (the other should never fail but we could still catch errors to not leave the system in an unknown state) but when applying the reverse configuration (ie: invert effective and new and re-apply) one must then be careful if that fails too (we do not want a forever loop :p). The code already runs all the get_dict and all the verify first, so we will only apply if all is ok, but still issues could occur.

@jjakob control-R should be implemented. It is a feature I use too and expect it - just did not think it was worth a demo 0.0 release - there is plenty to improve with the code in the branch - it surely has bugs -as I said POC :-)

generate should make a backup of the previous file before generating the new one. It will then make it possible to create the rollback as a file move and service reload.

@jjakob the code can be installed on a router (using my vyos update tool - after running vyos setup router )

It should not be too hard to convert the current parser to read.
https://gist.github.com/thomas-mangin/17a450a3e26a4cc41902475c0a1dfe5f

@jjakob you are right, there is no shell integration and this is using the python promt-toolkit library to handle input/output.

Thank you for reporting this issue, it looks like that parser allows ranges of IP address (IP hyphen IP) but the parser does not. You could get around using CIDR notation but this indeed need looking into.

should help to go further in the testing, but it is still failing on set_state but I do not know why it should be done if the interface is managed by openvpn. The relevant code is:

As a side note, if there is a need to migrate between backends, it would be possible to change the code to have an option to indicate which storage should be used and allow loading from one, saving to another as long as the XML schema is the same.

I see the POC as complete and conclusive, it would only make sense to spend more time on things once this is done (or to add support for control via HTTP/API).

Configuring 100 dummy interfaces (no thread) on a local VirtualBox, all tests done from boot

Thank you for this long answer @jjakob. I want to demonstrate that a full python solution can provide the performance we need. I appreciate that changing the Vyatta code need to be done carefully with many consideration about backward compatibility. What I am doing is surely 1.4 material. However I do not believe this is as hard to achieve as everyone may think, and as working code is the best way to discuss code design, that is what I am doing.

The cli is mostly functional. I am able to validate the data as it is typed in conf mode (the CLI has both completion and validation working), as soon as delete and show (the current show is "show configuration commands") are implemented, it will be mostly usable. The code can already load a configuration from file, allow some "set" edit and then allow the use of save (the config format is a number of "set" commands, one per line), respect the initialisation order of the XML.

I used @runar code on my branch working on T2522 .. Configuring 100 dummy interface went from 42 to 8 seconds.

I just had a look at VyConf and it is excellent, I fear that no one but @dmbaturin can maintain participate to it.
Also, VyConf will still need to fork all the python code and unless we have a resolution to T2088 - I am not sure what the best path forward will be

I tried to get a flamegraph showing what I was wanting to say but .. do not look very clear :-(

I believe it was git pull —tags which fixed it for me ..

Many scripts also so do implement if name == “main”

Also related to T2088