SNMP: Improvements in registration handling

proto/snmp/bgp_mib.c

@@ -195,6 +195,23 @@ snmp_bgp_state(const struct oid *oid)
   return state;
 }
 
+void
+snmp_bgp_reg_ok(struct snmp_proto *p, struct agentx_response *r, struct oid *oid)
+{
+  const struct oid *in_buf = ((void *) r) + sizeof(r);
+  int byte_ord = r->h.flags & AGENTX_NETWORK_BYTE_ORDER;
+  struct oid *dup = snmp_prefixize(p, in_buf, byte_ord);
+
+  ASSUME(snmp_bgp_state(oid) == snmp_bgp_state(dup));
+  mb_free(dup);
+}
+
+void
+snmp_bgp_reg_failed(struct snmp_proto *p, struct agentx_response UNUSED *r, struct oid UNUSED *oid)
+{
+  snmp_stop_subagent(p);
+}
+
 static void
 snmp_bgp_notify_common(struct snmp_proto *p, uint type, ip4_addr ip4, char last_error[], uint state_val)
 {
@@ -315,15 +332,13 @@ snmp_bgp_register(struct snmp_proto *p)
     /* Register the whole BGP4-MIB::bgp root tree node */
     struct snmp_register *registering = snmp_register_create(p, SNMP_BGP4_MIB);
 
-    struct oid *oid = mb_alloc(p->pool, snmp_oid_sizeof(2));
+    struct oid *oid = mb_alloc(p->pool,
-    STORE_U8(oid->n_subid, 2);
+      snmp_oid_sizeof(ARRAY_SIZE(bgp_mib_prefix)));
+    STORE_U8(oid->n_subid, ARRAY_SIZE(bgp_mib_prefix));
     STORE_U8(oid->prefix, SNMP_MGMT);
 
-    memcpy(oid->ids, bgp_mib_prefix, 2 * sizeof(u32));
+    memcpy(oid->ids, bgp_mib_prefix, sizeof(bgp_mib_prefix));
-
     registering->oid = oid;
-    add_tail(&p->register_queue, &registering->n);
-    p->register_to_ack++;
 
     /* snmp_register(struct snmp_proto *p, struct oid *oid, uint index, uint len, u8 is_instance, uint contid) */
     snmp_register(p, oid, 1, 0, SNMP_REGISTER_TREE, SNMP_DEFAULT_CONTEXT);

proto/snmp/bgp_mib.h

@@ -39,6 +39,8 @@ enum BGP4_MIB_PEER_TABLE {
 struct oid;
 
 void snmp_bgp_register(struct snmp_proto *p);
+void snmp_bgp_reg_ok(struct snmp_proto *p, struct agentx_response *r, struct oid *oid);
+void snmp_bgp_reg_failed(struct snmp_proto *p, struct agentx_response *r, struct oid *oid);
 
 u8 snmp_bgp_get_valid(u8 state);
 u8 snmp_bgp_getnext_valid(u8 state);

proto/snmp/snmp.c

@@ -225,9 +225,14 @@ snmp_start_locked(struct object_lock *lock)
 {
   struct snmp_proto *p = lock->data;
 
+  snmp_log("changing state to LOCKED");
   p->state = SNMP_LOCKED;
   sock *s = p->sock;
 
+  p->to_send = 0;
+  p->errs = 0;
+
+
   if (!p->bgp_trie)
     p->bgp_trie = f_new_trie(p->lp, 0);  // TODO user-data attachment size
 
@@ -348,9 +353,6 @@ snmp_start(struct proto *P)
   struct snmp_proto *p = (void *) P;
   struct snmp_config *cf = (struct snmp_config *) P->cf;
 
-  p->to_send = 0;
-  p->errs = 0;
-
   p->startup_timer = tm_new_init(p->pool, snmp_startup_timeout, p, 0, 0);
   p->ping_timer = tm_new_init(p->pool, snmp_ping_timeout, p, 0, 0);
 

proto/snmp/snmp_utils.c

@@ -416,6 +416,8 @@ snmp_register_create(struct snmp_proto *p, u8 mib_class)
 
   r->mib_class = mib_class;
 
+  add_tail(&p->register_queue, &r->n);
+
   return r;
 }
 

proto/snmp/subagent.c

@@ -77,14 +77,29 @@ static const char * const snmp_pkt_type[] UNUSED = {
   [AGENTX_RESPONSE_PDU]		  =  "Response-PDU",
 };
 
+
+static void
+snmp_register_ok(struct snmp_proto *p, struct agentx_response *res, struct oid *oid, u8 UNUSED class)
+{
+  // TODO switch based on oid type
+  snmp_bgp_reg_ok(p, res, oid);
+}
+
+static void
+snmp_register_failed(struct snmp_proto *p, struct agentx_response *res, struct oid *oid, u8 UNUSED class)
+{
+  // TODO switch based on oid type
+  snmp_bgp_reg_failed(p, res, oid);
+}
+
 void
-snmp_register_ack(struct snmp_proto *p, struct agentx_header *h, u8 class)
+snmp_register_ack(struct snmp_proto *p, struct agentx_response *res, u8 class)
 {
   struct snmp_register *reg;
   WALK_LIST(reg, p->register_queue)
   {
     // TODO add support for more mib trees (other than BGP)
-    if (snmp_register_same(reg, h, class))
+    if (snmp_register_same(reg, &res->h, class))
     {
       struct snmp_registered_oid *ro = \
 	 mb_alloc(p->p.pool, sizeof(struct snmp_registered_oid));
@@ -98,6 +113,11 @@ snmp_register_ack(struct snmp_proto *p, struct agentx_header *h, u8 class)
       p->register_to_ack--;
 
       add_tail(&p->bgp_registered, &ro->n);
+
+      if (res->error == AGENTX_RES_NO_ERROR)
+	snmp_register_ok(p, res, ro->oid, class);
+      else
+	snmp_register_failed(p, res, ro->oid, class);
       return;
     }
   }
@@ -373,7 +393,7 @@ parse_close_pdu(struct snmp_proto *p, byte * const pkt_start, uint size)
     static int snmp_testset(struct snmp_proto *p, const struct agentx_varbind *vb, uint pkt_size);
  */
 /* return 1 if the value could be set */
-static int
+static int UNUSED
 snmp_testset(struct snmp_proto *p, const struct agentx_varbind *vb, struct oid *oid, uint pkt_size)
 {
   /* Hard-coded no support for writing */
@@ -496,6 +516,9 @@ parse_test_set_pdu(struct snmp_proto *p, byte * const pkt_start, uint size)
   ADVANCE(pkt, size, AGENTX_HEADER_SIZE);
   uint pkt_size = LOAD_U32(h->payload, h->flags & AGENTX_NETWORK_BYTE_ORDER);
 
+  if (pkt_size < size)
+    return 0;
+
   sock *sk = p->sock;
   struct snmp_pdu c = SNMP_PDU_CONTEXT(sk);
   c.byte_ord = 0; /* use little-endian */
@@ -747,24 +770,6 @@ parse_pkt(struct snmp_proto *p, byte *pkt, uint size, uint *skip)
   }
 }
 
-static void
-snmp_register_ok(struct snmp_proto *p, struct agentx_response *r, uint size, u8 type)
-{
-  (void)p;(void)r;(void)size;(void)type;
-}
-
-static void
-snmp_register_failed(struct snmp_proto *p, struct agentx_response *r, uint size, u8 type)
-{
-  (void)p;(void)r;(void)size;(void)type;
-}
-
-static void
-unsupported_context(struct snmp_proto *p, struct agentx_response *r, uint size)
-{
-  (void)p;(void)r;(void)size;
-  // TODO unsupported_context
-}
 
 static uint
 parse_response(struct snmp_proto *p, byte *res, uint size)
@@ -814,17 +819,15 @@ parse_response(struct snmp_proto *p, byte *res, uint size)
     /* Registration errors */
     case AGENTX_RES_DUPLICATE_REGISTER:
     case AGENTX_RES_REQUEST_DENIED:
-      snmp_register_failed(p, r, size, h->type);
+      // TODO: more direct path to mib-specifiec code
-      break;
+      snmp_register_ack(p, r, size);
-
-    case AGENTX_RES_UNSUPPORTED_CONTEXT:
-      unsupported_context(p, r, size);
       break;
 
     /* We are trying to unregister a MIB, the unknownRegistration has same
      * effect as success */
     case AGENTX_RES_UNKNOWN_REGISTER:
     case AGENTX_RES_UNKNOWN_AGENT_CAPS:
+    case AGENTX_RES_UNSUPPORTED_CONTEXT:
     case AGENTX_RES_PARSE_ERROR:
     case AGENTX_RES_PROCESSING_ERR:
     default:
@@ -878,7 +881,7 @@ do_response(struct snmp_proto *p, byte *buf, uint size)
 
       const struct oid *oid = (void *) pkt;
 
-      snmp_register_ack(p, h, snmp_get_mib_class(oid));
+      snmp_register_ack(p, r, snmp_get_mib_class(oid));
 
       if (p->register_to_ack == 0)
       {