tcp: change tls send callback interface

Instead of 2 different tls send callbacks (with a 3rd one needed),
switch to a different model: 1 tls callback that is supposed to
replace the passed buffer with a tls processed version of it.
This simplifies the tls code and more importantly doesn't require
that the tls send code has very detailed knowledge about the tcp
state machine. Some of the saved complexity moved from the tls
module to the tcp code, but at least this way on changes there's
only one place to update.
The tls callbacks for reading and sending are now very different:
while the send callback has become now more of an encoder
callback, the read callback should still perform the tcp read by
itself. While this is not very consistent it does saves unneeded
memory copies.

tcp_int_send.h

@@ -29,20 +29,12 @@
 
 #include "tcp_conn.h"
 
-int tcpconn_do_send(int fd, struct tcp_connection* c,
-							char* buf, unsigned len,
-							snd_flags_t send_flags, long* resp, int locked);
-
-int tcpconn_1st_send(int fd, struct tcp_connection* c,
-							char* buf, unsigned len,
-							snd_flags_t send_flags, long* resp, int locked);
-
 int tcpconn_send_unsafe(int fd, struct tcp_connection *c,
-						char* buf, unsigned len, snd_flags_t send_flags);
+						const char* buf, unsigned len, snd_flags_t send_flags);
 
 /* direct non-blocking, unsafe (assumes locked) send on a tcp connection */
 int _tcpconn_write_nb(int fd, struct tcp_connection* c,
-									char* buf, int len);
+									const char* buf, int len);
 
 
 #endif /*__tcp_int_send_h*/

tcp_main.c

@@ -642,7 +642,7 @@ end:
 
 
 /* unsafe version, call while holding the connection write lock */
