Skip to content

Commit

Permalink
Reuse packets
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
dwmw2 committed Jun 28, 2021
1 parent 6e7f451 commit 0a68cdf
Show file tree
Hide file tree
Showing 6 changed files with 54 additions and 22 deletions.
2 changes: 1 addition & 1 deletion cstp.c
Expand Up @@ -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)
Expand Down
5 changes: 5 additions & 0 deletions library.c
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down
5 changes: 3 additions & 2 deletions mainloop.c
Expand Up @@ -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;

Expand Down
11 changes: 8 additions & 3 deletions oncp.c
Expand Up @@ -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;
}
Expand Down Expand Up @@ -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 */
Expand Down
45 changes: 33 additions & 12 deletions openconnect-internal.h
Expand Up @@ -142,6 +142,7 @@
/****************************************************************************/

struct pkt {
int alloc_len;
int len;
struct pkt *next;
union {
Expand Down Expand Up @@ -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) */ + \
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions ppp.c
Expand Up @@ -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;

Expand Down Expand Up @@ -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;
Expand Down

0 comments on commit 0a68cdf

Please sign in to comment.