Hi All,
I would like to create an RFC proposal to add a new function snmp_init_mib() that will shut down and then re-initialise the MIB tree within the lib-snmp library. This allows the PHP code to define the SNMP environment variables, and then initialise the MIB tree without importing any MIBs from the global config into the MIB tree.
Another consideration is that the MIB tree persists across FPM requests because the MIB tree is kept as a global structure within SNMP library memory. This proposal would allow the MIB to be reset within a request if needed.
Something like this function is required if you want to run multiple SNMP queries for different MIBs that define the same OIDs differently. I have come across a few vendor MIB files where this happens, and the php snmp module cannot work without the ability to reset the MIB tree, or without forking an external process for these specific MIBs.
I believe that the process is to send a message to the list, and gauge whether there is positive feedback before creating the RFC for voting. Please let me know if I should create the RFC first?
The PR is here: https://github.com/php/php-src/pull/21452/changes
regards
Steve
Hi
I believe that the process is to send a message to the list, and gauge whether there is positive feedback before creating the RFC for voting. Please let me know if I should create the RFC first?
The PR is here: https://github.com/php/php-src/pull/21452/changes
I suspect that SNMP is such a specialized and rarely used extension that
folks will be unable to meaningfully discuss an RFC for this feature -
at least not without you doing quite a bit of explaining. This might be
case where it's best to just find an informal agreement with you
providing a well-tested and well-explained patch rather than doing a
full RFC.
That said, I did some research, finding this man page:
https://linux.die.net/man/3/shutdown_mib.
I'm seeing that the manpage states for init_mib():
It should be called before any other routine that manipulates or accesses the MIB tree.
I'm seing that init_mib() is not currently called at all in ext/snmp. Is
the initialization implicitly happening as part of init_snmp()?
For shutdown_mib() I'm seeing:
It is strongly recommended that one does not invoke shutdown_mib while there are SNMP sessions being actively managed.
Will calling your proposed snmp_init_mib() function cause issues when
an existing SNMP object is being used after the call to snmp_init_mib()?
In your email you mention:
Another consideration is that the MIB tree persists across FPM requests because the MIB tree is kept as a global structure within SNMP library memory.
Would calling shutdown_mib() in RSHUTDOWN (i.e. the end-of-request
handler) solve the problem of the MIB persisting across requests or is
it sometimes desirable that the MIB is persisted?
Best regards
Tim Düsterhus
Hi
I believe that the process is to send a message to the list, and
gauge whether there is positive feedback before creating the RFC for
voting. Please let me know if I should create the RFC first?
The PR is here: https://github.com/php/php-src/pull/21452/changesI suspect that SNMP is such a specialized and rarely used extension
that folks will be unable to meaningfully discuss an RFC for this
feature - at least not without you doing quite a bit of explaining.
This might be case where it's best to just find an informal agreement
with you providing a well-tested and well-explained patch rather than
doing a full RFC.
I'm very happy to avoid the RFC process, and happy to explain/discuss
the changes :)
That said, I did some research, finding this man page:
https://linux.die.net/man/3/shutdown_mib.I'm seeing that the manpage states for init_mib():
It should be called before any other routine that manipulates or
accesses the MIB tree.I'm seing that init_mib() is not currently called at all in ext/snmp.
Is the initialization implicitly happening as part of init_snmp()?For shutdown_mib() I'm seeing:
It is strongly recommended that one does not invoke shutdown_mib
while there are SNMP sessions being actively managed.Will calling your proposed
snmp_init_mib()function cause issues
when an existing SNMP object is being used after the call to
snmp_init_mib()?
I have interpreted the "actively managed" sessions as ones with requests
in process (i.e. requests have been sent and we are using some form of
select() to wait for the reply). The other possibility is that calling
mib_shutdown() would cause issues if a request is sent before
iinit_mib() is called.
I have written a test program that keeps a session across a
mib_shutdown()/mib_init() with a query running on both sides, and it
worked as expected with no memory access issues.
In your email you mention:
Another consideration is that the MIB tree persists across FPM
requests because the MIB tree is kept as a global structure within
SNMP library memory.Would calling
shutdown_mib()in RSHUTDOWN (i.e. the end-of-request
handler) solve the problem of the MIB persisting across requests or is
it sometimes desirable that the MIB is persisted?
This is precisely why I mentioned the persistence of the MIB tree.
There is no reason to keep the tree between requests, so doing a
shutdown_mib() followed by an init_mib() between requests would be a
good idea, with the following caveats:
- init_mib() depends on some environment variables. I assume the
environment is reset between requests, and if so init_mib() would
need to be called between requests - init_mib() does read a bunch of files to parse the MIB tree, so it
would be inefficient to call it between requests if no changes have
been made to the MIB tree - I have just had a PR accepted in net-snmp to address a memory leak
in the library when init_mib() is called multiple times, so it would
cause a memory leak between requests until systems have been updated
with this new patch
Best regards
Tim Düsterhus
Hi
I believe that the process is to send a message to the list, and
gauge whether there is positive feedback before creating the RFC for
voting. Please let me know if I should create the RFC first?
The PR is here: https://github.com/php/php-src/pull/21452/changesI suspect that SNMP is such a specialized and rarely used extension
that folks will be unable to meaningfully discuss an RFC for this
feature - at least not without you doing quite a bit of explaining.
This might be case where it's best to just find an informal agreement
with you providing a well-tested and well-explained patch rather than
doing a full RFC.I'm very happy to avoid the RFC process, and happy to explain/discuss
the changes :)
On the topic of the RFC - @devnexen has suggested that a RFC is the
correct process for this change. I have no issues with following
whatever process is required to get this considered, but if most
maintainers won't be too concerned about changes to the module I am
interested who I should be discussing with that does know the code well
enough to make the decision on including it or not.
I have submitted a second PR below to add support for other SNMPv3
security protocols, and am wondering if I do end up going down the RFC
route whether it is best to merge everything into a single PR for "SNMP
changes", or if distinct changes should have their own PR with an
associated RFC?
https://github.com/php/php-src/pull/21451
While considering the previous question, I would like to add that I
would also like to add more options to control the output format. These
would just be duplicating the code for enum_print, quick_print and
oid_output_format. The question here is whether it should be a new
request with a new RFC, or if this is trivial enough to be considered
independently.
regards
Steve