From 578a8a43bc00fe26ed7168bd2a9c6f99dd885bb4 Mon Sep 17 00:00:00 2001 From: Vojtech Vilimek Date: Wed, 14 Aug 2024 18:24:25 +0200 Subject: [PATCH] SNMP: improved debug messages, shorter description --- proto/snmp/config.Y | 6 ++++-- proto/snmp/snmp.c | 12 +++++++----- proto/snmp/snmp.h | 3 +++ proto/snmp/subagent.c | 25 ++++++++++++++++--------- 4 files changed, 30 insertions(+), 16 deletions(-) diff --git a/proto/snmp/config.Y b/proto/snmp/config.Y index a9b25d2e..1aeef74a 100644 --- a/proto/snmp/config.Y +++ b/proto/snmp/config.Y @@ -19,7 +19,7 @@ CF_DEFINES CF_DECLS CF_KEYWORDS(SNMP, PROTOCOL, BGP, LOCAL, AS, REMOTE, ADDRESS, PORT, DESCRIPTION, - TIMEOUT, PRIORITY, CONTEXT, DEFAULT, MESSAGE) + TIMEOUT, PRIORITY, CONTEXT, DEFAULT, MESSAGE, VERBOSE) CF_GRAMMAR @@ -66,7 +66,7 @@ snmp_proto_item: SNMP_CFG->bgp_local_as = $3; } | SNMP DESCRIPTION text { - if (strlen($3) > UINT32_MAX) cf_error("Description is too long"); + if (strlen($3) > UINT16_MAX - 1) cf_error("Description is too long"); SNMP_CFG->description = $3; } | PRIORITY expr { @@ -89,6 +89,7 @@ snmp_proto_item: } */ | START DELAY TIME expr_us { SNMP_CFG->startup_delay = $4; } + | VERBOSE bool { SNMP_CFG->verbose = $2; } ; snmp_proto_opts: @@ -111,6 +112,7 @@ snmp_proto_start: proto_start SNMP SNMP_CFG->local_port = 0; SNMP_CFG->remote_port = 705; SNMP_CFG->bgp_local_as = 0; + SNMP_CFG->verbose = 0; SNMP_CFG->description = "bird"; SNMP_CFG->timeout = 15; diff --git a/proto/snmp/snmp.c b/proto/snmp/snmp.c index 0c55b58b..b17c80ef 100644 --- a/proto/snmp/snmp.c +++ b/proto/snmp/snmp.c @@ -177,7 +177,7 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) switch (state) { case SNMP_INIT: - TRACE(D_EVENTS, "TODO"); + TRACE(D_EVENTS, "starting protocol"); ASSERT(last == SNMP_DOWN); proto_notify_state(&p->p, PS_START); @@ -200,7 +200,7 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) /* Fall thru */ case SNMP_LOCKED: - TRACE(D_EVENTS, "snmp %s: address lock acquired", p->p.name); + TRACE(D_EVENTS, "address lock acquired"); ASSERT(last == SNMP_INIT); sock *s = sk_new(p->pool); @@ -265,7 +265,7 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) return PS_START; case SNMP_CONN: - TRACE(D_EVENTS, "MIBs registered"); + TRACE(D_EVENTS, "MIBs registered, AgentX session established"); ASSERT(last == SNMP_REGISTER); proto_notify_state(&p->p, PS_UP); return PS_UP; @@ -297,7 +297,7 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) return PS_DOWN; default: - die("unknown snmp state transition"); + die("unknown SNMP state transition"); return PS_DOWN; } } @@ -512,6 +512,7 @@ snmp_start(struct proto *P) p->bgp_local_id = cf->bgp_local_id; p->timeout = cf->timeout; p->startup_delay = cf->startup_delay; + p->verbose = cf->verbose; p->pool = p->p.pool; p->lp = lp_new(p->pool); @@ -562,7 +563,7 @@ snmp_reconfigure_logic(struct snmp_proto *p, const struct snmp_config *new) || old->timeout != new->timeout // TODO distinguish message timemout //(Open.timeout and timeout for timer) || old->priority != new->priority - || strncmp(old->description, new->description, UINT32_MAX)); + || strncmp(old->description, new->description, UINT16_MAX - 1)); } /* @@ -587,6 +588,7 @@ snmp_reconfigure(struct proto *P, struct proto_config *CF) { /* copy possibly changed values */ p->startup_delay = new->startup_delay; + p->verbose = new->verbose; ASSERT(p->ping_timer); int active = tm_active(p->ping_timer); diff --git a/proto/snmp/snmp.h b/proto/snmp/snmp.h index 3759ccfc..d52484eb 100644 --- a/proto/snmp/snmp.h +++ b/proto/snmp/snmp.h @@ -76,6 +76,7 @@ struct snmp_config { * nonallocated parts of configs with memcpy */ //const struct oid *oid_identifier; TODO + int verbose; }; #define SNMP_BGP_P_REGISTERING 0x01 @@ -134,6 +135,8 @@ struct snmp_proto { timer *startup_timer; struct mib_tree *mib_tree; + int verbose; + u32 ignore_ping_id; }; enum agentx_mibs { diff --git a/proto/snmp/subagent.c b/proto/snmp/subagent.c index 414e04ff..66027793 100644 --- a/proto/snmp/subagent.c +++ b/proto/snmp/subagent.c @@ -411,7 +411,6 @@ close_pdu(struct snmp_proto *p, enum agentx_close_reasons reason) static uint parse_close_pdu(struct snmp_proto *p, byte * const pkt_start) { - TRACE(D_PACKETS, "SNMP received agentx-Close-PDU"); byte *pkt = pkt_start; struct agentx_close_pdu *pdu = (void *) pkt; @@ -420,7 +419,7 @@ parse_close_pdu(struct snmp_proto *p, byte * const pkt_start) if (pkt_size != sizeof(struct agentx_close_pdu)) { - TRACE(D_PACKETS, "SNMP malformed agentx-Close-PDU, closing anyway"); + TRACE(D_PACKETS, "SNMP received agentx-Close-PDU that's malformed, closing anyway"); snmp_simple_response(p, AGENTX_RES_GEN_ERROR, 0); snmp_reset(p); return 0; @@ -428,14 +427,14 @@ parse_close_pdu(struct snmp_proto *p, byte * const pkt_start) if (!snmp_test_close_reason(pdu->reason)) { - TRACE(D_PACKETS, "SNMP invalid close reason %u", pdu->reason); + TRACE(D_PACKETS, "SNMP received agentx-Close-PDU with invalid close reason %u", pdu->reason); snmp_simple_response(p, AGENTX_RES_GEN_ERROR, 0); snmp_reset(p); return 0; } enum agentx_close_reasons reason = (enum agentx_close_reasons) pdu->reason; - TRACE(D_PACKETS, "SNMP close reason %u", reason); + TRACE(D_PACKETS, "SNMP received agentx-Close-PDU with close reason %u", reason); snmp_simple_response(p, AGENTX_RES_NO_ERROR, 0); snmp_reset(p); return pkt_size + AGENTX_HEADER_SIZE; @@ -531,7 +530,7 @@ parse_sets_pdu(struct snmp_proto *p, byte * const pkt_start, enum agentx_respons if (pkt_size != 0) { - TRACE(D_PACKETS, "SNMP received malformed set PDU (size)"); + TRACE(D_PACKETS, "SNMP received PDU is malformed (size)"); snmp_simple_response(p, AGENTX_RES_PARSE_ERROR, 0); snmp_reset(p); return 0; @@ -548,7 +547,7 @@ parse_sets_pdu(struct snmp_proto *p, byte * const pkt_start, enum agentx_respons //mb_free(tr); c.error = err; - TRACE(D_PACKETS, "SNMP received set PDU with error %u", c.error); + TRACE(D_PACKETS, "SNMP received PDU parsed with error %u", c.error); response_err_ind(r, c.error, 0); sk_send(p->sock, AGENTX_HEADER_SIZE); @@ -612,7 +611,7 @@ parse_cleanup_set_pdu(struct snmp_proto *p, byte * const pkt_start) if (pkt_size != 0) { return AGENTX_HEADER_SIZE; - TRACE(D_PACKETS, "SNMP received malformed agentx-CleanupSet-PDU"); + TRACE(D_PACKETS, "SNMP received agentx-CleanupSet-PDU is malformed"); snmp_reset(p); return 0; } @@ -747,7 +746,7 @@ parse_pkt(struct snmp_proto *p, byte *pkt, uint size) default: /* We reset the connection for malformed packet (Unknown packet type) */ - TRACE(D_PACKETS, "SNMP received unknown packet type (%u)", LOAD_U8(h->type)); + TRACE(D_PACKETS, "SNMP received PDU with unknown type (%u)", LOAD_U8(h->type)); snmp_reset(p); return 0; } @@ -772,7 +771,10 @@ parse_response(struct snmp_proto *p, byte *res) switch (r->error) { case AGENTX_RES_NO_ERROR: - TRACE(D_PACKETS, "SNMP received agetnx-Response-PDU"); + if (p->verbose || LOAD_U32(h->packet_id) != p->ignore_ping_id) + TRACE(D_PACKETS, "SNMP received agentx-Response-PDU"); + if (LOAD_U32(h->packet_id) == p->ignore_ping_id) + p->ignore_ping_id = 0; do_response(p, res); break; @@ -781,6 +783,8 @@ parse_response(struct snmp_proto *p, byte *res) case AGENTX_RES_REQUEST_DENIED: case AGENTX_RES_UNKNOWN_REGISTER: TRACE(D_PACKETS, "SNMP received agentx-Response-PDU with error %u", r->error); + if (LOAD_U32(h->packet_id) == p->ignore_ping_id) + p->ignore_ping_id = 0; snmp_register_ack(p, r); break; @@ -1334,6 +1338,9 @@ snmp_ping(struct snmp_proto *p) snmp_blank_header(h, AGENTX_PING_PDU); p->packet_id++; snmp_session(p, h); + if (p->verbose) + TRACE(D_PACKETS, "SNMP sending agentx-Ping-PDU"); + p->ignore_ping_id = p->packet_id; /* sending only header */ uint s = update_packet_size(h, (byte *) h + AGENTX_HEADER_SIZE);