Clean up handling of detail files && events. It's much more
authoraland <aland>
Tue, 19 Feb 2008 09:44:14 +0000 (09:44 +0000)
committeraland <aland>
Tue, 19 Feb 2008 09:44:14 +0000 (09:44 +0000)
understandable now...

Note also that SNMP appears to be broken.  Oops...

src/include/detail.h
src/main/detail.c
src/main/event.c

index 699b1be..67c668f 100644 (file)
@@ -18,5 +18,6 @@ int detail_print(rad_listen_t *this, char *buffer, size_t bufsize);
 int detail_encode(UNUSED rad_listen_t *this, UNUSED REQUEST *request);
 int detail_decode(UNUSED rad_listen_t *this, UNUSED REQUEST *request);
 int detail_parse(CONF_SECTION *cs, rad_listen_t *this);
+int detail_delay(rad_listen_t *this);
 
 #endif /* DETAIL_H */
index b5da335..6a07afd 100644 (file)
@@ -47,6 +47,7 @@ typedef struct listen_detail_t {
        time_t          timestamp;
        fr_ipaddr_t     client_ip;
        int             load_factor; /* 1..100 */
+       int             signal;
 
        int             has_rtt;
        int             srtt;
@@ -83,8 +84,10 @@ int detail_send(rad_listen_t *listener, REQUEST *request)
         *      caller it's OK to read more "detail" file stuff.
         */
        if (request->reply->code == 0) {
-               radius_signal_self(RADIUS_SIGNAL_SELF_DETAIL);
+               data->delay_time = 1;
+               data->signal = 1;
                data->state = STATE_NO_REPLY;
+               radius_signal_self(RADIUS_SIGNAL_SELF_DETAIL);
                return 0;
        }
 
@@ -147,17 +150,31 @@ int detail_send(rad_listen_t *listener, REQUEST *request)
         *      rtt / (rtt + delay) = load_factor / 100
         */
        data->delay_time = (data->srtt * (100 - data->load_factor)) / (data->load_factor);
+       if (data->delay_time == 0) data->delay_time = USEC / 10;
 
 #if 0
        DEBUG2("RTT %d\tdelay %d", data->srtt, data->delay_time);
 #endif
 
        data->last_packet = now;
+       data->signal = 1;
        data->state = STATE_REPLIED;
+       radius_signal_self(RADIUS_SIGNAL_SELF_DETAIL);
 
        return 0;
 }
 
+int detail_delay(rad_listen_t *listener)
+{
+       listen_detail_t *data = listener->data;
+
+       if (!data->signal) return 0;
+
+       data->signal = 0;
+
+       return data->delay_time;
+}
+
 
 /*
  *     Open the detail file, if we can.
@@ -172,9 +189,7 @@ static int detail_open(rad_listen_t *this)
        listen_detail_t *data = this->data;
 
        rad_assert(data->state == STATE_UNOPENED);
-       snprintf(buffer, sizeof(buffer), "%s.work", data->filename);
-       free(data->filename_work);
-       data->filename_work = strdup(buffer);
+       data->delay_time = USEC;
 
        /*
         *      Open detail.work first, so we don't lose
@@ -186,7 +201,7 @@ static int detail_open(rad_listen_t *this)
         *      establish the lock, to prevent rlm_detail from
         *      writing to it.
         */
