implement callback for "no reply to sent packet"

along with various other cleanups to make it work better.

diff --git a/src/bin/radclient-ng.c b/src/bin/radclient-ng.c
index 47406b0a8651a7ec9b458a64728c81bd03541849..06e531d3a79251f769bd9d9e1e0ac15692e79a54 100644 (file)
--- a/src/bin/radclient-ng.c
+++ b/src/bin/radclient-ng.c
@@ -929,20 +929,54 @@ static int8_t request_cmp(void const *one, void const *two)
        return fr_value_box_cmp(&vp1->data, &vp2->data);
 }
 
+static void cleanup(fr_bio_packet_t *client, rc_request_t *request)
+{
+       /*
+        *      Don't leave a dangling pointer around.
+        */
+       if (current == request) {
+               current = fr_dlist_prev(&rc_request_list, current);
+       }
+
+       talloc_free(request);
+
+       /*
+        *      There are more packets to send, then allow the writer to send them.
+        */
+       if (fr_dlist_num_elements(&rc_request_list) != 0) {
+               return;
+       }
+
+       /*
+        *      We're done all packets, and there's nothing more to read, stop.
+        *
+        *      @todo - find a way to make this zero!
+        */
+       if (fr_radius_client_bio_outstanding(client) <= 1) {
+               fr_assert(client_config.retry_cfg.el != NULL);
+
+               fr_event_loop_exit(client_config.retry_cfg.el, 1);
+       }
+}
+
 static void client_retry_log(UNUSED fr_bio_packet_t *client, fr_packet_t *packet)
 {
        rc_request_t *request = packet->uctx;
 
        DEBUG("Timeout - resending packet");
 
-       // @todo - log the actual / updated Acct-Delay-Time?
+       // @todo - log the updated Acct-Delay-Time from the packet?
 
        fr_radius_packet_log(&default_log, request->packet, &request->request_pairs, false);
 }
 
-static void client_release(UNUSED fr_bio_packet_t *client, UNUSED fr_packet_t *packet)
+static void client_release(fr_bio_packet_t *client, fr_packet_t *packet)
 {
-       fr_assert(0);
+       rc_request_t *request = packet->uctx;
+
+       ERROR("No reply to packet");
+
+       cleanup(client, request);
 }
 
 
@@ -1171,28 +1205,7 @@ static void client_read(fr_event_list_t *el, int fd, UNUSED int flags, void *uct
 
        fr_packet_free(&reply);
 
-       /*
-        *      Don't leave a dangling pointer around.
-        */
-       if (current == request) {
-               current = fr_dlist_prev(&rc_request_list, current);
-       }
-
-       talloc_free(request);
-
-       /*
-        *      There are more packets to send, then allow the writer to send them.
-        */
-       if (fr_dlist_num_elements(&rc_request_list) != 0) {
-               return;
-       }
-
-       /*
-        *      We're done all packets, and there's nothing more to read, stop.
-        */
-       if (!fr_radius_client_bio_outstanding(client)) {
-               fr_event_loop_exit(el, 1);
-       }
+       cleanup(client, request);
 }
 
 static void client_write(fr_event_list_t *el, int fd, UNUSED int flags, void *uctx)

diff --git a/src/lib/bio/retry.c b/src/lib/bio/retry.c
index 72a1a84f300ecfefbd1333a057d01069d1e4fdf8..fa9951bd21f6669dd2c10c318d1078fe2a48fffc 100644 (file)
--- a/src/lib/bio/retry.c
+++ b/src/lib/bio/retry.c
@@ -149,6 +149,15 @@ static int fr_bio_retry_reset_timer(fr_bio_retry_t *my)
  */
 static void fr_bio_retry_release(fr_bio_retry_t *my, fr_bio_retry_entry_t *item, fr_bio_retry_release_reason_t reason)
 {
+       /*
+        *      Remove the item before calling the application "release" function.
+        */
+       if (my->partial != item) {
+               (void) fr_rb_remove_by_inline_node(&my->rb, &item->node);
+       } else {
+               item->cancelled = true;
+       }
+
        /*
         *      Tell the caller that we've released it before doing anything else.  That way we can safely
         *      modify anything we want.
@@ -159,12 +168,7 @@ static void fr_bio_retry_release(fr_bio_retry_t *my, fr_bio_retry_entry_t *item,
         *      We've partially written this item.  Don't bother changing it's position in any of the lists,
         *      as it's in progress.
         */
-       if (my->partial == item) {
-               item->cancelled = true;
-               return;
-       }
-
-       (void) fr_rb_remove_by_inline_node(&my->rb, &item->node);
+       if (my->partial == item) return;
 
        /*
         *      We're deleting the timer entry.  Go reset the timer.
@@ -759,7 +763,7 @@ static ssize_t fr_bio_retry_read(fr_bio_t *bio, void *packet_ctx, void *buffer,
         *
         *      @todo - don't include application watchdog packets in the idle count?
         */
-       if (!my->partial && (fr_rb_num_elements(&my->rb) == 1)) my->info.last_idle = my->info.last_reply;
+       if (fr_bio_retry_outstanding((fr_bio_t *) my) == 1) my->info.last_idle = my->info.last_reply;
 
        /*
         *      We have a new reply.  If we've received all of the replies (i.e. one), OR we don't have a
@@ -962,3 +966,18 @@ fr_bio_retry_info_t const *fr_bio_retry_info(fr_bio_t *bio)
 
        return &my->info;
 }
+
+size_t fr_bio_retry_outstanding(fr_bio_t *bio)
+{
+       fr_bio_retry_t *my = talloc_get_type_abort(bio, fr_bio_retry_t);
+       size_t num;
+
+       num = fr_rb_num_elements(&my->rb);
+
+       if (!my->partial) return num;
+
+       /*
+        *      Only count partially written items if they haven't been cancelled.
+        */
+       return num + !my->partial->cancelled;
+}

