Skip to content
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

Preprocess vtysh #16501

Merged
merged 14 commits into from
Aug 2, 2024
Merged

Preprocess vtysh #16501

merged 14 commits into from
Aug 2, 2024

Conversation

donaldsharp
Copy link
Member

See individual commits and the #16427 PR. I want to get this in since David is out sick so I will try to shepherd this through the last little bit needed.

eqvinox added 14 commits July 31, 2024 08:08
Not used anywhere in FRR, kill it.

Signed-off-by: David Lamparter <[email protected]>
Use alloced=0 to indicate that the array used in a vector is not in fact
dynamically allocated memory (yet).

Signed-off-by: David Lamparter <[email protected]>
Allow using "+1" when meaningful (i.e. cmd_graph_merge wants -1 or +1)

Signed-off-by: David Lamparter <[email protected]>
The number of nodes in a graph will change as soon as cmd_graph_merge is
supported as an operation, therefore size this dynamically.

Signed-off-by: David Lamparter <[email protected]>
When merging graphs, it makes sense to allow starting with an empty one.

Signed-off-by: David Lamparter <[email protected]>
Export cmd_graph_merge() to python code via graph1.merge(graph2).

Signed-off-by: David Lamparter <[email protected]>
Make it a little easier to work on python code using this wrapper.

Signed-off-by: David Lamparter <[email protected]>
Add len(graph) and graph[i] wrappers to access arbitrary nodes in a
graph.

Signed-off-by: David Lamparter <[email protected]>
There's a wrapper for nodes' outgoing pointers, but not incoming yet.

Signed-off-by: David Lamparter <[email protected]>
FORK_TKN's join node is already exposed, mirror to expose JOIN_TKN's
fork node.

(contains minor cleanup to make checkpatch.pl shut up)

Signed-off-by: David Lamparter <[email protected]>
Expose all of the struct members of cmd_token, and retrieve them
dynamically rather than copying them around.  The problem with copying
them is that they can change as a result of merge(), and if there is an
existing wrapper object around it will not have its copy updated to
match.

Signed-off-by: David Lamparter <[email protected]>
The command graph has its tail end nodes pointing at the
`struct cmd_element` rather than a `struct cmd_token`.  This is a bit
weird to begin with, but becomes very annoying for the python bindings
where there is just no `struct cmd_element`.

Create a `CMD_ELEMENT_TKN` type for `cmd_token` instead, and replace the
tail end token in the python bindings with an instance of that.

Signed-off-by: David Lamparter <[email protected]>
There is entirely no point to these being conditional.  And pull them up
so the upcoming pre-parse code can work on a clean slate.

Signed-off-by: David Lamparter <[email protected]>
Store a parsed and built graph of the CLI nodes in vtysh, rather than
parsing and building that graph every time vtysh starts up.

This provides a 3x to 5x reduction in vtysh startup overhead:

`vtysh -c 'configure' -c 'interface lo' -c 'do show version'`

- before: 92.9M cycles, 1114 samples
- after: 16.5M cycles, 330 samples

This improvement is particularly visible for users scripting `vtysh -c`
calls, which notably includes topotests.

Signed-off-by: David Lamparter <[email protected]>
@mjstapp
Copy link
Contributor

mjstapp commented Aug 1, 2024

Looked like there are some style reports - can those be fixed?

@mjstapp
Copy link
Contributor

mjstapp commented Aug 2, 2024

Looks like the style warnings were spurious...

@mjstapp mjstapp merged commit 975e1a3 into FRRouting:master Aug 2, 2024
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants