session: encapsulate statistics counters

Group the core session starting, active, and finishing statistics behind the private ap_session_stat storage in session.c instead of exposing writable counters through ap_session.h. This keeps ownership inside the session core while preserving the existing CLI and ACCEL-PPP-MIB counter semantics.

Route session counter updates through ap_session_stat_*() helpers. Session start, activation, termination, finish, and shutdown-idle paths no longer open-code individual counter increments/decrements; the update policy now lives beside the session-owned storage and uses relaxed atomic operations for the simple state counters.

Make the CLI show-stat path render from a local snapshot and convert the PPP SNMP starting/active/finishing scalars from watched raw pointers to scalar handlers. PPP controllers now read max-session limits through ap_session_stat_starting() and ap_session_stat_active(), removing external direct access to ap_session_stat.

Signed-off-by: Denys Fedoryshchenko <denys.f@collabora.com>

9 files changed, 186 insertions, 82 deletions

diff --git a/accel-pppd/cli/std_cmd.c b/accel-pppd/cli/std_cmd.c
index c37777b1..9e7bf557 100644
--- a/accel-pppd/cli/std_cmd.c
+++ b/accel-pppd/cli/std_cmd.c
@@ -71,10 +71,16 @@ static int show_stat_exec(const char *cmd, char * const *fields, int fields_cnt,
 	cli_sendv(client, "  timer_pending: %u\r\n", triton_stat.timer_pending);
 
 //===========
-	cli_send(client, "sessions:\r\n");
-	cli_sendv(client, "  starting: %u\r\n", ap_session_stat.starting);
-	cli_sendv(client, "  active: %u\r\n", ap_session_stat.active);
-	cli_sendv(client, "  finishing: %u\r\n", ap_session_stat.finishing);
+	{
+		struct ap_session_stat ses_stat;
+
+		ap_session_stat_get(&ses_stat);
+
+		cli_send(client, "sessions:\r\n");
+		cli_sendv(client, "  starting: %u\r\n", ses_stat.starting);
+		cli_sendv(client, "  active: %u\r\n", ses_stat.active);
+		cli_sendv(client, "  finishing: %u\r\n", ses_stat.finishing);
+	}
 
 	return CLI_CMD_OK;
 }
