From d28a24252f0df8c8babf3a7321f303d3298029fc Mon Sep 17 00:00:00 2001 From: Denys Fedoryshchenko Date: Wed, 29 Apr 2026 13:12:31 +0300 Subject: pppoe: encapsulate statistics counters Group the PPPoE statistics in struct pppoe_stat_t and keep the storage private to pppoe.c instead of exporting writable counter globals through pppoe.h. The CLI now reads a snapshot with pppoe_stat_get(), while the packet/control paths update the counters through the PPPoE-owned storage using relaxed atomic operations. Convert the PPPoE SNMP starting/active scalars from watched raw pointers to scalar handlers. This removes the old pppoe_get_stat() pointer escape hatch and makes SNMP read the counters through pppoe_stat_starting() and pppoe_stat_active(), so the synchronization policy is applied consistently outside the PPPoE module. This also fixes the long-standing PPPoE starting counter behavior. PPPoE used to expose starting in the CLI and ACCEL-PPP-MIB, but never updated it, so it always reported zero. Track a per-connection ppp_starting state, increment starting when the controller begins channel setup, move the session from starting to active after establish_ppp() succeeds, and decrement starting on setup failure before PPP becomes active. This matches the state accounting used by the other PPP controllers. Signed-off-by: Denys Fedoryshchenko --- accel-pppd/ctrl/pppoe/cli.c | 22 ++++++++----- accel-pppd/ctrl/pppoe/disc.c | 2 +- accel-pppd/ctrl/pppoe/dpado.c | 3 +- accel-pppd/ctrl/pppoe/pppoe.c | 77 ++++++++++++++++++++++++++++--------------- accel-pppd/ctrl/pppoe/pppoe.h | 29 +++++++++------- 5 files changed, 84 insertions(+), 49 deletions(-) (limited to 'accel-pppd/ctrl') diff --git a/accel-pppd/ctrl/pppoe/cli.c b/accel-pppd/ctrl/pppoe/cli.c index d8399543..453c0cf3 100644 --- a/accel-pppd/ctrl/pppoe/cli.c +++ b/accel-pppd/ctrl/pppoe/cli.c @@ -88,16 +88,20 @@ help: static int show_stat_exec(const char *cmd, char * const *fields, int fields_cnt, void *client) { + struct pppoe_stat_t stat; + + pppoe_stat_get(&stat); + cli_send(client, "pppoe:\r\n"); - cli_sendv(client, " starting: %u\r\n", stat_starting); - cli_sendv(client, " active: %u\r\n", stat_active); - cli_sendv(client, " delayed PADO: %u\r\n", stat_delayed_pado); - cli_sendv(client, " recv PADI: %lu\r\n", stat_PADI_recv); - cli_sendv(client, " drop PADI: %lu\r\n", stat_PADI_drop); - cli_sendv(client, " sent PADO: %lu\r\n", stat_PADO_sent); - cli_sendv(client, " recv PADR(dup): %lu(%lu)\r\n", stat_PADR_recv, stat_PADR_dup_recv); - cli_sendv(client, " sent PADS: %lu\r\n", stat_PADS_sent); - cli_sendv(client, " filtered: %lu\r\n", stat_filtered); + cli_sendv(client, " starting: %u\r\n", stat.starting); + cli_sendv(client, " active: %u\r\n", stat.active); + cli_sendv(client, " delayed PADO: %u\r\n", stat.delayed_PADO); + cli_sendv(client, " recv PADI: %lu\r\n", stat.PADI_recv); + cli_sendv(client, " drop PADI: %lu\r\n", stat.PADI_drop); + cli_sendv(client, " sent PADO: %lu\r\n", stat.PADO_sent); + cli_sendv(client, " recv PADR(dup): %lu(%lu)\r\n", stat.PADR_recv, stat.PADR_dup_recv); + cli_sendv(client, " sent PADS: %lu\r\n", stat.PADS_sent); + cli_sendv(client, " filtered: %lu\r\n", stat.filtered); return CLI_CMD_OK; } diff --git a/accel-pppd/ctrl/pppoe/disc.c b/accel-pppd/ctrl/pppoe/disc.c index 8a82e1d2..e673a97a 100644 --- a/accel-pppd/ctrl/pppoe/disc.c +++ b/accel-pppd/ctrl/pppoe/disc.c @@ -329,7 +329,7 @@ static int disc_read(struct triton_md_handler_t *h) } if (mac_filter_check(ethhdr->h_source)) { - __sync_add_and_fetch(&stat_filtered, 1); + pppoe_stat_add_filtered(); continue; } diff --git a/accel-pppd/ctrl/pppoe/dpado.c b/accel-pppd/ctrl/pppoe/dpado.c index 71faa130..351a43dc 100644 --- a/accel-pppd/ctrl/pppoe/dpado.c +++ b/accel-pppd/ctrl/pppoe/dpado.c @@ -77,6 +77,7 @@ int dpado_parse(const char *str) { char *str1 = _strdup(str); char *ptr1, *ptr2, *ptr3, *endptr; + unsigned int active = pppoe_stat_active(); LIST_HEAD(range_list); struct dpado_range_t *r; @@ -131,7 +132,7 @@ int dpado_parse(const char *str) dpado_range_prev = NULL; list_for_each_entry(r, &dpado_range_list, entry) { - if (!dpado_range_prev || stat_active >= r->conn_cnt) { + if (!dpado_range_prev || active >= r->conn_cnt) { dpado_range_prev = r; if (r->entry.next != &dpado_range_list) dpado_range_next = list_entry(r->entry.next, typeof(*r), entry); diff --git a/accel-pppd/ctrl/pppoe/pppoe.c b/accel-pppd/ctrl/pppoe/pppoe.c index 6ec07c9d..475ee530 100644 --- a/accel-pppd/ctrl/pppoe/pppoe.c +++ b/accel-pppd/ctrl/pppoe/pppoe.c @@ -43,6 +43,7 @@ struct pppoe_conn_t { struct pppoe_serv_t *serv; uint16_t sid; uint8_t addr[ETH_ALEN]; + unsigned int ppp_starting:1; unsigned int ppp_started:1; struct pppoe_tag *relay_sid; @@ -109,17 +110,8 @@ static mempool_t conn_pool; static mempool_t pado_pool; static mempool_t padi_pool; -unsigned int stat_starting; -unsigned int stat_active; -unsigned int stat_delayed_pado; -unsigned long stat_PADI_recv; -unsigned long stat_PADI_drop; -unsigned long stat_PADO_sent; -unsigned long stat_PADR_recv; -unsigned long stat_PADR_dup_recv; -unsigned long stat_PADS_sent; +static struct pppoe_stat_t pppoe_stat; unsigned int total_padi_cnt; -unsigned long stat_filtered; pthread_rwlock_t serv_lock = PTHREAD_RWLOCK_INITIALIZER; LIST_HEAD(serv_list); @@ -131,6 +123,35 @@ static unsigned long *sid_map; static unsigned long *sid_ptr; static int sid_idx; +void __export pppoe_stat_get(struct pppoe_stat_t *stat) +{ + stat->starting = __atomic_load_n(&pppoe_stat.starting, __ATOMIC_RELAXED); + stat->active = __atomic_load_n(&pppoe_stat.active, __ATOMIC_RELAXED); + stat->delayed_PADO = __atomic_load_n(&pppoe_stat.delayed_PADO, __ATOMIC_RELAXED); + stat->PADI_recv = __atomic_load_n(&pppoe_stat.PADI_recv, __ATOMIC_RELAXED); + stat->PADI_drop = __atomic_load_n(&pppoe_stat.PADI_drop, __ATOMIC_RELAXED); + stat->PADO_sent = __atomic_load_n(&pppoe_stat.PADO_sent, __ATOMIC_RELAXED); + stat->PADR_recv = __atomic_load_n(&pppoe_stat.PADR_recv, __ATOMIC_RELAXED); + stat->PADR_dup_recv = __atomic_load_n(&pppoe_stat.PADR_dup_recv, __ATOMIC_RELAXED); + stat->PADS_sent = __atomic_load_n(&pppoe_stat.PADS_sent, __ATOMIC_RELAXED); + stat->filtered = __atomic_load_n(&pppoe_stat.filtered, __ATOMIC_RELAXED); +} + +unsigned int __export pppoe_stat_starting(void) +{ + return __atomic_load_n(&pppoe_stat.starting, __ATOMIC_RELAXED); +} + +unsigned int __export pppoe_stat_active(void) +{ + return __atomic_load_n(&pppoe_stat.active, __ATOMIC_RELAXED); +} + +void __export pppoe_stat_add_filtered(void) +{ + __atomic_add_fetch(&pppoe_stat.filtered, 1, __ATOMIC_RELAXED); +} + static uint8_t bc_addr[ETH_ALEN] = {0xff, 0xff, 0xff, 0xff, 0xff, 0xff}; static void pppoe_send_PADT(struct pppoe_conn_t *conn); @@ -168,9 +189,12 @@ static void disconnect(struct pppoe_conn_t *conn) struct pppoe_serv_t *serv = conn->serv; if (conn->ppp_started) { - dpado_check_prev(__sync_fetch_and_sub(&stat_active, 1)); + dpado_check_prev(__atomic_fetch_sub(&pppoe_stat.active, 1, __ATOMIC_RELAXED)); conn->ppp_started = 0; ap_session_terminate(&conn->ppp.ses, TERM_USER_REQUEST, 1); + } else if (conn->ppp_starting) { + __atomic_sub_fetch(&pppoe_stat.starting, 1, __ATOMIC_RELAXED); + conn->ppp_starting = 0; } pppoe_send_PADT(conn); @@ -227,7 +251,7 @@ static void ppp_finished(struct ap_session *ses) log_ppp_debug("pppoe: ppp finished\n"); if (conn->ppp_started) { - dpado_check_prev(__sync_fetch_and_sub(&stat_active, 1)); + dpado_check_prev(__atomic_fetch_sub(&pppoe_stat.active, 1, __ATOMIC_RELAXED)); conn->ppp_started = 0; triton_context_call(&conn->ctx, (triton_event_func)disconnect, conn); } @@ -445,6 +469,9 @@ static void connect_channel(struct pppoe_conn_t *conn) struct sockaddr_pppox sp; triton_event_fire(EV_CTRL_STARTING, &conn->ppp.ses); + conn->ppp_starting = 1; + __atomic_add_fetch(&pppoe_stat.starting, 1, __ATOMIC_RELAXED); + triton_event_fire(EV_CTRL_STARTED, &conn->ppp.ses); sock = net->socket(AF_PPPOX, SOCK_DGRAM, PX_PROTO_OE); @@ -481,9 +508,11 @@ static void connect_channel(struct pppoe_conn_t *conn) } #endif + conn->ppp_starting = 0; conn->ppp_started = 1; - dpado_check_next(__sync_add_and_fetch(&stat_active, 1)); + __atomic_sub_fetch(&pppoe_stat.starting, 1, __ATOMIC_RELAXED); + dpado_check_next(__atomic_add_fetch(&pppoe_stat.active, 1, __ATOMIC_RELAXED)); return; @@ -817,7 +846,7 @@ static void pppoe_send_PADO(struct pppoe_serv_t *serv, const uint8_t *addr, cons if (conf_verbose) print_packet(serv->ifname, "send", pack); - __sync_add_and_fetch(&stat_PADO_sent, 1); + __atomic_add_fetch(&pppoe_stat.PADO_sent, 1, __ATOMIC_RELAXED); pppoe_send(serv, pack); } @@ -866,7 +895,7 @@ static void pppoe_send_PADS(struct pppoe_conn_t *conn) if (conf_verbose) print_packet(conn->serv->ifname, "send", pack); - __sync_add_and_fetch(&stat_PADS_sent, 1); + __atomic_add_fetch(&pppoe_stat.PADS_sent, 1, __ATOMIC_RELAXED); pppoe_send(conn->serv, pack); } @@ -893,7 +922,7 @@ static void free_delayed_pado(struct delayed_pado_t *pado) { triton_timer_del(&pado->timer); - __sync_sub_and_fetch(&stat_delayed_pado, 1); + __atomic_sub_fetch(&pppoe_stat.delayed_PADO, 1, __ATOMIC_RELAXED); list_del(&pado->entry); if (pado->host_uniq) @@ -979,7 +1008,7 @@ static void pppoe_recv_PADI(struct pppoe_serv_t *serv, uint8_t *pack, int size) struct timespec ts; uint16_t ppp_max_payload = 0; - __sync_add_and_fetch(&stat_PADI_recv, 1); + __atomic_add_fetch(&pppoe_stat.PADI_recv, 1, __ATOMIC_RELAXED); if (ap_shutdown || pado_delay == -1) return; @@ -991,7 +1020,7 @@ static void pppoe_recv_PADI(struct pppoe_serv_t *serv, uint8_t *pack, int size) return; if (check_padi_limit(serv, ethhdr->h_source)) { - __sync_add_and_fetch(&stat_PADI_drop, 1); + __atomic_add_fetch(&pppoe_stat.PADI_drop, 1, __ATOMIC_RELAXED); if (conf_verbose) { clock_gettime(CLOCK_MONOTONIC, &ts); if (ts.tv_sec - 60 >= serv->last_padi_limit_warn) { @@ -1094,7 +1123,7 @@ tags_done: triton_timer_add(&serv->ctx, &pado->timer, 0); list_add_tail(&pado->entry, &serv->pado_list); - __sync_add_and_fetch(&stat_delayed_pado, 1); + __atomic_add_fetch(&pppoe_stat.delayed_PADO, 1, __ATOMIC_RELAXED); } else pppoe_send_PADO(serv, ethhdr->h_source, host_uniq_tag, relay_sid_tag, service_name_tag, ppp_max_payload); } @@ -1114,7 +1143,7 @@ static void pppoe_recv_PADR(struct pppoe_serv_t *serv, uint8_t *pack, int size) int vendor_id; uint16_t ppp_max_payload = 0; - __sync_add_and_fetch(&stat_PADR_recv, 1); + __atomic_add_fetch(&pppoe_stat.PADR_recv, 1, __ATOMIC_RELAXED); if (ap_shutdown) return; @@ -1227,7 +1256,7 @@ padr_tags_done: pthread_mutex_lock(&serv->lock); conn = find_channel(serv, (uint8_t *)ac_cookie_tag->tag_data); if (conn && !conn->ppp.ses.username) { - __sync_add_and_fetch(&stat_PADR_dup_recv, 1); + __atomic_add_fetch(&pppoe_stat.PADR_dup_recv, 1, __ATOMIC_RELAXED); pppoe_send_PADS(conn); } pthread_mutex_unlock(&serv->lock); @@ -1643,12 +1672,6 @@ void pppoe_server_stop(const char *ifname) pthread_rwlock_unlock(&serv_lock); } -void __export pppoe_get_stat(unsigned int **starting, unsigned int **active) -{ - *starting = &stat_starting; - *active = &stat_active; -} - static int init_secret(struct pppoe_serv_t *serv) { DES_cblock key; diff --git a/accel-pppd/ctrl/pppoe/pppoe.h b/accel-pppd/ctrl/pppoe/pppoe.h index 2510c320..42067590 100644 --- a/accel-pppd/ctrl/pppoe/pppoe.h +++ b/accel-pppd/ctrl/pppoe/pppoe.h @@ -109,16 +109,24 @@ extern int conf_accept_any_service; extern char *conf_ac_name; extern char *conf_pado_delay; -extern unsigned int stat_starting; -extern unsigned int stat_active; -extern unsigned int stat_delayed_pado; -extern unsigned long stat_PADI_recv; -extern unsigned long stat_PADO_sent; -extern unsigned long stat_PADR_recv; -extern unsigned long stat_PADR_dup_recv; -extern unsigned long stat_PADS_sent; -extern unsigned long stat_PADI_drop; -extern unsigned long stat_filtered; +struct pppoe_stat_t +{ + unsigned int starting; + unsigned int active; + unsigned int delayed_PADO; + unsigned long PADI_recv; + unsigned long PADI_drop; + unsigned long PADO_sent; + unsigned long PADR_recv; + unsigned long PADR_dup_recv; + unsigned long PADS_sent; + unsigned long filtered; +}; + +void pppoe_stat_get(struct pppoe_stat_t *stat); +unsigned int pppoe_stat_starting(void); +unsigned int pppoe_stat_active(void); +void pppoe_stat_add_filtered(void); extern pthread_rwlock_t serv_lock; extern struct list_head serv_list; @@ -142,4 +150,3 @@ int tr101_send_access_request(struct pppoe_tag *tr101, struct rad_packet_t *pack int tr101_send_accounting_request(struct pppoe_tag *tr101, struct rad_packet_t *pack); #endif - -- cgit v1.2.3