shukitchan commented on code in PR #10559:
URL: https://github.com/apache/trafficserver/pull/10559#discussion_r1360098868
##########
doc/admin-guide/plugins/rate_limit.en.rst:
##########
@@ -122,51 +121,117 @@ The basic use is as::
done using e.g. the ``conf_remap`` plugin,
:ts:cv:`proxy.config.http.keep_alive_no_activity_timeout_in`.
-The following options are available:
-.. program:: rate-limit
-
-.. option:: --limit
+The YAML configuration can have the following format, where the varies sections
Review Comment:
various?
##########
plugins/experimental/rate_limit/sni_selector.cc:
##########
@@ -139,6 +262,26 @@ SniSelector::setupQueueCont()
_queue_cont = TSContCreate(sni_queue_cont, TSMutexCreate());
TSReleaseAssert(_queue_cont);
TSContDataSet(_queue_cont, this);
- _action = TSContScheduleEveryOnPool(_queue_cont, QUEUE_DELAY_TIME.count(),
TS_THREAD_POOL_TASK);
+ _queue_action = TSContScheduleEveryOnPool(_queue_cont,
QUEUE_DELAY_TIME.count(), TS_THREAD_POOL_TASK);
}
}
+
+///////////////////////////////////////////////////////////////////////////////
+// Startup of the SNI selector hooks and config reload continuation and
instance
+//
+void
+SniSelector::startup()
+{
+ TSCont sni_cont = TSContCreate(sni_limit_cont, nullptr);
+
+ TSReleaseAssert(sni_cont);
+
+ _instance.store(new SniSelector());
+ TSHttpHookAdd(TS_SSL_CLIENT_HELLO_HOOK, sni_cont);
+ TSHttpHookAdd(TS_VCONN_CLOSE_HOOK, sni_cont);
+
+ auto config_cont = TSContCreate(sni_config_cont, TSMutexCreate());
+
+ TSReleaseAssert(config_cont);
+ TSContScheduleEveryOnPool(config_cont,
std::chrono::milliseconds{10000}.count(), TS_THREAD_POOL_TASK);
Review Comment:
Is there a reason why we want to change the config file every 10 seconds vs
using TSMgmtUpdateRegister() and wait for an explicit config reload
just to make it more convenient?
Can a race condition happen when the config yaml is only partial updated
when the config_cont is scheduled to run and therefore resulting in some
inconsistent behavior?
##########
doc/admin-guide/plugins/rate_limit.en.rst:
##########
@@ -122,51 +121,117 @@ The basic use is as::
done using e.g. the ``conf_remap`` plugin,
:ts:cv:`proxy.config.http.keep_alive_no_activity_timeout_in`.
-The following options are available:
-.. program:: rate-limit
-
-.. option:: --limit
+The YAML configuration can have the following format, where the varies sections
+and nodes are documented below.
+
+ .. code-block:: yaml
+
+ selector:
+ - sni: test1.example.com
+ limit: 1000
+ queue:
+ size: 1000
+ max-age: 30
+ metrics:
+ tag: example.com
+ prefix: ddos
+ ip-rep: main
+ exclude: internal
+ - sni: test2.example.com
+ aliases: [test3.example.com, test4.example.com]
+ limit: 100
+ ip-rep:
+ - name: main
+ buckets: 10
+ size: 15
+ percentage: 90
+ max-age: 300
+ perma-block:
+ limit: 100
+ threshold: 1
+ max-age: 1800
+ lists:
+ - name: internal
+ cidr:
+ - 10.0.0.0/8
+ - 192.168.0.0/16
+
+
+For the top level `selector` node, the following options are available:
+
+.. option:: sni
+
+ The SNI to match for this rate limiter.
+
+.. option:: limit
The maximum number of active client transactions.
-.. option:: --queue
+.. option:: aliases
- When the limit (above) has been reached, all new connections are placed
- on a FIFO queue. This option (optional) sets an upper bound on how many
- queued transactions we will allow. When this threshold is reached, all
- additional connections are immediately errored out in the TLS handshake.
+ A list of aliases for the SNI, which will also be matched by this rate
limiter.
- The queue is effectively disabled if this is set to ``0``, which implies
- that when the transaction limit is reached, we immediately start serving
- error responses.
+.. option:: ip-rep
- The default queue size is ``UINT_MAX``, which is essentially unlimited.
+ The name of the IP reputation node to use for this rate limiter. If not
+ specified, the IP reputation system is not used for this rate limiter.
-.. option:: --maxage
+.. option:: exclude
- An optional ``max-age`` for how long a transaction can sit in the delay
queue.
- The value (default 0) is the age in milliseconds.
+ A list of IP CIDR ranges to exclude from any rate limiting. Any IP
matching
+ this list will not be rate limited, even if the SNI matches.
-.. option:: --prefix
+.. option:: queue
- An optional metric prefix to use instead of the default
(plugin.rate_limiter).
+ If enabled, when the limit (above) has been reached, all new connections
+ are placed on a FIFO queue. This option sets an upper bound on
+ how many queued transactions we will allow. When this threshold is reached,
+ all additional connections are immediately errored out in the TLS handshake.
-.. option:: --tag
+ The queue option can include a `size` and a `max-age` option. The size is
+ default to ``UINT_MAX``, which is essentially unlimited. The max-age is
+ default to ``0``, which means no age limit.
- An optional metric tag to use instead of the default. When a tag is not
specified
- the plugin will use the FQDN of the SNI associated with each rate limiter
instance
- created during plugin initialization.
+ No queue is enable without this configuration directive, but it can also be
+ disable explicitly if the size is set to ``0``.
Review Comment:
disabled?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
