Re: review of draft-ietf-shim6-failure-detection-03.txt

[Jari Arkko <jari.arkko@piuha.net>]

Hi Iljitsch,

and thanks a lot for your very detailed comments! Further discussion
inline:

> Ok. First of all, have a look at my message from october 24th, some 
> comments are still valid.

Ok. Will follow-up.

> This draft works per-context exclusively. So if there are 2, 5 or 10 
> contexts between two hosts, this means 2, 5 or 10 times the amount of 
> work is done.
>
> I think it makes sense to see whether the mechanism can be made to be 
> more general, so that it can be used for other applications as well, 
> ranging from MIP (as indicated in the applicability draft) to tunnel 
> maintenance and maybe even peer-to-peer applications. However, it's 
> important that there is fate sharing between the reachability 
> protocol and the user protocol (shim in our case). I think this can 
> be solved by having the quick reachability verification stuff (= FBD) 
> encapsulated in the user protocol, but let the full path exploration 
> be a protocol of its own or live under ICMPv6 or some such. However, 
> this introduces some issues with knowing whether the same 
> correspondent lives at different addresses.

We have tried to keep the design general, but at the same time
the text in most recent versions talks only about Shim6. I'm not
sure I want to promise anything about the generality, or hold up
Shim6 because we have not figured out if the generality is there
or not.

One of the reason why I think generality is something that we
need to leave for future verification is that other contexts like
HIP have significantly more complex requirements than Shim6.
In particular, HIP needs to work with IPv4 and NATs.

I am uncertain if the ICMP mechanism for the path exploration
part is the best way forward. The entire REAP protocol could
certainly be encapsulated in any "user" protocol. But many
scenarios that I can see do not necessarily work well with ICMP,
particularly with one version of ICMP. In MOBIKE, for instance,
it would have been inappropriate to use anything else than the
regular NAT-T UDP at the bottom, because only that can show
whether actual MOBIKE/IKEv2/IPsec traffic will get through. And
what about HIP running over IPv4?

I am very sympathetic to making this general, and attempts trying
to use this for other purposes. Perhaps the ICMPv6 approach is
the right way forward. All I'm saying is that its less easy than it
may appear to be, and hence I do not want to hold up Shim6 work
by attempting to solve this issue now.

> In any event, I think just ignoring the possibility that there are 
> multiple contexts between two hosts isn't good enough. The issue of 
> avoiding "reachability probe storms" is somewhat related to this, and 
> also not addressed in the draft.

My recollection of the multiple contexts discussion is vague, but
note the issues that Marcelo pointed out. I agree that there's
a potential for more messages. But as a general rule, I'd like to
get a working, as simple as possible Shim6 mechanism out there.
Even if its not optimized for all situations. We can work on extensions
like fate sharing between contexts later, too.

Was there other issues related to probe storms? We do have
exponential back-off. I understand even with that, we're still
vulnerable to multiple packets from the same host, if there
are many contexts, and to multiple packets from a set of hosts,
if all hosts in the network experience the same probléms.

If there's some specific thing that we could do to address
these, I'd be happy to incorporate it to the draft.

> Another thing that's missing completely from this draft is a 
> discussion of how to use address pair preference information. This 
> makes it impossible to address traffic engineering needs.

This is important, but can be addressed separately.

> I believe section 3 (related work) is at best unnecessary and may 
> even confuse the reader.

I decided to take that section out entirely.

> The same is true of much of section 4  (definitions). Implementers
> know which addresses are usable and which  aren't, at least for the
> most part, so covering this in such detail  is superfluous. Things
> like "Both types of operational pairs could be  discovered and
> monitored through the following mechanisms:" have no  place in a
> protocol specification document, in my opinion. These need  to specify
> what IS, not what COULD be.

I think it is necessary to talk about our notion of usable addresses.

But you are right that the specification should be tighter than it
currently is. Take look at -05 when I post it, and see if you like the
result. I have basically required the support for getting information
to SHIM6 from the underlying IPv6 mechanisms, and left any other
possible optimizations (like L2 connectivity info) as optional.

> I suggest tightening the use of words like "operational", "work", 
> "reachable". They're mostly used interchangably in the draft.

Done.

