Re: [EXTERNAL] Re: What throughput is reasonable?

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, 2019-04-09 at 10:12 +0300, David Woodhouse wrote:
> On Tue, 2019-04-09 at 09:34 +0300, David Woodhouse wrote:
> > 
> > It's interesting that Nikos's code does 50% better than mine. I'll
> > investigate that.
> 
> It isn't that I'm using malloc() and free() for each packet. If I use a
> pool, a tiny amount of _int_malloc() disappears from the perf report
> but the overall performance remains the same.
> 
> Probably that I need to pay attention to which file descriptors woke me
> from poll() and service only those which need it, rather than doing
> everything, every time.

Not that simple either. This doesn't make it faster:

diff --git a/cstp.c b/cstp.c
index 14ed4b46..df64ae1a 100644
--- a/cstp.c
+++ b/cstp.c
@@ -894,7 +894,7 @@ int compress_packet(struct openconnect_info *vpninfo, int compr_type, struct pkt
 	return 0;
 }
 
-int cstp_mainloop(struct openconnect_info *vpninfo, int *timeout)
+int cstp_mainloop(struct openconnect_info *vpninfo, int *timeout, int readable)
 {
 	int ret;
 	int work_done = 0;
@@ -908,7 +908,7 @@ int cstp_mainloop(struct openconnect_info *vpninfo, int *timeout)
 	   we should probably remove POLLIN from the events we're looking for,
 	   and add POLLOUT. As it is, though, it'll just chew CPU time in that
 	   fairly unlikely situation, until the write backlog clears. */
-	while (1) {
+	while (readable) {
 		/* Some servers send us packets that are larger than
 		   negotiated MTU. We reserve some extra space to
 		   handle that */
diff --git a/dtls.c b/dtls.c
index d8e0d2f8..3148f502 100644
--- a/dtls.c
+++ b/dtls.c
@@ -242,7 +242,7 @@ int dtls_setup(struct openconnect_info *vpninfo, int dtls_attempt_period)
 	return 0;
 }
 
-int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout)
+int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout, int readable)
 {
 	int work_done = 0;
 	char magic_pkt;
@@ -271,12 +271,12 @@ int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout)
 		return 0;
 	}
 
