Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix index nodes in LLDP tables whose access right is not-accessible. #112

Merged
merged 2 commits into from
Jan 7, 2020

Conversation

shihhsien-wang
Copy link
Contributor

- What I did

Ensure not-accessible nodes (index nodes in MIB table) in tables in LLDP MIB are not readable by SNMP get operation.

- How I did it

Remove the code which processes read request of index nodes and their access right (MAX-ACCESS caluse) are not-accessible:

  • Index node lldpLocPortNum in lldpLocPortTable,
  • Index nodes lldpLocManAddrSubtype and lldpLocManAddr in lldpLocManAddrTable,
  • Index nodes lldpRemTimeMark, lldpRemLocalPortNum and lldpRemIndex in lldpRemTable, and
  • Index nodes lldpRemManAddrSubtype and lldpRemManAddr in lldpRemManAddrTable tables

- How to verify it

Before modified

# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.3.7.1.1
iso.0.8802.1.1.2.1.3.7.1.1.1 = INTEGER: 1
iso.0.8802.1.1.2.1.3.7.1.1.5 = INTEGER: 5
iso.0.8802.1.1.2.1.3.7.1.1.9 = INTEGER: 9
iso.0.8802.1.1.2.1.3.7.1.1.13 = INTEGER: 13
iso.0.8802.1.1.2.1.3.7.1.1.17 = INTEGER: 17
iso.0.8802.1.1.2.1.3.7.1.1.21 = INTEGER: 21
iso.0.8802.1.1.2.1.3.7.1.1.25 = INTEGER: 25
iso.0.8802.1.1.2.1.3.7.1.1.29 = INTEGER: 29
iso.0.8802.1.1.2.1.3.7.1.1.33 = INTEGER: 33
iso.0.8802.1.1.2.1.3.7.1.1.37 = INTEGER: 37
iso.0.8802.1.1.2.1.3.7.1.1.41 = INTEGER: 41
iso.0.8802.1.1.2.1.3.7.1.1.45 = INTEGER: 45
iso.0.8802.1.1.2.1.3.7.1.1.49 = INTEGER: 49
iso.0.8802.1.1.2.1.3.7.1.1.53 = INTEGER: 53
iso.0.8802.1.1.2.1.3.7.1.1.57 = INTEGER: 57
iso.0.8802.1.1.2.1.3.7.1.1.61 = INTEGER: 61
iso.0.8802.1.1.2.1.3.7.1.1.65 = INTEGER: 65
iso.0.8802.1.1.2.1.3.7.1.1.69 = INTEGER: 69
iso.0.8802.1.1.2.1.3.7.1.1.73 = INTEGER: 73
iso.0.8802.1.1.2.1.3.7.1.1.77 = INTEGER: 77
iso.0.8802.1.1.2.1.3.7.1.1.81 = INTEGER: 81
iso.0.8802.1.1.2.1.3.7.1.1.85 = INTEGER: 85
iso.0.8802.1.1.2.1.3.7.1.1.89 = INTEGER: 89
iso.0.8802.1.1.2.1.3.7.1.1.93 = INTEGER: 93
iso.0.8802.1.1.2.1.3.7.1.1.97 = INTEGER: 97
iso.0.8802.1.1.2.1.3.7.1.1.101 = INTEGER: 101
iso.0.8802.1.1.2.1.3.7.1.1.105 = INTEGER: 105
iso.0.8802.1.1.2.1.3.7.1.1.109 = INTEGER: 109
iso.0.8802.1.1.2.1.3.7.1.1.113 = INTEGER: 113
iso.0.8802.1.1.2.1.3.7.1.1.117 = INTEGER: 117
iso.0.8802.1.1.2.1.3.7.1.1.121 = INTEGER: 121
iso.0.8802.1.1.2.1.3.7.1.1.125 = INTEGER: 125
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.3.8.1.1
iso.0.8802.1.1.2.1.3.8.1.1.1.4.10.1.0.1 = INTEGER: 1
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.3.8.1.2
iso.0.8802.1.1.2.1.3.8.1.2.1.4.10.1.0.1 = STRING: "0A 01 00 01"
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.4.1.1.1
iso.0.8802.1.1.2.1.4.1.1.1.659.113.2 = Timeticks: (659) 0:00:06.59
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.4.1.1.2
iso.0.8802.1.1.2.1.4.1.1.2.659.113.2 = INTEGER: 113
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.4.1.1.3
iso.0.8802.1.1.2.1.4.1.1.3.669.113.2 = INTEGER: 2
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.4.2.1.1
iso.0.8802.1.1.2.1.4.2.1.1.649.113.2.1.4.192.168.3.20 = INTEGER: 1
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.4.2.1.2
iso.0.8802.1.1.2.1.4.2.1.2.649.113.2.1.4.192.168.3.20 = STRING: "C0 A8 03 14"

After modified

# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.3.7.1.1
iso.0.8802.1.1.2.1.3.7.1.1 = No Such Object available on this agent at this OID
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.3.8.1.1
iso.0.8802.1.1.2.1.3.8.1.1 = No Such Object available on this agent at this OID
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.3.8.1.2
iso.0.8802.1.1.2.1.3.8.1.2 = No Such Object available on this agent at this OID
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.4.1.1.1
iso.0.8802.1.1.2.1.4.1.1.1 = No Such Object available on this agent at this OID
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.4.1.1.2
iso.0.8802.1.1.2.1.4.1.1.2 = No Such Object available on this agent at this OID
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.4.1.1.3
iso.0.8802.1.1.2.1.4.1.1.3 = No Such Object available on this agent at this OID
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.4.2.1.1
iso.0.8802.1.1.2.1.4.2.1.1 = No Such Object available on this agent at this OID
# snmpwalk -c public -v 2c 192.168.3.19 1.0.8802.1.1.2.1.4.2.1.2
iso.0.8802.1.1.2.1.4.2.1.2 = No Such Object available on this agent at this OID

… access right (MAX-ACCESS caluse) are not-accessible:

* Index node lldpLocPortNum in lldpLocPortTable,
* Index nodes lldpLocManAddrSubtype and lldpLocManAddr in lldpLocManAddrTable,
* Index nodes lldpRemTimeMark, lldpRemLocalPortNum and lldpRemIndex in lldpRemTable, and
* Index nodes lldpRemManAddrSubtype and lldpRemManAddr in lldpRemManAddrTable tables
@msftclas
Copy link

msftclas commented Sep 5, 2019

CLA assistant check
All CLA requirements met.

@qiluo-msft
Copy link
Contributor

Could you clarify your motivation? Any reference?

Ensure not-accessible nodes (index nodes in MIB table) in tables in LLDP MIB are not readable by SNMP get operation.

@shihhsien-wang
Copy link
Contributor Author

Could you clarify your motivation? Any reference?

Ensure not-accessible nodes (index nodes in MIB table) in tables in LLDP MIB are not readable by SNMP get operation.

SMIv2 (RFC 2578) defines the access permission of a MIB object:

7.3.  Mapping of the MAX-ACCESS clause

   The MAX-ACCESS clause, which must be present, defines whether it
   makes "protocol sense" to read, write and/or create an instance of
   the object, or to include its value in a notification.  This is the
   maximal level of access for the object.  (This maximal level of
   access is independent of any administrative authorization policy.)

For MIB object whose MAX-ACCESS is not-accessible should be non-readable, just like index cpfcIfPriorityValue in cpfcIfPriorityTable in ciscoPfcExtMIB

[skipped]
cpfcIfPriorityValue OBJECT-TYPE
    SYNTAX          Integer32 (0..7)
    MAX-ACCESS      not-accessible
    STATUS          current
    DESCRIPTION
        "This object indicates the priority value of the PFC
        capable interface." 
    ::= { cpfcIfPriorityEntry 1 }

cpfcIfPriorityRequests OBJECT-TYPE
    SYNTAX          Counter64
    MAX-ACCESS      read-only
    STATUS          current
    DESCRIPTION
        "A count of invoked request premitives for a specific
        PFC priority of a particular interface." 
    ::= { cpfcIfPriorityEntry 2 }
[skipped]

In SONiC MIB implementation, this index is not return (non-readable):

[skipped]
# cpfcIfPriorityTable = '1.2'
# cpfcIfPriorityEntry = '1.2.x'
class cpfcIfPriorityTable(metaclass=MIBMeta, prefix='.1.3.6.1.4.1.9.9.813.1.2'):
    """
    'ciscoPfcExtMIB' http://oidref.com/1.3.6.1.4.1.9.9.813
    """
    pfc_updater = PfcPrioUpdater()

    prioRequests = \
        SubtreeMIBEntry('1.2', pfc_updater, ValueType.COUNTER_64, pfc_updater.requests_per_priority)
[skipped]

@qiluo-msft
Copy link
Contributor

Thanks @shihhsien-wang for the contribution!
I am ok with this PR. The sonic-mgmt is still testing with these OIDs removed ( https://github.com/Azure/sonic-mgmt/blob/master/ansible/library/snmp_facts.py). Could you also help submit a PR there to remove the test code, so I will merge both of them?

Copy link
Contributor

@qiluo-msft qiluo-msft left a comment

Choose a reason for hiding this comment

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

sonic-mgmt changes needed.

@shihhsien-wang
Copy link
Contributor Author

Thanks @shihhsien-wang for the contribution!
I am ok with this PR. The sonic-mgmt is still testing with these OIDs removed ( https://github.com/Azure/sonic-mgmt/blob/master/ansible/library/snmp_facts.py). Could you also help submit a PR there to remove the test code, so I will merge both of them?

May I also remove the test code for these nodes from sonic-snmpagent repo (auto test fail before)? Thanks.

@shihhsien-wang
Copy link
Contributor Author

Hi @qiluo-msft ,

I had update sonic-snmpagent here to pass the unit test, and sonic-mgmt #1319. Please help to review the code, thanks.

@qiluo-msft qiluo-msft merged commit 29ebe43 into sonic-net:master Jan 7, 2020
abdosi pushed a commit that referenced this pull request Jul 8, 2020
…112)

* Remove the code which processes read request of index nodes and their access right (MAX-ACCESS caluse) are not-accessible:
* Index node lldpLocPortNum in lldpLocPortTable,
* Index nodes lldpLocManAddrSubtype and lldpLocManAddr in lldpLocManAddrTable,
* Index nodes lldpRemTimeMark, lldpRemLocalPortNum and lldpRemIndex in lldpRemTable, and
* Index nodes lldpRemManAddrSubtype and lldpRemManAddr in lldpRemManAddrTable tables
* Update unit test code for LLDP code modification.
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.

4 participants