You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by zw...@apache.org on 2022/04/05 20:12:21 UTC

[trafficserver] branch 9.2.x updated (dfda50355 -> a6d1f24a4)

This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a change to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git


    from dfda50355 STEK share plugin using Raft (#8751)
     new 8ea2f4c00 Added metrics to the rate limit plugin and document the new options (#8395)
     new a6d1f24a4 Adds an IP reputation system to the SNI rate limiter (#8459)

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 doc/admin-guide/plugins/rate_limit.en.rst        | 196 +++++++++++++-
 doc/static/images/sdk/SieveLRU.png               | Bin 0 -> 137860 bytes
 plugins/experimental/rate_limit/Makefile.inc     |   1 +
 plugins/experimental/rate_limit/ip_reputation.cc | 323 +++++++++++++++++++++++
 plugins/experimental/rate_limit/ip_reputation.h  | 236 +++++++++++++++++
 plugins/experimental/rate_limit/iprep_simu.cc    | 299 +++++++++++++++++++++
 plugins/experimental/rate_limit/limiter.h        |  81 ++++++
 plugins/experimental/rate_limit/rate_limit.cc    |   5 +-
 plugins/experimental/rate_limit/sni_limiter.cc   | 114 +++++++-
 plugins/experimental/rate_limit/sni_limiter.h    |  24 ++
 plugins/experimental/rate_limit/sni_selector.cc  |  19 +-
 plugins/experimental/rate_limit/txn_limiter.cc   |  14 +
 plugins/experimental/rate_limit/utilities.cc     |  46 ++++
 plugins/experimental/rate_limit/utilities.h      |   1 +
 14 files changed, 1344 insertions(+), 15 deletions(-)
 create mode 100644 doc/static/images/sdk/SieveLRU.png
 create mode 100644 plugins/experimental/rate_limit/ip_reputation.cc
 create mode 100644 plugins/experimental/rate_limit/ip_reputation.h
 create mode 100644 plugins/experimental/rate_limit/iprep_simu.cc


[trafficserver] 01/02: Added metrics to the rate limit plugin and document the new options (#8395)

Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit 8ea2f4c003549034f4d32593763881ab67b0119c
Author: Jeff Elsloo <el...@users.noreply.github.com>
AuthorDate: Tue Oct 26 08:48:10 2021 -0600

    Added metrics to the rate limit plugin and document the new options (#8395)
    
    * Added metrics to the rate limit plugin and documented the new options.
    
    * * Addressed feedback in PR review
    
    * Fixed calculation for metric length that was counting too many things due to a prior implementation and lack of cleanup of this specific line of code
    
    (cherry picked from commit a3f04cb6dc12c0d192f4abd981b5ff09647484f4)
---
 doc/admin-guide/plugins/rate_limit.en.rst       | 134 ++++++++++++++++++++++++
 plugins/experimental/rate_limit/limiter.h       |  81 ++++++++++++++
 plugins/experimental/rate_limit/rate_limit.cc   |   5 +-
 plugins/experimental/rate_limit/sni_limiter.cc  |  10 ++
 plugins/experimental/rate_limit/sni_limiter.h   |   2 +
 plugins/experimental/rate_limit/sni_selector.cc |   4 +
 plugins/experimental/rate_limit/txn_limiter.cc  |  14 +++
 plugins/experimental/rate_limit/utilities.cc    |  46 ++++++++
 plugins/experimental/rate_limit/utilities.h     |   1 +
 9 files changed, 296 insertions(+), 1 deletion(-)

diff --git a/doc/admin-guide/plugins/rate_limit.en.rst b/doc/admin-guide/plugins/rate_limit.en.rst
index f50377223..ef3f3246f 100644
--- a/doc/admin-guide/plugins/rate_limit.en.rst
+++ b/doc/admin-guide/plugins/rate_limit.en.rst
@@ -80,6 +80,18 @@ are available:
    An optional `max-age` for how long a transaction can sit in the delay queue.
    The value (default 0) is the age in milliseconds.
 
+.. option:: --prefix
+
+   An optional metric prefix to use instead of the default (plugin.rate_limiter).
+
+.. option:: --tag
+
+   An optional metric tag to use instead of the default. When a tag is not specified
+   the plugin will use the scheme, FQDN, and port when it is non-standard. For example
+   a default plugin tag might be "https.example.com" or "http.example.com:8080"
+   noting that in the latter exampe, the non-standard scheme and port led to
+   ":8080" being appended to the string.
+
 Global Plugin
 -------------
 
@@ -122,6 +134,61 @@ The following options are available:
    An optional `max-age` for how long a transaction can sit in the delay queue.
    The value (default 0) is the age in milliseconds.
 
+.. option:: --prefix
+
+   An optional metric prefix to use instead of the default (plugin.rate_limiter).
+
+.. option:: --tag
+
+   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.
+
+Metrics
+-------
+Metric names are generated either using defaults or user-supplied values. In either
+case, the format of the metric names is as follows:
+
+   ``prefix.type.tag.metric``
+
+A user can specify their own prefixes and tags, but not types or metrics.
+
+``prefix``
+   The default prefix for all metrics is `plugin.rate_limiter`.
+
+``type``
+   There are two types of metrics: `sni` and `remap`. Each type corresponds with the
+   type of configuration used to generate the metric. The global configuration is for
+   rate limiting requests during TLS negotiation, hence, the type of ``sni``. Similarly
+   ``remap`` connotes a remap configuration.
+
+``tag``
+   By default the metric tag is derived from a description that is set conditionally.
+   When configured in global mode, the ``SNI`` argument allows a comma separated list
+   of FQDNs that require rate limiting. Each FQDN is associated with an instance of
+   the rate limiter, and the description of each limiter is set to the FQDN.
+
+   When configured on a remap, the plugin will generate a description based on the
+   configuration. When the scheme and port number are standard, the port is omitted
+   from the generated description, however, when the scheme and port combination are
+   non-standard, the port is appended. For example, a standard scheme and port would
+   lead to a description of ``http.example.com`` or ``https.example.com`` but if a
+   non-standard port was used, a description might be ``https.example.com:8443`` or
+   ``http.example.com:8080``. This approach allows each limiter to increment metrics
+   for the correct remaps.
+
+``metric``
+   There are four metrics that may be incremented, depending on which action the plugin takes:
+
+   ============== ===================================================================
+   Metric         Definition
+   ============== ===================================================================
+   ``queued``     Request queued due to being at the limit but under the queue limit.
+   ``rejected``   Request rejected due to being over the defined limits.
+   ``expired``    Queued connection is too old to be resumed and is rejected.
+   ``resumed``    Queued connection is resumed.
+   ============== ===================================================================
+
 Examples
 --------
 
@@ -158,3 +225,70 @@ In this case, the response would look like this when the queue is full: ::
     Content-Language: en
     Retry-After: 3600
     Content-Length: 207
+
+Metric Examples
+---------------
+The following examples show the metric names that result from various settings
+using a hypothetical domain of example.com with both global and remap configurations.
+Note that in this example the remap configuration contains both TLS and non-TLS
+remap rules.
+
+Defaults:
+::
+
+   proxy.rate_limiter.sni.example.com.queued
+   proxy.rate_limiter.sni.example.com.rejected
+   proxy.rate_limiter.sni.example.com.expired
+   proxy.rate_limiter.sni.example.com.resumed
+
+   proxy.rate_limiter.remap.https.example.com.queued
+   proxy.rate_limiter.remap.https.example.com.rejected
+   proxy.rate_limiter.remap.https.example.com.expired
+   proxy.rate_limiter.remap.https.example.com.resumed
+
+   proxy.rate_limiter.remap.http.example.com.queued
+   proxy.rate_limiter.remap.http.example.com.rejected
+   proxy.rate_limiter.remap.http.example.com.expired
+   proxy.rate_limiter.remap.http.example.com.resumed
+
+Defaults with non-standard scheme+port combinations in the remap rules:
+::
+
+   proxy.rate_limiter.sni.example.com.queued
+   proxy.rate_limiter.sni.example.com.rejected
+   proxy.rate_limiter.sni.example.com.expired
+   proxy.rate_limiter.sni.example.com.resumed
+
+   proxy.rate_limiter.remap.https.example.com:8443.queued
+   proxy.rate_limiter.remap.https.example.com:8443.rejected
+   proxy.rate_limiter.remap.https.example.com:8443.expired
+   proxy.rate_limiter.remap.https.example.com:8443.resumed
+
+   proxy.rate_limiter.remap.http.example.com:8080.queued
+   proxy.rate_limiter.remap.http.example.com:8080.rejected
+   proxy.rate_limiter.remap.http.example.com:8080.expired
+   proxy.rate_limiter.remap.http.example.com:8080.resumed
+
+With:
+  * ``--prefix=limiter`` on the global configuration
+  * ``--tag=tls.example.com`` on the global configuration
+  * ``@pparam=--prefix=limiter`` on the remap configurations
+  * ``@pparam=--tag=secure.example.com`` on the TLS-enabled remap configuration
+  * ``@pparam=--tag=insecure.example.com`` on the non-TLS-enabled remap configuration
+
+::
+
+   limiter.sni.tls.example.com.queued
+   limiter.sni.tls.example.com.rejected
+   limiter.sni.tls.example.com.expired
+   limiter.sni.tls.example.com.resumed
+
+   limiter.remap.secure.example.com.queued
+   limiter.remap.secure.example.com.rejected
+   limiter.remap.secure.example.com.expired
+   limiter.remap.secure.example.com.resumed
+
+   limiter.remap.insecure.example.com.queued
+   limiter.remap.insecure.example.com.rejected
+   limiter.remap.insecure.example.com.expired
+   limiter.remap.insecure.example.com.resumed
diff --git a/plugins/experimental/rate_limit/limiter.h b/plugins/experimental/rate_limit/limiter.h
index 4d56ffc95..9c4f4b0cd 100644
--- a/plugins/experimental/rate_limit/limiter.h
+++ b/plugins/experimental/rate_limit/limiter.h
@@ -31,6 +31,39 @@
 constexpr auto QUEUE_DELAY_TIME = std::chrono::milliseconds{200}; // Examine the queue every 200ms
 using QueueTime                 = std::chrono::time_point<std::chrono::system_clock>;
 
+enum {
+  RATE_LIMITER_TYPE_SNI = 0,
+  RATE_LIMITER_TYPE_REMAP,
+
+  RATE_LIMITER_TYPE_MAX
+};
+
+// order must align with the above
+static const char *types[] = {
+  "sni",
+  "remap",
+};
+
+// no metric for requests we accept; accepted requests should be counted under their usual metrics
+enum {
+  RATE_LIMITER_METRIC_QUEUED = 0,
+  RATE_LIMITER_METRIC_REJECTED,
+  RATE_LIMITER_METRIC_EXPIRED,
+  RATE_LIMITER_METRIC_RESUMED,
+
+  RATE_LIMITER_METRIC_MAX
+};
+
+// order must align with the above
+static const char *suffixes[] = {
+  "queued",
+  "rejected",
+  "expired",
+  "resumed",
+};
+
+static const char *RATE_LIMITER_METRIC_PREFIX = "plugin.rate_limiter";
+
 ///////////////////////////////////////////////////////////////////////////////
 // Base class for all limiters
 //
@@ -139,6 +172,50 @@ public:
     }
   }
 
+  void
+  initializeMetrics(uint type)
+  {
+    TSReleaseAssert(type < RATE_LIMITER_TYPE_MAX);
+    memset(_metrics, 0, sizeof(_metrics));
+
+    std::string metric_prefix = prefix;
+    metric_prefix.append("." + std::string(types[type]));
+
+    if (!tag.empty()) {
+      metric_prefix.append("." + tag);
+    } else if (!description.empty()) {
+      metric_prefix.append("." + description);
+    }
+
+    for (int i = 0; i < RATE_LIMITER_METRIC_MAX; i++) {
+      size_t const metricsz = metric_prefix.length() + strlen(suffixes[i]) + 2; // padding for dot+terminator
+      char *const metric    = (char *)TSmalloc(metricsz);
+      snprintf(metric, metricsz, "%s.%s", metric_prefix.data(), suffixes[i]);
+
+      _metrics[i] = TS_ERROR;
+
+      if (TSStatFindName(metric, &_metrics[i]) == TS_ERROR) {
+        _metrics[i] = TSStatCreate(metric, TS_RECORDDATATYPE_INT, TS_STAT_NON_PERSISTENT, TS_STAT_SYNC_SUM);
+      }
+
+      if (_metrics[i] != TS_ERROR) {
+        TSDebug(PLUGIN_NAME, "established metric '%s' as ID %d", metric, _metrics[i]);
+      } else {
+        TSError("failed to create metric '%s'", metric);
+      }
+
+      TSfree(metric);
+    }
+  }
+
+  void
+  incrementMetric(uint metric)
+  {
+    if (_metrics[metric] != TS_ERROR) {
+      TSStatIntIncrement(_metrics[metric], 1);
+    }
+  }
+
   // Initialize a new instance of this rate limiter
   bool initialize(int argc, const char *argv[]);
 
@@ -147,6 +224,8 @@ public:
   unsigned max_queue                = UINT_MAX; // No queue limit, but if sets will give an immediate error if at max
   std::chrono::milliseconds max_age = std::chrono::milliseconds::zero(); // Max age (ms) in the queue
   std::string description           = "";
+  std::string prefix                = RATE_LIMITER_METRIC_PREFIX; // metric prefix, i.e.: plugin.rate_limiter
+  std::string tag                   = "";                         // optional tag to append to the prefix (prefix.tag)
 
 private:
   std::atomic<unsigned> _active = 0; // Current active number of txns. This has to always stay <= limit above
@@ -154,4 +233,6 @@ private:
 
   TSMutex _queue_lock, _active_lock; // Resource locks
   std::deque<QueueItem> _queue;      // Queue for the pending TXN's. ToDo: Should also move (see below)
+
+  int _metrics[RATE_LIMITER_METRIC_MAX];
 };
diff --git a/plugins/experimental/rate_limit/rate_limit.cc b/plugins/experimental/rate_limit/rate_limit.cc
index 8220f55d7..a3c94d094 100644
--- a/plugins/experimental/rate_limit/rate_limit.cc
+++ b/plugins/experimental/rate_limit/rate_limit.cc
@@ -29,7 +29,7 @@
 #include "sni_limiter.h"
 
 ///////////////////////////////////////////////////////////////////////////////
-// As a global plugin, things works a little difference since we don't setup
+// As a global plugin, things works a little different since we don't setup
 // per transaction or via remap.config.
 extern int gVCIdx;
 
@@ -113,6 +113,9 @@ TSRemapNewInstance(int argc, char *argv[], void **ih, char * /* errbuf ATS_UNUSE
 {
   TxnRateLimiter *limiter = new TxnRateLimiter();
 
+  // set the description based on the pristine remap URL prior to advancing the pointer below
+  limiter->description = getDescriptionFromUrl(argv[0]);
+
   // argv contains the "to" and "from" URLs. Skip the first so that the
   // second one poses as the program name.
   --argc;
diff --git a/plugins/experimental/rate_limit/sni_limiter.cc b/plugins/experimental/rate_limit/sni_limiter.cc
index d1a5e0586..b63c50b1d 100644
--- a/plugins/experimental/rate_limit/sni_limiter.cc
+++ b/plugins/experimental/rate_limit/sni_limiter.cc
@@ -54,12 +54,14 @@ sni_limit_cont(TSCont contp, TSEvent event, void *edata)
           TSVConnReenableEx(vc, TS_EVENT_ERROR);
           TSDebug(PLUGIN_NAME, "Rejecting connection, we're at capacity and queue is full");
           TSUserArgSet(vc, gVCIdx, nullptr);
+          limiter->incrementMetric(RATE_LIMITER_METRIC_REJECTED);
 
           return TS_ERROR;
         } else {
           TSUserArgSet(vc, gVCIdx, reinterpret_cast<void *>(limiter));
           limiter->push(vc, contp);
           TSDebug(PLUGIN_NAME, "Queueing the VC, we are at capacity");
+          limiter->incrementMetric(RATE_LIMITER_METRIC_QUEUED);
         }
       } else {
         // Not at limit on the handshake, we can re-enable
@@ -103,6 +105,8 @@ SniRateLimiter::initialize(int argc, const char *argv[])
     {const_cast<char *>("limit"), required_argument, nullptr, 'l'},
     {const_cast<char *>("queue"), required_argument, nullptr, 'q'},
     {const_cast<char *>("maxage"), required_argument, nullptr, 'm'},
+    {const_cast<char *>("prefix"), required_argument, nullptr, 'p'},
+    {const_cast<char *>("tag"), required_argument, nullptr, 't'},
     // EOF
     {nullptr, no_argument, nullptr, '\0'},
   };
@@ -120,6 +124,12 @@ SniRateLimiter::initialize(int argc, const char *argv[])
     case 'm':
       this->max_age = std::chrono::milliseconds(strtol(optarg, nullptr, 10));
       break;
+    case 'p':
+      this->prefix = std::string(optarg);
+      break;
+    case 't':
+      this->tag = std::string(optarg);
+      break;
     }
     if (opt == -1) {
       break;
diff --git a/plugins/experimental/rate_limit/sni_limiter.h b/plugins/experimental/rate_limit/sni_limiter.h
index ea3581b9b..3889a0819 100644
--- a/plugins/experimental/rate_limit/sni_limiter.h
+++ b/plugins/experimental/rate_limit/sni_limiter.h
@@ -35,6 +35,8 @@ public:
     limit     = src.limit;
     max_queue = src.max_queue;
     max_age   = src.max_age;
+    prefix    = src.prefix;
+    tag       = src.tag;
   }
 
   bool initialize(int argc, const char *argv[]);
diff --git a/plugins/experimental/rate_limit/sni_selector.cc b/plugins/experimental/rate_limit/sni_selector.cc
index 60fc2ee85..d41b4df06 100644
--- a/plugins/experimental/rate_limit/sni_selector.cc
+++ b/plugins/experimental/rate_limit/sni_selector.cc
@@ -41,6 +41,7 @@ sni_queue_cont(TSCont cont, TSEvent event, void *edata)
       (void)contp; // Ugly, but silences some compilers.
       TSDebug(PLUGIN_NAME, "SNI=%s: Enabling queued VC after %ldms", key.data(), static_cast<long>(delay.count()));
       TSVConnReenable(vc);
+      limiter->incrementMetric(RATE_LIMITER_METRIC_RESUMED);
     }
 
     // Kill any queued VCs if they are too old
@@ -55,6 +56,7 @@ sni_queue_cont(TSCont cont, TSEvent event, void *edata)
         (void)contp;
         TSDebug(PLUGIN_NAME, "Queued VC is too old (%ldms), erroring out", static_cast<long>(age.count()));
         TSVConnReenableEx(vc, TS_EVENT_ERROR);
+        limiter->incrementMetric(RATE_LIMITER_METRIC_EXPIRED);
       }
     }
   }
@@ -73,6 +75,8 @@ SniSelector::insert(std::string_view sni, SniRateLimiter *limiter)
     TSDebug(PLUGIN_NAME, "Added global limiter for SNI=%s (limit=%u, queue=%u, max_age=%ldms)", sni.data(), limiter->limit,
             limiter->max_queue, static_cast<long>(limiter->max_age.count()));
 
+    limiter->initializeMetrics(RATE_LIMITER_TYPE_SNI);
+
     return true;
   }
 
diff --git a/plugins/experimental/rate_limit/txn_limiter.cc b/plugins/experimental/rate_limit/txn_limiter.cc
index f5b0951e0..6e3366588 100644
--- a/plugins/experimental/rate_limit/txn_limiter.cc
+++ b/plugins/experimental/rate_limit/txn_limiter.cc
@@ -40,6 +40,7 @@ txn_limit_cont(TSCont cont, TSEvent event, void *edata)
 
   case TS_EVENT_HTTP_POST_REMAP:
     limiter->push(static_cast<TSHttpTxn>(edata), cont);
+    limiter->incrementMetric(RATE_LIMITER_METRIC_QUEUED);
     return TS_EVENT_NONE;
     break;
 
@@ -47,6 +48,7 @@ txn_limit_cont(TSCont cont, TSEvent event, void *edata)
     retryAfter(static_cast<TSHttpTxn>(edata), limiter->retry);
     TSContDestroy(cont); // We are done with this continuation now
     TSHttpTxnReenable(static_cast<TSHttpTxn>(edata), TS_EVENT_HTTP_CONTINUE);
+    limiter->incrementMetric(RATE_LIMITER_METRIC_REJECTED);
     return TS_EVENT_CONTINUE;
     break;
 
@@ -74,6 +76,7 @@ txn_queue_cont(TSCont cont, TSEvent event, void *edata)
     // Since this was a delayed transaction, we need to add the TXN_CLOSE hook to free the slot when done
     TSHttpTxnHookAdd(txnp, TS_HTTP_TXN_CLOSE_HOOK, contp);
     TSHttpTxnReenable(txnp, TS_EVENT_HTTP_CONTINUE);
+    limiter->incrementMetric(RATE_LIMITER_METRIC_RESUMED);
   }
 
   // Kill any queued txns if they are too old
@@ -90,6 +93,7 @@ txn_queue_cont(TSCont cont, TSEvent event, void *edata)
       TSHttpTxnStatusSet(txnp, static_cast<TSHttpStatus>(limiter->error));
       TSHttpTxnHookAdd(txnp, TS_HTTP_SEND_RESPONSE_HDR_HOOK, contp);
       TSHttpTxnReenable(txnp, TS_EVENT_HTTP_ERROR);
+      limiter->incrementMetric(RATE_LIMITER_METRIC_EXPIRED);
     }
   }
 
@@ -109,6 +113,8 @@ TxnRateLimiter::initialize(int argc, const char *argv[])
     {const_cast<char *>("retry"), required_argument, nullptr, 'r'},
     {const_cast<char *>("header"), required_argument, nullptr, 'h'},
     {const_cast<char *>("maxage"), required_argument, nullptr, 'm'},
+    {const_cast<char *>("prefix"), required_argument, nullptr, 'p'},
+    {const_cast<char *>("tag"), required_argument, nullptr, 't'},
     // EOF
     {nullptr, no_argument, nullptr, '\0'},
   };
@@ -135,6 +141,12 @@ TxnRateLimiter::initialize(int argc, const char *argv[])
     case 'h':
       this->header = optarg;
       break;
+    case 'p':
+      this->prefix = std::string(optarg);
+      break;
+    case 't':
+      this->tag = std::string(optarg);
+      break;
     }
     if (opt == -1) {
       break;
@@ -148,6 +160,8 @@ TxnRateLimiter::initialize(int argc, const char *argv[])
     _action = TSContScheduleEveryOnPool(_queue_cont, QUEUE_DELAY_TIME.count(), TS_THREAD_POOL_TASK);
   }
 
+  this->initializeMetrics(RATE_LIMITER_TYPE_REMAP);
+
   return true;
 }
 
diff --git a/plugins/experimental/rate_limit/utilities.cc b/plugins/experimental/rate_limit/utilities.cc
index c648d98c1..0838689c0 100644
--- a/plugins/experimental/rate_limit/utilities.cc
+++ b/plugins/experimental/rate_limit/utilities.cc
@@ -70,3 +70,49 @@ retryAfter(TSHttpTxn txnp, unsigned retry)
     }
   }
 }
+
+///////////////////////////////////////////////////////////////////////////////
+// Parse a URL to obtain a description for use with metrics when no user
+// provided tag is available. This is used by the remap side of the plugin,
+// while the SNI side uses the FQDN associated with each limiter instance
+// which is obtained from the list of SNIs in the global plugin configuration.
+//
+std::string
+getDescriptionFromUrl(const char *url)
+{
+  TSMBuffer const buf = TSMBufferCreate();
+  TSMLoc url_loc      = nullptr;
+
+  const int url_len = strlen(url);
+  std::string description;
+
+  if (TS_SUCCESS == TSUrlCreate(buf, &url_loc) && TS_PARSE_DONE == TSUrlParse(buf, url_loc, &url, url + url_len)) {
+    int host_len, scheme_len = 0;
+    const char *s  = TSUrlSchemeGet(buf, url_loc, &scheme_len);
+    const char *h  = TSUrlHostGet(buf, url_loc, &host_len);
+    const int port = TSUrlPortGet(buf, url_loc);
+
+    const std::string hostname = std::string(h, host_len);
+    const std::string scheme   = std::string(s, scheme_len);
+
+    TSDebug(PLUGIN_NAME, "scheme = %s, host = %s, port = %d", scheme.c_str(), hostname.c_str(), port);
+
+    description = scheme;
+    description.append(".");
+    description.append(hostname);
+
+    // only append the port when it is non-standard
+    if (!(strncmp(s, TS_URL_SCHEME_HTTP, scheme_len) == 0 && port == 80) &&
+        !(strncmp(s, TS_URL_SCHEME_HTTPS, scheme_len) == 0 && port == 443)) {
+      description.append(":" + std::to_string(port));
+    }
+  }
+
+  if (url_loc != nullptr) {
+    TSHandleMLocRelease(buf, nullptr, url_loc);
+  }
+
+  TSMBufferDestroy(buf);
+
+  return description;
+}
diff --git a/plugins/experimental/rate_limit/utilities.h b/plugins/experimental/rate_limit/utilities.h
index 0ff58bff3..e936912e6 100644
--- a/plugins/experimental/rate_limit/utilities.h
+++ b/plugins/experimental/rate_limit/utilities.h
@@ -26,3 +26,4 @@ constexpr char const PLUGIN_NAME[] = "rate_limit";
 
 void delayHeader(TSHttpTxn txnp, std::string &header, std::chrono::milliseconds delay);
 void retryAfter(TSHttpTxn txnp, unsigned retry);
+std::string getDescriptionFromUrl(const char *url);


[trafficserver] 02/02: Adds an IP reputation system to the SNI rate limiter (#8459)

Posted by zw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

zwoop pushed a commit to branch 9.2.x
in repository https://gitbox.apache.org/repos/asf/trafficserver.git

commit a6d1f24a429740baeb43e7c98a2a8eef5b85463b
Author: Leif Hedstrom <zw...@apache.org>
AuthorDate: Tue Apr 5 14:10:37 2022 -0600

    Adds an IP reputation system to the SNI rate limiter (#8459)
    
    (cherry picked from commit fac9ad15e388540c366234b4c8873059c3994c29)
---
 doc/admin-guide/plugins/rate_limit.en.rst        |  62 ++++-
 doc/static/images/sdk/SieveLRU.png               | Bin 0 -> 137860 bytes
 plugins/experimental/rate_limit/Makefile.inc     |   1 +
 plugins/experimental/rate_limit/ip_reputation.cc | 323 +++++++++++++++++++++++
 plugins/experimental/rate_limit/ip_reputation.h  | 236 +++++++++++++++++
 plugins/experimental/rate_limit/iprep_simu.cc    | 299 +++++++++++++++++++++
 plugins/experimental/rate_limit/sni_limiter.cc   | 104 +++++++-
 plugins/experimental/rate_limit/sni_limiter.h    |  22 ++
 plugins/experimental/rate_limit/sni_selector.cc  |  15 +-
 9 files changed, 1048 insertions(+), 14 deletions(-)

diff --git a/doc/admin-guide/plugins/rate_limit.en.rst b/doc/admin-guide/plugins/rate_limit.en.rst
index ef3f3246f..5d51ac895 100644
--- a/doc/admin-guide/plugins/rate_limit.en.rst
+++ b/doc/admin-guide/plugins/rate_limit.en.rst
@@ -30,6 +30,17 @@ The limit counters and queues are per remap rule only, i.e. there is
 (currently) no way to group transaction limits from different remap rules
 into a single rate limiter.
 
+.. Note::
+    This is still work in progress, in particularly the configuration and
+    the IP reputation system needs some work. In particular:
+
+    * We need a proper YAML configuration overall, allowing us to configure
+      better per service controls as well as sharing resources between remap
+      rules or SNI.
+    * We need reloadable configurations.
+    * The IP reputation currently only works with the global plugin settings.
+    * There is no support for adding allow listed IPs to the IP reputation.
+
 Remap Plugin
 ------------
 
@@ -96,7 +107,10 @@ Global Plugin
 -------------
 
 As a global plugin, the rate limiting currently applies only for TLS enabled
-connections, based on the SNI from the TLS handshake. The basic use is as::
+connections, based on the SNI from the TLS handshake. As a global plugin we
+also have the support of an IP reputation system, see below for configurations.
+
+The basic use is as::
 
     rate_limit.so SNI=www1.example.com,www2.example.com --limit=2 --queue=2 --maxage=10000
 
@@ -144,6 +158,37 @@ The following options are available:
    the plugin will use the FQDN of the SNI associated with each rate limiter instance
    created during plugin initialization.
 
+.. option:: --iprep_buckets
+   The number of LRU buckets to use for the IP reputation. A good number here
+   is 10, but can be configured. The reason for the different buckets is to
+   account for a pseudo-sorted list of IPs on the frequency seen. Too few buckets
+   will not be enough to keep such a sorting, rendering the algorithm useless. To
+   function in our setup, the number of buckets must be less than ``100``.
+
+.. option:: --iprep_bucketsize
+   This is the size of the largest LRU bucket (the `entry bucket`), `15` is a good
+   value. This is a power of 2, so `15` means the largest LRU can hold `32768` entries.
+   Note that this option must be bigger then the `--iprep_buckets` setting, for the
+   bucket halfing to function.
+
+.. option:: --iprep_maxage
+   This is used for aging out entries out of the LRU, the default is `0` which means
+   no aging happens. Even with no aging, entries will eventually fall out of buckets
+   because of the LRU mechanism that kicks in. The aging is here to make sure a spike
+   in traffic from an IP doesn't keep the entry for too long in the LRUs.
+
+.. option:: --iprep_permablock_limit
+   The minimum number of hits an IP must reach to get moved to the permanent bucket.
+   In this bucket, entries will stay for 2x
+
+.. option:: --iprep_permablock_pressure
+   This option specifies from which bucket an IP is allowed to move from into the
+   perma block bucket. A good value here is likely `0` or `1`, which is very conservative.
+
+.. option:: --iprep_permablock_maxage
+   Similar to `--iprep_maxage` above, but only applies to the long term (`perma-block`)
+   bucket. Default is `0`, which means no aging to this bucket is applied.
+
 Metrics
 -------
 Metric names are generated either using defaults or user-supplied values. In either
@@ -189,6 +234,21 @@ A user can specify their own prefixes and tags, but not types or metrics.
    ``resumed``    Queued connection is resumed.
    ============== ===================================================================
 
+IP Reputation
+-------------
+
+The goal of the IP reputation system is to simply try to identify IPs which are more
+likely to be abusive than others. It's not a perfect system, and it relies heavily on
+the notion of pressure. The Sieve LRUs are always filled, so you have to make sure that
+you only start using them when the system thinks it's under pressure.
+
+The Sieve LRU is a chained set of (configurable) LRUs, each with smaller and smaller
+capacity. This essentially adds a notion of partially sorted elements; All IPs in
+LRU <n> generally are more active than the IPs in LRU <n+1>. LRU is specially marked
+for longer term blocking, only the most abusive elements would end up here.
+
+.. figure:: /static/images/sdk/SieveLRU.png
+
 Examples
 --------
 
diff --git a/doc/static/images/sdk/SieveLRU.png b/doc/static/images/sdk/SieveLRU.png
new file mode 100644
index 000000000..3e138e46d
Binary files /dev/null and b/doc/static/images/sdk/SieveLRU.png differ
diff --git a/plugins/experimental/rate_limit/Makefile.inc b/plugins/experimental/rate_limit/Makefile.inc
index 72469de5c..95ab01f43 100644
--- a/plugins/experimental/rate_limit/Makefile.inc
+++ b/plugins/experimental/rate_limit/Makefile.inc
@@ -21,4 +21,5 @@ experimental_rate_limit_rate_limit_la_SOURCES = \
   experimental/rate_limit/txn_limiter.cc \
   experimental/rate_limit/sni_limiter.cc \
   experimental/rate_limit/sni_selector.cc \
+  experimental/rate_limit/ip_reputation.cc \
   experimental/rate_limit/utilities.cc
diff --git a/plugins/experimental/rate_limit/ip_reputation.cc b/plugins/experimental/rate_limit/ip_reputation.cc
new file mode 100644
index 000000000..129cd9584
--- /dev/null
+++ b/plugins/experimental/rate_limit/ip_reputation.cc
@@ -0,0 +1,323 @@
+/** @file
+
+  Implementation details for the IP reputation classes.
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+
+#include <iostream>
+#include <cmath>
+
+#include "ip_reputation.h"
+
+namespace IpReputation
+{
+// These static class members are here to calculate a uint64_t hash of an IP
+uint64_t
+SieveLru::hasher(const sockaddr *sock)
+{
+  switch (sock->sa_family) {
+  case AF_INET: {
+    const sockaddr_in *sa = reinterpret_cast<const sockaddr_in *>(sock);
+
+    return (0xffffffff00000000 | sa->sin_addr.s_addr);
+  } break;
+  case AF_INET6: {
+    const sockaddr_in6 *sa6 = reinterpret_cast<const sockaddr_in6 *>(sock);
+
+    return (*reinterpret_cast<uint64_t const *>(sa6->sin6_addr.s6_addr) ^
+            *reinterpret_cast<uint64_t const *>(sa6->sin6_addr.s6_addr + sizeof(uint64_t)));
+  } break;
+  default:
+    // Clearly shouldn't happen ...
+    return 0;
+    break;
+  }
+}
+
+uint64_t
+SieveLru::hasher(const std::string &ip, u_short family) // Mostly a convenience function for testing
+{
+  switch (family) {
+  case AF_INET: {
+    sockaddr_in sa4;
+
+    inet_pton(AF_INET, ip.c_str(), &(sa4.sin_addr));
+    sa4.sin_family = AF_INET;
+    return hasher(reinterpret_cast<const sockaddr *>(&sa4));
+  } break;
+  case AF_INET6: {
+    sockaddr_in6 sa6;
+
+    inet_pton(AF_INET6, ip.c_str(), &(sa6.sin6_addr));
+    sa6.sin6_family = AF_INET6;
+    return hasher(reinterpret_cast<const sockaddr *>(&sa6));
+  } break;
+  default:
+    // Really shouldn't happen ...
+    return 0;
+  }
+}
+// Constructor, setting up the pre-sized LRU buckets etc.
+SieveLru::SieveLru(uint32_t num_buckets, uint32_t size) : _lock(TSMutexCreate())
+{
+  initialize(num_buckets, size);
+}
+
+// Initialize the Sieve LRU object
+void
+SieveLru::initialize(uint32_t num_buckets, uint32_t size)
+{
+  TSMutexLock(_lock);
+  TSAssert(!_initialized);             // Don't allow it to be initialized more than once!
+  TSReleaseAssert(size > num_buckets); // Otherwise we can't half the bucket sizes
+
+  _initialized = true;
+  _num_buckets = num_buckets;
+  _size        = size;
+
+  uint32_t cur_size = pow(2, 1 + _size - num_buckets);
+
+  _map.reserve(pow(2, size + 2));     // Allow for all the sieve LRUs, and extra room for the allow list
+  _buckets.reserve(_num_buckets + 2); // Two extra buckets, for the block list and allow list
+
+  // Create the other buckets, in smaller and smaller sizes (power of 2)
+  for (uint32_t i = lastBucket(); i <= entryBucket(); ++i) {
+    _buckets[i] = new SieveBucket(cur_size);
+    cur_size *= 2;
+  }
+
+  _buckets[blockBucket()] = new SieveBucket(cur_size / 2); // Block LRU, same size as entry bucket
+  _buckets[allowBucket()] = new SieveBucket(0);            // Allow LRU, this is unlimited
+  TSMutexUnlock(_lock);
+}
+
+// Increment the count for an element (will be created / added if new).
+std::tuple<uint32_t, uint32_t>
+SieveLru::increment(KeyClass key)
+{
+  TSMutexLock(_lock);
+  TSAssert(_initialized);
+
+  auto map_it = _map.find(key);
+
+  if (_map.end() == map_it) {
+    // This is a new entry, this can only be added to the last LRU bucket
+    SieveBucket *lru = _buckets[entryBucket()];
+
+    if (lru->full()) { // The LRU is full, replace the last item with a new one
+      auto last                                 = std::prev(lru->end());
+      auto &[l_key, l_count, l_bucket, l_added] = *last;
+
+      lru->moveTop(lru, last);
+      _map.erase(l_key);
+      *last = {key, 1, entryBucket(), SystemClock::now()};
+    } else {
+      // Create a new entry, the date is not used now (unless perma blocked), but could be useful for aging out stale elements.
+      lru->push_front({key, 1, entryBucket(), SystemClock::now()});
+    }
+    _map[key] = lru->begin();
+    TSMutexUnlock(_lock);
+
+    return {entryBucket(), 1};
+  } else {
+    auto &[map_key, map_item]              = *map_it;
+    auto &[list_key, count, bucket, added] = *map_item;
+    auto lru                               = _buckets[bucket];
+    auto max_age                           = (bucket == blockBucket() ? _perma_max_age : _max_age);
+
+    // Check if the entry is older than max_age (if set), if so just move it to the entry bucket and restart
+    // Yes, this will move likely abusive IPs but they will earn back a bad reputation; The goal here is to
+    // not let "spiked" entries sit in small buckets indefinitely. It also cleans up the code. We only check
+    // the actual system time every 10 request for an IP, if traffic is less frequent than that, the LRU will
+    // age it out properly.
+    if ((_max_age > std::chrono::seconds::zero()) && ((count % 10) == 0) &&
+        (std::chrono::duration_cast<std::chrono::seconds>(SystemClock::now() - added) > max_age)) {
+      auto last_lru = _buckets[entryBucket()];
+
+      count >>= 3; // Age the count by a factor of 1/8th
+      bucket = entryBucket();
+      last_lru->moveTop(lru, map_item);
+    } else {
+      ++count;
+
+      if (bucket > lastBucket()) {         // Not in the smallest bucket, so we may promote
+        auto p_lru = _buckets[bucket - 1]; // Move to previous bucket
+
+        if (!p_lru->full()) {
+          p_lru->moveTop(lru, map_item);
+          --bucket;
+        } else {
+          auto p_item                               = std::prev(p_lru->end());
+          auto &[p_key, p_count, p_bucket, p_added] = *p_item;
+
+          if (p_count <= count) {
+            // Swap places on the two elements, moving both to the top of their respective LRU buckets
+            p_lru->moveTop(lru, map_item);
+            lru->moveTop(p_lru, p_item);
+            --bucket;
+            ++p_bucket;
+          }
+        }
+      } else {
+        // Just move it to the top of the current LRU
+        lru->moveTop(lru, map_item);
+      }
+    }
+    TSMutexUnlock(_lock);
+
+    return {bucket, count};
+  }
+}
+
+// Lookup the status of the IP in the current tables, without modifying anything
+std::tuple<uint32_t, uint32_t>
+SieveLru::lookup(KeyClass key) const
+{
+  TSMutexLock(_lock);
+  TSAssert(_initialized);
+
+  auto map_it = _map.find(key);
+
+  if (_map.end() == map_it) {
+    TSMutexUnlock(_lock);
+
+    return {0, entryBucket()}; // Nothing found, return 0 hits and the entry bucket #
+  } else {
+    auto &[map_key, map_item]              = *map_it;
+    auto &[list_key, count, bucket, added] = *map_item;
+
+    TSMutexUnlock(_lock);
+
+    return {bucket, count};
+  }
+}
+
+// A little helper function, to properly move an IP to one of the two special buckets,
+// allow-bucket and block-bucket.
+int32_t
+SieveLru::move_bucket(KeyClass key, uint32_t to_bucket)
+{
+  TSMutexLock(_lock);
+  TSAssert(_initialized);
+
+  auto map_it = _map.find(key);
+
+  if (_map.end() == map_it) {
+    // This is a new entry, add it directly to the special bucket
+    SieveBucket *lru = _buckets[to_bucket];
+
+    if (lru->full()) { // The LRU is full, replace the last item with a new one
+      auto last                                 = std::prev(lru->end());
+      auto &[l_key, l_count, l_bucket, l_added] = *last;
+
+      lru->moveTop(lru, last);
+      _map.erase(l_key);
+      *last = {key, 1, to_bucket, SystemClock::now()};
+    } else {
+      // Create a new entry
+      lru->push_front({key, 1, to_bucket, SystemClock::now()});
+    }
+    _map[key] = lru->begin();
+  } else {
+    auto &[map_key, map_item]              = *map_it;
+    auto &[list_key, count, bucket, added] = *map_item;
+    auto lru                               = _buckets[bucket];
+
+    if (bucket != to_bucket) { // Make sure it's not already blocked
+      auto move_lru = _buckets[to_bucket];
+
+      // Free a space for a new entry, if needed
+      if (move_lru->size() >= move_lru->max_size()) {
+        auto d_entry                              = std::prev(move_lru->end());
+        auto &[d_key, d_count, d_bucket, d_added] = *d_entry;
+
+        move_lru->erase(d_entry);
+        _map.erase(d_key);
+      }
+      move_lru->moveTop(lru, map_item); // Move the LRU item to the perma-blocks
+      bucket = to_bucket;
+      added  = SystemClock::now();
+    }
+  }
+  TSMutexUnlock(_lock);
+
+  return to_bucket; // Just as a convenience, return the destination bucket for this entry
+}
+
+void
+SieveLru::dump()
+{
+  TSMutexLock(_lock);
+  TSAssert(_initialized);
+
+  for (uint32_t i = 0; i < _num_buckets + 1; ++i) {
+    long long cnt = 0, sum = 0;
+    auto lru = _buckets[i];
+
+    std::cout << std::endl
+              << "Dumping bucket " << i << " (size=" << lru->size() << ", max_size=" << lru->max_size() << ")" << std::endl;
+    for (auto &it : *lru) {
+      auto &[key, count, bucket, added] = it;
+
+      ++cnt;
+      sum += count;
+#if 0
+      if (0 == i) { // Also dump the content of the top bucket
+        std::cout << "\t" << key << "; Count=" << count << ", Bucket=" << bucket << std::endl;
+      }
+#endif
+    }
+
+    std::cout << "\tAverage count=" << (cnt > 0 ? sum / cnt : 0) << std::endl;
+  }
+  TSMutexUnlock(_lock);
+}
+
+// Debugging tools, these memory sizes are best guesses to how much memory the containers will actually use
+size_t
+SieveBucket::memorySize() const
+{
+  size_t total = sizeof(SieveBucket);
+
+  total += size() * (2 * sizeof(void *) + sizeof(LruEntry)); // Double linked list + object
+
+  return total;
+}
+
+size_t
+SieveLru::memoryUsed() const
+{
+  TSMutexLock(_lock);
+  TSAssert(_initialized);
+
+  size_t total = sizeof(SieveLru);
+
+  for (uint32_t i = 0; i <= _num_buckets + 1; ++i) {
+    total += _buckets[i]->memorySize();
+  }
+
+  total += _map.size() * (sizeof(void *) + sizeof(SieveBucket::iterator));
+  total += _map.bucket_count() * (sizeof(size_t) + sizeof(void *));
+  TSMutexUnlock(_lock);
+
+  return total;
+}
+
+} // namespace IpReputation
diff --git a/plugins/experimental/rate_limit/ip_reputation.h b/plugins/experimental/rate_limit/ip_reputation.h
new file mode 100644
index 000000000..4abbbcaa0
--- /dev/null
+++ b/plugins/experimental/rate_limit/ip_reputation.h
@@ -0,0 +1,236 @@
+/** @file
+
+  Include file for all the IP reputation classes.
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+#pragma once
+
+#include <string>
+#include <cstdint>
+#include <tuple>
+#include <unordered_map>
+#include <list>
+#include <vector>
+#include <chrono>
+#include <arpa/inet.h>
+
+#include "ts/ts.h"
+
+namespace IpReputation
+{
+using KeyClass    = uint64_t;
+using SystemClock = std::chrono::system_clock;
+
+// Key / Count / bucket # (rank, 0-<n>) / time added
+using LruEntry = std::tuple<KeyClass, uint32_t, uint32_t, std::chrono::time_point<SystemClock>>;
+
+// This is a wrapper around a std::list which lets us size limit the list to a
+// certain size.
+class SieveBucket : public std::list<LruEntry>
+{
+public:
+  SieveBucket(uint32_t max_size) : _max_size(max_size) {}
+
+  bool
+  full() const
+  {
+    return (_max_size > 0 ? (size() >= _max_size) : false);
+  }
+
+  size_t
+  max_size() const
+  {
+    return _max_size;
+  }
+
+  // Move an element to the top of an LRU. This *can* move it from the current LRU (bucket)
+  // to another, when promoted to a higher rank.
+  void
+  moveTop(SieveBucket *source_lru, SieveBucket::iterator &item)
+  {
+    splice(begin(), *source_lru, item);
+  }
+
+  // Debugging tools
+  size_t memorySize() const;
+
+private:
+  uint32_t _max_size;
+};
+
+using HashMap = std::unordered_map<KeyClass, SieveBucket::iterator>; // The hash map for finding the entry
+
+// This is a concept / POC: Ranked LRU buckets
+//
+// Also, obviously the std::string here is not awesome, rather, we ought to save the
+// hashed value from the IP as the key (just like the hashed in cache_promote).
+class SieveLru
+{
+public:
+  SieveLru() : _lock(TSMutexCreate()){}; // The unitialized version
+  SieveLru(uint32_t num_buckets, uint32_t size);
+  ~SieveLru()
+  {
+    for (uint32_t i = 0; i <= _num_buckets + 1; ++i) { // Remember to delete the two special allow/block buckets too
+      delete _buckets[i];
+    }
+  }
+
+  void initialize(uint32_t num_buckets = 10, uint32_t size = 15);
+
+  // Return value is the bucket (0 .. num_buckets) that the IP is in, and the
+  // current count of "hits". The lookup version is similar, except it doesn't
+  // modify the status of the IP (read-only).
+  std::tuple<uint32_t, uint32_t> increment(KeyClass key);
+
+  std::tuple<uint32_t, uint32_t>
+  increment(const sockaddr *sock)
+  {
+    return increment(hasher(sock));
+  }
+
+  // Move an IP to the perm-block or perma-allow LRUs. A zero (default) maxage is indefinite (no timeout).
+  uint32_t
+  block(KeyClass key)
+  {
+    return move_bucket(key, blockBucket());
+  }
+
+  uint32_t
+  allow(KeyClass key)
+  {
+    return move_bucket(key, allowBucket());
+  }
+
+  uint32_t
+  block(const sockaddr *sock)
+  {
+    return move_bucket(hasher(sock), blockBucket());
+  }
+
+  uint32_t
+  allow(const sockaddr *sock)
+  {
+    return move_bucket(hasher(sock), allowBucket());
+  }
+
+  // Lookup the current state of an IP
+  std::tuple<uint32_t, uint32_t> lookup(KeyClass key) const;
+
+  std::tuple<uint32_t, uint32_t>
+  lookup(const sockaddr *sock) const
+  {
+    return lookup(hasher(sock));
+  }
+
+  // A helper function to hash an INET or INET6 sockaddr to a 64-bit hash.
+  static uint64_t hasher(const sockaddr *sock);
+  static uint64_t hasher(const std::string &ip, u_short family = AF_INET);
+
+  // Identifying some of the special buckets:
+  //
+  //   entryBucket == the highest bucket, where new IPs enter (also the biggest bucket)
+  //   lastBucket  == the last bucket, which is most likely to be abusive
+  //   blockBucket == the bucket where we "permanently" block bad IPs
+  //   allowBucket == the bucket where we "permanently" allow good IPs (can not be blocked)
+  uint32_t
+  entryBucket() const
+  {
+    return _num_buckets;
+  }
+
+  constexpr uint32_t
+  lastBucket() const
+  {
+    return 1;
+  }
+
+  constexpr uint32_t
+  blockBucket() const
+  {
+    return 0;
+  }
+
+  uint32_t
+  allowBucket() const
+  {
+    return _num_buckets + 1;
+  }
+
+  size_t
+  bucketSize(uint32_t bucket) const
+  {
+    if (bucket <= (_num_buckets + 1)) {
+      return _buckets[bucket]->size();
+    } else {
+      return 0;
+    }
+  }
+
+  bool
+  initialized() const
+  {
+    return _initialized;
+  }
+
+  // Aging getters and setters
+  std::chrono::seconds
+  maxAge() const
+  {
+    return _max_age;
+  }
+
+  std::chrono::seconds
+  permaMaxAge() const
+  {
+    return _perma_max_age;
+  }
+
+  void
+  maxAge(std::chrono::seconds maxage)
+  {
+    _max_age = maxage;
+  }
+
+  void
+  permaMaxAge(std::chrono::seconds maxage)
+  {
+    _perma_max_age = maxage;
+  }
+
+  // Debugging tool, dumps some info around the buckets
+  void dump();
+  size_t memoryUsed() const;
+
+protected:
+  int32_t move_bucket(KeyClass key, uint32_t to_bucket);
+
+private:
+  HashMap _map;
+  std::vector<SieveBucket *> _buckets;
+  uint32_t _num_buckets               = 10;                           // Leave this at 10 ...
+  uint32_t _size                      = 0;                            // Set this up to initialize
+  std::chrono::seconds _max_age       = std::chrono::seconds::zero(); // Aging time in the SieveLru (default off)
+  std::chrono::seconds _perma_max_age = std::chrono::seconds::zero(); // Aging time in the SieveLru for perma-blocks
+  bool _initialized                   = false;                        // If this has been properly initialized yet
+  TSMutex _lock;                                                      // The lock around all data access
+};
+
+} // namespace IpReputation
diff --git a/plugins/experimental/rate_limit/iprep_simu.cc b/plugins/experimental/rate_limit/iprep_simu.cc
new file mode 100644
index 000000000..887df68b8
--- /dev/null
+++ b/plugins/experimental/rate_limit/iprep_simu.cc
@@ -0,0 +1,299 @@
+
+/** @file
+
+  Simulator application, for testing the behavior of the SieveLRU. This does
+  not build as part of the system, but put here for future testing etc.
+
+  @section license License
+
+  Licensed to the Apache Software Foundation (ASF) under one
+  or more contributor license agreements.  See the NOTICE file
+  distributed with this work for additional information
+  regarding copyright ownership.  The ASF licenses this file
+  to you under the Apache License, Version 2.0 (the
+  "License"); you may not use this file except in compliance
+  with the License.  You may obtain a copy of the License at
+
+      http://www.apache.org/licenses/LICENSE-2.0
+
+  Unless required by applicable law or agreed to in writing, software
+  distributed under the License is distributed on an "AS IS" BASIS,
+  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+  See the License for the specific language governing permissions and
+  limitations under the License.
+ */
+#include <getopt.h>
+
+#include <vector>
+#include <fstream>
+#include <chrono>
+#include <iostream>
+#include <iomanip>
+#include <cstdint>
+
+// Yeh well, sue me, boost is useful here, and this is not part of the actual core code
+#include <boost/algorithm/string.hpp>
+
+#include "ip_reputation.h"
+
+// Convenience class declarations
+using IpMap  = std::unordered_map<IpReputation::KeyClass, std::tuple<int, bool>>; //  count / false = good, true = bad
+using IpList = std::vector<IpMap::iterator>;
+
+// Holds all command line options
+struct CmdConfigs {
+  uint32_t start_buckets, end_buckets, incr_buckets;
+  uint32_t start_size, end_size, incr_size;
+  uint32_t start_threshold, end_threshold, incr_threshold;
+  uint32_t start_permablock, end_permablock, incr_permablock;
+};
+
+///////////////////////////////////////////////////////////////////////////////
+// Command line options / parsing, returns the parsed and populate CmdConfig
+// structure (from above).
+//
+std::tuple<int32_t, int32_t, int32_t>
+splitArg(std::string str)
+{
+  int32_t start = 0, end = 0, incr = 1;
+  std::vector<std::string> results;
+
+  boost::split(results, str, [](char c) { return c == '-' || c == '/'; });
+
+  if (results.size() > 0) {
+    start = std::stoi(results[0]);
+    if (results.size() > 1) {
+      end = std::stoi(results[1]);
+      if (results.size() > 2) {
+        incr = std::stoi(results[2]);
+      }
+    } else {
+      end = start;
+    }
+  } else {
+    std::cerr << "Malformed argument: " << str << std::endl;
+  }
+
+  return {start, end, incr};
+}
+
+CmdConfigs
+parseArgs(int argc, char **argv)
+{
+  CmdConfigs options;
+  int c;
+  constexpr struct option long_options[] = {
+    {"help", no_argument, NULL, 'h'},       {"buckets", required_argument, NULL, 'b'},   {"perma", required_argument, NULL, 'p'},
+    {"size", required_argument, NULL, 's'}, {"threshold", required_argument, NULL, 't'}, {NULL, 0, NULL, 0}};
+
+  // Make sure the optional values have been set
+
+  options.start_permablock = 0;
+  options.end_permablock   = 0;
+  options.incr_permablock  = 1;
+
+  while (1) {
+    int ix = 0;
+
+    c = getopt_long(argc, argv, "b:f:p:s:t:h?", long_options, &ix);
+    if (c == -1)
+      break;
+
+    switch (c) {
+    case 'h':
+    case '?':
+      std::cerr << "usage: iprep_simu -b|--buckets <size>[-<end bucket range>[/<increment>]]" << std::endl;
+      std::cerr << "                  -s|--size <bucket size>[-<end bucket size range>[/<increment>]]" << std::endl;
+      std::cerr << "                  -t|--threshold <bucket num>[-<end bucket num range>[/<increment>]]" << std::endl;
+      std::cerr << "                  [-p|--perma <permablock>[-<end permablock range>[/<increment>]]]" << std::endl;
+      std::cerr << "                 [-h|--help" << std::endl;
+      exit(0);
+      break;
+    case 'b':
+      std::tie(options.start_buckets, options.end_buckets, options.incr_buckets) = splitArg(optarg);
+      break;
+    case 's':
+      std::tie(options.start_size, options.end_size, options.incr_size) = splitArg(optarg);
+      break;
+    case 'p':
+      std::tie(options.start_permablock, options.end_permablock, options.incr_permablock) = splitArg(optarg);
+      break;
+    case 't':
+      std::tie(options.start_threshold, options.end_threshold, options.incr_threshold) = splitArg(optarg);
+      break;
+    default:
+      fprintf(stderr, "getopt returned weird stuff: 0%o\n", c);
+      exit(-1);
+      break;
+    }
+  }
+
+  return options; // RVO
+}
+
+///////////////////////////////////////////////////////////////////////////////
+// Load a configuration file, and populate the two structures with the
+// list of IPs (and their status) as well as the full sequence of requests.
+//
+// Returns a tuple with the number of good requests and bad requests, respectively.
+//
+std::tuple<uint32_t, uint32_t>
+loadFile(std::string fname, IpMap &all_ips, IpList &ips)
+{
+  std::ifstream infile(fname);
+
+  float timestamp; // The timestamp from the request(relative)
+  std::string ip;  // The IP
+  bool status;     // Bad (false) or Good (true) request?
+
+  uint32_t good_ips      = 0;
+  uint32_t bad_ips       = 0;
+  uint32_t good_requests = 0;
+  uint32_t bad_requests  = 0;
+
+  // Load in the entire file, and fill the request vector as well as the IP lookup table (state)
+  while (infile >> timestamp >> ip >> status) {
+    auto ip_hash = IpReputation::SieveLru::hasher(ip, ip.find(':') != std::string::npos ? AF_INET6 : AF_INET);
+    auto it      = all_ips.find(ip_hash);
+
+    if (!status) {
+      ++good_requests;
+    } else {
+      ++bad_requests;
+    }
+
+    if (all_ips.end() != it) {
+      auto &[key, data]       = *it;
+      auto &[count, d_status] = data;
+
+      ++count;
+      ips.push_back(it);
+    } else {
+      all_ips[ip_hash] = {0, status};
+      ips.push_back(all_ips.find(ip_hash));
+      if (!status) {
+        ++good_ips;
+      } else {
+        ++bad_ips;
+      }
+    }
+  }
+
+  std::cout << std::setprecision(3);
+  std::cout << "Total number of requests: " << ips.size() << std::endl;
+  std::cout << "\tGood requests: " << good_requests << " (" << 100.0 * good_requests / ips.size() << "%)" << std::endl;
+  std::cout << "\tBad requests: " << bad_requests << " (" << 100.0 * bad_requests / ips.size() << "%)" << std::endl;
+  std::cout << "Unique IPs in set: " << all_ips.size() << std::endl;
+  std::cout << "\tGood IPs: " << good_ips << " (" << 100.0 * good_ips / all_ips.size() << "%)" << std::endl;
+  std::cout << "\tBad IPs: " << bad_ips << " (" << 100.0 * bad_ips / all_ips.size() << "%)" << std::endl;
+  std::cout << std::endl;
+
+  return {good_requests, bad_requests};
+}
+
+int
+main(int argc, char *argv[])
+{
+  auto options = parseArgs(argc, argv);
+
+  // All remaining arguments should be files, so lets process them one by one
+  for (int file_num = optind; file_num < argc; ++file_num) {
+    IpMap all_ips;
+    IpList ips;
+
+    // Load the data from file
+    auto [good_requests, bad_requests] = loadFile(argv[file_num], all_ips, ips);
+
+    // Here starts the actual simulation, loop through variations
+    for (uint32_t size = options.start_size; size <= options.end_size; size += options.incr_size) {
+      for (uint32_t buckets = options.start_buckets; buckets <= options.end_buckets; buckets += options.incr_buckets) {
+        for (uint32_t threshold = options.start_threshold; threshold <= options.end_threshold;
+             threshold += options.incr_threshold) {
+          for (uint32_t permablock = options.start_permablock; permablock <= options.end_permablock;
+               permablock += options.incr_permablock) {
+            // Setup the buckets and metrics for this loop
+            IpReputation::SieveLru ipt(buckets, size);
+
+            auto start = std::chrono::system_clock::now();
+
+            // Some metrics
+            uint32_t good_blocked      = 0;
+            uint32_t good_allowed      = 0;
+            uint32_t bad_blocked       = 0;
+            uint32_t bad_allowed       = 0;
+            uint32_t good_perm_blocked = 0;
+            uint32_t bad_perm_blocked  = 0;
+
+            for (auto iter : ips) {
+              auto &[ip, data]       = *iter;
+              auto &[count, status]  = data;
+              auto [bucket, cur_cnt] = ipt.increment(ip);
+
+              // Currently we only allow perma-blocking on items in bucket 1, so check for that first.
+              if (cur_cnt > permablock && bucket == ipt.lastBucket()) {
+                bucket = ipt.block(ip);
+              }
+
+              if (bucket == ipt.blockBucket()) {
+                if (!status) {
+                  ++good_perm_blocked;
+                } else {
+                  ++bad_perm_blocked;
+                }
+              } else if (bucket <= threshold) {
+                if (!status) {
+                  ++good_blocked;
+                } else {
+                  ++bad_blocked;
+                }
+              } else {
+                if (!status) {
+                  ++good_allowed;
+                } else {
+                  ++bad_allowed;
+                }
+              }
+            }
+
+            auto end = std::chrono::system_clock::now();
+
+            uint32_t total_blocked      = bad_blocked + good_blocked;
+            uint32_t total_perm_blocked = bad_perm_blocked + good_perm_blocked;
+            uint32_t total_allowed      = bad_allowed + good_allowed;
+
+            // ipt.dump();
+
+            std::chrono::duration<double> elapsed_seconds = end - start;
+
+            std::cout << "Running with size=" << size << ", buckets=" << buckets << ", threshold=" << threshold
+                      << ", permablock=" << permablock << std::endl;
+            std::cout << "Processing time: " << elapsed_seconds.count() << std::endl;
+            std::cout << "Denied requests: " << total_blocked + total_perm_blocked << std::endl;
+            std::cout << "\tGood requests denied: " << good_blocked + good_perm_blocked << " ("
+                      << 100.0 * (good_blocked + good_perm_blocked) / good_requests << "%)" << std::endl;
+            std::cout << "\tBad requests denied: " << bad_blocked + bad_perm_blocked << " ("
+                      << 100.0 * (bad_blocked + bad_perm_blocked) / bad_requests << "%)" << std::endl;
+            std::cout << "Allowed requests: " << total_allowed << std::endl;
+            std::cout << "\tGood requests allowed: " << good_allowed << " (" << 100.0 * good_allowed / good_requests << "%)"
+                      << std::endl;
+            std::cout << "\tBad requests allowed: " << bad_allowed << " (" << 100.0 * bad_allowed / bad_requests << "%)"
+                      << std::endl;
+            if (permablock) {
+              std::cout << "Permanently blocked IPs: " << ipt.bucketSize(ipt.blockBucket()) << std::endl;
+              std::cout << "\tGood requests permanently denied: " << good_perm_blocked << " ("
+                        << 100.0 * good_perm_blocked / good_requests << "%)" << std::endl;
+              std::cout << "\tBad requests permanently denied: " << bad_perm_blocked << " ("
+                        << 100.0 * bad_perm_blocked / bad_requests << "%)" << std::endl;
+            }
+            std::cout << "Estimated score (lower is better): "
+                      << 100.0 * ((100.0 * good_blocked / good_requests + 100.0 * bad_allowed / bad_requests) /
+                                  (100.0 * good_allowed / good_requests + 100.0 * bad_blocked / bad_requests))
+                      << std::endl;
+            std::cout << "Memory used for IP Reputation data: " << ipt.memoryUsed() / (1024.0 * 1024.0) << "MB" << std::endl
+                      << std::endl;
+          }
+        }
+      }
+    }
+  }
+}
diff --git a/plugins/experimental/rate_limit/sni_limiter.cc b/plugins/experimental/rate_limit/sni_limiter.cc
index b63c50b1d..fcc8fb838 100644
--- a/plugins/experimental/rate_limit/sni_limiter.cc
+++ b/plugins/experimental/rate_limit/sni_limiter.cc
@@ -43,18 +43,58 @@ sni_limit_cont(TSCont contp, TSEvent event, void *edata)
     int len;
     const char *server_name = TSVConnSslSniGet(vc, &len);
     std::string_view sni_name(server_name, len);
+    SniRateLimiter *limiter = selector->find(sni_name);
 
-    if (!sni_name.empty()) { // This should likely always succeed, but without it we can't do anything
-      SniRateLimiter *limiter = selector->find(sni_name);
+    if (limiter) {
+      // Check if we have an IP reputation for this SNI, and if we should block
+      if (limiter->iprep.initialized()) {
+        const sockaddr *sock = TSNetVConnRemoteAddrGet(vc);
+        int pressure         = limiter->pressure();
+
+        TSDebug(PLUGIN_NAME, "CLIENT_HELLO on %.*s, pressure=%d", static_cast<int>(sni_name.length()), sni_name.data(), pressure);
+
+        // TSDebug(PLUGIN_NAME, "IP Reputation: pressure is currently %d", pressure);
+
+        if (pressure >= 0) { // When pressure is < 0, we're not yet at a level of pressure to be concerned about
+          char client_ip[INET6_ADDRSTRLEN] = "[unknown]";
+          auto [bucket, cur_cnt]           = limiter->iprep.increment(sock);
+
+          // Get the client IP string if debug is enabled
+          if (TSIsDebugTagSet(PLUGIN_NAME)) {
+            if (sock->sa_family == AF_INET) {
+              inet_ntop(AF_INET, &(((struct sockaddr_in *)sock)->sin_addr), client_ip, INET_ADDRSTRLEN);
+            } else if (sock->sa_family == AF_INET6) {
+              inet_ntop(AF_INET6, &(((struct sockaddr_in6 *)sock)->sin6_addr), client_ip, INET6_ADDRSTRLEN);
+            }
+          }
+
+          if (cur_cnt > limiter->iprep_permablock_count &&
+              bucket <= limiter->iprep_permablock_threshold) { // Mark for long-term blocking
+            TSDebug(PLUGIN_NAME, "Marking IP=%s for perma-blocking", client_ip);
+            bucket = limiter->iprep.block(sock);
+          }
+
+          if (static_cast<uint32_t>(pressure) > bucket) { // Remember the perma-block bucket is always 0, and we are >=0 already
+            // Block this IP from finishing the handshake
+            TSDebug(PLUGIN_NAME, "Rejecting connection from IP=%s, we're at pressure and IP was chosen to be blocked", client_ip);
+            TSUserArgSet(vc, gVCIdx, nullptr);
+            TSVConnReenableEx(vc, TS_EVENT_ERROR);
+
+            return TS_ERROR;
+          }
+        }
+      } else {
+        TSDebug(PLUGIN_NAME, "CLIENT_HELLO on %.*s, no IP reputation", static_cast<int>(sni_name.length()), sni_name.data());
+      }
 
-      TSDebug(PLUGIN_NAME, "CLIENT_HELLO on %.*s", static_cast<int>(sni_name.length()), sni_name.data());
-      if (limiter && !limiter->reserve()) {
+      // If we passed the IP reputation filter, continue rate limiting these connections
+      if (!limiter->reserve()) {
         if (!limiter->max_queue || limiter->full()) {
           // We are running at limit, and the queue has reached max capacity, give back an error and be done.
-          TSVConnReenableEx(vc, TS_EVENT_ERROR);
           TSDebug(PLUGIN_NAME, "Rejecting connection, we're at capacity and queue is full");
           TSUserArgSet(vc, gVCIdx, nullptr);
           limiter->incrementMetric(RATE_LIMITER_METRIC_REJECTED);
+          TSVConnReenableEx(vc, TS_EVENT_ERROR);
 
           return TS_ERROR;
         } else {
@@ -69,11 +109,11 @@ sni_limit_cont(TSCont contp, TSEvent event, void *edata)
         TSVConnReenable(vc);
       }
     } else {
+      // No limiter for this SNI at all, clear the args etc. just in case
+      TSUserArgSet(vc, gVCIdx, nullptr);
       TSVConnReenable(vc);
     }
-
-    break;
-  }
+  } break;
 
   case TS_EVENT_VCONN_CLOSE: {
     SniRateLimiter *limiter = static_cast<SniRateLimiter *>(TSUserArgGet(vc, gVCIdx));
@@ -107,10 +147,19 @@ SniRateLimiter::initialize(int argc, const char *argv[])
     {const_cast<char *>("maxage"), required_argument, nullptr, 'm'},
     {const_cast<char *>("prefix"), required_argument, nullptr, 'p'},
     {const_cast<char *>("tag"), required_argument, nullptr, 't'},
+    // These are all for the IP reputation system. ToDo: These should be global rather than per SNI ?
+    {const_cast<char *>("iprep_maxage"), required_argument, nullptr, 'a'},
+    {const_cast<char *>("iprep_buckets"), required_argument, nullptr, 'B'},
+    {const_cast<char *>("iprep_bucketsize"), required_argument, nullptr, 'S'},
+    {const_cast<char *>("iprep_permablock_limit"), required_argument, nullptr, 'L'},
+    {const_cast<char *>("iprep_permablock_pressure"), required_argument, nullptr, 'P'},
+    {const_cast<char *>("iprep_permablock_maxage"), required_argument, nullptr, 'A'},
     // EOF
     {nullptr, no_argument, nullptr, '\0'},
   };
 
+  TSDebug(PLUGIN_NAME, "Initializing an SNI Rate Limiter");
+
   while (true) {
     int opt = getopt_long(argc, const_cast<char *const *>(argv), "", longopt, nullptr);
 
@@ -130,11 +179,50 @@ SniRateLimiter::initialize(int argc, const char *argv[])
     case 't':
       this->tag = std::string(optarg);
       break;
+    case 'a':
+      this->_iprep_max_age = std::chrono::seconds(strtol(optarg, nullptr, 10));
+      break;
+    case 'B':
+      this->_iprep_num_buckets = strtol(optarg, nullptr, 10);
+      if (this->_iprep_num_buckets >= 100) {
+        TSError("sni_limiter: iprep_num_buckets must be in the range 1 .. 99, IP reputation disabled");
+        this->_iprep_num_buckets = 0;
+      }
+      break;
+    case 'S':
+      this->_iprep_size = strtol(optarg, nullptr, 10);
+      break;
+    case 'L':
+      this->iprep_permablock_count = strtol(optarg, nullptr, 10);
+      break;
+    case 'P':
+      this->iprep_permablock_threshold = strtol(optarg, nullptr, 10);
+      break;
+    case 'A':
+      this->_iprep_perma_max_age = std::chrono::seconds(strtol(optarg, nullptr, 10));
+      break;
     }
     if (opt == -1) {
       break;
     }
   }
 
+  // Enable and initialize the IP reputation if asked for
+  if (this->_iprep_num_buckets > 0 && this->_iprep_size > 0) {
+    TSDebug(PLUGIN_NAME, "Calling and _initialized is %d\n", this->iprep.initialized());
+    this->iprep.initialize(this->_iprep_num_buckets, this->_iprep_size);
+    TSDebug(PLUGIN_NAME, "IP-reputation enabled with %u buckets, max size is 2^%u", this->_iprep_num_buckets, this->_iprep_size);
+
+    TSDebug(PLUGIN_NAME, "Called and _initialized is %d\n", this->iprep.initialized());
+
+    // These settings are optional
+    if (this->_iprep_max_age != std::chrono::seconds::zero()) {
+      this->iprep.maxAge(this->_iprep_max_age);
+    }
+    if (this->_iprep_perma_max_age != std::chrono::seconds::zero()) {
+      this->iprep.permaMaxAge(this->_iprep_perma_max_age);
+    }
+  }
+
   return true;
 }
diff --git a/plugins/experimental/rate_limit/sni_limiter.h b/plugins/experimental/rate_limit/sni_limiter.h
index 3889a0819..93b1b5558 100644
--- a/plugins/experimental/rate_limit/sni_limiter.h
+++ b/plugins/experimental/rate_limit/sni_limiter.h
@@ -18,6 +18,7 @@
 #pragma once
 
 #include "limiter.h"
+#include "ip_reputation.h"
 #include "ts/ts.h"
 
 int sni_limit_cont(TSCont contp, TSEvent event, void *edata);
@@ -40,4 +41,25 @@ public:
   }
 
   bool initialize(int argc, const char *argv[]);
+
+  // ToDo: this ought to go into some better global IP reputation pool / settings. Waiting for YAML...
+  IpReputation::SieveLru iprep;
+  uint32_t iprep_permablock_count     = 0; // "Hits" limit for blocking permanently
+  uint32_t iprep_permablock_threshold = 0; // Pressure threshold for permanent block
+
+  // Calculate the pressure, which is either a negative number (ignore), or a number 0-<buckets>.
+  // 0 == block only perma-blocks.
+  int32_t
+  pressure() const
+  {
+    return ((active() - 1) / static_cast<float>(limit) * 100) - (99 - _iprep_num_buckets);
+  }
+
+private:
+  // ToDo: These should be moved to global configurations to have one shared IP Reputation.
+  // today the configuration of this is so klunky, that there is no easy way to make it "global".
+  std::chrono::seconds _iprep_max_age       = std::chrono::seconds::zero(); // Max age in the SieveLRUs for regular buckets
+  std::chrono::seconds _iprep_perma_max_age = std::chrono::seconds::zero(); // Max age in the SieveLRUs for perma-block buckets
+  uint32_t _iprep_num_buckets               = 10;                           // Number of buckets. ToDo: leave this at 10 always
+  uint32_t _iprep_size                      = 15;                           // Size of the biggest bucket; 15 == 2^15 == 32768
 };
