radius: implement 2 separate queues for authentication and accounting #284
Conversation
This commit separates the RADIUS request queue into two distinct queues: one for Authentication (RAD_SERV_AUTH) and one for Accounting (RAD_SERV_ACCT). Previously, a single tracked all requests, which led to Head-of-Line blocking where stalled accounting requests (e.g., due to DB load) could fill up the slots, preventing new authentication requests from being processed. This ensures that accounting issues do not impact service availability for new users. Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>
There was a problem hiding this comment.
Pull request overview
This PR separates the RADIUS request queue into two distinct queues—one for authentication and one for accounting—to prevent head-of-line blocking where stalled accounting requests could block new authentication requests.
Key Changes:
- Converted
req_cntandqueue_cntfrom single integers to 2-element arrays indexed by request type (AUTH=0, ACCT=1) - Modified queue selection logic to use
req->typeinstead ofreq->prio, effectively giving each service type its own concurrent request limit - Updated all access points to properly index the new arrays and adjusted display output to show separate auth/acct statistics
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| accel-pppd/radius/radius_p.h | Changed req_cnt and queue_cnt from scalar integers to 2-element arrays to track authentication and accounting requests separately |
| accel-pppd/radius/serv.c | Updated all queue management logic to index counters by request type, modified wakeup logic to only wake requests of the same type, and enhanced statistics display to show auth/acct counts separately |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| list_add_tail(&req->entry, &req->serv->req_queue[req->type]); | ||
| req->serv->queue_cnt[req->type]++; |
There was a problem hiding this comment.
The change from using req->prio to req->type for queue selection eliminates the priority-based queuing functionality that existed before. Previously, req_queue[0] held priority-0 requests and req_queue[1] held priority-1 requests (e.g., accounting stop requests had prio=1 as seen in acct.c line 484). Now all authentication requests go to queue[0] and all accounting requests go to queue[1], regardless of their priority value. This means priority-1 accounting stop requests will no longer be prioritized over regular accounting requests. Consider whether this loss of per-type priority handling is acceptable, or if a more sophisticated queue structure is needed.
| list_add_tail(&req->entry, &req->serv->req_queue[req->type]); | |
| req->serv->queue_cnt[req->type]++; | |
| list_add_tail(&req->entry, &req->serv->req_queue[req->prio]); | |
| req->serv->queue_cnt[req->prio]++; |
This commit separates the RADIUS request queue into two distinct queues: one for Authentication (RAD_SERV_AUTH) and one for Accounting (RAD_SERV_ACCT).
Previously, a single tracked all requests, which led to Head-of-Line blocking where stalled accounting requests (e.g., due to DB load) could fill up the slots, preventing new authentication requests from being processed.
This ensures that accounting issues do not impact service availability for new users.