sr-dev August 2010

sr-dev@lists.kamailio.org

26 participants
346 discussions

git:kamailio_3.0: tcp: fix fd passing bug
by Andrei Pelinescu-Onciul 19 Aug '10

19 Aug '10

Module: sip-router Branch: kamailio_3.0 Commit: 66619631879132e3d93fd858fa81c79ac9788617 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=6661963… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Wed Jun 16 21:03:06 2010 +0200 tcp: fix fd passing bug If connections are opened and closed very quickly when data is sent on them, it is possible that a connection gets closed (close() inside tcp_main) while a process waits for its fd, just before tcp_main attempts to send the fd. In this case the fd sending will fail (one cannot send a closed fd) and the process that asked for it will remain waiting forever. The fix always checks before attempting to send the fd if the fd is still open and the connection is not in a "bad" state. If not, a new error response is sent (no fd and connection == NULL). (backported from 9da6fae72b9883ab8dbbb4e681c4d4e96d6549e4) --- tcp_main.c | 51 +++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 45 insertions(+), 6 deletions(-) diff --git a/tcp_main.c b/tcp_main.c index 4840ddc..6a6c4c0 100644 --- a/tcp_main.c +++ b/tcp_main.c @@ -2050,14 +2050,25 @@ int tcp_send(struct dest_info* dst, union sockaddr_union* from, do_close_fd=0; goto release_c; } - if (unlikely(c!=tmp)){ - LOG(L_CRIT, "BUG: tcp_send: get_fd: got different connection:" + /* handle fd closed or bad connection/error + (it's possible that this happened in the time between + we found the intial connection and the time when we get + the fd) + */ + if (unlikely(c!=tmp || fd==-1 || c->state==S_CONN_BAD)){ + if (unlikely(c!=tmp && tmp!=0)) + BUG("tcp_send: get_fd: got different connection:" " %p (id= %d, refcnt=%d state=%d) != " " %p (n=%d)\n", c, c->id, atomic_get(&c->refcnt), c->state, tmp, n - ); + ); n=-1; /* fail */ + /* don't cache fd & close it */ + do_close_fd = (fd==-1)?0:1; +#ifdef TCP_FD_CACHE + use_fd_cache = 0; +#endif /* TCP_FD_CACHE */ goto end; } DBG("tcp_send: after receive_fd: c= %p n=%d fd=%d\n",c, n, fd); @@ -2721,6 +2732,12 @@ inline static void send_fd_queue_run(struct tcp_send_fd_q* q) struct send_fd_info* t; for (p=t=&q->data[0]; p<q->crt; p++){ + if (unlikely(p->tcp_conn->state == S_CONN_BAD || + p->tcp_conn->flags & F_CONN_FD_CLOSED || + p->tcp_conn->s ==-1)) { + /* bad and/or already closed connection => remove */ + goto rm_con; + } if (unlikely(send_fd(p->unix_sock, &(p->tcp_conn), sizeof(struct tcp_connection*), p->tcp_conn->s)<=0)){ if ( ((errno==EAGAIN)||(errno==EWOULDBLOCK)) && @@ -2736,7 +2753,11 @@ inline static void send_fd_queue_run(struct tcp_send_fd_q* q) p->unix_sock, (long)(p-&q->data[0]), p->retries, p->tcp_conn, p->tcp_conn->s, errno, strerror(errno)); +rm_con: #ifdef TCP_ASYNC + /* if a connection is on the send_fd queue it means it's + not watched for read anymore => could be watched only for + write */ if (p->tcp_conn->flags & F_CONN_WRITE_W){ io_watch_del(&io_h, p->tcp_conn->s, -1, IO_FD_CLOSING); p->tcp_conn->flags &=~F_CONN_WRITE_W; @@ -3006,6 +3027,7 @@ error: inline static int handle_ser_child(struct process_table* p, int fd_i) { struct tcp_connection* tcpconn; + struct tcp_connection* tmp; long response[2]; int cmd; int bytes; @@ -3101,9 +3123,26 @@ inline static int handle_ser_child(struct process_table* p, int fd_i) /* send the requested FD */ /* WARNING: take care of setting refcnt properly to * avoid race conditions */ - if (unlikely(send_fd(p->unix_sock, &tcpconn, sizeof(tcpconn), - tcpconn->s)<=0)){ - LOG(L_ERR, "ERROR: handle_ser_child: send_fd failed\n"); + if (unlikely(tcpconn->state == S_CONN_BAD || + (tcpconn->flags & F_CONN_FD_CLOSED) || + tcpconn->s ==-1)) { + /* connection is already marked as bad and/or has no + fd => don't try to send the fd (trying to send a + closed fd _will_ fail) */ + tmp = 0; + if (unlikely(send_all(p->unix_sock, &tmp, sizeof(tmp)) <= 0)) + BUG("handle_ser_child: CONN_GET_FD: send_all failed\n"); + /* no need to attempt to destroy the connection, it should + be already in the process of being destroyed */ + } else if (unlikely(send_fd(p->unix_sock, &tcpconn, + sizeof(tcpconn), tcpconn->s)<=0)){ + LOG(L_ERR, "handle_ser_child: CONN_GET_FD:" + " send_fd failed\n"); + /* try sending error (better then not sending anything) */ + tmp = 0; + if (unlikely(send_all(p->unix_sock, &tmp, sizeof(tmp)) <= 0)) + BUG("handle_ser_child: CONN_GET_FD:" + " send_fd send_all fallback failed\n"); } break; case CONN_NEW:

1 0

git:kamailio_3.0: io_wait: fix: check for EV_ERROR for kqueue()
by Andrei Pelinescu-Onciul 19 Aug '10

19 Aug '10

Module: sip-router Branch: kamailio_3.0 Commit: 37314964b0b8be0c560705a5a1d8cfc3db9242fd URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=3731496… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Thu Jun 17 16:15:10 2010 +0200 io_wait: fix: check for EV_ERROR for kqueue() Re-enabled and enhanced the check for EV_ERROR when using kqueue (*bsd). This is needed to workaround errors reported by kqueue when trying to delete (EV_DELETE) an already closed FD (this can happen in the tcp code, where we try to avoid applying immediately changes in the set of watched FDs and instead collect them and apply them after all the current kqueue events are processed => in some corner case situations it's possible to try to delete the FD from kqueue after the fd was close()'ed). This fix will ignore EV_ERROR with data == EBADF. All the other errors will result in a POLLERR flag for the callback. It fixes crashes with *bsd under tcp stress tests (lots of very short lived connections). (cherry picked from commit 05c080a53f88babad6729f79015f555f53fdf957) --- io_wait.h | 55 ++++++++++++++++++++++++++++++++++++++++--------------- 1 files changed, 40 insertions(+), 15 deletions(-) diff --git a/io_wait.h b/io_wait.h index 0557c00..44ef60c 100644 --- a/io_wait.h +++ b/io_wait.h @@ -45,6 +45,7 @@ * 2007-11-29 support for write (POLLOUT); added io_watch_chg() (andrei) * 2008-02-04 POLLRDHUP & EPOLLRDHUP support (automatically enabled if POLLIN * is set) (andrei) + * 2010-06-17 re-enabled & enhanced the EV_ERROR for kqueue (andrei) */ @@ -1097,31 +1098,55 @@ again: r, n, h->kq_array[r].ident, (long)h->kq_array[r].udata, h->kq_array[r].flags); #endif -#if 0 - if (unlikely(h->kq_array[r].flags & EV_ERROR)){ - /* error in changes: we ignore it, 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 */ - LOG(L_INFO, "INFO: io_wait_loop_kqueue: kevent error on " - "fd %ld: %s [%ld]\n", h->kq_array[r].ident, + if (unlikely((h->kq_array[r].flags & EV_ERROR) && + (h->kq_array[r].data == EBADF || + 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 + => 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, + after all the fd events were processed), a CON_ERR sent + 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. + */ + /* + example EV_ERROR for trying to delete a read watched fd, + that was already closed: + { + ident = 63, [fd] + filter = -1, [EVFILT_READ] + flags = 16384, [EV_ERROR] + fflags = 0, + data = 9, [errno = EBADF] + 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, strerror(h->kq_array[r].data), (long)h->kq_array[r].data); - }else{ -#endif + }else{ fm=(struct fd_map*)h->kq_array[r].udata; if (likely(h->kq_array[r].filter==EVFILT_READ)){ - revents=POLLIN | - (((int)!(h->kq_array[r].flags & EV_EOF)-1)&POLLHUP); + revents=POLLIN | + (((int)!(h->kq_array[r].flags & EV_EOF)-1)&POLLHUP) | + (((int)!(h->kq_array[r].flags & EV_ERROR)-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); + revents=POLLOUT | + (((int)!(h->kq_array[r].flags & EV_EOF)-1)&POLLHUP) | + (((int)!(h->kq_array[r].flags & EV_ERROR)-1)&POLLERR); while(fm->type && (fm->events & revents) && (handle_io(fm, revents, -1)>0) && repeat); } - /*} */ + } } error: return n;

1 0

git:kamailio_3.0: io_wait: kqueue: use a bigger array
by Andrei Pelinescu-Onciul 19 Aug '10

19 Aug '10

Module: sip-router Branch: kamailio_3.0 Commit: 8387cdd610ad15952e7f0e7d7f9bcbfb4a5b9cfe URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=8387cdd… 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) (cherry picked from commit a9cdfc2938ca73d6ba40f5896c6a8930c2e73f85) --- 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 5adb8d2..1b29813 100644 --- a/io_wait.c +++ b/io_wait.c @@ -542,13 +542,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){ @@ -557,7 +565,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:kamailio_3.0: io_wait: kqueue: handle ENOENT and more robust error handling
by Andrei Pelinescu-Onciul 19 Aug '10

19 Aug '10

Module: sip-router Branch: kamailio_3.0 Commit: a95f607d8118e668dccfc0cf85520addf3326cfe URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=a95f607… 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) (cherry picked from commit 8966d0a1787152fa64d1f78321ee539116bd448a) --- 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:kamailio_3.0: io_wait: kqueue: use the entire array during too many errors fallback
by Andrei Pelinescu-Onciul 19 Aug '10

