Module: sip-router
Branch: andrei/tcp_tls_changes
Commit: ab88df95e9fb22ca539bf399d597a844a63da676
URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=ab88df9…
Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org>
Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org>
Date: Fri Jul 16 15:14:11 2010 +0200
tcp: verbose and safer close()
- added tcp_safe_close(): a wrapper over close().
- repeat every close() on EINTR (not only the one called from
tcp_main). In general according to posix the state of the file
descriptor after an interrupted close() is undefined. However in
the ser case it's always safe to retry the close (there are no
other threads that might open new fds while the close() is
retried).
- ignore "expected" close() errors. On *BSDs (for example)
close()-ing a socket will return the last error
(e.g it can return ECONNRESET, EPIPE a.s.o.).
Note that this is not true on linux.
- on close failure be more verbose if there is enough information
for a useful error message (the corresp. tcp connection
structure is known and can still be used).
---
tcp_main.c | 94 ++++++++++++++++++++++++++++++++++++++++++++---------------
1 files changed, 70 insertions(+), 24 deletions(-)
diff --git a/tcp_main.c b/tcp_main.c
index aa4fc52..a1761f6 100644
--- a/tcp_main.c
+++ b/tcp_main.c
@@ -214,7 +214,6 @@
#endif
#define MAX_SEND_FD_QUEUE_SIZE tcp_main_max_fd_no
#define SEND_FD_QUEUE_SIZE 128 /* initial size */
-#define MAX_SEND_FD_RETRIES 96 /* FIXME: not used for now */
#define SEND_FD_QUEUE_TIMEOUT MS_TO_TICKS(2000) /* 2 s */
#endif
@@ -480,6 +479,42 @@ error:
+/** close a socket, handling errno.
+ * On EINTR, repeat the close().
+ * Filter expected errors (return success if close() failed because
+ * EPIPE, ECONNRST a.s.o). Note that this happens on *BSDs (on linux close()
+ * does not fail for socket level errors).
+ * @param s - open valid socket.
+ * @return - 0 on success, < 0 on error (whatever close() returns). On error
+ * errno is set.
+ */
+static int tcp_safe_close(int s)
+{
+ int ret;
+retry:
+ if (unlikely((ret = close(s)) < 0 )) {
+ switch(errno) {
+ case EINTR:
+ goto retry;
+ case EPIPE:
+ case ENOTCONN:
+ case ECONNRESET:
+ case ECONNREFUSED:
+ case ENETUNREACH:
+ case EHOSTUNREACH:
+ /* on *BSD we really get these errors at close() time
+ => ignore them */
+ ret = 0;
+ break;
+ default:
+ break;
+ }
+ }
+ return ret;
+}
+
+
+
/* blocking connect on a non-blocking fd; it will timeout after
* tcp_connect_timeout
* if BLOCKING_USE_SELECT and HAVE_SELECT are defined it will internally
@@ -1201,7 +1236,7 @@ find_socket:
*res_local_addr=*from;
return s;
error:
- if (s!=-1) close(s);
+ if (s!=-1) tcp_safe_close(s);
return -1;
}
@@ -1240,9 +1275,8 @@ struct tcp_connection* tcpconn_connect( union sockaddr_union* server,
}
tcpconn_set_send_flags(con, *send_flags);
return con;
- /*FIXME: set sock idx! */
error:
- if (s!=-1) close(s); /* close the opened socket */
+ if (s!=-1) tcp_safe_close(s); /* close the opened socket */
return 0;
}
@@ -1702,7 +1736,7 @@ inline static void tcp_fd_cache_add(struct tcp_connection *c, int fd)
h=c->id%TCP_FD_CACHE_SIZE;
if (likely(fd_cache[h].fd>0))
- close(fd_cache[h].fd);
+ tcp_safe_close(fd_cache[h].fd);
fd_cache[h].fd=fd;
fd_cache[h].id=c->id;
fd_cache[h].con=c;
@@ -2072,14 +2106,14 @@ redo_tls_encode:
again on exit */
if (unlikely(n < 0 || response[1] == CONN_EOF)) {
/* on error or eof, close fd */
- close(fd);
+ tcp_safe_close(fd);
} else if (response[1] == CONN_QUEUED_WRITE) {
#ifdef TCP_FD_CACHE
if (cfg_get(tcp, tcp_cfg, fd_cache)) {
tcp_fd_cache_add(c, fd);
} else
#endif /* TCP_FD_CACHE */
- close(fd);
+ tcp_safe_close(fd);
} else {
BUG("unexpected tcpconn_do_send() return & response:"
" %d, %ld\n", n, response[1]);
@@ -2091,7 +2125,7 @@ redo_tls_encode:
tcp_fd_cache_add(c, fd);
}else
#endif /* TCP_FD_CACHE */
- close(fd);
+ tcp_safe_close(fd);
/* here we can have only commands that _do_ _not_ dec refcnt.
(CONN_EOF, CON_ERROR, CON_QUEUED_WRITE are all treated above) */
goto release_c;
@@ -2107,7 +2141,11 @@ conn_wait_success:
tcp_fd_cache_add(c, fd);
} else
#endif /* TCP_FD_CACHE */
- close(fd);
+ if (unlikely (tcp_safe_close(fd) < 0))
+ LOG(L_ERR, "closing temporary send fd for %p: %s: "
+ "close(%d) failed (flags 0x%x): %s (%d)\n", c,
+ su2a(&c->rcv.src_su, sizeof(c->rcv.src_su)),
+ fd, c->flags, strerror(errno), errno);
tcpconn_chld_put(c); /* release c (dec refcnt & free on 0) */
return n;
conn_wait_error:
@@ -2122,7 +2160,13 @@ conn_wait_close:
c->state=S_CONN_BAD;
/* we are here only if we opened a new fd (and not reused a cached or
a reader one) => if the connect was successful close the fd */
- if (fd>=0) close(fd);
+ if (fd>=0) {
+ if (unlikely(tcp_safe_close(fd) < 0 ))
+ LOG(L_ERR, "closing temporary send fd for %p: %s: "
+ "close(%d) failed (flags 0x%x): %s (%d)\n", c,
+ su2a(&c->rcv.src_su, sizeof(c->rcv.src_su)),
+ fd, c->flags, strerror(errno), errno);
+ }
TCPCONN_LOCK;
if (c->flags & F_CONN_HASHED){
/* if some other parallel tcp_send did send CONN_ERROR to
@@ -2356,17 +2400,17 @@ error:
if (unlikely(fd_cache_e)){
tcp_fd_cache_rm(fd_cache_e);
fd_cache_e = 0;
- close(fd);
+ tcp_safe_close(fd);
}else
#endif /* TCP_FD_CACHE */
- if (do_close_fd) close(fd);
+ if (do_close_fd) tcp_safe_close(fd);
} else if (response[1] == CONN_QUEUED_WRITE) {
#ifdef TCP_FD_CACHE
if (unlikely((fd_cache_e==0) && use_fd_cache)){
tcp_fd_cache_add(c, fd);
}else
#endif /* TCP_FD_CACHE */
- if (do_close_fd) close(fd);
+ if (do_close_fd) tcp_safe_close(fd);
} else {
BUG("unexpected tcpconn_do_send() return & response: %d, %ld\n",
n, response[1]);
@@ -2379,7 +2423,13 @@ end:
tcp_fd_cache_add(c, fd);
}else
#endif /* TCP_FD_CACHE */
- if (do_close_fd) close(fd);
+ if (do_close_fd) {
+ if (unlikely(tcp_safe_close(fd) < 0))
+ LOG(L_ERR, "closing temporary send fd for %p: %s: "
+ "close(%d) failed (flags 0x%x): %s (%d)\n", c,
+ su2a(&c->rcv.src_su, sizeof(c->rcv.src_su)),
+ fd, c->flags, strerror(errno), errno);
+ }
/* here we can have only commands that _do_ _not_ dec refcnt.
(CONN_EOF, CON_ERROR, CON_QUEUED_WRITE are all treated above) */
release_c:
@@ -2859,7 +2909,7 @@ int tcp_init(struct socket_info* sock_info)
return 0;
error:
if (sock_info->socket!=-1){
- close(sock_info->socket);
+ tcp_safe_close(sock_info->socket);
sock_info->socket=-1;
}
return -1;
@@ -2882,15 +2932,11 @@ inline static void tcpconn_close_main_fd(struct tcp_connection* tcpconn)
#ifdef TCP_FD_CACHE
if (likely(cfg_get(tcp, tcp_cfg, fd_cache))) shutdown(fd, SHUT_RDWR);
#endif /* TCP_FD_CACHE */
-close_again:
- if (unlikely(close(fd)<0)){
- if (errno==EINTR)
- goto close_again;
+ if (unlikely(tcp_safe_close(fd)<0))
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);
- }
tcpconn->s=-1;
}
@@ -3917,13 +3963,13 @@ static inline int handle_new_connect(struct socket_info* si)
LOG(L_ERR, "ERROR: maximum number of connections exceeded: %d/%d\n",
*tcp_connections_no,
cfg_get(tcp, tcp_cfg, max_connections));
- close(new_sock);
+ tcp_safe_close(new_sock);
TCP_STATS_LOCAL_REJECT();
return 1; /* success, because the accept was succesfull */
}
if (unlikely(init_sock_opt_accept(new_sock)<0)){
LOG(L_ERR, "ERROR: handle_new_connect: init_sock_opt failed\n");
- close(new_sock);
+ tcp_safe_close(new_sock);
return 1; /* success, because the accept was succesfull */
}
(*tcp_connections_no)++;
@@ -3981,7 +4027,7 @@ static inline int handle_new_connect(struct socket_info* si)
}else{ /*tcpconn==0 */
LOG(L_ERR, "ERROR: handle_new_connect: tcpconn_new failed, "
"closing socket\n");
- close(new_sock);
+ tcp_safe_close(new_sock);
(*tcp_connections_no)--;
}
return 1; /* accept() was succesfull */
@@ -4363,7 +4409,7 @@ static inline void tcpconn_destroy_all()
if (likely(cfg_get(tcp, tcp_cfg, fd_cache)))
shutdown(fd, SHUT_RDWR);
#endif /* TCP_FD_CACHE */
- close(fd);
+ tcp_safe_close(fd);
}
(*tcp_connections_no)--;
c=next;
Module: sip-router
Branch: andrei/tcp_tls_changes
Commit: 6ecd49834d6f683188484f96d46874b48274dd8c
URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=6ecd498…
Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org>
Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org>
Date: Fri Jul 16 15:52:13 2010 +0200
tls: change read_ahead, buffers and freelist defaults
- disable ssl_read_ahead by default. It is not needed anymore
since now we have our own memory-like BIO, which buffers the
socket I/O. While in the normal direct socket access case it's
an important speed-up, in our case it would consume more memory
and introduce a minor slow-down (extra memcpy).
- if the openssl version supports it (>= 1.0.0) default to
ssl_release_buffers = 1 (which instructs openssl to free the
buffers as soon as possible) and ssl_freelist_max = 0 (don't
keep free buffers around). This should decrease openssl memory
consumption with no other impact (since we buffer everything in
our custom BIO anyway).
---
modules/tls/tls_cfg.c | 21 ++++++++++++++++-----
modules/tls/tls_domain.c | 2 +-
2 files changed, 17 insertions(+), 6 deletions(-)
diff --git a/modules/tls/tls_cfg.c b/modules/tls/tls_cfg.c
index 8f1cff9..88b9b0f 100644
--- a/modules/tls/tls_cfg.c
+++ b/modules/tls/tls_cfg.c
@@ -53,10 +53,19 @@ struct cfg_group_tls default_tls_cfg = {
3, /* log */
600, /* con_lifetime (s)*/
1, /* disable_compression */
- -1, /* ssl_release_buffers (use the default: off) */
- -1, /* ssl_freelist_max (use the default: 32) */
- -1, /* ssl_max_send_fragment (use the default: 16k)*/
- 1, /* ssl_read_ahead (set, use -1 for the openssl default value)*/
+#if OPENSSL_VERSION_NUMBER >= 0x01000000L
+ 1, /* ssl_release_buffers (on, avoid extra buffering) */
+#else
+ -1, /* ssl_release_buffers: old openssl, leave it untouched */
+#endif /* openssl >= 1.0.0 */
+#if OPENSSL_VERSION_NUMBER >= 0x01000000L && ! defined OPENSSL_NO_BUF_FREELISTS
+ 0, /* ssl_freelist_max (immediately free) */
+#else
+ -1, /* ssl_freelist_max: old openssl, leave it untouched */
+#endif /* openssl >= 1.0.0 */
+ -1, /* ssl_max_send_fragment (use the default: 16k), requires openssl
+ > 0.9.9 */
+ 0, /* ssl_read_ahead (off, not needed, we have our own buffering BIO)*/
-1, /* low_mem_threshold1 */
-1, /* low_mem_threshold2 */
10*1024*1024, /* ct_wq_max: 10 Mb by default */
@@ -172,7 +181,9 @@ cfg_def_t tls_cfg_def[] = {
" Works only for OpenSSL >= 0.9.9"},
{"ssl_read_ahead", CFG_VAR_INT | CFG_READONLY, -1, 1, 0, 0,
"Enables read ahead, reducing the number of BIO read calls done"
- " internally by the OpenSSL library" },
+ " internally by the OpenSSL library. Note that in newer tls"
+ " module versions it is better to have read ahead disabled, since"
+ " everything it is buffered in memory anyway"},
{"low_mem_threshold1", CFG_VAR_INT | CFG_ATOMIC, -1, 1<<30, 0, 0,
"sets the minimum amount of free memory for accepting new TLS"
" connections (KB)"},
diff --git a/modules/tls/tls_domain.c b/modules/tls/tls_domain.c
index 97dc942..34fc23a 100644
--- a/modules/tls/tls_domain.c
+++ b/modules/tls/tls_domain.c
@@ -828,7 +828,7 @@ int tls_fix_domains_cfg(tls_domains_cfg_t* cfg, tls_domain_t* srv_defaults,
#endif
#endif
#if defined (OPENSSL_NO_BUF_FREELISTS) || OPENSSL_VERSION_NUMBER < 0x01000000L
- if (ssl_freelist_max_len != 0)
+ if (ssl_freelist_max_len >= 0)
ERR("cannot change openssl freelist_max_len, openssl too old"
"(needed at least 1.0.0) or compiled without freelist support"
" (OPENSSL_NO_BUF_FREELIST)\n");
Module: sip-router
Branch: master
Commit: 2b2f4954554edaa7797a3e29da056b80393ec5dd
URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=2b2f495…
Author: Daniel-Constantin Mierla <miconda(a)gmail.com>
Committer: Daniel-Constantin Mierla <miconda(a)gmail.com>
Date: Thu Jul 15 14:02:46 2010 +0200
core: added pv name free function
- some PVs have dynamic name, therefore parsing them at runtime result in
memory leak
- the new attribute allow to free such kind of names
- not the case so far with stable versions but this can happen when using
such PVs with app_lua
---
pvapi.c | 17 ++++++++++++++---
pvar.h | 3 +++
2 files changed, 17 insertions(+), 3 deletions(-)
diff --git a/pvapi.c b/pvapi.c
index bbd4b87..baeddab 100644
--- a/pvapi.c
+++ b/pvapi.c
@@ -1148,14 +1148,25 @@ error:
}
/**
- *
+ * destroy the content of pv_spec_t structure
*/
-void pv_spec_free(pv_spec_t *spec)
+void pv_spec_destroy(pv_spec_t *spec)
{
if(spec==0) return;
- /* TODO: free name if it is PV */
+ /* free name if it is PV */
+ if(spec->pvp.pvn.nfree)
+ spec->pvp.pvn.nfree((void*)(&spec->pvp.pvn));
if(spec->trans)
tr_free((trans_t*)spec->trans);
+}
+
+/**
+ * free the pv_spec_t structure
+ */
+void pv_spec_free(pv_spec_t *spec)
+{
+ if(spec==0) return;
+ pv_spec_destroy(spec);
pkg_free(spec);
}
diff --git a/pvar.h b/pvar.h
index 58fe38b..e7701f2 100644
--- a/pvar.h
+++ b/pvar.h
@@ -84,6 +84,7 @@ enum _pv_type {
typedef enum _pv_type pv_type_t;
typedef int pv_flags_t;
+typedef void (*pv_name_free_f)(void*);
typedef struct _pv_value
{
@@ -95,6 +96,7 @@ typedef struct _pv_value
typedef struct _pv_name
{
int type; /*!< type of name */
+ pv_name_free_f nfree; /*!< function to free name structure */
union {
struct {
int type; /*!< type of int_str name - compatibility with AVPs */
@@ -174,6 +176,7 @@ int pv_set_spec_value(struct sip_msg* msg, pv_spec_p sp, int op,
int pv_printf(struct sip_msg* msg, pv_elem_p list, char *buf, int *len);
int pv_elem_free_all(pv_elem_p log);
void pv_value_destroy(pv_value_t *val);
+void pv_spec_destroy(pv_spec_t *spec);
void pv_spec_free(pv_spec_t *spec);
int pv_spec_dbg(pv_spec_p sp);
int pv_get_spec_index(struct sip_msg* msg, pv_param_p ip, int *idx, int *flags);
ctl command cfg.help fails like this:
# sip-proxy_ctl cfg.help
error: 400 - error at parameter 0: expected string type but end of packet encountered
-- juha