Details

      Description

      Instead of normal message about unmatched trap like this:

        1562:20141117:164856.535 unmatched trap received from [127.0.0.1]: 16:48:55 2014/11/17 PDU INFO:
        notificationtype               TRAP
        version                        0
        receivedfrom                   UDP: [127.0.0.1]:45150->[127.0.0.1]
        errorstatus                    0
        messageid                      0
        community                      public
        transactionid                  1
        errorindex                     0
        requestid                      0
      VARBINDS:
        DISMAN-EVENT-MIB::sysUpTimeInstance type=67 value=Timeticks: (55) 0:00:00.55
        SNMPv2-MIB::snmpTrapOID.0      type=6  value=OID: IF-MIB::linkUp.0.33
        IF-MIB::linkUp                 type=4  value=STRING: "eth2"
        SNMP-COMMUNITY-MIB::snmpTrapCommunity.0 type=4  value=STRING: "public"
        SNMPv2-MIB::snmpTrapEnterprise.0 type=6  value=OID: IF-MIB::linkUp

      I can see this in Zabbix server/proxy log file:
      1) incorrect trap parsing :

       27097:20141113:170456.632 invalid trap found [NG: "Minor"
       27097:20141113:170549.560 invalid trap found [a Protector"
       27097:20141113:170632.077 invalid trap found [ "public"
       27097:20141113:170635.208 invalid trap found [ue=STRING: "public"
       27097:20141113:170910.306 invalid trap found [Cannot open: ([3] The system cannot find the path specified. ) => not 

      2) lines without details about the unmatched trap:

       28599:20141113:014648.727 unmatched trap received from [10.10.10.10]: 01:46:47 2014/11/13 PDU INFO:
       28599:20141113:014648.743 unmatched trap received from [10.10.10.10]: 01:46:47 2014/11/13 PDU INFO:
       28599:20141113:014718.792 unmatched trap received from [10.10.10.10]: 01:47:17 2014/11/13 PDU INFO:
       28599:20141113:014718.808 unmatched trap received from [10.10.10.10]: 01:47:17 2014/11/13 PDU INFO:

        Activity

        Hide
        Alexander Vladishev added a comment -

        Similar issues: ZBX-5372, ZBX-5372

        Show
        Alexander Vladishev added a comment - Similar issues: ZBX-5372 , ZBX-5372
        Hide
        Andris Zeila added a comment -

        The problem is because snmptrapper reads file by 64Kb blocks and then parses the data without checking if the last trap is fully read. So there are chance for data being incorrectly parsed if snmp trapper tries to read trap file while a trap is being written to it. Also if the trap file has more thatnn 64Kb of new data, then the trap on 64Kb boundary will be incorrectly parsed.

        The proposed fix is to not check the last trap on the first read attempt, but keep it in the read buffer and remember its offset. If on the next attempt (the file is checked in 1 second intervals) there are no new data in the trap file, then process the buffered trap. Otherwise process traps normally untill the last one, which again should be kept in read buffer until the next attempt.

        Show
        Andris Zeila added a comment - The problem is because snmptrapper reads file by 64Kb blocks and then parses the data without checking if the last trap is fully read. So there are chance for data being incorrectly parsed if snmp trapper tries to read trap file while a trap is being written to it. Also if the trap file has more thatnn 64Kb of new data, then the trap on 64Kb boundary will be incorrectly parsed. The proposed fix is to not check the last trap on the first read attempt, but keep it in the read buffer and remember its offset. If on the next attempt (the file is checked in 1 second intervals) there are no new data in the trap file, then process the buffered trap. Otherwise process traps normally untill the last one, which again should be kept in read buffer until the next attempt.
        Hide
        Igors Homjakovs (Inactive) added a comment -

        Fixed in svn://svn.zabbix.com/branches/dev/ZBX-9088

        Show
        Igors Homjakovs (Inactive) added a comment - Fixed in svn://svn.zabbix.com/branches/dev/ZBX-9088
        Hide
        Andris Zeila added a comment - - edited

        (1) SNMP trapper should enter idle mode (1 second sleep) only after all data from file have been read. Currently it sleeps after reading and processing a single buffer.

        Igors Homjakovs RESOLVED in r51484.

        Andris Zeila CLOSED

        Show
        Andris Zeila added a comment - - edited (1) SNMP trapper should enter idle mode (1 second sleep) only after all data from file have been read. Currently it sleeps after reading and processing a single buffer. Igors Homjakovs RESOLVED in r51484. Andris Zeila CLOSED
        Hide
        Andris Zeila added a comment - - edited

        (2) Performance optimizations:

        1. We don't have to use memset() to reset the buffer contents. And instead of using strlen() we can simply keep the buffer offset to account for the unprocessed trap data that were moved to the beginning of buffer.
        2. Use memmove() to move the last (unprocessed) trap to the beginning of buffer.

        Igors Homjakovs RESOLVED in r51484.

        Andris Zeila CLOSED

        Show
        Andris Zeila added a comment - - edited (2) Performance optimizations: We don't have to use memset() to reset the buffer contents. And instead of using strlen() we can simply keep the buffer offset to account for the unprocessed trap data that were moved to the beginning of buffer. Use memmove() to move the last (unprocessed) trap to the beginning of buffer. Igors Homjakovs RESOLVED in r51484. Andris Zeila CLOSED
        Hide
        Andris Zeila added a comment - - edited

        (3) Currently if buffer is full with a single trap this trap is dropped with 'trap is too long' error. While unlikely it is still possible that the buffer contains the whole trap (or at least enough data). So instead of giving an error we should log a warning that trap might be truncated and try to process this data.

        Igors Homjakovs RESOLVED in r51484.

        Andris Zeila CLOSED

        Show
        Andris Zeila added a comment - - edited (3) Currently if buffer is full with a single trap this trap is dropped with 'trap is too long' error. While unlikely it is still possible that the buffer contains the whole trap (or at least enough data). So instead of giving an error we should log a warning that trap might be truncated and try to process this data. Igors Homjakovs RESOLVED in r51484. Andris Zeila CLOSED
        Hide
        Andris Zeila added a comment - - edited

        (4) Please check my changes in r51522, r51540. The trap parsing was not working in some combinations of trap data fragments.
        RESOLVED

        Igors Homjakovs Thank you. Looks good. Small change was added in r51583.

        Andris Zeila CLOSED

        Show
        Andris Zeila added a comment - - edited (4) Please check my changes in r51522, r51540. The trap parsing was not working in some combinations of trap data fragments. RESOLVED Igors Homjakovs Thank you. Looks good. Small change was added in r51583. Andris Zeila CLOSED
        Hide
        Andris Zeila added a comment -

        Successfully tested, but please review changes in (4)

        Show
        Andris Zeila added a comment - Successfully tested, but please review changes in (4)
        Hide
        Igors Homjakovs (Inactive) added a comment - - edited

        Fixed in 2.0.15rc1 (r51652), 2.2.9rc1 (r51654), 2.4.4rc1 (r51655), 2.5.0 (trunk) (r51656).

        Show
        Igors Homjakovs (Inactive) added a comment - - edited Fixed in 2.0.15rc1 (r51652), 2.2.9rc1 (r51654), 2.4.4rc1 (r51655), 2.5.0 (trunk) (r51656).
        Hide
        dimir added a comment - - edited

        (5) Compilation warning in trunk r51656:

        snmptrapper.c: In function ‘parse_traps’:
        snmptrapper.c:296:12: warning: ‘pzdate’ may be used uninitialized in this function [-Wmaybe-uninitialized]
        snmptrapper.c:297:12: warning: ‘pzaddr’ may be used uninitialized in this function [-Wmaybe-uninitialized]
        

        Igors Homjakovs No situation can occur when these variables stay uninitialized because if (NULL != begin) will be valid only after their initialization. CLOSED.

        Show
        dimir added a comment - - edited (5) Compilation warning in trunk r51656: snmptrapper.c: In function ‘parse_traps’: snmptrapper.c:296:12: warning: ‘pzdate’ may be used uninitialized in this function [-Wmaybe-uninitialized] snmptrapper.c:297:12: warning: ‘pzaddr’ may be used uninitialized in this function [-Wmaybe-uninitialized] Igors Homjakovs No situation can occur when these variables stay uninitialized because if (NULL != begin) will be valid only after their initialization. CLOSED.

          People

          • Assignee:
            Unassigned
            Reporter:
            Oleg Ivanivskyi
          • Votes:
            1 Vote for this issue
            Watchers:
            6 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: