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

Unified location processing #862

Open
vankoven opened this issue Nov 13, 2017 · 5 comments
Open

Unified location processing #862

vankoven opened this issue Nov 13, 2017 · 5 comments

Comments

@vankoven
Copy link
Contributor

vankoven commented Nov 13, 2017

Although #851 introduced significant changes to cfg.c it still has architectural issues: the same cleanup function for different directives, cleanup is not aware of that it cleans up actually (so it's not possible to clean up a part of repeatable directives, e.g. srv_group), tracking of child specs is a little bit hacky (see usage of __called_now flags), it's not possible to get current effective configuration.

Tempesta should build a tree of TfwCfgEntry to represent text configuration. This change will eliminate __called_* flags in TfwCfgSpec structure. In the same time the TfwCfgSpec structure must be used only as description of configuration directives.

Issues #67 and #102 is mostly about scenarios of interacting with configuration, so requirements from that issues must be taken into account during completing this task.

TfwHttpReq->location should always point to the full effective set of location-based setting. Currently it's required to look though current location, current vhost default location and global vhost settings to obtain complete set of settings.

Example:

tempesta/tempesta_fw/http.c

Lines 2004 to 2006 in 40dd107

TfwLocation *loc = req->location;
TfwLocation *loc_dflt = req->vhost->loc_dflt;
TfwLocation *base_loc = (tfw_vhost_get_default())->loc_dflt;

@vankoven vankoven added this to the 1.0 WebOS milestone Nov 13, 2017
@krizhanovsky krizhanovsky modified the milestones: backlog, 0.8 TDB v0.2 Jan 15, 2018
@vankoven
Copy link
Contributor Author

Historically there wasn't vhost and location directives and srv_group directive included some options that actually describe web service behaviour. This options are about HTTP processing features:

server_forward_retries
server_forward_timeout
server_retry_nonidempotent

While others describe the group of servers as single load-balaned array of server connections and 'low-level' connection and load balancer properties:

server_connect_retries
server_queue_size
sched
sticky_sessions

It seems reasonable to move options from the first group to vhost settings.

Need to review all the configuration directives and stabiles them. Future additions must extend but not change configuration format. Tempesta's upgrade to the newer version should be possible without any issues.

@krizhanovsky
Copy link
Contributor

krizhanovsky commented Nov 29, 2018

Some time ago we already made a step towards building AST for configuration files, but declined the approach due to complexity. During all the development we had a lot of issues with proper configuration files parsing. Meantime, modern cloud infrastructures (service mesh) require dynamically reconfigured services with replicated among cluster configurations. As the result JSON stored in some replicated database is popular. REST/gRPC APIs are another configuration challenge.

With all the above in mind I propose to consider to move configuration parser (current cfg.c) to user space:

  1. tempesta.sh calls a C++ configuration parser
  2. the parser reads and parses the file and generates binary data structures using Tempesta core header files (so we can move pretty printing, [cfg] More user-friendly configuration parsing error messages #709, for user space);
  3. Tempesta uses netlink sockets to get the data structures and dynamically update them in RCU fasshion;

Using the user space daemon with netlink interface allows us to mix Tempesta FW configuration with nftables - a user can write L3-L7 rules in a file and we can just call the user-space nft to handle the rules.

Next, the code base of the C++ program can be reused for #67 (RESTful & gRPC APIs), at least in the part of loading binary data structures. It seems much easier to generate a binary structures and load them to the kernel than adjust a human-written configuration file (in any allowed format) according to received configuration update and then, after JSON parsing, run another configuration file parser in the kernel. For #67 we still can use the Perl script to generate or adjust a human-readable configuration file, but

  1. the most crucial configuration part will work with binary data, so we'll have less problems with the code;
  2. any nice configuration features and different formats to load and represent the configuration will be in user-space - faster for development, less bugs and so on;
  3. the configuration parsing code can be used on admin node (running WebUI, Web UI #68) to process and distribute configuration among Tempesta nodes.

The last, but very wished, opportunity is to make a foundation for TL (#102) using Flex/Bison grammar tor the configuration file - no need to build the AST by hands! For this purpose we can move to a different syntax.

Just one lesson from old DPI project worth mentioning: the binary data passed from the user-space program to the kernel is also an intermediary representation - final data structures use pointers to kernel objects and various dynamic structures, so we also have to process the intermediary binary configuration representation to build data structures usable in run-time. However, the binary data structures are less error prone and probably we case do less memory allocations on building the final data structures since we already know all the sizes.

@krizhanovsky krizhanovsky modified the milestones: 1.1 QUIC, 1.0 Beta Nov 29, 2018
@vankoven
Copy link
Contributor Author

Totally agree here, parsing configuration out of the kernel is highly desired feature, especially when we speak about REST/gRPC APIs. I will add a more requirements for this userspace program:

Show current active configuration. With full runtime reconfiguration it's now clear which exact configuration is used at the moment. Some features, like grace_shutdown_time will force Tempesta to keep parts of the old configuration for a some time. Other reason to have the feature - is configuration function tests: assure that configuration is correctly parsed and it means exactly what it stated.

Apply configuration changes atomically. Now live reconfiguration is full reconfiguration and it takes ages to reload very huge configuration under heavy load. With atomic changes, e.g. "add new server group" instead of "leave 1M server groups as is and add a new server group", runtime reconfiguration will be less complicated and more reliable. This will also implement 50% of REST/gRPC APIs requirements.

Probably that configuration parser program should replace tempesta.sh itself. Now we havve a lot of different processing in tempesta.sh, it already uses some perl code, it has limitations (js_challenge dirrective mustn't be split into several lines...).

@vankoven
Copy link
Contributor Author

Some directives may be configured globally, but in some sections administrator may want to revert back to defaults. I suggest to use no prefix before directive to disable it (for boolean directives) or to switch to defaults. The same approach widely used in configs for network switches and should be known by systems administrators. E.g.

# Defaults for all vhosts
resp_hdr_set SomeHdr "Some value";

vhost outstanding {
# Need to disable 'resp_hdr_set' directive for this specific vhost
no resp_hdr_set;
}

@krizhanovsky krizhanovsky modified the milestones: 1.0 Beta, 0.9 TDBv0.2 Feb 2, 2019
@krizhanovsky krizhanovsky changed the title [CFG] Build configuration tree to represent config file Unified location processing Oct 11, 2019
@tempesta-tech tempesta-tech deleted a comment from vankoven Oct 11, 2019
@krizhanovsky
Copy link
Contributor

I rewrote #67 to move all the configuration logic from tile to TDB, so no need to update file parser. The only issue (also referenced in the code) is about location processing, so I deleted couple comments and rewritten the initial task statement.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants