Re: rpki-client http keep-alive support

2021-04-20 Thread Job Snijders
On Tue, Apr 20, 2021 at 10:27:51AM +0200, Claudio Jeker wrote:
> On Fri, Apr 16, 2021 at 12:21:56PM +0200, Claudio Jeker wrote:
> > This diff changes the http module to support keep-alive.
> > It splits requests (for a resource) from connections (to a server).
> > When a request is received the code tries to first use a IDLE connection,
> > if none is around a new connection is started (unless there are too many
> > connections inflight).
> > 
> > Idle connections are kept for 10sec and closed after that time. For
> > rpki-client this should work well since the RRDP exchange will be a burtst
> > of requests (one after another). There is only one server that is
> > connected twice during the run (one server hosting 2 repos).
> > 
> > The benefit of using keep-alive is less CPU time wasted on constant TLS
> > handshakes. I did not notice any speed improvements.
> > 
> > After fixing most issues in the http module the last few days this is less
> > urgent to go in. Still sending it out so people can play with it.
> 
> Updated diff to apply after all the KNF commits.

OK job@



Re: rpki-client http keep-alive support

2021-04-20 Thread Claudio Jeker
On Fri, Apr 16, 2021 at 12:21:56PM +0200, Claudio Jeker wrote:
> This diff changes the http module to support keep-alive.
> It splits requests (for a resource) from connections (to a server).
> When a request is received the code tries to first use a IDLE connection,
> if none is around a new connection is started (unless there are too many
> connections inflight).
> 
> Idle connections are kept for 10sec and closed after that time. For
> rpki-client this should work well since the RRDP exchange will be a burtst
> of requests (one after another). There is only one server that is
> connected twice during the run (one server hosting 2 repos).
> 
> The benefit of using keep-alive is less CPU time wasted on constant TLS
> handshakes. I did not notice any speed improvements.
> 
> After fixing most issues in the http module the last few days this is less
> urgent to go in. Still sending it out so people can play with it.

Updated diff to apply after all the KNF commits.

-- 
:wq Claudio

Index: http.c
===
RCS file: /cvs/src/usr.sbin/rpki-client/http.c,v
retrieving revision 1.31
diff -u -p -r1.31 http.c
--- http.c  19 Apr 2021 17:04:35 -  1.31
+++ http.c  20 Apr 2021 08:06:56 -
@@ -48,6 +48,7 @@
 #include 
 #include 
 
+#include 
 #include 
 #include 
 #include 
@@ -66,25 +67,31 @@
 
 #include "extern.h"
 
-#define HTTP_USER_AGENT"OpenBSD rpki-client"
-#define HTTP_BUF_SIZE  (32 * 1024)
-#define MAX_CONNECTIONS12
-
-#define WANT_POLLIN1
-#define WANT_POLLOUT   2
+#define HTTP_USER_AGENT"OpenBSD rpki-client"
+#define HTTP_BUF_SIZE  (32 * 1024)
+#define HTTP_IDLE_TIMEOUT  10
+#define MAX_CONNECTIONS64
+#define NPFDS  (MAX_CONNECTIONS + 1)
+
+enum res {
+   DONE,
+   WANT_POLLIN,
+   WANT_POLLOUT,
+};
 
 enum http_state {
STATE_FREE,
-   STATE_INIT,
STATE_CONNECT,
STATE_TLSCONNECT,
STATE_REQUEST,
STATE_RESPONSE_STATUS,
STATE_RESPONSE_HEADER,
STATE_RESPONSE_DATA,
-   STATE_RESPONSE_CHUNKED,
+   STATE_RESPONSE_CHUNKED_HEADER,
+   STATE_RESPONSE_CHUNKED_TRAILER,
STATE_WRITE_DATA,
-   STATE_DONE,
+   STATE_IDLE,
+   STATE_CLOSE,
 };
 
 struct http_proxy {
@@ -94,50 +101,108 @@ struct http_proxy {
 };
 
 struct http_connection {
-   char*url;
+   LIST_ENTRY(http_connection) entry;
char*host;
char*port;
-   const char  *path;  /* points into url */
-   char*modified_since;
char*last_modified;
+   char*redir_uri;
+   struct http_request *req;
+   struct pollfd   *pfd;
struct addrinfo *res0;
struct addrinfo *res;
struct tls  *tls;
char*buf;
size_t  bufsz;
size_t  bufpos;
-   size_t  id;
off_t   iosz;
+   time_t  idle_time;
int status;
-   int redirect_loop;
int fd;
-   int outfd;
+   int chunked;
+   int keep_alive;
short   events;
-   short   chunked;
enum http_state state;
 };
 
-struct msgbuf msgq;
-struct sockaddr_storage http_bindaddr;
-struct tls_config *tls_config;
-uint8_t *tls_ca_mem;
-size_t tls_ca_size;
+LIST_HEAD(http_conn_list, http_connection);
 
+struct http_request {
+   TAILQ_ENTRY(http_request)   entry;
+   char*uri;
+   char*modified_since;
+   char*host;
+   char*port;
+   const char  *path;  /* points into uri */
+   size_t   id;
+   int  outfd;
+   int  redirect_loop;
+};
+
+TAILQ_HEAD(http_req_queue, http_request);
+
+static struct http_conn_list   active = LIST_HEAD_INITIALIZER(active);
+static struct http_conn_list   idle = LIST_HEAD_INITIALIZER(idle);
+static struct http_req_queue   queue = TAILQ_HEAD_INITIALIZER(queue);
+static size_t http_conn_count;
+
+static struct msgbuf msgq;
+static struct sockaddr_storage http_bindaddr;
+static struct tls_config *tls_config;
+static uint8_t *tls_ca_mem;
+static size_t tls_ca_size;
+
+/* HTTP request API */
+static voidhttp_req_new(size_t, char *, char *, int);
+static voidhttp_req_free(struct http_request *);
+static voidhttp_req_done(size_t, enum http_result, const char *);
+static voidhttp_req_fail(size_t);
+static int http_req_schedule(struct http_request *);
+
+/* HTTP connection API */

Re: rpki-client http keep-alive support

2021-04-16 Thread Theo de Raadt
> The benefit of using keep-alive is less CPU time wasted on constant TLS
> handshakes. I did not notice any speed improvements.

And... not bouncing between inconsistant CA endpoints.  Further discussion
about that problem will occur elsewhere.