I'm happy to share the changes I made to the library,  but they were
minimal.  Really just:

(a)  adding this hook to snmp_api.c to allow response packets to be
fed back in...

// add hook to allow this to be called from outside
void snmp_sess_receive_packet(void *sessp, u_char *packet, int length,
struct sockaddr *from, int from_len)
{
  struct session_list *slp = (struct session_list *) sessp;
  if(slp) {
    netsnmp_session *sp = slp->session;
    struct snmp_internal_session *isp = slp->internal;
    netsnmp_transport *transport = slp->transport;
    if(transport && transport->sock > 0 && sp && isp && isp->requests) {
      // The opaque is expected to be malloc'd by the transport
receive fn
      // because it is then freed by _sess_process_packet() later.
      void *from2 = malloc(from_len);
      memcpy(from2, from, from_len);
      int rc = _sess_process_packet(sessp, sp, isp, transport, from2,
from_len, packet, length);
      if(rc && sp->s_snmp_errno) SET_SNMP_ERROR(sp->s_snmp_errno);
    }
  }
}

(b) In snmpusm.c,  declaring usm_build_probe_pdu() as a global
function (by removing "static").

(c) reworking snmpusm.c to use a hash-table of lists,  instead of just
one (struct usmUser *) list.  This was a bigger change,  but it was
also a _breaking_ change because there is an SNMP v3 agent table that
expects a certain ordering here.  I think the right compromise might
be to use a tree-structure so that lookups are efficient while
ordering is preserved,  or better still just use a hash table and
change the agent code to comb through the table any time it wants a
GET_NEXT.  But either way that is a big change for such an obscure
benefit,  and extra care would need to be taken - especially in the
presence of multiple threads.

Unfortunately the other code I wrote to interface between the rest of
the application and the library is too intertwined with product code
to share just as it is.  But I'd be happy to discuss choices if
someone were to look at this.  For now I can share this psuedo code
outline below, which is probably more helpful anyway.

Notes:
(1)  I used a red-black-tree for the retry-timeout collection when it
might have been more appropriate to use a binary-heap(?)
(2)  I had to maintain separate hash lookups keyed by reqid and msgid
so that processing SNMPv3 responses could work the same way as for
SNMPv2.  For v3 responses the msgid appears unencrypted, while the
reqid only emerges after decryption is successful.
(3) Since a response might come in for a request that has already been
retried the request-pdu needs to keep the list of all attempts made so
far. Those requests are kept in an ordered list by the pdu and only
deleted when the pdu life-cycle is complete.
(4) It might have worked better to make the Request object be the
callback magic pointer, rather than the more persistent "Target"
object,  which then needs yet another hash table lookup to get to the
original request that matches the pdu.  I think this was to make sure
it was OK to delete a request at any time,  but in practice the
lifecycle is clear so maybe there is one indirection too many in this
scheme.

The cast of characters here are:
snmp - one persistent object representing this wrapper
  \_target - one mostly-persistent object for each remote target IP
      \_pdu - query and response objects
         |_request - individual attempt

// the callback function given to the library.  Called after
snmp_timeout() or snmp_sess_receive_packet()
SNMP_callback(int op, netsnmp_session *ss, int reqid, netsnmp_pdu
*pdu, void *magic) {
  target = (target)magic;
  req = removeFromHashTable(target->requestsByReqid, pdu->reqid)
  // process Pdu/error and call back to client
  // now we can finally clean up
  for(req in pdu->requests) {
     // make sure req is removed from all 5 collections:
     removeFromList(pdu->requests, req)
     removeFromTree(snmp->timeoutTree, req)
     removeFromHashTable(target->requestsByReqid, pdu->reqid)
     removeFromHashTable(snmp->requestsByReqId, req->reqid)
     removeFromHashTable(snmp->requestsByMsgId, req->msgid)
     delete(req)
  }
}

// sending a request
SNMP_sendRequest(pdu, target) {
  long reqid = snmp_sess_async_send(target->sessp, pdu, SNMP_callback, target)
  req = newRequest(target, pdu, reqid)
  // req goes in up to 5 collections:
  addToList(pdu->requests,req)
  addToHashTable(target->requestsByReqId, reqid)
  addToTree(snmp->timeoutTree, req)
  addToHashTable(snmp->requestsByReqId, reqid, req);
  if(pdu->version == 3)
     addToHashTable(snmp->requestsByMsgId, pdu->msgid, req);
}

// this is called every 100mS or so
SNMP_timeout(snmp) {
     while((req = timedOutRequest(snmp)) != NULL) {
        removeReqFromTimeoutTree(req);
        snmp_sess_timeout(req->target->sessp);
     }
}

// this is called when a response packet appears on the socket as a
result of select() or poll()
SNMP_response(snmp, socket, msg) {
     Request req=NULL;
     long version=0,msgid=0,reqid=0;
     snmp_parse_version_msgid(msg, &version, &msgid, &reqid);
     if(version==3)
        req = removeFromHashTable(snmp->requestsByMsgId, msgid);
     else
        req = removeFromHashTable(snmp->requestsByReqId, reqid);
     if(req) {
        removeFromTree(snmp->timeoutTree, req);
        snmp_sess_receive_packet(req->target->sessp, msg->buf,
msg->len, (struct sockaddr *)&msg->peer, msg->peer_len);
     }
}

On Fri, Oct 22, 2021 at 9:07 AM Wes Hardaker
<harda...@users.sourceforge.net> wrote:
>
> Neil Mckee <neil.mckee...@gmail.com> writes:
>
> > You are very welcome. Let me know if you want to discuss any of the
> > particulars.
>
> Well, the biggest question in my mind is: are you willing to contribute
> the patches back?
>
> It's unclear if any of them break backwards compatability (which we
> always strive for), but certainly many of them would not.
>
> > Adding or importing your favorite tree implementation to the library
> > would be a lot of new code
>
> Well, there's one issue: lots of new code means bigger applications and
> we do strive to be small to run on small devices.  That being said, the
> sliding definition of "small" has certainly increased in size over time
> with the continued increase of storage/memory availability in even the
> smallest devices.
>
>
> --
> Wes Hardaker
> Please mail all replies to net-snmp-coders@lists.sourceforge.net


_______________________________________________
Net-snmp-coders mailing list
Net-snmp-coders@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/net-snmp-coders

Reply via email to