-       this->fd = open(buffer, O_RDWR);
+       this->fd = open(data->filename_work, O_RDWR);
        if (this->fd < 0) {
                DEBUG2("Polling for detail file %s", data->filename);
 
@@ -216,7 +231,7 @@ static int detail_open(rad_listen_t *this)
                /*
                 *      Rename detail to detail.work
                 */
-               if (rename(data->filename, buffer) < 0) {
+               if (rename(data->filename, data->filename_work) < 0) {
                        close(this->fd);
                        this->fd = -1;
                        return 0;
@@ -224,14 +239,7 @@ static int detail_open(rad_listen_t *this)
        } /* else detail.work existed, and we opened it */
 
        rad_assert(data->vps == NULL);
-
        rad_assert(data->fp == NULL);
-       data->fp = fdopen(this->fd, "r");
-       if (!data->fp) {
-               radlog(L_ERR, "Failed to re-open %s: %s",
-                      data->filename, strerror(errno));
-               return 0;
-       }
 
        data->state = STATE_UNLOCKED;
 
@@ -271,11 +279,6 @@ int detail_recv(rad_listen_t *listener,
        open_file:
                        rad_assert(listener->fd < 0);
                        
-                       /*
-                        *      FIXME: If the file doesn't exist, then
-                        *      return "sleep for 1s", to avoid busy
-                        *      looping.
-                        */
                        if (!detail_open(listener)) return 0;
 
                        rad_assert(data->state == STATE_UNLOCKED);
@@ -302,8 +305,24 @@ int detail_recv(rad_listen_t *listener,
                         *      radrelay.
                         */
                        if (rad_lockfd_nonblock(listener->fd, 0) < 0) {
+                               /*
+                                *      Close the FD.  The main loop
+                                *      will wake up in a second and
+                                *      try again.
+                                */
+                               close(listener->fd);
+                               listener->fd = -1;
+                               data->state = STATE_UNOPENED;
                                return 0;
                        }
+
+                       data->fp = fdopen(listener->fd, "r");
+                       if (!data->fp) {
+                               radlog(L_ERR, "FATAL: Failed to re-open detail file %s: %s",
+                                      data->filename, strerror(errno));
+                               exit(1);
+                       }
+
                        /*
                         *      Look for the header
                         */
@@ -319,6 +338,15 @@ int detail_recv(rad_listen_t *listener,
                                goto open_file;
                        }
 
+                       {
+                               struct stat buf;
+                               
+                               fstat(listener->fd, &buf);
+                               if (((off_t) ftell(data->fp)) == buf.st_size) {
+                                       goto cleanup;
+                               }
+                       }
+
                        /*
                         *      End of file.  Delete it, and re-set
                         *      everything.
@@ -326,7 +354,7 @@ int detail_recv(rad_listen_t *listener,
                        if (feof(data->fp)) {
                        cleanup:
                                unlink(data->filename_work);
-                               fclose(data->fp); /* closes listener->fd */
+                               if (data->fp) fclose(data->fp);
                                data->fp = NULL;
                                listener->fd = -1;
                                data->state = STATE_UNOPENED;
@@ -501,8 +529,8 @@ int detail_recv(rad_listen_t *listener,
         */
        packet = rad_alloc(1);
        if (!packet) {
-               data->state = STATE_NO_REPLY;   /* try again later */
-               return 0;       /* maybe memory will magically free up... */
+               radlog(L_ERR, "FATAL: Failed allocating memory for detail");
+               exit(1);
        }
 
        memset(packet, 0, sizeof(*packet));
@@ -609,20 +637,6 @@ int detail_recv(rad_listen_t *listener,
                return 0;
        }
 
-       {
-               struct stat buf;
-
-               fstat(listener->fd, &buf);
-               if (((off_t) ftell(data->fp)) == buf.st_size) {
-                       unlink(data->filename_work);
-                       fclose(data->fp); /* closes listener->fd */
-                       data->fp = NULL;
-                       listener->fd = -1;
-                       data->state = STATE_RUNNING;
-                       return 1;
-               }
-       }
-
        data->state = STATE_RUNNING;
 
        return 1;
@@ -690,6 +704,7 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
        int             rcode;
        listen_detail_t *data;
        RADCLIENT       *client;
+       char buffer[2048];
 
        if (!this->data) {
                this->data = rad_malloc(sizeof(*data));
@@ -697,6 +712,7 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
        }
 
        data = this->data;
+       data->delay_time = USEC;
 
        rcode = cf_section_parse(cs, data, detail_config);
        if (rcode < 0) {
@@ -714,6 +730,10 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
                return -1;
        }
 
+       snprintf(buffer, sizeof(buffer), "%s.work", data->filename);
+       free(data->filename_work);
+       data->filename_work = strdup(buffer); /* FIXME: leaked */
+
        data->vps = NULL;
        data->fp = NULL;
        data->state = STATE_UNOPENED;
index 0d413cc..bfad255 100644 (file)
@@ -27,6 +27,7 @@ RCSID("$Id$")
 #include <freeradius-devel/radiusd.h>
 #include <freeradius-devel/modules.h>
 #include <freeradius-devel/event.h>
+#include <freeradius-devel/detail.h>
 #include <freeradius-devel/radius_snmp.h>
 
 #include <freeradius-devel/rad_assert.h>
@@ -54,7 +55,6 @@ static struct timeval         now;
 static time_t                  start_time;
 static int                     have_children;
 static int                     has_detail_listener = FALSE;
-static int                     read_from_detail = TRUE;
 static int                     just_started = FALSE;
 
 #ifndef __MINGW32__
@@ -87,7 +87,6 @@ static rad_listen_t   *proxy_listeners[32];
 
 static void request_post_handler(REQUEST *request);
 static void wait_a_bit(void *ctx);
-static void event_poll_fd(void *ctx);
 static void event_socket_handler(fr_event_list_t *xel, UNUSED int fd, void *ctx);
 
 static void NEVER_RETURNS _rad_panic(const char *file, unsigned int line,
@@ -1862,7 +1861,8 @@ int received_request(rad_listen_t *listener,
        /*
         *      We may want to quench the new request.
         */
-       if (!can_handle_new_request(packet, client, root)) {
+       if ((listener->type != RAD_LISTEN_DETAIL) &&
+           !can_handle_new_request(packet, client, root)) {
                return 0;
        }
 
@@ -2108,12 +2108,21 @@ REQUEST *received_proxy_response(RADIUS_PACKET *packet)
 }
 
 
-static void handle_signal_self(int flag)
+static void event_detail_timer(void *ctx)
 {
-       if ((flag & RADIUS_SIGNAL_SELF_DETAIL) != 0) {
-               read_from_detail = TRUE;
+       rad_listen_t *listener = ctx;
+       RAD_REQUEST_FUNP fun;
+       REQUEST *request;
+
+       if (listener->recv(listener, &fun, &request)) {
+               if (!thread_pool_addrequest(request, fun)) {
+                       request->child_state = REQUEST_DONE;
+               }
        }
+}
 
+static void handle_signal_self(int flag)
+{
        if ((flag & (RADIUS_SIGNAL_SELF_EXIT | RADIUS_SIGNAL_SELF_TERM)) != 0) {
                if ((flag & RADIUS_SIGNAL_SELF_EXIT) != 0) {
                        fr_event_loop_exit(el, 1);
@@ -2143,6 +2152,37 @@ static void handle_signal_self(int flag)
                fr_event_loop_exit(el, 0x80);
        }
 
+       if ((flag & RADIUS_SIGNAL_SELF_DETAIL) != 0) {
+               rad_listen_t *this;
+               
+               for (this = mainconfig.listen;
+                    this != NULL;
+                    this = this->next) {
+                       int delay;
+                       struct timeval when;
+
+                       if (this->type != RAD_LISTEN_DETAIL) continue;
+                       
+                       delay = detail_delay(this);
+                       if (!delay) continue;
+
+                       fr_event_now(el, &now);
+                       when = now;
+                       tv_add(&when, delay);
+
+                       if (delay > 100000) {
+                               DEBUG2("Delaying next detail event for %d.%01u seconds.",
+                                      delay / USEC, (delay % USEC) / 100000);
+                       }
+
+                       if (!fr_event_insert(el, event_detail_timer, this,
+                                            &when, NULL)) {
+                               radlog(L_ERR, "Failed remembering timer");
+                               exit(1);
+                       }
+               }
+       }
+
        if ((flag & RADIUS_SIGNAL_SELF_NEW_FD) != 0) {
                rad_listen_t *this;
                
@@ -2154,7 +2194,7 @@ static void handle_signal_self(int flag)
                        if (!fr_event_fd_insert(el, 0, this->fd,
                                                event_socket_handler, this)) {
                                radlog(L_ERR, "Failed remembering handle for proxy socket!");
-                               _exit(1);
+                               exit(1);
                        }
                }
        }
@@ -2175,24 +2215,13 @@ void radius_signal_self(int flag)
        uint8_t buffer[16];
 
        /*
-        *      The caller is telling us it's OK to read from the
-        *      detail files.  However, we're not even listening on
-        *      the detail files.  Therefore, suppress the flag to
-        *      avoid bothering the main worker thread.
-        */
-       if ((flag == RADIUS_SIGNAL_SELF_DETAIL) &&
-           !has_detail_listener) {
-               return;
-       }
-
-       /*
         *      The read MUST be non-blocking for this to work.
         */
        rcode = read(self_pipe[0], buffer, sizeof(buffer));
        if (rcode > 0) {
                ssize_t i;
 
-               for (i = 1; i < rcode; i++) {
+               for (i = 0; i < rcode; i++) {
                        buffer[0] |= buffer[i];
                }
        } else {
@@ -2209,7 +2238,7 @@ static void event_signal_handler(UNUSED fr_event_list_t *xel,
                                 UNUSED int fd, UNUSED void *ctx)
 {
        ssize_t i, rcode;
-       char buffer[32];
+       uint8_t buffer[32];
 
        rcode = read(self_pipe[0], buffer, sizeof(buffer));
        if (rcode <= 0) return;
@@ -2217,7 +2246,7 @@ static void event_signal_handler(UNUSED fr_event_list_t *xel,
        /*
         *      Merge pending signals.
         */
-       for (i = 1; i < rcode; i++) {
+       for (i = 0; i < rcode; i++) {
                buffer[0] |= buffer[i];
        }
 
@@ -2225,99 +2254,9 @@ static void event_signal_handler(UNUSED fr_event_list_t *xel,
 }
 #endif
 
-/*
- *     The given FD is closed.  Delete it from the list of FD's to poll,
- *     and set a timer to periodically look for an FD again.
- */
-static void event_reset_fd(rad_listen_t *listener, int fd, int usec)
-{
-       struct timeval when;
-       gettimeofday(&when, NULL);
-
-       tv_add(&when, usec);    
-
-       if ((fd >= 0) && !fr_event_fd_delete(el, 0, fd)) {
-               rad_panic("Failed deleting fd");
-       }
-       
-       if (!fr_event_insert(el, event_poll_fd, listener,
-                              &when, NULL)) {
-               rad_panic("Failed inserting event");
-       }
-}
-
 
-/*
- *     Reads from the detail file if possible, OR sets up a timeout
- *     to see if the detail file is ready for reading.
- *
- *     FIXME: On Linux, use inotify to watch the detail file.  This
- *     
- */
-static void event_detail_handler(fr_event_list_t *xel, int fd, void *ctx)
-{
-       rad_listen_t *listener = ctx;
-       RAD_REQUEST_FUNP fun;
-       REQUEST *request;
-
-       rad_assert(xel == el);
-       rad_assert(fd == listener->fd);
-
-       xel = xel;
-
-       /*
-        *      We're too busy to read from the detail file.  Stop
-        *      watching the file, and instead go back to polling.
-        */
-       if (!read_from_detail) {
-               event_reset_fd(listener, fd, USEC);
-               return;
-       }
-       
-       /*
-        *      FIXME: Put this somewhere else, where it isn't called
-        *      all of the time...
-        */
-#if !defined(HAVE_PTHREAD_H) && defined(WNOHANG)
-       /*
-        *      If there are no child threads, then there may
-        *      be child processes.  In that case, wait for
-        *      their exit status, and throw that exit status
-        *      away.  This helps get rid of zxombie children.
-        */
-       while (waitpid(-1, &argval, WNOHANG) > 0) {
-               /* do nothing */
-       }
-#endif
-
-       /*
-        *      Didn't read anything from the detail file.
-        *      Wake up in one second to check it again.
-        */
-       if (!listener->recv(listener, &fun, &request)) {
-               event_reset_fd(listener, fd, USEC);
-               return;
-       }
-
-       /*
-        *      Drop the request into the thread pool,
-        *      and let the thread pool take care of
-        *      doing something with it.
-        */
-       if (!thread_pool_addrequest(request, fun)) {
-               request->child_state = REQUEST_DONE;
-       }
-
-       /*
-        *      Reset based on the fd we were given, as detail_recv()
-        *      may have read all of the file, and closed it.  It then
-        *      sets listener->fd = -1.
-        */
-       event_reset_fd(listener, fd, *(int *) listener->data);
-}
-
-
-static void event_socket_handler(fr_event_list_t *xel, UNUSED int fd, void *ctx)
+static void event_socket_handler(fr_event_list_t *xel, UNUSED int fd,
+                                void *ctx)
 {
        rad_listen_t *listener = ctx;
        RAD_REQUEST_FUNP fun;
@@ -2350,83 +2289,77 @@ static void event_socket_handler(fr_event_list_t *xel, UNUSED int fd, void *ctx)
        if (!thread_pool_addrequest(request, fun)) {
                request->child_state = REQUEST_DONE;
        }
-
-       /*
-        *      If we've read a packet from a socket OTHER than the
-        *      detail file, we start ignoring the detail file.
-        *
-        *      When the child thread pulls the request off of the
-        *      queues, it will check if the queues are empty.  Once
-        *      all queues are empty, it will signal us to read the
-        *      detail file again.
-        */
-       read_from_detail = FALSE;
 }
 
 
 /*
- *     This function is called periodically to see if a file
- *     descriptor is available for reading.
+ *     This function is called periodically to see if any FD's are
+ *     available for reading.
  */
-static void event_poll_fd(void *ctx)
+static void event_poll_fds(UNUSED void *ctx)
 {
        int rcode;
        RAD_REQUEST_FUNP fun;
        REQUEST *request;
-       rad_listen_t *listener = ctx;
-       fr_event_fd_handler_t handler;
+       rad_listen_t *this;
+       struct timeval when;
 
-       /*
-        *      Ignore the detail file if we're busy with other work.
-        */
-       if ((listener->type == RAD_LISTEN_DETAIL) && !read_from_detail) {
-               event_reset_fd(listener, listener->fd, USEC);
-               return;
-       }
+       fr_event_now(el, &now);
+       when = now;
+       when.tv_sec += 1;
 
-       /*
-        *      Try to read a RADIUS packet.  This also opens the FD.
-        *
-        *      FIXME: This does poll AND receive.
-        */
-       rcode = listener->recv(listener, &fun, &request);
+       for (this = mainconfig.listen; this != NULL; this = this->next) {
+               if (this->fd >= 0) continue;
 
-       /*
-        *      We read a request.  Add it to the list for processing.
-        */
-       if (rcode == 1) {
+               /*
+                *      Try to read something.
+                *
+                *      FIXME: This does poll AND receive.
+                */
+               rcode = this->recv(this, &fun, &request);
+               if (!rcode) continue;
+               
                rad_assert(fun != NULL);
                rad_assert(request != NULL);
-
+                       
                if (!thread_pool_addrequest(request, fun)) {
                        request->child_state = REQUEST_DONE;
                }
-       }
 
-       /*
-        *      Still no FD, OR the fd was read completely, and then
-        *      closed.  Re-set the polling timer and return.
-        */
-       if (listener->fd < 0) {
-               event_reset_fd(listener, -1, USEC);
-               return;
-       }
+               /*
+                *      We have an FD.  Start watching it.
+                */
+               if (this->fd >= 0) {
+                       /*
+                        *      ... unless it's a detail file.  In
+                        *      that case, we rely on the signal to
+                        *      self to know when to continue
+                        *      processing the detail file.
+                        */
+                       if (this->type == RAD_LISTEN_DETAIL) continue;
 
-       if (listener->type == RAD_LISTEN_DETAIL) {
-               handler = event_detail_handler;
-       } else {
-               handler = event_socket_handler;
+                       /*
+                        *      FIXME: this should be SNMP handler,
+                        *      and we should do SOMETHING when the
+                        *      fd is closed!
+                        */
+                       if (!fr_event_fd_insert(el, 0, this->fd,
+                                               event_socket_handler, this)) {
+                               char buffer[256];
+                               
+                               this->print(this, buffer, sizeof(buffer));
+                               rad_panic("Failed creating handler for snmp");
+                       }
+               }
        }
 
        /*
-        *      We have an FD.  Start watching it.
+        *      Reset the poll.
         */
-       if (!fr_event_fd_insert(el, 0, listener->fd,
-                                 handler, listener)) {
-               char buffer[256];
-               
-               listener->print(listener, buffer, sizeof(buffer));
-               rad_panic("Failed creating handler for socket");
+       if (!fr_event_insert(el, event_poll_fds, NULL,
+                            &when, NULL)) {
+               radlog(L_ERR, "Failed creating handler");
+               exit(1);
        }
 }
 
@@ -2439,9 +2372,6 @@ static void event_status(struct timeval *wake)
                        just_started = FALSE;
                }
                return;
-
-       } else {
-               read_from_detail = TRUE;
        }
 
        if (!wake) {
@@ -2449,7 +2379,7 @@ static void event_status(struct timeval *wake)
 
        } else if ((wake->tv_sec != 0) ||
                   (wake->tv_usec >= 100000)) {
-               DEBUG("Waking up in %d.%01u seconds.\n",
+               DEBUG("Waking up in %d.%01u seconds.",
                      (int) wake->tv_sec, (unsigned int) wake->tv_usec / 100000);
        }
 }
@@ -2461,6 +2391,7 @@ static void event_status(struct timeval *wake)
 int radius_event_init(CONF_SECTION *cs, int spawn_flag)
 {
        int i;
+       int has_snmp_listener = FALSE;
        rad_listen_t *this, *head = NULL;
 
        if (el) return 0;
@@ -2574,6 +2505,7 @@ int radius_event_init(CONF_SECTION *cs, int spawn_flag)
 
                case RAD_LISTEN_SNMP:
                        DEBUG("Listening on SNMP %s", buffer);
+                       has_snmp_listener = TRUE;
                        break;
 
                case RAD_LISTEN_PROXY:
@@ -2599,16 +2531,6 @@ int radius_event_init(CONF_SECTION *cs, int spawn_flag)
                 *      and detail file fd's.
                 */
                if (this->fd < 0) {
-                       struct timeval when;
-
-                       gettimeofday(&when, NULL);
-                       when.tv_sec += 1;
-
-                       if (!fr_event_insert(el, event_poll_fd, this,
-                                              &when, NULL)) {
-                               radlog(L_ERR, "Failed creating handler");
-                               exit(1);
-                       }
                        continue;
                }
 
@@ -2628,6 +2550,19 @@ int radius_event_init(CONF_SECTION *cs, int spawn_flag)
                }
        }
 
+       if (has_detail_listener || has_snmp_listener) {
+               struct timeval when;
+               
+               gettimeofday(&when, NULL);
+               when.tv_sec += 1;
+               
+               if (!fr_event_insert(el, event_poll_fds, NULL,
+                                    &when, NULL)) {
+                       radlog(L_ERR, "Failed creating handler");
+                       exit(1);
+               }
+       }
+
        mainconfig.listen = head;
 
        return 1;