make bngblaster sessions creation less bursty

[olivier-matz-6wind]

Is your feature request related to a problem? Please describe.

Currently, the creation of sessions in bngblaster happens every second. It is limited by start-rate and max-outstanding-sessions parameters. For instance, if both parameters are set to 1000, bngblaster will initiate a the creation of 1000 session at the first call of bbl_ctrl_job(), which means for PPPoE that a burst of 1000 PADI packets will be sent.

Receiving a burst like this is not realistic, and increases the risk of drops due to a full queue somewhere (on bngblaster NIC Tx queue, on device NIC Rx queue, on device socket queues, ...). The problem gets worst if the device sustain an high CPS: to measure a CPS of X, both start-rate and max-outstanding-sessions must be at least set to X, which causes a burst of X packets in the worst case.

Calling bbl_ctrl_job() more often (every 100ms, or less) would help to smooth the sessions creation, and decreases the risk of drops or timeouts.

Describe the solution you'd like

A made a draft patch that changes the behavior. With it, bbl_ctrl_job() is called every 100ms, but it can easily be configurable. I wanted to have some feedback about the proposal before going further.

--- a/code/bngblaster/src/bbl.c
+++ b/code/bngblaster/src/bbl.c
@@ -19,6 +19,8 @@
 #include "bbl_dhcp.h"
 #include "bbl_dhcpv6.h"
 
+static unsigned int ctrl_job_period_ns = 100 * 1000 * 1000;
+
 /* Global Context */
 bbl_ctx_s *g_ctx = NULL;
 
@@ -231,7 +233,6 @@ bbl_ctrl_job(timer_s *timer)
     bbl_interface_s *interface;
     bbl_network_interface_s *network_interface;
 
-    int rate = 0;
     uint32_t i;
 
     struct timespec timestamp;
