[dhcwg] Review comments for draft-ietf-dhc-dhcpv6-failover-protocol-00.txt

[Shawn Routhier <[e-mail]>]

During the last IETF there was a request to review the failover protocol draft.
I’ve now had a chance to review it and here are my comments.

regards,
Shawn

**

1) Glossary
Page 5, Delegable Prefix
The use of this term seems to be inconsistent.  In the glossary
it seems to be something like XXXX:0:1 from which a set of
prefixes to be delegated can be generated (say a set of /64s).
In 4.2.2 it seems to be treating the term as the prefix that would
be delegated for example XXXX:0:1:0/64.

2) Page 5, Failover endpoint - This mentions "prefixes associated with
that relationship".  I assume this means prefixes from which addresses
or prefixes may be leased but it could be read as simply the prefixes
that can be delegated.

3) Page 6, Resource - It might be useful to mention why temporary addresses
are not included in failover - either in this paragraph or maybe somewhere
else.  (Or maybe it was in the requirements document and I missed it?)

4) page 8,
4.2.1 Independent Allocation
I wonder if handing out addresses from only one partner's pool during
a PARTNER-DOWN state will have any odd effects.  In v4 the addresses could
sort of be re-mixed over time.  In v6 that won't happen.  Offhand I can't
think of any issues aside from the obvious pool imbalance, but it may
be worth spending some time to think if odd things could happen.

Paragraph 4 mentions that a partner MUST NOT lease Ipv6 addresses that
were assigned to its downed partner and later released by a client.
Should it also mention expired leases? declines?

5) 4.2.1.1 Independent allocation Algorithm
I am confused, shouldn't the low-order bit be 127?  Or is this text trying
to state that low-order bit within a prefix is set or cleared?  

Also while it is most likely that address ranges will be described via
a prefix it is also possible that they are described via start & end
addresses.  For example XXXX::100 to XXXX::200 (or if you prefer to
have a larger range XXXX::100:0:0:0 to XXXX::200:0:0:0).  When you
mention "prefix" are you intending to restrict the description? or
are you including something like turning the above ranges into a
set of prefixes for processing?

6) 4.2.2 Proportional Allocation
What happens if I have a large number of possible prefixes?  For example
the two servers are configured with a /32 and a prefix length of /64?
Does the spec need to either limit this in some fashion or supply some
guidelines or checks?

Are we expecting that the configuration will include the length of the
prefixes a server can delegate?

I don't see any option for the rewind optimization to avoid a secondary
in communications interrupted from running out of prefixes.

7) 5.3.4 POOLRESP
This definition isn't quite in line with 4.2.2.  This text has
"to indicate that it has responded to the secondary's request"
The use of "has responded to" suggests that the primary has completed
the response (as in has sent all the BNDUPDs and recevied all the BNDACKs)
4.2.2 states that the POOLRESP make be sent before, during or after any
BINDUPDs that need to be sent.

8) 5.4.2 OPTION_F_DNS_REMOVAL_INFO
I'm imagining the sub-options encapsulated within the option which
I think means the length depends on the sub-options.  Are you thinking
of some other arrangement?

9) 5.4.12 OPTION_F_PROTOCOL_VERSION
Does the document actually specify the version number somewhere?  I didn't
see it but maybe I missed it.  (Not the option code but the actual version
number.)

10) 5.4.20 OPTION_F_STATE_EXPIRATION_TIME
I believe this is the actual lifetime from 4.4?  If so it would be useful
to make that clear - either by changing one of the names to the other
or simply adding some text.

11) 6.1.1 Sending a CONNECT message
From the description of Failover Endpoint (page 5) it appears that one
server can be in failover relationships with multiple other servers and
one server could even be in multiple relationships with one other server.
The text in 6.2 seems to reinforce this as an option.  If this is true then
doesn't the CONNECT message need to contain information to specify which
relationship this CONNECT message is for?  (Note: I'm not arguing for
having multiple endpoints on the same pair of servers, I am wondering
how one of them will determine which of those endpoints the other is
trying to reach.)

12) 6.1.2 Receiving a CONNECT message
It sounds like the the two serves can use different versions of the protocol,
receive time, and unacked bind updates.  The latter two seem okay but the
text may want to mention that as a possibility.  The first one seems like
it might be difficult to deal with.

13) 6.3 Sending a STATE message
In the bullet list two of them use "this failover endpoint" while the
other two use "the failover endpoint".  I think they are also referring
to the failover endpoint sending the STATE message and find "this failover
endpoint" clearer.

14) 6.6 Unreachability detection
Does the document need to specify the messages are on this connection?
"...any message is transmitted on this connection...".

15) The second paragraph states that if the option OPTION_F_RECEIVE_TIME is
in the CONNECT or CONNECTACK message it is used.  But the descriptions of
CONNECT and CONNECTACK messages make it sound like that option is required.
One of the two sections of text should be clarified (I would vote for
always requiring the option and disconnecting if it isn't in the CONNECT
or CONNECTACK).

16) Figure 1, note 4
This has the following text:
...and the MCLT has passed since the entry in RELEASED, EXPIRED, or RESET states.
Should "in" be "into"?

17) Figure 1, note 7
This states that Addresses belonging to the primary server can be assigned to
the secondary server pool (or the reverse) but elsewhere it appears that
addresses are always handled via Independent Allocation rather than
Proportional Allocation.

18) 7.3 Tiems Required for Exchanging Binding Updates
partner-raw-clt-time
This mentions time context which is also mentioned later on but is never defined.
Perhaps adding an entry in the definitions would be useful?

state-expiration-time
As mentioned abouve for 5.4.20 I believe this is the actual lifetime as
handed to the client?  If so some text to clarify that would be useful.

19) 7.4 Sending Binding Updates
The double asterisk (**) is defined as "only if the value in the OPTION_F_BINDING_STATUS
is associated with a timeout".  I don't see a description of which status vaues
are associated with a timeout.  Does this mean that a timeout happened? or could
happen?  From the use I think this means status == ACTIVE and therefore there is
a time at which this entry would expire, but I don't think that is clear from
the text.

20) 7.5.2 Procesing Binding Updates, para 4
This paragraph seems to indicate that a BNDACK doesn't need to include
all of the OPTION_CLIENT_DATA options from the BNDUPD.  From the other
paragraphs it sounds like the BNDACK should contain all of them indicating
either success or failure.  In further reading 7.6 does state that the BNDACK
contains an option for each option in the BNDUPD so perhaps update the phrase
"each of which corresponds to one of the OPTION_CLIENT_DATA options"
to something like:
"one for each of the OPTION_CLIENT_DATA options"

21) Bullet item 6:
This states that the partner-lifetime can be updated from a BNDUPD.  In 7.3
the partner-lifetime is described as "The time that we will send (or have
sent) the partner...".  I think the text in 7.3 may need to update a bit
to indicate that the partner can update us as well.

22) 7.6 Sending Binding Acks
It occurs to me that this text and the previous text about IA_NAs and
IA_PDs doesn't say if multiple options for a single client  MUST or SHOULD
be in a single OPTION_CLIENT_DATA or if the server is free to put them
in multiple OPTION_CLIENT_DATA options (or even in separate BNDUPDs).
I'm not sure if the protocol should require any behavior but it might
be useful to at least point out the possibility.

23) 7.7 Receiving Binding Acks
Is there any processing for receving a NAK?  or should there be
text saying if a NAK is recieved don't do anything?

I also don't understand why the partner-lifetime is set to 0.  Given
the acked-partner-lifetime is set to it, retaining partner-lifetime doesn't
seem necessary but it doesn't seem necessary to 0 it either.  Is this
to provide a way to know if / when the partner will need to be updated if
it isn't 0?

24) 7.9 BNDUPD/ BNDACK Data Flow, Figure 4
At the bottom of the figure is the state expiration line in BNDACK.
I assume that there is nothing under storage on receiving side as
that server doesn't update the state-expiration-time.  Perhaps
explicitly mentioning that would be clearer?  Something like 
"no change" or "no update"?

25) 8.1 State Machine Operation
The last paragraph before the diagram mentions "+" and "-".  It would
be best to specify what each of them mean (I assume "-" means no
communication and "+" means communication.  I would also add a third
to mean both.  Some of the boxes include "-|+" while others simply
use "+-", having a single character would probably be clearer.

26) 8.3.2
This section makes use of PREVIOUS-STATE and CURRENT-STATE without
a definition of them.  I believe they are intended to be the variables
for those items? 

There seems to be some inconsistency in the use of PREVIOUS-STATE &
CURRENT-STATE and previous state & current state.  See step 2 para 2,
step 5 para2, step 5 para 4

27) In Step 6
Should the server proceed to transition from PREVIOUS-STATE to the
next state due to communications interrupted?  Or perhaps the text
from step 2 should be moved to here?  I don't see what having step
2 is doing.  In step 5 there is a mention of taking the communications
OK transition, so isn't step 6 the only other case?

28) 8.7.2 Transition out of RECOVER-DONE State
The text doesn't seem to match the diagram.  The text only mentions
transitioning when the partner is NORMAL or RECOVER-DONE.  The diagram
also (seems to) has (have) transitions for: RECOVER & RECOVER-WAIT to
COMMUNICATIONS-INTERRUPTED, POTENTIAL-CONFLICT to POTENTIAL-CONFLICT.

29) 8.8.1 Operation in NORMAL State
This note is for the NORMAL box in the diagram.  The box has (responsive)
which would generally indicate that both servers are responsive while
only the primary is.

30) 8.9 COMMUNICATIONS-INTERRUPTED State
Para 2 mentions starting a timer if configured for auto-partner down.
I assume this is the "Safe Period Timer" from the diagram?  If so
mentioning that name in the text would be clearer.  (Something like
"...then a timer (the Safe Period Timer from the Figure 5) is started...")

31) 9.3 Adding RRs to the DNS
There is a use case for doing DDNS updates on all lease renewals.
If there is a concern that the DNS might be damaged in some way
then having the DHCP server update it on each renew will tend
to heal it of breakages.

32) 9.4 Deleting RRs from the DNS
Is there any concern about the server making the resource FREE* crashing
after receiving the BNDACK but before doing the DDNS update?  I think
the end result of that would be the AAAA and PTR records still in the
DNS until the address is reused (for the PTR) and the client gets
a new address (for the AAAA) or possibly the server recovers and finishes
the cleanup.