Alvaro, > On Dec 8, 2015, at 9:36 AM, Alvaro Retana (aretana) <[email protected]> wrote: > > On 12/6/15, 4:09 AM, "Santosh P K" <[email protected]> wrote: > > Santosh: > > Hi! Thanks for picking up the pen on this one! > >> Thanks for your comments. Please see inline comments [SPK]. I have >> addressed rest of the comments from you and have attached the new version >> and also diff between 05 and 06 version. > > I looked at the proposed new text and the diffs. I left below only the > parts that I think still need more work, added back a couple of items and > answered some questions. > > Thanks! > > Alvaro. > > > >> >> Major: >> 1. Section 1. (Introduction) says that "This document extends BFD to >> provide solutions to use cases listed in >> [I-D.ietf-bfd-seamless-use-case]." Maybe it's just me, but I fail to see >> how all the use cases are satisfied - in part because the requirements in >> I-D.ietf-bfd-seamless-use-case are not clear (see my review for that >> document), and in part because this document isn't explicit about how the >> specification solves the use cases. For example, how does this document >> provide a solution for the use case in section 3.6. (BFD for Anycast >> Address)? >> 2. Normative References >> o I-D.ietf-bfd-multipoint should clearly be Normative because of the new >> bfd.SessionType state variable >> o I-D.ietf-bfd-generic-crypto-auth should also be Normative because of >> how the Security Considerations are written: pointing to is as a "MUST". >> Given that (as far as I can tell) there aren't implementations of >> I-D.ietf-bfd-generic-crypto-auth, we could end up with a Normative >> reference that blocks the publication of this document. I want to >> suggest that the comments be reworded as a suggestion or pointer to >> potential solutions, not as a mandate to use them. [Disclaimer: we will >> still need the SecDir to review.] >> >> [SPK] Carlos has replied to these comments and I am waiting for >> confirmation on these comments. > > Unless I missed something, Carlos [1] didn't reply to #1: how are the use > cases satisfied? I'm looking forward to an updated version of > I-D.ietf-bfd-seamless-use-case which may help. > > FWIW, I agree with his comments related to the references. And still > think that both should be Normative. BTW, I don't think that changing the > "MUST" to "SHOULD" when referring to I-D.ietf-bfd-generic-crypto-auth > changed that need.
What’s your (Alvar, Jeff) view on whether this doc should define bfd.SessionType and not multipoint? Seems to me that it is probably best (although an almost negligible preference) to define it here and have multipoint referencing s-bfd. Also, agree that we should reword the bfs-generic-crypto-auth reference beyond a SHOULD, as one potential solution and not as a necessary implementation. > > [1] > https://mailarchive.ietf.org/arch/msg/rtg-bfd/o81wfG8db34WhnX9qMPCZ-IEtAQ > > > > > > >> >> o "Diagnostic (Diag) MAY be set to appropriate value for communicating >> with peer" The obvious question is: which values? Section 7.4. >> (Diagnostic Values) tries to address that, but it says that "Diagnostic >> value in both directions MAY be set to a certain value...details of such >> are outside the scope of this specification." What about bfd.LocalDiag? >> Are those values still valid? As long as 7.4 is there, you could explain >> a little bit more why this doesn't matter. >> >> [SPK] I am not sure if understood the comment here. > > The (updated) text (7.2.2/7.3.2) says that Diag "MUST be set as per > Section 6.8.7 of [RFC5880]", which simply reads "Set to bfd.LocalDiag." > As far as I can tell, there are 4 values defined for Diag in rfc5880. > But, Section 7.4 of this document reads: > > 7.4. Diagnostic Values > > Diagnostic value in both directions MAY be set to a certain value, to > attempt to communicate further information to both ends. However, > details of such are outside the scope of this specification. > > > > So..the (updated) text (7.2.2/7.3.2) does say that Diag = bfd.LocalDiag, > but 7.4 seems to imply that other values (besides the ones in rfc5880) are > valid. That part is ok (about other values), but it is still no clear > whether the values from rfc5880 are also valid or not. > > > . . . > > There are still some other comments that I don't think were addressed from > 7.2.2 and 7.3.2. In most of the comments below the common thread is > whether the state variables described in rfc5880 are reused (if so, then > maybe the initialization/maintenance are different) or not (if so, are you > creating new variables here and not explicitly defining them?). > > For 7.2.2: > > * "Your Discriminator Set to value corresponding to remote entity.² Do you > mean bfd.RemoteDiscr? BTW, I think you should explain how this variable is > initialized. > > For 7.3.2: > > * Several fields are set to the value copied from the received packet > (Detect Mult, My Discriminator, Your Discriminator and Desired Min TX > Interval). I¹m assuming that all these values are copied into the state > variables defined in rfc5880. If so, then mentioning these fields seems > redundant given the header of the section. > > > * "bfd.RequiredMinRxInterval, value describing how many incoming control > packets this reflector BFD session can handle" But rfc5880 defines > bfd.RequiredMinRxInterval as "The minimum interval, in microseconds, > between received BFD Control packets". IOW, the definitions don't match. > > > > > >> o Unlike rfc5880, this document doesn't explicitly mention that "Some >> form of authentication SHOULD be included, since Echo packets may be >> spoofed." The recommendation of sending both S-BFD control and echo >> packets points at alleviating some of the spoofing concerns even though >> they are independent packets (in other words: the Echo packet can still >> u-turn at a different node). Please include a discussion of the >> alleviated security concern in the Security Considerations (since it is >> different than rfc5880). Also, it would be nice if it was mentioned >> explicitly whether authentication for Echo packets is needed/recommended >> or not. >> 7. Section 11. (Security Considerations) >> >> [SPK] I want this to be discussed in WG before I can address these >> comments. > > You mean the comment above, right? > > Discussing with the WG is perfectly fine with me. > > >> >> o "crypto sequence number" What are you referring to? I'm guessing the >> Sequence Number field in the Authentication Section - is that a good >> guess? Please be specific and include a reference. >> o The text says that the "SBFDReflector MUST compute the Authentication >> data", but that it "MUST NOT look at the crypto sequence number". Is >> that a contradiction? As defined in rfc5880, the Authentication Data >> seems to include everything in the Authentication Section, including the >> sequence number. >> >> [SPK] SBFDReflector has no state to check for sequence number. That is >> the reason why it MUST NOT look at the sequence number and only should >> check the authentication. > > Ahh.. So you mean that it shouldn't bother with the seq because it can't > verify it. What confused me (and may confuse others) is the meaning of > "look at": if the Reflector is computing the Authentication Data it may > look (as in notice that is there) at the sequence number vs actually > calculating it. I'm ok with leaving the current language (now that I > understand), but it might be nice to clarify. > > > >> >> o Why isn't the "loop problem" in Appendix A mentioned? >> >> [SPK] It is mentioned in appendix A. Did you mean why is this mentioned? >> This is to give more clarity on why we are overloading D bit to break the >> loop. > > I meant why isn't it mentioned in the Security Considerations section? If > it's a problem worth including in the document, I think it would be good > to point at it in a section that people may read. > > >> >> 8. Nowhere in this document (or draft-ietf-bfd-seamless-ip) is congestion >> mentioned. rfc5880 talks about some of the considerations. Are there >> new congestion-related considerations that arise because of eliminating >> some of the negotiation aspects? Thinking out loud: if a session doesn't >> have to be established (and everyone knows a remote discriminator), then >> there's a possibility of more nodes sending traffic to a specific >> reflector (just as an example). Please include some text indicating any >> congestion issues - or at least explaining why there aren't any new ones. >> >> >> [SPK] It has no new congestion issues. SBFDRefelector will be able to use >> "Required Min RX interval" to control rate from senders. >> >> " Required Min RX Interval >> >> MUST be set to a value describing how many incoming control >> packets this reflector BFD session can handle. Further details >> are described in Section 7.8." >> >> >> Do you still think we need to add a separate section explaining it? > > I do. > > Also, in Carlos' answer (see [1] above), he wrote: "Very much agree. I > believe there are subtle differences in the congestion considerations, > because if the simplified negotiation. Further, I believe those should > live in the -base doc. That is missing, and we should fill in this gap.” > I still agree with my old comment :-) Note I do not think we need to over-specify this… something simple. > > >> >> 3. Section 4.1. (S-BFD Discriminator Uniqueness) I think the text in >> this section is a little confusing. The requirement (the "S-BFD >> discriminator...MUST be unique within an administrative domain") is >> clearly stated at the start, but then the justification of why goes into >> how is IP is used (with no reference to where S-BFD for IP is specified) >> and concludes that (in that case) the "S-BFD discriminator only has to be >> unique within a local node" - at first read it sounds like there is a >> contradiction in the text. The paragraph closes with a reiteration of >> the uniqueness. Please clarify - maybe specifically state that the >> discussion (maybe in a separate paragraph) is to justify the uniqueness.. >> >> [SPK] It is not a contradicting it just gives an example to say if IP >> then you really don't need a discr unique to domain but SBFD is generic >> and hence it needs to be unique. Do you want to reword this? > > You did it again! ;-) "...if IP then you really don't need a discr > unique to domain but SBFD is generic and hence it needs to be unique..." > Maybe I'm just reading too much into it.. > > IMHO, because this document doesn't describe S-BFD for IP, then maybe it's > a good idea to move this discussion to draft-ietf-bfd-seamless-ip and > avoid the confusion altogether. > > > . . . > > There is one other comment I marked as Minor that I would like to see > addressed: > > * The base document is not describing the operation in IP and MPLS > environments, are the references to rfc5881, rfc5883, rfc5884 and rfc5885 > needed in 7.3 and 11? And do the procedures and security considerations > really apply? > > > > > >> 3. Section 7.2. (Initiator Procedures) >> o The example and the figure seem out of place in this section as the >> responder procedures haven't been introduced yet. >> >> [SPK] So should this section be placed after reflector procedure has been >> introduced? > > I think that would be nice. > >> >> 6. Section 8. (Scaling Aspect) The text indirectly implies that the >> scaling is better by saying that the number of sessions is less.. I >> understand the point, but it just sounds like a superfluous section to me. >> >> [SPK] Remove it? >> >> 7. Section 9. (Co-existence with Classical BFD Sessions) is another >> superfluous section; there's nothing here that you couldn't have said in >> 7.1. >> >> [SPK] Remove it? > > Both seem superfluous to me, but these are just nits, so it's up to you. > Thanks! — Carlos.
signature.asc
Description: Message signed with OpenPGP using GPGMail