@@ -288,11 +289,13 @@ bbl_ctrl_job(timer_s *timer)
             g_teardown_request = false;
         } else {
             /* Process teardown list in chunks. */
-            rate = g_ctx->config.sessions_stop_rate;
+            if(g_ctx->sessions_stop_credits < g_ctx->config.sessions_stop_period_ns)
+		g_ctx->sessions_stop_credits += ctrl_job_period_ns;
             while(!CIRCLEQ_EMPTY(&g_ctx->sessions_teardown_qhead)) {
                 session = CIRCLEQ_FIRST(&g_ctx->sessions_teardown_qhead);
-                if(rate > 0) {
-                    if(session->session_state != BBL_IDLE) rate--;
+                if(g_ctx->sessions_stop_credits >= g_ctx->config.sessions_stop_period_ns) {
+                    if(session->session_state != BBL_IDLE)
+                        g_ctx->sessions_stop_credits -= g_ctx->config.sessions_stop_period_ns;
                     bbl_session_clear(session);
                     /* Remove from teardown queue. */
                     CIRCLEQ_REMOVE(&g_ctx->sessions_teardown_qhead, session, session_teardown_qnode);
@@ -317,11 +320,12 @@ bbl_ctrl_job(timer_s *timer)
          * outstanding and setup rate. Sessions started will be removed
          * from idle list. */
         bbl_stats_update_cps();
-        rate = g_ctx->config.sessions_start_rate;
+	if(g_ctx->sessions_start_credits < g_ctx->config.sessions_start_period_ns)
+            g_ctx->sessions_start_credits += ctrl_job_period_ns;
         while(!CIRCLEQ_EMPTY(&g_ctx->sessions_idle_qhead)) {
             session = CIRCLEQ_FIRST(&g_ctx->sessions_idle_qhead);
-            if(rate > 0) {
-                rate--;
+            if(g_ctx->sessions_start_credits >= g_ctx->config.sessions_start_period_ns) {
+                g_ctx->sessions_start_credits -= g_ctx->config.sessions_start_period_ns;
                 if(g_ctx->sessions_outstanding < g_ctx->config.sessions_max_outstanding) {
                     g_ctx->sessions_outstanding++;
                     /* Start session */
@@ -595,7 +599,7 @@ main(int argc, char *argv[])
 
     /* Setup control job. */
     timer_add_periodic(&g_ctx->timer_root, &g_ctx->control_timer, "Control Timer", 
-                       1, 0, g_ctx, &bbl_ctrl_job);
+                       0, ctrl_job_period_ns, g_ctx, &bbl_ctrl_job);
 
     /* Setup control socket and job */
     if(g_ctx->ctrl_socket_path) {
diff --git a/code/bngblaster/src/bbl_config.c b/code/bngblaster/src/bbl_config.c
index 2cb7a04c89d4..d44a4cfd0d6b 100644
--- a/code/bngblaster/src/bbl_config.c
+++ b/code/bngblaster/src/bbl_config.c
@@ -3132,11 +3132,11 @@ json_parse_config(json_t *root)
         }
         JSON_OBJ_GET_NUMBER(section, value, "sessions", "start-rate", 1, 65535);
         if(value) {
-            g_ctx->config.sessions_start_rate = json_number_value(value);
+            g_ctx->config.sessions_start_period_ns = 1000000000UL / json_number_value(value);
         }
         JSON_OBJ_GET_NUMBER(section, value, "sessions", "stop-rate", 1, 65535);
         if(value) {
-            g_ctx->config.sessions_stop_rate = json_number_value(value);
+            g_ctx->config.sessions_stop_period_ns = 1000000000UL / json_number_value(value);
         }
         JSON_OBJ_GET_BOOL(section, value, "sessions", "iterate-vlan-outer");
         if(value) {
@@ -3223,12 +3223,12 @@ json_parse_config(json_t *root)
         JSON_OBJ_GET_NUMBER(section, value, "pppoe", "start-rate", 1, 65535);
         if(value) {
             fprintf(stderr, "JSON config warning: Deprecated configuration pppoe->start-rate\n");
-            g_ctx->config.sessions_start_rate = json_number_value(value);
+            g_ctx->config.sessions_start_period_ns = 1000000000UL / json_number_value(value);
         }
         JSON_OBJ_GET_NUMBER(section, value, "pppoe", "stop-rate", 1, 65535);
         if(value) {
             fprintf(stderr, "JSON config warning: Deprecated configuration pppoe->stop-rate\n");
-            g_ctx->config.sessions_stop_rate = json_number_value(value);
+            g_ctx->config.sessions_stop_period_ns = 1000000000UL / json_number_value(value);
         }
         /* ... Deprecated */
     
@@ -4494,8 +4494,8 @@ bbl_config_init_defaults()
     g_ctx->config.qdisc_bypass = true;
     g_ctx->config.sessions = 1;
     g_ctx->config.sessions_max_outstanding = 800;
-    g_ctx->config.sessions_start_rate = 400;
-    g_ctx->config.sessions_stop_rate = 400;
+    g_ctx->config.sessions_start_period_ns = 2500000; /* 400/s */
+    g_ctx->config.sessions_stop_period_ns = 2500000; /* 400/s */
     g_ctx->config.sessions_autostart = true;
     g_ctx->config.monkey_autostart = true;
     g_ctx->config.pppoe_discovery_timeout = 5;
diff --git a/code/bngblaster/src/bbl_ctx.h b/code/bngblaster/src/bbl_ctx.h
index fd5b411bcb2f..9eed4d141536 100644
--- a/code/bngblaster/src/bbl_ctx.h
+++ b/code/bngblaster/src/bbl_ctx.h
@@ -28,6 +28,9 @@ typedef struct bbl_ctx_
     struct timespec timestamp_resolved;
     struct timespec timestamp_established;
 
+    uint32_t sessions_start_credits;
+    uint32_t sessions_stop_credits;
+
     uint32_t interfaces;
     uint32_t sessions;
     uint32_t sessions_pppoe;
@@ -212,8 +215,8 @@ typedef struct bbl_ctx_
         /* Global Session Settings */
         uint32_t sessions;
         uint32_t sessions_max_outstanding;
-        uint16_t sessions_start_rate;
-        uint16_t sessions_stop_rate;
+        uint32_t sessions_start_period_ns;
+        uint32_t sessions_stop_period_ns;
         uint16_t sessions_start_delay;
         bool sessions_reconnect;
         bool sessions_autostart;

Describe alternatives you've considered

Increase all queue sizes on devices and tester.

Additional context

None.

[GIC-de]

This looks good but I need some time to review, so I created a draft merge request:
#321

[olivier-matz-6wind]

Hello @GIC-de,

Thank you for the comment. Please let me know if you want me to do any additional work on the patch.

Sorry I closed the issue by mistake, and I don't have the permission to reopen it...

[GIC-de]

I’ll be making a few minor improvements and will adjust the token granting to be more dynamic. Specifically, if the timer fires late (e.g. after more than 100 ms), the number of tokens will be calculated based on the actual time elapsed since the last execution. This is straightforward to implement using the timer library, which already provides both the expected and actual execution times. By computing the delta between these two and adding the interval, we can determine the correct amount of tokens to grant. I’m not yet sure whether this change will make it into the upcoming release, but I’ll do my best to include it.