19 Aug '10

Module: sip-router Branch: kamailio_3.0 Commit: aed168981e6dae1333afcdaff5976b44c4fac738 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=aed1689… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Thu Jul 8 15:18:52 2010 +0200 io_wait: kqueue: use the entire array during too many errors fallback Minor fix/optimization: if there are too many errors in the changelist and the kevent() call has to be retried, use the entire array (don't rely on the current watched fd number which will be smaller then the array real size, since commit 996826). (only kqueue using systems are affected by this fix: *bsd and darwin) (cherry picked from commit a9cdfc2938ca73d6ba40f5896c6a8930c2e73f85) --- io_wait.h | 105 +++++++++++++++++++++++++++++++------------------------------ 1 files changed, 53 insertions(+), 52 deletions(-) Diff: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commitdiff;h=aed…

1 0

git:kamailio_3.0: io_wait: fix kqueue and too many errors in changelist
by Andrei Pelinescu-Onciul 19 Aug '10

19 Aug '10

Module: sip-router Branch: kamailio_3.0 Commit: d3a1deaefdd0f23388fece48530a0ae1f39ad93a URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=d3a1dea… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Thu Jun 17 18:43:14 2010 +0200 io_wait: fix kqueue and too many errors in changelist kevent() tries to return errors in the changelist back in the supplied eventlist array. However if this is not large enough, the whole kevent() syscall will fail. Now if kevent() fails with EBADF the call will be retried with a smaller set of changes, until the entire original changelist is applied. Fixes also kq_ev_change() flush mode: on error it will try to apply the changes one-by-one. (this affects only systems that have kqueue: *bsd and darwin) (cherry picked from commit b0bd3201826ee693d6f96c6e336477b9d9db7c32) --- io_wait.h | 62 ++++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 files changed, 55 insertions(+), 7 deletions(-) diff --git a/io_wait.h b/io_wait.h index 44ef60c..93f1426 100644 --- a/io_wait.h +++ b/io_wait.h @@ -247,6 +247,7 @@ static inline int kq_ev_change(io_wait_h* h, int fd, int filter, int flag, void* data) { int n; + int r; struct timespec tspec; if (h->kq_nchanges>=h->kq_changes_size){ @@ -257,11 +258,36 @@ static inline int kq_ev_change(io_wait_h* h, int fd, int filter, int flag, tspec.tv_nsec=0; again: n=kevent(h->kq_fd, h->kq_changes, h->kq_nchanges, 0, 0, &tspec); - if (n==-1){ - if (errno==EINTR) goto again; - LOG(L_ERR, "ERROR: io_watch_add: kevent flush changes " + if (unlikely(n == -1)){ + if (likely(errno == EBADF)) { + /* 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) + 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; + } + } + } else if (errno == EINTR) goto again; + else { + LOG(L_ERR, "ERROR: io_watch_add: kevent flush changes" " failed: %s [%d]\n", strerror(errno), errno); - return -1; + h->kq_nchanges=0; /* reset changes array */ + return -1; + } } h->kq_nchanges=0; /* changes array is empty */ } @@ -1076,22 +1102,43 @@ inline static int io_wait_loop_kqueue(io_wait_h* h, int t, int repeat) int n, r; struct timespec tspec; struct fd_map* fm; + int orig_changes; + int apply_changes; int revents; tspec.tv_sec=t; tspec.tv_nsec=0; + orig_changes=h->kq_nchanges; + apply_changes=orig_changes; + do { again: - n=kevent(h->kq_fd, h->kq_changes, h->kq_nchanges, h->kq_array, + 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{ + else if (errno==EBADF) { + /* 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; } } - h->kq_nchanges=0; /* reset changes array */ + /* remove applied changes */ + h->kq_nchanges -= apply_changes; + if (unlikely(apply_changes < orig_changes)) { + orig_changes -= apply_changes; + memmove(&h->kq_changes[0], &h->kq_changes[apply_changes], + sizeof(h->kq_changes[0])*h->kq_nchanges); + apply_changes = orig_changes<h->fd_no ? orig_changes : h->fd_no; + } else { + orig_changes = 0; + apply_changes = 0; + } for (r=0; r<n; r++){ #ifdef EXTRA_DEBUG DBG("DBG: kqueue: event %d/%d: fd=%d, udata=%lx, flags=0x%x\n", @@ -1148,6 +1195,7 @@ again: } } } + } while(unlikely(orig_changes)); error: return n; }

1 0

git:kamailio_3.0: io_wait: fix kqueue io_wait_add & POLLIN
by Andrei Pelinescu-Onciul 19 Aug '10

19 Aug '10

Module: sip-router Branch: kamailio_3.0 Commit: 8e6609c4416dd4f1196daa793b75c305ca22155e URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=8e6609c… 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. (cherry picked from commit e5be1a067158c8ba49d33082eb403937546e7c69) --- 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:kamailio_3.0: io_wait: don't update FD watched status on error
by Andrei Pelinescu-Onciul 19 Aug '10

19 Aug '10

Module: sip-router Branch: kamailio_3.0 Commit: bf75177d709d97d70db9ca21d4f9526bd50fbcf5 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=bf75177… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Fri Jun 18 09:48:21 2010 +0200 io_wait: don't update FD watched status on error If the syscall to change the events or delete a watched FD fails, don't update/delete the FD status in fd_hash. For /dev/poll if a change fails when re-adding the FD, delete it from the hash (in the /dev/poll case to change the events a FD is watched for one has to remove it and re-add it with the new events). The syscalls should never fail in an un-handled way, but in the unlikely event that it happens this change will make the code more robust. (cherry picked from commit 2d8cd170ab867ab15296b30f0b784abe1adc1bca) --- io_wait.h | 15 +++++++++------ 1 files changed, 9 insertions(+), 6 deletions(-) diff --git a/io_wait.h b/io_wait.h index 93f1426..01df1e6 100644 --- a/io_wait.h +++ b/io_wait.h @@ -627,7 +627,6 @@ inline static int io_watch_del(io_wait_h* h, int fd, int idx, int flags) goto error; } events=e->events; - unhash_fd_map(e); switch(h->poll_method){ case POLL_POLL: @@ -647,7 +646,6 @@ inline static int io_watch_del(io_wait_h* h, int fd, int idx, int flags) #endif #ifdef HAVE_SIGIO_RT case POLL_SIGIO_RT: - fix_fd_array; /* the O_ASYNC flag must be reset all the time, the fd * can be changed only if O_ASYNC is reset (if not and * the fd is a duplicate, you will get signals from the dup. fd @@ -667,6 +665,7 @@ inline static int io_watch_del(io_wait_h* h, int fd, int idx, int flags) " failed: %s [%d]\n", strerror(errno), errno); goto error; } + fix_fd_array; /* only on success */ break; #endif #ifdef HAVE_EPOLL @@ -737,6 +736,7 @@ again_devpoll: h->poll_method); goto error; } + unhash_fd_map(e); /* only on success */ h->fd_no--; return 0; error: @@ -808,14 +808,14 @@ inline static int io_watch_chg(io_wait_h* h, int fd, short events, int idx ) add_events=events & ~e->events; del_events=e->events & ~events; - e->events=events; switch(h->poll_method){ case POLL_POLL: + fd_array_chg(events #ifdef POLLRDHUP - /* listen to POLLRDHUP by default (if POLLIN) */ - events|=((int)!(events & POLLIN) - 1) & POLLRDHUP; + /* listen to POLLRDHUP by default (if POLLIN) */ + | (((int)!(events & POLLIN) - 1) & POLLRDHUP) #endif /* POLLRDHUP */ - fd_array_chg(events); + ); break; #ifdef HAVE_SELECT case POLL_SELECT: @@ -921,6 +921,8 @@ again_devpoll2: LOG(L_ERR, "ERROR: io_watch_chg: re-adding fd to " "/dev/poll failed: %s [%d]\n", strerror(errno), errno); + /* error re-adding the fd => mark it as removed/unhash */ + unhash_fd_map(e); goto error; } break; @@ -931,6 +933,7 @@ again_devpoll2: h->poll_method); goto error; } + e->events=events; /* only on success */ return 0; error: return -1;

1 0

git:sr_3.0: dns cache: fix manually inserted SRV record order
by Andrei Pelinescu-Onciul 19 Aug '10

19 Aug '10

Module: sip-router Branch: sr_3.0 Commit: d495d4cd2c3bdc4323f1850fe1c6ed48f342f317 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=d495d4c… Author: Miklos Tirpak <miklos(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Wed Aug 18 15:32:36 2010 +0200 dns cache: fix manually inserted SRV record order The manually inserted SRV records thought the RPC interface were not ordered by their priority and weight which resulted in wrong SRV look-ups, i.e. not the highest priority record was chosen first. (cherry picked from commit 78beed37b8975e11851553c41c17578481772dad) --- dns_cache.c | 44 ++++++++++++++++++++++++++++++++++++++++++++ 1 files changed, 44 insertions(+), 0 deletions(-) diff --git a/dns_cache.c b/dns_cache.c index e382373..47c7ba4 100644 --- a/dns_cache.c +++ b/dns_cache.c @@ -4072,6 +4072,8 @@ static void dns_cache_add_record(rpc_t* rpc, void* ctx, unsigned short type) ticks_t expire; int err, h; int size; + struct dns_rr *new_rr, **rr_p, **rr_iter; + struct srv_rdata *srv_rd; /* eliminate gcc warnings */ ip_addr = 0; @@ -4206,6 +4208,7 @@ static void dns_cache_add_record(rpc_t* rpc, void* ctx, unsigned short type) /* let the rr point to the new structure */ rr = (struct dns_rr*)translate_pointer((char*)new, (char*)old, (char*)rr); + new_rr = rr; if (type == T_SRV) { /* fix the priority, weight, and port */ @@ -4240,6 +4243,7 @@ static void dns_cache_add_record(rpc_t* rpc, void* ctx, unsigned short type) " the cache"); goto error; } + new_rr = rr; switch(type) { case T_A: @@ -4257,6 +4261,46 @@ static void dns_cache_add_record(rpc_t* rpc, void* ctx, unsigned short type) /* maximum expire value has been already fixed by * dns_cache_clone_entry() */ } + + if (type == T_SRV) { + /* SRV records must be ordered by their priority and weight. + * With modifying an exising rr, or adding new rr to the DNS entry, + * the ordered list might got broken which needs to be fixed. + */ + rr_p = NULL; + for ( rr_iter = &new->rr_lst; + *rr_iter; + rr_iter = &((*rr_iter)->next) + ) { + if (*rr_iter == new_rr) { + rr_p = rr_iter; + continue; + } + srv_rd = (struct srv_rdata*)(*rr_iter)->rdata; + if ((priority < srv_rd->priority) || + ((priority == srv_rd->priority) && (weight > srv_rd->weight)) + ) + break; /* insert here */ + } + + if (!rr_p) + for ( rr_p = rr_iter; + *rr_p && (*rr_p != new_rr); + rr_p = &((*rr_p)->next) + ); + if (!rr_p) { + LOG(L_ERR, "ERROR: Failed to correct the orderd list of SRV resource records\n"); + goto error; + } + + if (*rr_iter != new_rr->next) { + /* unlink rr from the list */ + *rr_p = (*rr_p)->next; + /* link it before *rr_iter */ + new_rr->next = *rr_iter; + *rr_iter = new_rr; + } + } } }

1 0

git:sr_3.0: tcp: fix double increment for the established stats
by Andrei Pelinescu-Onciul 19 Aug '10

19 Aug '10

Module: sip-router Branch: sr_3.0 Commit: 401f1b938365d5ca4fa6951fed71f8c9c13e1b49 URL: http://git.sip-router.org/cgi-bin/gitweb.cgi/sip-router/?a=commit;h=401f1b9… Author: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Committer: Andrei Pelinescu-Onciul <andrei(a)iptel.org> Date: Wed Aug 18 18:36:14 2010 +0200 tcp: fix double increment for the established stats The counter/stats for established TCP connections were incremented twice in the case of accept()-ed connections: once immediately after the accept() and another time after the first packet received or sent on the connection. Now they are incremented only after the first successful send or receive. (cherry picked from commit 66cda7bc5f642ce892124cfb35f1e5effd78e9d6) --- tcp_main.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/tcp_main.c b/tcp_main.c index 255753d..2258bc3 100644 --- a/tcp_main.c +++ b/tcp_main.c @@ -3499,7 +3499,15 @@ static inline int handle_new_connect(struct socket_info* si) return 1; /* success, because the accept was succesfull */ } (*tcp_connections_no)++; - TCP_STATS_ESTABLISHED(S_CONN_ACCEPT); + /* stats for established connections are incremented after + the first received or sent packet. + Alternatively they could be incremented here for accepted + connections, but then the connection state must be changed to + S_CONN_OK: + TCP_STATS_ESTABLISHED(S_CONN_ACCEPT); + ... + tcpconn=tcpconn_new(new_sock, &su, dst_su, si, si->proto, S_CONN_OK); + */ dst_su=&si->su; if (unlikely(si->flags & SI_IS_ANY)){

1 0

← Newer
1
...
9
10
11
12
13
14
15
...
35
Older →

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

2010

2009

2008

sr-dev August 2010