-inline static int _wbufq_add(struct  tcp_connection* c, char* data, 
+inline static int _wbufq_add(struct  tcp_connection* c, const char* data, 
 							unsigned int size)
 {
 	struct tcp_wbuffer_queue* q;
@@ -661,8 +661,8 @@ inline static int _wbufq_add(struct  tcp_connection* c, char* data,
 		LOG(L_ERR, "ERROR: wbufq_add(%d bytes): write queue full or timeout "
 					" (%d, total %d, last write %d s ago)\n",
 					size, q->queued, *tcp_total_wq,
-					TICKS_TO_S(t-q->wr_timeout-
-						cfg_get(tcp, tcp_cfg, send_timeout)));
+					TICKS_TO_S(t-(q->wr_timeout-
+								cfg_get(tcp, tcp_cfg, send_timeout))));
 		if (q->first && TICKS_LT(q->wr_timeout, t)){
 			if (unlikely(c->state==S_CONN_CONNECT)){
 #ifdef USE_DST_BLACKLIST
@@ -740,7 +740,7 @@ error:
  * inserts data at the beginning, it ignores the max queue size checks and
  * the timeout (use sparingly)
  * Note: it should never be called on a write buffer after wbufq_run() */
-inline static int _wbufq_insert(struct  tcp_connection* c, char* data, 
+inline static int _wbufq_insert(struct  tcp_connection* c, const char* data, 
 							unsigned int size)
 {
 	struct tcp_wbuffer_queue* q;
@@ -1714,8 +1714,15 @@ inline static void tcp_fd_cache_add(struct tcp_connection *c, int fd)
 
 inline static int tcpconn_chld_put(struct tcp_connection* tcpconn);
 
-static int tcpconn_send_put(struct tcp_connection* c, char* buf, unsigned len,
-							snd_flags_t send_flags);
+static int tcpconn_send_put(struct tcp_connection* c, const char* buf,
+							unsigned len, snd_flags_t send_flags);
+static int tcpconn_do_send(int fd, struct tcp_connection* c,
+							const char* buf, unsigned len,
+							snd_flags_t send_flags, long* resp, int locked);
+
+static int tcpconn_1st_send(int fd, struct tcp_connection* c,
+							const char* buf, unsigned len,
+							snd_flags_t send_flags, long* resp, int locked);
 
 /* finds a tcpconn & sends on it
  * uses the dst members to, proto (TCP|TLS) and id and tries to send
@@ -1723,7 +1730,7 @@ static int tcpconn_send_put(struct tcp_connection* c, char* buf, unsigned len,
  * returns: number of bytes written (>=0) on success
  *          <0 on error */
 int tcp_send(struct dest_info* dst, union sockaddr_union* from,
-					char* buf, unsigned len)
+					const char* buf, unsigned len)
 {
 	struct tcp_connection *c;
 	struct ip_addr ip;
@@ -1841,30 +1848,47 @@ int tcp_send(struct dest_info* dst, union sockaddr_union* from,
 			 * pending and nobody else will try to write on it. However
 			 * this might produce out-of-order writes. If this is not
 			 * desired either lock before the write or use 
-			 * _wbufq_insert(...) */
+			 * _wbufq_insert(...)
+			 * NOTE2: _wbufq_insert() is used now (no out-of-order).
+			 */
 #ifdef USE_TLS
-			if (unlikely(c->type==PROTO_TLS))
-				n=tls_1st_send(fd, c, buf, len, dst->send_flags,
-									&response[1]);
-			else
+			if (unlikely(c->type==PROTO_TLS)) {
+			/* for TLS the TLS processing and the send must happen
+			   atomically w/ respect to other sends on the same connection
+			   (otherwise reordering might occur which would break TLS) =>
+			   lock. However in this case this send will always be the first
+			   => we can have the send() out of the lock.
+			*/
+				lock_get(&c->write_lock);
+					n = tls_encode(c, &buf, &len);
+				lock_release(&c->write_lock);
+				if (likely(n >= 0 && len))
+					n=tcpconn_1st_send(fd, c, buf, len, dst->send_flags,
+										&response[1], 0);
+				else if (n < 0)
+					response[1] = CONN_ERROR;
+				else
+					/* len == 0 => nothing to send  (but still have to
+					   inform tcp_main about the new connection) */
+					response[1] = CONN_NEW_COMPLETE;
+			} else
 #endif /* USE_TLS */
 				n=tcpconn_1st_send(fd, c, buf, len, dst->send_flags,
 									&response[1], 0);
-			if (unlikely(n<0))
+			if (unlikely(n<0)) /* this will catch CONN_ERROR too */
 				goto conn_wait_error;
 			if (unlikely(response[1]==CONN_EOF)){
 				/* if close-after-send requested, don't bother
 				   sending the fd back to tcp_main, try closing it
 				   immediately (no other tcp_send should use it,
 				   because it is marked as close-after-send before
-				   being added to the hash */
+				   being added to the hash) */
 				goto conn_wait_close;
 			}
 			/* send to tcp_main */
 			response[0]=(long)c;
-			if (unlikely(response[1]!=CONN_NOP &&
-						(send_fd(unix_tcp_sock, response,
-									sizeof(response), fd) <= 0))){
+			if (unlikely(send_fd(unix_tcp_sock, response,
+									sizeof(response), fd) <= 0)){
 				LOG(L_ERR, "BUG: tcp_send %s: %ld for %p"
 							" failed:" " %s (%d)\n",
 							su2a(&dst->to, sizeof(dst->to)),
@@ -1906,9 +1930,22 @@ int tcp_send(struct dest_info* dst, union sockaddr_union* from,
 		/* new connection => send on it directly */
 #ifdef USE_TLS
 		if (unlikely(c->type==PROTO_TLS)) {
-			response[1] = CONN_ERROR; /* in case tls is not loaded */
-			n = tls_do_send(fd, c, buf, len, dst->send_flags,
-							&response[1]);
+			/* for TLS the TLS processing and the send must happen
+			   atomically w/ respect to other sends on the same connection
+			   (otherwise reordering might occur which would break TLS) =>
+			   lock.
+			*/
+			lock_get(&c->write_lock);
+				n = tls_encode(c, &buf, &len);
+				if (likely(n > 0))
+					n = tcpconn_do_send(fd, c, buf, len, dst->send_flags,
+											&response[1], 1);
+				else if (n == 0)
+					/* len == 0 => nothing to send */
+					response[1] = CONN_NOP;
+				else  /* n < 0 */
+					response[1] = CONN_ERROR;
+			lock_release(&c->write_lock);
 		} else
 #endif /* USE_TLS */
 			n = tcpconn_do_send(fd, c, buf, len, dst->send_flags,
@@ -2013,8 +2050,8 @@ end_no_conn:
  * @param len - data length,
  * @return >=0 on success, -1 on error.
  */
-static int tcpconn_send_put(struct tcp_connection* c, char* buf, unsigned len,
-							snd_flags_t send_flags)
+static int tcpconn_send_put(struct tcp_connection* c, const char* buf,
+								unsigned len, snd_flags_t send_flags)
 {
 	struct tcp_connection *tmp;
 	int fd;
@@ -2047,7 +2084,19 @@ static int tcpconn_send_put(struct tcp_connection* c, char* buf, unsigned len,
 #endif /* TCP_CONNECT_WAIT */
 				{
 					do_close_fd=0;
-					if (unlikely(_wbufq_add(c, buf, len)<0)){
+#ifdef USE_TLS
+					if (unlikely(c->type==PROTO_TLS)) {
+						n = tls_encode(c, &buf, &len);
+						if (unlikely(n < 0)) {
+							lock_release(&c->write_lock);
+							response[1] = CONN_ERROR;
+							c->state=S_CONN_BAD;
+							c->timeout=get_ticks_raw(); /* force timeout */
+							goto error;
+						}
+					}
+#endif /* USE_TLS */
+					if (unlikely(len && (_wbufq_add(c, buf, len)<0))){
 						lock_release(&c->write_lock);
 						n=-1;
 						response[1] = CONN_ERROR;
@@ -2128,8 +2177,22 @@ static int tcpconn_send_put(struct tcp_connection* c, char* buf, unsigned len,
 	
 #ifdef USE_TLS
 		if (unlikely(c->type==PROTO_TLS)) {
-			response[1] = CONN_ERROR; /* in case tls is not loaded */
-			n = tls_do_send(fd, c, buf, len, send_flags, &response[1]);
+			/* for TLS the TLS processing and the send must happen
+			   atomically w/ respect to other sends on the same connection
+			   (otherwise reordering might occur which would break TLS) =>
+			   lock.
+			*/
+			lock_get(&c->write_lock);
+				n = tls_encode(c, &buf, &len);
+				if (likely(n > 0))
+					n = tcpconn_do_send(fd, c, buf, len, send_flags,
+											&response[1], 1);
+				else if (n == 0)
+					/* len == 0 => nothing to send */
+					response[1] = CONN_NOP;
+				else /* n < 0 */
+					response[1] = CONN_ERROR;
+			lock_release(&c->write_lock);
 		} else
 #endif
 			n = tcpconn_do_send(fd, c, buf, len, send_flags, &response[1], 0);
@@ -2202,7 +2265,7 @@ release_c:
  * @return <0 on error, number of bytes sent on success.
  */
 int tcpconn_send_unsafe(int fd, struct tcp_connection *c,
-						char* buf, unsigned len, snd_flags_t send_flags)
+						const char* buf, unsigned len, snd_flags_t send_flags)
 {
 	int n;
 	long response[2];
@@ -2257,8 +2320,8 @@ int tcpconn_send_unsafe(int fd, struct tcp_connection *c,
  * @return >=0 on success, < 0 on error && *resp == CON_ERROR.
  *
  */
-int tcpconn_do_send(int fd, struct tcp_connection* c,
-							char* buf, unsigned len,
+static int tcpconn_do_send(int fd, struct tcp_connection* c,
+							const char* buf, unsigned len,
 							snd_flags_t send_flags, long* resp,
 							int locked)
 {
@@ -2416,25 +2479,26 @@ end:
  * @param buf - data to be sent.
  * @param len - data length.
  * @param send_flags
- * @param resp - filled with a fd sending cmd. for tcp_main on success:
- *                      CONN_NOP - nothing needs to be done (unused right now).
+ * @param resp - filled with a fd sending cmd. for tcp_main on success. It
+ *                      _must_ be one of the commands listed below:
  *                      CONN_NEW_PENDING_WRITE - new connection, first write
  *                                 was partially successful (or EAGAIN) and
  *                                 was queued (connection should be watched
  *                                 for write and the write queue flushed).
  *                                 The fd should be sent to tcp_main.
  *                      CONN_NEW_COMPLETE - new connection, first write
- *                                 completed successfuly and no data is queued.
- *                                 The fd should be sent to tcp_main.
+ *                                 completed successfully and no data is
+ *                                 queued. The fd should be sent to tcp_main.
  *                      CONN_EOF - no error, but the connection should be
  *                                  closed (e.g. SND_F_CON_CLOSE send flag).
+ *                      CONN_ERROR - error, _must_ return < 0.
  * @param locked - if set assume the connection is already locked (call from
  *                  tls) and do not lock/unlock the connection.
  * @return >=0 on success, < 0 on error (on error *resp is undefined).
  *
  */
-int tcpconn_1st_send(int fd, struct tcp_connection* c,
-							char* buf, unsigned len,
+static int tcpconn_1st_send(int fd, struct tcp_connection* c,
+							const char* buf, unsigned len,
 							snd_flags_t send_flags, long* resp,
 							int locked)
 {
@@ -2682,8 +2746,10 @@ close_again:
 	if (unlikely(close(fd)<0)){
 		if (errno==EINTR)
 			goto close_again;
-		LOG(L_ERR, "ERROR: tcpconn_put_destroy; close() failed: %s (%d)\n",
-				strerror(errno), errno);
+		LOG(L_ERR, "ERROR: tcpconn_close_main_fd(%p): %s "
+					"close(%d) failed (flags 0x%x): %s (%d)\n", tcpconn,
+					su2a(&tcpconn->rcv.src_su, sizeof(tcpconn->rcv.src_su)),
+					fd, tcpconn->flags, strerror(errno), errno);
 	}
 }
 
@@ -2737,6 +2803,7 @@ inline static void tcpconn_destroy(struct tcp_connection* tcpconn)
 		}
 		if (likely(!(tcpconn->flags & F_CONN_FD_CLOSED))){
 			tcpconn_close_main_fd(tcpconn);
+			tcpconn->flags|=F_CONN_FD_CLOSED;
 			(*tcp_connections_no)--;
 		}
 		_tcpconn_free(tcpconn); /* destroys also the wbuf_q if still present*/
@@ -2999,7 +3066,7 @@ rm_con:
  *  returns number of bytes written on success, -1 on error (and sets errno)
  */
 int _tcpconn_write_nb(int fd, struct tcp_connection* c,
-									char* buf, int len)
+									const char* buf, int len)
 {
 	int n;
 	

tcp_server.h

@@ -35,7 +35,7 @@
 /* "public" functions*/
 
 int tcp_send(struct dest_info* dst, union sockaddr_union* from,
-				char* buf, unsigned len);
+				const char* buf, unsigned len);
 
 int tcpconn_add_alias(int id, int port, int proto);
 

tls_hooks.c

@@ -37,7 +37,7 @@
 
 #ifdef TLS_HOOKS
 
-struct tls_hooks tls_hook= {0, 0, 0, 0, 0 ,0 ,0 ,0 ,0 };
+struct tls_hooks tls_hook= {0, 0, 0, 0, 0 ,0 ,0};
 
 static int tls_hooks_loaded=0;
 

tls_hooks.h

@@ -51,15 +51,16 @@
 
 
 struct tls_hooks{
+	/* read using tls (should use tcp internal read functions to
+	   get the data from the connection) */
 	int  (*read)(struct tcp_connection* c, int* flags);
-	/* send using tls on a tcp connection */
-	int (*do_send)(int fd, struct tcp_connection* c, const char* buf,
-							unsigned int len, snd_flags_t send_flags,
-							long* resp);
-	/* 1st send using tls on a new async. tcp connection */
-	int (*fst_send)(int fd, struct tcp_connection* c, const char* buf,
-							unsigned int len, snd_flags_t send_flags,
-							long* resp);
+	/* process data for sending. Should replace pbuf & plen with
+	   an internal buffer containing the tls records.
+	   Should return *plen (if >=0).
+	   If it returns < 0 => error (tcp connection will be closed).
+	*/
+	int (*encode)(struct tcp_connection* c, const char** pbuf,
+						unsigned int* plen);
 	int  (*on_tcpconn_init)(struct tcp_connection *c, int sock);
 	void (*tcpconn_clean)(struct tcp_connection* c);
 	void (*tcpconn_close)(struct tcp_connection*c , int fd);
@@ -98,10 +99,8 @@ extern struct tls_hooks tls_hook;
 
 #define tls_tcpconn_init(c, s)	tls_hook_call(on_tcpconn_init, 0, (c), (s))
 #define tls_tcpconn_clean(c)	tls_hook_call_v(tcpconn_clean, (c))
-#define tls_do_send(fd, c, buf, len, send_flags, resp) \
-	tls_hook_call(do_send, -1, (fd), (c), (buf), (len), (send_flags), (resp))
-#define tls_1st_send(fd, c, buf, len, send_flags, resp) \
-	tls_hook_call(fst_send, -1, (fd), (c), (buf), (len), (send_flags), (resp))
+#define tls_encode(c, pbuf, plen) \
+	tls_hook_call(encode, -1, (c), (pbuf), (plen))
 #define tls_close(conn, fd)		tls_hook_call_v(tcpconn_close, (conn), (fd))
 #define tls_read(c, flags)				tls_hook_call(read, -1, (c), (flags))