Hi Adrian,
Thanks for your thorough review and thoughtful comments.
Please see inline.
Original
From: AdrianFarrelviaDatatracker <noreply@xxxxxxxx>
To: rtg-dir@xxxxxxxx <rtg-dir@xxxxxxxx>;
Cc: draft-ietf-bfd-unaffiliated-echo.all@xxxxxxxx <draft-ietf-bfd-unaffiliated-echo.all@xxxxxxxx>;last-call@xxxxxxxx <last-call@xxxxxxxx>;rtg-bfd@xxxxxxxx <rtg-bfd@xxxxxxxx>;
Date: 2024年09月26日 22:17
Subject: Rtgdir last call review of draft-ietf-bfd-unaffiliated-echo-11
Reviewer: Adrian Farrel
Review result: Has Issues
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
https://wiki.ietf.org/en/group/rtg/RtgDir
Although these comments are primarily for the use of the Routing ADs, it would
be helpful if you could consider them along with any other IETF Last Call
comments that you receive, and strive to resolve them through discussion or by
updating the draft.
Cheers,
Adrian
Document: draft-ietf-bfd-unaffiliated-echo
Reviewer: Adrian Farrel
Review Date: 2024-09-26
IETF LC End Date: 2024-10-09
Intended Status: Standards Track
Summary:
I have some minor concerns about this document that I think should be resolved
before publication.
This draft is generally in a good condition and is readable. There are a few
issues of English usage that the RFC Editor will clean up.
review.
I wish that there was discussion of implementation status in the draft.
The Abstract helpfully says "updates 5800", but it also needs to say
how. For example...
This document updates RFC 5880 by defining a new method BFD Echo-Only
method without requiring an implementation to support the full BFD
protocol.
Similar to the previous point, the Introduction needs to indicate the
update. For example...
OLD
This document specifies the use of the Unaffiliated BFD Echo over
IPv4 and IPv6 for single IP hop.
NEW
This document updates [RFC5880] by defining a new method BFD Echo-Only
method without requiring an implementation to support the full BFD
protocol. It describes the use of the Unaffiliated BFD Echo over
IPv4 and IPv6 for a single IP hop. A full description of the updates
to [RFC5800] is provided in Section 3.
END
[XM]>>> OK. Will change the text as you suggested.
---
I know where you are going with the idea of an "Unaffiliated BFD Echo
Session", but I find the concept a little peculiar because, well, it
isn't really a session (especially as shown in Figure 1) because the
remote end of the "session" doesn't even know that it exists. I think,
therefore, you may need some explanatory text to say something like...
An Unaffiliated BFD Echo Session is not actually a BFD session
because there is no coordination of BFD protocol state between the
two link ends: the remote end does not support BFD and so cannot
engage in a BFD session. The initiator may regard the Unaffiliated
BFD Echo Session as a BFD session within its implementation.
[I know that the second paragraph of section 2 can imply the above, but
I don't think it is explicit enough. So you might just insert my
suggested text (with any edits you consider appropriate) before that
paragraph.]
Node A Node B
+----------------+ +----------------+
| | | |
| ------------| | |
| |Unaffiliated| | |
| | BFD Echo ---> | |
| | Session | | |
| | | Unaffiliated BFD Echo | |
| | ------------------------------- BFD |
| | | | packets |
| | <------------------------------- looped |
| ------------| | |
| | | |
| | | |
+----------------+ +----------------+
BFD supported BFD not supported
[XM]>>> Thank you for the nice picture! Will use it in the next revision.
---
In section 2, I had to read a piece of text many times and then re-read
RFC 5800 before I understood what you are saying. You have...
For unaffiliated echo, a Unaffiliated BFD Echo session is created on
device A, and the Unaffiliated BFD Echo session MUST follow the BFD
state machine defined in Section 6.2 of [RFC5880], except that the
received state is not sent but looped back from the remote system.
Unaffiliated BFD Echo does not use the AdminDown state.
I think the point is that the state transition events do not arise from
BFD messages initiated by the peer BFD speaker, but come from BFD
messages initiated at the local node and looped back by the remote node.
So...
For unaffiliated echo, a Unaffiliated BFD Echo session is created on
device A, and the Unaffiliated BFD Echo session MUST follow the BFD
state machine defined in Section 6.2 of [RFC5880], except that the
received state is not extracted from BFD messages originated by the
remote system, but from messages originated by the local system and
looped back from the remote system. Therefore, Unaffiliated BFD Echo
does not use the AdminDown state.
a DOWN state and an AdminDown state-transition event. I think it is the
event that is not used because you would not send an unaffiliated echo
to say "admin down". However:
- I don't think you would send an unaffiliated echo to say "down" either
- A timer pop would still put you into DOWN from INIT or UP
- There has to be a way for the local operator to put the session into
DOWN (that seems to be absent from the state machine in 5800), so it
can probably stay absent here.
Section 4, question...
Could an attacker interpose themselves between the two nodes and perform
loopback? Loopback is an easy function with no requirement to generate
any additional security, so it is easier than impersonating a full BFD
implementation.
happen? I think it would mean that traffic would continue to be dropped
until the IGP noticed the failure.
Using "(s)" for optional plurals is unnecessary. English allows the
plural to include the singular, so you can say, for example,...
The faults can be on interfaces, data links, and even the
forwarding engines.
[XM]>>> Thank you for the education. :-) Will do s/data link(s)/data links.
---
OLD
BFD defines Asynchronous and Demand modes to satisfy various
deployment scenarios.
NEW
BFD defines the Asynchronous mode and the Demand mode to satisfy
various deployment scenarios.
END
[XM]>>> OK.
---
OLD
It also supports an Echo function to reduce
the device requirement for BFD.
NEW (I think)
It also supports an Echo function to that
reduces the level of BFD support required in device implementations.
END
[XM]>>> OK. I believe you mean "an Echo function that", not "an Echo function to that".
---
OLD
The former scenario is referred to as affiliated BFD Echo, which is
not changed by this document in any way. The latter scenario is
referred to as Unaffiliated BFD Echo, which is specified in this
document.
NEW
The former scenario is referred to as "Affiliated BFD Echo", which is
not changed by this document in any way. The latter scenario is
referred to as "Unaffiliated BFD Echo", which is specified in this
document.
END
[XM]>>> OK.
---
It would be nice if Section 2 began with text, not direct into the
figure.
Review result: Has Issues
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
https://wiki.ietf.org/en/group/rtg/RtgDir
Although these comments are primarily for the use of the Routing ADs, it would
be helpful if you could consider them along with any other IETF Last Call
comments that you receive, and strive to resolve them through discussion or by
updating the draft.
Cheers,
Adrian
Document: draft-ietf-bfd-unaffiliated-echo
Reviewer: Adrian Farrel
Review Date: 2024-09-26
IETF LC End Date: 2024-10-09
Intended Status: Standards Track
Summary:
I have some minor concerns about this document that I think should be resolved
before publication.
This draft is generally in a good condition and is readable. There are a few
issues of English usage that the RFC Editor will clean up.
[XM]>>> Understood. Thank you!
review.
I wish that there was discussion of implementation status in the draft.
[XM]>>> As far as I know, there are implementations from at least Broadcom, Huawei, Juniper, and ZTE.
For Broadcom's implementation, the below public link was provided by Jeff Haas on the mailing list.
https://manualzz.com/doc/o/1277tk/broadcom-bcm88690--bcm88800--bcm88480--and-bcm88280-packe...-32.13.5--configuring-bfd-echo?p=571
For other implementations, I don't know any public links.
The Abstract helpfully says "updates 5800", but it also needs to say
how. For example...
This document updates RFC 5880 by defining a new method BFD Echo-Only
method without requiring an implementation to support the full BFD
protocol.
[XM]>>> OK. Will flesh out "updates RFC 5880" following your text.
---
Similar to the previous point, the Introduction needs to indicate the
update. For example...
OLD
This document specifies the use of the Unaffiliated BFD Echo over
IPv4 and IPv6 for single IP hop.
NEW
This document updates [RFC5880] by defining a new method BFD Echo-Only
method without requiring an implementation to support the full BFD
protocol. It describes the use of the Unaffiliated BFD Echo over
IPv4 and IPv6 for a single IP hop. A full description of the updates
to [RFC5800] is provided in Section 3.
END
[XM]>>> OK. Will change the text as you suggested.
---
I know where you are going with the idea of an "Unaffiliated BFD Echo
Session", but I find the concept a little peculiar because, well, it
isn't really a session (especially as shown in Figure 1) because the
remote end of the "session" doesn't even know that it exists. I think,
therefore, you may need some explanatory text to say something like...
An Unaffiliated BFD Echo Session is not actually a BFD session
because there is no coordination of BFD protocol state between the
two link ends: the remote end does not support BFD and so cannot
engage in a BFD session. The initiator may regard the Unaffiliated
BFD Echo Session as a BFD session within its implementation.
[I know that the second paragraph of section 2 can imply the above, but
I don't think it is explicit enough. So you might just insert my
suggested text (with any edits you consider appropriate) before that
paragraph.]
[XM]>>> You're exactly correct. Will follow your suggestion to add the explanatory text.
Node A Node B
+----------------+ +----------------+
| | | |
| ------------| | |
| |Unaffiliated| | |
| | BFD Echo ---> | |
| | Session | | |
| | | Unaffiliated BFD Echo | |
| | ------------------------------- BFD |
| | | | packets |
| | <------------------------------- looped |
| ------------| | |
| | | |
| | | |
+----------------+ +----------------+
BFD supported BFD not supported
[XM]>>> Thank you for the nice picture! Will use it in the next revision.
---
In section 2, I had to read a piece of text many times and then re-read
RFC 5800 before I understood what you are saying. You have...
For unaffiliated echo, a Unaffiliated BFD Echo session is created on
device A, and the Unaffiliated BFD Echo session MUST follow the BFD
state machine defined in Section 6.2 of [RFC5880], except that the
received state is not sent but looped back from the remote system.
Unaffiliated BFD Echo does not use the AdminDown state.
I think the point is that the state transition events do not arise from
BFD messages initiated by the peer BFD speaker, but come from BFD
messages initiated at the local node and looped back by the remote node.
So...
For unaffiliated echo, a Unaffiliated BFD Echo session is created on
device A, and the Unaffiliated BFD Echo session MUST follow the BFD
state machine defined in Section 6.2 of [RFC5880], except that the
received state is not extracted from BFD messages originated by the
remote system, but from messages originated by the local system and
looped back from the remote system. Therefore, Unaffiliated BFD Echo
does not use the AdminDown state.
[XM]>>> Yes, your text is better. Will use it in the next revision.
a DOWN state and an AdminDown state-transition event. I think it is the
event that is not used because you would not send an unaffiliated echo
to say "admin down". However:
- I don't think you would send an unaffiliated echo to say "down" either
- A timer pop would still put you into DOWN from INIT or UP
- There has to be a way for the local operator to put the session into
DOWN (that seems to be absent from the state machine in 5800), so it
can probably stay absent here.
[XM]>>> You're right that there is no AdminDown state on the FSM diagram in Section 6.2 of RFC 5880. However I found that in Section 4.1 of RFC 5880 AdminDown is listed as one of BFD session states.
Section 4, question...
Could an attacker interpose themselves between the two nodes and perform
loopback? Loopback is an easy function with no requirement to generate
any additional security, so it is easier than impersonating a full BFD
implementation.
[XM]>>> In theory it would happen, however in the real deployment I doubt it would happen. Currently we have two specific use cases of the Unaffiliated BFD Echo, one is between RG and IP Edge (as described in Section 6.2.2 of BBF TR-146), another one is between DC Gateway and VM of Server (as described in draft-wang-bfd-one-arm-use-case). For the two use cases it seems difficult for an attacker to interpose itself between the two nodes.
happen? I think it would mean that traffic would continue to be dropped
until the IGP noticed the failure.
[XM]>>> Any suggestions on the text? :-)
[XM]>>> I couldn't think of a way to protect against this if not putting new requirements on the adjacent looping system.
Using "(s)" for optional plurals is unnecessary. English allows the
plural to include the singular, so you can say, for example,...
The faults can be on interfaces, data links, and even the
forwarding engines.
[XM]>>> Thank you for the education. :-) Will do s/data link(s)/data links.
---
OLD
BFD defines Asynchronous and Demand modes to satisfy various
deployment scenarios.
NEW
BFD defines the Asynchronous mode and the Demand mode to satisfy
various deployment scenarios.
END
[XM]>>> OK.
---
OLD
It also supports an Echo function to reduce
the device requirement for BFD.
NEW (I think)
It also supports an Echo function to that
reduces the level of BFD support required in device implementations.
END
[XM]>>> OK. I believe you mean "an Echo function that", not "an Echo function to that".
---
OLD
The former scenario is referred to as affiliated BFD Echo, which is
not changed by this document in any way. The latter scenario is
referred to as Unaffiliated BFD Echo, which is specified in this
document.
NEW
The former scenario is referred to as "Affiliated BFD Echo", which is
not changed by this document in any way. The latter scenario is
referred to as "Unaffiliated BFD Echo", which is specified in this
document.
END
[XM]>>> OK.
---
It would be nice if Section 2 began with text, not direct into the
figure.
[XM]>>> OK. Propose to add the text as below.
NEW
This section specifies the Unaffiliated BFD Echo procedures.
END
Best Regards,
Xiao Min
-- last-call mailing list -- last-call@xxxxxxxx To unsubscribe send an email to last-call-leave@xxxxxxxx