> 4.5.  Miscellaneous
>
>    Addresses can become deprecated [3].  When other operational
>    addresses exist, nodes generally wish to move their communications
>    away from the deprecated addresses.
>
>    Similarly, IPv6 source address selection [6] may guide the selection
>    of a particular source address - destination address pair.
>
> This doesn't say what shim6 implementers should do. In my opinion: 
> keep using deprecated addresses as the ULID/primary locator as long 
> as possible, but prefer non-deprecated addresses when selecting 
> alternative locators.

Right. But I actually already deleted Section 4.5 and the discussion
of deprecated addresses. IPv6 specifications already call for use
of non-deprecated addresses for new communications, and disallow
the use of invalid addresses. So its not clear that we need to say more.

>
> "A "path" is defined as" should probably be in the "definitions" 
> section.

I rewrote most of the text that involved the use of
the word "path"; it was used incorrectly most of the
time.

> "when link layer" -> when a link layer

Fixed.

>
>    1.  The base timing unit for this mechanism is named Keepalive
>        Timeout.  Until a negotiation mechanism to negotiate different
>        values for this timer becomes available, the value (3 seconds)
>        specified in Section 6.5 SHOULD be used.
>
> Sending a keepalive every 3 seconds? That's way too often, IMO.

Note that this is only if you have one-way traffic. But do you have
a suggested other value for the timer? We need to receive
a couple at least during a Send Timeout interval (10s), and the entire
process needs to last less time than it takes for ULPs to give up.

>
>    2.  Whenever outgoing data packets are generated
>
> Data packets as opposed to what other types of packets?

I added a clarification. Basically, its all packets including both
ULP packets and SHIM6 control messages, but NOT keepalives
or probes.

>    4.  The reception of a REAP keepalive packet leads to stopping the
>        timer associated with the return traffic from the peer.
>
> So when we receive a keepalive from the other side, _we_ stop sending 
> keepalives? This may be the right thing to do, but it's not obvious 
> to me why. Some explanation would help.

Keepalives are only used if there's one-way communication. Since the
other side sends a keepalive, its not sending anything else at that
time. Hence we have no need for keepalives.

>
> What if an attacker generates spurious keepalives in order to get us 
> to stop sending keepalives on our side?

Hmm... this is a potential concern. The attacker would have
to send one message every 10 s, and he'd be able to prevent
the node from sending its own keepalives. But this wouldn't
result in the breakage of the communications, the nodes
would just move to exploring state and quickly confirm that
they have connectivity.

Even applying IPsec does not help here, because IPsec runs
on top of the shim layer.

I have written something about this in the security considerations
section. Let me know if you want to do something more.

