Skip to content

Additional SNMP library output formatting options - #21502

Open
eskyuu wants to merge 19 commits into
php:masterfrom
eskyuu:snmp-displayoptions
Open

Additional SNMP library output formatting options#21502
eskyuu wants to merge 19 commits into
php:masterfrom
eskyuu:snmp-displayoptions

Conversation

@eskyuu

@eskyuu eskyuu commented Mar 23, 2026

Copy link
Copy Markdown
Contributor

RFC: https://wiki.php.net/rfc/snmp_improvements_2026

The SNMP library has more formatting options than php-snmp allows. This PR adds some additional functions to control how the net-snmp library returns results to PHP.

Comment thread ext/snmp/snmp.c Outdated
@eskyuu
eskyuu marked this pull request as ready for review April 8, 2026 08:24
@eskyuu
eskyuu requested a review from kocsismate as a code owner April 8, 2026 08:24
@eskyuu
eskyuu marked this pull request as draft May 14, 2026 05:32
@TimWolla TimWolla added the RFC label Jun 11, 2026
@eskyuu
eskyuu force-pushed the snmp-displayoptions branch from 0a5be4a to 443eb75 Compare June 14, 2026 00:20
@eskyuu
eskyuu force-pushed the snmp-displayoptions branch from 443eb75 to 1de112d Compare June 14, 2026 00:32
@eskyuu
eskyuu marked this pull request as ready for review June 14, 2026 00:32
@eskyuu

eskyuu commented Jun 28, 2026

Copy link
Copy Markdown
Contributor Author

The net-snmp library has quite a few options for controlling how OIDs and values are displayed. PHP already has a number of examples of these:
https://www.php.net/manual/en/function.snmp-set-oid-output-format.php

The base purpose of this PR is to extend the number of options available to include all options supported by the net-snmp library. The SNMP library configuration is stored in global memory, so some of the code takes care of handling the state of the library when using the PHP SNMP object.

The implementation is to add 3 new functions that control all options for MIB parsing, string output and value output. These do duplicate the existing functions like snmp_set_enum_print(), snmp_set_quick_print(), but having both doesn't cause any issues. I think the new interface is more flexible (we only need a new ENUM for each new options instead of a get/set), and there is no harm in having both functions available for existing options. There is an option to deprecate the old functions if you want to standardise the way people control the library (and also to add get functions to read the current state of a given option).

I also added all new options as parameters to the PHP SNMP object.

Whenever a SNMP query is run from a PHP SNMP object, the existing state of the net-snmp library needs to be saved, then updated to match what the SNMP object wants, and finally restored back to the original state. I have created 2 functions (save_snmplib_output_options() and set_snmplib_output_options()) to make it easier to save and restore the state.

I also discovered that changes to the SNMP library state were persisting across PHP-FPM requests. I did this by running a script that updated the state, then running another script many times that performed a SNMP query and printed the output. The query script would produce inconsistent output depending on which FPM process handles the request. This led me to add PHP_RINIT_FUNCTION() and PHP_RSHUTDOWN_FUNCTION() code into the SNMP module to save the state of the SNMP library before a FPM request, and then restore it at the end.

Comment thread ext/snmp/php_snmp.h Outdated
Comment thread ext/snmp/snmp.c Outdated
Comment thread ext/snmp/snmp.c Outdated
Comment thread ext/snmp/snmp.c Outdated
Comment thread ext/snmp/snmp.stub.php Outdated
Comment on lines +93 to +107
/**
* @var int
* @cvalue NETSNMP_STRING_OUTPUT_GUESS
*/
const SNMP_STRING_OUTPUT_GUESS = UNKNOWN;
/**
* @var int
* @cvalue NETSNMP_STRING_OUTPUT_ASCII
*/
const SNMP_STRING_OUTPUT_ASCII = UNKNOWN;
/**
* @var int
* @cvalue NETSNMP_STRING_OUTPUT_HEX
*/
const SNMP_STRING_OUTPUT_HEX = UNKNOWN;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can these be combined? If not maybe an enum is better so there is type safety.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean by type safety? I've extended the original technique used in the php-snmp code to copy the NETSNMP* enums directly so I can use pass them through to the netsnmp library calls. All values are checked before being used.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I mean was to use a PHP enum that translates to the C values (by using and use Z_PARAM_ENUM), rather than passing an int. It's type safety for the end user writing PHP code.

But if those contants can be bitmasked, that technique doesn't work.

@eskyuu eskyuu Aug 20, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this look now?

The only issue I can see is that the proposal was to merge this into the next release for all current versions, but Z_PARAM_ENUM() is not implemented in PHP 8.4 (and 8.5?)

Comment thread ext/snmp/snmp.stub.php Outdated
Comment thread ext/snmp/snmp.stub.php Outdated
@eskyuu
eskyuu requested a review from Girgias July 11, 2026 05:25
@eskyuu
eskyuu force-pushed the snmp-displayoptions branch from a23a795 to a468bbf Compare August 20, 2026 01:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants