Hi Carlos, This all looks good to me. Thanks for the quick resolution! I'll clear my discuss assuming that you will submit the updated version very very soon.
Regards, Alia On Tue, May 3, 2016 at 12:13 PM, Carlos Pignataro (cpignata) < cpign...@cisco.com> wrote: > Hi, Alia, > > Thanks for the response! > > We are on the exact same page regarding items #1 and #2. > > For item #3, we really want to modularize the specs and not tie the -base > to the transports. Note that we mention “UDP” but also “associated channel > type”. > > For #3, here’s the change I implemented: > > S-BFD packet MUST be demultiplexed with lower layer information > - (e.g., dedicated destination UDP port, associated channel type). > - Following procedure SHOULD be executed on both initiator and > - reflector. > + (e.g., dedicated destination UDP port [I-D.ietf-bfd-seamless-ip], > + associated channel type [I-D.ietf-pals-seamless-vccv]). Following > + procedure SHOULD be executed on both initiator and reflector. > > And please find attached full diffs addressing all the Discuss points. > > Thanks! > > — Carlos. > > > > On May 3, 2016, at 10:53 AM, Alia Atlas <akat...@gmail.com> wrote: > > Hi Carlos, > > On Mon, May 2, 2016 at 6:34 PM, Carlos Pignataro (cpignata) < > cpign...@cisco.com> wrote: > >> Hi, Alia, >> >> Thanks for your review and for bringing up these issues — please see >> inline. >> >> > On May 2, 2016, at 5:24 PM, Alia Atlas <akat...@gmail.com> wrote: >> > >> > Alia Atlas has entered the following ballot position for >> > draft-ietf-bfd-seamless-base-09: Discuss >> > >> > When responding, please keep the subject line intact and reply to all >> > email addresses included in the To and CC lines. (Feel free to cut this >> > introductory paragraph, however.) >> > >> > >> > Please refer to >> https://www.ietf.org/iesg/statement/discuss-criteria.html >> > for more information about IESG DISCUSS and COMMENT positions. >> > >> > >> > The document, along with other ballot positions, can be found here: >> > https://datatracker.ietf.org/doc/draft-ietf-bfd-seamless-base/ >> > >> > >> > >> > ---------------------------------------------------------------------- >> > DISCUSS: >> > ---------------------------------------------------------------------- >> > >> > a) In Sec 7.2.3: "If the SBFDReflector is generating a response S-BFD >> > control packet for a local entity that is in >> > service, then "state" in response BFD control packets MUST be set >> > to UP." >> > So far, it looked like the SBFDReflector only sends BFD control >> > packets in response to receiving such packets >> > from SBFDInitiators. This paragraph (not just copied) does not >> > clearly describe the desired behavior. If the >> > monitored local entity is "temporarily out of service", does the >> > SBFDReflector respond back to the SBFDInitiator >> > with 2 BFD control packets - one indicating UP (as a MUST) and then >> > the next indicating ADMINDOWN? Is the >> > SBFDReflector expected to store a list of active SBFDInitiators and >> > proactively send BFD control packets indicating >> > ADMINDOWN? Please clarify in non-trivial detail. >> >> The way in which that particular bullet in that subsection is written can >> be a bit confusing. >> >> First, you are right that the SBFDReflector only sends packets in >> response to S-BFD control packets from the SBFDInitiators. This is clearly >> spelled out in Section 5, and in other places that explain how the >> reflector is stateless. >> >> The SBFDReflector only response and does not stores a list of >> SBFDInitiators to proactively send S-BFD packets (see Section 5). Further, >> it does not respond with two packets. (UP and ADMINDOWN). >> >> I think this can be rewritten to better explain what happens, as follows: >> >> OLD: >> o If the SBFDReflector wishes to communicate to some or all >> SBFDInitiators that monitored local entity is "temporarily out of >> service", then S-BFD control packets with "state" set to ADMINDOWN >> are sent to those SBFDInitiators. The SBFDInitiators, upon >> reception of such packets, MUST NOT conclude loss of reachability >> to corresponding remote entity, and MUST back off packet >> transmission interval for the remote entity to an interval no >> faster than 1 second. If the SBFDReflector is generating a >> response S-BFD control packet for a local entity that is in >> service, then "state" in response BFD control packets MUST be set >> to UP. >> >> NEW: >> o If the SBFDReflector, upon receiving an S-BFD control packet from >> an SBFDInitiators, wishes to communicate to those >> SBFDInitiators that a monitored local entity is "temporarily out of >> service", then an S-BFD control packet with "state" set to ADMINDOWN >> is sent in response to those SBFDInitiators. The SBFDInitiators, >> upon >> reception of such packets, MUST NOT conclude loss of reachability >> to corresponding remote entity, and MUST back off packet >> transmission interval for the remote entity to an interval no >> faster than 1 second. If, on the other hand, the SBFDReflector is >> generating a >> response S-BFD control packet for a local entity that is in >> service, then "state" in response BFD control packets MUST be set >> to UP. >> >> Is that more clear? >> > > Slightly - but what about: > > "When the SBFDReflector receives an S-BFD control packet from an > SBFDInitiator, > then the SBFDReflector needs to determine what state to send in the > response S-BFD > control packet. If the monitored local entity is in service, then the > "state" MUST be > set to UP. However, if the monitored local entity is "temporarily out of > service" for > rapidly processing S-BFD packets, for instance due to an overload, then > the "state" > SHOULD be set to ADMINDOWN. The SBFDReflector SHOULD send a response > S-BFD control packet. > > When an SBFDInitiator receives a response S-BFD control packet, if the > state specified > is ADMINDOWN, the SBFDInitiator MUST NOT conclude loss of reachability > to corresponding remote entity, and MUST back off packet transmission > interval for the > remote entity to an interval no faster than 1 second. " > > Either wording or a mixture is just fine. >> >> >> > >> > b) Appendix A: The looping problem is nicely defined but the text still >> > discusses three potential solutions; clearly the >> > use of the D bit has been chosen. It would be much nicer to have the >> > justification in line, but for this discuss - the >> > unselected alternatives don't belong. >> > >> >> Sorry I’m not sure I understand fully your point. Are you suggesting we >> mention in the actual reason for the D-bit procedures outside the Appendix >> (although the procedures for the D bit are explained in Section 6.2, 7.2.2, >> 7.2.3, 7.3.2, and 7.2.2), while still leave the Appendix as-is? >> >> If so we can do that, but want to confirm. >> > > I'm suggesting that you mention the reason for the D-bit procedures > outside the Appendix and remove the Appendix. Alternately, keep the > Appendix but remove discussion of the other ways the problem could have > been solved and add a reference from the D-bit procedures to the Appendix. > > Once this is an RFC, it doesn't matter what the other possible and > unselected design choices were. > > > c) Sec 7.2.1: " S-BFD packet MUST be demultiplexed with lower layer >> > information >> > (e.g., dedicated destination UDP port, associated channel type)." >> > Where precisely is this defined or described? Is there an allocation >> > for a dedicated UDP >> > port for S-BFD? I don't see any normative reference to such. In >> > particular, since the format >> > for an S-BFD control packet is exactly the same as for BFD and since >> only >> > this demultiplexing >> > with lower layer information is used to tell the difference between >> S-BFD >> > and BFD packets, >> > this document requires more specifics. >> > >> >> This is similar to RFC 5880 and RFC 5881. The actual S-BFD applications >> specify this. For example, bfd-seamless-ip defines the UDP port. We >> purposely do not want to have the specification (either explicitly or >> normatively pointed to) from this document, as this is just the base >> specification. >> > > Why? Unlike RFC 5880, this document mentions UDP ports as an example of a > demultiplexer. > While I do understand that BFD can run with different transports, it is > useful to clearly articulate > one use transport that has enough information to be actually implemented. > In this case, that's > just a normative reference to another document progressing at the same > time. > > I can't get too worked up about normative vs. informative references in > general - the guideline I > use is whether an implementor would need to read the reference to properly > implement the > functionality. > > If you feel extremely strongly that the reference must be informative, I'm > not going to dig in my > heels - but PLEASE put a reference by the mention of the UDP port. > > > >> > >> > ---------------------------------------------------------------------- >> > COMMENT: >> > ---------------------------------------------------------------------- >> > >> > 1) In the last paragraph of Sec 4.2: " Even when following the separate >> > discriminator pool approach, >> > collision is still possible between one S-BFD application to another >> > S-BFD application, that may be using different values and algorithms >> > to derive S-BFD discriminator values. If the two applications are >> > using S-BFD for a same purpose (e.g., network reachability), then the >> > colliding S-BFD discriminator value can be shared. If the two >> > applications are using S-BFD for a different purpose, then the >> > collision must be addressed. How such collisions are addressed is >> > outside the scope of this document." >> > >> > Sec 4.1 talks about the need for the S-BFD Discriminator to be unique >> > within an Administrative Domain. >> > I don't see any details of that addressed here. What is addressed >> > here seems to be the case for multiple >> > S-BFD discriminators applying to the same node - which is specifically >> > discouraged at the end of Sec 3. >> > Rather than simply describing the issue as "outside the scope of this >> > document", please either describe it >> > as "future work and multiple S-BFD discriminators is discouraged" or >> > add a reference. >> > >> >> Good point, will do. >> >> > 2) In Sec 6.1: "bfd.SessionType:" is defined but the only possible >> values >> > are for SBFD. Is it possible for a BFD >> > session to still use the same bfd structure? I don't see a value for >> > SessionType there; I'd expect to see at least >> > a value for the original BFD session and possible an undefined or >> > unspecified value for future proofing. >> > >> > >> >> Traditional BFD does not use this state variable. That’s why we don’t >> need to define a value for BFD. However, future specs can when it is >> relevant (e.g, using BFD for various types as opposed to S-BFD), as for >> example bfd-multipoint. >> > > Right - I understand that. I'm thinking a bit from the implementation > perspective. If I have the same data-structures and similar logic for BFD > and S-BFD, then there'll be a bfd.SessionType even for BFD sessions that > don't need it. Clarifying a value of "Unused" or "Classical BFD" gives > clarity that one > of the S-BFD options doesn't need to be chosen. > > This is just a comment. It's up to your best judgement. > > Thanks, > Alia > > > Please let us know your thoughts on the responses above, and we can send >> out diffs. >> >> Thanks! >> >> — Carlos. >> >> >> > > >