diff --git a/accel-pppd/ctrl/ipoe/ipoe.c b/accel-pppd/ctrl/ipoe/ipoe.c
index 42d142fe..5592845f 100644
--- a/accel-pppd/ctrl/ipoe/ipoe.c
+++ b/accel-pppd/ctrl/ipoe/ipoe.c
@@ -1392,10 +1392,10 @@ static struct ipoe_session *ipoe_session_create_dhcpv4(struct ipoe_serv *serv, s
 	if (ap_shutdown)
 		return NULL;
 
-	if (conf_max_starting && ap_session_stat.starting >= conf_max_starting)
+	if (conf_max_starting && ap_session_stat_starting() >= conf_max_starting)
 		return NULL;
 
-	if (conf_max_sessions && ap_session_stat.active + ap_session_stat.starting >= conf_max_sessions)
+	if (conf_max_sessions && ap_session_stat_active() + ap_session_stat_starting() >= conf_max_sessions)
 		return NULL;
 
 	ses = ipoe_session_alloc(serv->ifname);
@@ -2137,10 +2137,10 @@ static struct ipoe_session *ipoe_session_create_up(struct ipoe_serv *serv, struc
 	if (ap_shutdown)
 		return NULL;
 
-	if (conf_max_starting && ap_session_stat.starting >= conf_max_starting)
+	if (conf_max_starting && ap_session_stat_starting() >= conf_max_starting)
 		return NULL;
 
-	if (conf_max_sessions && ap_session_stat.active + ap_session_stat.starting >= conf_max_sessions)
+	if (conf_max_sessions && ap_session_stat_active() + ap_session_stat_starting() >= conf_max_sessions)
 		return NULL;
 
 	if (connlimit_loaded && connlimit_check(serv->opt_shared ? cl_key_from_ipv4(saddr) : serv->ifindex))
diff --git a/accel-pppd/ctrl/l2tp/l2tp.c b/accel-pppd/ctrl/l2tp/l2tp.c
index 91b49d79..cf0c502c 100644
--- a/accel-pppd/ctrl/l2tp/l2tp.c
+++ b/accel-pppd/ctrl/l2tp/l2tp.c
@@ -2836,10 +2836,10 @@ static int l2tp_recv_SCCRQ(const struct l2tp_serv_t *serv,
 		return 0;
 	}
 
-	if (conf_max_starting && ap_session_stat.starting >= conf_max_starting)
+	if (conf_max_starting && ap_session_stat_starting() >= conf_max_starting)
 		return 0;
 
-	if (conf_max_sessions && ap_session_stat.active + ap_session_stat.starting >= conf_max_sessions)
+	if (conf_max_sessions && ap_session_stat_active() + ap_session_stat_starting() >= conf_max_sessions)
 		return 0;
 
 	if (triton_module_loaded("connlimit")
@@ -3382,10 +3382,10 @@ static int l2tp_recv_ICRQ(struct l2tp_conn_t *conn,
 		return 0;
 	}
 
-	if (conf_max_starting && ap_session_stat.starting >= conf_max_starting)
+	if (conf_max_starting && ap_session_stat_starting() >= conf_max_starting)
 		return 0;
 
-	if (conf_max_sessions && ap_session_stat.active + ap_session_stat.starting >= conf_max_sessions)
+	if (conf_max_sessions && ap_session_stat_active() + ap_session_stat_starting() >= conf_max_sessions)
 		return 0;
 
 	if (triton_module_loaded("connlimit")
@@ -3722,10 +3722,10 @@ static int l2tp_recv_OCRQ(struct l2tp_conn_t *conn,
 		return 0;
 	}
 
-	if (conf_max_starting && ap_session_stat.starting >= conf_max_starting)
+	if (conf_max_starting && ap_session_stat_starting() >= conf_max_starting)
 		return 0;
 
-	if (conf_max_sessions && ap_session_stat.active + ap_session_stat.starting >= conf_max_sessions)
+	if (conf_max_sessions && ap_session_stat_active() + ap_session_stat_starting() >= conf_max_sessions)
 		return 0;
 
 	if (triton_module_loaded("connlimit")
diff --git a/accel-pppd/ctrl/pppoe/pppoe.c b/accel-pppd/ctrl/pppoe/pppoe.c
index 475ee530..93824af5 100644
--- a/accel-pppd/ctrl/pppoe/pppoe.c
+++ b/accel-pppd/ctrl/pppoe/pppoe.c
@@ -1013,10 +1013,10 @@ static void pppoe_recv_PADI(struct pppoe_serv_t *serv, uint8_t *pack, int size)
 	if (ap_shutdown || pado_delay == -1)
 		return;
 
-	if (conf_max_starting && ap_session_stat.starting >= conf_max_starting)
+	if (conf_max_starting && ap_session_stat_starting() >= conf_max_starting)
 		return;
 
-	if (conf_max_sessions && ap_session_stat.active + ap_session_stat.starting >= conf_max_sessions)
+	if (conf_max_sessions && ap_session_stat_active() + ap_session_stat_starting() >= conf_max_sessions)
 		return;
 
 	if (check_padi_limit(serv, ethhdr->h_source)) {
@@ -1148,10 +1148,10 @@ static void pppoe_recv_PADR(struct pppoe_serv_t *serv, uint8_t *pack, int size)
 	if (ap_shutdown)
 		return;
 
-	if (conf_max_starting && ap_session_stat.starting >= conf_max_starting)
+	if (conf_max_starting && ap_session_stat_starting() >= conf_max_starting)
 		return;
 
-	if (conf_max_sessions && ap_session_stat.active + ap_session_stat.starting >= conf_max_sessions)
+	if (conf_max_sessions && ap_session_stat_active() + ap_session_stat_starting() >= conf_max_sessions)
 		return;
 
 	if (!memcmp(ethhdr->h_dest, bc_addr, ETH_ALEN)) {
diff --git a/accel-pppd/ctrl/pptp/pptp.c b/accel-pppd/ctrl/pptp/pptp.c
index efd2e592..f8e498d6 100644
--- a/accel-pppd/ctrl/pptp/pptp.c
+++ b/accel-pppd/ctrl/pptp/pptp.c
@@ -699,12 +699,12 @@ static int pptp_connect(struct triton_md_handler_t *h)
 			continue;
 		}
 
-		if (conf_max_starting && ap_session_stat.starting >= conf_max_starting) {
+		if (conf_max_starting && ap_session_stat_starting() >= conf_max_starting) {
 			close(sock);
 			continue;
 		}
 
-		if (conf_max_sessions && ap_session_stat.active + ap_session_stat.starting >= conf_max_sessions) {
+		if (conf_max_sessions && ap_session_stat_active() + ap_session_stat_starting() >= conf_max_sessions) {
 			close(sock);
 			continue;
 		}
diff --git a/accel-pppd/ctrl/sstp/sstp.c b/accel-pppd/ctrl/sstp/sstp.c
index 71fb17e7..21c519fa 100644
--- a/accel-pppd/ctrl/sstp/sstp.c
+++ b/accel-pppd/ctrl/sstp/sstp.c
@@ -2330,12 +2330,12 @@ static int sstp_connect(struct triton_md_handler_t *h)
 			continue;
 		}
 
-		if (conf_max_starting && ap_session_stat.starting >= conf_max_starting) {
+		if (conf_max_starting && ap_session_stat_starting() >= conf_max_starting) {
 			close(sock);
 			continue;
 		}
 
-		if (conf_max_sessions && ap_session_stat.active + ap_session_stat.starting >= conf_max_sessions) {
+		if (conf_max_sessions && ap_session_stat_active() + ap_session_stat_starting() >= conf_max_sessions) {
 			close(sock);
 			continue;
 		}
diff --git a/accel-pppd/extra/net-snmp/statPPP.c b/accel-pppd/extra/net-snmp/statPPP.c
index 37e4630c..7dc1f659 100644
--- a/accel-pppd/extra/net-snmp/statPPP.c
+++ b/accel-pppd/extra/net-snmp/statPPP.c
@@ -6,9 +6,22 @@
 #include <net-snmp/net-snmp-config.h>
 #include <net-snmp/net-snmp-includes.h>
 #include <net-snmp/agent/net-snmp-agent-includes.h>
+
+#include "ap_session.h"
 #include "statPPP.h"
 
-#include "ppp.h"
+static int handle_statPPPStarting(netsnmp_mib_handler *handler,
+				  netsnmp_handler_registration *reginfo,
+				  netsnmp_agent_request_info *reqinfo,
+				  netsnmp_request_info *requests);
+static int handle_statPPPActive(netsnmp_mib_handler *handler,
+				netsnmp_handler_registration *reginfo,
+				netsnmp_agent_request_info *reqinfo,
+				netsnmp_request_info *requests);
+static int handle_statPPPFinishing(netsnmp_mib_handler *handler,
+				   netsnmp_handler_registration *reginfo,
+				   netsnmp_agent_request_info *reqinfo,
+				   netsnmp_request_info *requests);
 
 /*
  * Our initialization routine, called automatically by the agent
@@ -17,77 +30,115 @@
 void
 init_statPPP(void)
 {
-  netsnmp_handler_registration *reg;
-  netsnmp_watcher_info         *winfo;
-
     static oid statPPPStarting_oid[] = { 1,3,6,1,4,1,8072,100,1,2,1 };
     static oid statPPPActive_oid[] = { 1,3,6,1,4,1,8072,100,1,2,2 };
     static oid statPPPFinishing_oid[] = { 1,3,6,1,4,1,8072,100,1,2,3 };
 
-  /*
-   * a debugging statement.  Run the agent with -DstatPPP to see
-   * the output of this debugging statement.
-   */
   DEBUGMSGTL(("statPPP", "Initializing the statPPP module\n"));
 
-
-    /*
-     * Register scalar watchers for each of the MIB objects.
-     * The ASN type and RO/RW status are taken from the MIB definition,
-     * but can be adjusted if needed.
-     *
-     * In most circumstances, the scalar watcher will handle all
-     * of the necessary processing.  But the NULL parameter in the
-     * netsnmp_create_handler_registration() call can be used to
-     * supply a user-provided handler if necessary.
-     *
-     * This approach can also be used to handle Counter64, string-
-     * and OID-based watched scalars (although variable-sized writeable
-     * objects will need some more specialised initialisation).
-     */
     DEBUGMSGTL(("statPPP",
                 "Initializing statPPPStarting scalar integer.  Default value = %d\n",
                 0));
-    reg = netsnmp_create_handler_registration(
-             "statPPPStarting", NULL,
+    if (netsnmp_register_scalar(netsnmp_create_handler_registration(
+             "statPPPStarting", handle_statPPPStarting,
               statPPPStarting_oid, OID_LENGTH(statPPPStarting_oid),
-              HANDLER_CAN_RONLY);
-    winfo = netsnmp_create_watcher_info(
-                &ap_session_stat.starting, sizeof(ap_session_stat.starting),
-                 ASN_INTEGER, WATCHER_FIXED_SIZE);
-    if (netsnmp_register_watched_scalar( reg, winfo ) < 0 ) {
-        snmp_log( LOG_ERR, "Failed to register watched statPPPStarting" );
+              HANDLER_CAN_RONLY)) < 0 ) {
+        snmp_log( LOG_ERR, "Failed to register statPPPStarting" );
     }
 
     DEBUGMSGTL(("statPPP",
                 "Initializing statPPPActive scalar integer.  Default value = %d\n",
                 0));
-    reg = netsnmp_create_handler_registration(
-             "statPPPActive", NULL,
+    if (netsnmp_register_scalar(netsnmp_create_handler_registration(
+             "statPPPActive", handle_statPPPActive,
               statPPPActive_oid, OID_LENGTH(statPPPActive_oid),
-              HANDLER_CAN_RONLY);
-    winfo = netsnmp_create_watcher_info(
-                &ap_session_stat.active, sizeof(ap_session_stat.active),
-                 ASN_INTEGER, WATCHER_FIXED_SIZE);
-    if (netsnmp_register_watched_scalar( reg, winfo ) < 0 ) {
-        snmp_log( LOG_ERR, "Failed to register watched statPPPActive" );
+              HANDLER_CAN_RONLY)) < 0 ) {
+        snmp_log( LOG_ERR, "Failed to register statPPPActive" );
     }
 
     DEBUGMSGTL(("statPPP",
                 "Initializing statPPPFinishing scalar integer.  Default value = %d\n",
                 0));
-    reg = netsnmp_create_handler_registration(
-             "statPPPFinishing", NULL,
+    if (netsnmp_register_scalar(netsnmp_create_handler_registration(
+             "statPPPFinishing", handle_statPPPFinishing,
               statPPPFinishing_oid, OID_LENGTH(statPPPFinishing_oid),
-              HANDLER_CAN_RONLY);
-    winfo = netsnmp_create_watcher_info(
-                &ap_session_stat.finishing, sizeof(ap_session_stat.finishing),
-                 ASN_INTEGER, WATCHER_FIXED_SIZE);
-    if (netsnmp_register_watched_scalar( reg, winfo ) < 0 ) {
-        snmp_log( LOG_ERR, "Failed to register watched statPPPFinishing" );
+              HANDLER_CAN_RONLY)) < 0 ) {
+        snmp_log( LOG_ERR, "Failed to register statPPPFinishing" );
     }
 
 
   DEBUGMSGTL(("statPPP",
               "Done initalizing statPPP module\n"));
 }
+
+static int handle_statPPPStarting(netsnmp_mib_handler *handler,
+				  netsnmp_handler_registration *reginfo,
+				  netsnmp_agent_request_info *reqinfo,
+				  netsnmp_request_info *requests)
+{
+	long stat;
+
+	(void)handler;
+	(void)reginfo;
+
+	switch (reqinfo->mode) {
+	case MODE_GET:
+		stat = ap_session_stat_starting();
+		snmp_set_var_typed_value(requests->requestvb, ASN_INTEGER,
+					 (u_char *)&stat, sizeof(stat));
+		break;
+	default:
+		snmp_log(LOG_ERR, "unknown mode (%d) in handle_statPPPStarting\n", reqinfo->mode);
+		return SNMP_ERR_GENERR;
+	}
+
+	return SNMP_ERR_NOERROR;
+}
+
+static int handle_statPPPActive(netsnmp_mib_handler *handler,
+				netsnmp_handler_registration *reginfo,
+				netsnmp_agent_request_info *reqinfo,
+				netsnmp_request_info *requests)
+{
+	long stat;
+
+	(void)handler;
+	(void)reginfo;
+
+	switch (reqinfo->mode) {
+	case MODE_GET:
+		stat = ap_session_stat_active();
+		snmp_set_var_typed_value(requests->requestvb, ASN_INTEGER,
+					 (u_char *)&stat, sizeof(stat));
+		break;
+	default:
+		snmp_log(LOG_ERR, "unknown mode (%d) in handle_statPPPActive\n", reqinfo->mode);
+		return SNMP_ERR_GENERR;
+	}
+
+	return SNMP_ERR_NOERROR;
+}
+
+static int handle_statPPPFinishing(netsnmp_mib_handler *handler,
+				   netsnmp_handler_registration *reginfo,
+				   netsnmp_agent_request_info *reqinfo,
+				   netsnmp_request_info *requests)
+{
+	long stat;
+
+	(void)handler;
+	(void)reginfo;
+
+	switch (reqinfo->mode) {
+	case MODE_GET:
+		stat = ap_session_stat_finishing();
+		snmp_set_var_typed_value(requests->requestvb, ASN_INTEGER,
+					 (u_char *)&stat, sizeof(stat));
+		break;
+	default:
+		snmp_log(LOG_ERR, "unknown mode (%d) in handle_statPPPFinishing\n", reqinfo->mode);
+		return SNMP_ERR_GENERR;
+	}
+
+	return SNMP_ERR_NOERROR;
+}
diff --git a/accel-pppd/include/ap_session.h b/accel-pppd/include/ap_session.h
index 27471859..a4d3b867 100644
--- a/accel-pppd/include/ap_session.h
+++ b/accel-pppd/include/ap_session.h
@@ -136,10 +136,14 @@ extern int ap_shutdown;
 extern int sock_fd;
 extern int sock6_fd;
 extern int urandom_fd;
-extern struct ap_session_stat ap_session_stat;
 extern int conf_max_sessions;
 extern int conf_max_starting;
 
+void ap_session_stat_get(struct ap_session_stat *stat);
+unsigned int ap_session_stat_starting(void);
+unsigned int ap_session_stat_active(void);
+unsigned int ap_session_stat_finishing(void);
+
 void ap_session_init(struct ap_session *ses);
 void ap_session_set_ifindex(struct ap_session *ses);
 int ap_session_starting(struct ap_session *ses);
diff --git a/accel-pppd/session.c b/accel-pppd/session.c
index a1b194a3..c16f68ae 100644
--- a/accel-pppd/session.c
+++ b/accel-pppd/session.c
@@ -62,13 +62,58 @@ static spinlock_t seq_lock;
 static long long unsigned seq;
 static struct timespec seq_ts;
 
-struct ap_session_stat __export ap_session_stat;
+static struct ap_session_stat ap_session_stat;
 
 static void (*shutdown_cb)(void);
 
 static void generate_sessionid(struct ap_session *ses);
 static void save_seq(void);
 
+void __export ap_session_stat_get(struct ap_session_stat *stat)
+{
+	stat->starting = __atomic_load_n(&ap_session_stat.starting, __ATOMIC_RELAXED);
+	stat->active = __atomic_load_n(&ap_session_stat.active, __ATOMIC_RELAXED);
+	stat->finishing = __atomic_load_n(&ap_session_stat.finishing, __ATOMIC_RELAXED);
+}
+
+unsigned int __export ap_session_stat_starting(void)
+{
+	return __atomic_load_n(&ap_session_stat.starting, __ATOMIC_RELAXED);
+}
+
+unsigned int __export ap_session_stat_active(void)
+{
+	return __atomic_load_n(&ap_session_stat.active, __ATOMIC_RELAXED);
+}
+
+unsigned int __export ap_session_stat_finishing(void)
+{
+	return __atomic_load_n(&ap_session_stat.finishing, __ATOMIC_RELAXED);
+}
+
+static void ap_session_stat_inc(unsigned int *stat)
+{
+	__atomic_add_fetch(stat, 1, __ATOMIC_RELAXED);
+}
+
+static void ap_session_stat_dec(unsigned int *stat)
+{
+	__atomic_sub_fetch(stat, 1, __ATOMIC_RELAXED);
+}
+
+static void ap_session_stat_move(unsigned int *from, unsigned int *to)
+{
+	ap_session_stat_dec(from);
+	ap_session_stat_inc(to);
+}
+
+static int ap_session_stat_idle(void)
+{
+	return !ap_session_stat_starting()
+	    && !ap_session_stat_active()
+	    && !ap_session_stat_finishing();
+}
+
 void __export ap_session_init(struct ap_session *ses)
 {
 	memset(ses, 0, sizeof(*ses));
@@ -113,7 +158,7 @@ int __export ap_session_starting(struct ap_session *ses)
 		ses->state = AP_STATE_STARTING;
 	}
 
-	__sync_add_and_fetch(&ap_session_stat.starting, 1);
+	ap_session_stat_inc(&ap_session_stat.starting);
 
 	pthread_rwlock_wrlock(&ses_lock);
 	list_add_tail(&ses->entry, &ses_list);
@@ -156,8 +201,7 @@ void __export ap_session_activate(struct ap_session *ses)
 		return;
 
 	ses->state = AP_STATE_ACTIVE;
-	__sync_sub_and_fetch(&ap_session_stat.starting, 1);
-	__sync_add_and_fetch(&ap_session_stat.active, 1);
+	ap_session_stat_move(&ap_session_stat.starting, &ap_session_stat.active);
 
 	if (!ses->session_timeout && conf_session_timeout)
 		ses->session_timeout = conf_session_timeout;
@@ -198,14 +242,14 @@ void __export ap_session_finished(struct ap_session *ses)
 
 	switch (ses->state) {
 		case AP_STATE_ACTIVE:
-			__sync_sub_and_fetch(&ap_session_stat.active, 1);
+			ap_session_stat_dec(&ap_session_stat.active);
 			break;
 		case AP_STATE_RESTORE:
 		case AP_STATE_STARTING:
-			__sync_sub_and_fetch(&ap_session_stat.starting, 1);
+			ap_session_stat_dec(&ap_session_stat.starting);
 			break;
 		case AP_STATE_FINISHING:
-			__sync_sub_and_fetch(&ap_session_stat.finishing, 1);
+			ap_session_stat_dec(&ap_session_stat.finishing);
 			break;
 	}
 
@@ -266,7 +310,7 @@ void __export ap_session_finished(struct ap_session *ses)
 		ses->backup->storage->free(ses->backup);
 #endif
 
-	if (ap_shutdown && !ap_session_stat.starting && !ap_session_stat.active && !ap_session_stat.finishing) {
+	if (ap_shutdown && ap_session_stat_idle()) {
 		if (shutdown_cb)
 			shutdown_cb();
 		else
@@ -299,11 +343,10 @@ void __export ap_session_terminate(struct ap_session *ses, int cause, int hard)
 	}
 
 	if (ses->state == AP_STATE_ACTIVE)
-		__sync_sub_and_fetch(&ap_session_stat.active, 1);
+		ap_session_stat_move(&ap_session_stat.active, &ap_session_stat.finishing);
 	else
-		__sync_sub_and_fetch(&ap_session_stat.starting, 1);
+		ap_session_stat_move(&ap_session_stat.starting, &ap_session_stat.finishing);
 
-	__sync_add_and_fetch(&ap_session_stat.finishing, 1);
 	ses->terminating = 1;
 	ses->state = AP_STATE_FINISHING;
 
@@ -333,7 +376,7 @@ int ap_shutdown_soft(void (*cb)(void), int term)
 
 	pthread_rwlock_rdlock(&ses_lock);
 
-	if (!ap_session_stat.starting && !ap_session_stat.active && !ap_session_stat.finishing) {
+	if (ap_session_stat_idle()) {
 		pthread_rwlock_unlock(&ses_lock);
 		if (shutdown_cb)
 			shutdown_cb();