Issue: When the incoming config has say 30K entries of a prefix-lists,
current implementation is to schedule the configs to be batched and
only after batching the entire config, the processing of the configs
take place. As part of batching this config, we perform string
concatenation to save all the configs in the buffer which over time
results in taking longer time.
Ex: Imagine each line of config is 50 chars. With a delimiter of ‘- ‘ we end
up adding 52 chars to buffer for each command i.e. 52*30000 = 156K of chars.
Strlcat is an expensive operation and every time we strlcat, we have to
traverse at end of string to append new char.
Because of this, we end up adding extra 6-8 secs for accepting the config.
Fix: The idea here is to bring back something similar to the backoff
count implemented as part of 20e9a402 (lib: introduce configuration
back-off timer for YANG-modeled commands).
Essentially we keep a cap of 5000 per batch. So once 5000k config
commands are batched, we process them, clear the buffer, set the count
to 0 and then continue processing the rest of the config.
option1 file has 30K entries of prefix-list
Without Fix:
root@mlx-3700-20:mgmt:/var/log/raja/frr# time sudo vtysh -f option1
<SNIP>..............
Waiting for children to finish applying config...
[25191|staticd] done
[25189|watchfrr] done
[25178|ospfd] done
[25190|pbrd] done
[25181|bgpd] done
[25175|zebra] done
real 0m20.123s
user 0m9.384s
sys 0m2.403s
With Fix:
root@mlx-3700-20:mgmt:/var/log/raja/frr# time sudo vtysh -f option1
<SNIP>..............
Waiting for children to finish applying config...
[19887|staticd] done
[19885|watchfrr] done
[19886|pbrd] done
[19874|ospfd] done
[19877|bgpd] done
[19871|zebra] done
real 0m12.168s
user 0m7.511s
sys 0m1.981s
Issue: 3589101
Ticket# 3589101
Signed-off-by: Rajasekar Raja <rajasekarr@nvidia.com>
Commit: 9112fb367b
Introduced the idea of setting the socket buffer
send/receive sizes. BSD's in general have the fun
issue of not allowing nearly as large as a size as
linux. Since the above commit was developed on linux
and not run on bsd it was never tested. Modify the
codebase to use the backoff setsockopt that we have
in the code base and use the returned values to allow
us to notice what was set and respond appropriately.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
The output buffer vty->obuf is a linked list where
each element is of 4KB.
Currently, when a huge sh command like <show ip route json>
is executed on a large scale, all the vty_outs are
processed and the entire data is accumulated.
After the entire vty execution, vtysh_flush proceeses
and puts this data in the socket (131KB at a time).
Problem here is the memory spike for such heavy duty
show commands.
The fix here is to chunkify the output on VTY shell by
flushing it intermediately for every 128 KB of output
accumulated and free the memory allocated for the buffer data.
This way, we achieve ~25-30% reduction in the memory spike.
Fixes: #16498
Note: This is a continuation of MR #16498
Signed-off-by: Srujana <skanchisamud@nvidia.com>
Signed-off-by: Rajasekar Raja <rajasekarr@nvidia.com>
Change input/output arguments of the RPC callback from lists of
(xpath/value) tuples to YANG data trees.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
This operation basically implements support for RESTCONF operations. It
receives an xpath and a data tree in JSON/XML format, instead of a list
of (xpath, value) tuples as required by the current protobuf interface.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
When exiting from a level below the config node, like `router rip`,
vtysh executes a resync by sending "end" and "conf term [file-lock]"
commands to all the daemons. As statet in the description comment, it's
done "in case one of the daemons is somewhere else". I don't think this
actually ever happens, but even if it is, it is a bug in a daemon that
needs to be fixed. This resync was okay before the introduction of
mgmtd, but now it unlocks and locks back the datastores during the
configuration reading process, which can lead to a failure which is
explained in the previous commit.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Like in RESTCONF GET request and NETCONF get-data request, make it
possible to request state-only, config-only, or all data.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Currently it's the same as get-tree request for the backend, but it is
going to be expanded in the following commits.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
`nb_cli_apply_changes` can be called with base xpath which should be
prepended to xpaths of every change in a transaction. This base xpath is
respected by regular northbound CLI but not by mgmtd. This commit fixes
the problem.
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
Modify empty json object to take input obj
instead of allocating always one.
There are situation where in error condition or no data
case print empty json (`{}`) with already allocated
Signed-off-by: Chirag Shah <chirag@nvidia.com>
There's a workaround in the code from a bug from back in 2004, it ends
and re-enters config mode anytime an `exit` is done from a level below
the top-level config node (e.g., from a `router isis` node). We need to
re-enter config mode with or without a lock according to how we actually
entered it to begin with.
fixes#13920
Signed-off-by: Christian Hopps <chopps@labn.net>
Move away from things like "lock if not locked" type code, require the
user has locked prior to geting to that point.
For now we warn if we are taking a lock we already had; however, this
should really be a failure point.
New requirements:
SETCFG -
not implicit commit - requires user has locked candidate DS and they
must unlock after
implicit commit - requires user has locked candidate and running DS
both locks will be unlocked on reply to the SETCFG
COMMITCFG -
requires user has locked candidate and running DS and they must unlock
after
rollback - this code now get both locks and then does an unlock and
early return thing on the adapter side. It needs to be un-special
cased in follow up work that would also include tests for this
functionality.
Signed-off-by: Christian Hopps <chopps@labn.net>
- log names of datastores not numbers
- improve logging for mgmt_msg_read
- Rather than use a bool, instead store the pending const string name of
the command being run that has postponed the CLI. This adds some nice
information to the logging when enabled.
Signed-off-by: Christian Hopps <chopps@labn.net>
- Previously was substituting a pointer to local allocated session for the
session_id returned from the FE adapter. This complexity isn't needed.
- Get rid of "%llu" format and the casts that came with it, instead use PRIu64
and the actual (uint64_t) type.
Signed-off-by: Christian Hopps <chopps@labn.net>
When the user specifies `--command-log-always` in CLI arguments then also log
commands executed from loading the config file.
Signed-off-by: Christian Hopps <chopps@labn.net>
When daemons transition to mgmtd they should stop reading their split config
files, and let mgmtd do that, otherwise things can get out of sync.
Signed-off-by: Christian Hopps <chopps@labn.net>
Effectively a massive search and replace of
`struct thread` to `struct event`. Using the
term `thread` gives people the thought that
this event system is a pthread when it is not
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This is a first in a series of commits, whose goal is to rename
the thread system in FRR to an event system. There is a continual
problem where people are confusing `struct thread` with a true
pthread. In reality, our entire thread.c is an event system.
In this commit rename the thread.[ch] files to event.[ch].
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
This commit contains fixes for the following issues found
- 'mgmt commit check' issued through 'vtysh -f' was actually commtting the changeset.
- On config validation failure backend, mgmtd was not passing the correct error-reason
to frontend.
- 'mgmt rollback ...' was reverting the change on backend, but config on mgmtd daemon
remains intact
Signed-off-by: Pushpasis Sarkar <pushpasis@gmail.com>
This commit introduces the Frontend Interface which can be used
by front-end management clients like Netconf server, Restconf
Server and CLI to interact with new FRR Management daemon (MGMTd)
to access and sometimes modify FRR management data.
This commit includes the following functionalities in the changeset:
1. Add new Frontend server for clients connect to.
2. Add a C-based Frontend client library which can be used by Frontend
clients to communicate with MGMTd via the Frontend interface.
3. Maintain a frontend adapter for each connection from an appropriate
Frontend client to facilitate client requests and track one or more
client sessions across it.
4. Define the protobuf message format for messages to be exchanged
between MGMTd Frontend module and the Frontend client.
5. This changeset also introduces an instance of MGMT Frontend client
embedded within the lib/vty module that can be leveraged by any FRR
daemon to connect to MGMTd's Frontend interface. The same has been
integrated with and initialized within the MGMTd daemon's process
context to implement a bunch of 'set-config', 'commit-apply',
'get-config' and 'get-data' commands via VTYSH
Co-authored-by: Pushpasis Sarkar <pushpasis@gmail.com>
Co-authored-by: Abhinay Ramesh <rabhinay@vmware.com>
Co-authored-by: Ujwal P <ujwalp@vmware.com>
Signed-off-by: Yash Ranjan <ranjany@vmware.com>
Initial commit: 23b2a7ef52
changed the json output of `show bgp <afi> <safi> json` to
not have pretty print because when under a situation where
there are a bunch of routes with a large scale ecmp show
output was taking forever and this commit cut 2 minutes out
of vtysh run time.
Subusequent commit: f4ec52f7cc
changed this back.
When upgrading to latest version the long run time was noticed
due to testing. Let's add back this functionality such that
FRR can have reduced run times with vtysh when it's really
needed.
Signed-off-by: Donald Sharp <sharpd@nvidia.com>
Some results:
```
====
PCRE
====
% ./a.out "^65001" "65001"
comparing: ^65001 / 65001
ret status: 0
[14:31] donatas-pc donatas /home/donatas
% ./a.out "^65001_" "65001"
comparing: ^65001_ / 65001
ret status: 0
=====
PCRE2
=====
% ./a.out "^65001" "65001"
comparing: ^65001 / 65001
ret status: 0
[14:30] donatas-pc donatas /home/donatas
% ./a.out "^65001_" "65001"
comparing: ^65001_ / 65001
ret status: 1
```
Seems that if using PCRE2, we need to escape outer `()` chars and `|`. Sounds
like a bug.
But this is only with some older PCRE2 versions. With >= 10.36, I wasn't able
to reproduce this, everything is fine and working as expected.
Adding _FRR_PCRE2_POSIX definition because pcre2posix.h does not have
include's guard.
Signed-off-by: Donatas Abraitis <donatas@opensourcerouting.org>
There's a common pattern of "get VRF context for CLI node" here, which
first got a helper macro in zebra that then permeated into pimd.
Unfortunately the pimd copy wasn't quite adjusted correctly and thus
caused two coverity warnings (CID 1517453, CID 1517454).
Fix the PIM one, and clean up by providing a common base macro in
`lib/vty.h`.
Also rename the macros (add `_VRF`) to make more clear what they do.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This adds the plumbing necessary to yield back a file descriptor to
vtysh. The fd is passed on the command status code bytes through
AF_UNIX SCM_RIGHTS.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
... this is copypasted all over the codebase & should've been a helper
to begin with really.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
These are just used to iterate over active vty sessions, a vector is a
weird choice there.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
These had no remaining users for a while now. The logging backend has
its own list of receivers.
Signed-off-by: David Lamparter <equinox@opensourcerouting.org>
This commit fixes the following problem:
- enter the interface node
- move the interface to another VRF
- try to continue configuring the interface
It is not possible to continue configuration because the XPath stored in
the vty doesn't correspond with the actual state of the system anymore.
For example:
```
nfware# conf
nfware(config)# interface enp2s0
<-- move the enp2s0 to a different VRF -->
nfware(config-if)# ip router isis 1
% Failed to get iface dnode in candidate DB
```
To fix the issue, go through all connected vty shells and update the
stored XPath.
Suggested-by: Renato Westphal <renato@opensourcerouting.org>
Signed-off-by: Igor Ryzhov <iryzhov@nfware.com>
