From 9b5b085ddb5ee649a7fc518ad534d513c68ad699 Mon Sep 17 00:00:00 2001 From: Stefan Zegenhagen Date: Tue, 31 Jul 2012 10:38:49 +0200 Subject: [snmp/agent] Errors in vacmAccessTable RowStatus handling There are problems with the handling of vacmAccessTableStatus that cause some SNMP test suites to report errors. Most notably, errorneous set operations frequently cause "genErr" errors to be returned which usually is a sign for bad coding. These "genErr" errors are usually caused by badmatch exceptions coming from {ok, Row} = snmpa_vacm:get_row(RowIndex) if the row does not exist. This patch fixes the badmatch errors and adjusts the semantic of the RowStatus handling in that table to be compliant with the RowStatus textual description of SNPMv2-TC MIB. --- lib/snmp/src/agent/snmp_view_based_acm_mib.erl | 121 ++++++++++++++++--------- 1 file changed, 79 insertions(+), 42 deletions(-) (limited to 'lib/snmp') diff --git a/lib/snmp/src/agent/snmp_view_based_acm_mib.erl b/lib/snmp/src/agent/snmp_view_based_acm_mib.erl index 479a44795f..71dcc0f849 100644 --- a/lib/snmp/src/agent/snmp_view_based_acm_mib.erl +++ b/lib/snmp/src/agent/snmp_view_based_acm_mib.erl @@ -565,48 +565,85 @@ vacmAccessTable(is_set_ok, RowIndex, Cols0) -> case (catch verify_vacmAccessTable_cols(Cols0, [])) of {ok, Cols} -> IsValidKey = is_valid_key(RowIndex), - case lists:keysearch(?vacmAccessStatus, 1, Cols) of - %% Ok, if contextMatch is init - {value, {Col, ?'RowStatus_active'}} -> - {ok, Row} = snmpa_vacm:get_row(RowIndex), - case element(?vacmAContextMatch, Row) of - noinit -> {inconsistentValue, Col}; - _ -> {noError, 0} - end; - {value, {Col, ?'RowStatus_notInService'}} -> % Ok, if not notReady - {ok, Row} = snmpa_vacm:get_row(RowIndex), - case element(?vacmAStatus, Row) of - ?'RowStatus_notReady' -> {inconsistentValue, Col}; - _ -> {noError, 0} - end; - {value, {Col, ?'RowStatus_notReady'}} -> % never ok! - {inconsistentValue, Col}; - {value, {Col, ?'RowStatus_createAndGo'}} -> % ok, if it doesn't exist - Res = lists:keysearch(?vacmAccessContextMatch, 1, Cols), - case snmpa_vacm:get_row(RowIndex) of - false when (IsValidKey =:= true) andalso - is_tuple(Res) -> {noError, 0}; - false -> {noCreation, Col}; % Bad RowIndex - _ -> {inconsistentValue, Col} - end; - {value, {Col, ?'RowStatus_createAndWait'}} -> % ok, if it doesn't exist - case snmpa_vacm:get_row(RowIndex) of - false when (IsValidKey =:= true) -> {noError, 0}; - false -> {noCreation, Col}; % Bad RowIndex - _ -> {inconsistentValue, Col} - end; - {value, {_Col, ?'RowStatus_destroy'}} -> % always ok! - {noError, 0}; - _ -> % otherwise, it's a change; it must exist - case snmpa_vacm:get_row(RowIndex) of - {ok, _} -> - {noError, 0}; - false -> - {inconsistentName, element(1, hd(Cols))} - end - end; - Error -> - Error + StatusCol = lists:keyfind(?vacmAccessStatus, 1, Cols), + OldRow = snmpa_vacm:get_row(RowIndex), + case {StatusCol, OldRow} of + {{Col, ?'RowStatus_active'}, false} -> + % row does not yet exist => inconsistentValue + % (see SNMPv2-TC.mib, RowStatus textual convention) + {inconsistentValue, Col}; + {{Col, ?'RowStatus_active'}, {ok, Row}} -> + %% Ok, if contextMatch is init + case element(?vacmAContextMatch, Row) of + noinit -> + % check whether ContextMatch is being set in the same operation + case proplists:get_value(?vacmAccessContextMatch, Cols) of + undefined -> + % no, we can't make this row active yet + {inconsistentValue, Col}; + _ -> + % ok, activate the row + {noError, 0} + end; + _ -> + {noError, 0} + end; + {{Col, ?'RowStatus_notInService'}, false} -> + % row does not yet exist => inconsistentValue + % (see SNMPv2-TC.mib, RowStatus textual convention) + {inconsistentValue, Col}; + {{Col, ?'RowStatus_notInService'}, {ok, Row}} -> + % Ok, if not notReady + case element(?vacmAStatus, Row) of + ?'RowStatus_notReady' -> + {inconsistentValue, Col}; + _ -> + {noError, 0} + end; + {{Col, ?'RowStatus_notReady'}, _} -> + % never ok! + {inconsistentValue, Col}; + {{Col, ?'RowStatus_createAndGo'}, false} -> + % ok, if it doesn't exist + Res = lists:keysearch(?vacmAccessContextMatch, 1, Cols), + if + IsValidKey /= true -> + % bad RowIndex => noCreation + {noCreation, Col}; + is_tuple(Res) -> + % required field is present => noError + {noError, 0}; + true -> + % required field is missing => inconsistentValue + {inconsistentValue, Col} + end; + {{Col, ?'RowStatus_createAndGo'}, _} -> + % row already exists => inconsistentValue + {inconsistentValue, Col}; + {{Col, ?'RowStatus_createAndWait'}, false} -> + % ok, if it doesn't exist + if + IsValidKey =:= true -> + % RowIndex is valid => noError + {noError, 0}; + true -> + {noCreation, Col} + end; + {{Col, ?'RowStatus_createAndWait'}, _} -> + % Row already exists => inconsistentValue + {inconsistentValue, Col}; + {value, {_Col, ?'RowStatus_destroy'}} -> + % always ok! + {noError, 0}; + {_, false} -> + % otherwise, it's a row change; row does not exist => inconsistentName + {inconsistentName, element(1, hd(Cols))}; + _ -> + % row change and row exists => noError + {noError, 0} + end; + Error -> + Error end; vacmAccessTable(set, RowIndex, Cols0) -> case (catch verify_vacmAccessTable_cols(Cols0, [])) of -- cgit v1.2.3