From: Martijn van Duren Subject: Re: snmpd: move smi_getticks to appl_sysuptime To: tech@openbsd.org Date: Sun, 9 Nov 2025 09:10:32 +0100 ping On 10/28/25 9:11 AM, Martijn van Duren wrote: > ping > > On Wed, 2025-10-08 at 12:27 +0200, Martijn van Duren wrote: >> ping >> >> On Mon, 2024-03-18 at 18:22 +0100, Martijn van Duren wrote: >>> smi_getticks is snmpd's current way of supplying the sysUpTime. SMI >>> doesn't define the sysUpTime (that would be SNMPv2-MIB), so this >>> placing is wrong. Another issue with it is that it uses gettimeofday, >>> where CLOCK_MONOTONIC would be a better fit. >>> >>> The definition of sysUpTime is: >>> "The time (in hundredths of a second) since the >>> network management portion of the system was last >>> re-initialized." >>> Where I wasn't able to find a clear definition of "network management >>> portion". However, the AgentX RFC2741 states in section 6.2.16: >>> res.sysUpTime >>> >>> This field contains the current value of sysUpTime for the >>> context indicated within the PDU to which this PDU is a >>> response. >>> Implying that the sysUpTime is at the very least on a per context basis. >>> For this reason I went with placing the information in appl_context. >>> >>> Because of this the start time should be set when the context is >>> created, implying that the context should be created during appl_init, >>> instead of relying on a random query creating it (either AgentX setup, >>> or an SNMP packet). >>> >>> As for the 'struct appl_context *' vs 'const char *', I've stuck >>> with 'const char *' for consistency and keep things a bit more in >>> line with the SNMP/AgentX protocol point of view. >>> >>> While here fix the types of sysORLastChange and sysORUpTime. >>> >>> OK? >>> >>> martijn@ diff e187005a676727d547bd9d1ddb5f1fc0f99df471 c11d3fa04dc651329001615fdd8ee9886239e54a commit - e187005a676727d547bd9d1ddb5f1fc0f99df471 commit + c11d3fa04dc651329001615fdd8ee9886239e54a blob - fdc7e42a707112fead524d974bcf3d262029eda1 blob + 3da88e17c2e299b5b374b4ebe6438da6ef1a91ab --- usr.sbin/snmpd/application.c +++ usr.sbin/snmpd/application.c @@ -28,6 +28,7 @@ #include #include #include +#include #include "application.h" #include "log.h" @@ -48,7 +49,7 @@ struct appl_agentcap { uint32_t aa_index; struct ber_oid aa_oid; char aa_descr[256]; - int aa_uptime; + uint32_t aa_uptime; TAILQ_ENTRY(appl_agentcap) aa_entry; }; @@ -57,10 +58,13 @@ struct appl_context { char ac_name[APPL_CONTEXTNAME_MAX + 1]; RB_HEAD(appl_regions, appl_region) ac_regions; + TAILQ_HEAD(, appl_agentcap) ac_agentcaps; int ac_agentcap_lastid; - int ac_agentcap_lastchange; + uint32_t ac_agentcap_lastchange; + struct timespec ac_starttime; + TAILQ_ENTRY(appl_context) ac_entries; }; @@ -179,6 +183,8 @@ appl(void) void appl_init(void) { + if (appl_context(NULL, 1) == NULL) + fatal("Failed to create default context"); appl_blocklist_init(); appl_internal_init(); appl_agentx_init(); @@ -220,7 +226,7 @@ appl_context(const char *name, int create) } /* Always allow the default namespace */ - if (!create && name[0] != '\0') { + if (!create) { errno = ENOENT; return NULL; } @@ -235,6 +241,9 @@ appl_context(const char *name, int create) ctx->ac_agentcap_lastchange = 0; TAILQ_INSERT_TAIL(&contexts, ctx, ac_entries); + + if (clock_gettime(CLOCK_MONOTONIC, &ctx->ac_starttime) == -1) + fatal("clock_gettime"); return ctx; } @@ -271,7 +280,7 @@ appl_addagentcaps(const char *ctxname, struct ber_oid cap->aa_context = ctx; cap->aa_index = ++ctx->ac_agentcap_lastid; cap->aa_oid = *oid; - cap->aa_uptime = smi_getticks(); + cap->aa_uptime = appl_sysuptime(ctxname); if (strlcpy(cap->aa_descr, descr, sizeof(cap->aa_descr)) >= sizeof(cap->aa_descr)) { log_info("%s: Can't add agent capabilities %s: " @@ -331,10 +340,27 @@ void appl_agentcap_free(struct appl_agentcap *cap) { TAILQ_REMOVE(&(cap->aa_context->ac_agentcaps), cap, aa_entry); - cap->aa_context->ac_agentcap_lastchange = smi_getticks(); + cap->aa_context->ac_agentcap_lastchange = + appl_sysuptime(cap->aa_context->ac_name); free(cap); } +uint32_t +appl_sysuptime(const char *ctxname) +{ + struct appl_context *ctx; + struct timespec now, uptime; + + if ((ctx = appl_context(ctxname, 0)) == NULL) + return 0; + + if (clock_gettime(CLOCK_MONOTONIC, &now) == -1) + fatal("clock_gettime"); + + timespecsub(&now, &ctx->ac_starttime, &uptime); + return uptime.tv_sec * 100 + uptime.tv_nsec / 10000000; +} + struct ber_element * appl_sysorlastchange(struct ber_oid *oid) { blob - 5c35942137a00b8de9ea34f1f9ae5f6b9651207f blob + 884dc530020f25e1d03d7df382b7485921a7fdee --- usr.sbin/snmpd/application.h +++ usr.sbin/snmpd/application.h @@ -123,6 +123,7 @@ void appl(void); void appl_init(void); void appl_shutdown(void); struct appl_context *appl_context(const char *, int); +uint32_t appl_sysuptime(const char *); enum appl_error appl_addagentcaps(const char *, struct ber_oid *, const char *, struct appl_backend *); enum appl_error appl_removeagentcaps(const char *, struct ber_oid *, blob - cf6c628e9b5e6c53014ea8aa3fd8796ad3ab245e blob + 0930824f1db50d27be2b5b2faba0768ce8701deb --- usr.sbin/snmpd/application_agentx.c +++ usr.sbin/snmpd/application_agentx.c @@ -423,7 +423,8 @@ appl_agentx_recv(int fd, short event, void *cookie) case AX_PDU_TYPE_PING: ax_response(conn->conn_ax, pdu->ap_header.aph_sessionid, pdu->ap_header.aph_transactionid, - pdu->ap_header.aph_packetid, smi_getticks(), + pdu->ap_header.aph_packetid, + appl_sysuptime(pdu->ap_context.aos_string), APPL_ERROR_NOERROR, 0, NULL, 0); event_add(&(conn->conn_wev), NULL); break; @@ -433,7 +434,8 @@ appl_agentx_recv(int fd, short event, void *cookie) ax_pdutype2string(pdu->ap_header.aph_type)); ax_response(conn->conn_ax, pdu->ap_header.aph_sessionid, pdu->ap_header.aph_transactionid, - pdu->ap_header.aph_packetid, smi_getticks(), + pdu->ap_header.aph_packetid, + appl_sysuptime(pdu->ap_context.aos_string), APPL_ERROR_PROCESSINGERROR, 1, pdu->ap_payload.ap_vbl.ap_varbind, pdu->ap_payload.ap_vbl.ap_nvarbind); @@ -455,8 +457,8 @@ appl_agentx_recv(int fd, short event, void *cookie) fail: ax_response(conn->conn_ax, pdu->ap_header.aph_sessionid, pdu->ap_header.aph_transactionid, - pdu->ap_header.aph_packetid, smi_getticks(), - error, 0, NULL, 0); + pdu->ap_header.aph_packetid, + appl_sysuptime(pdu->ap_context.aos_string), error, 0, NULL, 0); event_add(&(conn->conn_wev), NULL); ax_pdu_free(pdu); @@ -565,13 +567,14 @@ appl_agentx_open(struct appl_agentx_connection *conn, ax_response(conn->conn_ax, session->sess_id, pdu->ap_header.aph_transactionid, pdu->ap_header.aph_packetid, - smi_getticks(), APPL_ERROR_NOERROR, 0, NULL, 0); + appl_sysuptime(NULL), APPL_ERROR_NOERROR, 0, NULL, 0); event_add(&(conn->conn_wev), NULL); return; fail: ax_response(conn->conn_ax, 0, pdu->ap_header.aph_transactionid, - pdu->ap_header.aph_packetid, 0, error, 0, NULL, 0); + pdu->ap_header.aph_packetid, appl_sysuptime(NULL), + error, 0, NULL, 0); event_add(&(conn->conn_wev), NULL); if (session != NULL) free(session->sess_descr.aos_string); @@ -597,7 +600,7 @@ appl_agentx_close(struct appl_agentx_session *session, ax_response(conn->conn_ax, pdu->ap_header.aph_sessionid, pdu->ap_header.aph_transactionid, pdu->ap_header.aph_packetid, - smi_getticks(), error, 0, NULL, 0); + appl_sysuptime(NULL), error, 0, NULL, 0); event_add(&(conn->conn_wev), NULL); if (error == APPL_ERROR_NOERROR) return; @@ -676,7 +679,7 @@ appl_agentx_register(struct appl_agentx_session *sessi fail: ax_response(session->sess_conn->conn_ax, session->sess_id, pdu->ap_header.aph_transactionid, pdu->ap_header.aph_packetid, - smi_getticks(), error, 0, NULL, 0); + appl_sysuptime(pdu->ap_context.aos_string), error, 0, NULL, 0); event_add(&(session->sess_conn->conn_wev), NULL); } @@ -703,7 +706,7 @@ appl_agentx_unregister(struct appl_agentx_session *ses fail: ax_response(session->sess_conn->conn_ax, session->sess_id, pdu->ap_header.aph_transactionid, pdu->ap_header.aph_packetid, - smi_getticks(), error, 0, NULL, 0); + appl_sysuptime(pdu->ap_context.aos_string), error, 0, NULL, 0); event_add(&(session->sess_conn->conn_wev), NULL); } @@ -835,7 +838,7 @@ appl_agentx_addagentcaps(struct appl_agentx_session *s fail: ax_response(session->sess_conn->conn_ax, session->sess_id, pdu->ap_header.aph_transactionid, pdu->ap_header.aph_packetid, - smi_getticks(), error, 0, NULL, 0); + appl_sysuptime(pdu->ap_context.aos_string), error, 0, NULL, 0); event_add(&(session->sess_conn->conn_wev), NULL); } @@ -860,7 +863,7 @@ appl_agentx_removeagentcaps(struct appl_agentx_session fail: ax_response(session->sess_conn->conn_ax, session->sess_id, pdu->ap_header.aph_transactionid, pdu->ap_header.aph_packetid, - smi_getticks(), error, 0, NULL, 0); + appl_sysuptime(pdu->ap_context.aos_string), error, 0, NULL, 0); event_add(&(session->sess_conn->conn_wev), NULL); } blob - 95032aea33b2fcd931b317151a73c8d5780a82d2 blob + 22b890324bfb16a0977624bc5ca1f86cad6653ce --- usr.sbin/snmpd/application_internal.c +++ usr.sbin/snmpd/application_internal.c @@ -577,7 +577,7 @@ appl_internal_system(struct ber_oid *oid) else if (ober_oid_cmp(&OID(MIB_sysOID, 0), oid) == 0) return ober_add_oid(NULL, &s->sys_oid); else if (ober_oid_cmp(&OID(MIB_sysUpTime, 0), oid) == 0) { - value = ober_add_integer(NULL, smi_getticks()); + value = ober_add_integer(NULL, appl_sysuptime(NULL)); ober_set_header(value, BER_CLASS_APPLICATION, SNMP_T_TIMETICKS); } else if (ober_oid_cmp(&OID(MIB_sysContact, 0), oid) == 0) return ober_add_string(NULL, s->sys_contact); blob - 09a34fafd0b4d8262cce2fded935589079e6ec6b blob + 88ef067c4012494d0cf169a2aff723c03a6ada68 --- usr.sbin/snmpd/smi.c +++ usr.sbin/snmpd/smi.c @@ -62,23 +62,6 @@ RB_HEAD(keytree, oid); RB_PROTOTYPE(keytree, oid, o_keyword, smi_key_cmp); struct keytree smi_keytree; -u_long -smi_getticks(void) -{ - struct timeval now, run; - u_long ticks; - - gettimeofday(&now, NULL); - if (timercmp(&now, &snmpd_env->sc_starttime, <=)) - return (0); - timersub(&now, &snmpd_env->sc_starttime, &run); - ticks = run.tv_sec * 100; - if (run.tv_usec) - ticks += run.tv_usec / 10000; - - return (ticks); -} - char * smi_oid2string(struct ber_oid *o, char *buf, size_t len, size_t skip) { blob - c6e31c0cceb6d7d2df62e41b110c8f3517b41967 blob + 47cf842ff3044e353b44da88dfe391a2561ee190 --- usr.sbin/snmpd/smi.h +++ usr.sbin/snmpd/smi.h @@ -22,6 +22,5 @@ struct ber_element; struct ber_oid; char *smi_oid2string(struct ber_oid *, char *, size_t, size_t); -u_long smi_getticks(void); char *smi_print_element(struct ber_element *); char *smi_print_element_legacy(struct ber_element *); blob - 60af191fb6f00ff295542f65ed9c733b66038830 blob + 85581e7ee9b020d658101571df8bf8a761d7b5f6 --- usr.sbin/snmpd/snmpd.c +++ usr.sbin/snmpd/snmpd.c @@ -218,7 +218,6 @@ main(int argc, char *argv[]) log_init(debug, LOG_DAEMON); log_setverbose(verbose); - gettimeofday(&env->sc_starttime, NULL); env->sc_engine_boots = 0; proc_init(ps, procs, nitems(procs), debug, argc0, argv0, proc_id); blob - bc1b9ff7d6b8fab49046e4fe3bec9ba5fca31a35 blob + 44773fae763979f619296a7421a8ea87e7429a46 --- usr.sbin/snmpd/snmpd.h +++ usr.sbin/snmpd/snmpd.h @@ -22,7 +22,6 @@ #include #include -#include #include #include #include @@ -402,7 +401,6 @@ struct snmpd { const char *sc_confpath; struct addresslist sc_addresses; struct axmasterlist sc_agentx_masters; - struct timeval sc_starttime; u_int32_t sc_engine_boots; char sc_rdcommunity[SNMPD_MAXCOMMUNITYLEN]; blob - c68ada5fb0bc53a5c520699e2f5eb7e6114c6dc3 blob + f676ead16b9622eea34346322e562365ec058815 --- usr.sbin/snmpd/trap.c +++ usr.sbin/snmpd/trap.c @@ -24,9 +24,9 @@ #include #include +#include "application.h" #include "log.h" #include "mib.h" -#include "smi.h" #include "snmp.h" #include "snmpd.h" @@ -60,7 +60,7 @@ trap_send(struct ber_oid *oid, struct ber_element *elm /* Add mandatory varbind elements */ trap = ober_add_sequence(NULL); vblist = ober_printf_elements(trap, "{Odt}{OO}", - &uptime, smi_getticks(), + &uptime, appl_sysuptime(NULL), BER_CLASS_APPLICATION, SNMP_T_TIMETICKS, &trapoid, oid); if (elm != NULL)