Re: [Anima] Mirja Kühlewind's Discuss on draft-ietf-anima-grasp-12: (with DISCUSS and COMMENT)

[Brian E Carpenter <[e-mail]>]

Responding to Mirja's comments (her DISCUSS points were
already discussed in other messages):
On 24/05/2017 03:19, Mirja Kühlewind wrote:

> ----------------------------------------------------------------------
> COMMENT:
> ----------------------------------------------------------------------
> 
> Other mostly editorial comments:
> - ASA needs to be spelled out in the intro.

It is ;-)

> - I would recommend to move section 2 and 3.3 into the appendix

Moving the requirements (s2): already asked the WG about this.

"High Level Design Choices" (s3.3): But this is a description
of the design. Are people confused by the word "choice" perhaps?
Just in case that's the issue, I will simply delete it and the
section will be "High Level Design".

> - section 3.5.4.2: "A neighbor with multiple interfaces will respond with
> a cached discovery response if any."
>    "cached response" is explained in the next section and not clear in
> this paragraph.

It's hard to give an overview without deferring some details. We'll try
to simplify the text.

> - section 3.5.4.3: "After a GRASP device successfully discovers a locator
> for a Discovery
>    Responder supporting a specific objective, it MUST cache this
>    information, including the interface index via which it was
>    discovered.  This cache record MAY be used for future negotiation or
>    synchronization, and the locator SHOULD be passed on when
> appropriate
>    as a Divert option to another Discovery Initiator."
>    Not sure why the first is a MUST and the later is a SHOULD. I guess a
> SHOULD for caching would be sufficient.

Fair enough, although not implementing a cache would be a bad idea IMHO.

> - section 3.8.6 "If a node receives a Request message for an objective
> for which no
>    ASA is currently listening, it MUST immediately close the relevant
>    socket to indicate this to the initiator."
>    How is that indicated? Should really be further clarified

That's covered by one of Martin's comments - a transport session
failure indicates a failed negotiation or synchronization.
[BTW I'm impressed that you & Martin picked this up. I discovered
it experimentally with the prototype and we have a couple of API
error codes for it, but I failed to think about adding it to
the text until I saw Martin's comments.]

> - Also section 3.8.6: "In case of a clash, it MUST discard the Request
> message, in
>    which case the initiator will detect a timeout."
>    Why don't you send an error message instead? How does the initiator
> know that is should retry (assuming there is a TCP connection underneath
> that provides reliable transport)?
First of all, this is an incredibly rare event, even given the birthday
paradox: it's a collision of two random 32 bit numbers. So it really
isn't worth any extra machinery.

Second, an ASA always needs to be ready to retry anything in case of
failure. That's kind of the First Law of Autonomics: never give up.

> - Also section 3.8.9: "If not, the initiator MUST abandon or restart the
> negotiation
>    procedure, to avoid an indefinite wait."
>   How does the initiator decide for abandoning or restarting instead?
> Needs clarification!

It's irrelevant here whether the initiator abandons or retries. All
this means is that when the timer pops, the negotiation has failed.
We will reword this accordingly.

> - Could be useful to include an optional reasoning field in the Invalid
> Message and make copying the received message up to the maximum message
> size of this message a SHOULD (section 3.8.12.).

By making the body ?any we certainly allow this. Also, ?any
could be a CBOR array like [reason, rcvd-message]

Personally I think we might leave this undefined until we have some
experience. This is another example where CBOR's flexibility is
a great asset.

> - Not sure I fully understand the purpose of the No Operation Message
> (section 3.8.13.). If you just want to open a socket for probing, you
> perform a TCP handshake and send a RST right after. No need for further
> application layer interactions. And should there also be an optional
> reasoning phrase?

I hope you can agree it's harmless. (Actually your laptop probably
received some of these in Chicago, because we were running GRASP
sessions on the IETF network quite often.) The reason I found it
essential was to discover the port number assigned by the O/S
to a multicast socket, which you can only get by sending a
multicast and then using getsockname(). It seemed more civilised
to use a defined no-op than to just send a null packet.

> - Not sure why the objectives flag is needed. I assume that unknown
> objectives are ignored anyway and if a objective is known the receiver
> should know if that objective is valid for the respective message type
> (section 3.10.2).

Discovery-only can be useful and the "dry run" flag is dynamic. Also
of course it is another hook for extensibility.

> - section 3.10.4: "An issue requiring particular attention is that GRASP
> itself is a stateless protocol."
>    It's not. It caches information and needs to remember previous
> messages sent to reply correctly.

Right, but it isn't transaction-safe (ACID) which I guess is what we meant.

> - section 5: "Generally speaking, no personal information is expected to
> be
>       involved in the signaling protocol, so there should be no direct
> impact on personal privacy."
>    I don't think this is true because the protocol is so generic that you
> cannot say anything about the services it is used for.

Well, we can say what we *expect*, but you're correct. Since we are
requiring crypto, we should be OK anyway. Will reword slightly.

> Please see also further comments from Martin's tsv-art review (Thanks
> again!)!

Thanks
   Brian