From 730666be7fb81e440387f72c8cea1234ddab0ccb Mon Sep 17 00:00:00 2001 From: Vojtech Vilimek Date: Mon, 4 Sep 2023 13:58:59 +0200 Subject: [PATCH] SNMP: Add timeout config option, refactoring --- proto/snmp/bgp_mib.c | 2 + proto/snmp/config.Y | 44 +++++++++++++++----- proto/snmp/snmp.c | 91 ++++++++++++++++++----------------------- proto/snmp/snmp.h | 18 ++------ proto/snmp/snmp_utils.c | 22 ++-------- proto/snmp/subagent.c | 54 ++++++++++++------------ proto/snmp/subagent.h | 2 +- 7 files changed, 109 insertions(+), 124 deletions(-) diff --git a/proto/snmp/bgp_mib.c b/proto/snmp/bgp_mib.c index 464fcf32..5963c971 100644 --- a/proto/snmp/bgp_mib.c +++ b/proto/snmp/bgp_mib.c @@ -699,6 +699,8 @@ bgp_find_dynamic_oid(struct snmp_proto *p, struct oid *o_start, const struct oid if (cmp < 0 || (cmp == 0 && snmp_is_oid_empty(o_end))) { snmp_log("ip4_less() returned true"); + + // TODO repair struct oid *o = snmp_oid_duplicate(p->p.pool, o_start); snmp_oid_ip4_index(o, 5, net4_prefix(&net)); diff --git a/proto/snmp/config.Y b/proto/snmp/config.Y index eb16c4fd..3d9fc122 100644 --- a/proto/snmp/config.Y +++ b/proto/snmp/config.Y @@ -10,6 +10,7 @@ CF_HDR #include "proto/snmp/snmp.h" +#include "proto/snmp/subagent.h" CF_DEFINES @@ -17,7 +18,8 @@ CF_DEFINES CF_DECLS -CF_KEYWORDS(SNMP, PROTOCOL, BPG, LOCAL, AS, REMOTE, PORT) +CF_KEYWORDS(SNMP, PROTOCOL, BPG, LOCAL, AS, REMOTE, ADDRESS, PORT, DESCRIPTION, + TIMEOUT, PRIORITY) CF_GRAMMAR @@ -27,15 +29,35 @@ snmp_proto: snmp_proto_start '{' | snmp_proto proto_item ';' | snmp_proto snmp_bgp_bond ';' - | snmp_proto LOCAL PORT expr ';' { SNMP_CFG->local_port = $4; if (($4<1) || -($4>65535)) cf_error("Invalid port number"); } - | snmp_proto REMOTE PORT expr ';' { SNMP_CFG->remote_port = $4; if (($4<1) || -($4>65545)) cf_error("Invalid port number"); } - | snmp_proto LOCAL ipa ';' { SNMP_CFG->local_ip = $3; } - | snmp_proto REMOTE ipa ';' { SNMP_CFG->remote_ip = $3; if(ipa_zero($3)) -cf_error("Invalid remote ip address"); } - | snmp_proto LOCAL AS expr ';' { if ($4<1 || $4>65535) cf_error("invalid local as"); - SNMP_CFG->local_as = $4; } + | snmp_proto LOCAL PORT expr ';' { + if (($4 < 1) || ($4 > 65535)) cf_error("Invalid port number"); + SNMP_CFG->local_port = $4; + } + | snmp_proto REMOTE PORT expr ';' { + if (($4 < 1) || ($4 > 65535)) cf_error("Invalid port number"); + SNMP_CFG->remote_port = $4; + } + | snmp_proto LOCAL ADDRESS ipa ';' { SNMP_CFG->local_ip = $4; } + | snmp_proto REMOTE ADDRESS ipa ';' { + if (ipa_zero($4)) cf_error("Invalid remote ip address"); + SNMP_CFG->remote_ip = $4; + } + | snmp_proto LOCAL AS expr ';' { + if ($4 < 1 || $4 > 65535) cf_error("Invalid local AS"); + SNMP_CFG->local_as = $4; + } + | snmp_proto DESCRIPTION text ';' { + if (strlen($3) > UINT32_MAX) cf_error("Description is too long"); + SNMP_CFG->description = $3; + } + | snmp_proto TIMEOUT expr ';' { + if ($3 < 1 || $3 > 255) cf_error("Timeout must be in range 1-255"); + SNMP_CFG->timeout = $3; + } + | snmp_proto PRIORITY expr ';' { + if ($3 > 255) cf_error("Registration priority must be in range 0-255"); + SNMP_CFG->priority = $3; + } ; snmp_proto_start: proto_start SNMP @@ -51,7 +73,9 @@ snmp_proto_start: proto_start SNMP SNMP_CFG->remote_port = 705; SNMP_CFG->local_as = 0; + SNMP_CFG->description = "bird"; SNMP_CFG->timeout = 15; + SNMP_CFG->priority = AGENTX_PRIORITY; } proto_name ; diff --git a/proto/snmp/snmp.c b/proto/snmp/snmp.c index a232fd32..a2cef68f 100644 --- a/proto/snmp/snmp.c +++ b/proto/snmp/snmp.c @@ -80,7 +80,6 @@ #include "snmp_utils.h" static const char * const snmp_state[] = { - [SNMP_ERR] = "SNMP ERROR", [SNMP_INIT] = "SNMP INIT", [SNMP_LOCKED] = "SNMP LOCKED", [SNMP_OPEN] = "SNMP CONNECTION OPENED", @@ -107,8 +106,7 @@ snmp_init(struct proto_config *CF) snmp_log("changing proto_snmp state to INIT"); p->state = SNMP_INIT; - // p->timeout = cf->timeout; - p->timeout = 15; + p->timeout = cf->timeout; snmp_log("snmp_reconfigure() lip: %I:%u rip: %I:%u", cf->local_ip, cf->local_port, cf->remote_ip, cf->remote_port); @@ -116,9 +114,10 @@ snmp_init(struct proto_config *CF) return P; } -static inline void +static inline int snmp_cleanup(struct snmp_proto *p) { + /* Function tm_stop() is called inside rfree() */ rfree(p->startup_timer); p->startup_timer = NULL; @@ -131,14 +130,15 @@ snmp_cleanup(struct snmp_proto *p) rfree(p->lock); p->lock = NULL; - p->state = SNMP_DOWN; + // TODO cleanup lists, hash table, trie, ... + + return (p->state = SNMP_DOWN); } void snmp_down(struct snmp_proto *p) { snmp_cleanup(p); - proto_notify_state(&p->p, PS_DOWN); } @@ -154,12 +154,8 @@ snmp_sock_err(sock *sk, int err) rfree(p->sock); p->sock = NULL; - rfree(p->lock); - p->lock = NULL; - - snmp_log("changing proto_snmp state to ERR[OR]"); - p->state = SNMP_ERR; - // snmp_shutdown((struct proto *) p); + snmp_log("changing proto_snmp state to LOCKED"); + p->state = SNMP_LOCKED; // TODO ping interval tm_start(p->startup_timer, 4 S); @@ -181,8 +177,8 @@ snmp_connected(sock *sk) snmp_start_subagent(p); - // TODO ping interval - tm_set(p->ping_timer, 15 S); + // TODO ping interval + tm_set(p->ping_timer, p->timeout S); } static void @@ -215,8 +211,10 @@ snmp_start_locked(struct object_lock *lock) if (sk_open(s) < 0) { + // TODO rather set the startup time, then reset whole SNMP proto log(L_ERR "Cannot open listening socket"); snmp_down(p); + // TODO go back to SNMP_INIT and try reconnecting after timeout } snmp_log("socket ready!, trying to connect"); @@ -225,26 +223,27 @@ snmp_start_locked(struct object_lock *lock) static void snmp_startup(struct snmp_proto *p) { - //snmp_log("changing proto_snmp state to INIT"); - - if (p->state == SNMP_LOCKED || - p->state == SNMP_OPEN || - p->state == SNMP_REGISTER || - p->state == SNMP_CONN) + if (p->state != SNMP_INIT && + p->state != SNMP_LOCKED && + p->state != SNMP_DOWN) { snmp_log("startup() already in connected state %u", p->state); return; } - snmp_log("snmp_startup()"); + if (p->lock) + { + snmp_start_locked(p->lock); + return; + } + + snmp_log("snmp_startup(), preprating lock"); p->state = SNMP_INIT; - /* starting agentX communicaiton channel */ + /* Starting AgentX communicaiton channel. */ - snmp_log("preparing lock"); struct object_lock *lock; - /* we could have the lock already acquired but be in ERROR state */ lock = p->lock = olock_new(p->p.pool); // lock->addr @@ -287,13 +286,13 @@ snmp_ping_timer(struct timer *tm) // snmp_log("snmp_ping_timer() "); struct snmp_proto *p = tm->data; - if (p->state == SNMP_CONN) + if (p->state == SNMP_REGISTER || + p->state == SNMP_CONN) { snmp_ping(p); } - //tm_set(tm, current_time() + (15 S)); - tm_set(tm, current_time() + 15 S); + tm_set(tm, current_time() + p->timeout S); } static int @@ -349,24 +348,13 @@ static int snmp_reconfigure(struct proto *P, struct proto_config *CF) { struct snmp_proto *p = SKIP_BACK(struct snmp_proto, p, P); - const struct snmp_config *cf = SKIP_BACK(struct snmp_config, cf, CF); + const struct snmp_config *new = SKIP_BACK(struct snmp_config, cf, CF); + const struct snmp_config *old = SKIP_BACK(struct snmp_config, cf, p->p.cf); - p->local_ip = cf->local_ip; - p->remote_ip = cf->remote_ip; - p->local_port = cf->local_port; - p->remote_port = cf->remote_port; - p->local_as = cf->local_as; - p->timeout = 15; - - /* workaround to make the registration happen */ - p->register_to_ack = 1; - - /* TODO walk all bind protocols and find their (new) IP - to update HASH table */ - snmp_log("snmp_reconfigure() lip: %I:%u rip: %I:%u", - p->local_ip, p->local_port, p->remote_ip, p->remote_port); - - return 1; + return !memcpy((byte *) old + sizeof(struct proto_config), + ((byte *) new) + sizeof(struct proto_config), + OFFSETOF(struct snmp_config, description) - sizeof(struct proto_config)) + && ! strncmp(old->description, new->description, UINT32_MAX); } static void @@ -434,6 +422,7 @@ bp->stats.fsm_established_transitions); cli_msg(-1006, " outgoinin_conn state %u", bp->outgoing_conn.state + 1); cli_msg(-1006, " incoming_conn state: %u", bp->incoming_conn.state + 1); + cli_msg(-1006, ""); } } @@ -453,27 +442,25 @@ snmp_shutdown(struct proto *P) tm_stop(p->ping_timer); - /* connection established -> close the connection */ - if (p->state == SNMP_REGISTER || + if (p->state == SNMP_OPEN || + p->state == SNMP_REGISTER || p->state == SNMP_CONN) { + /* We have connection established (at leased send out Open-PDU). */ p->state = SNMP_STOP; - /* startup time is reused for connection closing */ p->startup_timer->hook = snmp_stop_timeout; - // TODO timeout option - tm_set(p->startup_timer, 15 S); + tm_set(p->startup_timer, p->timeout S); snmp_stop_subagent(p); return PS_STOP; } - /* no connection to close */ else { - snmp_cleanup(p); - return PS_DOWN; + /* We did not create a connection, we clean the lock and other stuff. */ + return snmp_cleanup(p); } } diff --git a/proto/snmp/snmp.h b/proto/snmp/snmp.h index c6edc14b..abbbae9c 100644 --- a/proto/snmp/snmp.h +++ b/proto/snmp/snmp.h @@ -30,8 +30,6 @@ #define SNMP_TX_BUFFER_SIZE 8192 enum snmp_proto_state { - SNMP_ERR = 0, - SNMP_INIT = 1, SNMP_LOCKED, SNMP_OPEN, @@ -39,18 +37,6 @@ enum snmp_proto_state { SNMP_CONN, SNMP_STOP, SNMP_DOWN, - -/* - SNMP_ERR = 0, - SNMP_DELAY, - SNMP_INIT, - SNMP_REGISTER, - SNMP_CONN, - SNMP_STOP, - SNMP_DOWN, - SNMP_LISTEN, - SNMP_RESET, -*/ }; /* hash table macros */ @@ -79,9 +65,12 @@ struct snmp_config { u16 remote_port; u32 local_as; u8 timeout; + u8 priority; //struct iface *iface; + const char *description; list bgp_entries; u32 bonds; + // TODO add support for subagent oid identification }; struct snmp_bgp_peer { @@ -116,7 +105,6 @@ struct snmp_proto { u32 local_as; sock *sock; - // timeout for what ?? u8 timeout; u32 session_id; diff --git a/proto/snmp/snmp_utils.c b/proto/snmp/snmp_utils.c index 393c4286..b4eb41df 100644 --- a/proto/snmp/snmp_utils.c +++ b/proto/snmp/snmp_utils.c @@ -46,7 +46,7 @@ void snmp_oid_copy(struct oid *dest, const struct oid *src) { STORE_U8(dest->n_subid, src->n_subid); STORE_U8(dest->prefix, src->prefix); - STORE_U8(dest->include, src->include); + STORE_U8(dest->include, src->include ? 1 : 0); STORE_U8(dest->pad, 0); for (int i = 0; i < src->n_subid; i++) @@ -262,23 +262,9 @@ snmp_put_blank(byte *buf) byte * snmp_put_oid(byte *buf, struct oid *oid) { - put_u8(buf, oid->n_subid); - put_u8(++buf, oid->prefix); - put_u8(++buf, oid->include); - put_u8(++buf, 0); // padding - - /* last increment */ - ++buf; - - /* copy OID data */ -#ifdef SNMP_NATIVE - for (uint i = 0; i < oid->n_subid; i++) - *(((u32 *) buf) + i) = oid->ids[i]; -#else - put_u32s(buf, oid->ids, oid->n_subid * 4); -#endif - - return buf + oid->n_subid * 4; + struct oid *oid_buf = (void *) buf; + snmp_oid_copy(oid_buf, oid); + return buf + snmp_oid_size(oid); } /** diff --git a/proto/snmp/subagent.c b/proto/snmp/subagent.c index 5f6acf1a..f7dee60c 100644 --- a/proto/snmp/subagent.c +++ b/proto/snmp/subagent.c @@ -80,19 +80,16 @@ static const char * const snmp_pkt_type[] = { static void open_pdu(struct snmp_proto *p, struct oid *oid) { + const struct snmp_config *cf = SKIP_BACK(struct snmp_config, cf, p->p.cf); sock *sk = p->sock; struct snmp_pdu_context c = SNMP_PDU_CONTEXT(sk); byte *buf = c.buffer; - // TODO should be configurable; with check on string length - const char *str = "bird"; - /* +4 for timeout (1B with 4B alignment) */ - if (c.size < AGENTX_HEADER_SIZE + 4 + snmp_oid_size(oid) + snmp_str_size(str)) + if (c.size < AGENTX_HEADER_SIZE + 4 + snmp_oid_size(oid) + + + snmp_str_size(cf->description)) { - return; - // TODO create and add message info into message queue snmp_manage_tbuf(p, &c); buf = c.buffer; } @@ -106,10 +103,10 @@ open_pdu(struct snmp_proto *p, struct oid *oid) STORE_U32(h->transaction_id, 1); STORE_U32(h->packet_id, 1); - c.size -= (4 + snmp_oid_size(oid) + snmp_str_size(str)); + c.size -= (4 + snmp_oid_size(oid) + snmp_str_size(cf->description)); c.buffer = snmp_put_fbyte(c.buffer, p->timeout); c.buffer = snmp_put_oid(c.buffer, oid); - c.buffer = snmp_put_str(c.buffer, str); + c.buffer = snmp_put_str(c.buffer, cf->description); uint s = update_packet_size(p, buf, c.buffer); int ret = sk_send(sk, s); @@ -122,7 +119,7 @@ open_pdu(struct snmp_proto *p, struct oid *oid) } void -snmp_notify_pdu(struct snmp_proto *p, struct oid *oid, void *opaque, uint size, int include_uptime) +snmp_notify_pdu(struct snmp_proto *p, struct oid *oid, void *data, uint size, int include_uptime) { sock *sk = p->sock; @@ -163,8 +160,8 @@ snmp_notify_pdu(struct snmp_proto *p, struct oid *oid, void *opaque, uint size, for (uint i = 0; i < uptime.n_subid; i++) STORE_U32(vb->name.ids[i], uptime_ids[i]); ADVANCE(c.buffer, c.size, snmp_varbind_header_size(vb)); - snmp_varbind_ticks(vb, c.size, current_time() TO_S); - ADVANCE(c.buffer, c.size, agentx_type_size(AGENTX_TIME_TICKS)); + snmp_varbind_ticks(vb, c.size, (current_time() TO_S) / 100); + ADVANCE(c.buffer, c.size, snmp_varbind_size(vb, c.byte_ord)); } /* snmpTrapOID.0 oid */ @@ -175,7 +172,7 @@ snmp_notify_pdu(struct snmp_proto *p, struct oid *oid, void *opaque, uint size, .pad = 0, }; u32 trap0_ids[] = { 3, 1, 1, 4, 1, 0 }; - + struct agentx_varbind *trap_vb = snmp_create_varbind(c.buffer, &trap0); for (uint i = 0; i < trap0.n_subid; i++) STORE_U32(trap_vb->name.ids[i], trap0_ids[i]); @@ -183,9 +180,8 @@ snmp_notify_pdu(struct snmp_proto *p, struct oid *oid, void *opaque, uint size, snmp_put_oid(SNMP_VB_DATA(trap_vb), oid); ADVANCE(c.buffer, c.size, snmp_varbind_size(trap_vb, c.byte_ord)); - // TODO fix the endianess - memcpy(c.buffer, opaque, size); - ADVANCE(c.buffer, c.size, (size + snmp_varbind_hdr_size_from_oid(oid))); + memcpy(c.buffer, data, size); + ADVANCE(c.buffer, c.size, size); uint s = update_packet_size(p, sk->tbuf, c.buffer); @@ -198,7 +194,7 @@ snmp_notify_pdu(struct snmp_proto *p, struct oid *oid, void *opaque, uint size, snmp_log("sk_send error"); #undef TRAP0_HEADER_SIZE -#undef UPTIME_SIZE +#undef UPTIME_SIZE } /* index allocate / deallocate pdu * / @@ -259,10 +255,11 @@ un_register_pdu(struct snmp_proto *p, struct oid *oid, uint index, uint len, u8 c.byte_ord = h->flags & AGENTX_NETWORK_BYTE_ORDER; /* do not override timeout */ - STORE_U32(ur->timeout, 15); + STORE_U8(ur->timeout, p->timeout); /* default priority */ - STORE_U32(ur->priority, AGENTX_PRIORITY); - STORE_U32(ur->range_subid, (len > 1) ? index : 0); + STORE_U8(ur->priority, AGENTX_PRIORITY); + STORE_U8(ur->range_subid, (len > 1) ? index : 0); + STORE_U8(ur->pad, 0); snmp_put_oid(c.buffer, oid); ADVANCE(c.buffer, c.size, snmp_oid_size(oid)); @@ -286,15 +283,14 @@ un_register_pdu(struct snmp_proto *p, struct oid *oid, uint index, uint len, u8 snmp_log("sk_send error"); } -/* register pdu */ +/* Register-PDU */ void snmp_register(struct snmp_proto *p, struct oid *oid, uint index, uint len, u8 is_instance) { un_register_pdu(p, oid, index, len, AGENTX_REGISTER_PDU, is_instance); } - -/* unregister pdu */ +/* Unregister-PDU */ void UNUSED snmp_unregister(struct snmp_proto *p, struct oid *oid, uint index, uint len) { @@ -830,7 +826,7 @@ parse_close_pdu(struct snmp_proto UNUSED *p, byte UNUSED *req, uint UNUSED size) p->state = SNMP_ERR; - proto_notify(PS_DOWN, &p->p); + proto_state_notify(&p->p, PS_DOWN); */ return 0; } @@ -1011,7 +1007,7 @@ parse_gets2_pdu(struct snmp_proto *p, byte * const pkt_start, uint size, uint *s goto wait; } - /* update buffer pointer and remaining size counters */ + /* Update buffer pointer and remaining size counters. */ ADVANCE(pkt, pkt_size, sz); size -= sz; @@ -1042,7 +1038,7 @@ parse_gets2_pdu(struct snmp_proto *p, byte * const pkt_start, uint size, uint *s ADVANCE(pkt, pkt_size, sz); size -= sz; - // TODO check for oversized oids before any allocation (in prefixize()) + // TODO check for oversized OIDs before any allocation (in prefixize()) /* We create copy of OIDs outside of rx-buffer and also prefixize them */ o_start = snmp_prefixize(p, o_start_b, c.byte_ord); @@ -1115,7 +1111,7 @@ send: mb_free(o_start); mb_free(o_end); - /* number of bytes parsed form rx-buffer */ + /* number of bytes parsed form RX-buffer */ return pkt - pkt_start; partial: @@ -1128,6 +1124,8 @@ partial: snmp_log("new rx-buffer size %u", h->payload); *skip = AGENTX_HEADER_SIZE; p->partial_response = response_header; + + /* number of bytes parsed from RX-buffer */ return pkt - pkt_start; wait: @@ -1222,7 +1220,7 @@ snmp_rx(sock *sk, uint size) return 1; } -/* ping pdu */ +/* Ping-PDU */ void snmp_ping(struct snmp_proto *p) { @@ -1245,7 +1243,7 @@ snmp_ping(struct snmp_proto *p) snmp_log("sending ping packet ... tpos 0x%p", sk->tpos); snmp_dump_packet(sk->tpos, AGENTX_HEADER_SIZE + 4); /* sending only header -> pkt - buf */ - uint s = update_packet_size(p, sk->tpos, c.buffer); + uint s = update_packet_size(p, sk->tpos, c.buffer); int ret = sk_send(sk, s); if (ret > 0) snmp_log("sk_send OK!"); diff --git a/proto/snmp/subagent.h b/proto/snmp/subagent.h index d23d2d8e..79173ba9 100644 --- a/proto/snmp/subagent.h +++ b/proto/snmp/subagent.h @@ -196,7 +196,7 @@ struct agentx_un_register_pdu { u8 timeout; u8 priority; u8 range_subid; - u8 padd; + u8 pad; }; struct agentx_bulk_state {