From 865ffe5b31ea16f760b62d39c97003ddae4ec7e4 Mon Sep 17 00:00:00 2001 From: Srujana Date: Tue, 30 Jul 2024 20:39:33 +0000 Subject: [PATCH] lib: Memory spike reduction for sh cmds at scale The output buffer vty->obuf is a linked list where each element is of 4KB. Currently, when a huge sh command like is executed on a large scale, all the vty_outs are processed and the entire data is accumulated. After the entire vty execution, vtysh_flush proceeses and puts this data in the socket (131KB at a time). Problem here is the memory spike for such heavy duty show commands. The fix here is to chunkify the output on VTY shell by flushing it intermediately for every 128 KB of output accumulated and free the memory allocated for the buffer data. This way, we achieve ~25-30% reduction in the memory spike. Fixes: #16498 Note: This is a continuation of MR #16498 Ticket :#4050898 Signed-off-by: Srujana Signed-off-by: Rajasekar Raja --- lib/vty.c | 29 +++++++++++++++++++++++++++++ lib/vty.h | 8 ++++++++ vtysh/vtysh.c | 17 +++++++++++++++++ vtysh/vtysh.h | 2 ++ 4 files changed, 56 insertions(+) diff --git a/lib/vty.c b/lib/vty.c index 9b35935a8dc7..cbf3bd6dd5b5 100644 --- a/lib/vty.c +++ b/lib/vty.c @@ -124,6 +124,8 @@ static char integrate_default[] = SYSCONFDIR INTEGRATE_DEFAULT_CONFIG; static bool do_log_commands; static bool do_log_commands_perm; +static int vtysh_flush(struct vty *vty); + void vty_frame(struct vty *vty, const char *format, ...) { va_list args; @@ -264,8 +266,18 @@ int vty_out(struct vty *vty, const char *format, ...) case VTY_SHELL_SERV: case VTY_FILE: default: + vty->vty_buf_size_accumulated += strlen(filtered); /* print without crlf replacement */ buffer_put(vty->obuf, (uint8_t *)filtered, strlen(filtered)); + /* For every chunk of memory, we invoke vtysh_flush where we + * put the data of collective vty->obuf Linked List items on the + * socket and free the vty->obuf data. + */ + if (vty->vty_buf_size_accumulated >= + VTY_MAX_INTERMEDIATE_FLUSH) { + vty->vty_buf_size_accumulated = 0; + vtysh_flush(vty); + } break; } @@ -1993,6 +2005,8 @@ static void vtysh_accept(struct thread *thread) int client_len; struct sockaddr_un client; struct vty *vty; + int ret = 0; + uint32_t sndbufsize = VTY_SEND_BUF_MAX; vty_event_serv(VTYSH_SERV, vtyserv); @@ -2016,6 +2030,20 @@ static void vtysh_accept(struct thread *thread) close(sock); return; } + + /* + * Increasing the SEND socket buffer size so that the socket can hold + * before sending it to VTY shell. + */ + ret = setsockopt(sock, SOL_SOCKET, SO_SNDBUF, (char *)&sndbufsize, + sizeof(sndbufsize)); + if (ret < 0) { + flog_err(EC_LIB_SOCKET, + "Cannot set socket %d send buffer size, %s", sock, + safe_strerror(errno)); + close(sock); + return; + } set_cloexec(sock); #ifdef VTYSH_DEBUG @@ -2102,6 +2130,7 @@ static int vtysh_flush(struct vty *vty) vty_close(vty, false); return -1; case BUFFER_EMPTY: + vty->vty_buf_size_accumulated = 0; break; } return 0; diff --git a/lib/vty.h b/lib/vty.h index 3454a5136c5f..e45da461bfd6 100644 --- a/lib/vty.h +++ b/lib/vty.h @@ -223,6 +223,8 @@ struct vty { * Used for password-obfuscate to differentiate whether the passwords * are encrpyted or not after frr restart.*/ bool read_from_conf; + + uint64_t vty_buf_size_accumulated; }; static inline void vty_push_context(struct vty *vty, int node, uint64_t id) @@ -325,6 +327,12 @@ struct vty_arg { /* Vty read buffer size. */ #define VTY_READ_BUFSIZ 512 +/* Vty max send buffer size */ +#define VTY_SEND_BUF_MAX 16777216 + +/* Vty flush intermediate size */ +#define VTY_MAX_INTERMEDIATE_FLUSH 131072 + /* Directory separator. */ #ifndef DIRECTORY_SEP #define DIRECTORY_SEP '/' diff --git a/vtysh/vtysh.c b/vtysh/vtysh.c index f39db5df17d0..25c45208a967 100644 --- a/vtysh/vtysh.c +++ b/vtysh/vtysh.c @@ -4117,6 +4117,7 @@ static int vtysh_connect(struct vtysh_client *vclient) struct sockaddr_un addr; struct stat s_stat; const char *path; + uint32_t rcvbufsize = VTYSH_RCV_BUF_MAX; if (!vclient->path[0]) snprintf(vclient->path, sizeof(vclient->path), "%s/%s.vty", @@ -4166,6 +4167,22 @@ static int vtysh_connect(struct vtysh_client *vclient) close(sock); return -1; } + + /* + * Increasing the RECEIVE socket buffer size so that the socket can hold + * after receving from other process. + */ + ret = setsockopt(sock, SOL_SOCKET, SO_RCVBUF, (char *)&rcvbufsize, + sizeof(rcvbufsize)); + if (ret < 0) { +#ifdef DEBUG + fprintf(stderr, "Cannot set socket %d rcv buffer size, %s\n", + sock, safe_strerror(errno)); +#endif /* DEBUG */ + close(sock); + return -1; + } + vclient->fd = sock; return 0; diff --git a/vtysh/vtysh.h b/vtysh/vtysh.h index 9d4c0a1d9677..111674e52887 100644 --- a/vtysh/vtysh.h +++ b/vtysh/vtysh.h @@ -50,6 +50,8 @@ extern struct thread_master *master; #define VTYSH_PATHD 0x80000 #define VTYSH_PIM6D 0x100000 +#define VTYSH_RCV_BUF_MAX 16777216 + #define VTYSH_WAS_ACTIVE (-2) /* commands in REALLYALL are crucial to correct vtysh operation */