-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
gRPC northbound plugin #4082
gRPC northbound plugin #4082
Conversation
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7134/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
clang_check |
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
just first commit done.
more wil be done later.
@@ -413,7 +413,8 @@ enum nb_error { | |||
|
|||
/* Northbound clients. */ | |||
enum nb_client { | |||
NB_CLIENT_CLI = 0, | |||
NB_CLIENT_NONE = 0, | |||
NB_CLIENT_CLI, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why having a NONE_CLIENT ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not strictly necessary, but it's useful when you want to indicate the absence of a value without using zero directly (which would actually translate to NB_CLIENT_CLI
). Currently NB_CLIENT_NONE
is being used by the nb_running_unlock()
function to indicate that no client owns the configuration lock.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These look good to me. The locks could be a little tricky--they look correct to me, but it might be good if someone else takes a look to make certain I didn't miss something. Otherwise, I'll push this in a couple of days.
AC_LANG([C]) | ||
AC_PROG_CC | ||
AC_PROG_CPP | ||
AC_PROG_CXX | ||
AM_PROG_CC_C_O |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
will it be possible to compile frr with C-only ?
I mean, I expect no implicit dependency with C compiler
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. The autoconf documentation says that AC_PROG_CXX
will set CXX
to g++
if it fails to find a C++ compiler, without returning an error. This means a C++ compiler won't be required to build FRR (except if you want to build the gRPC module).
would the async mode of using grpc be a better fit for us - or isn't that supported by the c++ version? |
I don't see which benefit we'd gain from using the async model. gRPC runs its own event loop on a separate pthread, so there's no chance it can block the main pthread in any way. Also, the sync model can handle multiple parallel requests normally (upon initialization, gRPC spawns a pthread pool for itself). I read somewhere that the advantage of the async model is higher level of control over threading, at the cost of much more complex code. This probably only makes sense for applications that need to handle thousands of RPCs per second, which shouldn't be our case. I suspect however that we might need the async model in the future to implement subscriptions to YANG notifications. The subscriptions will be long-lived stream RPCs that will block indefinitely, and I'm not sure if we can do this using the sync model. Something to keep in mind for later. |
I got an issue when trying to compile.
|
Works on my machine 😂 Seriously speaking, the gRPC module shouldn't require C++11 as you can see below:
Notice that neither -std=c++1 nor -std=gnu++11 are being used and the code is compiling just fine. Please provide more details about this problem like a full build log so that I can try to see what's going wrong. |
ok; I managed to compile, thanks to github.com/grpc/issues/16739. I could not go further, since I lack some information on ruby ( for experimenting the testing case). |
First you need to install gRPC for ruby:
Then you need to generate ruby libraries from the FRR service definitions file:
Now the example ruby script should be able to include the files below and you should be good to go: require "grpc"
require "frr-northbound_services_pb" A similar process should be required if you want to configure/monitor FRR using other languages, like Python and Go. EDIT:
It's also possible to install gRPC from your package manager, but this isn't mentioned by the gRPC documentation so it's probably not a good idea. |
@rwestphal Looks like this PR got out of date! |
…onfiguration The ability to lock the running configuration to prevent other users from changing it is a very important one. We already supported the "configure exclusive" command but the lock was applied to the CLI users only (other clients like ConfD could still commit configuration transactions, ignoring the CLI lock). This commit introduces a global lock for the running configuration that is shared by all northbound clients, and provides a public API to manipulate it. This way other northbound clients will also be able to lock/unlock the running configuration if required (the upcoming gRPC northbound plugin will have RPCs for that). NOTE: this is a management-level lock for the running configuration, not to be confused with low-level locks used to avoid data races. Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
The upcoming gRPC-based northbound plugin will run on a separate pthread, and it will need to have access to the running configuration global variable. Introduce a rw-lock to control concurrent access to the running configuration. Add the lock inside the "nb_config" structure so that it can be used to protect candidate configurations as well (this might be necessary depending on the threading scheme of future northbound plugins). Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
This is an experimental plugin for now. Full documentation will come later. Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
5763720
to
ec2ac5f
Compare
💚 Basic BGPD CI results: SUCCESS, 0 tests failedResults table
For details, please contact louberger |
Continuous Integration Result: SUCCESSFULCongratulations, this patch passed basic tests Tested-by: NetDEF / OpenSourceRouting.org CI System CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-7384/ This is a comment from an automated CI system. Warnings Generated during build:Checkout code: Successful with additional warnings
CLANG Static Analyzer Summary
No Changes in Static Analysis warnings compared to base12 Static Analyzer issues remaining.See details at |
This PR adds a new gRPC-based northbound plugin to FRR.
This plugin provides an easy-to-use interface to configure and monitor FRR programmatically, using a variety of different programming languages (e.g. python, ruby, go, etc). It should be an alternative for users who want to automate FRR routers without going through the complexities of the NETCONF/RESTCONF protocols [1], while still benefiting from the advantages of using YANG-modeled data.
To use this plugin, each daemon must be started using the
-M grpc:PORT
parameter. The daemons are managed separately [2] and must listen on different TCP ports. As of now the gRPC plugin listens on the loopback address only, support for remote clients with SSL/TLS authentication will be added later.Once the FRR daemons are started with the gRPC plugin, management clients can execute all RPCs defined in the FRR Northbound service definition file: https://github.com/opensourcerouting/frr/blob/grpc-nb-plugin/grpc/frr-northbound.proto
Here's an example client script written in Ruby: https://gist.github.com/rwestphal/6528ebeb8a612711ca3771b843a4eb14
Documentation for this and the other northbound plugins should be added later on a separate PR.
Needless to say, only daemons converted to the new northbound model will benefit from this new plugin.
Performance
The gRPC plugin delivers much better performance compared to what we get when using the ConfD/Sysrepo plugins, especially when fetching bulk operational data.
The table below shows the time required to fetch 1kk RIP routes using a variety of different methods:
As it can be seen, while the gRPC plugin performs better compared to the other northbound clients, it's still slower compared to using the CLI "show ... json" commands to obtain JSON-encoded data. There's a lot of room for optimization though. The callback given to
nb_oper_data_iterate()
can be refactored to uselyd_new()
andlyd_new_leaf()
instead of the expensivelyd_new_path()
function (which does full XPath parsing). And the operational-data northbound callbacks can be refactored as well to reduce the amount of memory allocations. Once these optimizations are in place, we should be able get much better performance using the gRPC plugin to fetch bulk operational data.Caveats
running_config
global variable, calling the northbound configuration callbacks from a pthread other than the main one is unsafe as lots of other variables aren't protected by any lock. What needs to be done to address this issue is to create some kind of thread communication mechanism to process the Commit() RPC in the main pthread and return the results to the gRPC pthread. This plugin should be considered experimental until that work is done.Implementation
This plugin was written in C++ since gRPC doesn't have official support for C.
[1] NETCONF is supported by the ConfD and Sysrepo plugins, and RESTCONF is supported by the ConfD plugin only.
[2] The long term goal is to move all northbound plugins to watchfrr in order to centralize and simplify the way we configure FRR, but this should take a while to happen
[3] This command doesn't exist in vanilla FRR and was added for benchmarking purposes only.