-	while (1) {
+	while (readable) {
 		int len = vpninfo->ip_info.mtu;
 		unsigned char *buf;
 
 		if (!vpninfo->dtls_pkt) {
-			vpninfo->dtls_pkt = malloc(sizeof(struct pkt) + len);
+			vpninfo->dtls_pkt = pkt_alloc(vpninfo, sizeof(struct pkt) + len);
 			if (!vpninfo->dtls_pkt) {
 				vpn_progress(vpninfo, PRG_ERR, _("Allocation failed\n"));
 				break;
@@ -502,7 +502,7 @@ int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout)
 		vpn_progress(vpninfo, PRG_TRACE,
 			     _("Sent DTLS packet of %d bytes; DTLS send returned %d\n"),
 			     this->len, ret);
-		free(this);
+		pkt_free(vpninfo, this);
 	}
 
 	return work_done;
diff --git a/mainloop.c b/mainloop.c
index fe185fe2..c733e321 100644
--- a/mainloop.c
+++ b/mainloop.c
@@ -45,7 +45,7 @@ int queue_new_packet(struct pkt_q *q, void *buf, int len)
 
 /* This is here because it's generic and hence can't live in either of the
    tun*.c files for specific platforms */
-int tun_mainloop(struct openconnect_info *vpninfo, int *timeout)
+int tun_mainloop(struct openconnect_info *vpninfo, int *timeout, int readable)
 {
 	struct pkt *this;
 	int work_done = 0;
@@ -58,13 +58,13 @@ int tun_mainloop(struct openconnect_info *vpninfo, int *timeout)
 		return 0;
 	}
 
-	if (read_fd_monitored(vpninfo, tun)) {
+	if (readable && read_fd_monitored(vpninfo, tun)) {
 		struct pkt *out_pkt = vpninfo->tun_pkt;
 		while (1) {
 			int len = vpninfo->ip_info.mtu;
 
 			if (!out_pkt) {
-				out_pkt = malloc(sizeof(struct pkt) + len + vpninfo->pkt_trailer);
+				out_pkt = pkt_alloc(vpninfo, sizeof(struct pkt) + len + vpninfo->pkt_trailer);
 				if (!out_pkt) {
 					vpn_progress(vpninfo, PRG_ERR, _("Allocation failed\n"));
 					break;
@@ -104,7 +104,7 @@ int tun_mainloop(struct openconnect_info *vpninfo, int *timeout)
 		vpninfo->stats.rx_pkts++;
 		vpninfo->stats.rx_bytes += this->len;
 
-		free(this);
+		pkt_free(vpninfo, this);
 	}
 	/* Work is not done if we just got rid of packets off the queue */
 	return work_done;
@@ -177,7 +177,7 @@ int openconnect_mainloop(struct openconnect_info *vpninfo,
 			 int reconnect_interval)
 {
 	int ret = 0;
-
+	int tun_r = 1, udp_r = 1, tcp_r = 1;
 	vpninfo->reconnect_timeout = reconnect_timeout;
 	vpninfo->reconnect_interval = reconnect_interval;
 
@@ -217,7 +217,7 @@ int openconnect_mainloop(struct openconnect_info *vpninfo,
 				}
 			}
 
-			ret = vpninfo->proto->udp_mainloop(vpninfo, &timeout);
+			ret = vpninfo->proto->udp_mainloop(vpninfo, &timeout, udp_r);
 			if (vpninfo->quit_reason)
 				break;
 			did_work += ret;
@@ -229,14 +229,14 @@ int openconnect_mainloop(struct openconnect_info *vpninfo,
 				break;
 		}
 
-		ret = vpninfo->proto->tcp_mainloop(vpninfo, &timeout);
+		ret = vpninfo->proto->tcp_mainloop(vpninfo, &timeout, tcp_r);
 		if (vpninfo->quit_reason)
 			break;
 		did_work += ret;
 
 		/* Tun must be last because it will set/clear its bit
 		   in the select_rfds according to the queue length */
-		did_work += tun_mainloop(vpninfo, &timeout);
+		did_work += tun_mainloop(vpninfo, &timeout, tun_r);
 		if (vpninfo->quit_reason)
 			break;
 
@@ -304,6 +304,9 @@ int openconnect_mainloop(struct openconnect_info *vpninfo,
 		tv.tv_usec = (timeout % 1000) * 1000;
 
 		select(vpninfo->_select_nfds, &rfds, &wfds, &efds, &tv);
+		tun_r = FD_ISSET(vpninfo->tun_fd, &rfds);
+		udp_r = FD_ISSET(vpninfo->dtls_fd, &rfds);
+		tcp_r = FD_ISSET(vpninfo->ssl_fd, &rfds);
 #endif
 	}
 
diff --git a/openconnect-internal.h b/openconnect-internal.h
index 8df7d24d..2fb2452f 100644
--- a/openconnect-internal.h
+++ b/openconnect-internal.h
@@ -263,7 +263,7 @@ struct vpn_proto {
 	/* Establish the TCP connection (and obtain configuration) */
 	int (*tcp_connect)(struct openconnect_info *vpninfo);
 
-	int (*tcp_mainloop)(struct openconnect_info *vpninfo, int *timeout);
+	int (*tcp_mainloop)(struct openconnect_info *vpninfo, int *timeout, int readable);
 
 	/* Add headers common to each HTTP request */
 	void (*add_http_headers)(struct openconnect_info *vpninfo, struct oc_text_buf *buf);
@@ -273,7 +273,7 @@ struct vpn_proto {
 
 	/* This will actually complete the UDP connection setup/handshake on the wire,
 	   as well as transporting packets */
-	int (*udp_mainloop)(struct openconnect_info *vpninfo, int *timeout);
+	int (*udp_mainloop)(struct openconnect_info *vpninfo, int *timeout, int readable);
 
 	/* Close the connection but leave the session setup so it restarts */
 	void (*udp_close)(struct openconnect_info *vpninfo);
@@ -526,6 +526,7 @@ struct openconnect_info {
 	struct pkt *cstp_pkt;
 	struct pkt *dtls_pkt;
 	struct pkt *tun_pkt;
+	struct pkt *pkt_pool;
 	int pkt_trailer; /* How many bytes after payload for encryption (ESP HMAC) */
 
 	z_stream inflate_strm;
@@ -799,6 +800,23 @@ OPENCONNECT_CMD_SOCKET dumb_socketpair(OPENCONNECT_CMD_SOCKET socks[2], int make
 
 /****************************************************************************/
 
+
+
+static inline struct pkt *pkt_alloc(struct openconnect_info *vpninfo, int len)
+{
+	struct pkt *ret = vpninfo->pkt_pool;
+	if (ret)
+		vpninfo->pkt_pool = ret->next;
+	else
+		ret = malloc(len);
+	return ret;
+}
+static inline void pkt_free(struct openconnect_info *vpninfo, struct pkt *pkt)
+{
+	pkt->next = vpninfo->pkt_pool;
+	vpninfo->pkt_pool = pkt;
+}
+
 /* iconv.c */
 #ifdef HAVE_ICONV
 char *openconnect_utf8_to_legacy(struct openconnect_info *vpninfo, const char *utf8);
@@ -831,7 +849,7 @@ void dtls_ssl_free(struct openconnect_info *vpninfo);
 
 /* dtls.c */
 int dtls_setup(struct openconnect_info *vpninfo, int dtls_attempt_period);
-int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout);
+int dtls_mainloop(struct openconnect_info *vpninfo, int *timeout, int readable);
 void dtls_close(struct openconnect_info *vpninfo);
 void dtls_shutdown(struct openconnect_info *vpninfo);
 void gather_dtls_ciphers(struct openconnect_info *vpninfo, struct oc_text_buf *buf, struct oc_text_buf *buf12);
@@ -844,7 +862,7 @@ char *openconnect_bin2base64(const char *prefix, const uint8_t *data, unsigned l
 /* cstp.c */
 void cstp_common_headers(struct openconnect_info *vpninfo, struct oc_text_buf *buf);
 int cstp_connect(struct openconnect_info *vpninfo);
-int cstp_mainloop(struct openconnect_info *vpninfo, int *timeout);
+int cstp_mainloop(struct openconnect_info *vpninfo, int *timeout, int readable);
 int cstp_bye(struct openconnect_info *vpninfo, const char *reason);
 int decompress_and_queue_packet(struct openconnect_info *vpninfo, int compr_type,
 				unsigned char *buf, int len);
@@ -963,7 +981,7 @@ int hotp_hmac(struct openconnect_info *vpninfo, const void *challenge);
 #endif
 
 /* mainloop.c */
-int tun_mainloop(struct openconnect_info *vpninfo, int *timeout);
+int tun_mainloop(struct openconnect_info *vpninfo, int *timeout, int readable);
 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);

Attachment: smime.p7s
Description: S/MIME cryptographic signature

_______________________________________________
openconnect-devel mailing list
openconnect-devel@xxxxxxxxxxxxxxxxxxx
http://lists.infradead.org/mailman/listinfo/openconnect-devel

[Index of Archives]     [Linux Samsung SoC]     [Linux Rockchip SoC]     [Linux Actions SoC]     [Linux for Synopsys ARC Processors]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]


  Powered by Linux