SNMP: improve AgentX registration tracking

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) reg;
-  (void)p;
+  snmp_reset(p);
 }
 
 /*
@@ -235,24 +223,15 @@ 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);
 
-    struct oid *oid = mb_allocz(p->pool,
+  struct oid *oid = mb_allocz(p->pool, sizeof(bgp4_mib_oid));
-      snmp_oid_size_from_len(ARRAY_SIZE(bgp_mib_prefix)));
+  memcpy(oid, &bgp4_mib_oid, sizeof(bgp4_mib_oid));
-    print(;
-    STORE_U8(oid->n_subid, ARRAY_SIZE(bgp_mib_prefix));
-    STORE_U8(oid->prefix, SNMP_MGMT);
 
-    // TODO use STATIC_OID
+  reg->reg_hook_ok = NULL;
-    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;
 
   /*
@@ -261,7 +240,6 @@ snmp_bgp4_register(struct snmp_proto *p)
    */
   snmp_register(p, oid, 0, 0, SNMP_REGISTER_TREE);
 }
-}
 
 static int
 snmp_bgp_valid_ip4(const struct oid *o)

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 */
 };

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
+  return (LOAD_U32(r->session_id) == h->session_id) &&
-    (r->mib == mib) &&
+    (LOAD_U32(r->transaction_id) == h->transaction_id) &&
-    (r->session_id == h->session_id) &&
+    (LOAD_U32(r->packet_id) == h->packet_id);
-    (r->transaction_id == h->transaction_id) &&
-    (r->packet_id == h->packet_id);
 }
 
 

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);

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);
+      snmp_register_ack(p, r);
-      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);
       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;
+      snmp_register_ack(p, r);
-
-      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);
       break;
 
     case SNMP_CONN: