]> git.ipfire.org Git - thirdparty/bird.git/commitdiff
SNMP: improved debug messages, shorter description
authorVojtech Vilimek <vojtech.vilimek@nic.cz>
Wed, 14 Aug 2024 16:24:25 +0000 (18:24 +0200)
committerVojtech Vilimek <vojtech.vilimek@nic.cz>
Wed, 14 Aug 2024 16:24:25 +0000 (18:24 +0200)
proto/snmp/config.Y
proto/snmp/snmp.c
proto/snmp/snmp.h
proto/snmp/subagent.c

index a9b25d2eaf91347c911e3ee371cf110507781a32..1aeef74a919fd8a31bd3a74c8383c803c281941d 100644 (file)
@@ -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;
index 0c55b58b0ac19d596e841f10aa5f3d886d9cfa3e..b17c80ef58fb65ec8599f9b49c6838d2c7491078 100644 (file)
@@ -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);
index 3759ccfc0600ec23e0596be71bff04de89a14bc9..d52484eb86d39d8d9fba5b8c8d416b5e4391c03d 100644 (file)
@@ -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 {
index 414e04ff7152544f7599608df72456a3337181d5..66027793dd4da4b08f595276818af6dba89791e7 100644 (file)
@@ -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);