>    6.  Send Timeout seconds (10 s; see Section 6.5) after the
>        transmission of a data packet with no return traffic on this
>        context, a full reachability exploration is started.  This
>        timeout period is larger than the Keepalive Timeout to
>        accommodate for lost keepalives and regular variations in round
>        trip times.
>
> The keepalives are sent at an interval of 3 seconds (or shorter, I 
> imagine that an implementation isn't going to keep an exact timer for 
> each context, any rounding must obviously be in the down direction) 
> and the timeout is 10 seconds. In these 10 seconds you'd normally 
> receive 3 keepalives, while 1 is enough to indicate that the other 
> side is still alive. The other 2 are only there in case of packet 
> loss. I think that's excessive. Starting the full reachability 
> exploration because of incidental packet loss isn't such a big deal 
> that it warrants sending three times as many packets as necessary. 
> (Note that this assumes the worst case of a unidirectional transport 
> protocol which isn't exactly the common case.)

The question is what the right number is. We want to avoid
entering exploration needlessly, so I'd rule 1 keepalive out.
We now have 3, are you arguing for 2? I'd be fine with that,
but I note that we don't have a lot of evidence to support
either view. We're going to have to revisit this after we get
the experience.

Left the draft now as-is. Others may comment on what we
should do here.

> I find it jarring that section 5.5 immediately jumps to examples, 
> before the protocol is fleshed out.

Yes - but on the other hand we need an overview before
the bits are laid out to view... I did not change this yet, but
if you have a specific suggestion, I'd be happy to change the
draft.

> Although the examples are  referred by using a number, they aren't
> actually numbered.

Fixed.

>
> Why would a keepalive need an id field?

So that a probe reception report can indicate
seeing a recent keepalive.

>
> In the first long example B times out once, but then doesn't send 
> more probes until it sees return traffic from A. Why does B stop 
> sending probes?

It just happens to have a retransmission policy that
places the next probing to a time beyond the reception
of the first succesful message from A. At that point it
knows enough to not continue its own process. I added
something about this to the draft.

> I believe that since the id of the last received probe is included, 
> the iseeyou flag is unnecessary.

But we also have the case where you report seeing data packets but
no probes.

But if you have ideas on how this could be simplified -- perhaps by
not thinking about the data packets during exploration -- those
would be welcome.

> Although copying back the last seen id seems to do the job, I can't 
> help but feel that it would be preferable to add timers to reach 
> round trip times and copy back more received ids and also sent ids. 
> This allows the receiver of a probe to determine which of the probes 
> that made it to the other side did so faster, so it can select the 
> address pair with the shortest round trip time.

Right. But all that can go into extensions. I'd like to have the
minimum necessary to get this spec done.

> Including sent ids along with the addresses the probe with that id 
> was sent to helps the receiver determine that some probes didn't make 
> it (yet). If a probe didn't work in one direction of an address pair, 
> it's reasonable to assume that it may also not work in the other 
> direction and try other pairs first.

True as well, but again perhaps material for future
optimizations.

The rest of the comments I have to defer beyond the submission
deadline, unfortunately. Need to ship the draft now. I will respond
to the e-mail later, however.

--Jari

> I believe the next example (the fifth, I think?) is flawed because A 
> doesn't send anything after it has sent a data packet, even though it 
> never receives a return packet. The same is true for the example 
> after that.

> When does address pair exploration conclude, both in the cases where 
> there is alternative reachability, and in the case when there appears 
> to be no reachability at all? The exponential backoff isn't described.
>
> The keepalive is a fairly long packet. I think just a shim header as 
> would be used for data packets but with no ULP following the shim 
> header would be sufficient.
>
> Requiring random numbers in packets that are sent rather frequently 
> is a bad idea, because it depletes the typically limited amount of 
> entropy that's available for strong random number generation rather 
> quickly and semi-random number generation may be somewhat expensive 
> (and not that good). And I don't see what good an id does in a 
> keepalive anyway... Also, there may be reasons to have non-random 
> numbers, such as ease of lookup.
>
> The description of the different messages repeats text that is the 
> same for different messages, which is very tedious and makes it 
> likely that people will miss the things that are actually different. 
> (That's why programmer's keyboards don't have ctrl-c/ctrl-v keys: 
> reuse code, don't copy it.)
>
> The use of options for mandatory fields is awkward.
>
> "   The node maintains also the Send and Keepalive timers."
>
> These timers were previously unnamed, it would be better to use their 
> names as soon as they're introduced.
>
> "   Upon the reception of a payload packet in the Operational state, the
>    node starts the Keepalive timer if it is not yet running, and stops
>    the Send timer if it was running.  If the node is in the Exploring
>    state it transitions to the ExploringOK state, sends a Probe message
>    with the I See You flag set to 1 (Yes), and starts the Send timer.
>    In the ExploringOK state the node stops the Send timer if it was
>    running, but does not do anything else."
>
> I don't have a good feeling about this... It's too hard to determine 
> what should be happening. Maybe it would be better rather than go 
> down the list of packets that are sent/received and describe the 
> behavior in each state, to take one state at a time and describe what 
> happens with packets in that state.
>
>    Upon a timeout on the Keepalive timer the node sends a Keepalive
>    message.  This can only happen in the Operational state.
>
> Why?
>
> Why are there different Exploring and ExploringOK states? In both 
> cases, the host needs to continue trying different addresses, it's 
> mostly/only the other side that needs to behave differently when 
> there is successful reachability from them to us but not (yet) from 
> us to them.
>
>    Garbage collection of SHIM6 contexts terminates contexts that are
>    either unused or have failed due to the inability of the exploration
>    process to find a working pair.
>
> How is the latter determined?
>
> This is an important issue. For instance, once that a shim context is 
> rehomed, will it ever return to using the primary locators?
>
>    In the PDF version of this specification, an informational drawing
>    illustrates the state machine.  Where the text and the drawing
>    differ, the text takes precedence.
>
> I'm not reviewing the PDF state machine here as the text is normative.
>
>    A tabular representation of the state machine is shown below.  Like
>    the drawing, this representation is only informational.
>
> Then I'm ignoring this too.
>
> But I would be happier if they'd be removed, because either they're 
> superfluous as they're not normative, or they're actually necessary 
> to understand the protocol, which is even worse because they're not 
> part of the normative text.
>
>