diff --git a/src/lib/bio/retry.h b/src/lib/bio/retry.h
index ebb8c9eda4c64f5029b5306e274af07aca009525..c0b633173519b67f3817133bcb76538a4ff696f7 100644 (file)
--- a/src/lib/bio/retry.h
+++ b/src/lib/bio/retry.h
@@ -149,4 +149,6 @@ ssize_t             fr_bio_retry_rewrite(fr_bio_t *bio, fr_bio_retry_entry_t *retry_ctx, co
 
 fr_bio_retry_info_t const *fr_bio_retry_info(fr_bio_t *bio) CC_HINT(nonnull);
 
+size_t         fr_bio_retry_outstanding(fr_bio_t *bio) CC_HINT(nonnull);
+
 #undef _CONST

diff --git a/src/protocols/radius/client.c b/src/protocols/radius/client.c
index 3fcc934d102dca4684fb46238e8db30ee527c2ba..f3d52b3b7910948efdca7796c40a9fff45bb2cbe 100644 (file)
--- a/src/protocols/radius/client.c
+++ b/src/protocols/radius/client.c
@@ -249,8 +249,6 @@ int fr_radius_client_fd_bio_write(fr_radius_client_fd_bio_t *my, void *request_c
         */
        fr_assert((size_t) slen == packet->data_len);
 
-       my->info.outstanding++;
-
        return 0;
 }
 
@@ -441,10 +439,6 @@ static bool radius_client_retry_response(fr_bio_t *bio, fr_bio_retry_entry_t **r
 
                *retry_ctx_p = id_ctx->retry_ctx;
                response->uctx = id_ctx->packet;
-
-               fr_assert(my->info.outstanding > 0);
-               my->info.outstanding--;
-
                return true;
        }
 
@@ -465,16 +459,10 @@ static void radius_client_retry_release(fr_bio_t *bio, fr_bio_retry_entry_t *ret
 {
        fr_radius_client_fd_bio_t *my = talloc_get_type_abort(bio->uctx, fr_radius_client_fd_bio_t);
        fr_radius_id_ctx_t *id_ctx = retry_ctx->uctx;
+       fr_packet_t *packet = id_ctx->packet;
 
        fr_assert(id_ctx->packet == retry_ctx->packet_ctx);
 
-       fprintf(stderr, "RELEASE %p %d\n", my->cfg.packet_cb_cfg.release, reason);
-
-       /*
-        *      Tell the application that this packet did not see a reply/
-        */
-       if (my->cfg.packet_cb_cfg.release && (reason == FR_BIO_RETRY_NO_REPLY)) my->cfg.packet_cb_cfg.release(&my->common, id_ctx->packet);
-
        fr_radius_code_id_push(my->codes, id_ctx->packet);
 
        /*
@@ -491,6 +479,11 @@ static void radius_client_retry_release(fr_bio_t *bio, fr_bio_retry_entry_t *ret
        id_ctx->request_ctx = NULL;
        id_ctx->retry_ctx = NULL;
 
+       /*
+        *      Tell the application that this packet did not see a reply/
+        */
+       if (my->cfg.packet_cb_cfg.release && (reason == FR_BIO_RETRY_NO_REPLY)) my->cfg.packet_cb_cfg.release(&my->common, packet);
+
        /*
         *      IO was blocked due to IDs.  We now have a free ID, so perhaps we can resume writes.  But only
         *      if the IO handlers didn't mark us as "write blocked".
@@ -603,7 +596,7 @@ size_t fr_radius_client_bio_outstanding(fr_bio_packet_t *bio)
 {
        fr_radius_client_fd_bio_t *my = talloc_get_type_abort(bio, fr_radius_client_fd_bio_t);
 
-       return my->info.outstanding;
+       return fr_bio_retry_outstanding(my->retry);
 }
 
 fr_radius_client_bio_info_t const *fr_radius_client_bio_info(fr_bio_packet_t *bio)

diff --git a/src/protocols/radius/client.h b/src/protocols/radius/client.h
index 8c2f7e79f43604876ee1a1ea88123147e02294d2..77c2d9dbab6b8d0fbe3edebb3759c491adf182d4 100644 (file)
--- a/src/protocols/radius/client.h
+++ b/src/protocols/radius/client.h
@@ -51,8 +51,6 @@ typedef struct {
 typedef struct {
        bool                    connected;
 
-       size_t                  outstanding;
-
        fr_bio_fd_info_t const  *fd_info;
 
        fr_bio_retry_info_t const       *retry_info;