move assertion to correct place. Found by PVS-Studio
[freeradius.git] / src / main / detail.c
index 616eccc..3c0e95c 100644 (file)
@@ -41,8 +41,6 @@ RCSID("$Id$")
 
 #ifdef WITH_DETAIL
 
-extern bool check_config;
-
 #define USEC (1000000)
 
 static FR_NAME_NUMBER state_names[] = {
@@ -82,11 +80,14 @@ int detail_send(rad_listen_t *listener, REQUEST *request)
                data->signal = 1;
                data->state = STATE_NO_REPLY;
 
-               RDEBUG("Detail - No response configured for request %d.  Will retry in %d seconds",
-                      request->number, data->retry_interval);
+               RDEBUG("detail (%s): No response to request.  Will retry in %d seconds",
+                      data->name, data->retry_interval);
        } else {
                int rtt;
                struct timeval now;
+
+               RDEBUG("detail (%s): Done %s packet.", data->name, fr_packet_codes[request->packet->code]);
+
                /*
                 *      We call gettimeofday a lot.  But it should be OK,
                 *      because there's nothing else to do.
@@ -154,8 +155,8 @@ int detail_send(rad_listen_t *listener, REQUEST *request)
                 */
                if (data->delay_time > (USEC / 4)) data->delay_time= USEC / 4;
 
-               RDEBUG3("Received response for request %d.  Will read the next packet in %d seconds",
-                       request->number, data->delay_time / USEC);
+               RDEBUG3("detail (%s): Received response for request %d.  Will read the next packet in %d seconds",
+                       data->name, request->number, data->delay_time / USEC);
 
                data->last_packet = now;
                data->signal = 1;
@@ -165,7 +166,7 @@ int detail_send(rad_listen_t *listener, REQUEST *request)
 
 #ifdef WITH_DETAIL_THREAD
        if (write(data->child_pipe[1], &c, 1) < 0) {
-               ERROR("Failed writing ack to reader thread: %s", fr_syserror(errno));
+               RERROR("detail (%s): Failed writing ack to reader thread: %s", data->name, fr_syserror(errno));
        }
 #else
        radius_signal_self(RADIUS_SIGNAL_SELF_DETAIL);
@@ -203,7 +204,22 @@ static int detail_open(rad_listen_t *this)
         *      this file will be read && processed before the
         *      file globbing is done.
         */
+       data->fp = NULL;
        data->work_fd = open(data->filename_work, O_RDWR);
+
+       /*
+        *      Couldn't open it for a reason OTHER than "it doesn't
+        *      exist".  Complain and tell the admin.
+        */
+       if ((data->work_fd < 0) && (errno != ENOENT)) {
+               ERROR("Failed opening detail file %s: %s",
+                     data->filename_work, fr_syserror(errno));
+               return 0;
+       }
+
+       /*
+        *      The file doesn't exist.  Poll for it again.
+        */
        if (data->work_fd < 0) {
 #ifndef HAVE_GLOB_H
                return 0;
@@ -214,7 +230,7 @@ static int detail_open(rad_listen_t *this)
                char const      *filename;
                glob_t          files;
 
-               DEBUG2("Polling for detail file %s", data->filename);
+               DEBUG2("detail (%s): Polling for detail file", data->name);
 
                memset(&files, 0, sizeof(files));
                if (glob(data->filename, 0, NULL, &files) != 0) {
@@ -245,10 +261,10 @@ static int detail_open(rad_listen_t *this)
                 */
                filename = files.gl_pathv[found];
 
-               DEBUG("Detail - Renaming %s -> %s", filename, data->filename_work);
+               DEBUG("detail (%s): Renaming %s -> %s", data->name, filename, data->filename_work);
                if (rename(filename, data->filename_work) < 0) {
-                       ERROR("Detail - Failed renaming %s to %s: %s",
-                             filename, data->filename_work, fr_syserror(errno));
+                       ERROR("detail (%s): Failed renaming %s to %s: %s",
+                             data->name, filename, data->filename_work, fr_syserror(errno));
                        goto noop;
                }
 
@@ -258,7 +274,11 @@ static int detail_open(rad_listen_t *this)
                 *      And try to open the filename.
                 */
                data->work_fd = open(data->filename_work, O_RDWR);
-               if (data->work_fd < 0) return 0;
+               if (data->work_fd < 0) {
+                       ERROR("Failed opening detail file %s: %s",
+                                       data->filename_work, fr_syserror(errno));
+                       return 0;
+               }
 #endif
        } /* else detail.work existed, and we opened it */
 
@@ -269,9 +289,10 @@ static int detail_open(rad_listen_t *this)
 
        data->client_ip.af = AF_UNSPEC;
        data->timestamp = 0;
-       data->offset = 0;
+       data->offset = data->last_offset = data->timestamp_offset = 0;
        data->packets = 0;
        data->tries = 0;
+       data->done_entry = false;
 
        return 1;
 }
@@ -299,6 +320,7 @@ int detail_recv(rad_listen_t *listener)
 {
        RADIUS_PACKET *packet;
        listen_detail_t *data = listener->data;
+       RAD_REQUEST_FUNP fun = NULL;
 
        /*
         *      We may be in the main thread.  It needs to update the
@@ -309,11 +331,38 @@ int detail_recv(rad_listen_t *listener)
        packet = detail_poll(listener);
        if (!packet) return -1;
 
+       if (DEBUG_ENABLED2) {
+               VALUE_PAIR *vp;
+               vp_cursor_t cursor;
+
+               DEBUG2("detail (%s): Read packet from %s", data->name, data->filename_work);
+               for (vp = fr_cursor_init(&cursor, &packet->vps);
+                    vp;
+                    vp = fr_cursor_next(&cursor)) {
+                       debug_pair(vp);
+               }
+       }
+
+       switch (packet->code) {
+       case PW_CODE_ACCOUNTING_REQUEST:
+               fun = rad_accounting;
+               break;
+
+       case PW_CODE_COA_REQUEST:
+       case PW_CODE_DISCONNECT_REQUEST:
+               fun = rad_coa_recv;
+               break;
+
+       default:
+               rad_free(&packet);
+               data->state = STATE_REPLIED;
+               return 0;
+       }
+
        /*
         *      Don't bother doing limit checks, etc.
         */
-       if (!request_receive(listener, packet, &data->detail_client,
-                            rad_accounting)) {
+       if (!request_receive(NULL, listener, packet, &data->detail_client, fun)) {
                rad_free(&packet);
                data->state = STATE_NO_REPLY;   /* try again later */
                return 0;
@@ -324,9 +373,11 @@ int detail_recv(rad_listen_t *listener)
 #else
 int detail_recv(rad_listen_t *listener)
 {
+       char c = 0;
        ssize_t rcode;
        RADIUS_PACKET *packet;
        listen_detail_t *data = listener->data;
+       RAD_REQUEST_FUNP fun = NULL;
 
        /*
         *      Block until there's a packet ready.
@@ -336,13 +387,41 @@ int detail_recv(rad_listen_t *listener)
 
        rad_assert(packet != NULL);
 
-       if (!request_receive(listener, packet, &data->detail_client,
-                                    rad_accounting)) {
-               char c = 0;
-               rad_free(&packet);
+       if (DEBUG_ENABLED2) {
+               VALUE_PAIR *vp;
+               vp_cursor_t cursor;
+
+               DEBUG2("detail (%s): Read packet from %s", data->name, data->filename_work);
+               for (vp = fr_cursor_init(&cursor, &packet->vps);
+                    vp;
+                    vp = fr_cursor_next(&cursor)) {
+                       debug_pair(vp);
+               }
+       }
+
+       switch (packet->code) {
+       case PW_CODE_ACCOUNTING_REQUEST:
+               fun = rad_accounting;
+               break;
+
+       case PW_CODE_COA_REQUEST:
+       case PW_CODE_DISCONNECT_REQUEST:
+               fun = rad_coa_recv;
+               break;
+
+       default:
+               data->state = STATE_REPLIED;
+               goto signal_thread;
+       }
+
+       if (!request_receive(NULL, listener, packet, &data->detail_client, fun)) {
                data->state = STATE_NO_REPLY;   /* try again later */
+
+       signal_thread:
+               rad_free(&packet);
                if (write(data->child_pipe[1], &c, 1) < 0) {
-                       ERROR("Failed writing ack to reader thread: %s", fr_syserror(errno));
+                       ERROR("detail (%s): Failed writing ack to reader thread: %s", data->name,
+                             fr_syserror(errno));
                }
        }
 
@@ -355,6 +434,7 @@ int detail_recv(rad_listen_t *listener)
 
 static RADIUS_PACKET *detail_poll(rad_listen_t *listener)
 {
+       int             y;
        char            key[256], op[8], value[1024];
        vp_cursor_t     cursor;
        VALUE_PAIR      *vp;
@@ -363,308 +443,363 @@ static RADIUS_PACKET *detail_poll(rad_listen_t *listener)
        listen_detail_t *data = listener->data;
 
        switch (data->state) {
-               case STATE_UNOPENED:
-       open_file:
-                       rad_assert(data->work_fd < 0);
+       case STATE_UNOPENED:
+open_file:
+               rad_assert(data->work_fd < 0);
 
-                       if (!detail_open(listener)) return NULL;
+               if (!detail_open(listener)) return NULL;
 
-                       rad_assert(data->state == STATE_UNLOCKED);
-                       rad_assert(data->work_fd >= 0);
+               rad_assert(data->state == STATE_UNLOCKED);
+               rad_assert(data->work_fd >= 0);
 
-                       /* FALL-THROUGH */
+               /* FALL-THROUGH */
 
+       /*
+        *      Try to lock fd.  If we can't, return.
+        *      If we can, continue.  This means that
+        *      the server doesn't block while waiting
+        *      for the lock to open...
+        */
+       case STATE_UNLOCKED:
+               /*
+                *      Note that we do NOT block waiting for
+                *      the lock.  We've re-named the file
+                *      above, so we've already guaranteed
+                *      that any *new* detail writer will not
+                *      be opening this file.  The only
+                *      purpose of the lock is to catch a race
+                *      condition where the execution
+                *      "ping-pongs" between radiusd &
+                *      radrelay.
+                */
+               if (rad_lockfd_nonblock(data->work_fd, 0) < 0) {
                        /*
-                        *      Try to lock fd.  If we can't, return.
-                        *      If we can, continue.  This means that
-                        *      the server doesn't block while waiting
-                        *      for the lock to open...
-                        */
-               case STATE_UNLOCKED:
-                       /*
-                        *      Note that we do NOT block waiting for
-                        *      the lock.  We've re-named the file
-                        *      above, so we've already guaranteed
-                        *      that any *new* detail writer will not
-                        *      be opening this file.  The only
-                        *      purpose of the lock is to catch a race
-                        *      condition where the execution
-                        *      "ping-pongs" between radiusd &
-                        *      radrelay.
+                        *      Close the FD.  The main loop
+                        *      will wake up in a second and
+                        *      try again.
                         */
-                       if (rad_lockfd_nonblock(data->work_fd, 0) < 0) {
-                               /*
-                                *      Close the FD.  The main loop
-                                *      will wake up in a second and
-                                *      try again.
-                                */
-                               close(data->work_fd);
-                               data->work_fd = -1;
-                               data->state = STATE_UNOPENED;
-                               return NULL;
-                       }
+                       close(data->work_fd);
+                       data->fp = NULL;
+                       data->work_fd = -1;
+                       data->state = STATE_UNOPENED;
+                       return NULL;
+               }
+
+               /*
+                *      Only open for writing if we're
+                *      marking requests as completed.
+                */
+               data->fp = fdopen(data->work_fd, data->track ? "r+" : "r");
+               if (!data->fp) {
+                       ERROR("detail (%s): FATAL: Failed to re-open detail file: %s",
+                             data->name, fr_syserror(errno));
+                       fr_exit(1);
+               }
+
+               /*
+                *      Look for the header
+                */
+               data->state = STATE_HEADER;
+               data->delay_time = USEC;
+               data->vps = NULL;
+
+               /* FALL-THROUGH */
+
+       case STATE_HEADER:
+       do_header:
+               data->done_entry = false;
+               data->timestamp_offset = 0;
+
+               data->tries = 0;
+               if (!data->fp) {
+                       data->state = STATE_UNOPENED;
+                       goto open_file;
+               }
+
+               {
+                       struct stat buf;
 
-                       data->fp = fdopen(data->work_fd, "r");
-                       if (!data->fp) {
-                               ERROR("FATAL: Failed to re-open detail file %s: %s",
-                                      data->filename, fr_syserror(errno));
-                               fr_exit(1);
+                       if (fstat(data->work_fd, &buf) < 0) {
+                               ERROR("detail (%s): Failed to stat detail file: %s",
+                                     data->name, fr_syserror(errno));
+
+                               goto cleanup;
+                       }
+                       if (((off_t) ftell(data->fp)) == buf.st_size) {
+                               goto cleanup;
                        }
+               }
 
-                       /*
-                        *      Look for the header
-                        */
-                       data->state = STATE_HEADER;
-                       data->delay_time = USEC;
-                       data->vps = NULL;
-
-                       /* FALL-THROUGH */
-
-               case STATE_HEADER:
-               do_header:
-                       data->tries = 0;
-                       if (!data->fp) {
-                               data->state = STATE_UNOPENED;
-                               goto open_file;
+               /*
+                *      End of file.  Delete it, and re-set
+                *      everything.
+                */
+               if (feof(data->fp)) {
+               cleanup:
+                       DEBUG("detail (%s): Unlinking %s", data->name, data->filename_work);
+                       unlink(data->filename_work);
+                       if (data->fp) fclose(data->fp);
+                       data->fp = NULL;
+                       data->work_fd = -1;
+                       data->state = STATE_UNOPENED;
+                       rad_assert(data->vps == NULL);
+
+                       if (data->one_shot) {
+                               INFO("detail (%s): Finished reading \"one shot\" detail file - Exiting", data->name);
+                               radius_signal_self(RADIUS_SIGNAL_SELF_EXIT);
                        }
 
-                       {
-                               struct stat buf;
+                       return NULL;
+               }
 
-                               if (fstat(data->work_fd, &buf) < 0) {
-                                       ERROR("Failed to stat "
-                                              "detail file %s: %s",
-                                               data->filename,
-                                               fr_syserror(errno));
+               /*
+                *      Else go read something.
+                */
+               if (!fgets(buffer, sizeof(buffer), data->fp)) {
+                       DEBUG("detail (%s): Failed reading header from file - %s",
+                             data->name, data->filename_work);
+                       goto cleanup;
+               }
 
-                                       goto cleanup;
-                               }
-                               if (((off_t) ftell(data->fp)) == buf.st_size) {
-                                       goto cleanup;
-                               }
-                       }
+               /*
+                *      Badly formatted file: delete it.
+                */
+               if (!strchr(buffer, '\n')) {
+                       DEBUG("detail (%s): Invalid line without trailing LF - %s", data->name, buffer);
+                       goto cleanup;
+               }
+
+               if (!sscanf(buffer, "%*s %*s %*d %*d:%*d:%*d %d", &y)) {
+                       DEBUG("detail (%s): Failed reading detail file header in line - %s", data->name, buffer);
+                       goto cleanup;
+               }
+
+               data->state = STATE_READING;
+               /* FALL-THROUGH */
+
+
+       /*
+        *      Read more value-pair's, unless we're
+        *      at EOF.  In that case, queue whatever
+        *      we have.
+        */
+       case STATE_READING:
+               rad_assert(data->fp != NULL);
+
+               fr_cursor_init(&cursor, &data->vps);
+
+               /*
+                *      Read a header, OR a value-pair.
+                */
+               while (fgets(buffer, sizeof(buffer), data->fp)) {
+                       data->last_offset = data->offset;
+                       data->offset = ftell(data->fp); /* for statistics */
 
                        /*
-                        *      End of file.  Delete it, and re-set
-                        *      everything.
+                        *      Badly formatted file: delete it.
                         */
-                       if (feof(data->fp)) {
-                       cleanup:
-                               DEBUG("Detail - unlinking %s",
-                                     data->filename_work);
-                               unlink(data->filename_work);
-                               if (data->fp) fclose(data->fp);
-                               data->fp = NULL;
-                               data->work_fd = -1;
-                               data->state = STATE_UNOPENED;
-                               rad_assert(data->vps == NULL);
-
-                               if (data->one_shot) {
-                                       INFO("Finished reading \"one shot\" detail file - Exiting");
-                                       radius_signal_self(RADIUS_SIGNAL_SELF_EXIT);
-                               }
-
-                               return NULL;
+                       if (!strchr(buffer, '\n')) {
+                               WARN("detail (%s): Skipping line without trailing LF - %s", data->name, buffer);
+                               fr_pair_list_free(&data->vps);
+                               goto cleanup;
                        }
 
                        /*
-                        *      Else go read something.
+                        *      We're reading VP's, and got a blank line.
+                        *      That indicates the end of an entry.  Queue the
+                        *      packet.
                         */
-                       break;
+                       if (buffer[0] == '\n') {
+                               data->state = STATE_QUEUED;
+                               data->tries = 0;
+                               data->packets++;
+                               goto alloc_packet;
+                       }
 
                        /*
-                        *      Read more value-pair's, unless we're
-                        *      at EOF.  In that case, queue whatever
-                        *      we have.
+                        *      We have a full "attribute = value" line.
+                        *      If it doesn't look reasonable, skip it.
+                        *
+                        *      FIXME: print an error for badly formatted attributes?
                         */
-               case STATE_READING:
-                       if (data->fp && !feof(data->fp)) break;
-                       data->state = STATE_QUEUED;
-
-                       /* FALL-THROUGH */
-
-               case STATE_QUEUED:
-                       goto alloc_packet;
+                       if (sscanf(buffer, "%255s %7s %1023s", key, op, value) != 3) {
+                               DEBUG("detail (%s): Skipping badly formatted line - %s", data->name, buffer);
+                               continue;
+                       }
 
                        /*
-                        *      Periodically check what's going on.
-                        *      If the request is taking too long,
-                        *      retry it.
+                        *      Should be =, :=, +=, ...
                         */
-               case STATE_RUNNING:
-                       if (time(NULL) < (data->running + data->retry_interval)) {
-                               return NULL;
+                       if (!strchr(op, '=')) {
+                               DEBUG("detail (%s): Skipping line without operator - %s", data->name, buffer);
+                               continue;
                        }
 
-                       DEBUG("No response to detail request.  Retrying");
-                       /* FALL-THROUGH */
+                       /*
+                        *      Skip non-protocol attributes.
+                        */
+                       if (!strcasecmp(key, "Request-Authenticator")) continue;
 
                        /*
-                        *      If there's no reply, keep
-                        *      retransmitting the current packet
-                        *      forever.
+                        *      Set the original client IP address, based on
+                        *      what's in the detail file.
+                        *
+                        *      Hmm... we don't set the server IP address.
+                        *      or port.  Oh well.
                         */
-               case STATE_NO_REPLY:
-                       data->state = STATE_QUEUED;
-                       goto alloc_packet;
+                       if (!strcasecmp(key, "Client-IP-Address")) {
+                               data->client_ip.af = AF_INET;
+                               if (ip_hton(&data->client_ip, AF_INET, value, false) < 0) {
+                                       DEBUG("detail (%s): Failed parsing Client-IP-Address", data->name);
+                                       fr_pair_list_free(&data->vps);
+                                       goto cleanup;
+                               }
+                               continue;
+                       }
 
                        /*
-                        *      We have a reply.  Clean up the old
-                        *      request, and go read another one.
+                        *      The original time at which we received the
+                        *      packet.  We need this to properly calculate
+                        *      Acct-Delay-Time.
                         */
-               case STATE_REPLIED:
-                       pairfree(&data->vps);
-                       data->state = STATE_HEADER;
-                       goto do_header;
-       }
+                       if (!strcasecmp(key, "Timestamp")) {
+                               data->timestamp = atoi(value);
+                               data->timestamp_offset = data->last_offset;
+
+                               vp = fr_pair_afrom_num(data, PW_PACKET_ORIGINAL_TIMESTAMP, 0);
+                               if (vp) {
+                                       vp->vp_date = (uint32_t) data->timestamp;
+                                       vp->type = VT_DATA;
+                                       fr_cursor_insert(&cursor, vp);
+                               }
+                               continue;
+                       }
 
-       fr_cursor_init(&cursor, &data->vps);
+                       if (!strcasecmp(key, "Donestamp")) {
+                               data->timestamp = atoi(value);
+                               data->done_entry = true;
+                               continue;
+                       }
 
-       /*
-        *      Read a header, OR a value-pair.
-        */
-       while (fgets(buffer, sizeof(buffer), data->fp)) {
-               data->offset = ftell(data->fp); /* for statistics */
+                       DEBUG3("detail (%s): Trying to read VP from line - %s", data->name, buffer);
 
-               /*
-                *      Badly formatted file: delete it.
-                *
-                *      FIXME: Maybe flag an error?
-                */
-               if (!strchr(buffer, '\n')) {
-                       pairfree(&data->vps);
-                       goto cleanup;
+                       /*
+                        *      Read one VP.
+                        *
+                        *      FIXME: do we want to check for non-protocol
+                        *      attributes like radsqlrelay does?
+                        */
+                       vp = NULL;
+                       if ((fr_pair_list_afrom_str(data, buffer, &vp) > 0) &&
+                           (vp != NULL)) {
+                               fr_cursor_merge(&cursor, vp);
+                       } else {
+                               DEBUG("detail (%s): Failed reading VP from line - %s", data->name, buffer);
+                               goto cleanup;
+                       }
                }
 
                /*
-                *      We're reading VP's, and got a blank line.
-                *      Queue the packet.
+                *      The writer doesn't check that the
+                *      record was completely written.  If the
+                *      disk is full, this can result in a
+                *      truncated record which has no trailing
+                *      blank line.  When that happens, it's a
+                *      bad record, and we ignore it.
                 */
-               if ((data->state == STATE_READING) &&
-                   (buffer[0] == '\n')) {
-                       data->state = STATE_QUEUED;
-                       break;
+               if (feof(data->fp)) {
+                       DEBUG("detail (%s): Truncated record: treating it as EOF for detail file %s",
+                             data->name, data->filename_work);
+                       fr_pair_list_free(&data->vps);
+                       goto cleanup;
                }
 
                /*
-                *      Look for date/time header, and read VP's if
-                *      found.  If not, keep reading lines until we
-                *      find one.
+                *      Some kind of non-eof error.
+                *
+                *      FIXME: Leave the file in-place, and warn the
+                *      administrator?
                 */
-               if (data->state == STATE_HEADER) {
-                       int y;
+               DEBUG("detail (%s): Unknown error, deleting detail file %s",
+                     data->name, data->filename_work);
+               goto cleanup;
 
-                       if (sscanf(buffer, "%*s %*s %*d %*d:%*d:%*d %d", &y)) {
-                               data->state = STATE_READING;
-                       }
-                       continue;
-               }
+       case STATE_QUEUED:
+               goto alloc_packet;
 
-               /*
-                *      We have a full "attribute = value" line.
-                *      If it doesn't look reasonable, skip it.
-                *
-                *      FIXME: print an error for badly formatted attributes?
-                */
-               if (sscanf(buffer, "%255s %7s %1023s", key, op, value) != 3) {
-                       WARN("Skipping badly formatted line %s",
-                              buffer);
-                       continue;
+       /*
+        *      Periodically check what's going on.
+        *      If the request is taking too long,
+        *      retry it.
+        */
+       case STATE_RUNNING:
+               if (time(NULL) < (data->running + (int)data->retry_interval)) {
+                       return NULL;
                }
 
-               /*
-                *      Should be =, :=, +=, ...
-                */
-               if (!strchr(op, '=')) continue;
-
-               /*
-                *      Skip non-protocol attributes.
-                */
-               if (!strcasecmp(key, "Request-Authenticator")) continue;
+               DEBUG("detail (%s): No response to detail request.  Retrying", data->name);
+               /* FALL-THROUGH */
 
-               /*
-                *      Set the original client IP address, based on
-                *      what's in the detail file.
-                *
-                *      Hmm... we don't set the server IP address.
-                *      or port.  Oh well.
-                */
-               if (!strcasecmp(key, "Client-IP-Address")) {
-                       data->client_ip.af = AF_INET;
-                       if (ip_hton(value, AF_INET, &data->client_ip) < 0) {
-                               ERROR("Failed parsing Client-IP-Address");
+       /*
+        *      If there's no reply, keep
+        *      retransmitting the current packet
+        *      forever.
+        */
+       case STATE_NO_REPLY:
+               data->state = STATE_QUEUED;
+               goto alloc_packet;
 
-                               pairfree(&data->vps);
-                               goto cleanup;
+       /*
+        *      We have a reply.  Clean up the old
+        *      request, and go read another one.
+        */
+       case STATE_REPLIED:
+               if (data->track) {
+                       rad_assert(data->fp != NULL);
+
+                       if (fseek(data->fp, data->timestamp_offset, SEEK_SET) < 0) {
+                               DEBUG("detail (%s): Failed seeking to timestamp offset: %s",
+                                    data->name, fr_syserror(errno));
+                       } else if (fwrite("\tDone", 1, 5, data->fp) < 5) {
+                               DEBUG("detail (%s): Failed marking request as done: %s",
+                                    data->name, fr_syserror(errno));
+                       } else if (fflush(data->fp) != 0) {
+                               DEBUG("detail (%s): Failed flushing marked detail file to disk: %s",
+                                    data->name, fr_syserror(errno));
                        }
-                       continue;
-               }
 
-               /*
-                *      The original time at which we received the
-                *      packet.  We need this to properly calculate
-                *      Acct-Delay-Time.
-                */
-               if (!strcasecmp(key, "Timestamp")) {
-                       data->timestamp = atoi(value);
-
-                       vp = paircreate(data, PW_PACKET_ORIGINAL_TIMESTAMP, 0);
-                       if (vp) {
-                               vp->vp_date = (uint32_t) data->timestamp;
-                               vp->type = VT_DATA;
-                               fr_cursor_insert(&cursor, vp);
+                       if (fseek(data->fp, data->offset, SEEK_SET) < 0) {
+                               DEBUG("detail (%s): Failed seeking to next detail request: %s",
+                                    data->name, fr_syserror(errno));
                        }
-                       continue;
                }
 
-               /*
-                *      Read one VP.
-                *
-                *      FIXME: do we want to check for non-protocol
-                *      attributes like radsqlrelay does?
-                */
-               vp = NULL;
-               if ((userparse(data, buffer, &vp) > 0) &&
-                   (vp != NULL)) {
-                       fr_cursor_insert(&cursor, vp);
-               }
+               fr_pair_list_free(&data->vps);
+               data->state = STATE_HEADER;
+               goto do_header;
        }
 
        /*
-        *      Some kind of error.
-        *
-        *      FIXME: Leave the file in-place, and warn the
-        *      administrator?
-        */
-       if (ferror(data->fp)) goto cleanup;
-
-       data->tries = 0;
-       data->packets++;
-
-       /*
         *      Process the packet.
         */
  alloc_packet:
-       data->tries++;
-
-       /*
-        *      The writer doesn't check that the record was
-        *      completely written.  If the disk is full, this can
-        *      result in a truncated record.  When that happens,
-        *      treat it as EOF.
-        */
-       if (data->state != STATE_QUEUED) {
-               ERROR("Truncated record: treating it as EOF for detail file %s", data->filename_work);
-               goto cleanup;
+       if (data->done_entry) {
+               DEBUG2("detail (%s): Skipping record for timestamp %lu", data->name, data->timestamp);
+               fr_pair_list_free(&data->vps);
+               data->state = STATE_HEADER;
+               goto do_header;
        }
 
+       data->tries++;
+
        /*
         *      We're done reading the file, but we didn't read
         *      anything.  Clean up, and don't return anything.
         */
        if (!data->vps) {
+               WARN("detail (%s): Read empty packet from file %s",
+                    data->name, data->filename_work);
                data->state = STATE_HEADER;
-               if (!data->fp || feof(data->fp)) goto cleanup;
                return NULL;
        }
 
@@ -672,18 +807,28 @@ static RADIUS_PACKET *detail_poll(rad_listen_t *listener)
         *      Allocate the packet.  If we fail, it's a serious
         *      problem.
         */
-       packet = rad_alloc(NULL, 1);
+       packet = rad_alloc(NULL, true);
        if (!packet) {
-               ERROR("FATAL: Failed allocating memory for detail");
+               ERROR("detail (%s): FATAL: Failed allocating memory for detail", data->name);
                fr_exit(1);
-               _exit(1);
        }
 
        memset(packet, 0, sizeof(*packet));
        packet->sockfd = -1;
        packet->src_ipaddr.af = AF_INET;
        packet->src_ipaddr.ipaddr.ip4addr.s_addr = htonl(INADDR_NONE);
+
+       /*
+        *      If everything's OK, this is a waste of memory.
+        *      Otherwise, it lets us re-send the original packet
+        *      contents, unmolested.
+        */
+       packet->vps = fr_pair_list_copy(packet, data->vps);
+
        packet->code = PW_CODE_ACCOUNTING_REQUEST;
+       vp = fr_pair_find_by_num(packet->vps, PW_PACKET_TYPE, 0, TAG_ANY);
+       if (vp) packet->code = vp->vp_integer;
+
        gettimeofday(&packet->timestamp, NULL);
 
        /*
@@ -694,29 +839,33 @@ static RADIUS_PACKET *detail_poll(rad_listen_t *listener)
                packet->src_ipaddr = data->client_ip;
        }
 
-       vp = pairfind(packet->vps, PW_PACKET_SRC_IP_ADDRESS, 0, TAG_ANY);
+       vp = fr_pair_find_by_num(packet->vps, PW_PACKET_SRC_IP_ADDRESS, 0, TAG_ANY);
        if (vp) {
                packet->src_ipaddr.af = AF_INET;
                packet->src_ipaddr.ipaddr.ip4addr.s_addr = vp->vp_ipaddr;
+               packet->src_ipaddr.prefix = 32;
        } else {
-               vp = pairfind(packet->vps, PW_PACKET_SRC_IPV6_ADDRESS, 0, TAG_ANY);
+               vp = fr_pair_find_by_num(packet->vps, PW_PACKET_SRC_IPV6_ADDRESS, 0, TAG_ANY);
                if (vp) {
                        packet->src_ipaddr.af = AF_INET6;
                        memcpy(&packet->src_ipaddr.ipaddr.ip6addr,
                               &vp->vp_ipv6addr, sizeof(vp->vp_ipv6addr));
+                       packet->src_ipaddr.prefix = 128;
                }
        }
 
-       vp = pairfind(packet->vps, PW_PACKET_DST_IP_ADDRESS, 0, TAG_ANY);
+       vp = fr_pair_find_by_num(packet->vps, PW_PACKET_DST_IP_ADDRESS, 0, TAG_ANY);
        if (vp) {
                packet->dst_ipaddr.af = AF_INET;
                packet->dst_ipaddr.ipaddr.ip4addr.s_addr = vp->vp_ipaddr;
+               packet->dst_ipaddr.prefix = 32;
        } else {
-               vp = pairfind(packet->vps, PW_PACKET_DST_IPV6_ADDRESS, 0, TAG_ANY);
+               vp = fr_pair_find_by_num(packet->vps, PW_PACKET_DST_IPV6_ADDRESS, 0, TAG_ANY);
                if (vp) {
                        packet->dst_ipaddr.af = AF_INET6;
                        memcpy(&packet->dst_ipaddr.ipaddr.ip6addr,
                               &vp->vp_ipv6addr, sizeof(vp->vp_ipv6addr));
+                       packet->dst_ipaddr.prefix = 128;
                }
        }
 
@@ -731,57 +880,46 @@ static RADIUS_PACKET *detail_poll(rad_listen_t *listener)
        packet->dst_ipaddr.ipaddr.ip4addr.s_addr = htonl((INADDR_LOOPBACK & ~0xffffff) | ((data->counter >> 24) & 0xff));
 
        /*
-        *      If everything's OK, this is a waste of memory.
-        *      Otherwise, it lets us re-send the original packet
-        *      contents, unmolested.
+        *      Create / update accounting attributes.
         */
-       packet->vps = paircopy(packet, data->vps);
-
-       /*
-        *      Prefer the Event-Timestamp in the packet, if it
-        *      exists.  That is when the event occurred, whereas the
-        *      "Timestamp" field is when we wrote the packet to the
-        *      detail file, which could have been much later.
-        */
-       vp = pairfind(packet->vps, PW_EVENT_TIMESTAMP, 0, TAG_ANY);
-       if (vp) {
-               data->timestamp = vp->vp_integer;
-       }
+       if (packet->code == PW_CODE_ACCOUNTING_REQUEST) {
+               /*
+                *      Prefer the Event-Timestamp in the packet, if it
+                *      exists.  That is when the event occurred, whereas the
+                *      "Timestamp" field is when we wrote the packet to the
+                *      detail file, which could have been much later.
+                */
+               vp = fr_pair_find_by_num(packet->vps, PW_EVENT_TIMESTAMP, 0, TAG_ANY);
+               if (vp) {
+                       data->timestamp = vp->vp_integer;
+               }
 
-       /*
-        *      Look for Acct-Delay-Time, and update
-        *      based on Acct-Delay-Time += (time(NULL) - timestamp)
-        */
-       vp = pairfind(packet->vps, PW_ACCT_DELAY_TIME, 0, TAG_ANY);
-       if (!vp) {
-               vp = paircreate(packet, PW_ACCT_DELAY_TIME, 0);
-               rad_assert(vp != NULL);
-               pairadd(&packet->vps, vp);
-       }
-       if (data->timestamp != 0) {
-               vp->vp_integer += time(NULL) - data->timestamp;
+               /*
+                *      Look for Acct-Delay-Time, and update
+                *      based on Acct-Delay-Time += (time(NULL) - timestamp)
+                */
+               vp = fr_pair_find_by_num(packet->vps, PW_ACCT_DELAY_TIME, 0, TAG_ANY);
+               if (!vp) {
+                       vp = fr_pair_afrom_num(packet, PW_ACCT_DELAY_TIME, 0);
+                       rad_assert(vp != NULL);
+                       fr_pair_add(&packet->vps, vp);
+               }
+               if (data->timestamp != 0) {
+                       vp->vp_integer += time(NULL) - data->timestamp;
+               }
        }
 
        /*
         *      Set the transmission count.
         */
-       vp = pairfind(packet->vps, PW_PACKET_TRANSMIT_COUNTER, 0, TAG_ANY);
+       vp = fr_pair_find_by_num(packet->vps, PW_PACKET_TRANSMIT_COUNTER, 0, TAG_ANY);
        if (!vp) {
-               vp = paircreate(packet, PW_PACKET_TRANSMIT_COUNTER, 0);
+               vp = fr_pair_afrom_num(packet, PW_PACKET_TRANSMIT_COUNTER, 0);
                rad_assert(vp != NULL);
-               pairadd(&packet->vps, vp);
+               fr_pair_add(&packet->vps, vp);
        }
        vp->vp_integer = data->tries;
 
-       if (debug_flag) {
-               fr_printf_log("detail_recv: Read packet from %s\n", data->filename_work);
-               for (vp = fr_cursor_init(&cursor, &packet->vps);
-                    vp;
-                    vp = fr_cursor_next(&cursor)) {
-                       debug_pair(vp);
-               }
-       }
-
        data->state = STATE_RUNNING;
        data->running = packet->timestamp.tv_sec;
 
@@ -797,7 +935,7 @@ void detail_free(rad_listen_t *this)
 
 #ifdef WITH_DETAIL_THREAD
        if (!check_config) {
-               size_t ret;
+               ssize_t ret;
                void *arg = NULL;
 
                /*
@@ -808,7 +946,7 @@ void detail_free(rad_listen_t *this)
                data->child_pipe[0] = -1;
 
                /*
-                *      Tell it to stop (interrupting it's sleep)
+                *      Tell it to stop (interrupting its sleep)
                 */
                pthread_kill(data->pthread_id, SIGTERM);
 
@@ -817,10 +955,12 @@ void detail_free(rad_listen_t *this)
                 */
                ret = read(data->master_pipe[0], &arg, sizeof(arg));
                if (ret < 0) {
-                       ERROR("Reader thread exited without informing the master: %s", fr_syserror(errno));
-               }
-               if (ret != sizeof(arg)) {
-                       ERROR("Invalid thread pointer received from reader thread during exit");
+                       ERROR("detail (%s): Reader thread exited without informing the master: %s",
+                             data->name, fr_syserror(errno));
+               } else if (ret != sizeof(arg)) {
+                       ERROR("detail (%s): Invalid thread pointer received from reader thread during exit",
+                             data->name);
+                       ERROR("detail (%s): Expected %zu bytes, got %zi bytes", data->name, sizeof(arg), ret);
                }
 
                close(data->master_pipe[0]);
@@ -863,8 +1003,8 @@ static int detail_delay(listen_detail_t *data)
        delay += (USEC * 3) / 4;
        delay += fr_rand() % (USEC / 2);
 
-       DEBUG2("Detail listener %s state %s waiting %d.%06d sec",
-              data->filename,
+       DEBUG2("detail (%s): Detail listener state %s waiting %d.%06d sec",
+              data->name,
               fr_int2str(state_names, data->state, "?"),
               (delay / USEC), delay % USEC);
 
@@ -888,8 +1028,9 @@ int detail_encode(UNUSED rad_listen_t *this, UNUSED REQUEST *request)
 
        data->signal = 0;
 
-       DEBUG2("Detail listener %s state %s signalled %d waiting %d.%06d sec",
-              data->filename, fr_int2str(state_names, data->state, "?"),
+       DEBUG2("detail (%s): Detail listener state %s signalled %d waiting %d.%06d sec",
+              data->name,
+              fr_int2str(state_names, data->state, "?"),
               data->signal,
               data->delay_time / USEC,
               data->delay_time % USEC);
@@ -901,13 +1042,25 @@ int detail_encode(UNUSED rad_listen_t *this, UNUSED REQUEST *request)
 /*
  *     Overloaded to return "should we fix delay times"
  */
-int detail_decode(UNUSED rad_listen_t *this, UNUSED REQUEST *request)
+int detail_decode(rad_listen_t *this, REQUEST *request)
 {
 #ifdef WITH_DETAIL_THREAD
+       listen_detail_t *data = this->data;
+
+       RDEBUG("Received %s from detail file %s",
+              fr_packet_codes[request->packet->code], data->filename_work);
+
+       rdebug_pair_list(L_DBG_LVL_1, request, request->packet->vps, "\t");
+
        return 0;
 #else
        listen_detail_t *data = this->data;
 
+       RDEBUG("Received %s from detail file %s",
+              fr_packet_codes[request->packet->code], data->filename_work);
+
+       rdebug_pair_list(L_DBG_LVL_1, request, request->packet->vps, "\t");
+
        return data->signal;
 #endif
 }
@@ -933,7 +1086,8 @@ static void *detail_handler_thread(void *arg)
                        if (data->child_pipe[0] < 0) {
                                packet = NULL;
                                if (write(data->master_pipe[1], &packet, sizeof(packet)) < 0) {
-                                       ERROR("Failed writing exit status to master: %s", fr_syserror(errno));
+                                       ERROR("detail (%s): Failed writing exit status to master: %s",
+                                             data->name, fr_syserror(errno));
                                }
                                return NULL;
                        }
@@ -946,15 +1100,20 @@ static void *detail_handler_thread(void *arg)
                 */
                do {
                        if (write(data->master_pipe[1], &packet, sizeof(packet)) < 0) {
-                               ERROR("Failed passing detail packet pointer to master: %s", fr_syserror(errno));
+                               ERROR("detail (%s): Failed passing detail packet pointer to master: %s",
+                                     data->name, fr_syserror(errno));
                        }
 
                        if (read(data->child_pipe[0], &c, 1) < 0) {
-                               ERROR("Failed getting detail packet ack from master: %s", fr_syserror(errno));
+                               ERROR("detail (%s): Failed getting detail packet ack from master: %s",
+                                     data->name, fr_syserror(errno));
                                break;
                        }
 
                        if (data->delay_time > 0) usleep(data->delay_time);
+
+                       packet = detail_poll(this);
+                       if (!packet) break;
                } while (data->state != STATE_REPLIED);
        }
 
@@ -964,22 +1123,14 @@ static void *detail_handler_thread(void *arg)
 
 
 static const CONF_PARSER detail_config[] = {
-       { "detail",   PW_TYPE_FILE_OUTPUT | PW_TYPE_DEPRECATED,
-         offsetof(listen_detail_t, filename), NULL,  NULL },
-       { "filename",   PW_TYPE_FILE_OUTPUT | PW_TYPE_REQUIRED,
-         offsetof(listen_detail_t, filename), NULL,  NULL },
-       { "load_factor",   PW_TYPE_INTEGER,
-         offsetof(listen_detail_t, load_factor), NULL, STRINGIFY(10)},
-       { "poll_interval",   PW_TYPE_INTEGER,
-         offsetof(listen_detail_t, poll_interval), NULL, STRINGIFY(1)},
-       { "retry_interval",   PW_TYPE_INTEGER,
-         offsetof(listen_detail_t, retry_interval), NULL, STRINGIFY(30)},
-       { "one_shot",   PW_TYPE_BOOLEAN,
-         offsetof(listen_detail_t, one_shot), NULL, NULL},
-       { "max_outstanding",   PW_TYPE_INTEGER,
-         offsetof(listen_detail_t, load_factor), NULL, NULL},
-
-       { NULL, -1, 0, NULL, NULL }             /* end the list */
+       { "detail", FR_CONF_OFFSET(PW_TYPE_FILE_OUTPUT | PW_TYPE_DEPRECATED, listen_detail_t, filename), NULL },
+       { "filename", FR_CONF_OFFSET(PW_TYPE_FILE_OUTPUT | PW_TYPE_REQUIRED, listen_detail_t, filename), NULL },
+       { "load_factor", FR_CONF_OFFSET(PW_TYPE_INTEGER, listen_detail_t, load_factor), STRINGIFY(10) },
+       { "poll_interval", FR_CONF_OFFSET(PW_TYPE_INTEGER, listen_detail_t, poll_interval), STRINGIFY(1) },
+       { "retry_interval", FR_CONF_OFFSET(PW_TYPE_INTEGER, listen_detail_t, retry_interval), STRINGIFY(30) },
+       { "one_shot", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, listen_detail_t, one_shot), "no" },
+       { "track", FR_CONF_OFFSET(PW_TYPE_BOOLEAN, listen_detail_t, track), "no" },
+       CONF_PARSER_TERMINATOR
 };
 
 /*
@@ -990,7 +1141,7 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
        int             rcode;
        listen_detail_t *data;
        RADCLIENT       *client;
-       char buffer[2048];
+       char            buffer[2048];
 
        data = this->data;
 
@@ -1000,6 +1151,9 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
                return -1;
        }
 
+       data->name = cf_section_name2(cs);
+       if (!data->name) data->name = data->filename;
+
        /*
         *      We don't do duplicate detection for "detail" sockets.
         */
@@ -1011,19 +1165,19 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
                return -1;
        }
 
-       if ((data->load_factor < 1) || (data->load_factor > 100)) {
-               cf_log_err_cs(cs, "Load factor must be between 1 and 100");
-               return -1;
-       }
+       FR_INTEGER_BOUND_CHECK("load_factor", data->load_factor, >=, 1);
+       FR_INTEGER_BOUND_CHECK("load_factor", data->load_factor, <=, 100);
 
-       if ((data->poll_interval < 1) || (data->poll_interval > 20)) {
-               cf_log_err_cs(cs, "poll_interval must be between 1 and 20");
-               return -1;
-       }
+       FR_INTEGER_BOUND_CHECK("poll_interval", data->poll_interval, >=, 1);
+       FR_INTEGER_BOUND_CHECK("poll_interval", data->poll_interval, <=, 60);
 
-       if (check_config) return 0;
+       FR_INTEGER_BOUND_CHECK("retry_interval", data->retry_interval, >=, 4);
+       FR_INTEGER_BOUND_CHECK("retry_interval", data->retry_interval, <=, 3600);
 
-       if (data->max_outstanding == 0) data->max_outstanding = 1;
+       /*
+        *      Only checking the config.  Don't start threads or anything else.
+        */
+       if (check_config) return 0;
 
        /*
         *      If the filename is a glob, use "detail.work" as the
@@ -1034,8 +1188,8 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
                char *p;
 
 #ifndef HAVE_GLOB_H
-               WARN("Detail file \"%s\" appears to use file globbing, but it is not supported on this system.",
-                    data->filename);
+               WARN("detail (%s): File \"%s\" appears to use file globbing, but it is not supported on this system",
+                    data->name, data->filename);
 #endif
                strlcpy(buffer, data->filename, sizeof(buffer));
                p = strrchr(buffer, FR_DIR_SEP);
@@ -1044,6 +1198,16 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
                } else {
                        buffer[0] = '\0';
                }
+
+               /*
+                *      Globbing cannot be done across directories.
+                */
+               if ((strchr(buffer, '*') != NULL) ||
+                   (strchr(buffer, '[') != NULL)) {
+                       cf_log_err_cs(cs, "Wildcard directories are not supported");
+                       return -1;
+               }
+
                strlcat(buffer, "detail.work",
                        sizeof(buffer) - strlen(buffer));
 
@@ -1067,7 +1231,7 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
        memset(client, 0, sizeof(*client));
        client->ipaddr.af = AF_INET;
        client->ipaddr.ipaddr.ip4addr.s_addr = INADDR_NONE;
-       client->prefix = 0;
+       client->ipaddr.prefix = 0;
        client->longname = client->shortname = data->filename;
        client->secret = client->shortname;
        client->nas_type = talloc_strdup(data, "none"); /* Part of 'data' not dynamically allocated */
@@ -1077,14 +1241,12 @@ int detail_parse(CONF_SECTION *cs, rad_listen_t *this)
         *      Create the communication pipes.
         */
        if (pipe(data->master_pipe) < 0) {
-               ERROR("radiusd: Error opening internal pipe: %s",
-                     fr_syserror(errno));
+               ERROR("detail (%s): Error opening internal pipe: %s", data->name, fr_syserror(errno));
                fr_exit(1);
        }
 
        if (pipe(data->child_pipe) < 0) {
-               ERROR("radiusd: Error opening internal pipe: %s",
-                     fr_syserror(errno));
+               ERROR("detail (%s): Error opening internal pipe: %s", data->name, fr_syserror(errno));
                fr_exit(1);
        }