2005-02-23 Andrew J. Schorr <ajschorr@alumni.princeton.edu>
* buffer.h: Make the struct buffer and struct buffer_data structures
private by moving them inside buffer.c. Add comments for all
functions. Rename buffer_write as buffer_put (to be more consistent
with the buffer_putc and buffer_putstr functions). Declare a new
buffer_write function that is used to write data to a file descriptor
and/or add it to the buffer queue. Remove unused function
buffer_flush_vty_all. Create a new enum typedef buffer_status_t
to be used as the return code for all buffer_flush* functions
and buffer_write.
* buffer.c: The struct buffer and struct buffer_data declarations
are now private to this file. In conjunction with that, remove
some unnecessary fields: struct buffer (alloc, unused_head,
unused_tail, length), struct buffer_data (prev).
(buffer_data_new) Removed: functionality incorporated into buffer_add.
(buffer_data_free) Removed: use a macro BUFFER_DATA_FREE instead.
(buffer_new) Use calloc instead of malloc + memset(zero).
Supply an appropriate default size if the specified size is 0.
(buffer_free) Eliminate code duplication by calling buffer_reset to
free the contents of the buffer (and remove unused code related
to unused_head).
(buffer_empty,buffer_putc,buffer_putstr) Aesthetic change (make more
compact).
(buffer_reset) Use macro BUFFER_DATA_FREE. No need to set
alloc and length to 0 (these fields have been removed).
(buffer_add) Fix scope to be static. Call XMALLOC directly instead
of calling removed buffer_data_new function. Simplify the logic
(since it's now a singly-linked list instead of doubly-linked).
(buffer_write) Renamed to buffer_put. Change to void, since return
code of 1 was meaningless. No need to adjust length field, since
it has been removed.
(buffer_putw,buffer_flush,buffer_flush_vty_all,buffer_flush_vty)
Remove unused functions.
(buffer_flush_all) Rewrite using buffer_flush_available to eliminate
a possible failure mode if IOV_MAX is less than the number of buffers
on the queue.
(buffer_flush_window) Incorporate logic from buffer_flush_vty.
Log an error message if there is a writev error.
(buffer_flush_available) Be more paranoid: check for case where
buffer is already empty. Use new ERRNO_IO_RETRY macro, and use
new enum for return codes. Simplify deletion logic (since it's
now a singly-linked list).
(buffer_write) New function for use with non-blocking I/O.
* vty.h: Replace the struct vty sb_buffer field with a fixed-size
(5-character) sb_buf field and an sb_len field, since using
a struct buffer was inappropriate for this task. Add some useful
comments about telnet window size negotiation.
* vty.c: Include <arpa/telnet.h> (no longer included by zebra.h).
Remove VTY_OBUF_SIZE (instead use buffer_new default size).
Make telnet_backward_char and telnet_space_char static const.
(vty_out) Replace buffer_write with buffer_put.
(vty_log_out) Check for I/O errors. If fatal, close the vty session.
Consolidate 3 separate writes into a single write call.
(vty_will_echo,vty_command,vty_next_line,vty_previous_line,
vty_end_config,vty_describe_fold,vty_clear_buf,vty_serv_sock_addrinfo,
vty_serv_sock_family,vty_serv_un,vty_use_backup_config,exec_timeout,
vty_config_write,vty_save_cwd) Fix scope to static.
(vty_new) Let buffer_new use its default buffer size.
(vty_write) Fix signature: 2nd arg should be const char *.
Replaced buffer_write with buffer_put.
(vty_telnet_option) Fix minor bug (window height or width greater than
255 was broken). Use sb_buf and sb_len instead of removed sb_buffer
(which was being used improperly).
(vty_read) On error, use ERRNO_IO_RETRY to decide whether it's fatal.
If the error is fatal, call buffer_reset so vty_close does not attempt
to flush the data. Use new sb_buf and sb_len instead of sb_buffer
to store the SB negotiation string.
(vty_flush) When vty->lines is 0, call buffer_flush_available instead
of buffer_flush_window. Look at the return code from buffer_flush
to detect I/O errors (and in that case, log an error message and
close the vty).
(vty_create) Fix scope to static. Initialize sb_len to 0 instead
of creating sb_buffer.
(vty_accept) Set socket nonblocking.
(vtysh_accept) Use new set_nonblocking function instead of calling
fcntl directly.
(vtysh_flush) New function called from vtysh_read (after command
execution) and from vtysh_write. This flushes the buffer
and reacts appropriately to the return code (by closing the vty
or scheduling further flushes).
(vtysh_read) Check whether error is fatal using ERRNO_IO_RETRY.
If not, just try again later. Otherwise, call buffer_reset before
calling vty_close (to avoid trying to flush the buffer in vty_close).
Fix logic to allow case where a command does not arrive atomically
in a single read call by checking for the terminating NUL char.
(vtysh_write) Use new vtysh_flush helper function.
(vty_close) No need to call buffer_empty, just call buffer_flush_all
in any case (it will check whether the buffer is empty).
Do not free sb_buffer (since it has been removed).
(vty_log_fixed) Use writev instead of write.
* zebra.h: Do not include <arpa/telnet.h>, since this is used only
by lib/vty.c.
diff --git a/lib/vty.c b/lib/vty.c
index 053e9ee..887e3ba 100644
--- a/lib/vty.c
+++ b/lib/vty.c
@@ -35,6 +35,9 @@
#include "filter.h"
#include "vty.h"
#include "privs.h"
+#include "network.h"
+
+#include <arpa/telnet.h>
/* Vty events */
enum event
@@ -50,9 +53,6 @@
#endif /* VTYSH */
};
-/* Minimum size of output buffers; to be rounded up to multiple of pagesize. */
-#define VTY_OBUF_SIZE 4096
-
static void vty_event (enum event, int, struct vty *);
/* Extern host structure from command.c */
@@ -137,7 +137,7 @@
p = buf;
/* Pointer p must point out buffer. */
- buffer_write (vty->obuf, (u_char *) p, len);
+ buffer_put (vty->obuf, (u_char *) p, len);
/* If p is not different with buf, it is allocated buffer. */
if (p != buf)
@@ -151,24 +151,38 @@
vty_log_out (struct vty *vty, const char *level, const char *proto_str,
const char *format, va_list va)
{
+ int ret;
int len;
char buf[1024];
if (level)
- snprintf (buf, sizeof buf, "%s: %s: ", level, proto_str);
+ len = snprintf(buf, sizeof(buf), "%s: %s: ", level, proto_str);
else
- snprintf (buf, sizeof buf, "%s: ", proto_str);
- write (vty->fd, buf, strlen (buf));
-
- len = vsnprintf (buf, sizeof buf, format, va);
- if (len < 0)
+ len = snprintf(buf, sizeof(buf), "%s: ", proto_str);
+ if ((len < 0) || ((size_t)len >= sizeof(buf)))
return -1;
- write (vty->fd, (u_char *)buf, len);
- snprintf (buf, sizeof buf, "\r\n");
- write (vty->fd, buf, 2);
+ if (((ret = vsnprintf(buf+len, sizeof(buf)-len, format, va)) < 0) ||
+ ((size_t)((len += ret)+2) > sizeof(buf)))
+ return -1;
- return len;
+ buf[len++] = '\r';
+ buf[len++] = '\n';
+
+ if (write(vty->fd, buf, len) < 0)
+ {
+ if (ERRNO_IO_RETRY(errno))
+ /* Kernel buffer is full, probably too much debugging output, so just
+ drop the data and ignore. */
+ return -1;
+ /* Fatal I/O error. */
+ zlog_warn("%s: write failed to vty client fd %d, closing: %s",
+ __func__, vty->fd, safe_strerror(errno));
+ buffer_reset(vty->obuf);
+ vty_close(vty);
+ return -1;
+ }
+ return 0;
}
/* Output current time to the vty. */
@@ -226,7 +240,7 @@
}
/* Send WILL TELOPT_ECHO to remote server. */
-void
+static void
vty_will_echo (struct vty *vty)
{
unsigned char cmd[] = { IAC, WILL, TELOPT_ECHO, '\0' };
@@ -272,12 +286,10 @@
vty_new ()
{
struct vty *new = XCALLOC (MTYPE_VTY, sizeof (struct vty));
- int pgsz = getpagesize();
- new->obuf = (struct buffer *) buffer_new ((((VTY_OBUF_SIZE-1)/pgsz)+1)*pgsz);
+ new->obuf = buffer_new(0); /* Use default buffer size. */
new->buf = XCALLOC (MTYPE_VTY, VTY_BUFSIZ);
new->max = VTY_BUFSIZ;
- new->sb_buffer = NULL;
return new;
}
@@ -349,7 +361,7 @@
}
/* Command execution over the vty interface. */
-int
+static int
vty_command (struct vty *vty, char *buf)
{
int ret;
@@ -385,18 +397,18 @@
return ret;
}
-char telnet_backward_char = 0x08;
-char telnet_space_char = ' ';
+static const char telnet_backward_char = 0x08;
+static const char telnet_space_char = ' ';
/* Basic function to write buffer to vty. */
static void
-vty_write (struct vty *vty, char *buf, size_t nbytes)
+vty_write (struct vty *vty, const char *buf, size_t nbytes)
{
if ((vty->node == AUTH_NODE) || (vty->node == AUTH_ENABLE_NODE))
return;
/* Should we do buffering here ? And make vty_flush (vty) ? */
- buffer_write (vty->obuf, (u_char *)buf, nbytes);
+ buffer_put (vty->obuf, buf, nbytes);
}
/* Ensure length of input buffer. Is buffer is short, double it. */
@@ -517,7 +529,7 @@
}
/* Show next command line history. */
-void
+static void
vty_next_line (struct vty *vty)
{
int try_index;
@@ -542,7 +554,7 @@
}
/* Show previous command line history. */
-void
+static void
vty_previous_line (struct vty *vty)
{
int try_index;
@@ -611,7 +623,7 @@
}
/* When '^Z' is received from vty, move down to the enable mode. */
-void
+static void
vty_end_config (struct vty *vty)
{
vty_out (vty, "%s", VTY_NEWLINE);
@@ -855,7 +867,7 @@
vector_only_index_free (matched);
}
-void
+static void
vty_describe_fold (struct vty *vty, int cmd_width,
unsigned int desc_width, struct desc *desc)
{
@@ -1012,7 +1024,7 @@
vty_redraw_line (vty);
}
-void
+static void
vty_clear_buf (struct vty *vty)
{
memset (vty->buf, 0, vty->max);
@@ -1146,36 +1158,41 @@
switch (buf[0])
{
case SB:
- buffer_reset(vty->sb_buffer);
+ vty->sb_len = 0;
vty->iac_sb_in_progress = 1;
return 0;
break;
case SE:
{
- char *buffer;
- int length;
-
if (!vty->iac_sb_in_progress)
return 0;
- buffer = (char *)vty->sb_buffer->head->data;
- length = vty->sb_buffer->length;
-
- if (buffer == NULL)
- return 0;
-
- if (buffer[0] == '\0')
+ if ((vty->sb_len == 0) || (vty->sb_buf[0] == '\0'))
{
vty->iac_sb_in_progress = 0;
return 0;
}
- switch (buffer[0])
+ switch (vty->sb_buf[0])
{
case TELOPT_NAWS:
- if (length < 5)
- break;
- vty->width = buffer[2];
- vty->height = vty->lines >= 0 ? vty->lines : buffer[4];
+ if (vty->sb_len != TELNET_NAWS_SB_LEN)
+ zlog_warn("RFC 1073 violation detected: telnet NAWS option "
+ "should send %d characters, but we received %lu",
+ TELNET_NAWS_SB_LEN, (u_long)vty->sb_len);
+ else if (sizeof(vty->sb_buf) < TELNET_NAWS_SB_LEN)
+ zlog_err("Bug detected: sizeof(vty->sb_buf) %lu < %d, "
+ "too small to handle the telnet NAWS option",
+ (u_long)sizeof(vty->sb_buf), TELNET_NAWS_SB_LEN);
+ else
+ {
+ vty->width = ((vty->sb_buf[1] << 8)|vty->sb_buf[2]);
+ vty->height = ((vty->sb_buf[3] << 8)|vty->sb_buf[4]);
+#ifdef TELNET_OPTION_DEBUG
+ vty_out(vty, "TELNET NAWS window size negotiation completed: "
+ "width %d, height %d%s",
+ vty->width, vty->height, VTY_NEWLINE);
+#endif
+ }
break;
}
vty->iac_sb_in_progress = 0;
@@ -1272,9 +1289,21 @@
vty->t_read = NULL;
/* Read raw data from socket */
- nbytes = read (vty->fd, buf, VTY_READ_BUFSIZ);
- if (nbytes <= 0)
- vty->status = VTY_CLOSE;
+ if ((nbytes = read (vty->fd, buf, VTY_READ_BUFSIZ)) <= 0)
+ {
+ if (nbytes < 0)
+ {
+ if (ERRNO_IO_RETRY(errno))
+ {
+ vty_event (VTY_READ, vty_sock, vty);
+ return 0;
+ }
+ zlog_warn("%s: read error on vty client fd %d, closing: %s",
+ __func__, vty->fd, safe_strerror(errno));
+ }
+ buffer_reset(vty->obuf);
+ vty->status = VTY_CLOSE;
+ }
for (i = 0; i < nbytes; i++)
{
@@ -1293,7 +1322,9 @@
if (vty->iac_sb_in_progress && !vty->iac)
{
- buffer_putc(vty->sb_buffer, buf[i]);
+ if (vty->sb_len < sizeof(vty->sb_buf))
+ vty->sb_buf[vty->sb_len] = buf[i];
+ vty->sb_len++;
continue;
}
@@ -1459,58 +1490,64 @@
vty_flush (struct thread *thread)
{
int erase;
+ buffer_status_t flushrc;
int vty_sock = THREAD_FD (thread);
struct vty *vty = THREAD_ARG (thread);
+
vty->t_write = NULL;
/* Tempolary disable read thread. */
- if (vty->lines == 0)
- if (vty->t_read)
- {
- thread_cancel (vty->t_read);
- vty->t_read = NULL;
- }
+ if ((vty->lines == 0) && vty->t_read)
+ {
+ thread_cancel (vty->t_read);
+ vty->t_read = NULL;
+ }
/* Function execution continue. */
- if (vty->status == VTY_MORE || vty->status == VTY_MORELINE)
- erase = 1;
- else
- erase = 0;
+ erase = ((vty->status == VTY_MORE || vty->status == VTY_MORELINE));
- if (vty->lines == 0)
- buffer_flush_window (vty->obuf, vty->fd, vty->width, 25, 0, 1);
- else if (vty->status == VTY_MORELINE)
- buffer_flush_window (vty->obuf, vty->fd, vty->width, 1, erase, 0);
- else
- buffer_flush_window (vty->obuf, vty->fd, vty->width,
- vty->lines >= 0 ? vty->lines : vty->height,
- erase, 0);
-
- if (buffer_empty (vty->obuf))
- {
- if (vty->status == VTY_CLOSE)
- vty_close (vty);
- else
- {
- vty->status = VTY_NORMAL;
-
- if (vty->lines == 0)
- vty_event (VTY_READ, vty_sock, vty);
- }
- }
+ /* N.B. if width is 0, that means we don't know the window size. */
+ if ((vty->lines == 0) || (vty->width == 0))
+ flushrc = buffer_flush_available(vty->obuf, vty->fd);
+ else if (vty->status == VTY_MORELINE)
+ flushrc = buffer_flush_window(vty->obuf, vty->fd, vty->width,
+ 1, erase, 0);
+ else
+ flushrc = buffer_flush_window(vty->obuf, vty->fd, vty->width,
+ vty->lines >= 0 ? vty->lines :
+ vty->height,
+ erase, 0);
+ switch (flushrc)
+ {
+ case BUFFER_ERROR:
+ zlog_warn("buffer_flush failed on vty client fd %d, closing",
+ vty->fd);
+ buffer_reset(vty->obuf);
+ vty_close(vty);
+ return 0;
+ case BUFFER_EMPTY:
+ if (vty->status == VTY_CLOSE)
+ vty_close (vty);
else
{
- vty->status = VTY_MORE;
-
+ vty->status = VTY_NORMAL;
if (vty->lines == 0)
- vty_event (VTY_WRITE, vty_sock, vty);
+ vty_event (VTY_READ, vty_sock, vty);
}
+ break;
+ case BUFFER_PENDING:
+ /* There is more data waiting to be written. */
+ vty->status = VTY_MORE;
+ if (vty->lines == 0)
+ vty_event (VTY_WRITE, vty_sock, vty);
+ break;
+ }
return 0;
}
/* Create new vty structure. */
-struct vty *
+static struct vty *
vty_create (int vty_sock, union sockunion *su)
{
struct vty *vty;
@@ -1545,7 +1582,7 @@
vty->lines = -1;
vty->iac = 0;
vty->iac_sb_in_progress = 0;
- vty->sb_buffer = buffer_new (1024);
+ vty->sb_len = 0;
if (! no_password_check)
{
@@ -1608,6 +1645,7 @@
zlog_warn ("can't accept vty socket : %s", safe_strerror (errno));
return -1;
}
+ set_nonblocking(vty_sock);
p = sockunion2hostprefix (&su);
@@ -1670,7 +1708,7 @@
}
#if defined(HAVE_IPV6) && !defined(NRL)
-void
+static void
vty_serv_sock_addrinfo (const char *hostname, unsigned short port)
{
int ret;
@@ -1736,7 +1774,7 @@
#endif /* HAVE_IPV6 && ! NRL */
/* Make vty server socket. */
-void
+static void
vty_serv_sock_family (const char* addr, unsigned short port, int family)
{
int ret;
@@ -1805,7 +1843,7 @@
#include <sys/un.h>
/* VTY shell UNIX domain socket. */
-void
+static void
vty_serv_un (const char *path)
{
int ret;
@@ -1879,7 +1917,6 @@
int accept_sock;
int sock;
int client_len;
- int flags;
struct sockaddr_un client;
struct vty *vty;
@@ -1899,12 +1936,10 @@
return -1;
}
- /* set to non-blocking*/
- if ( ((flags = fcntl (sock, F_GETFL)) == -1)
- || (fcntl (sock, F_SETFL, flags|O_NONBLOCK) == -1) )
+ if (set_nonblocking(sock) < 0)
{
- zlog_warn ("vtysh_accept: could not set vty socket to non-blocking,"
- " %s, closing", safe_strerror (errno));
+ zlog_warn ("vtysh_accept: could not set vty socket %d to non-blocking,"
+ " %s, closing", sock, safe_strerror (errno));
close (sock);
return -1;
}
@@ -1924,6 +1959,26 @@
}
static int
+vtysh_flush(struct vty *vty)
+{
+ switch (buffer_flush_available(vty->obuf, vty->fd))
+ {
+ case BUFFER_PENDING:
+ vty_event(VTYSH_WRITE, vty->fd, vty);
+ break;
+ case BUFFER_ERROR:
+ zlog_warn("%s: write error to fd %d, closing", __func__, vty->fd);
+ buffer_reset(vty->obuf);
+ vty_close(vty);
+ return -1;
+ break;
+ case BUFFER_EMPTY:
+ break;
+ }
+ return 0;
+}
+
+static int
vtysh_read (struct thread *thread)
{
int ret;
@@ -1931,15 +1986,26 @@
int nbytes;
struct vty *vty;
unsigned char buf[VTY_READ_BUFSIZ];
+ unsigned char *p;
u_char header[4] = {0, 0, 0, 0};
sock = THREAD_FD (thread);
vty = THREAD_ARG (thread);
vty->t_read = NULL;
- nbytes = read (sock, buf, VTY_READ_BUFSIZ);
- if (nbytes <= 0)
+ if ((nbytes = read (sock, buf, VTY_READ_BUFSIZ)) <= 0)
{
+ if (nbytes < 0)
+ {
+ if (ERRNO_IO_RETRY(errno))
+ {
+ vty_event (VTYSH_READ, sock, vty);
+ return 0;
+ }
+ zlog_warn("%s: read failed on vtysh client fd %d, closing: %s",
+ __func__, sock, safe_strerror(errno));
+ }
+ buffer_reset(vty->obuf);
vty_close (vty);
#ifdef VTYSH_DEBUG
printf ("close vtysh\n");
@@ -1948,28 +2014,35 @@
}
#ifdef VTYSH_DEBUG
- printf ("line: %s\n", buf);
+ printf ("line: %.*s\n", nbytes, buf);
#endif /* VTYSH_DEBUG */
- vty_ensure (vty, nbytes);
- memcpy (vty->buf, buf, nbytes);
-
- /* Pass this line to parser. */
- ret = vty_execute (vty);
+ for (p = buf; p < buf+nbytes; p++)
+ {
+ vty_ensure(vty, vty->length+1);
+ vty->buf[vty->length++] = *p;
+ if (*p == '\0')
+ {
+ /* Pass this line to parser. */
+ ret = vty_execute (vty);
+ /* Note that vty_execute clears the command buffer and resets
+ vty->length to 0. */
- vty_clear_buf (vty);
-
- /* Return result. */
+ /* Return result. */
#ifdef VTYSH_DEBUG
- printf ("result: %d\n", ret);
- printf ("vtysh node: %d\n", vty->node);
+ printf ("result: %d\n", ret);
+ printf ("vtysh node: %d\n", vty->node);
#endif /* VTYSH_DEBUG */
- header[3] = ret;
- buffer_write(vty->obuf, header, 4);
+ header[3] = ret;
+ buffer_put(vty->obuf, header, 4);
- if (!vty->t_write && buffer_flush_available(vty->obuf, vty->fd))
- vty_event (VTYSH_WRITE, vty->fd, vty);
+ if (!vty->t_write && (vtysh_flush(vty) < 0))
+ /* Try to flush results; exit if a write error occurs. */
+ return 0;
+ }
+ }
+
vty_event (VTYSH_READ, sock, vty);
return 0;
@@ -1981,8 +2054,7 @@
struct vty *vty = THREAD_ARG (thread);
vty->t_write = NULL;
- if (buffer_flush_available(vty->obuf, vty->fd))
- vty_event (VTYSH_WRITE, vty->fd, vty);
+ vtysh_flush(vty);
return 0;
}
@@ -2028,16 +2100,11 @@
thread_cancel (vty->t_timeout);
/* Flush buffer. */
- if (! buffer_empty (vty->obuf))
- buffer_flush_all (vty->obuf, vty->fd);
+ buffer_flush_all (vty->obuf, vty->fd);
/* Free input buffer. */
buffer_free (vty->obuf);
- /* Free SB buffer. */
- if (vty->sb_buffer)
- buffer_free (vty->sb_buffer);
-
/* Free command history. */
for (i = 0; i < VTY_MAXHIST; i++)
if (vty->hist[i])
@@ -2118,7 +2185,7 @@
vty_close (vty);
}
-FILE *
+static FILE *
vty_use_backup_config (char *fullpath)
{
char *fullpath_sav, *fullpath_tmp;
@@ -2298,16 +2365,20 @@
vty_log_fixed (const char *buf, size_t len)
{
unsigned int i;
+ struct iovec iov[2];
+
+ iov[0].iov_base = buf;
+ iov[0].iov_len = len;
+ iov[1].iov_base = "\r\n";
+ iov[1].iov_len = 2;
for (i = 0; i < vector_max (vtyvec); i++)
{
struct vty *vty;
- if ((vty = vector_slot (vtyvec, i)) != NULL)
- if (vty->monitor)
- {
- write(vty->fd, buf, len);
- write(vty->fd, "\r\n", 2);
- }
+ if (((vty = vector_slot (vtyvec, i)) != NULL) && vty->monitor)
+ /* N.B. We don't care about the return code, since process is
+ most likely just about to die anyway. */
+ writev(vty->fd, iov, 2);
}
}
@@ -2417,7 +2488,7 @@
}
/* Set time out value. */
-int
+static int
exec_timeout (struct vty *vty, const char *min_str, const char *sec_str)
{
unsigned long timeout = 0;
@@ -2635,7 +2706,7 @@
}
/* Display current configuration. */
-int
+static int
vty_config_write (struct vty *vty)
{
vty_out (vty, "line vty%s", VTY_NEWLINE);
@@ -2749,8 +2820,8 @@
}
}
-void
-vty_save_cwd ()
+static void
+vty_save_cwd (void)
{
char cwd[MAXPATHLEN];
char *c;