From: Boris Lytochkin Date: Wed, 31 Aug 2011 08:36:22 +0000 (+0000) Subject: more tuning based on discussion in FR #40816: X-Git-Tag: php-5.4.0beta1~323 X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=3aa6f31abd1630b7f40e7b18f80b4912261c73f5;p=php more tuning based on discussion in FR #40816: * parse all OIDs earlier, detect all wrong OIDs before any query is made (GET-operations) * introduce ERRNO_MULTIPLE_SET_QUERIES: warn if request contains more OIDs than max_oids and SET operation (and type&value checks) will be done in chunks. fix set method when request contains more OIDs than max_oids (2nd and subsequent chunk were ignored) --- diff --git a/ext/snmp/snmp.c b/ext/snmp/snmp.c index 039903d489..b41938f27d 100644 --- a/ext/snmp/snmp.c +++ b/ext/snmp/snmp.c @@ -110,6 +110,7 @@ typedef struct snmp_session php_snmp_session; #define PHP_SNMP_ERRNO_ERROR_IN_REPLY 3 #define PHP_SNMP_ERRNO_OID_NOT_INCREASING 4 #define PHP_SNMP_ERRNO_OID_PARSING_ERROR 5 +#define PHP_SNMP_ERRNO_MULTIPLE_SET_QUERIES 6 ZEND_DECLARE_MODULE_GLOBALS(snmp) static PHP_GINIT_FUNCTION(snmp); @@ -670,11 +671,8 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st, struct snmp_session *ss; struct snmp_pdu *pdu=NULL, *response; struct variable_list *vars; - oid name[MAX_NAME_LEN]; - size_t name_length; oid root[MAX_NAME_LEN]; size_t rootlen = 0; - int gotroot = 0; int status, count, found; char buf[2048]; char buf2[2048]; @@ -689,35 +687,10 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st, /* reset errno and errstr */ php_snmp_error(getThis(), NULL TSRMLS_CC, PHP_SNMP_ERRNO_NOERROR, ""); - if (st & SNMP_CMD_WALK) { - if (objid_query->count > 1) { - php_snmp_error(getThis(), NULL TSRMLS_CC, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Multi OID walks are not supported!"); - RETURN_FALSE; - } - rootlen = MAX_NAME_LEN; - if (strlen(objid_query->vars[0].oid)) { /* on a walk, an empty string means top of tree - no error */ - if (snmp_parse_oid(objid_query->vars[0].oid, root, &rootlen)) { - gotroot = 1; - } else { - php_snmp_error(getThis(), NULL TSRMLS_CC, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[0].oid); - RETVAL_FALSE; - return; - } - } - - if (!gotroot) { - memmove((char *) root, (char *) objid_mib, sizeof(objid_mib)); - rootlen = sizeof(objid_mib) / sizeof(oid); - gotroot = 1; - } - - memmove((char *)name, (char *)root, rootlen * sizeof(oid)); - name_length = rootlen; + if (st & SNMP_CMD_WALK) { /* remember root OID */ + memmove((char *)root, (char *)(objid_query->vars[0].name), (objid_query->vars[0].name_length) * sizeof(oid)); + rootlen = objid_query->vars[0].name_length; objid_query->offset = objid_query->count; - - memmove((char *)objid_query->vars[0].name, (char *)root, rootlen * sizeof(oid)); - objid_query->vars[0].name_length = rootlen; - } if ((ss = snmp_open(session)) == NULL) { @@ -728,6 +701,10 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st, return; } + if ((st & SNMP_CMD_SET) && objid_query->count > objid_query->step) { + php_snmp_error(getThis(), NULL TSRMLS_CC, PHP_SNMP_ERRNO_MULTIPLE_SET_QUERIES, "Can not fit all OIDs for SET query into one packet, using multiple queries"); + } + while (keepwalking) { keepwalking = 0; if (st & SNMP_CMD_WALK) { @@ -738,7 +715,7 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st, pdu->non_repeaters = objid_query->non_repeaters; pdu->max_repetitions = objid_query->max_repetitions; } - snmp_add_null_var(pdu, name, name_length); + snmp_add_null_var(pdu, objid_query->vars[0].name, objid_query->vars[0].name_length); } else { if (st & SNMP_CMD_GET) { pdu = snmp_pdu_create(SNMP_MSG_GET); @@ -753,14 +730,6 @@ static void php_snmp_internal(INTERNAL_FUNCTION_PARAMETERS, int st, return; } for (count = 0; objid_query->offset < objid_query->count && count < objid_query->step; objid_query->offset++, count++){ - objid_query->vars[objid_query->offset].name_length = MAX_OID_LEN; - if (!snmp_parse_oid(objid_query->vars[objid_query->offset].oid, objid_query->vars[objid_query->offset].name, &(objid_query->vars[objid_query->offset].name_length))) { - php_snmp_error(getThis(), NULL TSRMLS_CC, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[objid_query->offset].oid); - snmp_free_pdu(pdu); - snmp_close(ss); - RETVAL_FALSE; - return; - } if (st & SNMP_CMD_SET) { if ((snmp_errno = snmp_add_var(pdu, objid_query->vars[objid_query->offset].name, objid_query->vars[objid_query->offset].name_length, objid_query->vars[objid_query->offset].type, objid_query->vars[objid_query->offset].value))) { snprint_objid(buf, sizeof(buf), objid_query->vars[objid_query->offset].name, objid_query->vars[objid_query->offset].name_length); @@ -787,6 +756,10 @@ retry: if (status == STAT_SUCCESS) { if (response->errstat == SNMP_ERR_NOERROR) { if (st & SNMP_CMD_SET) { + if (objid_query->offset < objid_query->count) { /* we have unprocessed OIDs */ + keepwalking = 1; + continue; + } snmp_free_pdu(response); snmp_close(ss); RETVAL_TRUE; @@ -870,13 +843,13 @@ retry: /* OID increase check */ if (st & SNMP_CMD_WALK) { - if (objid_query->oid_increasing_check == TRUE && snmp_oid_compare(name, name_length, vars->name, vars->name_length) >= 0) { + if (objid_query->oid_increasing_check == TRUE && snmp_oid_compare(objid_query->vars[0].name, objid_query->vars[0].name_length, vars->name, vars->name_length) >= 0) { snprint_objid(buf2, sizeof(buf2), vars->name, vars->name_length); php_snmp_error(getThis(), NULL TSRMLS_CC, PHP_SNMP_ERRNO_OID_NOT_INCREASING, "Error: OID not increasing: %s", buf2); keepwalking = 0; } else { - memmove((char *)name, (char *)vars->name, vars->name_length * sizeof(oid)); - name_length = vars->name_length; + memmove((char *)(objid_query->vars[0].name), (char *)vars->name, vars->name_length * sizeof(oid)); + objid_query->vars[0].name_length = vars->name_length; keepwalking = 1; } } @@ -950,7 +923,7 @@ retry: * OID parser (and type, value for SNMP_SET command) */ -static int php_snmp_parse_oid(int st, struct objid_query *objid_query, zval **oid, zval **type, zval **value TSRMLS_DC) +static int php_snmp_parse_oid(zval *object, int st, struct objid_query *objid_query, zval **oid, zval **type, zval **value TSRMLS_DC) { char *pptr; HashPosition pos_oid, pos_type, pos_value; @@ -1070,6 +1043,34 @@ static int php_snmp_parse_oid(int st, struct objid_query *objid_query, zval **oi } } + /* now parse all OIDs */ + if (st & SNMP_CMD_WALK) { + if (objid_query->count > 1) { + php_snmp_error(object, NULL TSRMLS_CC, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Multi OID walks are not supported!"); + efree(objid_query->vars); + return FALSE; + } + objid_query->vars[0].name_length = MAX_NAME_LEN; + if (strlen(objid_query->vars[0].oid)) { /* on a walk, an empty string means top of tree - no error */ + if (!snmp_parse_oid(objid_query->vars[0].oid, objid_query->vars[0].name, &(objid_query->vars[0].name_length))) { + php_snmp_error(object, NULL TSRMLS_CC, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[0].oid); + efree(objid_query->vars); + return FALSE; + } + } else { + memmove((char *)objid_query->vars[0].name, (char *)objid_mib, sizeof(objid_mib)); + objid_query->vars[0].name_length = sizeof(objid_mib) / sizeof(oid); + } + } else { + for (objid_query->offset = 0; objid_query->offset < objid_query->count; objid_query->offset++) { + objid_query->vars[objid_query->offset].name_length = MAX_OID_LEN; + if (!snmp_parse_oid(objid_query->vars[objid_query->offset].oid, objid_query->vars[objid_query->offset].name, &(objid_query->vars[objid_query->offset].name_length))) { + php_snmp_error(object, NULL TSRMLS_CC, PHP_SNMP_ERRNO_OID_PARSING_ERROR, "Invalid object identifier: %s", objid_query->vars[objid_query->offset].oid); + efree(objid_query->vars); + return FALSE; + } + } + } objid_query->offset = 0; objid_query->step = objid_query->count; return (objid_query->count > 0); @@ -1450,7 +1451,7 @@ static void php_snmp(INTERNAL_FUNCTION_PARAMETERS, int st, int version) } } - if (!php_snmp_parse_oid(st, &objid_query, oid, type, value TSRMLS_CC)) { + if (!php_snmp_parse_oid(getThis(), st, &objid_query, oid, type, value TSRMLS_CC)) { RETURN_FALSE; } @@ -2370,6 +2371,7 @@ PHP_MINIT_FUNCTION(snmp) REGISTER_SNMP_CLASS_CONST_LONG("ERRNO_ERROR_IN_REPLY", (long)PHP_SNMP_ERRNO_ERROR_IN_REPLY); REGISTER_SNMP_CLASS_CONST_LONG("ERRNO_OID_NOT_INCREASING", (long)PHP_SNMP_ERRNO_OID_NOT_INCREASING); REGISTER_SNMP_CLASS_CONST_LONG("ERRNO_OID_PARSING_ERROR", (long)PHP_SNMP_ERRNO_OID_PARSING_ERROR); + REGISTER_SNMP_CLASS_CONST_LONG("ERRNO_MULTIPLE_SET_QUERIES", (long)PHP_SNMP_ERRNO_MULTIPLE_SET_QUERIES); return SUCCESS; } diff --git a/ext/snmp/tests/snmp-object.phpt b/ext/snmp/tests/snmp-object.phpt index 7830ec2cd7..06b6492bd7 100644 --- a/ext/snmp/tests/snmp-object.phpt +++ b/ext/snmp/tests/snmp-object.phpt @@ -122,6 +122,23 @@ var_dump($z); var_dump(($session->get($oid1) === $oldvalue1)); var_dump($session->close()); +echo "Multiple OID with max_oids = 1\n"; +$oid2 = 'SNMPv2-MIB::sysLocation.0'; +$session = new SNMP(SNMP::VERSION_3, $hostname, $rwuser, $timeout, $retries); +$session->setSecurity('authPriv', 'MD5', $auth_pass, 'AES', $priv_pass); +$session->max_oids = 1; +$oldvalue2 = $session->get($oid2); +$newvalue2 = $oldvalue2 . '0'; +$z = $session->set(array($oid1, $oid2), array('s','s'), array($newvalue1, $newvalue2)); +var_dump($z); +var_dump(($session->get($oid1) === $newvalue1)); +var_dump(($session->get($oid2) === $newvalue2)); +$z = $session->set(array($oid1, $oid2), array('s','s'), array($oldvalue1, $oldvalue2)); +var_dump($z); +var_dump(($session->get($oid1) === $oldvalue1)); +var_dump(($session->get($oid2) === $oldvalue2)); +var_dump($session->close()); + echo "SNMPv3, setting contextEngineID (authPriv)\n"; $session = new SNMP(SNMP::VERSION_3, $hostname, $rwuser, $timeout, $retries); $session->setSecurity('authPriv', 'MD5', $auth_pass, 'AES', $priv_pass, '', 'aeeeff'); @@ -200,6 +217,18 @@ bool(true) bool(true) bool(true) bool(true) +Multiple OID with max_oids = 1 + +Warning: SNMP::set(): Can not fit all OIDs for SET query into one packet, using multiple queries in %s on line %d +bool(true) +bool(true) +bool(true) + +Warning: SNMP::set(): Can not fit all OIDs for SET query into one packet, using multiple queries in %s on line %d +bool(true) +bool(true) +bool(true) +bool(true) SNMPv3, setting contextEngineID (authPriv) string(%d) "%S" bool(true) diff --git a/ext/snmp/tests/snmp3-error.phpt b/ext/snmp/tests/snmp3-error.phpt index b2e51da3f9..3c0c499221 100644 --- a/ext/snmp/tests/snmp3-error.phpt +++ b/ext/snmp/tests/snmp3-error.phpt @@ -17,14 +17,14 @@ echo "Checking error handling\n"; // string object_id [, int timeout [, int retries]]); var_dump(snmp3_get($hostname, $community, '', '', '', '', '')); -var_dump(snmp3_get($hostname, $community, '', '', '', '', '', '')); -var_dump(snmp3_get($hostname, $community, 'bugusPriv', '', '', '', '', '')); -var_dump(snmp3_get($hostname, $community, 'authNoPriv', 'TTT', '', '', '', '')); -var_dump(snmp3_get($hostname, $community, 'authNoPriv', 'MD5', '', '', '', '')); -var_dump(snmp3_get($hostname, $community, 'authNoPriv', 'MD5', 'te', '', '', '')); -var_dump(snmp3_get($hostname, $community, 'authPriv', 'MD5', $auth_pass, 'BBB', '', '')); -var_dump(snmp3_get($hostname, $community, 'authPriv', 'MD5', $auth_pass, 'AES', '', '')); -var_dump(snmp3_get($hostname, $community, 'authPriv', 'MD5', $auth_pass, 'AES', 'ty', '')); +var_dump(snmp3_get($hostname, $community, '', '', '', '', '', '.1.3.6.1.2.1.1.1.0')); +var_dump(snmp3_get($hostname, $community, 'bugusPriv', '', '', '', '', '.1.3.6.1.2.1.1.1.0')); +var_dump(snmp3_get($hostname, $community, 'authNoPriv', 'TTT', '', '', '', '.1.3.6.1.2.1.1.1.0')); +var_dump(snmp3_get($hostname, $community, 'authNoPriv', 'MD5', '', '', '', '.1.3.6.1.2.1.1.1.0')); +var_dump(snmp3_get($hostname, $community, 'authNoPriv', 'MD5', 'te', '', '', '.1.3.6.1.2.1.1.1.0')); +var_dump(snmp3_get($hostname, $community, 'authPriv', 'MD5', $auth_pass, 'BBB', '', '.1.3.6.1.2.1.1.1.0')); +var_dump(snmp3_get($hostname, $community, 'authPriv', 'MD5', $auth_pass, 'AES', '', '.1.3.6.1.2.1.1.1.0')); +var_dump(snmp3_get($hostname, $community, 'authPriv', 'MD5', $auth_pass, 'AES', 'ty', '.1.3.6.1.2.1.1.1.0')); var_dump(snmp3_get($hostname, 'somebogususer', 'authPriv', 'MD5', $auth_pass, 'AES', $priv_pass, '.1.3.6.1.2.1.1.1.0', $timeout, $retries)); var_dump(snmp3_set($hostname, $community, 'authPriv', 'MD5', $auth_pass, 'AES', $priv_pass, '', 's'));