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 6582097ad45ad3dd5dbbc7b84fb1713f2ab059a5..d9ac12def8e6323cf9f949bb419426aed766864c 100644 (file)
--- 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 8e43f3677dd58cb4919d0944a45f5e33295b203f..bdd89af5aeb210e3e0f21e46aa9eb42d99afb904 100644 (file)
--- 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 834b5be268818d07441e662b1b2ed8919cb56025..beb8d7cb07698300d9ecf4191a2c88ac58fbe89b 100644 (file)
--- 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 58d7ad3df497534d4925b9956d00c7670cccb38f..f8653d7b2b04948bb5bf30c7e485ca5ff5f64114 100644 (file)
--- 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 63afa4b875b8dfacd39a76e9d0308de8f010e27e..2435bf2162b6ce986b3cb8341d822121e00b6a93 100644 (file)
--- 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 59007c8d63ec9cea952d6d3460491e85e978d401..6c7ef2c205aae41b9b728a8a2200018a541c756c 100644 (file)
--- 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;