Hi Bob,
thank you for further clarifications and the new ideas. Please find my
follow-up in-line and tagger GIM2>>.
I'll check for nits and grammar and will publish the new version shortly.
Regards,
Greg
On Tue, May 29, 2018 at 3:22 AM, Bob Briscoe <i...@bobbriscoe.net
<mailto:i...@bobbriscoe.net>> wrote:
Greg,
On 26/05/18 20:49, Greg Mirsky wrote:
Hi Bob,
thank you for the thorough review, detailed questions and helpful
comments. Please find my answers in-line and tagged GIM>>.
I've updated the working version of the draft based on your
comments and suggestions. Appreciate your feedback whether all
questions have been addressed.
Attached please find the diff of -16 and the working version and
the copy of the working version of the draft.
Regards,
Greg
On Mon, May 21, 2018 at 5:20 PM, Bob Briscoe <i...@bobbriscoe.net
<mailto:i...@bobbriscoe.net>> wrote:
Reviewer: Bob Briscoe
Review result: Not Ready
Altho this is a TSV-ART review, I did not find many
transport-related issues to
focus on, except a need to justify why lack of information
for adapting the
transmit interval is not an issue.
Nonetheless, I did find a few other non-trivial technical
issues, including 2
security issues, enumerated below (I mis-spent some of my
early research career
working on a multicast session control and security, for
which we used
beaconing control channels). However, I only have passing
prior knowledge of
BFD, so my critique might be off-beam.
==Main Technical Concerns===
1/ Mandatory return path?
RFC5880 is the base RFC that this draft updates. RFC5880 says
that
"unidirectional links" are in scope, but only as long as
there is a return path.
The introduction of this bfd-multipoint draft seems to
contradict that, making
a return path optional: "
As an option, the tail may notify the head of the lack of
multipoint
connectivity. Details of tail notification to the head
are outside
the scope of this document.
"
It's allowable for irrelevant details to be outside the
scope, but surely it
needs to be clear whether at least the existence of a return
path is mandatory.
GIM>> Thank you for highlighting this issue. I think that the
second paragraph of Introduction is the appropriate place to note
that this mechanism does not require existence of a return path
from tails to the head. Would the following be acceptable:
NEW TEXT:
Use of BFD in
Demand mode enables a tail monitor availability of a
multipoint path
even without the existence of some kind of a return path to
the head.
2/ Mechanism for verifying connectivity, or not?
The introduction seems to contradict itself:
"
As multipoint transmissions are inherently unidirectional,
this
mechanism purports only to verify this unidirectional
connectivity.
"
"
Term "connectivity" in this document is not being used in
the context
of connectivity verification in transport network but as an
alternative to "continuity", i.e. existence of a
forwarding path
between the sender and the receiver.
"
How can this mechanism verify connectivity, but not be used
in the context of
connectivity verification in the transport network?
GIM>> This draft defines the base specification for multipoint
BFD. In order for multipoint BFD to support the transport-like
connectivity verification we need to do work similar to described
in RFC 6428.
[BB]: Caveat: I am having to talk in generalizations, cos I don't
actually know how you are going to get this protocol to work in a
wide range of circumstances, given inherent problems like
multipoint feedback implosion {Note 1}.
My point was that, having broken up the drafts in this way, this
draft on its own no longer defined a workable protocol. Therefore,
it needed some references to other drafts (even if they are
placeholders), so that the extent of the pre-requisite collection
of work is clear. The refs you give later go a long way to fixing
this issue.
If each pre-requisite protocol is intended to only represent one
example, the citation can say that and the ref can be informative.
But with zero examples for all the prerequisite parts, all the
reader sees is a dismembered octopus, not a protocol.
3/ Use case
The introduction seems to be written rather academically.
Surely, in cases
where there is never a return path, only the tails will ever
be able to verify
connectivity. The head could continue transmitting BFD
packets (and data
packets) for years without ever knowing whether it is
connected to anything.
Knowledge of connectivity is surely of little use if it
excludes the link
sender, which is the node that always controls routing.
If there are scenarios where it is useful for tails but not
the head to be able
to verify connectivity, can you please give a concrete example?
GIM>> One example could be a multicast system with 1+1
protection. Without multipoint BFD tails would not be able to
detect the failure of the muticast path from the head. Other
examples discussed in several drafts:
* BESS WG draft MVPN fast upstream failover
<https://tools.ietf.org/html/draft-ietf-bess-mvpn-fast-failover-03>
* Individual draft BFD for Multipoint Networks and VRRP Use
Case
<https://tools.ietf.org/html/draft-mirsky-bfd-p2mp-vrrp-use-case-01>
* Individual draft BFD for Multipoint Networks and PIM-SM Use
Case
<https://tools.ietf.org/html/draft-mirsky-pim-bfd-p2mp-use-case-00>
I am not sure how references to non-WG drafts affect the progress
of this document. Appreciate your suggestion.
[BB]: In my experience, informative refs to non-WG drafts as
use-cases would be OK during doc development. However, if a non-WG
draft fails to proceed, its citation has to be removed later. So
choose those that are most likely to proceed.
Nonetheless, if you cite some specs that turn this into a workable
protocol (see previous issue) use-cases might not be necessary.
GIM2>> I've added reference to use of this mechanism in BGP/MPLS MVPN.
4/ Interval adaptation
Text is needed to describe why it is not an issue for the
head to be unaware
whether it needs to adapt its transmit interval. Otherwise,
this seems
potentially problematic.
GIM>> Very interesting, thank you. I wouldn't say that the case
when a tail cannot process incoming mpBFD control packets at the
offered rate is entirely non-issue. Such scenario must be handled
by the implementation and may be controlled by local policy,
e.g., close the MultipointTail session.
[BB]: Fair enough.
In some scenarios, this issue will not necessarily be so unlikely tho:
* If asymmetric crypto is used to solve the group message
authentication problem (see later), the processing burden on any
lightweight endpoints might lead to message verification leaving
less available processor resource than needed for the host's other
tasks.
* Each tail might be joined to a very large number of multipoint
sessions.
Where would you suggest to add the text?
I would suggest a new section listing potential issues when there
is no back channel.
GIM2>> I've tried to start the new section but decided to insert the
note in the first paragraph of Timer Manipulation section. Hope that
is acceptable.
5/ Inability to authenticate the sender with symmetric keys
In unicast scenarios, symmetric keys can be used for message
authentication,
because each end knows there is only one other node with the
shared key. But,
in multipoint scenarios, all the tails would share the key,
so a shared key
does not authenticate who sent the message - any tail can
spoof the head from
the viewpoint of the other tails.
Therefore text is needed to say that:
* multipoint message authentication is limited to cases where
all tails are
trusted not to spoof the head, if shared keys are used. *
otherwise asymmetric
message authentication would be needed, e.g. TESLA [RFC4082]
GIM>> Thank you for the suggested text. Would the Security
Considerations section be appropriate place:
[BB]: Well, the point limits the applicability of the assumption
about security in 5. 'Assumptions', so this would fit well there.
Then "Security Considerations" should point to everywhere in the
doc that discusses security, such as this (to save time for
security reviewers).
NEW TEXT:
Use of shared keys to authenticate BFD Control packet in
multipoint
scenarios is limited because tail can spoof the head from the
viewpoint of the other tails. Thus, if shared keys are used, all
tails MUST be trusted not to spoof the head.
[BB]: Normally a MUST is applied to implementations. It would be
rather odd to require users/operators to satisfy a spec
requirement, particularly requiring them to trust each other. I
think this should be written as an applicability statement not a
normative requirement.
GIM2>> I've adopted text suggested by Spencer and moved the paragraph
to section Assumption.
Otherwise, asymmetric
message authentication would be needed, e.g., Timed Efficient
Stream
Loss-Tolerant Authentication (TESLA) as described in [RFC4082].
[BB]: If you are going to allow for cases where all tails are
trusted not to spoof the head, then the assumption written in
section 5 is no longer correct.
[FYI, RFC4082 is only a generic description. Many RFCs have been
written to authenticate specific protocols along TESLA lines.]
A related nit: Section 5 says all tails are assumed to have a
common
authentication key. In cases with symmetric message
authentication, surely the
head also needs the same key.
GIM>> Thank you. Please check the updated text:
NEW TEXT:
If authentication is in use, the head and all tails must be
configured to have a common authentication key in order for
the tails
to validate received the multipoint BFD Control packets.
[BB]: Yup. Except delete "received the".
Also see above about whether this is now a correct assumption.
GIM2>> I think that s/must/may/ will keep the Assumption valid.
6/ Source address spoofing
A 3-way handshake makes a protocol robust against simple
source address
spoofing. Without a 3WHS, surely the spec. needs to highlight
this
vulnerability or discuss ways to address it or why it is not
an issue.
GIM>> Because mpBFD control packets cannot be demultiplexed by
tail based on the value of Your Discriminator field as per RFC 5880,
the new procedure outlined in Section 4.7:
IP and MPLS multipoint tails MUST demultiplex BFD packets
based on a
combination of the source address, My Discriminator and the
identity
of the multipoint tree which the Multipoint BFD Control packet was
received from. Together they uniquely identify the head of the
multipoint path.
and described in details in Section 4.13.2:
If the Multipoint (M) bit is set
If the Your Discriminator field is nonzero, the packet
MUST be
discarded.
Select a session as based on source address, My
Discriminator
and the identity of the multipoint tree which the Multipoint
BFD Control packet was received. If a session is found, and
bfd.SessionType is not MultipointTail, the packet MUST be
discarded. If a session is not found, a new session of type
MultipointTail MAY be created, or the packet MAY be
discarded.
This choice is outside the scope of this specification.
Would you suggest additional text to a use case where the new
demultiplexing is not sufficent to protect from source address
spoofing?
[BB]: I seem to have become co-opted into redesigning this
protocol. I'd prefer to limit my involvement to reviewing :)
7/ Scope
On eight occasions an issue is raised, but resolution is
stated as outside the
scope of this document. It is OK to limit the scope of a
spec, for example to
allow for multiple solutions to each issue. But at least one
solution must
already exist for each of these eight issues. So, at least
one example solution
ought to be cited in each case. If any issues are open, then
this should not be
on the standards track.
It would be more useful to state why each issue is out of
scope. This would be
helped by stating from the start what the scope of the
document is.
GIM>> I've listed all eight occasions with the explanation for
each one:
1. Details of tail notification to the head are outside the
scope of this document. - Notifications by tails addressed in
draft-ietf-bfd-multipoint-active-tail
<https://datatracker.ietf.org/doc/draft-ietf-bfd-multipoint-active-tail/>.
Will add as informational reference.
[BB]: Good.
Nonetheless, given you have confirmed that a reverse path is
optional, the doc still needs to address the case where there is
no reverse path.
GIM2>> Introduction notes:
Use of BFD in Demand mode enables a tail monitor availability
of a multipoint path even without the existence of some kind of a
return path to the head.
{Note 1} For the active tail draft, you might find the following
ideas for scaling multipoint feedback useful:
*Statistical feedback:*
Nonnenmacher, Jö. & Biersack, E.W., "Scalable Feedback for Large
Groups <https://dl.acm.org/citation.cfm?id=312251>," IEEE/ACM
Transactions on Networking 7(3):375--386 (June 1999)
FUHRMANN, T., AND WIDMER, J. "On the scaling of feedback
algorithms for very large multicast groups
<https://dl.acm.org/citation.cfm?id=2294709>," Computer
Communications 24, 5-6 (March 2001), 539 547;
WIDMER, J., AND FUHRMANN, T. Extremum feedback for very large
multicast groups. Tech. Rep. TR 12-2001, Prakfische Informatik IV,
University of Mannheim, Germany, May 2001.
Also, anycast can be used to select different representative
feedback tails, e.g. for a certain time, which might overlap with
the periods for which a few other tails are selected using
subsequent anycasts.
*Logical 'AND' feedback:*
Burbridge, T., Soppera, A., Briscoe, R. and Jacquet, A. "Method
and device for co-ordinating networked group members
<https://worldwide.espacenet.com/publicationDetails/biblio?II=0&ND=3&adjacent=true&locale=en_EP&FT=D&date=20060406&CC=US&NR=2006075022A1&KC=A1#>"
Patent WO2004059479, (Jul 2004; Priority 24 Dec 2002)
[AFAICT this patent is still being maintained, so use of it in a
protocol would require an IPR declaration.]
1. Details of how the head keeps track of tails and how tails
alert their connectivity to the head are outside scope of
this document. - Same as #1.
[BB]: And my response is same as #1.
1. Bootstrapping BFD session to multipoint MPLS LSP in case of
penultimate hop popping is outside the scope of this
document. - It may use control plane as in MVPN draft. Will
add as informational reference.
[BB]: Good.
1. Use of other types of encapsulation of the BFD control
message over multipoint LSP is outside the scope of this
document. - This in reference to ACH encapsulation that is
discussed in draft-mirsky-mpls-p2mp-bfd
<https://tools.ietf.org/html/draft-mirsky-mpls-p2mp-bfd-03>.
Should it be added as informational reference? What would be
the imacpt of progressing this specification?
[BB]: See earlier comment about citing individual drafts (I don't
have enough BFD knowledge to give a BFD-specific answer).
Also, in my review I should also have said: when creating new
encapsulations, pls see the common transport issues related to
encapsulation:
https://trac.ietf.org/trac/tsv/wiki/tsvdir-common-issues#TunnelingprotocolsandTransportRelatedIssues
<https://trac.ietf.org/trac/tsv/wiki/tsvdir-common-issues#TunnelingprotocolsandTransportRelatedIssues>
1. Change in the value of bfd.RequiredMinRxInterval is outside
the scope of this document. - Same as #1.
[BB]: And my response is same as #1.
1. If a session is not found, a new session of type
MultipointTail MAY be created, or the packet MAY be
discarded. This choice is outside the scope of this
specification. - I propose to add "based on local policy" to
the last sentence.
[BB]: On what basis will local policy decide? It's my job as a
reviewer to ensure that this spec does not contain any loose ends
(open issues).
GIM2>> It could be based on the maximum number of MultipointTail
sessions and number of active MultipointTail sessions on the node. I'd
clarify it as:
This choice MAY be controlled by the local policy, e.g. maximum
number of
MultipointTail sessions and number of active MultipointTail sessions,
and is outside the scope of this specification.
1. The exact method of selection is application-specific and is
thus outside the scope of this specification. - This is
copied from RFC 5880: "The exact method of selection is
application specific and is thus outside the scope of this
specification." as the section is to replace Section 6.8.6.
[BB]: OK.
1. If a matching session is not found, a new session of type
PointToPoint MAY be created, or the packet MAY be discarded.
This choice is outside the scope of this specification. -
Same as #6.
[BB]: And my response is same as #6.
[Sry, my embedded comments have broken your numbered list.]
There is also one issue that is "for further discussion".
Does this mean the
document is not ready yet?
GIM>> I think that the question left for further discussion is
non-technical:
The semantic difference between Down and AdminDown state is for
further discussion.
I propose to remove the sentence altogether.
[BB]: OK.
8/ Incremental deployment
Section 4.4.1. "New State Variable Values" defines
bfd.SessionType =
PointToPoint as well as a couple of Multipoint alternatives.
Presumably this
spec does not require all existing PointToPoint systems to
support this state
value. Is the implication that only Multipoint systems that
happen to be in
PointToPoint mode should use this state?
GIM>> You're aboultely right, existing implementations of BFD
don't need to support bfd.SessionType variable. Only
implementations that support the base BFD, single-hop or
multi-hop, and this specification, mpBFD, should support
bfd.SessionType and set it to PointToPoint value when BFD is in
single-hop or multi-hop mode. When in mpBFD mode, bfd.SessionType
will be set to either MultipointHead or MultipointClient.
[BB]: Doesn't something need to be written (or referenced) to
clarify all this? AFAIR, this spec. never made clear that
multipoint is a fork in implementations.
GIM2>> And so is S-BFD. (Note, bfd.SessionType introduced in RFC 7880
S-BFD but missed to define PointToPoint value).
==Nits==
* Sometimes 'tree' is used to mean a multipoint path in
general. I suspect
'path' was intended.
GIM>> I've found six occasions of "tree" and s/tree/path/
4.8. Packet consumption on tails
s/Node/Nodes/
s/packet/packets/
s/demultiplex/demultiplexed/
GIM>> Accepted all three.
4.9. Bringing Up and Shutting Down Multipoint BFD Service
"
a newly
started head (that does not have any previous state
information
available) SHOULD start with...
"
...
"
... (so long as the restarted head
is using the same or a larger value of
bfd.DesiredMinTxInterval than
it did previously).
"
If it has no state available, how can it know whether a value
is larger than
previously?
GIM>> You are right, the BFD system at the head would not know
the previous value of bfd.DesiredMinTxInterval. This text is to
caution operator from decreasing bfd.DesiredMinTxInterval upon
restart of the BFD system.
4.9. Bringing Up and Shutting Down Multipoint BFD Service
There are a number of "SHOULD"s and "SHOULD NOT"s that do not
state or give
examples of circumstances in which the "SHOULD" would not be
appropriate. If
there are none, "MUST" would be more appropriate.
GIM>> In the first paragraph SHOULD may not be followed if the
implementation can differentiate between the very first start and
restarts of BFD system. If it is the first start of BFD system,
the head MAY directly progress to Up state skipping Down state.
The last paragraph describes graceful shuttdown. The head MAY
shut the BFD mp session abruptly by just stopping transmission of
BFD Control packets.
[BB]: I assume you will say all this in the next rev, not just in
this email.
GIM2>> Appended the following:
NEW TEXT:
Alternatively, the head MAY stop transmitting BFD Control packets
and not send any more BFD Control packets with the new state (Down or
AdminDown). Tails
will declare the multipoint session down only after the detection time
interval runs out.
4.10. Timer Manipulation
"
Because of the one-to-many mapping, a session of type
MultipointHead
SHOULD NOT initiate a Poll Sequence in conjunction with
timer value
changes. However, to indicate a change in the packets,
MultipointHead session MUST send packets with the P bit set.
MultipointTail session MUST NOT reply if the packet has M
and P bits
set and bfd.RequiredMinRxInterval set to 0.
"
The initial "SHOULD NOT" needs to be written another way. Perhaps
"
...a session of type MultipointHead
does not initiate a Poll Sequence
"
The head's normative action is defined by the following
"MUST", then the tail's
"MUST NOT reply" is what prevents the poll sequence happening.
GIM>> A Poll Sequence starts with the initiator setting Poll bit.
Unless the peer sends BFD Control packet with Finl bit set the
poll sequence would continue indefinetely. The initial SHOULD
NOT, in my opinion, correctly points that the mechanism of Poll
Sequence not to be used by MultipointHead when changing
transmission interval. I think that MUST in the first paragraph
can be downgraded to MAY because the MultipointHead doesn't need
to use transition period when changing the transmission interval
to lower level, i.e., decreasing frequency. May I propose the
following:
OLD TEXT:
Because of the one-to-many mapping, a session of type
MultipointHead
SHOULD NOT initiate a Poll Sequence in conjunction with timer
value
changes. However, to indicate a change in the packets,
MultipointHead session MUST send packets with the P bit set.
NEW TEXT:
Because of the one-to-many mapping, a session of type
MultipointHead
SHOULD NOT initiate a Poll Sequence in conjunction with timer
value
changes. However, to indicate a change in the packets,
MultipointHead session MAY send packets with the P bit set
during transition period.
[BB]: If I were an implementer, I would not know what this is
saying I ought to implement. The spec needs to answer this
question: If the head changes the packets what happens differently
if it sets the P bit vs. if it doesn't?
GIM2>> I think we can expect that the implementer is familiar with RFC
5880 and, very likely, has implemented it one or more times already.
4.11. Detection Times
Delete "in the calculation" (repetition).
GIM>> Done.
4.13.1. Reception of BFD Control Packets
Some actions seem to be only relevant to PointToPoint
sessions, but they are
stated for all session types. Specifically: "the transmission
of Echo packets,
if any, MUST cease." "the Poll Sequence MUST be terminated."
"MUST cease the
periodic transmission of BFD Control packets" "MUST send
periodic BFD Control
packets"
"
If bfd.SessionType is PointToPoint, update the Detection Time as
described in section 6.8.4 of [RFC5880]. If
bfd.SessionType is
MultipointTail,
"
The second sentence above ought to start on a new line as an
Else if.
GIM>> Hope I got it right:
If bfd.SessionType is PointToPoint, update the Detection
Time as
described in section 6.8.4 of [RFC5880].
Else
If bfd.SessionType is MultipointTail, then update the
Detection
Time as the product of the last received values of
Desired Min
TX Interval and Detect Mult, as described in Section 5.11 of
this specification.
4.13.2. Demultiplexing BFD Control Packets
"
This section is part of the replacement for [RFC5880]
section 6.8.6,
separated for clarity.
"
Do you mean "This section replaces the sentence: "If the
Multipoint (M) bit is
nonzero, the packet MUST be discarded." in [RFC5880] section
6.8.6?
The statements under "If the Multipoint (M) bit is set" are
not formatted like
the rest of the if-else logic, and I think an Else is missing
at the start of
the statement after the nested "If".
GIM>> Agree, the paragraph is not structured properly. How about
this formating:
If the Multipoint (M) bit is set
If the Your Discriminator field is nonzero, the packet
MUST be
discarded.
Select a session as based on source address, My
Discriminator
and the identity of the multipoint path which the Multipoint
BFD Control packet was received.
If a session is found, and bfd.SessionType is not
MultipointTail, the packet MUST be discarded.
Else
If a session is not found, a new session of type
MultipointTail MAY be created, or the packet MAY be
discarded. This choice is outside the scope of this
specification.
[BB]: As long as this represents the logic you want, fine. The
point is that the indentation is the only clue to whether one 'if'
is conditional on a previous 'if', or not.
HTH
Bob
--
________________________________________________________________
Bob Briscoehttp://bobbriscoe.net/
