[Asterisk-code-review] manager: Send fewer packets (...asterisk[master])

[Friendly Automation]

Friendly Automation has submitted this change and it was merged. ( https://gerrit.asterisk.org/c/asterisk/+/11636 )

Change subject: manager: Send fewer packets
......................................................................

manager: Send fewer packets

The functions that build manager message headers do so in a way that
results in a single messages being split across multiple packets. While
this doesn't matter to the remote end, it makes network captures noisier
and harder to follow, and also means additional system calls.

With this patch, we build up more of the message content into the TLS
buffer before flushing to the network. This change is completely
internal to the manager code and does not affect any of the existing
API's consumers.

Change-Id: I50128b0769060ca5272dbbb5e60242d131eaddf9
---
M main/manager.c
1 file changed, 64 insertions(+), 21 deletions(-)

Approvals:
  Kevin Harwell: Looks good to me, but someone else must approve
  George Joseph: Looks good to me, approved
  Friendly Automation: Approved for Submit



diff --git a/main/manager.c b/main/manager.c
index 69cbe37..904ae1f 100644
--- a/main/manager.c
+++ b/main/manager.c
@@ -3054,6 +3054,15 @@
 /*! \brief initial allocated size for the astman_append_buf and astman_send_*_va */
 #define ASTMAN_APPEND_BUF_INITSIZE   256
 
+static void astman_flush(struct mansession *s, struct ast_str *buf)
+{
+	if (s->hook || (s->tcptls_session && s->tcptls_session->stream)) {
+		send_string(s, ast_str_buffer(buf));
+	} else {
+		ast_verbose("No connection stream in astman_append, should not happen\n");
+	}
+}
+
 /*!
  * utility functions for creating AMI replies
  */
@@ -3103,21 +3112,32 @@
 static void astman_send_response_full(struct mansession *s, const struct message *m, char *resp, char *msg, char *listflag)
 {
 	const char *id = astman_get_header(m, "ActionID");
+	struct ast_str *buf;
 
-	astman_append(s, "Response: %s\r\n", resp);
-	if (!ast_strlen_zero(id)) {
-		astman_append(s, "ActionID: %s\r\n", id);
-	}
-	if (listflag) {
-		astman_append(s, "EventList: %s\r\n", listflag);	/* Start, complete, cancelled */
-	}
-	if (msg == MSG_MOREDATA) {
+	buf = ast_str_thread_get(&astman_append_buf, ASTMAN_APPEND_BUF_INITSIZE);
+	if (!buf) {
 		return;
-	} else if (msg) {
-		astman_append(s, "Message: %s\r\n\r\n", msg);
-	} else {
-		astman_append(s, "\r\n");
 	}
+
+	ast_str_set(&buf, 0, "Response: %s\r\n", resp);
+
+	if (!ast_strlen_zero(id)) {
+		ast_str_append(&buf, 0, "ActionID: %s\r\n", id);
+	}
+
+	if (listflag) {
+		/* Start, complete, cancelled */
+		ast_str_append(&buf, 0, "EventList: %s\r\n", listflag);
+	}
+
+	if (msg != MSG_MOREDATA) {
+		if (msg) {
+			ast_str_append(&buf, 0, "Message: %s\r\n", msg);
+		}
+		ast_str_append(&buf, 0, "\r\n");
+	}
+
+	astman_flush(s, buf);
 }
 
 void astman_send_response(struct mansession *s, const struct message *m, char *resp, char *msg)
@@ -3172,18 +3192,43 @@
 	astman_send_response_full(s, m, "Success", msg, listflag);
 }
 
-void astman_send_list_complete_start(struct mansession *s, const struct message *m, const char *event_name, int count)
+static struct ast_str *astman_send_list_complete_start_common(struct mansession *s, const struct message *m, const char *event_name, int count)
 {
 	const char *id = astman_get_header(m, "ActionID");
+	struct ast_str *buf;
 
-	astman_append(s, "Event: %s\r\n", event_name);
-	if (!ast_strlen_zero(id)) {
-		astman_append(s, "ActionID: %s\r\n", id);
+	buf = ast_str_thread_get(&astman_append_buf, ASTMAN_APPEND_BUF_INITSIZE);
+	if (!buf) {
+		return NULL;
 	}
-	astman_append(s,
+
+	ast_str_set(&buf, 0, "Event: %s\r\n", event_name);
+	if (!ast_strlen_zero(id)) {
+		ast_str_append(&buf, 0, "ActionID: %s\r\n", id);
+	}
+	ast_str_append(&buf, 0,
 		"EventList: Complete\r\n"
 		"ListItems: %d\r\n",
 		count);
+
+	return buf;
+}
+
+static void astman_send_list_complete(struct mansession *s, const struct message *m, const char *event_name, int count)
+{
+	struct ast_str *buf = astman_send_list_complete_start_common(s, m, event_name, count);
+	if (buf) {
+		ast_str_append(&buf, 0, "\r\n");
+		astman_flush(s, buf);
+	}
+}
+
+void astman_send_list_complete_start(struct mansession *s, const struct message *m, const char *event_name, int count)
+{
+	struct ast_str *buf = astman_send_list_complete_start_common(s, m, event_name, count);
+	if (buf) {
+		astman_flush(s, buf);
+	}
 }
 
 void astman_send_list_complete_end(struct mansession *s)
@@ -4511,8 +4556,7 @@
 	regfree(&regexbuf);
 	ast_free(regex_string);
 
-	astman_send_list_complete_start(s, m, "ChannelsHungupListComplete", channels_matched);
-	astman_send_list_complete_end(s);
+	astman_send_list_complete(s, m, "ChannelsHungupListComplete", channels_matched);
 
 	return 0;
 }
@@ -6320,8 +6364,7 @@
 	}
 	ao2_iterator_destroy(&it_chans);
 
-	astman_send_list_complete_start(s, m, "CoreShowChannelsComplete", numchans);
-	astman_send_list_complete_end(s);
+	astman_send_list_complete(s, m, "CoreShowChannelsComplete", numchans);
 
 	ao2_ref(channels, -1);
 	return 0;

-- 
To view, visit https://gerrit.asterisk.org/c/asterisk/+/11636
To unsubscribe, or for help writing mail filters, visit https://gerrit.asterisk.org/settings

Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Change-Id: I50128b0769060ca5272dbbb5e60242d131eaddf9
Gerrit-Change-Number: 11636
Gerrit-PatchSet: 4
Gerrit-Owner: Sean Bright <sean.bright at gmail.com>
Gerrit-Reviewer: Friendly Automation
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.digium.com/pipermail/asterisk-code-review/attachments/20190731/53abaa7c/attachment.html>