SNMP: tmp ``microimprovement'' flush worktree

proto/snmp/bgp4_mib.h

@@ -1,29 +1,10 @@
 #ifndef _BIRD_SNMP_BGP4_MIB_H_
 #define _BIRD_SNMP_BGP4_MIB_H_
 
-#ifdef _BIRD_SNMP_SUBAGENT_H_
-#define BIRD_SNMP_BGP4_SKIP
-#endif
-
 #include "snmp.h"
 #include "proto/bgp/bgp.h"
-
-void snmp_bgp4_register(struct snmp_proto *p);
-
-struct bgp4_mib {
-  enum snmp_tags tag; /* always BGP4_MIB, see subagent.h for more details */
-
-  ip4_addr addr;
-  const struct bgp_proto *bgp_proto;
-  const struct bgp_conn *bgp_conn;
-  const struct bgp_stats *bgp_stats;
-  const struct bgp_config *bgp_conf;
-};
-
 #include "subagent.h"
 
-#ifndef BIRD_SNMP_BGP4_SKIP
-
 #define BGP4_MIB 15
 
 /* peers attributes */
@@ -63,6 +44,8 @@ enum bgp4_mib_peer_entry_row {
 u8 snmp_bgp_get_valid(u8 state);
 u8 snmp_bgp_getnext_valid(u8 state);
 
+void snmp_bgp4_register(struct snmp_proto *p);
+
 enum snmp_search_res snmp_bgp_search(struct snmp_proto *p, struct agentx_varbind **vb_search, const struct oid *o_end, struct snmp_pdu *c);
 enum snmp_search_res snmp_bgp_search2(struct snmp_proto *p, struct oid **searched, const struct oid *o_end, uint contid);
 void snmp_bgp_fill(struct snmp_proto *p, struct agentx_varbind **vb, struct snmp_pdu *c);
@@ -142,5 +125,3 @@ STATIC_ASSERT(BGP4_MIB_ESTABLISHED == BS_ESTABLISHED + 1);
 #define BGP4_MIB_BACKWARD_TRANS_NOTIFICATION 2
 
 #endif
-
-#endif

proto/snmp/mib_tree.c

@@ -57,6 +57,27 @@ mib_tree_init(pool *p, struct mib_tree *t)
   (void) mib_tree_add(p, t, oid, 0);
 }
 
+int
+mib_tree_hint(pool *p, struct mib_tree *t, const struct oid *oid, uint size)
+{
+  mib_node_u *node = mib_tree_add(p, t, oid, 0);
+  if (!node || mib_node_is_leaf(node))
+    return 0;
+
+  struct mib_node *inner = &node->inner;
+  if (inner->child_len >= size + 1)
+    return 1;
+
+  u32 old_len = inner->child_len;
+  inner->child_len = size + 1;
+  inner->children = realloc(inner->children,
+    inner->child_len * sizeof(mib_node_u *));
+
+  for (u32 i = old_len; i < inner->child_len; i++)
+    inner->children[i] = NULL;
+  return 1;
+}
+
 
 // TODO: This function does not work with leaf nodes inside the snmp_internet prefix
 // area
@@ -602,6 +623,61 @@ mib_tree_walk_to_oid(const struct mib_walk_state *walk, struct oid *result, u32
   return 0;
 }
 
+/**
+ * mib_tree_walk_is_oid_descendant - check if OID is in walk subtree
+ * @walk: MIB tree walk state
+ * @oid: OID to use
+ *
+ * Return 0 if @walk specify same path in MIB tree as @oid, return +1 if @oid is
+ * in @walk subtree, return -1 otherwise.
+ */
+int
+mib_tree_walk_is_oid_descendant(const struct mib_walk_state *walk, const struct oid *oid)
+{
+  /* walk stack index skipped zero prefix and OID subidentifier index */
+  u32 i = 1, j = 0;
+
+  if (!walk->stack_pos && snmp_is_oid_empty(oid))
+    return 0;
+
+  if (snmp_oid_is_prefixed(oid))
+  {
+    for (; i < MIN(walk->stack_pos - 1, ARRAY_SIZE(snmp_internet) + 1); i++)
+    {
+      if (walk->stack[i]->empty.id != snmp_internet[i - 1])
+	return -1;
+    }
+
+    if (i == walk->stack_pos)
+      return +1;
+
+    if (i < walk->stack_pos &&
+	walk->stack[i]->empty.id != (u32)LOAD_U8(oid->prefix))
+      return -1;
+
+    i++;
+  }
+
+  u32 ids = LOAD_U8(oid->n_subid);
+  for (; i < walk->stack_pos && j < ids; i++, j++)
+  {
+    if (walk->stack[i]->empty.id != LOAD_U32(oid->ids[j]))
+      return -1;
+  }
+
+  if (i < walk->stack_pos)
+    return -1;
+  else if (i == walk->stack_pos && j == ids)
+    return 0;
+  else if (i == walk->stack_pos)
+    return +1;
+  else
+  {
+    die("unreachable");
+    return -1;
+  }
+}
+
 mib_node_u *
 mib_tree_walk_next(const struct mib_tree *t, struct mib_walk_state *walk)
 {
@@ -656,7 +732,10 @@ mib_tree_walk_next_leaf(const struct mib_tree *t, struct mib_walk_state *walk)
   (void)t;
 
   if (walk->stack_pos == 0)
+  {
+    snmp_log("walk next leaf no leafs");
     return NULL;
+  }
 
   u32 next_id = 0;
   mib_node_u *node = walk->stack[walk->stack_pos - 1];
@@ -671,6 +750,7 @@ mib_tree_walk_next_leaf(const struct mib_tree *t, struct mib_walk_state *walk)
   {
     /* walk->stack_pos == 1, so we NULL out the last stack field */
     walk->stack[--walk->stack_pos] = NULL;
+    snmp_log("walk next leaf single leaf");
     return NULL;
   }
 
@@ -680,7 +760,10 @@ continue_while:
     node = walk->stack[walk->stack_pos - 1];
 
     if (mib_node_is_leaf(node))
+    {
+      snmp_log("walk next leaf %p at level %u", node, walk->stack_pos - 1);
       return (struct mib_leaf *) node;
+    }
 
     struct mib_node *node_inner = &node->inner;
     for (u32 id = next_id; id < node_inner->child_len; id++)
@@ -700,6 +783,7 @@ continue_while:
     walk->stack[--walk->stack_pos] = NULL;
   }
 
+  snmp_log("walk next leaf no more leafs");
   return NULL;
 }
 

proto/snmp/mib_tree.h

