Subject: Re: [Gen-art] Gen-ART review of draft-ietf-mpls-gach-adv-06

From: Martin Thomson <[email-masked]>
Date: Sun, 10 Mar 2013 13:52:51 -0400

I sent the following review a few weeks back.  I just wanted to make sure
that it didn't get accidentally stored in /dev/null.

On 15 February 2013 14:33, Martin Thomson <[email-masked]> wrote:

> I am the assigned Gen-ART reviewer for this draft. For background on
> Gen-ART, please see the FAQ at
>
> <http://wiki.tools.ietf.org/area/gen/trac/wiki/GenArtfaq>.
>
> Please resolve these comments along with any other Last Call comments
> you may receive.
>
> Document: draft-ietf-mpls-gach-adv-06
> Reviewer: Martin Thomson
> Review Date: 2013-02-15
> IETF LC End Date: 2013-02-27
> IESG Telechat date: (if known)
>
> Summary: The document is almost ready for publication as proposed
> standard.  There is a major issue that should be easy to resolve.
>
> Major issues:
>
> Section 6.3 duplicates the description of HMAC provided in RFC 2104.
> That is likely to cause a bug.
>
> If you reference RFC 2104, then the only requirement is a clear
> specification for what message is input to the HMAC.  Currently, this
> is buried.  It is unclear if the input includes the G-ACh header
> defined in RFC 5586 (it doesn't need to, but this needs to be made
> explicit).
>
> Filling the authentication part with 0x878FE1F3 seems unnecessary busy
> work, but it's harmless as long as the hash function produces a
> multiple of 32 bits of output.
>
> Minor issues:
>
> To avoid forward compatibility issues, reserved fields should come
> with guidance that says: "Implementations of this protocol version
> MUST set reserved fields to all zero bits when sending and ignore any
> value when receiving messages."
>
> In Section 4.4, how does the duration interact with the lifetime?
> What happens when the duration is longer than lifetime such that the
> TLV is expunged before the duration is up?
>
> Section 5.2 states:
>    [...] If one
>    of the received TLV objects has the same Type as a previously
>    received TLV then the data from the new object SHALL replace the data
>    associated with that Type unless the X specification dictates a
>    different behavior.
>
> This leads to different retention characteristics depending on some
> arbitrary application-specific requirements.  It also complicates
> implementations.  Is there a strong motivation for the "unless the X
> specification dictates a different behavior" part of this statement?
>
> If this behaviour is desirable, a note regarding what happens to the
> composed TLV when some of the values that contribute to it might
> expire might be necessary.
>
> Regarding the last paragraph of Section 6.3:
>           This also means that the use of hash functions with larger
>           output sizes will increase the size of the GAP message as
>           transmitted on the wire.
>
> If you want to prevent hash truncation, then use 'MUST'.  Personally,
> I see no reason to do so.  It's a good way to get smaller messages,
> with a corresponding reduction in the strength of the assurance
> provided.
>
> Nits:
>
> Section 3 could use some subheadings to aid navigation (and referencing).
>
> Section 3 describes the size of fields only through ASCII-art.  It's a
> fairly simple thing to add a bit count to the description of each
> field.  That includes the reserved fields, which have no descriptions.
>
> I like the text in the editors note on page 8.  Why is it not the
> actual text already?
>
> Sections 4.2 and 4.3 could probably use a note that notes that
> retention of these TLVs doesn't make any sense.  These could be sent
> with zero lifetime, except that if these are sent along with the
> Source Address TLV, that's not possible... unless you send multiple
> application data blocks for the same application.  Is that possible?
>

---

From: Jari Arkko <[email-masked]>
Date: Wed, 27 Mar 2013 09:22:43 +0200

Thank you for the review, Martin!

The issue with replication of RFC 2104 has been raised as a Discuss by other ADs. That will get resolved. But do the authors or chairs have responses for the other things in Martin's review?

Jari

On Mar 10, 2013, at 7:52 PM, Martin Thomson <[email-masked]> wrote:

> I sent the following review a few weeks back.  I just wanted to make sure that it didn't get accidentally stored in /dev/null.
> 
> 
> _______________________________________________
> Gen-art mailing list
> [email-masked]
> https://www.ietf.org/mailman/listinfo/gen-art

---

From: Stewart Bryant <[email-masked]>
Date: Wed, 27 Mar 2013 20:11:45 +0000

> The issue with replication of RFC 2104 has been raised as a Discuss by other ADs. That will get resolved. But do the authors or chairs have responses for the other things in Martin's review?
Hi Jari & Martin,

>> multiple of 32 bits of output.
This overlaps with the existing comments and we will fix.

>> value when receiving messages."
We will add text.

>> TLV is expunged before the duration is up?
Well that would normally be a silly thing to do, but we do not propose 
to stop
it lest it be something an application actually wants.

This would be functionally equivalent to issuing a short term note
that was only transiently valid, or issuing some sort of synchronization
message. I can't think why you would want to do it, but why would we
stop it?
>> specification dictates a different behavior" part of this statement?
Yes, on the one hand we do not wish to constrain the applications, and
on the other we trust the application developers and the IETF review
process (required for a code point) to stop silly behaviour.

>> with a corresponding reduction in the strength of the assurance
OK we will look at this when we deal with the security rewrite.
>> Section 3 could use some subheadings to aid navigation (and referencing).
We will look at that
>> field.  That includes the reserved fields, which have no descriptions.
We can do that.
>> I like the text in the editors note on page 8.  Why is it not the
That is a cut and paste error during AD review.
>> application data blocks for the same application.  Is that possible?
As I recall this is allowed.

Stewart

---

From: Martin Thomson <[email-masked]>
Date: Wed, 27 Mar 2013 14:41:01 -0700

Hi Stewart,

Looks like you have most of this in hand.  A few comments.

On 27 March 2013 13:11, Stewart Bryant <[email-masked]> wrote:
> Well that would normally be a silly thing to do, but we do not propose to
> stop

There was an implicit question here.  Namely:  Do you want to codify
anything regarding expectations on this?  Can I rely on an
attribute/action with a long duration after the retention interval has
expired?

> process (required for a code point) to stop silly behaviour.

I don't find that answer particularly satisfactory.  If I were to
build a generic implementation of this, I wouldn't want to allow for
this feature because it's a fairly large complication.  That's what
I'm trying to get to: what motivation can you provide to an
implementer to support this feature?  And in case I'm not being clear,
what would you want to put in the draft?

---

From: Stewart Bryant <[email-masked]>
Date: Thu, 28 Mar 2013 13:27:52 +0000

> attribute/action with a long duration after the retention interval has
That is surely an application issue.

The TX advertizes a parameter, and says how long it assumes it to be valid.
What the application does with it is up to the application.

> .
A case where data might not be replaced would be a case where it was logged.
For example I might report the temperature of my optical interface and
want a small history, or I might want a small history of alarm events.
Alternatively if the event rate was too fast for the RX process some
types might be fifoed.

Our point was to specify the normal behaviour, but permit an application
to request a variation.  Given that this is an advertisment protocol,
the normative behaviour is the TX side and the RX side is largely 
illustrative,
because what the RX does with any of this is optional, so I do not think 
there
is an issue here.

- Stewart