From e10f21aa3a69f713dccbd97b96f19338317f283f Mon Sep 17 00:00:00 2001 From: Vojtech Vilimek Date: Thu, 15 Aug 2024 10:54:13 +0200 Subject: [PATCH] SNMP: logging improvements --- proto/snmp/snmp.c | 14 +++++++------- proto/snmp/subagent.c | 44 +++++++++++++++++++++++++++++-------------- 2 files changed, 37 insertions(+), 21 deletions(-) diff --git a/proto/snmp/snmp.c b/proto/snmp/snmp.c index b17c80ef..1f0127d1 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, "starting protocol"); + /* We intentionally do not log anything */ 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, "address lock acquired"); + TRACE(D_EVENTS, "SNMP Address lock acquired"); ASSERT(last == SNMP_INIT); sock *s = sk_new(p->pool); @@ -231,7 +231,7 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) /* Try opening the socket, schedule a retry on fail */ if (sk_open(s) < 0) { - TRACE(D_EVENTS, "opening of communication socket failed"); + TRACE(D_EVENTS, "SNMP Opening of communication socket failed"); rfree(s); p->sock = NULL; // TODO handle 0 timeout @@ -240,7 +240,7 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) return PS_START; case SNMP_OPEN: - TRACE(D_EVENTS, "communication socket opened, starting AgentX subagent"); + TRACE(D_EVENTS, "SNMP Communication socket opened, starting AgentX subagent"); ASSERT(last == SNMP_LOCKED); p->sock->rx_hook = snmp_rx; @@ -253,7 +253,7 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) return PS_START; case SNMP_REGISTER: - TRACE(D_EVENTS, "registering MIBs"); + TRACE(D_EVENTS, "SNMP Registering MIBs"); ASSERT(last == SNMP_OPEN); tm_stop(p->startup_timer); /* stop timeout */ @@ -273,7 +273,7 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) case SNMP_STOP: if (p->sock && p->state != SNMP_OPEN && !sk_tx_buffer_empty(p->sock)) { - TRACE(D_EVENTS, "closing AgentX session"); + TRACE(D_EVENTS, "SNMP Closing AgentX session"); if (p->state == SNMP_OPEN || p->state == SNMP_REGISTER || p->state == SNMP_CONN) snmp_stop_subagent(p); @@ -291,7 +291,7 @@ snmp_set_state(struct snmp_proto *p, enum snmp_proto_state state) /* Fall thru */ case SNMP_DOWN: - TRACE(D_EVENTS, "AgentX session closed"); + TRACE(D_EVENTS, "SNMP AgentX session closed"); snmp_cleanup(p); proto_notify_state(&p->p, PS_DOWN); return PS_DOWN; diff --git a/proto/snmp/subagent.c b/proto/snmp/subagent.c index 66027793..083ae998 100644 --- a/proto/snmp/subagent.c +++ b/proto/snmp/subagent.c @@ -35,7 +35,7 @@ static uint parse_response(struct snmp_proto *p, byte *buf); static void do_response(struct snmp_proto *p, byte *buf); static uint parse_gets_pdu(struct snmp_proto *p, byte *pkt); static struct agentx_response *prepare_response(struct snmp_proto *p, struct snmp_pdu *c); -static void response_err_ind(struct agentx_response *res, enum agentx_response_errs err, u16 ind); +static void response_err_ind(struct snmp_proto *p, struct agentx_response *res, enum agentx_response_errs err, u16 ind); static uint update_packet_size(struct agentx_header *start, byte *end); /* standard SNMP internet prefix (.1.3.6.1) */ @@ -121,7 +121,7 @@ snmp_simple_response(struct snmp_proto *p, enum agentx_response_errs error, u16 ASSUME(c.size >= sizeof(struct agentx_response)); struct agentx_response *res = prepare_response(p, &c); - response_err_ind(res, error, index); + response_err_ind(p, res, error, index); sk_send(sk, sizeof(struct agentx_response)); } @@ -139,6 +139,8 @@ 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; + TRACE(D_PACKETS, "SNMP sending agentx-Open-PDU"); + struct snmp_pdu c; snmp_pdu_context(&c, p, sk); @@ -194,7 +196,7 @@ snmp_notify_pdu(struct snmp_proto *p, struct oid *oid, void *data, uint size, in return; sock *sk = p->sock; - + TRACE(D_PACKETS, "SNMP sending agentx-Notify-PDU"); struct snmp_pdu c; snmp_pdu_context(&c, p, sk); @@ -354,6 +356,7 @@ un_register_pdu(struct snmp_proto *p, struct oid *oid, u32 bound, uint index, en void snmp_register(struct snmp_proto *p, struct oid *oid, u32 bound, uint index, u8 is_instance) { + TRACE(D_PACKETS, "SNMP sending agentx-Register-PDU"); un_register_pdu(p, oid, bound, index, AGENTX_REGISTER_PDU, is_instance); } @@ -366,9 +369,10 @@ snmp_register(struct snmp_proto *p, struct oid *oid, u32 bound, uint index, u8 i * * For more detailed description see un_register_pdu() function. */ -void +void UNUSED snmp_unregister(struct snmp_proto *p, struct oid *oid, u32 bound, uint index) { + TRACE(D_PACKETS, "SNMP sending agentx-Unregister-PDU"); un_register_pdu(p, oid, bound, index, AGENTX_UNREGISTER_PDU, 0); } @@ -384,6 +388,8 @@ close_pdu(struct snmp_proto *p, enum agentx_close_reasons reason) struct snmp_pdu c; snmp_pdu_context(&c, p, sk); + TRACE(D_PACKETS, "SNMP sending agentx-Close-PDU with reason %u", reason); + #define REASON_SIZE sizeof(u32) (void) snmp_tbuf_reserve(&c, AGENTX_HEADER_SIZE + REASON_SIZE); @@ -491,20 +497,20 @@ parse_test_set_pdu(struct snmp_proto *p, byte * const pkt_start) if (c.error != AGENTX_RES_NO_ERROR) { - response_err_ind(res, c.error, c.index + 1); + response_err_ind(p, res, c.error, c.index + 1); snmp_reset(p); } else if (all_possible) { /* All values in the agentx-TestSet-PDU are OK, realy to commit them */ - response_err_ind(res, AGENTX_RES_NO_ERROR, 0); + response_err_ind(p, res, AGENTX_RES_NO_ERROR, 0); } else { // Currently the only reachable branch - //TRACE(D_PACKETS, "SNMP SET action failed (not writable)"); + TRACE(D_PACKETS, "SNMP SET action failed (not writable)"); /* This is a recoverable error, we do not need to reset the connection */ - response_err_ind(res, AGENTX_RES_NOT_WRITABLE, c.index + 1); + response_err_ind(p, res, AGENTX_RES_NOT_WRITABLE, c.index + 1); } sk_send(sk, s); @@ -548,7 +554,7 @@ parse_sets_pdu(struct snmp_proto *p, byte * const pkt_start, enum agentx_respons c.error = err; TRACE(D_PACKETS, "SNMP received PDU parsed with error %u", c.error); - response_err_ind(r, c.error, 0); + response_err_ind(p, r, c.error, 0); sk_send(p->sock, AGENTX_HEADER_SIZE); /* Reset the connection on unrecoverable error */ @@ -725,8 +731,15 @@ parse_pkt(struct snmp_proto *p, byte *pkt, uint size) switch (LOAD_U8(h->type)) { case AGENTX_GET_PDU: + TRACE(D_PACKETS, "SNMP received agentx-Get-PDU"); + return parse_gets_pdu(p, pkt); + case AGENTX_GET_NEXT_PDU: + TRACE(D_PACKETS, "SNMP received agentx-GetNext-PDU"); + return parse_gets_pdu(p, pkt); + case AGENTX_GET_BULK_PDU: + TRACE(D_PACKETS, "SNMP received agentx-GetBulk-PDU"); return parse_gets_pdu(p, pkt); case AGENTX_CLOSE_PDU: @@ -800,7 +813,7 @@ parse_response(struct snmp_proto *p, byte *res) case AGENTX_RES_PARSE_ERROR: case AGENTX_RES_PROCESSING_ERR: default: - TRACE(D_PACKETS, "SNMP agentx-Response-PDU with unexepected error %u", r->error); + TRACE(D_PACKETS, "SNMP received agentx-Response-PDU with unexepected error %u", r->error); snmp_reset(p); break; } @@ -965,6 +978,7 @@ update_packet_size(struct agentx_header *start, byte *end) /* * response_err_ind - update response error and index + * @p: SNMP protocol instance * @res: response PDU header * @err: error status * @ind: index of error, ignored for noAgentXError @@ -973,12 +987,13 @@ update_packet_size(struct agentx_header *start, byte *end) * error is not noError, also set the corrent response PDU payload size. */ static inline void -response_err_ind(struct agentx_response *res, enum agentx_response_errs err, u16 ind) +response_err_ind(struct snmp_proto *p, struct agentx_response *res, enum agentx_response_errs err, u16 ind) { STORE_U16(res->error, (u16) err); if (err != AGENTX_RES_NO_ERROR && err != AGENTX_RES_GEN_ERROR) { - //TRACE(D_PACKETS, "Last PDU resulted in error %u", err); + if (p->verbose) + TRACE(D_PACKETS, "SNMP last PDU resulted in error %u", err); STORE_U16(res->index, ind); /* Reset VarBindList to null */ STORE_U32(res->h.payload, @@ -986,7 +1001,8 @@ response_err_ind(struct agentx_response *res, enum agentx_response_errs err, u16 } else if (err == AGENTX_RES_GEN_ERROR) { - //TRACE(D_PACKETS, "Last PDU resulted in error %u genErr", err); + if (p->verbose) + TRACE(D_PACKETS, "SNMP last PDU resulted in error genErr"); STORE_U16(res->index, 0); /* Reset VarBindList to null */ STORE_U32(res->h.payload, @@ -1215,7 +1231,7 @@ parse_gets_pdu(struct snmp_proto *p, byte * const pkt_start) #endif /* We update the error, index pair on the beginning of the packet. */ - response_err_ind(response_header, c.error, c.index + 1); + response_err_ind(p, response_header, c.error, c.index + 1); uint s = update_packet_size(&response_header->h, c.buffer); /* We send the message in TX buffer. */