SNMP: improve AgentX registration tracking

diff --git a/proto/snmp/bgp4_mib.c b/proto/snmp/bgp4_mib.c
index d81d0432a4995d6d475c55e6a6b67a21b5e4351f..df6a40aaf8e6bfc3fde3a18bb7f2d4378a8e5784 100644 (file)
--- a/proto/snmp/bgp4_mib.c
+++ b/proto/snmp/bgp4_mib.c
@@ -59,24 +59,12 @@ snmp_bgp_last_error(const struct bgp_proto *bgp, char err[2])
   err[1] = bgp->last_error_code & 0x000000FF;
 }
 
-static void
-snmp_bgp_reg_ok(struct snmp_proto *p, const struct agentx_response *res, struct snmp_registration *reg)
-{
-  /* TODO(contexts): add meaningful action */
-  const struct oid * const oid = reg->oid;
-  (void)oid;
-  (void)p;
-  (void) res;
-}
-
 static void
 snmp_bgp_reg_failed(struct snmp_proto *p, const struct agentx_response *res, struct snmp_registration *reg)
 {
-  /* TODO(contexts): add meaningful action */
-  const struct oid * const oid = reg->oid;
   (void) res;
-  (void)oid;
-  (void)p;
+  (void) reg;
+  snmp_reset(p);
 }
 
 /*
@@ -236,31 +224,21 @@ snmp_bgp_notify_backward_trans(struct snmp_proto *p, struct bgp_proto *bgp)
 void
 snmp_bgp4_register(struct snmp_proto *p)
 {
-  u32 bgp_mib_prefix[] = { SNMP_MIB_2, SNMP_BGP4_MIB };
+  /* Register the whole BGP4-MIB::bgp root tree node */
+  struct snmp_registration *reg;
+  reg = snmp_registration_create(p, BGP4_MIB_ID);
 
-  {
-    /* Register the whole BGP4-MIB::bgp root tree node */
-    struct snmp_registration *reg;
-    reg = snmp_registration_create(p, BGP4_MIB_ID);
-
-    struct oid *oid = mb_allocz(p->pool,
-      snmp_oid_size_from_len(ARRAY_SIZE(bgp_mib_prefix)));
-    print(;
-    STORE_U8(oid->n_subid, ARRAY_SIZE(bgp_mib_prefix));
-    STORE_U8(oid->prefix, SNMP_MGMT);
-
-    // TODO use STATIC_OID
-    memcpy(oid->ids, bgp_mib_prefix, sizeof(bgp_mib_prefix));
-    reg->oid = oid;
-    reg->reg_hook_ok = snmp_bgp_reg_ok;
-    reg->reg_hook_fail = snmp_bgp_reg_failed;
-
-    /*
-     * We set both upper bound and index to zero, therefore only single OID
-     * is being registered.
-     */
-    snmp_register(p, oid, 0, 0, SNMP_REGISTER_TREE);
-  }
+  struct oid *oid = mb_allocz(p->pool, sizeof(bgp4_mib_oid));
+  memcpy(oid, &bgp4_mib_oid, sizeof(bgp4_mib_oid));
+
+  reg->reg_hook_ok = NULL;
+  reg->reg_hook_fail = snmp_bgp_reg_failed;
+
+  /*
+   * We set both upper bound and index to zero, therefore only single OID
+   * is being registered.
+   */
+  snmp_register(p, oid, 0, 0, SNMP_REGISTER_TREE);
 }
 
 static int

diff --git a/proto/snmp/snmp.h b/proto/snmp/snmp.h
index 26dcf2a5185f28453ae1b170cbeee8b7492d0e89..83f3b5bf1e181cfb31c7ad61eec858b148ba8be5 100644 (file)
--- a/proto/snmp/snmp.h
+++ b/proto/snmp/snmp.h
@@ -121,7 +121,6 @@ struct snmp_proto {
   u32 transaction_id;
   u32 packet_id;
 
-  uint registrations_to_ack;               /* counter of pending responses to register-pdu */
   list registration_queue;                 /* list containing snmp_register records */
 
   // map
@@ -158,7 +157,6 @@ struct snmp_registration {
   u32 session_id;
   u32 transaction_id;
   u32 packet_id;
-  struct oid *oid;
   snmp_reg_hook_t reg_hook_ok; /* hook called when successful response to OID registration is recieved */
   snmp_reg_hook_t reg_hook_fail; /* hook called when OID registration fail */
 };

diff --git a/proto/snmp/snmp_utils.c b/proto/snmp/snmp_utils.c
index 6187eaba3bef82dc35f9a4a716f60ce228fbb25f..51cdb6b6d6ebd6cac1a13cf94e61fbc0fe4d6391 100644 (file)
--- a/proto/snmp/snmp_utils.c
+++ b/proto/snmp/snmp_utils.c
@@ -665,13 +665,11 @@ snmp_registration_create(struct snmp_proto *p, enum agentx_mibs mib)
 }
 
 int
