Adrian, Thank you very much for the thorough review!
Authors & Shepherd, Please go through and update this draft ASAP. It is on the IESG telechat on May 5 and is/will be reviewed very soon. Thanks, Alia On Wed, Apr 27, 2016 at 9:11 AM, Adrian Farrel <[email protected]> wrote: > Hello, > > I have been selected as the Routing Directorate reviewer for this draft. > The > Routing Directorate seeks to review all routing or routing-related drafts > as > they pass through IETF last call and IESG review, and sometimes on special > request. The purpose of the review is to provide assistance to the Routing > ADs. > For more information about the Routing Directorate, please see > http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir > > Although these comments are primarily for the use of the Routing ADs, it > would be helpful if you could consider them before or along with any IETF > Last Call comments that you receive, and strive to resolve them through > discussion or by updating the draft. > > Document: draft-ietf-ospf-sbfd-discriminator-04.txt > Reviewer: Adrian Farrel > Review Date: 27 April 2016 > IETF LC End Date: 26 April 2016 > Intended Status: Standards Track > > Summary: > I have some minor concerns about this document that I think should be > resolved before publication. > > Comments: > > This is a simple document that doesn't require much to implement or > understand. It was disappointing, however, to find a large number of > small issues and nits. I don't believe any of these are blocking to > the utility of the document and if it went for publication in its > current state it would not be harmful. But in the interest of making > our documents useful and accessible, and for the purpose of eliminating > all possible interoperability and deployment, I think it would be > valuable to clean up the issues I have listed. > > Major Issues: > No major issues found. > > Minor Issues: > > I should like to see some small amount of text on the scaling impact on > OSPF. 1. How much additional information will implementations have to > store per node/link in the network? 2. What is the expected churn in > LSAs introduced by this mechanism (especially when the Reflector is > turned on and off)? > > In the second case there is a security implication as well. Can I DoS > the routing system by toggling some BFD Reflectors? Needs text! > > You *do* have... > A change in information in the S-BFD Discriminator TLV MUST NOT > trigger any SPF computation at a receiving router. > ...which is a help. > > --- > > Section 1 has > > This is achieved by using unique > network-wide discriminators to identify the Network Targets (e.g., IP > addresses). > > You may be aware of IPv6 :-) > > Although 2.1 gives some hints on the size of a discriminator, I had to > go back to 5880 to check that *all* discriminators are exactly 4 octets. > So saying "e.g., IP addresses" is at best confusing. > > BTW, draft-ietf-bfd-seamless-base and draft-ietf-bfd-seamless-ip don't > give any hints on this. > > Oh, and what is "network-wide"? > > I suggest... > > This is achieved by using four-octet discriminators as defined in > [RFC5880] to identify the Network Targets. > > --- > > In Section 2 you have > Upon receipt of the TLV, a > router may decide to ignore this TLV or install the S-BFD > discriminator in BFD Target Identifier Table. > > I think "ignore" is ambiguous. You need to be very clear that "ignore" > means: > - take no local action > - retain the TLV in the opaque LSA > - continue to advertise the opaque LSA according to its scope > > In Section 3 you also have > A router not supporting the S-BFD Discriminator TLV will just > silently ignore the TLV as specified in [RFC7770]. > > Am I missing something when I read 7770? I don't find anything about > handling unknown TLVs. > > --- > > Section 2 para 3 > s/superset/union/ > ("superset" would allow you to include any other discriminators!) > > --- > > Section 2.1 > To be totally unambiguous... > OLD > Length - Total length of the discriminator (Value field) in octets, > not including the optional padding. The Length is a multiple of 4 > octets, and consequently specifies how many Discriminators are > included in the TLV. > NEW > Length - Total length of all discriminator in the Value field in > octets, not including the optional padding. The Length is a multiple > of 4 octets, and consequently specifies how many Discriminators are > included in the TLV. > END > > However (!) are you sure that you can include optional padding? I think > that 7770 uses padding to take the V field up to a 4 octet boundary. > Since all of your discriminators are exactly a multiple of 4 octets it > seems that there will never be any padding and it would be less > confusing to write... > > NEW > Length - Total length of all discriminators in the TLV counted in > octets. The Length is a multiple of 4 octets, and consequently > specifies how many Discriminators are included in the TLV. > END > > --- > > At the end of section 2.1 you have > > S-BFD discriminator is associated with the > BFD Target Identifier type, that allows demultiplexing to a specific > task or service. > > This is a wonderfully throw-away statement with no context and no > further explanation in the document that I could find. Maybe this is > just missing a reference to another document, or maybe it needs some > clarification. > > --- > > Section 2.2 has > > The flooding scope for S-BFD Discriminator information advertised > through OSPF can be limited to one or more OSPF areas, or can be > extended across the entire OSPF routing domain. > > Note that the S-BFD session may be required to pan multiple areas, in > which case the flooding scope may comprise these areas. This could > be the case for an ABR, for instance, advertising the S-BFD > Discriminator information within the backbone area and/or a subset of > its attached IGP area(s). > > As I understand flooding scope the options for Opaque LSAs (see 5250) > are: > > o Link-state type-9 denotes a link-local scope. > > o Link-state type-10 denotes an area-local scope. > > o Link-state type-11 denotes that the LSA is flooded throughout the > Autonomous System (AS). > > Your text seems to imply something different. In particular, you seem to > be suggesting that I can have a scope that is greater than one area but > less than the whole AS (assuming "whole AS" == "entire OSPF routing > domain"). > > This needs re-writing to clarify what you want to achieve and to bring > it in line with 5250. > > Note that the 4th para of Section 2.2 seems to have this right. > > === > > Nits > > Has Trilok's affiliation changed? > -- > Capitalise the document title > --- > Expand acronyms in the Abstract if they do not appear with an asterisk > in http://www.rfc-editor.org/materials/abbrev.expansion.txt > --- > Throughout the text, expand acronyms on first use if they do not appear > within http://www.rfc-editor.org/materials/abbrev.expansion.txt with an > asterisk. > --- > Decide whether "discriminator" or "Discriminator" > --- > In 2.1 you have > Value - S-BFD network target discriminator value or values. > But there is no "Value" in the figure. > --- > 2.2 para 2 > s/pan/span/ > --- > 2.2 > In the case of domain- > wide flooding, i.e., where the originator is sitting in a remote > area, the mechanism described in section 5 of [RFC5250] should be > used. > s/should/SHOULD/? > But if you mean should or SHOULD (not MUST), what are the exception > cases? > --- > > Thanks, > Adrian > >
_______________________________________________ OSPF mailing list [email protected] https://www.ietf.org/mailman/listinfo/ospf