diff --git a/plugins/experimental/rate_limit/sni_selector.cc b/plugins/experimental/rate_limit/sni_selector.cc
index d41b4df06..e4c22d02d 100644
--- a/plugins/experimental/rate_limit/sni_selector.cc
+++ b/plugins/experimental/rate_limit/sni_selector.cc
@@ -86,6 +86,10 @@ SniSelector::insert(std::string_view sni, SniRateLimiter *limiter)
 SniRateLimiter *
 SniSelector::find(std::string_view sni)
 {
+  if (sni.empty()) { // Likely shouldn't happen, but we can shortcircuit
+    return nullptr;
+  }
+
   auto limiter = _limiters.find(sni);
 
   if (limiter != _limiters.end()) {
@@ -105,17 +109,18 @@ SniSelector::factory(const char *sni_list, int argc, const char *argv[])
   char *saveptr;
   char *sni   = strdup(sni_list); // We make a copy of the sni list, to not touch the original string
   char *token = strtok_r(sni, ",", &saveptr);
-  SniRateLimiter def_limiter;
-
-  def_limiter.initialize(argc, argv); // Creates the template limiter
-  _needs_queue_cont = (def_limiter.max_queue > 0);
 
+  // Todo: We are repeating initializing here with the same configurations, but once we move this to
+  // YAML, and refactor this, it'll be better. And this is not particularly expensive.
   while (nullptr != token) {
-    SniRateLimiter *limiter = new SniRateLimiter(def_limiter); // Make a shallow copy
+    SniRateLimiter *limiter = new SniRateLimiter();
     TSReleaseAssert(limiter);
 
+    limiter->initialize(argc, argv);
     limiter->description = token;
 
+    _needs_queue_cont = (limiter->max_queue > 0);
+
     insert(std::string_view(limiter->description), limiter);
     token = strtok_r(nullptr, ",", &saveptr);
   }