-snmp_registration_match(struct snmp_registration *r, struct agentx_header *h, enum agentx_mibs mib)
+snmp_registration_match(struct snmp_registration *r, struct agentx_header *h)
 {
-  return
-    (r->mib == mib) &&
-    (r->session_id == h->session_id) &&
-    (r->transaction_id == h->transaction_id) &&
-    (r->packet_id == h->packet_id);
+  return (LOAD_U32(r->session_id) == h->session_id) &&
+    (LOAD_U32(r->transaction_id) == h->transaction_id) &&
+    (LOAD_U32(r->packet_id) == h->packet_id);
 }
 
 

diff --git a/proto/snmp/snmp_utils.h b/proto/snmp/snmp_utils.h
index 41ec42c4969135ebce93212cdcf8284546267aba..5455f0d6fe105e85062e4bede740f24b3d34c457 100644 (file)
--- a/proto/snmp/snmp_utils.h
+++ b/proto/snmp/snmp_utils.h
@@ -96,7 +96,7 @@ byte *snmp_put_fbyte(byte *buf, u8 data);
  *
  */
 struct snmp_registration *snmp_registration_create(struct snmp_proto *p, enum agentx_mibs mib);
-int snmp_registration_match(struct snmp_registration *r, struct agentx_header *h, enum agentx_mibs mib);
+int snmp_registration_match(struct snmp_registration *r, struct agentx_header *h);
 
 void snmp_dump_packet(byte *pkt, uint size);
 void snmp_oid_dump(const struct oid *oid);

diff --git a/proto/snmp/subagent.c b/proto/snmp/subagent.c
index 7e5ebcf098da328294e7b2382f38c42f504a0956..30305bd9f22260cf53c8aee4b8cda4851a9316df 100644 (file)
--- a/proto/snmp/subagent.c
+++ b/proto/snmp/subagent.c
@@ -80,28 +80,27 @@ snmp_blank_header(struct agentx_header *h, enum agentx_pdu_types type)
  * @oid: MIB subtree Object Identifier in cpu native byte order
  */
 void
-snmp_register_ack(struct snmp_proto *p, struct agentx_response *res, const struct oid *oid)
+snmp_register_ack(struct snmp_proto *p, struct agentx_response *res)
 {
-  enum agentx_mibs mib = agentx_get_mib(oid);
-
   struct snmp_registration *reg;
   WALK_LIST(reg, p->registration_queue)
   {
-    if (snmp_registration_match(reg, &res->h, mib))
+    if (snmp_registration_match(reg, &res->h))
     {
       rem_node(&reg->n);
-      p->registrations_to_ack--;
 
-      if (res->error == AGENTX_RES_NO_ERROR)
+      if (res->error == AGENTX_RES_NO_ERROR && reg->reg_hook_ok)
        reg->reg_hook_ok(p, res, reg);
-      else
+      else if (res->error != AGENTX_RES_NO_ERROR && reg->reg_hook_fail)
        reg->reg_hook_fail(p, res, reg);
 
-      mb_free(reg->oid);
       mb_free(reg);
-      return;
+      break;
     }
   }
+
+  if (EMPTY_LIST(p->registration_queue))
+    snmp_up(p);
 }
 
 /*
@@ -784,12 +783,7 @@ parse_response(struct snmp_proto *p, byte *res)
     case AGENTX_RES_UNKNOWN_REGISTER:
       // TODO more direct path to mib-specific code
       TRACE(D_PACKETS, "SNMP received agentx-Response-PDU with error %u", r->error);
-      byte *pkt = res + sizeof(struct agentx_response);
-      const struct oid *failed_buf = (struct oid *) pkt;
-      uint failed_size = snmp_oid_size(failed_buf);
-      struct oid *failed = tmp_alloc(failed_size);
-      snmp_oid_from_buf(failed, failed_buf);
-      snmp_register_ack(p, r, failed);
+      snmp_register_ack(p, r);
       break;
 
     /*
@@ -820,6 +814,7 @@ void
 snmp_register_mibs(struct snmp_proto *p)
 {
   snmp_bgp4_register(p);
+  ASSUME(!EMPTY_LIST(p->registration_queue));
 }
 
 /*
@@ -856,16 +851,7 @@ do_response(struct snmp_proto *p, byte *pkt)
       break;
 
     case SNMP_REGISTER:;
-      pkt += AGENTX_HEADER_SIZE;
-
-      const struct oid *oid_buf = (struct oid *) pkt;
-      uint oid_size = snmp_oid_size(oid_buf);
-      struct oid *oid = tmp_alloc(oid_size);
-      snmp_oid_from_buf(oid, oid_buf);
-      snmp_register_ack(p, r, oid);
-
-      if (p->registrations_to_ack == 0)
-       snmp_set_state(p, SNMP_CONN);
+      snmp_register_ack(p, r);
       break;
 
     case SNMP_CONN: