[SNMP4J] Counting Invalid Traps
Elise Atkins
elise.atkins at tavve.com
Mon Nov 2 20:14:34 CET 2009
Frank,
First off, I am not that familiar with all the snmp rfs so I did a bit
of googling and using rfcs 1907 & 2572 I
came up with definitions for two of the counters and what they should count:
snmpASNParseErrors counter is defined in rfc 1907
On page 8 of rfc 1907 the counter is defined
snmpInASNParseErrs OBJECT-TYPE
SYNTAX Counter32
MAX-ACCESS read-only
STATUS current
DESCRIPTION
"The total number of ASN.1 or BER errors encountered by the
SNMP entity when decoding received SNMP messages."
::= { snmp 6 }
From this I think that when the MessageDispatcherImpl.processMessage
catches the IOException, the counter that is incremented should be
snmpASNParseErrors not snmpInvalidMsgs since the only time I found
IOException was thrown when processing a PDU was when the BER decoding
encounters an error. If you concur with this hypothesis then the check
for the packet starting with an ASN.1 sequence (0x30) at the beginning
of processMessage should just return after incrementing the counter or
throw an IOException and let the catch IOException block further down
handle the counter incrementing.
snmpInvalidMsgs counter looks like it was added for snmp v3 pdus. The
relevant sections are:
On page 18 of rfc 2572 the counter is defined
snmpInvalidMsgs OBJECT-TYPE
SYNTAX Counter32
MAX-ACCESS read-only
STATUS current
DESCRIPTION "The total number of packets received by the SNMP
engine which were dropped because there were invalid
or inconsistent components in the SNMP message.
"
::= { snmpMPDStats 2 }
On page 31 of rfc 2572 the only reference I found for when to set the
counter.
If the authFlag is not set and privFlag is set, then the
snmpInvalidMsgs counter is incremented, the message is
discarded without further processing, and a FAILURE result is
returned. SNMPv3 Message Processing is complete.
And I found in the MPv3.java line 1044 where this error is counted.
My goat is count all the packets that are discarded in processing. It
seems to me that if you add up all the various error counters used in
processing a pdu that result in discarding the pdu and subtract this
number from the snmpInPackets the result should the number of packets
that make it to the CommandResponder.processPdu method. For this to be
true, a packet is can only increment an error counter once.
Sincerely,
Elise Atkins
Frank Fock wrote:
> Elise,
>
> Where are the code locations where you think errors
> are counted twice?
>
> Do you have a RFC pointer that notes that a BER encoding
> error must not counted as an invalid message?
>
> Thanks,
> Frank
>
> Elise Atkins wrote:
>> Frank,
>>
>> Thanks for making the change so quickly. I have one nit picky
>> comment. If you have a ASN.1 parse error, it gets counted as an error
>> and then processing continues and the MPv1 class throws an exception
>> and it counted again as an invalid packet. A cleaner solution to
>> prevent double counting would be to add a return after the first
>> counter event is fired or log it and not count it and continue
>> processing and have the catch(IOException) block count it.
>>
>> Thanks,
>> Elise
>>
>> Frank Fock wrote:
>>> Hi Elise,
>>>
>>> Thank you for reporting this bug [SJF-18].
>>> The current snapshot contains a fix for it,
>>> which is slightly different to yours.
>>>
>>> Regards,
>>> Frank
>>>
>>> Elise Atkins wrote:
>>>> I have used SNMP4J for several years and have been very satisfied.
>>>> Recently I did some testing to see what happens when invalid V1
>>>> traps and snmp requests were received by the snmp4j code. A
>>>> CounterListner was added to the MessageDispatcherImpl and the
>>>> number of incoming traps/requests were accurately counted but the
>>>> number of invalid traps was not. The large majority of traps were
>>>> silently discarded. After looking at the following code snippet
>>>> from MessageDispatcherImpl and the MPv1.java code I concluded that
>>>> when the MPv1 encountered an error it threw an exception that was
>>>> caught by the catch block near the end of the snippet. The packet
>>>> is then silently discarded unless the
>>>> Snmp4JSettings.forwardRuntimeExceptions is true and the
>>>> TransportMapping class catches and counts the exception.
>>>>
>>>> I think that a better solution would be to add the following lines
>>>> the catch block:
>>>> CounterEvent event = new CounterEvent(this,
>>>>
>>>> SnmpConstants.snmpInvalidMsgs);
>>>> fireIncrementCounter(event);
>>>>
>>>> The packet would be then counted as invalid. I did set the
>>>> forwardRuntimeExceptions to true and was able to get a count of the
>>>> invalid packets and all seemed ok until a little later. I also use
>>>> AgentXSubagent to connect to the snmpd service. When that service
>>>> was not running or misconfigured and the subagent could not
>>>> connect, a socket was orphaned and eventually things stopped
>>>> working because there were too many sockets opened. I fixed that by
>>>> calling close and disconnect on the subagent when the connect
>>>> method failed. But I am now worried that having the
>>>> forwardRuntimeExceptions set to true will cause additional problems
>>>> down the line.
>>>>
>>>> public void processMessage(TransportMapping sourceTransport,
>>>> Address incomingAddress,
>>>> BERInputStream wholeMessage) {
>>>> fireIncrementCounter(new CounterEvent(this,
>>>> SnmpConstants.snmpInPkts));
>>>> if (!wholeMessage.markSupported()) {
>>>> String txt = "Message stream must support marks";
>>>> logger.error(txt);
>>>> throw new IllegalArgumentException(txt);
>>>> }
>>>> try {
>>>> wholeMessage.mark(16);
>>>> BER.MutableByte type = new BER.MutableByte();
>>>> // decode header but do not check length here, because we do
>>>> only decode
>>>> // the first 16 bytes.
>>>> BER.decodeHeader(wholeMessage, type, false);
>>>> if (type.getValue() != BER.SEQUENCE) {
>>>> logger.error("ASN.1 parse error (message is not a sequence)");
>>>> CounterEvent event = new CounterEvent(this,
>>>>
>>>> SnmpConstants.snmpInASNParseErrs);
>>>> fireIncrementCounter(event);
>>>> }
>>>> Integer32 version = new Integer32();
>>>> version.decodeBER(wholeMessage);
>>>> MessageProcessingModel mp =
>>>> getMessageProcessingModel(version.getValue());
>>>> if (mp == null) {
>>>> logger.warn("SNMP version "+version+" is not supported");
>>>> CounterEvent event = new CounterEvent(this,
>>>>
>>>> SnmpConstants.snmpInBadVersions);
>>>> fireIncrementCounter(event);
>>>> }
>>>> else {
>>>> // reset it
>>>> wholeMessage.reset();
>>>> // dispatch it
>>>> dispatchMessage(sourceTransport, mp, incomingAddress,
>>>> wholeMessage);
>>>> }
>>>> }
>>>> catch (Exception ex) {
>>>> logger.error(ex);
>>>> if (logger.isDebugEnabled()) {
>>>> ex.printStackTrace();
>>>> }
>>>> if (SNMP4JSettings.isFowardRuntimeExceptions()) {
>>>> throw new RuntimeException(ex);
>>>> }
>>>> }
>>>> catch (OutOfMemoryError oex) {
>>>> logger.error(oex);
>>>> if (SNMP4JSettings.isFowardRuntimeExceptions()) {
>>>> throw oex;
>>>> }
>>>> }
>>>> }
>>>>
>>>> Sincerely,
>>>> Elise Atkins
>>>>
>>
>>
>
--
ZoneRanger "Management Through Firewalls" <http://www.tavve.com/?leadlander>
More information about the SNMP4J
mailing list