Draft:  draft-ietf-l3vpn-vr-mib-04.txt
Reviewer: Elwyn Davies [elwynd@dial.pipex.com]
Review Date: Monday 12/19/2005 6:36 AM CST
LC Date: 21 December 2005

Summary: There appear to be a few issues which need to be sorted out.  
Two are significant: The first, relating to hard coding of bits 
selecting protocols, appears to seriously limit the flexibility of the 
MIB and the second relating to the binding of interfaces to VRs, 
appears to contradict my understanding of how VRs are supposed to 
work with physical interfaces - if my understanding is correct, 
this part is seriously broken: My understanding is that on the provider 
side each physical interface would feed packets to many, if not all, 
VRs with demultiplexing based on some feature of the protocol packets - 
the MIB appears to mandate that an interface can only service one VR.  
There are also a number of editorial nits.

Details:

I found the introductory text of this document (ss2-4) to be rather 
perfunctory in places.  In particular a lttle more up front explanation 
of the use of communities and contextNames to partition the MIB and the 
connection with SNMP protocol versions would help a naive reader.

Issue:
The hard coding of bits (rip (0), ospf(1), bgp (2), isis (3)) to 
represent particular routing protocols strikes me as inflexible and not 
very future proof [Speak it softly, but there are some other routing 
protocols in existence already].  Also it strikes me that it would be 
helpful for the manager to be able to see what protocols are actually 
available before trying to start them, especially if there were several 
versions available perhaps (e.g., various varieties of BGP). I would 
have thought a more flexible approach might have been to provide a 
read-only table in the common part of the MIB indicating what routing 
protocols were available and then use indices in this table to control 
the individual protocols.  But I am not really knowledgeable here, so 
please feel free to ignore this comment if there is good reason for 
doing it this way.

s4.5, para 1: Is there a fundamental problem in this section?  
'...mapping from IfIndex to a unique VR' and the whole last sentence 
imply each (physical?) interface links to exactly one VR.  I thought the 
whole point of VRs was that packets from each physical interface were 
demultiplexed onto one of many VRs at least on the provider side of a PE 
router - on the customer side, I understand that it may well be that the 
physical interface selects the VR. OTOH, maybe you are talking about 
virtual interfaces.. in which case this should be made clear.

s4.6: Do you need to distinguish between candidate routes and installed 
routes? The number of candidate routes for BGP may be MUCH larger: it 
depends which resources you are trying to control here - routing 
protocol memory usage or forwarding table resources.

s8, para 1: This para ends 'These are the tables and objects and their 
sensitivity/vulnerability:' This seems to imply that this should be 
followed by a list of items from the MIB and associated analysis but 
this is (apparently) not present.

Refs:

I think at least [PPVPN-VR] and possibly [PPVPN-FW] ought to be 
normative since the former defines the model on which this MIB is based 
and in turn depends on the framework.

Editorial:

s2.0: Explicitly expand VPN, PE acronyms

s2.0, para 2: s/Following are the goals, in defining this MIB module:/In 
defining this MIB module, the goals are as follows:/

s4.1, Item (1): A naive reader (like me) would be unclear as to whether 
(a) and (b) are alternative ways of doing things or it is the 
combination that represents the required way of doing things: please 
clarify this.  If I understand correctly from later, (a) and (b) are two 
alternative ways of identifying a particular VR depending on which 
protocol is used to access the MIB [Question: Can you also use the 
Community String with SNMPv3?]

s4.1, Item (1) (a) and (b) also contain very fine examples of the 
grocer's apostrophe.  Neither the Community String or the contextName 
own anything and so use of the possessive suffix is inappropriate.  I 
believe the intention is to make the terms plural.

s4.1, Item (1): RFC References for SNMPv2c and SNMPv3 would be desirable.

s4.2, Item (2):  'allowing the implementation to remain standards 
compliant' - I know what you mean but saying this in the definition of 
what you hope to become a standard is somewhat confusing!  I would have 
thought the benefits were compatibility with conventional, non-virtual 
routers and potentially reduced implementation effort due to reuse of 
structures and code.

s4.1, Item (3): Maybe refs for each of these MIBs might be appropriate.

s4.1, Figure: The figure requires a title.

s4.2, para 4: 'A SNMP manager SHOULD NOT assume global significance of 
any VRID value
other than 0.'  Either provide a pointer to s4.3 to explain about VRID 0 
or (better, IMO) move sentences 3, 4 and 5 of s4.3, para 2 into s4.1.

s4.3, para 2, sentence 3: s/should be/is/ [There is no option here!]
s4.3, para 2, last sentence: Better: 'The Administrative VR is not 
associated with any actual routing and would (MUST?) not have [??any 
routing/any kind of] MIBs.

s4.4, para 1: s/turned down/disabled by setting VrAdminStatus to 'down'./

s4.4, para 2: s/is expected to change along/will be expected to reflect 
changes to/

s4.5, para 1: I presume it was intended to select between '?attached or 
connected?'.  Maybe 'bound' is the term you are looking for?

s4.6, para 1: The introduction says 'following parameters' but there is 
only one at present.

s4.9.1, para 1: s/every virtual router/all the virtual routers/