Work at SourceForge, help us to make it a better place! We have an immediate need for a Support Technician in our San Francisco or Denver office.

Close

Code Merge Request #3: Add Juniper Vlan mappings from int descriptions (rejected)

OrangePeelBeef asked you to merge 0 commits from /u/orangepeelbeef/snmp-info/ to master, 2014-02-21

Add juniper vlan mappings from interface descriptions

Commit Date  

Discussion

  • Regarding the first and third patch: it looks like you're adding "layer 3" interfaces as members of a VLAN and I'm not sure that it's desirable to override the data returned by the switch.

    Originally the i_vlan_membership() method was written to provide information about L2 ports (so switching/bridging), making it easier to see "which vlans are allowed on a port" for devices implementing the standard BRIDGE-MIB or Q-BRIDGE-MIB, which is basically VLAN oriented and not port oriented.

    IMHO a switchport can be a member of one or more VLANs and will perform MAC learning etc, while a L3 interface (be it a SVI / VLAN interface or a subinterface or service instance on a non-bridging port) is only associated with one VLAN and will never learn MAC addresses.

    Throwing them together with i_vlan_membership() output feels like a layering violation to me. Sure, SNMP::Info currently reports "vlan membership" on L3 ports for eg Cisco devices, but I'd like to argue that that is undesired behavior and shouldn't be extended to other classes.

    I'm not sure how the other SNMP::Info developers and users feel about this, but I think it would be better to keep a degree of separation between the methods / info we return on L2 and L3 ports. Depending on your use case, we could implement a new method that returns the L3 interface associated with each VLAN, if present.

    Regarding the second patch: yes, VLAN 1 is "special" for some vendors and its use is discouraged, but by skipping it we wouldn't report the switch config correctly. Default setting on almost all switches is "a VLAN with VID 1 is present" and "all bridge ports are untagged members of VLAN 1".

     
  • After discussion with other developers via IRC, the concensus is:

    • 2nd patch: whether or not information about VLAN 1 is required should be a decision for the application layer; there are valid use cases for VLAN 1 and we shouldn't force such a change on all users of SNMP::Info.

    • 1st/3rd patch: mixing L2 and L3 information together this way is not desirable. It would be better to implement separate methods for associating L3 information with a 802.1Q VLAN ID and/or associating 802.1Q VLAN IDs with interfaces that have IP addresses.