ping, any comments on the considerations or the code?

On Fri, 2023-12-29 at 16:47 +0100, Martijn van Duren wrote:
> tl;dr implications which need consideration:
> - slower start-up time
> - extra memory usage
> - extra files in /usr/share/snmp/mibs
> - Permissions to ship particular MIBs.
> 
> This series of diffs moves towards removing smi.c and mib.h's
> definitions. The current approach is severely limited, error-prone, and
> labour-intensive. Allowing the parsing of MIB files will make this a lot
> more trivial.
> 
> What does this change functionally? With all patches applied it changes
> the format of OIDs when logging, and support for the new syntax in
> snmpd.conf. The old syntax will still be used in "trap handle" to not
> break scripts and - available in snmpd.conf with a deprecation warning.
> For the latter most cases won't trigger this warning since most names
> match with the official naming.
> 
> What is supported:
> - SMIv2 only. I have no plans, nor desire to implement SMIv1.
> - SNMPv2-SMI (RFC2578) parsing
> - SNMPv2-TC (RFC2579) parsing
> - Specifying multiple MIB directories
> - Duplicate modules will always choose the newest revision
> - If a(n global) OID is defined as a simple object identity and via a
>   macro the macro - will overwrite the object identity definition.
> - mib_oid2string(). Similar to smi_oid2string(), but prints it in the
>   form <module>::<descriptor>, similar to what net-snmp and libsmi do,
>   and has an argument to determine whether to print in textual, or
>   numeric for instead of peeking into snmpd_env directly.
> - mib_string2oid(). Similar to smi_string2oid(), but allows for both
>   <module>::<descriptor> and <descriptor> searching. Apart from that
>   it prioritises case-sensitive over case-insensitive matching for
>   the module-form.
> 
> SNMPv2-CONF, OBJECT-TYPE index pretty printing, and anything not related
> to OID<->descriptor translations (apart from grocking the text) is not
> (yet) supported.
> 
> Some behaviour that might be a little unexpected:
> - SNMPv2-SMI, SNMPv2-TC, and SNMPv2-CONF use internal definitions and
>   if encountered while parsing will be skipped.
> - As per RFC2578 section 3: All information modules start with exactly
>   one invocation of the MODULE-IDENTITY macro, which provides contact
>   information as well as revision history to distinguish between
>   versions of the same information module. This invocation must appear
>   immediately after any IMPORTs statements.
>   This means that e.g. OPENBSD-SNMPD-CONF and IPV6-TC won't parse.
> - If a descriptor in IMPORTS can't be resolved (in a case-sensitive
>   manner) the module doing the IMPORTS will also be dropped. This means
>   that IPV6-MIB, IPV6-ICMP-MIB, IPV6-TCP-MIB, and IPV6-UDP-MIB won't
>   load because they require IPV6-TC. This isn't a big problem, since
>   snmpd(8) doesn't support this MIB-suite, which also has been declared
>   obsolete by RFC8096.
> - I expect that people will include quite a few invalid MIBs that won't
>   parse at some point. To prevent memory leaks no pointers are passed
>   around inside yyparse(), unless directly hooked into the current
>   module. This means that memory usage can be quite a bit bigger during
>   startup, but unless you load in the entire librenms MIB-suite I would
>   argue nothing outrageous.
> - The MIB-database is needed in both the parent and the snmpe process,
>   this unfortunately means that all MIB files have to be parsed twice.
> 
> What would need to change in /usr/share/snmp/mibs?
> OPENBSD-SNMPD-CONF doesn't parse, it doesn't contain any definitions and
> is only used to trick net-snmp into loading a bunch of other MIBs. I've
> discussed this with sthen@ and it should be easy to work around this in
> the net-snmp port and remove this file.
> As for the MIB-files that need to be added. I've identified support for
> the following MIBs by snmpd(8) (and vmd(8)):
> - BRIDGE-MIB
> - HOST-RESOURCES-MIB
> - IF-MIB
> - IP-FORWARD-MIB
> - IP-MIB
> - SNMP-FRAMEWORK-MIB
> - SNMP-USER-BASED-SM-MIB
> - SNMPv2-MIB
> - UCD-DISKIO-MIB
> - VM-MIB
> This would require including:
> - IANA-RTPROTO-MIB (for IP-FORWARD-MIB)
> - IANA-STORAGE-MEDIA-TYPE-MIB (for VM-MIB)
> - IANAifType-MIB (for IF-MIB)
> - INET-ADDRESS-MIB (for IP{,-FORWARD}-MIB)
> - UCD-SNMP-MIB (for UCD-DISKIO-MIB)
> - UUID-TC-MIB (for VM-MIB)
> - HCNUM-TC (for UCD-SNMP-MIB)
> And some soft dependencies, that could be returned as values, but not
> strict dependencies:
> - SNMP-USM-AES-MIB
> - SNMP-USM-HMAC-SHA2-MIB
> - SNMPv2-TM
> - TRANSPORT-ADDRESS-MIB
> And although completely ignored by mib.y, it's probably useful to
> include SNMPv2-{CONF,SMI,TC} for people wanting to inspect things
> manually or for tools not using internal definitions.
> There's more that could be interesting, but let's stick with this
> for now and discuss others as they come up.
> This would leave us a total of 30 files and ~900kb of diskspace.
> Although I'm relatively certain we can include the IANA/IETF MIBs,
> I'm not sure about the 2 UCD ones, which are in the care of net-snmp.
> 
> So with mib.y and these MIB-files installed, what does that leave
> us with? mib.h currently has 620 OIDs, where mib.y results in a
> total of 1420. For performance I've tested this on my modern intel
> laptop with nvme, as well as my Alpha XP1000 with spinning disk.
> Startup time up to and including the printing of the
> "snmpe <engineid> ready" message.
> For Alpha I go from 0.2s up to ~5.4s. On my intel machine I go from
> 0.03s up to 0.2s. For a full walk (with debugging on) I haven't seen any
> noticeable difference in speed. On both platforms the memory footprint
> grows by about 3M to about 5M/6M. During parsing I've seen memory spike
> up to 18M (which is a bit harder to properly trace).
> 
> Since I've mentioned the full librenms suite, here's the numbers from
> my laptop as a fun gimmick: startup time is around 82s with memory
> spiking to about 1100M and settling at about 108M.
> 
> martijn@
>