sr-dev June 2010

sr-dev@lists.kamailio.org

26 participants
164 discussions

git:master: modules/mediaproxy: Fix SDP mangling for unsupported streams.
by Juha Heinanen 20 Jun '10

20 Jun '10

Module: sip-router Branch: master Commit: 5f78020b2d7ba364becf5e5c8b6f5105f560011c URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=5f78020… Author: Juha Heinanen <jh(a)tutpro.com> Committer: Juha Heinanen <jh(a)tutpro.com> Date: Sun Jun 20 16:21:12 2010 +0300 modules/mediaproxy: Fix SDP mangling for unsupported streams. - Credits to Saul Ibarra. --- modules/mediaproxy/mediaproxy.c | 6 +++++- 1 files changed, 5 insertions(+), 1 deletions(-) diff --git a/modules/mediaproxy/mediaproxy.c b/modules/mediaproxy/mediaproxy.c index 67a0c72..855fb7d 100644 --- a/modules/mediaproxy/mediaproxy.c +++ b/modules/mediaproxy/mediaproxy.c @@ -1733,7 +1733,7 @@ use_media_proxy(struct sip_msg *msg, char *dialog_id, ice_candidate_data *ice_da } } - for (i=0, j=1; i<session.stream_count && j<len; i++) { + for (i=0, j=1; i<session.stream_count; i++) { stream = session.streams[i]; if (stream.transport != TSupported) { if (!stream.local_ip && removed_session_ip) { @@ -1748,6 +1748,10 @@ use_media_proxy(struct sip_msg *msg, char *dialog_id, ice_candidate_data *ice_da continue; } + if (j >= len) { + break; + } + if (!isnullport(stream.port)) { if (!replace_element(msg, &stream.port, &tokens[j])) { LM_ERR("failed to replace port in media stream number %d\n", i+1);

1 0

git:andrei/tcp_tls_changes: tcp: more consistent IO_FD_CLOSING usage
by Andrei Pelinescu-Onciul 19 Jun '10

19 Jun '10

Module: sip-router Branch: andrei/tcp_tls_changes Commit: 504ef98ed34366303476032013c28a4a7d1c8131 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=504ef98… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Sat Jun 19 00:21:58 2010 +0200 tcp: more consistent IO_FD_CLOSING usage - extra safety checks before using IO_FD_CLOSING in io_watch_del() calls (must be used only when the fd will be close() imediately afterwards). - add IO_FD_CLOSING when POLLERR or POLLHUP is detected in tcp_main and the socket receive buffer is empty (in this case the fd will be shortly closed) --- tcp_main.c | 79 ++++++++++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 59 insertions(+), 20 deletions(-) diff --git a/tcp_main.c b/tcp_main.c index dcdb6fc..487a442 100644 --- a/tcp_main.c +++ b/tcp_main.c @@ -3099,18 +3099,31 @@ inline static int handle_tcp_child(struct tcp_child* tcp_c, int fd_i) case CONN_RELEASE: tcp_c->busy--; if (unlikely(tcpconn_put(tcpconn))){ + /* if refcnt was 1 => it was used only in the + tcp reader => it's not hashed or watched for IO + anymore => no need to io_watch_del() */ tcpconn_destroy(tcpconn); break; } if (unlikely(tcpconn->state==S_CONN_BAD)){ + if (tcpconn_try_unhash(tcpconn)) { #ifdef TCP_ASYNC - if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ - io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ + io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + tcpconn->flags &= ~F_CONN_WRITE_W; + } +#endif /* TCP_ASYNC */ + tcpconn_put_destroy(tcpconn); + } +#ifdef TCP_ASYNC + else if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ + /* should never happen: if it's already unhashed, it + should not be watched for IO */ + BUG("unhashed connection watched for write\n"); + io_watch_del(&io_h, tcpconn->s, -1, 0); tcpconn->flags &= ~F_CONN_WRITE_W; } #endif /* TCP_ASYNC */ - if (tcpconn_try_unhash(tcpconn)) - tcpconn_put_destroy(tcpconn); break; } /* update the timeout*/ @@ -3147,12 +3160,17 @@ inline static int handle_tcp_child(struct tcp_child* tcp_c, int fd_i) TCP_EV_SEND_TIMEOUT(0, &tcpconn->rcv); TCP_STATS_SEND_TIMEOUT(); } - if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ - io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + if (tcpconn_try_unhash(tcpconn)) { + if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ + io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + tcpconn->flags&=~F_CONN_WRITE_W; + } + tcpconn_put_destroy(tcpconn); + } else if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ + BUG("unhashed connection watched for write\n"); + io_watch_del(&io_h, tcpconn->s, -1, 0); tcpconn->flags&=~F_CONN_WRITE_W; } - if (tcpconn_try_unhash(tcpconn)) - tcpconn_put_destroy(tcpconn); break; }else{ crt_timeout=MIN_unsigned(con_lifetime, @@ -3177,14 +3195,22 @@ inline static int handle_tcp_child(struct tcp_child* tcp_c, int fd_i) LOG(L_CRIT, "ERROR: tcp_main: handle_tcp_child: failed to add" " new socket to the fd list\n"); tcpconn->flags&=~F_CONN_READ_W; + if (tcpconn_try_unhash(tcpconn)) { #ifdef TCP_ASYNC - if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ - io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + if (unlikely(tcpconn->flags & F_CONN_WRITE_W)){ + io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); + tcpconn->flags&=~F_CONN_WRITE_W; + } +#endif /* TCP_ASYNC */ + tcpconn_put_destroy(tcpconn); + } +#ifdef TCP_ASYNC + else if (unlikely(tcpconn->flags & F_CONN_WRITE_W)) { + BUG("unhashed connection watched for write\n"); + io_watch_del(&io_h, tcpconn->s, -1, 0); tcpconn->flags&=~F_CONN_WRITE_W; } #endif /* TCP_ASYNC */ - if (tcpconn_try_unhash(tcpconn)) - tcpconn_put_destroy(tcpconn); break; } DBG("handle_tcp_child: CONN_RELEASE %p refcnt= %d\n", @@ -3457,10 +3483,16 @@ inline static int handle_ser_child(struct process_table* p, int fd_i) POLLIN|POLLOUT, -1)<0)){ LOG(L_CRIT, "ERROR: tcp_main: handle_ser_child:" " failed to change socket watch events\n"); - io_watch_del(&io_h, tcpconn->s, -1, IO_FD_CLOSING); - tcpconn->flags&=~F_CONN_READ_W; - if (tcpconn_try_unhash(tcpconn)) + if (tcpconn_try_unhash(tcpconn)) { + io_watch_del(&io_h, tcpconn->s, -1, + IO_FD_CLOSING); + tcpconn->flags&=~F_CONN_READ_W; tcpconn_put_destroy(tcpconn); + } else { + BUG("unhashed connection watched for IO\n"); + io_watch_del(&io_h, tcpconn->s, -1, 0); + tcpconn->flags&=~F_CONN_READ_W; + } break; } } @@ -3782,10 +3814,6 @@ inline static int handle_tcpconn_ev(struct tcp_connection* tcpconn, short ev, (wbufq_run(tcpconn->s, tcpconn, &empty_q)<0) || (empty_q && tcpconn_close_after_send(tcpconn)) )){ - if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, 0)<0)){ - LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del(1) failed:" - " for %p, fd %d\n", tcpconn, tcpconn->s); - } if ((tcpconn->flags & F_CONN_READ_W) && (ev & POLLIN)){ /* connection is watched for read and there is a read event * (unfortunately if we have POLLIN here we don't know if @@ -3798,6 +3826,11 @@ inline static int handle_tcpconn_ev(struct tcp_connection* tcpconn, short ev, * conn. to a a child only if needed (another syscall + at * least 2 * syscalls in the reader + ...) */ if ((ioctl(tcpconn->s, FIONREAD, &bytes)>=0) && (bytes>0)){ + if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, 0)<0)){ + LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del(1)" + " failed: for %p, fd %d\n", + tcpconn, tcpconn->s); + } tcpconn->flags&=~(F_CONN_WRITE_W|F_CONN_READ_W| F_CONN_WANTS_RD|F_CONN_WANTS_WR); tcpconn->flags|=F_CONN_FORCE_EOF|F_CONN_WR_ERROR; @@ -3805,6 +3838,11 @@ inline static int handle_tcpconn_ev(struct tcp_connection* tcpconn, short ev, } /* if bytes==0 or ioctl failed, destroy the connection now */ } + if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, + IO_FD_CLOSING) < 0)){ + LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del() failed:" + " for %p, fd %d\n", tcpconn, tcpconn->s); + } tcpconn->flags&=~(F_CONN_WRITE_W|F_CONN_READ_W| F_CONN_WANTS_RD|F_CONN_WANTS_WR); if (unlikely(ev & POLLERR)){ @@ -3898,7 +3936,8 @@ send_to_child: tcpconn->flags&=~F_CONN_READER; #ifdef TCP_ASYNC if (tcpconn->flags & F_CONN_WRITE_W){ - if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, 0)<0)){ + if (unlikely(io_watch_del(&io_h, tcpconn->s, fd_i, + IO_FD_CLOSING) < 0)){ LOG(L_ERR, "ERROR: handle_tcpconn_ev: io_watch_del(4)" " failed:" " for %p, fd %d\n", tcpconn, tcpconn->s);

1 0

git:andrei/tcp_tls_changes: io_wait: kqueue: use a bigger array
by Andrei Pelinescu-Onciul 19 Jun '10

19 Jun '10

Module: sip-router Branch: andrei/tcp_tls_changes Commit: 99682621f360bcbd5b0def1f7498f457e25f59ab URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=9968262… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Sat Jun 19 00:16:38 2010 +0200 io_wait: kqueue: use a bigger array Use a bigger array for kevent(). Instead of the fd number, use 2* max_fd_no (2 because read and write events are not merged) and with extra space for possible changelist errors. (minor optimization) --- io_wait.c | 15 ++++++++++++--- io_wait.h | 9 ++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/io_wait.c b/io_wait.c index 0c73402..f0e01be 100644 --- a/io_wait.c +++ b/io_wait.c @@ -549,13 +549,21 @@ int init_io_wait(io_wait_h* h, int max_fd, enum poll_types poll_method) #endif #ifdef HAVE_KQUEUE case POLL_KQUEUE: - h->kq_array=local_malloc(sizeof(*(h->kq_array))*h->max_fd_no); + h->kq_changes_size=KQ_CHANGES_ARRAY_SIZE; + /* kevent returns different events for read & write + => to get all the possible events in one call we + need twice the number of added fds + space + for possible changelist errors. + OTOH if memory is to be saved at all costs, one can + decrease the array size. + */ + h->kq_array_size=2 * h->max_fd_no + h->kq_changes_size; + h->kq_array=local_malloc(sizeof(*(h->kq_array))*h->kq_array_size); if (h->kq_array==0){ LOG(L_CRIT, "ERROR: init_io_wait: could not alloc" " kqueue event array\n"); goto error; } - h->kq_changes_size=KQ_CHANGES_ARRAY_SIZE; h->kq_changes=local_malloc(sizeof(*(h->kq_changes))* h->kq_changes_size); if (h->kq_changes==0){ @@ -564,7 +572,8 @@ int init_io_wait(io_wait_h* h, int max_fd, enum poll_types poll_method) goto error; } h->kq_nchanges=0; - memset((void*)h->kq_array, 0, sizeof(*(h->kq_array))*h->max_fd_no); + memset((void*)h->kq_array, 0, + sizeof(*(h->kq_array))*h->kq_array_size); memset((void*)h->kq_changes, 0, sizeof(*(h->kq_changes))* h->kq_changes_size); if (init_kqueue(h)<0){ diff --git a/io_wait.h b/io_wait.h index c28f53d..e04efd8 100644 --- a/io_wait.h +++ b/io_wait.h @@ -148,7 +148,9 @@ struct io_wait_handler{ int flags; struct fd_map* fd_hash; int fd_no; /* current index used in fd_array and the passed size for - ep_array & kq_array*/ + ep_array (for kq_array at least + max(twice the size, kq_changes_size) should be + be passed). */ int max_fd_no; /* maximum fd no, is also the size of fd_array, fd_hash and ep_array*/ /* common stuff for POLL, SIGIO_RT and SELECT @@ -170,6 +172,7 @@ struct io_wait_handler{ struct kevent* kq_array; /* used for the eventlist*/ struct kevent* kq_changes; /* used for the changelist */ size_t kq_nchanges; + size_t kq_array_size; /* array size */ size_t kq_changes_size; /* size of the changes array */ #endif #ifdef HAVE_DEVPOLL @@ -1115,7 +1118,7 @@ inline static int io_wait_loop_kqueue(io_wait_h* h, int t, int repeat) do { again: n=kevent(h->kq_fd, h->kq_changes, apply_changes, h->kq_array, - h->fd_no, &tspec); + h->kq_array_size, &tspec); if (unlikely(n==-1)){ if (unlikely(errno==EINTR)) goto again; /* signal, ignore it */ else { @@ -1127,7 +1130,7 @@ again: /* some of the FDs in kq_changes are bad (already closed) and there is not enough space in kq_array to return all of them back */ - apply_changes = h->fd_no; + apply_changes = h->kq_array_size; goto again; } }

1 0

git:andrei/tcp_tls_changes: tls: fix initial state error handling
by Andrei Pelinescu-Onciul 19 Jun '10

19 Jun '10

Module: sip-router Branch: andrei/tcp_tls_changes Commit: 42b8ccc2b8a68a874c5b89cfa45d470c364b6bdc URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=42b8ccc… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Sat Jun 19 17:03:45 2010 +0200 tls: fix initial state error handling tls_connect() and tls_accept() non openssl related errors (*error == SSL_ERROR_NONE) were treated as "normal" openssl errors. This could result in silently dropped packets in low memory conditions, instead of dropping the whole underlying tcp connection. --- modules/tls/tls_server.c | 68 ++++++++++++++++++++++++++++++++++----------- 1 files changed, 51 insertions(+), 17 deletions(-) diff --git a/modules/tls/tls_server.c b/modules/tls/tls_server.c index be661de..121a016 100644 --- a/modules/tls/tls_server.c +++ b/modules/tls/tls_server.c @@ -226,9 +226,13 @@ static void tls_dump_cert_info(char* s, X509* cert) * @param c - tcp connection with tls (extra_data must be a filled * tcp_extra_data structure). The state must be S_TLS_ACCEPTING. * @param error set to the error reason (SSL_ERROR_*). + * Note that it can be SSL_ERROR_NONE while the return is < 0 + * ("internal" error, not at the SSL level, see below). * @return >=1 on success, 0 and <0 on error. 0 means the underlying SSL - * connection was closed/shutdown. < 0 is also returned for - * WANT_READ or WANT_WRITE + * connection was closed/shutdown. < 0 is returned for any + * SSL_ERROR (including WANT_READ or WANT_WRITE), but also + * for internal non SSL related errors (in this case -2 is + * returned and error==SSL_ERROR_NONE). * */ int tls_accept(struct tcp_connection *c, int* error) @@ -239,7 +243,8 @@ int tls_accept(struct tcp_connection *c, int* error) struct tls_extra_data* tls_c; int tls_log; - if (LOW_MEM_NEW_CONNECTION_TEST()){ + *error = SSL_ERROR_NONE; + if (unlikely(LOW_MEM_NEW_CONNECTION_TEST())){ ERR("tls: ssl bug #1491 workaround: not enough memory for safe" " operation: %lu\n", shm_available()); goto err; @@ -250,7 +255,6 @@ int tls_accept(struct tcp_connection *c, int* error) if (unlikely(tls_c->state != S_TLS_ACCEPTING)) { BUG("Invalid connection state %d (bug in TLS code)\n", tls_c->state); - /* Not critical */ goto err; } ret = SSL_accept(ssl); @@ -283,18 +287,23 @@ int tls_accept(struct tcp_connection *c, int* error) } return ret; err: - return -1; + /* internal non openssl related errors */ + return -2; } -/** wrapper around SSL_connect, usin SSL return convention. +/** wrapper around SSL_connect, using SSL return convention. * It will also log critical errors and certificate debugging info. * @param c - tcp connection with tls (extra_data must be a filled * tcp_extra_data structure). The state must be S_TLS_CONNECTING. * @param error set to the error reason (SSL_ERROR_*). + * Note that it can be SSL_ERROR_NONE while the return is < 0 + * ("internal" error, not at the SSL level, see below). * @return >=1 on success, 0 and <0 on error. 0 means the underlying SSL - * connection was closed/shutdown. < 0 is also returned for - * WANT_READ or WANT_WRITE + * connection was closed/shutdown. < 0 is returned for any + * SSL_ERROR (including WANT_READ or WANT_WRITE), but also + * for internal non SSL related errors (in this case -2 is + * returned and error==SSL_ERROR_NONE). * */ int tls_connect(struct tcp_connection *c, int* error) @@ -305,7 +314,8 @@ int tls_connect(struct tcp_connection *c, int* error) struct tls_extra_data* tls_c; int tls_log; - if (LOW_MEM_NEW_CONNECTION_TEST()){ + *error = SSL_ERROR_NONE; + if (unlikely(LOW_MEM_NEW_CONNECTION_TEST())){ ERR("tls: ssl bug #1491 workaround: not enough memory for safe" " operation: %lu\n", shm_available()); goto err; @@ -314,10 +324,8 @@ int tls_connect(struct tcp_connection *c, int* error) tls_c=(struct tls_extra_data*)c->extra_data; ssl=tls_c->ssl; - *error = SSL_ERROR_NONE; if (unlikely(tls_c->state != S_TLS_CONNECTING)) { BUG("Invalid connection state %d (bug in TLS code)\n", tls_c->state); - /* Not critical */ goto err; } ret = SSL_connect(ssl); @@ -352,7 +360,8 @@ int tls_connect(struct tcp_connection *c, int* error) } return ret; err: - return -1; + /* internal non openssl related errors */ + return -2; } @@ -598,16 +607,24 @@ redo_wr: n = SSL_write(ssl, buf + offs, len - offs); if (unlikely(n <= 0)) ssl_error = SSL_get_error(ssl, n); - } else + } else { + /* tls_connect failed/needs more IO */ + if (unlikely(n < 0 && ssl_error == SSL_ERROR_NONE)) + goto error; err_src = "TLS connect:"; + } } else if (unlikely(tls_c->state == S_TLS_ACCEPTING)) { n = tls_accept(c, &ssl_error); if (unlikely(n>=1)) { n = SSL_write(ssl, buf + offs, len - offs); if (unlikely(n <= 0)) ssl_error = SSL_get_error(ssl, n); - } else + } else { + /* tls_accept failed/needs more IO */ + if (unlikely(n < 0 && ssl_error == SSL_ERROR_NONE)) + goto error; err_src = "TLS accept:"; + } } else { n = SSL_write(ssl, buf + offs, len - offs); if (unlikely(n <= 0)) @@ -630,6 +647,8 @@ redo_wr: if (unlikely(n <= 0)){ switch(ssl_error) { case SSL_ERROR_NONE: + BUG("unexpected SSL_ERROR_NONE for n=%d\n", n); + goto error; break; case SSL_ERROR_ZERO_RETURN: /* SSL EOF */ @@ -908,6 +927,11 @@ redo_read: if (unlikely(n>=1)) { n = SSL_read(ssl, r->pos, bytes_free); } else { + /* tls_connect failed/needs more IO */ + if (unlikely(n < 0 && ssl_error == SSL_ERROR_NONE)) { + lock_release(&c->write_lock); + goto error; + } err_src = "TLS connect:"; goto ssl_read_skipped; } @@ -916,6 +940,11 @@ redo_read: if (unlikely(n>=1)) { n = SSL_read(ssl, r->pos, bytes_free); } else { + /* tls_accept failed/needs more IO */ + if (unlikely(n < 0 && ssl_error == SSL_ERROR_NONE)) { + lock_release(&c->write_lock); + goto error; + } err_src = "TLS accept:"; goto ssl_read_skipped; } @@ -954,6 +983,10 @@ ssl_read_skipped: lock_release(&c->write_lock); switch(ssl_error) { case SSL_ERROR_NONE: + if (unlikely(n < 0)) { + BUG("unexpected SSL_ERROR_NONE for n=%d\n", n); + goto error; + } break; case SSL_ERROR_ZERO_RETURN: /* SSL EOF */ @@ -967,7 +1000,8 @@ ssl_read_skipped: if (unlikely(rd.pos != rd.used)) { /* data still in the read buffer */ BUG("SSL_ERROR_WANT_READ but data still in" - " the rbio (%d bytes)\n", rd.used - rd.pos); + " the rbio (%p, %d bytes at %d)\n", rd.buf, + rd.used - rd.pos, rd.pos); goto bug; } if (unlikely((*flags & (RD_CONN_EOF | RD_CONN_SHORT_READ)) == 0)) @@ -1029,9 +1063,9 @@ ssl_read_skipped: BUG("unexpected value (n = %d)\n", n); else { if (unlikely(n < bytes_free)) - BUG("read buffer not exhausted (rbio still has %d bytes," + BUG("read buffer not exhausted (rbio %p still has %d bytes," "last SSL_read %d / %d)\n", - rd.used - rd.pos, n, bytes_free); + rd.buf, rd.used - rd.pos, n, bytes_free); /* n <= bytes_free */ /* queue read data if not fully consumed by SSL_read() * (very unlikely situation)

1 0

git:andrei/tcp_tls_changes: io_wait: kqueue: handle ENOENT and more robust error handling
by Andrei Pelinescu-Onciul 18 Jun '10

18 Jun '10

Module: sip-router Branch: andrei/tcp_tls_changes Commit: 8966d0a1787152fa64d1f78321ee539116bd448a URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=8966d0a… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Sat Jun 19 00:44:24 2010 +0200 io_wait: kqueue: handle ENOENT and more robust error handling - handle also ENOENT (along EBADF) when kevent fails due to errors in the changelist. ENOENT can be returned in the following valid scenario: fd scheduled for delayed removal from the watched fd list, fd closed (which automatically removes the fd from the kqueue watched list), new opened fd which gets the same number, delayed changes applied (kevent()). - treat all the other kevent errors or EV_ERRORs in a similar way but log them (at BUG() level). - return POLLERR|POLLHUP for EV_EOF with a non-null fflags. (only kqueue, meaning *bsd and darwin are affected by this fix) --- io_wait.h | 92 +++++++++++++++++++++++++++++++++++++------------------------ 1 files changed, 56 insertions(+), 36 deletions(-) diff --git a/io_wait.h b/io_wait.h index b110b6b..c28f53d 100644 --- a/io_wait.h +++ b/io_wait.h @@ -259,34 +259,33 @@ static inline int kq_ev_change(io_wait_h* h, int fd, int filter, int flag, again: n=kevent(h->kq_fd, h->kq_changes, h->kq_nchanges, 0, 0, &tspec); if (unlikely(n == -1)){ - if (likely(errno == EBADF)) { + if (unlikely(errno == EINTR)) goto again; + else { + /* for a detailed explanation of what follows see + io_wait_loop_kqueue EV_ERROR case */ + if (unlikely(!(errno == EBADF || errno == ENOENT))) + BUG("kq_ev_change: kevent flush changes failed" + " (unexpected error): %s [%d]\n", + strerror(errno), errno); + /* ignore error even if it's not a EBADF/ENOENT */ /* one of the file descriptors is bad, probably already closed => try to apply changes one-by-one */ for (r = 0; r < h->kq_nchanges; r++) { retry2: n = kevent(h->kq_fd, &h->kq_changes[r], 1, 0, 0, &tspec); if (n==-1) { - if (errno == EBADF) - continue; /* skip over it */ - if (errno == EINTR) + if (unlikely(errno == EINTR)) goto retry2; - LOG(L_ERR, "ERROR: io_watch_add: kevent flush changes" - " failed: %s [%d]\n", - strerror(errno), errno); - /* shift the array */ - memmove(&h->kq_changes[0], &h->kq_changes[r+1], - sizeof(h->kq_changes[0])* - (h->kq_nchanges-r-1)); - h->kq_nchanges-=(r+1); - return -1; + /* for a detailed explanation of what follows see + io_wait_loop_kqueue EV_ERROR case */ + if (unlikely(!(errno == EBADF || errno == ENOENT))) + BUG("kq_ev_change: kevent flush changes failed:" + " (unexpected error) %s [%d] (%d/%d)\n", + strerror(errno), errno, + r, h->kq_nchanges); + continue; /* skip over it */ } } - } else if (errno == EINTR) goto again; - else { - LOG(L_ERR, "ERROR: io_watch_add: kevent flush changes" - " failed: %s [%d]\n", strerror(errno), errno); - h->kq_nchanges=0; /* reset changes array */ - return -1; } } h->kq_nchanges=0; /* changes array is empty */ @@ -1118,17 +1117,18 @@ again: n=kevent(h->kq_fd, h->kq_changes, apply_changes, h->kq_array, h->fd_no, &tspec); if (unlikely(n==-1)){ - if (errno==EINTR) goto again; /* signal, ignore it */ - else if (errno==EBADF) { + if (unlikely(errno==EINTR)) goto again; /* signal, ignore it */ + else { + /* for a detailed explanation of what follows see below + the EV_ERROR case */ + if (unlikely(!(errno==EBADF || errno==ENOENT))) + BUG("io_wait_loop_kqueue: kevent: unexpected error" + " %s [%d]\n", strerror(errno), errno); /* some of the FDs in kq_changes are bad (already closed) and there is not enough space in kq_array to return all of them back */ apply_changes = h->fd_no; goto again; - }else{ - LOG(L_ERR, "ERROR: io_wait_loop_kqueue: kevent:" - " %s [%d]\n", strerror(errno), errno); - goto error; } } /* remove applied changes */ @@ -1148,14 +1148,13 @@ again: r, n, h->kq_array[r].ident, (long)h->kq_array[r].udata, h->kq_array[r].flags); #endif - if (unlikely((h->kq_array[r].flags & EV_ERROR) && - (h->kq_array[r].data == EBADF || - h->kq_array[r].udata == 0))){ + if (unlikely((h->kq_array[r].flags & EV_ERROR) || + h->kq_array[r].udata == 0)){ /* error in changes: we ignore it if it has to do with a bad fd or update==0. It can be caused by trying to remove an already closed fd: race between adding something to the - changes array, close() and applying the changes. - E.g. for ser tcp: tcp_main sends a fd to child fore reading + changes array, close() and applying the changes (EBADF). + E.g. for ser tcp: tcp_main sends a fd to child for reading => deletes it from the watched fds => the changes array will contain an EV_DELETE for it. Before the changes are applied (they are at the end of the main io_wait loop, @@ -1163,6 +1162,16 @@ again: to tcp_main by a sender (send fail) is processed and causes the fd to be closed. When the changes are applied => error for the EV_DELETE attempt of a closed fd. + Something similar can happen when a fd is scheduled + for removal, is close()'ed before being removed and + re-opened(a new sock. get the same fd). When the + watched fd changes will be applied the fd will be valid + (so no EBADF), but it's not already watch => ENOENT. + We report a BUG for the other errors (there's nothing + constructive we can do if we get an error we don't know + how to handle), but apart from that we ignore it in the + idea that it is better apply the rest of the changes, + rather then dropping all of them. */ /* example EV_ERROR for trying to delete a read watched fd, @@ -1176,9 +1185,12 @@ again: udata = 0x0 } */ - if (h->kq_array[r].data != EBADF) - LOG(L_INFO, "INFO: io_wait_loop_kqueue: kevent error on " - "fd %ld: %s [%ld]\n", (long)h->kq_array[r].ident, + if (h->kq_array[r].data != EBADF && + h->kq_array[r].data != ENOENT) + BUG("io_wait_loop_kqueue: kevent unexpected error on " + "fd %ld udata %lx: %s [%ld]\n", + (long)h->kq_array[r].ident, + (long)h->kq_array[r].udata, strerror(h->kq_array[r].data), (long)h->kq_array[r].data); }else{ @@ -1186,20 +1198,28 @@ again: if (likely(h->kq_array[r].filter==EVFILT_READ)){ revents=POLLIN | (((int)!(h->kq_array[r].flags & EV_EOF)-1)&POLLHUP) | - (((int)!(h->kq_array[r].flags & EV_ERROR)-1)&POLLERR); + (((int)!((h->kq_array[r].flags & EV_EOF) && + h->kq_array[r].fflags != 0) - 1)&POLLERR); while(fm->type && (fm->events & revents) && (handle_io(fm, revents, -1)>0) && repeat); }else if (h->kq_array[r].filter==EVFILT_WRITE){ revents=POLLOUT | (((int)!(h->kq_array[r].flags & EV_EOF)-1)&POLLHUP) | - (((int)!(h->kq_array[r].flags & EV_ERROR)-1)&POLLERR); + (((int)!((h->kq_array[r].flags & EV_EOF) && + h->kq_array[r].fflags != 0) - 1)&POLLERR); while(fm->type && (fm->events & revents) && (handle_io(fm, revents, -1)>0) && repeat); + }else{ + BUG("io_wait_loop_kqueue: unknown filter: kqueue: event " + "%d/%d: fd=%d, filter=%d, flags=0x%x, fflags=0x%x," + " data=%lx, udata=%lx\n", + r, n, h->kq_array[r].ident, h->kq_array[r].filter, + h->kq_array[r].flags, h->kq_array[r].fflags, + (long)h->kq_array[r].data, (long)h->kq_array[r].udata); } } } } while(unlikely(orig_changes)); -error: return n; } #endif

1 0

git:andrei/tcp_tls_changes: io_wait: fix kqueue io_wait_add & POLLIN
by Andrei Pelinescu-Onciul 18 Jun '10

18 Jun '10

Module: sip-router Branch: andrei/tcp_tls_changes Commit: e5be1a067158c8ba49d33082eb403937546e7c69 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=e5be1a0… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Sat Jun 19 00:35:47 2010 +0200 io_wait: fix kqueue io_wait_add & POLLIN A "goto error" was placed outside the error handling "if", resulting in any io_watch_add(), that tried to enable write watching on a new FD, returning failure (fortunately this kind of io_watch_add() usage doesn't happen very often, usually write watch is enabled via io_watch_chg() on FDs already io_watch_add()'ed for reading). Only POLL_KQUEUE was affected by this bug, meaning the default on all *bsd and darwin. --- io_wait.h | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/io_wait.h b/io_wait.h index 01df1e6..b110b6b 100644 --- a/io_wait.h +++ b/io_wait.h @@ -499,7 +499,7 @@ again2: case POLL_KQUEUE: if (likely( events & POLLIN)){ if (unlikely(kq_ev_change(h, fd, EVFILT_READ, EV_ADD, e)==-1)) - goto error; + goto error; } if (unlikely( events & POLLOUT)){ if (unlikely(kq_ev_change(h, fd, EVFILT_WRITE, EV_ADD, e)==-1)) @@ -507,8 +507,8 @@ again2: if (likely(events & POLLIN)){ kq_ev_change(h, fd, EVFILT_READ, EV_DELETE, 0); } + goto error; } - goto error; } break; #endif

1 0

Git Crash Course For sip-router
by Jan Janak 18 Jun '10

18 Jun '10

Hello, Here is a brief git crash course for those who are used to CVS or SVN. The purpose of this crash course is to get you started, it describes basic operations that you will need to do simple changes in the sip-router git repository. You can find links to more documentation at the end. First of all, you'll need to install git. The name of the corresponding Debian package is git-core. Debian Lenny contains version 1.5.6.5 which is sufficient. Newer git versions or packages for other distributions or architectures can be obtained from http://git-scm.com/. As the second step you need to configure the git client. I have attached configuration files to this email for your convenience, save files .gitconfig and .gitignore in your home directory. File sip-router.org_CA.pem is optional, you will only need the file if you plan on accessing the repository over https (that's not the case for developers with write access). Edit file ~/.gitconfig and configure your name and email address there. This is the name and email address that will be visible in git commits. Unlike CVS or SVN commits, git commits contain full name and email address of the commiter, this is the name and email that will be visible in git history. Note that the email address to be used in From header of commit logs sent to the development mailing list is configured separately on the server (there are technical reasons for this). Here is how I configured my name in email address in ~/.gitconfig: [user] name = Jan Janak email = jan(a)iptel.org The git configuration file in your home directory sets up a bunch of short aliases for most common git commands, so you do not have to type $ git checkout every time you are checking out a branch, you can just type 'git co' instead, and if you alias git to 'g' in your ~/.bashrc then it becomes just 'g co' which is much more convenient. Another setting that is particular to our repository is the number of characters used for tabs in the git pager. Git uses less as the default pager and less is configured to use 8 spaces for tabs by default. We use 4 spaces in our sources and thus you might want to re-configure the git pager in your ~/.bashrc. The following setting makes the output of 'git diff' and friends look prettier: export GIT_PAGER="less --tabs=4" Now you can start working with git. The first command that you need to run is 'git clone', this is the command that is used to retrieve the sip-router repository from the server: $ git clone ssh://<username>@git.sip-router.org/sip-router Replace <username> with your real username. The contents of the repository will be stored in 'sip-router' subdirectory of your current working directory. This command is the git equivalent of cvs/svn checkout. Like cvs/svn it will make the most recent version of the sources available in the subdirectory. UNLIKE cvs/svn, this command does not retrieve just the latest version of the sources, it copies the whole history of the project to your local hard drive, not just the latest revision, but all revisions that are stored in the repository, since tday 1. You do not have to be worried about the size of the repository, the full history of SER takes about 40MB only. Also, you will need to run the command only once (or occassionally if you wipe your local copy out entirely). There are other git commands (covered later) that can be used to keep your local copy up-to-date. Those commands can figure out the difference between your local version of the repository and the version on the server and they will transfer only the data that has changed. In other words, 'git clone' is similar to the scp command, it creates a local identical copy of the remote repository by copying the whole remote repository to the local machine. Once you have a local copy of the sip-router repository, you can try to see what is in there: $ cd sip-router $ git branch The command above shows a list of branches that are present in your local copy of the repository. Most likely you will see just one branch called 'master' marked with an asterisk. This means that your local repository contains only one branch called 'master' and the asterisk tells you that this is the branch that is currently checked out. So the sources that you see in that directory come from the master branch. The local master branch is an exact copy of the 'master' branch from the remote repository at git.sip-router.org and it contains latest sip-router sources. Typically, the branch with the most up-to-date code is called 'master' in git. It is the git equivalent of CVS HEAD branch and SVN trunk branch. NOTE: Branches are sometimes called 'heads' in git documentation, but you can just remember that 'branch' and 'head' refers to the same thing. If you look at the remote version of the repository with gitweb at http://git.sip-router.org/cgi-bin/gitweb.cgi?p=sip-router;a=heads you will notice that there are more branches than the 'master' branch which was created by 'git clone' in the local copy of the repository. The command 'git clone' only makes the master branch directly available in the local copy of the repository. For other branches it merely records the fact that the branches exist in the remote repository. To display branches from both the local and the remote repository you can run: $ git branch -a The output should look roughly like this: * master origin/HEAD origin/MAIN origin/Maintainer origin/andrei/cvshead origin/andrei/sctp origin/andrei/sctp_fixes origin/andrei/sctp_one_to_one origin/andrei_sctp origin/cancel_fix origin/cvs-head origin/janakj/cvshead origin/janakj/doxygen origin/master Branches that start with "origin/" are branches that exist in the remote repository. The string origin is just a short alias for the full URI of the remote repository, such as ssh://git.sip-router.org/sip-router. It is a convention that the repository you cloned from initially is called the origin. Once you have the local repository, you can start making changes in it. Modifying the code on the master branch directly is really easy, because git clone already created the master branch for you in local repository and it checked out the master branch by default. Here is how can you make a simple change in one file and commit the result: $ edit main.c $ git add main.c $ git commit As you can see above you need to call 'git add' before you call 'git commit'. 'git add' marks your changes to be commited during the next commit, this is useful, for example, if you want to leave some local changes uncommited while you make the commit. Unlike CVS or SVN, git does not assume that all local changes should be commited during the next commit. It lets you explicitly select a set of changes to be commited. You can avoid the need to call two commands with 'git commit -a', see the git documentation for more details. When you are done with the commit, you can display the list of changes in the repository with 'git log'. The command displays the history of changes in the whole repository, if you want to limit the list to main.c only then you can run 'git log main.c'. If you look closely at the output of 'git log' then you will notice strange lines starting with "commit" followed by a long string of characters: commit 0253726f99ac151f748712a5d39d71f42d9defc6 The long string is how commits in git are represented. This string is a SHA1 checksum of the commit and it is the git equivalent SVN revision numbers. Like SVN revision numbers and UNLIKE CVS versions, this identifier represents the state of the whole repository, not just individual files. Because such long identifier are tedious to type, git supports abbreviated format, you can type just the beginning of the string as long as the beginning of the string is unique. In other words, there must be at most one commit in the database of all commits with this prefix. For example the following two commands refer to the same commit: $ git diff b0f6ec8784712e4c1436fc9a4a3b54296e94ba5c $ git diff b0f6e If somebody creates another commit with an SHA1 id starting with b0f6e later then 'git diff b0f6e' would not longer work and git would complain that the prefix is ambiguous. All changes, such as the commit above, that you do affect the local repository only. There is absolutely no communication with the the remote repository in any of the commands above (git add, git commit, git log). This is possible because we created a full copy of the remote repository with 'git clone'. This is also the reason why all the commands are so fast, unlike their equivalents in CVS or SVN. The fact that you perform all actions/modifications on the local repository only and then you instruct git to do its black magic and synchronize your local changes with the remote repository is probably the biggest difference from CVS/SVN and one of the things that is hard to understand for people who are used to CVS/SVN. This is also why git falls into the category of distributed revision control systems. When you are satisfied with your local changes/commits, you can decide to make them available to others. This can be done by uploading your local changes to the remote repository (the one you initially cloned from). In git terminology this operation is called 'push', so you can push your local changes to the remote repository with 'git push': $ git push origin master:master The first parameter is the name of the remote repository, the second parameter is the name of the local branch and the name of the remote branch, delimited by ':'. You can omit the 2nd parameter: $ git push origin and then 'git push' will upload all local branches to the remote repository. You can also omit the first parameter: $ git push and in that case repository 'origin' is used. The opposite of 'git push' is 'git pull'. This is the operation that you can use to synchronize your local repository with the remote repository. 'git pull' downloads all changes (made by others) from the remote repository that are not yet in your local repository and integrates them into your local repository. You can run 'git pull' whenever you are online to fetch latest changes and keep your local copy of the repository up-to-date. 'git push' and 'git pull' are similar to rsync, they update either the local or the remote copy of the repository by tranferring only what is missing. 'git push' is the git equivalent of 'cvs ci' and 'svn ci'. 'git pull' is the git equivalent of 'cvs update' or 'svn update'. If you make only small changes to the repository then you can modify the master branch directly, as described above. This approach is suitable only for small, trivial changes. If are working on a change which takes longer to implement, consists of many commits, then it is better to create a separate branch and do the development on a separate branch. Branches in git are cheap, remember that all operations, including branch creation, are local operations. You do not even have to push your local branches into the remote repository, you can just keep them in the local copy for your own purposes. You can create a new branch in your local repository with 'git checkout': $ git checkout --track -b mybranch master This command creates a new branch called 'mybranch', configures the branch to track the master branch and checks the new branch out. The word 'track' in this context means that the newly created branch will receive all updates from the master branch whenever you run 'git pull' with this branch checked out. In other words, the branch tracks another branch (master in this case) by merging all changes from the original branch. Command line option -b instructs git to to create a new branch. If this option was omitted then git would assume that 'mybranch' is an existing branch and it would try to check the branch out. The newly created branch exists in the local repository only. If you want to push the branch to the remote repository then you can use 'git push' again: $ git push origin mybranch:janakj/mybranch Because mybranch is a private topic branch of mine, I am pushing it as janakj/mybranch to the remote repository at git.sip-router.org. The remote repository only permits new "username" branches. In other words their name must start with your username. If you try: $ git push origin mybranch:mybranch then git will report an error. The main reason why we have this restriction in place is because we wanted to make sure that people do not push all local branches to the remote repository by accident. This can easily happen if you run 'git push' without any parameters. You can also delete a local branch that is no longer needed by running $ git branch -D mybranch This will delete the local branch only. If you pushed the branch to the remote repository then you might also want to delete the branch in the remote repository with 'git push': $ git push :janakj/mybranch This is a special syntax of 'git push', if you omit the name of the local branch then 'git push' deletes the remote branch whose name follows ':'. Test Repository =============== We have a test repository at git.sip-router.org which you can use to test various git operations. Clone the repository with: $ git clone ssh://username@git.sip-router.org/test and there you can test whatever you want. The repository can also be browsed with gitweb at http://git.sip-router.org/cgi-bin/gitweb.cgi?p=test;a=summary Further Reading =============== [1] Git related pages in the wiki: http://sip-router.org/wiki/git [2] http://eagain.net/articles/git-for-computer-scientists/ [3] Git SVN Crashcourse: http://git.or.cz/course/svn.html [4] http://git-scm.com/documentation

7 18

git:master: core: MATCH op fix for ip comparisons with RVEs
by Andrei Pelinescu-Onciul 18 Jun '10

18 Jun '10

Module: sip-router Branch: master Commit: e535756d2bee95d01d7b5f2b0ec53d9c43214613 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=e535756… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Fri Jun 18 23:04:24 2010 +0200 core: MATCH op fix for ip comparisons with RVEs Commit e085834 broke regular expression support in ip comparisons with rval expressions (e.g. $re="127\..*"; if (src_ip=~$re) ...). --- route.c | 44 +++++++++++++++++++++++++++++++++++++------- 1 files changed, 37 insertions(+), 7 deletions(-) diff --git a/route.c b/route.c index 0e07556..4f86912 100644 --- a/route.c +++ b/route.c @@ -1272,6 +1272,8 @@ inline static int comp_str(int op, str* left, int rtype, case SELECT_ST: case RVE_ST: case PVAR_ST: + case STRING_ST: + case STR_ST: /* we need to compile the RE on the fly */ re=(regex_t*)pkg_malloc(sizeof(regex_t)); if (re==0){ @@ -1293,7 +1295,6 @@ inline static int comp_str(int op, str* left, int rtype, case RE_ST: ret=(regexec(r->re, left->s, 0, 0, 0)==0); break; - case STRING_ST: default: LOG(L_CRIT, "BUG: comp_str: Bad operator type %d, " "for ~= \n", rtype); @@ -1630,17 +1631,46 @@ inline static int comp_ip(int op, struct ip_addr* ip, int rtype, r_expop.str=*right; switch(op){ case EQUAL_OP: - case MATCH_OP: /* 0: try if ip or network (ip/mask) */ if (mk_net_str(&net, right) == 0) { ret=(matchnet(ip, &net)==1); break; } - /* 1: compare with ip2str*/ - /* - ret=comp_string(op, ip_addr2a(ip), STR_ST, &r_expop, msg, ctx); - if (likely(ret==1)) break; - */ + /* 2: resolve (name) & compare w/ all the ips */ + he=resolvehost(right->s); + if (he==0){ + DBG("comp_ip: could not resolve %s\n", r->str.s); + }else if (he->h_addrtype==ip->af){ + for(h=he->h_addr_list;(ret!=1)&& (*h); h++){ + ret=(memcmp(ip->u.addr, *h, ip->len)==0); + } + if (ret==1) break; + } + /* 3: (slow) rev dns the address + * and compare with all the aliases + * !!??!! review: remove this? */ + if (unlikely((received_dns & DO_REV_DNS) && + ((he=rev_resolvehost(ip))!=0) )){ + /* compare with primary host name */ + ret=comp_string(op, he->h_name, STR_ST, &r_expop, msg, ctx); + /* compare with all the aliases */ + for(h=he->h_aliases; (ret!=1) && (*h); h++){ + ret=comp_string(op, *h, STR_ST, &r_expop, msg, ctx); + } + }else{ + ret=0; + } + break; + case MATCH_OP: + /* 0: try if ip or network (ip/mask) + (one should not use MATCH for that, but try to be nice)*/ + if (mk_net_str(&net, right) == 0) { + ret=(matchnet(ip, &net)==1); + break; + } + /* 1: compare with ip2str (but only for =~)*/ + ret=comp_string(op, ip_addr2a(ip), STR_ST, &r_expop, msg, ctx); + if (likely(ret==1)) break; /* 2: resolve (name) & compare w/ all the ips */ he=resolvehost(right->s); if (he==0){

1 0

git:master: core: functions for parsing a network address
by Andrei Pelinescu-Onciul 18 Jun '10

18 Jun '10

Module: sip-router Branch: master Commit: dc4cdb294cc135a01032dd613d79bfe625351612 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=dc4cdb2… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Fri Jun 18 22:03:46 2010 +0200 core: functions for parsing a network address - added mk_net_str(): initialize a struct net from a string of the form ip/mask, ip/bitlen or ip - renamed mk_net() and mk_net_bitlen() to mk_new_net() and mk_new_net_bitlen() --- cfg.y | 10 ++-- ip_addr.c | 140 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- ip_addr.h | 7 ++- 3 files changed, 142 insertions(+), 15 deletions(-) diff --git a/cfg.y b/cfg.y index ae214f2..f2b71a5 100644 --- a/cfg.y +++ b/cfg.y @@ -2073,7 +2073,7 @@ exp_elem: pkg_free(s_tmp.s); if (ip_tmp) { $$=mk_elem($2, $1, 0, NET_ST, - mk_net_bitlen(ip_tmp, ip_tmp->len*8) ); + mk_new_net_bitlen(ip_tmp, ip_tmp->len*8) ); } else { $$=mk_elem($2, $1, 0, RVE_ST, $3); } @@ -2113,19 +2113,19 @@ exp_elem2: */ ipnet: - ip SLASH ip { $$=mk_net($1, $3); } + ip SLASH ip { $$=mk_new_net($1, $3); } | ip SLASH NUMBER { if (($3<0) || ($3>$1->len*8)) { yyerror("invalid bit number in netmask"); $$=0; } else { - $$=mk_net_bitlen($1, $3); + $$=mk_new_net_bitlen($1, $3); /* - $$=mk_net($1, htonl( ($3)?~( (1<<(32-$3))-1 ):0 ) ); + $$=mk_new_net($1, htonl( ($3)?~( (1<<(32-$3))-1 ):0 ) ); */ } } - | ip { $$=mk_net_bitlen($1, $1->len*8); } + | ip { $$=mk_new_net_bitlen($1, $1->len*8); } | ip SLASH error { $$=0; yyerror("netmask (eg:255.0.0.0 or 8) expected"); } ; diff --git a/ip_addr.c b/ip_addr.c index 06a5bf2..5651809 100644 --- a/ip_addr.c +++ b/ip_addr.c @@ -34,11 +34,10 @@ * 2004-10-01 mk_net fixes bad network addresses now (andrei) */ -/*! - * \file - * \brief SIP-router core :: - * \ingroup core - * Module: \ref core +/** inernal ip addresses representation functions. + * @file ip_addr.c + * @ingroup core + * Module: @ref core */ @@ -48,9 +47,11 @@ #include "ip_addr.h" #include "dprint.h" #include "mem/mem.h" +#include "resolve.h" +#include "trim.h" -struct net* mk_net(struct ip_addr* ip, struct ip_addr* mask) +struct net* mk_new_net(struct ip_addr* ip, struct ip_addr* mask) { struct net* n; int warning; @@ -88,7 +89,7 @@ error: -struct net* mk_net_bitlen(struct ip_addr* ip, unsigned int bitlen) +struct net* mk_new_net_bitlen(struct ip_addr* ip, unsigned int bitlen) { struct ip_addr mask; int r; @@ -103,13 +104,136 @@ struct net* mk_net_bitlen(struct ip_addr* ip, unsigned int bitlen) mask.af=ip->af; mask.len=ip->len; - return mk_net(ip, &mask); + return mk_new_net(ip, &mask); error: return 0; } +/** fills a net structure from an ip and a mask. + * + * This function will not print any error messages or allocate + * memory (as opposed to mk_new_net() above). + * + * @param n - destination net structure + * @param ip + * @param mask + * @return -1 on error (af mismatch), 0 on success + */ +int mk_net(struct net* n, struct ip_addr* ip, struct ip_addr* mask) +{ + int r; + + if (unlikely((ip->af != mask->af) || (ip->len != mask->len))) { + return -1; + } + n->ip=*ip; + n->mask=*mask; + /* fix the network part of the mask */ + for (r=0; r<n->ip.len/4; r++) { /*ipv4 & ipv6 addresses are multiple of 4*/ + n->ip.u.addr32[r] &= n->mask.u.addr32[r]; + }; + return 0; +} + + + +/** fills a net structure from an ip and a bitlen. + * + * This function will not print any error messages or allocate + * memory (as opposed to mk_new_net_bitlen() above). + * + * @param n - destination net structure + * @param ip + * @param bitlen + * @return -1 on error (af mismatch), 0 on success + */ +int mk_net_bitlen(struct net* n, struct ip_addr* ip, unsigned int bitlen) +{ + struct ip_addr mask; + int r; + + if (unlikely(bitlen>ip->len*8)) + /* bitlen too big */ + return -1; + memset(&mask,0, sizeof(mask)); + for (r=0;r<bitlen/8;r++) mask.u.addr[r]=0xff; + if (bitlen%8) mask.u.addr[r]= ~((1<<(8-(bitlen%8)))-1); + mask.af=ip->af; + mask.len=ip->len; + + return mk_net(n, ip, &mask); +} + + + +/** initializes a net structure from a string. + * @param dst - net structure that will be filled + * @param s - string of the form "ip", "ip/mask_len" or "ip/ip_mak". + * @return -1 on error, 0 on succes + */ +int mk_net_str(struct net* dst, str* s) +{ + struct ip_addr* t; + char* p; + struct ip_addr ip; + str addr; + str mask; + unsigned int bitlen; + + /* test for ip only */ + t = str2ip(s); +#ifdef USE_IPV6 + if (unlikely(t == 0)) + t = str2ip6(s); +#endif /* USE_IPV6 */ + if (likely(t)) + return mk_net_bitlen(dst, t, t->len*8); + /* not a simple ip, maybe an ip/netmask pair */ + p = q_memchr(s->s, '/', s->len); + if (likely(p)) { + addr.s = s->s; + addr.len = (int)(long)(p - s->s); + mask.s = p + 1; + mask.len = s->len - (addr.len + 1); + /* allow '/' enclosed by whitespace */ + trim_trailing(&addr); + trim_leading(&mask); + t = str2ip(&addr); + if (likely(t)) { + /* it can be a number */ + if (str2int(&mask, &bitlen) == 0) + return mk_net_bitlen(dst, t, bitlen); + ip = *t; + t = str2ip(&mask); + if (likely(t)) + return mk_net(dst, &ip, t); + /* error */ + return -1; + } +#ifdef USE_IPV6 + else { + t = str2ip6(&addr); + if (likely(t)) { + /* it can be a number */ + if (str2int(&mask, &bitlen) == 0) + return mk_net_bitlen(dst, t, bitlen); + ip = *t; + t = str2ip6(&mask); + if (likely(t)) + return mk_net(dst, &ip, t); + /* error */ + return -1; + } + } +#endif /* USE_IPV6 */ + } + return -1; +} + + + void print_ip(char* p, struct ip_addr* ip, char *s) { switch(ip->af){ diff --git a/ip_addr.h b/ip_addr.h index fd19c42..50fbf58 100644 --- a/ip_addr.h +++ b/ip_addr.h @@ -240,8 +240,11 @@ struct socket_id{ -struct net* mk_net(struct ip_addr* ip, struct ip_addr* mask); -struct net* mk_net_bitlen(struct ip_addr* ip, unsigned int bitlen); +struct net* mk_new_net(struct ip_addr* ip, struct ip_addr* mask); +struct net* mk_new_net_bitlen(struct ip_addr* ip, unsigned int bitlen); +int mk_net(struct net* n, struct ip_addr* ip, struct ip_addr* mask); +int mk_net_bitlen(struct net* n, struct ip_addr* ip, unsigned int bitlen); +int mk_net_str(struct net* dst, str* s); void print_ip(char* prefix, struct ip_addr* ip, char* suffix); void stdout_print_ip(struct ip_addr* ip);

1 0

git:master: core: fix selects in optimized rvalue expressions
by Andrei Pelinescu-Onciul 18 Jun '10

18 Jun '10

Module: sip-router Branch: master Commit: 2f34ba7d8d36f1a8465055367d725b9af4117c99 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=2f34ba7… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Fri Jun 18 22:22:59 2010 +0200 core: fix selects in optimized rvalue expressions - do not attempt to resolve_select() twice the same select. This happened when a rve containing only a select was optimized, the select moved out of the rve and fix_expr() tried to "fix" the already resolved select later. - changed BUG() messages for failed resolve_select() into ERR(). --- route.c | 20 ++++++++++++++------ route_struct.h | 5 +++-- rvalue.c | 2 +- 3 files changed, 18 insertions(+), 9 deletions(-) diff --git a/route.c b/route.c index 9a8f649..0e07556 100644 --- a/route.c +++ b/route.c @@ -588,7 +588,9 @@ int fix_expr(struct expr* exp) exp->r.re=re; exp->r_type=RE_ST; }else if (exp->r_type!=RE_ST && exp->r_type != AVP_ST - && exp->r_type != SELECT_ST && exp->r_type!= RVE_ST + && exp->r_type != SELECT_ST && + exp->r_type != SELECT_UNFIXED_ST && + exp->r_type!= RVE_ST && exp->r_type != PVAR_ST){ LOG(L_CRIT, "BUG: fix_expr : invalid type for match\n"); return E_BUG; @@ -601,19 +603,21 @@ int fix_expr(struct expr* exp) return ret; } } - if (exp->l_type==SELECT_O) { + if (exp->l_type==SELECT_UNFIXED_O) { if ((ret=resolve_select(exp->l.select)) < 0) { - BUG("Unable to resolve select\n"); + ERR("Unable to resolve select\n"); print_select(exp->l.select); return ret; } + exp->l_type=SELECT_O; } - if ((exp->r_type==SELECT_O)||(exp->r_type==SELECT_ST)) { + if (exp->r_type==SELECT_UNFIXED_ST) { if ((ret=resolve_select(exp->r.select)) < 0) { - BUG("Unable to resolve select\n"); - print_select(exp->l.select); + ERR("Unable to resolve select\n"); + print_select(exp->r.select); return ret; } + exp->r_type=SELECT_ST; } /* PVAR don't need fixing */ ret=0; @@ -1895,6 +1899,10 @@ inline static int eval_elem(struct run_act_ctx* h, struct expr* e, case PVAR_O: ret=comp_pvar(e->op, e->l.param, e->r_type, &e->r, msg, h); break; + + case SELECT_UNFIXED_O: + BUG("unexpected unfixed select operand %d\n", e->l_type); + break; /* default: LOG(L_CRIT, "BUG: eval_elem: invalid operand %d\n", diff --git a/route_struct.h b/route_struct.h index 4894bd5..3f7e854 100644 --- a/route_struct.h +++ b/route_struct.h @@ -76,7 +76,7 @@ enum _expr_l_type{ METHOD_O=51, URI_O, FROM_URI_O, TO_URI_O, SRCIP_O, SRCPORT_O, DSTIP_O, DSTPORT_O, PROTO_O, AF_O, MSGLEN_O, ACTION_O, NUMBER_O, AVP_O, SNDIP_O, SNDPORT_O, TOIP_O, TOPORT_O, SNDPROTO_O, - SNDAF_O, RETCODE_O, SELECT_O, PVAR_O, RVEXP_O}; + SNDAF_O, RETCODE_O, SELECT_O, PVAR_O, RVEXP_O, SELECT_UNFIXED_O}; /* action types */ enum action_type{ FORWARD_T=1, SEND_T, DROP_T, LOG_T, ERROR_T, ROUTE_T, EXEC_T, @@ -121,7 +121,8 @@ enum _operand_subtype{ SELECT_ST, PVAR_ST, LVAL_ST, RVE_ST, RETCODE_ST, CASE_ST, - BLOCK_ST, JUMPTABLE_ST, CONDTABLE_ST, MATCH_CONDTABLE_ST + BLOCK_ST, JUMPTABLE_ST, CONDTABLE_ST, MATCH_CONDTABLE_ST, + SELECT_UNFIXED_ST }; typedef enum _expr_l_type expr_l_type; diff --git a/rvalue.c b/rvalue.c index 38a0eb7..9b3faca 100644 --- a/rvalue.c +++ b/rvalue.c @@ -2768,7 +2768,7 @@ static int fix_rval(struct rvalue* rv) return fix_actions(rv->v.action); case RV_SEL: if (resolve_select(&rv->v.sel)<0){ - BUG("Unable to resolve select\n"); + ERR("Unable to resolve select\n"); print_select(&rv->v.sel); } return 0;

1 0

← Newer
1
...
6
7
8
9
10
11
12
...
17
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

sr-dev June 2010