From c36a5f0d2308f6bc3984fb246f87d4f6fc3543fe Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Thu, 29 Jan 2015 11:01:45 +0000 Subject: [PATCH] Improve packet queue handling When receiving a netperf UDP stream, it was spending 90% of its time walking the packet queue in queue_packet() to find the tail. Signed-off-by: David Woodhouse --- cstp.c | 10 ++++----- dtls.c | 21 ++++++------------ esp.c | 7 ++---- library.c | 3 +++ mainloop.c | 34 +++++++++++------------------ oncp.c | 15 ++++++------- openconnect-internal.h | 49 ++++++++++++++++++++++++++++++++++++------ 7 files changed, 78 insertions(+), 61 deletions(-) diff --git a/cstp.c b/cstp.c index ea48dd35..2f5dfc37 100644 --- a/cstp.c +++ b/cstp.c @@ -1052,7 +1052,8 @@ int cstp_mainloop(struct openconnect_info *vpninfo, int *timeout) case KA_KEEPALIVE: /* No need to send an explicit keepalive if we have real data to send */ - if (vpninfo->dtls_state != DTLS_CONNECTED && vpninfo->outgoing_queue) + if (vpninfo->dtls_state != DTLS_CONNECTED && + vpninfo->outgoing_queue.head) break; vpn_progress(vpninfo, PRG_DEBUG, _("Send CSTP Keepalive\n")); @@ -1065,10 +1066,9 @@ int cstp_mainloop(struct openconnect_info *vpninfo, int *timeout) } /* Service outgoing packet queue, if no DTLS */ - while (vpninfo->dtls_state != DTLS_CONNECTED && vpninfo->outgoing_queue) { - struct pkt *this = vpninfo->outgoing_queue; - vpninfo->outgoing_queue = this->next; - vpninfo->outgoing_qlen--; + while (vpninfo->dtls_state != DTLS_CONNECTED && + (vpninfo->current_ssl_pkt = dequeue_packet(&vpninfo->outgoing_queue))) { + struct pkt *this = vpninfo->current_ssl_pkt; if (vpninfo->cstp_compr) { ret = compress_packet(vpninfo, vpninfo->cstp_compr, this); diff --git a/dtls.c b/dtls.c index 5efc98a6..b940cdef 100644 --- a/dtls.c +++ b/dtls.c @@ -746,7 +746,7 @@ int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout) case KA_KEEPALIVE: /* No need to send an explicit keepalive if we have real data to send */ - if (vpninfo->outgoing_queue) + if (vpninfo->outgoing_queue.head) break; vpn_progress(vpninfo, PRG_DEBUG, _("Send DTLS Keepalive\n")); @@ -765,14 +765,11 @@ int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout) /* Service outgoing packet queue */ unmonitor_write_fd(vpninfo, dtls); - while (vpninfo->outgoing_queue) { - struct pkt *this = vpninfo->outgoing_queue; + while (vpninfo->outgoing_queue.head) { + struct pkt *this = dequeue_packet(&vpninfo->outgoing_queue); struct pkt *send_pkt = this; int ret; - vpninfo->outgoing_queue = this->next; - vpninfo->outgoing_qlen--; - /* One byte of header */ this->cstp.hdr[7] = AC_PKT_DATA; @@ -793,8 +790,7 @@ int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout) if (ret == SSL_ERROR_WANT_WRITE) { monitor_write_fd(vpninfo, dtls); - vpninfo->outgoing_queue = this; - vpninfo->outgoing_qlen++; + requeue_packet(&vpninfo->outgoing_queue, this); } else if (ret != SSL_ERROR_WANT_READ) { /* If it's a real error, kill the DTLS connection and requeue the packet to be sent over SSL */ @@ -803,8 +799,7 @@ int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout) ret); openconnect_report_ssl_errors(vpninfo); dtls_reconnect(vpninfo); - vpninfo->outgoing_queue = this; - vpninfo->outgoing_qlen++; + requeue_packet(&vpninfo->outgoing_queue, this); work_done = 1; } return work_done; @@ -817,13 +812,11 @@ int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout) _("DTLS got write error: %s. Falling back to SSL\n"), gnutls_strerror(ret)); dtls_reconnect(vpninfo); - vpninfo->outgoing_queue = this; - vpninfo->outgoing_qlen++; + requeue_packet(&vpninfo->outgoing_queue, this); work_done = 1; } else if (gnutls_record_get_direction(vpninfo->dtls_ssl)) { monitor_write_fd(vpninfo, dtls); - vpninfo->outgoing_queue = this; - vpninfo->outgoing_qlen++; + requeue_packet(&vpninfo->outgoing_queue, this); } return work_done; diff --git a/esp.c b/esp.c index 16ce436d..f4c40816 100644 --- a/esp.c +++ b/esp.c @@ -216,6 +216,7 @@ int esp_mainloop(struct openconnect_info *vpninfo, int *timeout) { struct esp *esp = &vpninfo->esp_in[vpninfo->current_esp_in]; struct esp *old_esp = &vpninfo->esp_in[vpninfo->current_esp_in ^ 1]; + struct pkt *this; int work_done = 0; int ret; @@ -317,13 +318,9 @@ int esp_mainloop(struct openconnect_info *vpninfo, int *timeout) return 0; unmonitor_write_fd(vpninfo, dtls); - while (vpninfo->outgoing_queue) { - struct pkt *this = vpninfo->outgoing_queue; + while ((this = dequeue_packet(&vpninfo->outgoing_queue))) { int len; - vpninfo->outgoing_queue = this->next; - vpninfo->outgoing_qlen--; - len = encrypt_esp_packet(vpninfo, this); if (len > 0) { ret = send(vpninfo->dtls_fd, (void *)&this->esp, len, 0); diff --git a/library.c b/library.c index 4819beda..eabed47d 100644 --- a/library.c +++ b/library.c @@ -69,6 +69,9 @@ struct openconnect_info *openconnect_vpninfo_new(const char *useragent, #ifndef _WIN32 vpninfo->tun_fd = -1; #endif + init_pkt_queue(&vpninfo->incoming_queue); + init_pkt_queue(&vpninfo->outgoing_queue); + init_pkt_queue(&vpninfo->oncp_control_queue); vpninfo->ssl_fd = vpninfo->dtls_fd = -1; vpninfo->cmd_fd = vpninfo->cmd_fd_write = -1; vpninfo->tncc_fd = -1; diff --git a/mainloop.c b/mainloop.c index 0dde5737..1bd3bc8b 100644 --- a/mainloop.c +++ b/mainloop.c @@ -25,16 +25,7 @@ #include "openconnect-internal.h" -void queue_packet(struct pkt **q, struct pkt *new) -{ - while (*q) - q = &(*q)->next; - - new->next = NULL; - *q = new; -} - -int queue_new_packet(struct pkt **q, void *buf, int len) +int queue_new_packet(struct pkt_q *q, void *buf, int len) { struct pkt *new = malloc(sizeof(struct pkt) + len); if (!new) @@ -51,6 +42,7 @@ int queue_new_packet(struct pkt **q, void *buf, int len) tun*.c files for specific platforms */ int tun_mainloop(struct openconnect_info *vpninfo, int *timeout) { + struct pkt *this; int work_done = 0; if (read_fd_monitored(vpninfo, tun)) { @@ -72,33 +64,31 @@ int tun_mainloop(struct openconnect_info *vpninfo, int *timeout) vpninfo->stats.tx_pkts++; vpninfo->stats.tx_bytes += out_pkt->len; - - queue_packet(&vpninfo->outgoing_queue, out_pkt); - out_pkt = NULL; - work_done = 1; - vpninfo->outgoing_qlen++; - if (vpninfo->outgoing_qlen == vpninfo->max_qlen) { + + if (queue_packet(&vpninfo->outgoing_queue, out_pkt) == + vpninfo->max_qlen) { + out_pkt = NULL; unmonitor_read_fd(vpninfo, tun); break; } + out_pkt = NULL; } vpninfo->tun_pkt = out_pkt; - } else if (vpninfo->outgoing_qlen < vpninfo->max_qlen) { + } else if (vpninfo->outgoing_queue.count < vpninfo->max_qlen) { monitor_read_fd(vpninfo, tun); } - while (vpninfo->incoming_queue) { - struct pkt *this = vpninfo->incoming_queue; + while ((this = dequeue_packet(&vpninfo->incoming_queue))) { - if (os_write_tun(vpninfo, this)) + if (os_write_tun(vpninfo, this)) { + requeue_packet(&vpninfo->incoming_queue, this); break; + } vpninfo->stats.rx_pkts++; vpninfo->stats.rx_bytes += this->len; - vpninfo->incoming_queue = this->next; - free(this); } /* Work is not done if we just got rid of packets off the queue */ diff --git a/oncp.c b/oncp.c index 03522a53..fdade2bb 100644 --- a/oncp.c +++ b/oncp.c @@ -1687,20 +1687,18 @@ int oncp_mainloop(struct openconnect_info *vpninfo, int *timeout) ; } #endif - if (vpninfo->oncp_control_queue) { - vpninfo->current_ssl_pkt = vpninfo->oncp_control_queue; - vpninfo->oncp_control_queue = vpninfo->oncp_control_queue->next; + vpninfo->current_ssl_pkt = dequeue_packet(&vpninfo->oncp_control_queue); + if (vpninfo->current_ssl_pkt) goto handle_outgoing; - } + if (vpninfo->dtls_state == DTLS_CONNECTING) { vpninfo->current_ssl_pkt = (struct pkt *)&esp_enable_pkt; goto handle_outgoing; } /* Service outgoing packet queue, if no DTLS */ - while (vpninfo->dtls_state != DTLS_CONNECTED && vpninfo->outgoing_queue) { - struct pkt *this = vpninfo->outgoing_queue; - vpninfo->outgoing_queue = this->next; - vpninfo->outgoing_qlen--; + while (vpninfo->dtls_state != DTLS_CONNECTED && + (vpninfo->current_ssl_pkt = dequeue_packet(&vpninfo->outgoing_queue))) { + struct pkt *this = vpninfo->current_ssl_pkt; /* Little-endian overall record length */ store_le16(this->oncp.hdr, (this->len + 20)); @@ -1712,7 +1710,6 @@ int oncp_mainloop(struct openconnect_info *vpninfo, int *timeout) _("Sending uncompressed data packet of %d bytes\n"), this->len); - vpninfo->current_ssl_pkt = this; goto handle_outgoing; } diff --git a/openconnect-internal.h b/openconnect-internal.h index 11350fdc..5aa1293b 100644 --- a/openconnect-internal.h +++ b/openconnect-internal.h @@ -246,6 +246,45 @@ struct vpn_proto { void (*udp_shutdown)(struct openconnect_info *vpninfo); }; +struct pkt_q { + struct pkt *head; + struct pkt **tail; + int count; +}; + +static inline struct pkt *dequeue_packet(struct pkt_q *q) +{ + struct pkt *ret = q->head; + + if (ret) { + q->head = ret->next; + if (!--q->count) + q->tail = &q->head; + } + return ret; +} + +static inline void requeue_packet(struct pkt_q *q, struct pkt *p) +{ + p->next = q->head; + q->head = p; + if (!q->count++) + q->tail = &p->next; +} + +static inline int queue_packet(struct pkt_q *q, struct pkt *p) +{ + *(q->tail) = p; + p->next = NULL; + q->tail = &p->next; + return ++q->count; +} + +static inline void init_pkt_queue(struct pkt_q *q) +{ + q->tail = &q->head; +} + struct esp { #if defined(ESP_GNUTLS) gnutls_cipher_hd_t cipher; @@ -439,7 +478,7 @@ struct openconnect_info { struct pkt *deflate_pkt; /* For compressing outbound packets into */ struct pkt *pending_deflated_pkt; /* The original packet associated with above */ struct pkt *current_ssl_pkt; /* Partially sent SSL packet */ - struct pkt *oncp_control_queue; /* Control packets to be sent on oNCP next */ + struct pkt_q oncp_control_queue; /* Control packets to be sent on oNCP next */ /* Packet buffers for receiving into */ struct pkt *cstp_pkt; struct pkt *dtls_pkt; @@ -516,9 +555,8 @@ struct openconnect_info { int got_pause_cmd; char cancel_type; - struct pkt *incoming_queue; - struct pkt *outgoing_queue; - int outgoing_qlen; + struct pkt_q incoming_queue; + struct pkt_q outgoing_queue; int max_qlen; struct oc_stats stats; openconnect_stats_vfn stats_handler; @@ -802,8 +840,7 @@ int openconnect_hash_yubikey_password(struct openconnect_info *vpninfo, /* mainloop.c */ int tun_mainloop(struct openconnect_info *vpninfo, int *timeout); -int queue_new_packet(struct pkt **q, void *buf, int len); -void queue_packet(struct pkt **q, struct pkt *new); +int queue_new_packet(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);