Re: [trill] RtgDir review of draft-ietf-trill-directory-assist-mechanisms-07.txt

["Joel M. Halpern" <[e-mail]>]

Thank you Donald.  Points of agreement elided, some responses to try to
clarify my observations.  I will note that from your comments about 3.1, 
I believe my concerns, now moved to 3.7, are larger, as I had assumed 
that the magic was in some other protocol, and you now say it is not 
defined there.

Yours,
Joel

On 4/15/16 11:23 AM, Donald Eastlake wrote:
> Hi Joel
>
> Thanks for your thorough review and comments. See below
>
> On Wed, Apr 13, 2016 at 4:47 PM, Joel M. Halpern <[e-mail]
>  <mailto:[e-mail]>>
>
> wrote:
>
...
>> Major Issues:
>
>> In the state machine transitions in section 2.3.3
>
>> for push servers, it appears that if the event indicating that the
>
>> server is being shut down occurs while the server is already Going
>
>> Stand-By or Uncompleting, the transitions indicate that this
>> "going
>
>> down" event will be lost.  A strict reading of this would seem to
>
>> mean that the "go Down" event would need to recur after the
>> timeout
>
>> condition.  This would seem to be best addressed by a new state
>
>> "Going-Down" whose timeout behavior is to move to down state.
>
> I understand your point but "going down" and the like are called
>
> "events or conditions" in this draft, not just events.
>
> The problem with adding a single "Going-Down" state is that
> transition
>
> to that state would lose the information as to whether or not the
> Push
>
> Directory had been advertising that it was pushing complete
>
> information or not. The reason to remember this is that you would
> want
>
> to behave a differently if the "going down" condition was revoked
>
> before it completed. This information could be preserved in a
> Boolean
>
> pseudo variable but the current style of state machine in this draft
>
> avoids such pseudo variables and encodes all of the relevant push
>
> directory's state into the state machine state. Thus, I can see
> three
>
> possible responses to your comment:
>
> 1) Change wording to emphasize that these "events or conditions" can
>
> be conditions that cause a state transition some substantial time
>
> after they become true.
>
> 2) Add two new states: (1) going down - was complete; (2) going down
> -
>
> was incomplete.
>
> 3) Change the style of state machine to admit pseudo variables which
>
> can be set and testing as part of the state machinery.
>
> Option 1 is just some minor wording changes but adopting either
>
> options 2 or 3 involves more extensive changes so I would prefer to
>
> avoid them.
>

 From what I have seen, trying to build a state machine with conditions 
rather than events is fraught with problems and tends to lead to errors 
in implementation.  It amounts to hiding pseudo-variables inside the 
states, but not describing them.

Thus, I would much prefer solution 2, but it is of course up to the WG.

...

>> Minor Issues:
>
>> In section 2.3.3 describing the state transitions for push
>
>> servers, there is an event (event 1) described as "the server was
>
>> Down but is now Up."  The state transition diagram describes this
>> as
>
>> being a valid event that does not change the servers state if the
>
>> server is in any state other than "Down." In one sense, this is
>
>> reasonable, saying that such an event is harmless.  I would
>> however
>
>> expect some sort of logging or administrative notification, as
>
>> something in the system is quite confused.
>
> Again, I see your point but it seems to me to be a matter of state
>
> machine style. Note that the "event" is described as a condition, so
>
> from that point of view, it is true anytime the state is other than
>
> Down. On the other hand, if you view it as strictly an event, you
> are
>
> left with the question of what to put at the intersection of a state
>
> and event in the table when it is impossible for that event to occur
>
> in that state. Some people note this with an "N/A" (not applicable)
>
> entry. In fact, previous TRILL state diagrams such as in RFC 7177
> use
>
> "N/A" so it would probably be simplest to change to that for
>
> consistency.

I think N/A would be good.

...
>> Text in section 3.2.2.1 on lifetimes and the information
>
>> maintenance in section 3.3 imply that the clients and servers must
>
>> maintain a connection. Presumably, this is required already by the
>
>> RBridge Channel protocol, and I understand that we should not
>> repeat
>
>> the entire protocol here.  It would seem to make readers life MUCH
>
>> simpler if the text noted that the RBridge Channel protocol
>> requires
>
>> that there be a maintained connection between the client and the
>
>> server, and that these mechanisms leverage the presence of that
>
>> connection.
>
> The basic RBridge Channel protocol [RFC7178] is a datagram protocol
>
> rather than a connection protocol. So there is no guaranteed
>
> continuity of connection between RBridges that have previously
>
> exchanged RBridge Channel messages. But connection would only be
> lost
>
> if the network partitions since RBridge Channel messages look like
>
> data packets to any transit RBridges and will get forwarded as long
> as
>
> there is any route. Network partition is immediately visible in the
>
> link state database to the RBridges at both ends of an RBridge
> Channel
>
> exchange.  Section 3.7 provides that if a Pull Directory is no
> longer
>
> reachable (i.e., RBridge Channel protocol packets would no longer
> get
>
> through), then all pull responses from that Pull Directory MUST be
>
> discarded since cache consistency update messages can't get through.
>
> Perhaps a reference to Section 3.7 should be added to Section 3.3.
>

I don't think a reference to 3.7 is sufficient, although it is helpful.
If the protocol is a datagram protocol, and if it is important to 
discard data from unreachable pull servers, then I think 3.7 NEEDS to 
say more than just ~if you happen to magically figure out you can't 
reach the server, discard data it has given you.~  From the rest of the 
text, this is an important and unspecified protocol mechanism.

...
In the flooding flag and behavior, (long text elided) I don't think 
there is anything wrong with the intended behavior.  It is just that the 
very brief description of the FL flag leads the reader to an incorrect 
expectation.  Yes, it gets sorted out, but that is not good.  What I 
would suggest is when the flag is defined (with whatever name you 
choose) note that "for the qtypes 2,3,and 4, the flag indicates that the 
server should flood its response."

...
>
> Thanks,
>
> Donald