From: Alan T. DeKok Date: Thu, 23 May 2024 02:03:11 +0000 (-0400) Subject: implement callback for "no reply to sent packet" X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=9a4ef48ceac42359a16f79e563b8f60df1972ab7;p=thirdparty%2Ffreeradius-server.git 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 47406b0a865..06e531d3a79 100644 --- 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 72a1a84f300..fa9951bd21f 100644 --- 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 ebb8c9eda4c..c0b63317351 100644 --- 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 3fcc934d102..f3d52b3b791 100644 --- 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 8c2f7e79f43..77c2d9dbab6 100644 --- 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;