From: David Woodhouse <dwmw2@infradead.org>
Date: Mon, 28 Jun 2021 13:44:51 +0000 (+0100)
Subject: Reuse packets
X-Git-Tag: v8.20~123
X-Git-Url: https://www.infradead.org/git/?a=commitdiff_plain;h=0a68cdff40f77809e113cd2c464a232cb8686bf3;p=users%2Fdwmw2%2Fopenconnect.git

Reuse packets

I see malloc/free showing up at ~5% of perf traces, and it's entirely
pointless when we could be reusing packets.

This trick isn't *perfect* and there's potential for a pathological
case where all the packets on the free_queue are too small to be
reused but we never get rid of them anyway — but rounding up to 2KiB
should mean that never happens in practice, and the alignment we get
from that rounding probably doesn't hurt performance anyway.

Signed-off-by: David Woodhouse <dwmw2@infradead.org>
---

diff --git a/cstp.c b/cstp.c
index 6582097a..d9ac12de 100644
--- a/cstp.c
+++ b/cstp.c
@@ -761,7 +761,7 @@ int decompress_and_queue_packet(struct openconnect_info *vpninfo, int compr_type
 	   negotiated MTU after decompression. We reserve some extra
 	   space to handle that */
 	int receive_mtu = MAX(16384, vpninfo->ip_info.mtu);
-	struct pkt *new = malloc(sizeof(struct pkt) + receive_mtu);
+	struct pkt *new = alloc_pkt(vpninfo, receive_mtu);
 	const char *comprname = "";
 
 	if (!new)
diff --git a/library.c b/library.c
index 8e43f367..bdd89af5 100644
--- a/library.c
+++ b/library.c
@@ -69,6 +69,7 @@ struct openconnect_info *openconnect_vpninfo_new(const char *useragent,
 #ifndef _WIN32
 	vpninfo->tun_fd = -1;
 #endif
+	init_pkt_queue(&vpninfo->free_queue);
 	init_pkt_queue(&vpninfo->incoming_queue);
 	init_pkt_queue(&vpninfo->outgoing_queue);
 	init_pkt_queue(&vpninfo->tcp_control_queue);
@@ -691,6 +692,10 @@ void openconnect_vpninfo_free(struct openconnect_info *vpninfo)
 	free_pkt(vpninfo, vpninfo->tun_pkt);
 	free_pkt(vpninfo, vpninfo->dtls_pkt);
 	free_pkt(vpninfo, vpninfo->cstp_pkt);
+	struct pkt *pkt;
+	while ((pkt = dequeue_packet(&vpninfo->free_queue)))
+		free(pkt);
+
 	free(vpninfo->bearer_token);
 	free(vpninfo);
 }
diff --git a/mainloop.c b/mainloop.c
index 834b5be2..beb8d7cb 100644
--- a/mainloop.c
+++ b/mainloop.c
@@ -30,9 +30,10 @@
 
 #include "openconnect-internal.h"
 
-int queue_new_packet(struct pkt_q *q, void *buf, int len)
+int queue_new_packet(struct openconnect_info *vpninfo,
+		     struct pkt_q *q, void *buf, int len)
 {
-	struct pkt *new = malloc(sizeof(struct pkt) + len);
+	struct pkt *new = alloc_pkt(vpninfo, len);
 	if (!new)
 		return -ENOMEM;
 
diff --git a/oncp.c b/oncp.c
index 58d7ad3d..f8653d7b 100644
--- a/oncp.c
+++ b/oncp.c
@@ -371,12 +371,16 @@ static const struct pkt esp_enable_pkt = {
 
 static int queue_esp_control(struct openconnect_info *vpninfo, int enable)
 {
-	struct pkt *new = malloc(sizeof(*new) + 13);
+	struct pkt *new = alloc_pkt(vpninfo, esp_enable_pkt.len);
 	if (!new)
 		return -ENOMEM;
 
-	memcpy(new, &esp_enable_pkt, sizeof(*new) + 13);
+	new->oncp = esp_enable_pkt.oncp;
+	new->len = esp_enable_pkt.len;
+
+	memcpy(new->data, esp_enable_pkt.data, esp_enable_pkt.len);
 	new->data[12] = enable;
+
 	queue_packet(&vpninfo->tcp_control_queue, new);
 	return 0;
 }
@@ -963,7 +967,8 @@ int oncp_mainloop(struct openconnect_info *vpninfo, int *timeout, int readable)
 			}
 
 			/* OK, we have a whole packet, and we have stuff after it */
-			queue_new_packet(&vpninfo->incoming_queue, vpninfo->cstp_pkt->data, iplen);
+			queue_new_packet(vpninfo, &vpninfo->incoming_queue,
+					 vpninfo->cstp_pkt->data, iplen);
 			kmplen -= iplen;
 			if (kmplen) {
 				/* Still data packets to come in this KMP300 */
diff --git a/openconnect-internal.h b/openconnect-internal.h
index 63afa4b8..2435bf21 100644
--- a/openconnect-internal.h
+++ b/openconnect-internal.h
@@ -142,6 +142,7 @@
 /****************************************************************************/
 
 struct pkt {
+	int alloc_len;
 	int len;
 	struct pkt *next;
 	union {
@@ -388,17 +389,6 @@ static inline void init_pkt_queue(struct pkt_q *q)
 	q->tail = &q->head;
 }
 
-
-static inline struct pkt *alloc_pkt(struct openconnect_info *vpninfo, int len)
-{
-	return malloc(sizeof(struct pkt) + len);
-}
-
-static inline void free_pkt(struct openconnect_info *vpninfo, struct pkt *pkt)
-{
-	free(pkt);
-}
-
 #define TLS_OVERHEAD 5 /* packet + header */
 #define DTLS_OVERHEAD (1 /* packet + header */ + 13 /* DTLS header */ + \
 	 20 /* biggest supported MAC (SHA1) */ +  32 /* biggest supported IV (AES-256) */ + \
@@ -753,6 +743,7 @@ struct openconnect_info {
 	int got_pause_cmd;
 	char cancel_type;
 
+	struct pkt_q free_queue;
 	struct pkt_q incoming_queue;
 	struct pkt_q outgoing_queue;
 	struct pkt_q tcp_control_queue;		/* Control packets to be sent via TCP */
@@ -798,6 +789,35 @@ struct openconnect_info {
 	int (*ssl_write)(struct openconnect_info *vpninfo, char *buf, size_t len);
 };
 
+
+static inline struct pkt *alloc_pkt(struct openconnect_info *vpninfo, int len)
+{
+	int alloc_len = sizeof(struct pkt) + len;
+
+	if (vpninfo->free_queue.head &&
+	    vpninfo->free_queue.head->alloc_len >= alloc_len)
+		return dequeue_packet(&vpninfo->free_queue);
+
+	if (alloc_len < 2048)
+		alloc_len = 2048;
+
+	struct pkt *pkt = malloc(alloc_len);
+	if (pkt)
+		pkt->alloc_len = alloc_len;
+	return pkt;
+}
+
+static inline void free_pkt(struct openconnect_info *vpninfo, struct pkt *pkt)
+{
+	if (!pkt)
+		return;
+
+	if (vpninfo->free_queue.count < vpninfo->max_qlen * 2)
+		requeue_packet(&vpninfo->free_queue, pkt);
+	else
+		free(pkt);
+}
+
 #ifdef _WIN32
 #define monitor_read_fd(_v, _n) _v->_n##_monitored |= FD_READ
 #define monitor_write_fd(_v, _n) _v->_n##_monitored |= FD_WRITE
@@ -1232,7 +1252,8 @@ int openconnect_install_ctx_verify(struct openconnect_info *vpninfo,
 
 /* mainloop.c */
 int tun_mainloop(struct openconnect_info *vpninfo, int *timeout, int readable);
-int queue_new_packet(struct pkt_q *q, void *buf, int len);
+int queue_new_packet(struct openconnect_info *vpninfo,
+		     struct pkt_q *q, void *buf, int len);
 int keepalive_action(struct keepalive_info *ka, int *timeout);
 int ka_stalled_action(struct keepalive_info *ka, int *timeout);
 int ka_check_deadline(int *timeout, time_t now, time_t due);
diff --git a/ppp.c b/ppp.c
index 59007c8d..6c7ef2c2 100644
--- a/ppp.c
+++ b/ppp.c
@@ -76,7 +76,7 @@ static struct pkt *hdlc_into_new_pkt(struct openconnect_info *vpninfo, struct pk
 	/* Every byte in payload and 2-byte FCS potentially expands to two bytes,
 	 * plus 2 for flag (0x7e) at start and end. We know that we will output
 	 * at least 4 bytes so we can stash those in the header. */
-	struct pkt *p = malloc(sizeof(struct pkt) + len*2 + 2);
+	struct pkt *p = alloc_pkt(vpninfo, len*2 + 2);
 	if (!p)
 		return NULL;
 
@@ -360,10 +360,10 @@ static int buf_append_ppp_tlv_be32(struct oc_text_buf *buf, int tag, uint32_t va
 	return buf_append_ppp_tlv(buf, tag, 4, &val_be);
 }
 
-static int queue_config_packet(struct openconnect_info *vpninfo,
-				uint16_t proto, int id, int code, int len, const void *payload)
+static int queue_config_packet(struct openconnect_info *vpninfo, uint16_t proto,
+			       int id, int code, int len, const void *payload)
 {
-	struct pkt *p = malloc(sizeof(struct pkt) + len + 4);
+	struct pkt *p = alloc_pkt(vpninfo, len + 4);
 
 	if (!p)
 		return -ENOMEM;