@@ -26,12 +26,52 @@ struct mib_node {
 
 struct mib_walk_state;
 
+//typedef enum snmp_search_res (*snmp_filler_hook_t)(struct mib_walk_state *state, struct snmp_data *data);
+
 struct mib_leaf {
   struct mib_node_core c;
-  enum snmp_search_res (*filler)(struct snmp_proto *p, struct snmp_pdu *c);
+
-  //enum snmp_search_res (*filler)(struct snmp_proto_pdu *pc, struct agentx_varbind **vb);
+  /**
-  int (*call_next)(struct snmp_proto *p, struct snmp_pdu *c, struct mib_walk_state *state);
+   * filler - hook for filling VarBind data value
+   * @state: self referencing MIB tree walk state
+   * @data: box holding destiantion VarBind and SNMP protocol instance
+   *
+   * If corresponding leaf node has filled in AgentX type and/or size, it is
+   * guaranteed that PDU buffer have enough space. Hook mustn't be NULL.
+   * If the leaf node has set valid type, the varbind type will be automatically
+   * set by the snmp_walk_fill() servicing routine. If the field type is set to
+   * AGENTX_INVALID, it is expected that filler() hook will also fill
+   * the VarBind type.
+   */
+  enum snmp_search_res (*filler)(struct mib_walk_state *state, struct snmp_data *data);
+
+  /**
+   * call_next - signal multileaf
+   * @state: self referencing MIB tree walk state
+   * @data: box holding destination VarBind and SNMP protocol insntace
+   *
+   * MIB modules can implement subtrees by a single leaf node in MIB node tree.
+   * When the tree is walked, the specific leaf node has to be returned multiple
+   * times. The @call_next hook determines if we should move to next leaf node.
+   * It is expected that call_next() hook may change the VarBind to be filled.
+   *
+   * Hook may be NULL meaning the leaf node is not multileaf/subtree.
+   *
+   */
+  int (*call_next)(struct mib_walk_state *state, struct snmp_data *data);
+
+  /**
+   * type of produced VarBind, may be replaced in packet instanciation by
+   * AGENTX_NO_SUCH_OBJECT, AGENTX_NO_SUCH_INSTANCE or AGENTX_END_OF_MIB_VIEW
+   * The field is unspecified if equal to AGENTX_INVALID.
+   */
   enum agentx_type type;
+
+  /*
+   * Specify upper bound of VarBind data size. If set to -1, all handling must
+   * be done in filler() hook. In all other cases the filler() hook has
+   * guaranteed that the space is available.
+   */
   int size;
 };
 
@@ -70,6 +110,9 @@ mib_node_u *mib_tree_find(const struct mib_tree *tree, struct mib_walk_state *wa
 mib_node_u *mib_tree_walk_next(const struct mib_tree *t, struct mib_walk_state *walk);
 struct mib_leaf *mib_tree_walk_next_leaf(const struct mib_tree *t, struct mib_walk_state *walk);
 
+int mib_tree_hint(pool *p, struct mib_tree *t, const struct oid *oid, uint size);
+int mib_tree_walk_is_oid_descendant(const struct mib_walk_state *walk, const struct oid *oid);
+
 static inline int
 mib_node_is_leaf(const mib_node_u *node)
 {

proto/snmp/snmp.c

@@ -117,6 +117,7 @@
 #include "snmp.h"
 #include "subagent.h"
 #include "snmp_utils.h"
+#include "mib_tree.h"
 
 static void snmp_start_locked(struct object_lock *lock);
 static void snmp_sock_err(sock *sk, int err);
@@ -493,6 +494,7 @@ snmp_start(struct proto *P)
 
   p->pool = p->p.pool;
   p->lp = lp_new(p->pool);
+  p->mib_tree = mb_alloc(p->pool, sizeof(struct mib_tree));
   p->bgp_trie = f_new_trie(p->lp, 0);
 
   p->startup_timer = tm_new_init(p->pool, snmp_startup_timeout, p, 0, 0);
@@ -502,7 +504,9 @@ snmp_start(struct proto *P)
 
   /* We create copy of bonds to BGP protocols. */
   HASH_INIT(p->bgp_hash, p->pool, 10);
-  snmp_bgp_start(p);
+
+  mib_tree_init(p->pool, p->mib_tree);
+  snmp_bgp4_start(p);
 
   return snmp_set_state(p, SNMP_INIT);
 }
@@ -561,7 +565,7 @@ snmp_reconfigure(struct proto *P, struct proto_config *CF)
   {
     /* Reinitialize the hash after snmp_shutdown() */
     HASH_INIT(p->bgp_hash, p->pool, 10);
-    snmp_bgp_start(p);
+    snmp_bgp4_start(p);
   }
 
   return config_changed;

proto/snmp/snmp.h

@@ -28,7 +28,7 @@
 
 #define SNMP_RX_BUFFER_SIZE 8192
 #define SNMP_TX_BUFFER_SIZE 8192
-#define SNMP_PKT_SIZE_MAX 8192
+#define SNMP_PKT_SIZE_MAX 4098
 
 enum snmp_proto_state {
   SNMP_DOWN = 0,
@@ -41,11 +41,6 @@ enum snmp_proto_state {
   SNMP_RESET,
 };
 
-enum snmp_tags {
-  EMPTY_TAG = 0,
-  BGP4_TAG,
-};
-
 struct snmp_bond {
   node n;
   struct proto_config *config;
@@ -90,8 +85,6 @@ struct snmp_registered_oid {
   struct oid *oid;
 };
 
-struct mib_tree;      /* see mib_tree.h */
-
 struct snmp_proto {
   struct proto p;
   struct object_lock *lock;

proto/snmp/snmp_test.c

@@ -33,7 +33,7 @@
 static int t_oid_empty(void);
 static int t_oid_compare(void);
 static int t_oid_prefixize(void);
-static int t_walk_to_oid(void);
+static int t_walk_oid_desc(void);
 static int t_tree_find(void);
 static int t_tree_traversal(void);
 static int t_tree_leafs(void);
@@ -45,6 +45,7 @@ static int t_tree_delete(void);
 #define SMALL_TESTS_NUM 10
 static int tree_sizes[] = { 0, 1, 10, 100, 1000 };
 
+/* smaller than theoretical maximum (2^32) to fit in memory */
 #define OID_MAX_ID 16
 
 #define SNMP_EXPECTED(actual, expected) \
@@ -183,8 +184,6 @@ random_oid(void)
     return random_prefixable_oid();
 }
 
-
-
 static int
 t_oid_empty(void)
 {
@@ -603,6 +602,7 @@ t_walk_to_oid(void)
   return 1;
 }
 
+
 static void
 test_both(void *buffer, uint size, const struct oid *left, const struct oid
 *right, const struct oid *expected)
@@ -735,6 +735,121 @@ generate_oids(struct oid *oids[], struct oid *sorted[], int size, struct oid *(*
   return (size > 1) ? last_used + 1 : size;
 }
 
+static int
+t_walk_oid_desc(void)
+{
+  lp_state tmps;
+  lp_save(tmp_linpool, &tmps);
+
+  pool *pool = &root_pool;
+
+  struct mib_tree storage, *tree = &storage;
+  mib_tree_init(pool, tree);
+
+  STATIC_ASSERT(ARRAY_SIZE(tree_sizes) > 0);
+  int size = tree_sizes[ARRAY_SIZE(tree_sizes) - 1];
+  ASSERT(size > 0);
+  struct oid **oids = mb_alloc(pool, size * sizeof(struct oid *));
+  struct oid **sorted = mb_alloc(pool, size * sizeof(struct oid *));
+
+  (void) generate_oids(oids, sorted, size, random_oid);
+
+  for (int i = 0; i < size; i++)
+    (void) mib_tree_add(pool, tree, oids[i], 0);
+
+  for (int test = 0; test < size; test++)
+  {
+    int i = xrandom(size);
+
+    char buffer[1024];
+    struct oid *oid = (void *) buffer;
+
+    memcpy(buffer, oids[i], snmp_oid_size(oids[i]));
+
+    struct mib_walk_state walk;
+    mib_tree_walk_init(&walk, NULL);
+    (void) mib_tree_find(tree, &walk, oid);
+
+    int type = xrandom(4);
+    switch (type)
+    {
+      case 0:
+	bt_assert(mib_tree_walk_is_oid_descendant(&walk, oids[i]) == 0);
+	break;
+
+      case 1:
+      {
+	/* oid is longer than walk or has same length */
+	u8 ids = LOAD_U8(oid->n_subid);
+	u32 upto = MIN(OID_MAX_LEN - ids, 16);
+
+	if (!upto)
+	  continue;
+
+	u32 new = xrandom(upto) + 1;
+	STORE_U8(oid->n_subid, ids + new);
+	ASSERT(snmp_oid_size(oid) < 1024);
+
+	for (u32 i = 0; i < new; i++)
+	  STORE_U32(oid->ids[ids + i], xrandom(OID_MAX_ID));
+
+	bt_assert(mib_tree_walk_is_oid_descendant(&walk, oid) > 0);
+
+	break;
+      }
+      case 2:
+      case 3:
+      {
+	/* oid is shorter than walk */
+	u8 ids = LOAD_U8(oid->n_subid);
+
+	if (ids == 0 || ids == OID_MAX_LEN)
+	  continue;
+
+	u32 split = (ids > 1) ? xrandom(ids - 1) + 1 : 0;
+	u32 ext = (type == 3) ? xrandom(MIN(OID_MAX_LEN - ids, 16)) : 0;
+
+	STORE_U16(oid->n_subid, split + ext);
+	for (u32 i = 0; i < ext; i++)
+	  STORE_U32(oid->ids[split + i], xrandom(OID_MAX_ID));
+
+	int no_change = 1;
+	for (u32 j = 0; j < MIN(ids, split + ext); j++)
+	{
+	  if (LOAD_U32(oid->ids[split + j]) != LOAD_U32(oids[i]->ids[split + j]))
+	    no_change = 1;
+	}
+
+	if (no_change)
+	  continue;
+
+	bt_assert(mib_tree_walk_is_oid_descendant(&walk, oid) < 0);
+	break;
+      }
+    }
+  }
+
+  {
+    struct mib_walk_state walk;
+    mib_tree_walk_init(&walk, tree);
+
+    u32 zero = 0;
+    const struct oid *null_oid = (void *) &zero;
+    u32 index = xrandom(size);
+
+    bt_assert(mib_tree_walk_is_oid_descendant(&walk, null_oid) == 0);
+    bt_assert(mib_tree_walk_is_oid_descendant(&walk, oids[index]) > 0);
+    (void) mib_tree_find(tree, &walk, oids[index]);
+    bt_assert(mib_tree_walk_is_oid_descendant(&walk, null_oid) < 0);
+  }
+
+  u32 null_oid = 0;
+  mib_tree_remove(tree, (struct oid *) &null_oid);
+  lp_restore(tmp_linpool, &tmps);
+
+  return 1;
+}
+
 static void UNUSED
 print_dups(const struct oid *oids[], uint size)
 {
@@ -1686,8 +1801,8 @@ int main(int argc, char **argv)
   bt_init(argc, argv);
   bt_bird_init();
 
-  unsigned seed = rand();
+  //unsigned seed = rand();
-  //unsigned seed = 1000789714;
+  unsigned seed = 1000789714;
   log("random seed is %d", seed);
   srandom(seed);
 
@@ -1696,6 +1811,7 @@ int main(int argc, char **argv)
   bt_test_suite(t_oid_prefixize, "Function transforming OID to prefixed form");
   bt_test_suite(t_oid_ancestor, "Function finding common ancestor of two OIDs");
   bt_test_suite(t_walk_to_oid, "Function transforming MIB tree walk state to OID");
+  bt_test_suite(t_walk_oid_desc, "Function comparing MIB tree walk to OID");
 
   bt_test_suite(t_tree_find, "MIB tree search");
   bt_test_suite(t_tree_traversal, "MIB tree traversal");

proto/snmp/snmp_utils.c

@@ -65,6 +65,7 @@ snmp_varbind_set_name_len(struct snmp_proto *p, struct agentx_varbind **vb, u8 l
   uint diff_size = (len - LOAD_U8(oid->n_subid)) * sizeof(u32);
   if (c->size < diff_size)
   {
+    snmp_log("varbind_set_name_len small buffer");
     snmp_manage_tbuf(p, c);
     oid = &(*vb)->name;
   }
@@ -81,7 +82,10 @@ snmp_varbind_duplicate_hdr(struct snmp_proto *p, struct agentx_varbind **vb, str
   ASSUME(vb != NULL && *vb != NULL);
   uint hdr_size = snmp_varbind_header_size(*vb);
   if (c->size < hdr_size)
+  {
+    snmp_log("varbind_duplicate small buffer");
     snmp_manage_tbuf(p, c);
+  }
 
   ASSERT(c->size >= hdr_size);
   byte *buffer = c->buffer;
@@ -290,7 +294,7 @@ snmp_set_varbind_type(struct agentx_varbind *vb, enum agentx_type t)
       return SNMP_SEARCH_OK;
 
     default:
-      die("invalid varbind type");
+      die("invalid varbind type %d", (int) t);
   }
 }
 
@@ -644,11 +648,12 @@ snmp_oid_ip4_index(struct oid *o, uint start, ip4_addr addr)
 }
 
 
-/** snmp_oid_compare - find the lexicographical order relation between @left and @right
+/**
- * both @left and @right has to be non-blank.
+ * snmp_oid_compare - find the lexicographical order relation between @left and @right
  * @left: left object id relation operant
  * @right: right object id relation operant
  *
+ * both @left and @right has to be non-blank.
  * function returns 0 if left == right,
  *   -1 if left < right,
  *   and 1 otherwise
@@ -748,7 +753,7 @@ snmp_registration_create(struct snmp_proto *p, u8 mib_class)
   r->transaction_id = p->transaction_id;
   // TODO where is incremented? is this valid?
   r->packet_id = p->packet_id + 1;
-  log(L_INFO "using registration packet_id %u", r->packet_id);
+  snmp_log("using registration packet_id %u", r->packet_id);
 
   r->mib_class = mib_class;
 
@@ -760,7 +765,7 @@ snmp_registration_create(struct snmp_proto *p, u8 mib_class)
 int
 snmp_registration_match(struct snmp_registration *r, struct agentx_header *h, u8 class)
 {
-  log(L_INFO "snmp_reg_same() r->packet_id %u p->packet_id %u", r->packet_id, h->packet_id);
+  snmp_log("snmp_reg_same() r->packet_id %u p->packet_id %u", r->packet_id, h->packet_id);
   return
     (r->mib_class == class) &&
     (r->session_id == h->session_id) &&
@@ -961,7 +966,7 @@ snmp_oid_log(const struct oid *oid)
   for (int id = 0; id < oid->n_subid; id++)
     pos += snprintf(pos, buf + 1024 - pos, ".%u", oid->ids[id]);
 
-  log(L_INFO "%s", buf);
+  snmp_log("%s", buf);
 }
 
 
@@ -1064,3 +1069,120 @@ snmp_oid_common_ancestor(const struct oid *left, const struct oid *right, struct
   STORE_U8(out->n_subid, subids);
 }
 
+/*
+ * SNMP MIB tree walking
+ */
+struct mib_leaf *
+snmp_walk_init(struct mib_tree *tree, struct mib_walk_state *walk, const struct oid *oid, struct snmp_data *data)
+{
+  mib_tree_walk_init(walk, tree);
+
+  snmp_vb_to_tx(data->p, oid, data->c);
+
+  mib_node_u *node = mib_tree_find(tree, walk, &data->c->sr_vb_start->name);
+
+  // TODO hide me in mib_tree code
+  /* mib_tree_find() returns NULL if the oid is longer than existing any path */
+  if (node == NULL && walk->stack_pos > 0)
+    node = walk->stack[walk->stack_pos - 1];
+
+  return (!node || !mib_node_is_leaf(node)) ? NULL : &node->leaf;
+}
+
+// TODO alter the varbind
+struct mib_leaf *
+snmp_walk_next(struct mib_tree *tree, struct mib_walk_state *walk, struct snmp_data *data)
+{
+  ASSUME(tree && walk);
+
+  if (!walk->stack_pos)
+    return NULL;
+
+  mib_node_u *node = walk->stack[walk->stack_pos - 1];
+
+  int found = 0;
+  struct mib_leaf *leaf = &node->leaf;
+  if (mib_node_is_leaf(node) && LOAD_U8(data->c->sr_vb_start->name.include))
+  {
+    found = 1;
+    STORE_U8(data->c->sr_vb_start->name.include, 0);
+  }
+
+  if (!found && mib_node_is_leaf(node) && leaf->call_next && !leaf->call_next(walk, data))
+    found = 1;
+
+  while (!found && (leaf = mib_tree_walk_next_leaf(tree, walk)) != NULL)
+  {
+    int old = snmp_oid_size(&data->c->sr_vb_start->name);
+    if (mib_tree_walk_to_oid(walk, &data->c->sr_vb_start->name, 20 * sizeof(u32)))
+    {
+      snmp_log("walk_next copy failed");
+      return NULL;
+    }
+
+    int new = snmp_oid_size(&data->c->sr_vb_start->name);
+    data->c->buffer += (new - old);
+
+    if (leaf->call_next && !leaf->call_next(walk, data))
+      found = 1;
+    else if (!leaf->call_next)
+      found = 1;
+  }
+
+  if (!found)
+    return NULL;
+
+
+  return leaf;
+}
+
+enum snmp_search_res
+snmp_walk_fill(struct mib_leaf *leaf, struct mib_walk_state *walk, struct snmp_data *data)
+{
+  struct agentx_varbind *vb = data->c->sr_vb_start;
+
+  if (!leaf)
+  //if (!leaf || mib_tree_walk_is_oid_descendant(walk, &vb->name) < 0)
+    return SNMP_SEARCH_NO_OBJECT;
+
+  uint size = 0;
+  if (leaf->size >= 0)
+  {
+    if (leaf->type == AGENTX_OCTET_STRING || leaf->type == AGENTX_OPAQUE ||
+	  leaf->type == AGENTX_OBJECT_ID)
+    {
+      snmp_set_varbind_type(vb, leaf->type);
+      size = leaf->size;
+    }
+    else if (leaf->type != AGENTX_INVALID)
+    {
+      snmp_set_varbind_type(vb, leaf->type);
+      size = agentx_type_size(leaf->type);
+    }
+    else
+      size = leaf->size;
+  }
+
+  snmp_log("walk_fill got size %u based on lt %u ls %u, calling filler()", size, leaf->type, leaf->size);
+
+  if (size >= data->c->size)
+  {
+    snmp_log("walk_fill small buffer size %d to %d", size, data->c->size);
+    snmp_manage_tbuf(data->p, data->c);
+  }
+
+  enum snmp_search_res res = leaf->filler(walk, data);
+
+  vb = data->c->sr_vb_start;
+
+  if (res != SNMP_SEARCH_OK)
+    snmp_set_varbind_type(vb, snmp_search_res_to_type(res));
+
+  u16 type = snmp_load_varbind_type(vb);
+  /* Test that hook() did not overwrite the VarBind type to non-matching type */
+  ASSUME(type == leaf->type || type == AGENTX_END_OF_MIB_VIEW || type == AGENTX_NO_SUCH_OBJECT ||
+    type == AGENTX_NO_SUCH_INSTANCE);
+
+  return res;
+}
+

proto/snmp/snmp_utils.h

@@ -2,6 +2,7 @@
 #define _BIRD_SNMP_UTILS_H_
 
 #include "subagent.h"
+#include "mib_tree.h"
 
 uint snmp_pkt_len(const byte *start, const byte *end);
 
@@ -110,4 +111,11 @@ enum agentx_type snmp_search_res_to_type(enum snmp_search_res res);
 #define AGENTX_TYPE_IP4_SIZE ((uint) agentx_type_size(AGENTX_IP_ADDRESS))
 #define AGENTX_TYPE_COUNTER32_SIZE ((uint) agentx_type_size(AGENTX_COUNTER_32))
 
+/*
+ * SNMP MIB tree walking
+ */
+struct mib_leaf *snmp_walk_init(struct mib_tree *tree, struct mib_walk_state *state, const struct oid *start_rx, struct snmp_data *data);
+struct mib_leaf *snmp_walk_next(struct mib_tree *tree, struct mib_walk_state *state, struct snmp_data *data);
+enum snmp_search_res snmp_walk_fill(struct mib_leaf *leaf, struct mib_walk_state *state, struct snmp_data *data);
+
 #endif

proto/snmp/subagent.c

@@ -74,6 +74,7 @@ static uint update_packet_size(struct agentx_header *start, byte *end);
 
 static enum snmp_search_res search_mib(struct snmp_proto *p, struct agentx_varbind **vb_search, const struct oid *o_end, struct snmp_pdu *c);
 
+/* standard SNMP internet prefix (1.3.6.1) */
 const u32 snmp_internet[] = { SNMP_ISO, SNMP_ORG, SNMP_DOD, SNMP_INTERNET };
 
 static inline int
@@ -145,7 +146,6 @@ snmp_register_ack(struct snmp_proto *p, struct agentx_response *res, u8 class)
   }
 }
 
-
 /*
  * snmp_error - handle a malformed packet
  * @p: SNMP protocol instance
@@ -158,7 +158,6 @@ static inline void
 snmp_error(struct snmp_proto *p)
 {
   snmp_reset(p);
-  //snmp_set_state(p, SNMP_RESET);
 }
 
 /*
@@ -206,7 +205,10 @@ open_pdu(struct snmp_proto *p, struct oid *oid)
   /* Make sure that we have enough space in TX-buffer */
   if (c.size < AGENTX_HEADER_SIZE + TIMEOUT_SIZE + snmp_oid_size(oid) +
       + snmp_str_size(cf->description))
+  {
+    snmp_log("agentx-Open-PDU small buffer");
     snmp_manage_tbuf(p, &c);
+  }
 
   struct agentx_header *h = (void *) c.buffer;
   ADVANCE(c.buffer, c.size, AGENTX_HEADER_SIZE);
@@ -254,9 +256,9 @@ snmp_notify_pdu(struct snmp_proto *p, struct oid *oid, void *data, uint size, in
 
 // TODO use more readable anonymous structure decl.
 #define UPTIME_SIZE \
-  (6 * sizeof(u32)) /* sizeof( { u32 vb_type, u32 oid_hdr, u32 ids[4] } ) */
+  sizeof( struct { u32 vb_type; u32 oid_hdr; u32 ids[4]; } )
 #define TRAP0_HEADER_SIZE \
-  (7 * sizeof(u32)) /* sizeof( { u32 vb_type, u32 oid_hdr, u32 ids[6] } ) */
+  sizeof( struct { u32 vb_type; u32 oid_hdr; u32 ids[6]; } )
 
   uint sz = AGENTX_HEADER_SIZE + TRAP0_HEADER_SIZE + snmp_oid_size(oid) \
     + size;
@@ -266,7 +268,10 @@ snmp_notify_pdu(struct snmp_proto *p, struct oid *oid, void *data, uint size, in
 
   /* Make sure that we have enough space in TX-buffer */
   if (c.size < sz)
+  {
+    snmp_log("agentx-Notify-PDU small buffer");
     snmp_manage_tbuf(p, &c);
+  }
 
   struct agentx_header *h = (void *) c.buffer;
   ADVANCE(c.buffer, c.size, AGENTX_HEADER_SIZE);
@@ -361,7 +366,10 @@ un_register_pdu(struct snmp_proto *p, struct oid *oid, u32 bound, uint index, en
       ((bound > 1) ? BOUND_SIZE : 0);
 
   if (c.size < sz)
+  {
+    snmp_log("agentx-Register-PDU small buffer");
     snmp_manage_tbuf(p, &c);
+  }
 
   struct agentx_header *h = (void *) c.buffer;
   ADVANCE(c.buffer, c.size, AGENTX_HEADER_SIZE);
@@ -443,7 +451,10 @@ close_pdu(struct snmp_proto *p, enum agentx_close_reasons reason)
 
 #define REASON_SIZE sizeof(u32)
   if (c.size < AGENTX_HEADER_SIZE + REASON_SIZE)
+  {
+    snmp_log("agentx-Close-PDU small buffer");
     snmp_manage_tbuf(p, &c);
+  }
 
   struct agentx_header *h = (void *) c.buffer;
   ADVANCE(c.buffer, c.size, AGENTX_HEADER_SIZE);
@@ -574,7 +585,10 @@ parse_test_set_pdu(struct snmp_proto *p, byte * const pkt_start)
   snmp_pdu_context(&c, sk);
 
   if (c.size < AGENTX_HEADER_SIZE)
+  {
+    snmp_log("agentx-TestSet-PDU small buffer");
     snmp_manage_tbuf(p, &c);
+  }
 
   res = prepare_response(p, &c);
 
@@ -670,7 +684,10 @@ parse_sets_pdu(struct snmp_proto *p, byte * const pkt_start, enum agentx_respons
   struct snmp_pdu c;
   snmp_pdu_context(&c, p->sock);
   if (c.size < sizeof(struct agentx_response))
+  {
+    snmp_log("parse_sets_pdu small buffer");
     snmp_manage_tbuf(p, &c);
+  }
 
   struct agentx_response *r = prepare_response(p, &c);
 
@@ -781,15 +798,17 @@ static uint
 parse_pkt(struct snmp_proto *p, byte *pkt, uint size)
 {
   /* TX-buffer free space */
-  ASSERT(snmp_is_active(p));
-  if (!space_for_response(p->sock))
-    return 0;
-
-  ASSERT(snmp_is_active(p));
   if (size < AGENTX_HEADER_SIZE)
     return 0;
 
   struct agentx_header *h = (void *) pkt;
+  if (h->flags & AGENTX_NETWORK_BYTE_ORDER)
+  {
+    TRACE(D_PACKETS, "SNMP received PDU with unexpected byte order");
+    snmp_reset(p);
+    return 0;
+  }
+
   uint pkt_size = LOAD_U32(h->payload);
 
   /* RX side checks - too big packet */
@@ -797,7 +816,7 @@ parse_pkt(struct snmp_proto *p, byte *pkt, uint size)
   {
     snmp_simple_response(p, AGENTX_RES_GEN_ERROR, 0);
     snmp_reset(p);
-    return 0; // TODO return size??
+    return 0; /* no bytes parsed */
   }
 
   /* This guarantees that we have the full packet already received */
@@ -827,24 +846,20 @@ parse_pkt(struct snmp_proto *p, byte *pkt, uint size)
     p->session_id = copy.session_id;
     p->transaction_id = copy.transaction_id;
     p->packet_id = copy.packet_id;
-    log(L_INFO "restoring packet_id %u from temporal state", p->packet_id);
+    snmp_log("restoring packet_id %u from temporal state", p->packet_id);
 
     /*
      * After unexpected state, we simply reset the session
      * only sending the agentx-Response-PDU.
      */
     snmp_reset(p);
-    return 0; // return size??
+    return 0;
   }
 
-  ASSERT(snmp_is_active(p));
   if (h->flags & AGENTX_NON_DEFAULT_CONTEXT)
   {
-    // TODO add non-default context support
+    TRACE(D_PACKETS, "SNMP received PDU with non-default context");
-    TRACE(D_PACKETS, "SNMP received PDU with unexpected byte order");
     snmp_simple_response(p, AGENTX_RES_UNSUPPORTED_CONTEXT, 0);
-    /* We always accept the packet length as correct, up to set limit */
-    // TODO limit
     return pkt_size + AGENTX_HEADER_SIZE;
   }
 
@@ -1071,7 +1086,10 @@ snmp_get_next2(struct snmp_proto *p, struct agentx_varbind **vb_search, struct o
 
   o_start = &(*vb_search)->name;
   if (c->size < snmp_varbind_hdr_size_from_oid(o_start))
+  {
+    snmp_log("get_next2 small buffer");
     snmp_manage_tbuf(p, c);
+  }
 
   snmp_set_varbind_type(*vb_search, AGENTX_END_OF_MIB_VIEW);
   return 0;
@@ -1146,7 +1164,9 @@ snmp_get_next3(struct snmp_proto *p, struct oid *o_start, struct oid *o_end,
   }
 
   if (c->size < snmp_varbind_hdr_size_from_oid(o_start))
+  {
     snmp_manage_tbuf(p, c);
+  }
 
   vb = snmp_create_varbind(c->buffer, o_start);
   vb->type = AGENTX_END_OF_MIB_VIEW;
@@ -1328,7 +1348,9 @@ snmp_oid_prefixize(struct snmp_proto *p, const struct oid *oid, struct snmp_pdu
   uint oid_size = snmp_oid_size(oid);
 
   if (c->size < oid_size)
+  {
     snmp_manage_tbuf(p, c);
+  }
 
   // TODO check if the @oid is prefixable
   ASSERT(c->size >= oid_size);
@@ -1345,15 +1367,17 @@ snmp_oid_prefixize(struct snmp_proto *p, const struct oid *oid, struct snmp_pdu
  * @c: PDU context
  *
  * Create NULL initialized VarBind inside TX buffer (from @c) whose vb->name is
- * @oid. The @oid is not prefixed and is prefixable, the @oid is prefixed first.
+ * @oid. The @oid prefixed if possible. The result is stored in @c->sr_vb_start.
- * The protocol @p is used in cases of TX buffer space shortage.
  */
-struct agentx_varbind *
+void
 snmp_vb_to_tx(struct snmp_proto *p, const struct oid *oid, struct snmp_pdu *c)
 {
   uint vb_hdr_size = snmp_varbind_hdr_size_from_oid(oid);
   if (c->size < vb_hdr_size)
+  {
+    snmp_log("SNMP vb_to_tx small buffer");
     snmp_manage_tbuf(p, c);
+  }
 
   ASSERT(c->size >= vb_hdr_size);
   struct agentx_varbind *vb = (void *) c->buffer;
@@ -1366,37 +1390,15 @@ snmp_vb_to_tx(struct snmp_proto *p, const struct oid *oid, struct snmp_pdu *c)
     u8 subids = LOAD_U8(oid->n_subid) - 5;
     ADVANCE(c->buffer, c->size, snmp_oid_size_from_len(subids));
     (void) snmp_oid_prefixize_unsafe(&vb->name, oid);
-    return vb;
+
+    c->sr_vb_start = vb;
+    return;
   }
 
   ADVANCE(c->buffer, c->size, snmp_oid_size(oid));
   snmp_oid_copy2(&vb->name, oid);
-  return vb;
-}
 
-/*
+  c->sr_vb_start = vb;
- * snmp_oid_to_scratch - allocate temporal Object Identifier in prefixed form
- * @oid: prefixed Object Identifier if possible
- */
-static struct oid *
-snmp_oid_to_scratch(const struct oid *oid)
-{
-  struct oid *dest;
-  if (snmp_oid_is_prefixable(oid) && !snmp_oid_is_prefixed(oid))
-  {
-    u8 subids = LOAD_U8(oid->n_subid) - 5;
-    uint prefixed_size = sizeof(struct oid) + (subids * sizeof(u32));
-    dest = tmp_alloc(prefixed_size);
-    snmp_oid_prefixize_unsafe(dest, oid);
-
-    return dest;
-  }
-
-  uint oid_size = snmp_oid_size(oid);
-  dest = tmp_alloc(oid_size);
-  snmp_oid_copy2(dest, oid);
-
-  return dest;
 }
 
 /*
@@ -1429,6 +1431,7 @@ response_err_ind(struct snmp_proto *p, struct agentx_response *res, enum agentx_
 {
   STORE_U32(res->error, (u16) err);
   // TODO deal with auto-incrementing of snmp_pdu context c.ind
+  // FIXME for packets with errors reset reset payload size to null (by move c.buffer appropriately)
   if (err != AGENTX_RES_NO_ERROR && err != AGENTX_RES_GEN_ERROR)
   {
     TRACE(D_PACKETS, "Last PDU resulted in error %u", err);
@@ -1439,7 +1442,7 @@ response_err_ind(struct snmp_proto *p, struct agentx_response *res, enum agentx_
   }
   else if (err == AGENTX_RES_GEN_ERROR)
   {
-    TRACE(D_PACKETS, "Last PDU resulted in error %u", err);
+    TRACE(D_PACKETS, "Last PDU resulted in error %u genErr", err);
     STORE_U32(res->index, 0);
     TRACE(D_PACKETS, "Storing packet size %u (was %u)", sizeof(struct agentx_response) - AGENTX_HEADER_SIZE, LOAD_U32(res->h.payload));
     STORE_U32(res->h.payload,
@@ -1461,59 +1464,45 @@ parse_gets_error(struct snmp_proto *p, struct snmp_pdu *c, uint len)
   return len + AGENTX_HEADER_SIZE;
 }
 
-static enum snmp_search_res
+/*
-snmp_mib_fill2(struct snmp_proto *p, struct snmp_pdu *c, mib_node_u *mib_node)
+ * AgentX GetPDU, GetNextPDU and GetBulkPDU
-{
+ */
-  if (!mib_node || !mib_node_is_leaf(mib_node))
-  {
-    snmp_set_varbind_type(c->sr_vb_start, AGENTX_NO_SUCH_OBJECT);
-    ADVANCE(c->buffer, c->size, snmp_varbind_header_size(c->sr_vb_start));
-    return AGENTX_NO_SUCH_OBJECT;
-  }
-
-  struct mib_leaf *leaf = &mib_node->leaf;
-
-  return leaf->filler(p, c);
-}
-
 void
-snmp_get_pdu(struct snmp_proto *p, struct snmp_pdu *c, struct mib_walk_state *walk)
+snmp_get_pdu(struct snmp_proto *p, struct snmp_pdu *c, const struct oid *o_start, struct mib_walk_state *walk)
 {
-  mib_node_u *node;
+  snmp_log("snmp_get_pdu()");
-  node = mib_tree_find(p->mib_tree, walk, &c->sr_vb_start->name);
+  struct snmp_data d = {
+    .p = p,
+    .c = c,
+  };
 
-  (void) snmp_mib_fill2(p, c, node);
+  struct mib_leaf *leaf;
+  leaf = snmp_walk_init(p->mib_tree, walk, o_start, &d);
+
+  snmp_log("found node %p", leaf);
+
+  enum snmp_search_res res;
+  res = snmp_walk_fill(leaf, walk, &d);
+
+  snmp_log("fill result %u", res);
+
+  if (res != SNMP_SEARCH_OK)
+    snmp_set_varbind_type(c->sr_vb_start, snmp_search_res_to_type(res));
 }
 
 int
-snmp_get_next_pdu(struct snmp_proto *p, struct snmp_pdu *c, struct mib_walk_state *walk)
+snmp_get_next_pdu(struct snmp_proto *p, struct snmp_pdu *c, const struct oid *o_start, struct mib_walk_state *walk)
 {
-  mib_node_u *node;
+  struct snmp_data d = {
-  node = mib_tree_find(p->mib_tree, walk, &c->sr_vb_start->name);
+    .p = p,
+    .c = c,
+  };
 
-  int inclusive = c->sr_vb_start->name.include;
+  snmp_walk_init(p->mib_tree, walk, o_start, &d);
-
+  struct mib_leaf *leaf = snmp_walk_next(p->mib_tree, walk, &d);
-  int move_next;
-  if (!node && inclusive)
-    move_next = 1;
-  else if (!node && !inclusive)
-    move_next = 1;
-  else if (node && inclusive && mib_node_is_leaf(node))
-    move_next = 0;
-  else if (node && inclusive)
-    move_next = 1;
-  else if (node && !inclusive)
-    move_next = 0;
-
-  struct mib_leaf *leaf = &node->leaf;
-  if (move_next && node && mib_node_is_leaf(node))
-    move_next = leaf->call_next(p, c, walk);
-
-  if (move_next)
-    node = (mib_node_u *) mib_tree_walk_next_leaf(p->mib_tree, walk);
 
   enum snmp_search_res res;
-  res = snmp_mib_fill2(p, c, node);
+  res = snmp_walk_fill(leaf, walk, &d);
 
   if (res != SNMP_SEARCH_OK)
     snmp_set_varbind_type(c->sr_vb_start, AGENTX_END_OF_MIB_VIEW);
@@ -1522,14 +1511,64 @@ snmp_get_next_pdu(struct snmp_proto *p, struct snmp_pdu *c, struct mib_walk_stat
 }
 
 void
-snmp_get_bulk_pdu(struct snmp_proto *p, struct snmp_pdu *c, struct mib_walk_state *walk, struct agentx_bulk_state *bulk)
+snmp_get_bulk_pdu(struct snmp_proto *p, struct snmp_pdu *c, const struct oid *o_start, struct mib_walk_state *walk, struct agentx_bulk_state *bulk)
 {
   if (c->index >= bulk->getbulk.non_repeaters)
     bulk->repeaters++;
 
   // store the o_start and o_end
 
-  bulk->has_any |= snmp_get_next_pdu(p, c, walk);
+  bulk->has_any |= snmp_get_next_pdu(p, c, o_start, walk);
+}
+
+static inline const struct oid *
+snmp_load_oids(byte **pkt_ptr, uint *pkt_sz, struct snmp_pdu *c)
+{
+  byte *pkt = *pkt_ptr;
+  uint pkt_size = *pkt_sz;
+
+  uint sz;
+  const struct oid *start = (const struct oid *) pkt;
+
+  if ((sz = snmp_oid_size(start)) > pkt_size)
+  {
+    snmp_log("load_oids start %u / %u", sz, pkt_size);
+    c->error = AGENTX_RES_PARSE_ERROR;
+    *pkt_ptr = pkt;
+    *pkt_sz = pkt_size;
+    return NULL;
+  }
+
+  ADVANCE(pkt, pkt_size, sz);
+
+  const struct oid *end = (const struct oid *) pkt;
+  if ((sz = snmp_oid_size(end)) > pkt_size)
+  {
+    snmp_log("load_oids end %u / %u", sz, pkt_size);
+    c->error = AGENTX_RES_PARSE_ERROR;
+    *pkt_ptr = pkt;
+    *pkt_sz = pkt_size;
+    return NULL;
+  }
+
+  ADVANCE(pkt, pkt_size, sz);
+
+  if (!snmp_is_oid_empty(end) &&
+      snmp_oid_compare(start, end) > 0)
+  {
+    c->error = AGENTX_RES_GEN_ERROR;
+    *pkt_ptr = pkt;
+    *pkt_sz = pkt_size;
+    return NULL;
+  }
+
+  ASSERT(start != NULL);
+  ASSERT(end != NULL);
+
+  c->sr_o_end = end;
+  *pkt_ptr = pkt;
+  *pkt_sz = pkt_size;
+  return start;
 }
 
 /*
@@ -1544,6 +1583,7 @@ snmp_get_bulk_pdu(struct snmp_proto *p, struct snmp_pdu *c, struct mib_walk_stat
 static uint
 parse_gets_pdu(struct snmp_proto *p, byte * const pkt_start)
 {
+  snmp_log("parse_gets_pdu msg");
   // TODO checks for c.size underflow
   struct mib_walk_state walk;
   byte *pkt = pkt_start;
@@ -1560,11 +1600,12 @@ parse_gets_pdu(struct snmp_proto *p, byte * const pkt_start)
    * Get-Bulk processing stops if all the varbind have type END_OF_MIB_VIEW
    * has_any is true if some varbind has type other than END_OF_MIB_VIEW
    */
-  struct agentx_bulk_state bulk_state = { };
+  struct agentx_bulk_state bulk_state = { 0 };
   if (h->type == AGENTX_GET_BULK_PDU)
   {
     if (pkt_size < sizeof(struct agentx_getbulk))
     {
+      snmp_log("parse_gets GetBulkPDU prepare");
       c.error = AGENTX_RES_PARSE_ERROR;
       c.index = 0;
       return parse_gets_error(p, &c, pkt_size);
@@ -1594,62 +1635,27 @@ parse_gets_pdu(struct snmp_proto *p, byte * const pkt_start)
   {
     lp_restore(tmp_linpool, &tmps);
 
-    /* We load search range start OID */
+    const struct oid *start_rx;
-    const struct oid *o_start_rx = (void *) pkt;
+    if (!(start_rx = snmp_load_oids(&pkt, &pkt_size, &c)))
-    uint sz;
-    if ((sz = snmp_oid_size(o_start_rx)) > pkt_size)
     {
-      c.error = AGENTX_RES_PARSE_ERROR;
+      snmp_log("snmp_load_oid ends with an error");
-      return parse_gets_error(p, &c, pkt_size);
-    }
-
-    /* Update buffer pointer and remaining size counters. */
-    ADVANCE(pkt, pkt_size, sz);
-
-    /*
-     * We load search range end OID
-     * The exactly same process of sanity checking is preformed while loading
-     * the SearchRange's end OID
-     */
-    const struct oid *o_end_rx = (void *) pkt;
-    if ((sz = snmp_oid_size(o_end_rx)) > pkt_size)
-    {
-      c.error = AGENTX_RES_PARSE_ERROR;
-      return parse_gets_error(p, &c, pkt_size);
-    }
-
-    ADVANCE(pkt, pkt_size, sz);
-
-    /* We don't too to check for oversided OID because the PDU has 8k size limit */
-
-    /* We create copy of OIDs outside of rx-buffer and also prefixize them */
-    c.sr_vb_start = snmp_vb_to_tx(p, o_start_rx, &c);
-    c.sr_o_end = snmp_oid_to_scratch(o_end_rx);
-
-    ASSERT(c.sr_vb_start); // TODO implement failed parsing logic
-    ASSERT(c.sr_o_end);
-
-    if (!snmp_is_oid_empty(c.sr_o_end) &&
-	snmp_oid_compare(&c.sr_vb_start->name, c.sr_o_end) > 0)
-    {
-      c.error = AGENTX_RES_GEN_ERROR;
       return parse_gets_error(p, &c, pkt_size);
     }
 
     switch (h->type)
     {
       case AGENTX_GET_PDU:
-	snmp_get_pdu(p, &c, &walk);
+	snmp_get_pdu(p, &c, start_rx, &walk);
 	//snmp_mib_fill(p, &vb_start, &c);
 	break;
 
       case AGENTX_GET_NEXT_PDU:
-	snmp_get_next_pdu(p, &c, &walk);
+	snmp_get_next_pdu(p, &c, start_rx, &walk);
 	//snmp_get_next2(p, &vb_start, o_end, &c);
 	break;
 
       case AGENTX_GET_BULK_PDU:
-	snmp_get_bulk_pdu(p, &c, &walk, &bulk_state);
+	snmp_get_bulk_pdu(p, &c, start_rx, &walk, &bulk_state);
 	#if 0
 	if (c.index >= bulk_state.getbulk.non_repeaters)
 	  bulk_state.repeaters++;
@@ -1663,7 +1669,7 @@ parse_gets_pdu(struct snmp_proto *p, byte * const pkt_start)
 	break;
 
       default:
-	die("incorrect usage");
+	die("implementation failure");
     }
 
     c.sr_vb_start = NULL;
@@ -1744,7 +1750,7 @@ snmp_stop_subagent(struct snmp_proto *p)
 int
 snmp_rx(sock *sk, uint size)
 {
-  log(L_INFO "snmp_rx with size %u", size);
+  snmp_log("snmp_rx with size %u", size);
   struct snmp_proto *p = sk->data;
   byte *pkt_start = sk->rbuf;
   byte *end = pkt_start + size;
@@ -1780,7 +1786,7 @@ snmp_rx(sock *sk, uint size)
 void
 snmp_tx(sock *sk)
 {
-  log(L_INFO "snmp_tx()");
+  snmp_log("snmp_tx()");
   /* We still not have enough space */
   if (!space_for_response(sk))
     return;
@@ -1819,7 +1825,7 @@ snmp_ping(struct snmp_proto *p)
   ADVANCE(c.buffer, c.size, AGENTX_HEADER_SIZE);
   snmp_blank_header(h, AGENTX_PING_PDU);
   p->packet_id++;
-  log(L_INFO "incrementing packet_id to %u (ping)", p->packet_id);
+  snmp_log("incrementing packet_id to %u (ping)", p->packet_id);
   snmp_session(p, h);
 
   /* sending only header */
@@ -2099,7 +2105,7 @@ snmp_manage_tbuf(struct snmp_proto *p, struct snmp_pdu *c)
   if (c->sr_vb_start != NULL)
     diff = (void *) c->sr_vb_start - (void *) sk->tbuf;
 
-  log(L_INFO "snmp_manage_tbuf2()");
+  snmp_log("snmp_manage_tbuf2()");
   sk_set_tbsize(sk, sk->tbsize + 2048);
   c->size += 2048;
 
@@ -2123,7 +2129,7 @@ snmp_manage_tbuf2(struct snmp_proto *p, void **ptr, struct snmp_pdu *c)
   if (ptr)
     diff = *ptr - (void *) sk->tbuf;
 
-  log(L_INFO "snmp_manage_tbuf()");
+  snmp_log("snmp_manage_tbuf()");
   sk_set_tbsize(sk, sk->tbsize + 2048);
   c->size += 2048;
 

proto/snmp/subagent.h

@@ -1,3 +1,4 @@
+
 #ifndef _BIRD_SNMP_SUBAGENT_H_
 #define _BIRD_SNMP_SUBAGENT_H_
 
@@ -51,7 +52,7 @@ enum agentx_type {
   AGENTX_NO_SUCH_INSTANCE   = 129,
   AGENTX_END_OF_MIB_VIEW    = 130,
 
-  AGENTX_INVALID	    =  -1,
+  AGENTX_INVALID	    =   0,
 } PACKED;
 
 enum snmp_search_res {
@@ -169,6 +170,15 @@ struct oid {
   u32 ids[];
 };
 
+#define STATIC_OID(sbids)						      \
+  struct {								      \
+    u8 n_subid;								      \
+    u8 prefix;								      \
+    u8 include;								      \
+    u8 reserved;							      \
+    u32 ids[sbids];							      \
+  }
+
 /* enforced by MIB tree, see mib_tree.h for more info */
 #define OID_MAX_LEN 32
 
@@ -301,21 +311,22 @@ struct snmp_pdu {
 
   /* Search Range */
   struct agentx_varbind *sr_vb_start; /* search range starting OID inside TX buffer (final storage) */
-  struct oid *sr_o_end;		    /* search range ending OID */
+  const struct oid *sr_o_end;	      /* search range ending OID */
 
   /* Control */
   enum agentx_response_errs error;  /* storage for result of current action */
   u32 index;			    /* index on which the error was found */
 
-  union snmp_mibs_data *mibs_data;  /* data passed from MIB search phase to MIB fill phase */
 };
 
-#include "bgp4_mib.h"
+/*
-
+ * snmp_data - Comprehensive hadle for Agentx PDU state
-union snmp_mibs_data {
+ * @p: SNMP protocol instance
-  enum snmp_tags empty;
+ * @c: contextual data for currrently constructed AgentX PDU
-
+ */
-  struct bgp4_mib bgp4;
+struct snmp_data {
+  struct snmp_proto *p;
+  struct snmp_pdu *c;
 };
 
 struct snmp_packet_info {
@@ -346,16 +357,17 @@ void snmp_notify_pdu(struct snmp_proto *p, struct oid *oid, void *data, uint siz
 
 void snmp_manage_tbuf(struct snmp_proto *p, struct snmp_pdu *c);
 
-struct agentx_varbind *snmp_vb_to_tx(struct snmp_proto *p, const struct oid *oid, struct snmp_pdu *c);
+void snmp_vb_to_tx(struct snmp_proto *p, const struct oid *oid, struct snmp_pdu *c);
 u8 snmp_get_mib_class(const struct oid *oid);
 
 void snmp_register_mibs(struct snmp_proto *p);
-void snmp_bgp_start(struct snmp_proto *p);
+
+/* MIB modules */
+void snmp_bgp4_start(struct snmp_proto *p);
 
 
-// debug wrapper
+#if 1
-#if 0
+#define snmp_log(...) log(L_INFO "SNMP " __VA_ARGS__)
-#define snmp_log(...) log(L_INFO "snmp " __VA_ARGS__)
 #else
 #define snmp_log(...) do { } while(0)
 #endif