CLI fix for long-lived sessions during high loads

When there is a continuos stream of CLI commands, cli_get_command()
always returns 1 (there is a new command). Anyway, the socket receive
buffer was reset only when there was no command at all, leading to a
strange behavior: after a while, the CLI receive buffer came to its end,
then read() was called with zero size buffer, it returned 0 which was
interpreted as EOF.

Fixing this by:
* resetting the buffer any time CLI RX gets to EOL
* explicitly refusing to pipeline

In future, we may implement CLI pipelining, yet to make it conveniently,
a push-parser may come handy instead of the current implementation.

diff --git a/doc/reply_codes b/doc/reply_codes
index 02f4e656084891aa21e41cdaa8c02746f4d2a156..6d2684943d79fd5921a1134c12784ab5d0b45a74 100644 (file)
--- a/doc/reply_codes
+++ b/doc/reply_codes
@@ -75,3 +75,4 @@ Reply codes of BIRD command-line interface
 9000   Command too long
 9001   Parse error
 9002   Invalid symbol type
+9003   Pipelining not supported

diff --git a/nest/cli.c b/nest/cli.c
index b54a0d76dc26dcaf5cf07bff0b150058a1237061..7b517cd20fe8b5b75b052f6df0f483357a291f5f 100644 (file)
--- a/nest/cli.c
+++ b/nest/cli.c
@@ -293,13 +293,20 @@ cli_event(void *data)
     c->cont(c);
   else
     {
-      err = cli_get_command(c);
-      if (!err)
-       return;
-      if (err < 0)
-       cli_printf(c, 9000, "Command too long");
-      else
-       cli_command(c);
+      switch (cli_get_command(c))
+      {
+       case CGC_OK:
+         cli_command(c);
+         break;
+       case CGC_INCOMPLETE:
+         return;
+       case CGC_TOO_LONG:
+         cli_printf(c, 9000, "Command too long");
+         break;
+       case CGC_TRAILING:
+         cli_printf(c, 9003, "Pipelining not supported");
+         break;
+      }
     }
 
   cli_write_trigger(c);

diff --git a/nest/cli.h b/nest/cli.h
index 8a3294c52224d5e618219ddd5d77b6f0e7831902..90442544eb49dd14f4b29a06238c7a8aaee58a66 100644 (file)
--- a/nest/cli.h
+++ b/nest/cli.h
@@ -29,7 +29,7 @@ typedef struct cli {
   node n;                              /* Node in list of all log hooks */
   pool *pool;
   void *priv;                          /* Private to sysdep layer */
-  byte *rx_buf, *rx_pos, *rx_aux;      /* sysdep */
+  byte *rx_buf, *rx_pos;               /* sysdep */
   struct cli_out *tx_buf, *tx_pos, *tx_write;
   event *event;
   void (*cont)(struct cli *c);
@@ -81,6 +81,13 @@ static inline int cli_access_restricted(void)
 /* Functions provided by sysdep layer */
 
 void cli_write_trigger(cli *);
-int cli_get_command(cli *);
+enum cli_get_command_result {
+  CGC_OK,
+  CGC_INCOMPLETE,
+  CGC_TOO_LONG,
+  CGC_TRAILING,
+};
+
+enum cli_get_command_result cli_get_command(cli *);
 
 #endif

diff --git a/sysdep/unix/main.c b/sysdep/unix/main.c
index 392aff9d8755208f58a709371ecc825d4ae057f6..02ee0ec6814dd0f720189962d7afad9849178bcd 100644 (file)
--- a/sysdep/unix/main.c
+++ b/sysdep/unix/main.c
@@ -430,11 +430,11 @@ cli_tx(sock *s)
   cli_write(s->data);
 }
 
-int
+enum cli_get_command_result
 cli_get_command(cli *c)
 {
   sock *s = c->priv;
-  byte *t = c->rx_aux ? : s->rbuf;
+  byte *t = s->rbuf;
   byte *tend = s->rpos;
   byte *d = c->rx_pos;
   byte *dend = c->rx_buf + CLI_RX_BUF_SIZE - 2;
@@ -445,18 +445,21 @@ cli_get_command(cli *c)
        t++;
       else if (*t == '\n')
        {
-         t++;
+         if (++t < tend)
+           return CGC_TRAILING;
+
+         s->rpos = s->rbuf;
          c->rx_pos = c->rx_buf;
-         c->rx_aux = t;
          *d = 0;
-         return (d < dend) ? 1 : -1;
+         return (d < dend) ? CGC_OK : CGC_TOO_LONG;
        }
       else if (d < dend)
        *d++ = *t++;
     }
-  c->rx_aux = s->rpos = s->rbuf;
+
+  s->rpos = s->rbuf;
   c->rx_pos = d;
-  return 0;
+  return CGC_INCOMPLETE;
 }
 
 static int
@@ -493,7 +496,6 @@ cli_connect(sock *s, uint size UNUSED)
   s->pool = c->pool;           /* We need to have all the socket buffers allocated in the cli pool */
   s->fast_rx = 1;
   c->rx_pos = c->rx_buf;
-  c->rx_aux = NULL;
   rmove(s, c->pool);
   return 1;
 }

diff --git a/test/birdtest.c b/test/birdtest.c
index 6ad743ceadaefd0e86a039b550318abb3a9151c0..2207ca6e2aa403380ce1296b98e3ebaa3ff7beb8 100644 (file)
--- a/test/birdtest.c
+++ b/test/birdtest.c
@@ -549,6 +549,6 @@ int sysdep_commit(struct config *new UNUSED, struct config *old UNUSED) { return
 void sysdep_shutdown_done(void) {}
 
 #include "nest/cli.h"
-int cli_get_command(cli *c UNUSED) { return 0; }
+enum cli_get_command_result cli_get_command(cli *c UNUSED) { return CGC_OK; }
 void cli_write_trigger(cli *c UNUSED) {}
 cli *cmd_reconfig_stored_cli;