Subject: [lisp] RtgDir review: draft-ietf-lisp-lcaf-14.txt

From: Stig Venaas <[email-masked]>
Date: Wed, 7 Sep 2016 13:41:14 -0700

Hello,

I have been selected as the Routing Directorate reviewer for this
draft. The Routing Directorate seeks to review all routing or
routing-related drafts as they pass through IETF last call and IESG
review, and sometimes on special request. The purpose of the review is
to provide assistance to the Routing ADs. For more information about
the Routing Directorate, please see
http://trac.tools.ietf.org/area/rtg/trac/wiki/RtgDir

Although these comments are primarily for the use of the Routing ADs,
it would be helpful if you could consider them along with any other
IETF Last Call comments that you receive, and strive to resolve them
through discussion or by updating the draft.

Document: draft-ietf-lisp-lcaf-14.txt
Reviewer: Stig Venaas
Review Date: 2016-09-07
IETF LC End Date:
Intended Status: Experimental

Summary:
I have some minor concerns about this document that I think should be
resolved before publication.

The draft is almost ready but there are several places where the text
is a bit unclear,
and where there could be potential interoperability issues if people
get it wrong. Here
are the issues I found in the order they appear in the document. They
are all mostly
minor issues, but a few of them are just nits.

In section 1 I find this sentence a bit misleading since [AFI] doesn't
talk about the formatting.

   Currently defined AFIs include IPv4 and IPv6 addresses, which are
   formatted according to code-points assigned in [AFI] as follows:

Is the formatting shown here for AFI 1 and 2 defined in another
document? It would be good to have a reference. If it is introduced
in this document, then it should not be in the introduction.

In section 2, first paragraph it says:
   There is an address family currently defined for IPv4 or IPv6
   addresses.

It would be better to say that address families are defined for IPv4
and IPv6 addresses.

In section 3 we have this paragraph:

   The Address Family AFI definitions from [AFI] only allocate code-
   points for the AFI value itself.  The length of the address or entity
   that follows is not defined and is implied based on conventional
   experience.  Where the LISP protocol uses LISP Canonical Addresses
   specifically, the address length definitions will be in this
   specification and take precedent over any other specification.

I'm not sure what conventional experience means. Please try to find a
better way to say it. Regarding the last sentence, what if a you want
to define new LCAF encodings in the future? Is it good to say that this
specification takes precedent over any other?

In section 3 we have this paragraph:

   Length:  this 16-bit field is in units of bytes and covers all of the
      LISP Canonical Address payload, starting and including the byte
      after the Length field.  So any LCAF encoded address will have a
      minimum length of 8 bytes when the Length field is 0.  The 8 bytes
      include the AFI, Flags, Type, Reserved, and Length fields.  When
      the AFI is not next to encoded address in a control message, then
      the encoded address will have a minimum length of 6 bytes when the
      Length field is 0.  The 6 bytes include the Flags, Type, Reserved,
      and Length fields.

Can you phrase this differently? First it says that any LCAF encoded
address has a minimum length of 8, but then it goes on to say how it
sometimes is only 6. I understand what you're trying to say, but there
may be a better way of stating it.

In section 3 it says:

   Rsvd2:  this 8-bit field is reserved for future use and MUST be
      transmitted as 0 and ignored on receipt.

Some of the LCAFs specified in this document are using it though. Hence
you may need to change this text, and maybe not make it reserved.

The last sentence of section 3 is:

   Therefore, when a locator-set has a mix of AFI records and
   LCAF records, all LCAF records will appear after all the AFI records.

This is not necessarily true. Isn't it possible that one at some point
in the future might use other AFI records that have AFI > 16387? IANA
has assigned several values beyond 16387.

In 4.1 it says:
   AFI = x:  x can be any AFI value from [AFI].

Is 16387 always or sometimes allowed? Might be good to clarify that.
This also applies
to all the other LCAF types with similar language.

Section 4.4:

   RTR RLOC Address:  this is an encapsulation address used by an ITR or
      PITR which resides behind a NAT device.  This address is known to
      have state in a NAT device so packets can flow from it to the LISP
      ETR behind the NAT.  There can be one or more NAT Tunnel Router
      (NTR) [I-D.ermagan-lisp-nat-traversal] addresses supplied in these
      set of fields.  The number of NTRs encoded is determined by the
      LCAF length field.  When there are no NTRs supplied, the NTR
      fields can be omitted and reflected by the LCAF length field or an
      AFI of 0 can be used to indicate zero NTRs encoded.

It is not quite clear to me if the NTR fields here are the RTR RLOC
addresses. Does no NTR fields mean that there are no RTR RLOC addresses?
If AFI 0 is used, would there be exactly 1 RTR RLOC address (of length
0), and that would have AFI 0?

Section 4.5 first paragraph:

   Multicast group information can be published in the mapping database.
   So a lookup on an group address EID can return a replication list of
   RLOC group addresses or RLOC unicast addresses.

Can you have a mix of group addresses and unicast addresses? It's also
not so clear from the format what source prefixes are. Are the source
prefixes the same as the unicast RLOCs mentioned above? Can you have
both multiple source addresses and then multiple group addresses? Can
they be in arbitrarty order, or all sources followed by all groups?
It seems one will need to inspect the address itself to tell whether it
is a unicast or multicast address. This is probably fine.

Section 4.6
Add description of Rsvd3.

Section 4.7
Add description of Rsvd3 and Rsvd4.

Section 4.10
In this section there are several examples of using the AFI List Type,
but I miss a general definition. It appears that there can be a variable
number of AFIs in the list. Any number > 0? It might be useful to have
a length field associated with each AFI, or is it OK that parsing fails if
an AFI is unknown, so that the address length is not known?

Section 4.10.3
Isn't it unusual to include the 0 termination? Since you know the
length it is not really needed. When parsing one will need to check
the length either way, what should one do it the string accidentally
is not 0-terminated?

Section 4.10.4
I'm assuming that the recursion is more generic than this example?
It might be worth trying to explain the more generic concept and
format, and make sure that this is described as just an example.

Section 4.10.5
This also appears to be just an example.

Section 5.2
   Key Field Num:  the number of fields (minus 1) the key can be broken
      up into.  The width of the fields are fixed length.  So for a key
      size of 8 bytes, with a Key Field Num of 4 allows 4 fields of 2
      bytes in length.  Allowing for a reasonable number of 16 field
      separators, valid values range from 0 to 15.

It says minus 1. Doesn't that mean that a Key Field Num of 4 indicates
5 fields?

   Key Wildcard Fields:  describes which fields in the key are not used
      as part of the key lookup.  This wildcard encoding is a bitfield.
      Each bit is a don't-care bit for a corresponding field in the key.
      Bit 0 (the low-order bit) in this bitfield corresponds the first
      field, right-justified in the key, bit 1 the second field, and so
      on.  When a bit is set in the bitfield it is a don't-care bit and

What does right-justified mean? Does it mean that the first field is the
"low order" one? Basically at the end of the message? And the highest
order bit corresponds to the part of the key right after the wilcards?
I think this need to be explained better to ensure interoperability.

   Key:  the variable length key used to do a LISP Database Mapping
      lookup.  The length of the key is the value n (shown above) minus
      3.

Isn't it exactly the value n, since the length is n + 3?

Section 5.4
   Length value n:  length in bytes of fields that follow.
This is a bit confusing. I think 2+n is the length in bytes that follow
the lenght field. While n is the number of bytes after the JSON length.

Section 5.5

It says "AFI = x" twice in the diagram. Based on this and the way it
is described it might seem that the two AFI values must be the same
value x, but they can differ, right? I this applies to several other
messages types as well.

Section 7
It looks like the table in the IANA considerations doesn't include all
the types defined in this document.

I'm happy to discuss my comments if needed, and also review any
updates to this draft.

Stig

---

From: "Joel M. Halpern" <[email-masked]>
Date: Wed, 7 Sep 2016 17:14:22 -0400

Thanks Stig.  This looks very useful.

Authors, can you please respond?
Yours,
Joel

---

From: Dino Farinacci <[email-masked]>
Date: Wed, 7 Sep 2016 14:31:44 -0700

I will try to respond tomorrow. Thanks for your comments Stig.

Dino

> On Sep 7, 2016, at 2:14 PM, Joel M. Halpern <[email-masked]> wrote:
>

---

From: Dino Farinacci <[email-masked]>
Date: Thu, 8 Sep 2016 14:02:54 -0700

> Intended Status: Experimental

Thanks Stig for your comments. See responses below. As well as a new version -15 not submitted yet but for your quick review. Also find a lcaf-rfcdiff.html file for easy diff viewing.

> The draft is almost ready but there are several places where the text

No problem. This document has had a long history and many opinions related to it has come and gone but we tried to reflect everyone’s point of view.

> in this document, then it should not be in the introduction.

No it is not shown in any document. All the AFI document says is that a 16-bit AFI value of 1 means an IP address follows. That means 4 bytes of address. And 16 for IPv6 when AFI is 2.

As you can see the reference to the AFI document is at the end of the sentence.

> It would be better to say that address families are defined for IPv4

Changed.

> I'm not sure what conventional experience means. Please try to find a

Well like I said above, the AFI values defined in the AFI document are just type values and there is no length defined. So “conventional wisdom” means that if a spec says an AFI value 1 is IPv4, we know what follows is 4 bytes. Ditto for IPv6, MAC addresses, etc.

> specification takes precedent over any other?

LCAF encodings and definitions do not change the length of an AFI encoded address. So I am not sure what you mean.

> Can you phrase this differently? First it says that any LCAF encoded

No not really. It is precise up to the sentence “When the AFI is not ..”. 

> may be a better way of stating it.

This special case is for some LISP control-messages where the AFI is another place in the message but the address is somewhere else. Rather than have the AFI appear twice, the LCAF length needs to be different.

The length field always includes the entire contiguous set of LCAF bytes including type, flags, length field, etc.

The language is precise and accurate. Let me know how you think stating what I did in this comment response can be put in without writing a lot of text about a special case.

> you may need to change this text, and maybe not make it reserved.

I change to say the field is LCAF Type dependent and check the LCAF Type definition to see what bits of this field is not reserved.

> has assigned several values beyond 16387.

I will change to order must be in AFI value order.

> to all the other LCAF types with similar language.

Always. And since 16387 is in [AFI] and the sentence above says “can be any AFI value from [AFI]” that implies it can be an LCAF AFI. If I said “including the LCAF AFI, I would have to make this change in a dozen places and would be repetitive.

> 0), and that would have AFI 0?

Good catch. NTR was an old term and the NAT-traversal draft now uses the term RTR. I will fix.

> Can you have a mix of group addresses and unicast addresses? It's also

Yes, it just depends what address you put following the AFI value.

> not so clear from the format what source prefixes are. Are the source

I will state that when an (S,G) is encoded that is what the source address field is used for.

> they be in arbitrarty order, or all sources followed by all groups?

As shown it is not a list. And if the AFI=0 precedes the Source/Subnet Address field it means there is no source address encoded.

> is a unicast or multicast address. This is probably fine.

Right.

> Add description of Rsvd3 and Rsvd4.

Changed.

> number of AFIs in the list. Any number > 0? It might be useful to have

Yes, a variable number. 

> an AFI is unknown, so that the address length is not known?

Well each AFI has an implicit length per address entry and the LCAF Type length has the total length. So why would we need to have yet another length and complicate parsing even more? Please be aware (and I’m sure you are being a senior coder), that the more fields you add to parse, the more cross-checking of field semantics and lengths have to be done. This really complicates the code and if part of the LCAF Type is parseable and others are not, then you have a question about what you use and what you discard.

> the length either way, what should one do it the string accidentally

Well the AFI definition in [AFI] doesn’t say how to terminate the string. But the length field will imply it. However, I wrote the “distinguished-name” draft to specify when AFI=17 is used (not only in a LCAF but by itself), how to terminate the string. I could include that reference, but that draft is not a working group document.

So please advise.

> I'm assuming that the recursion is more generic than this example?

Yes.

> format, and make sure that this is described as just an example.

I think that can raise too many combinations. It will be hard to explain why someone will want to recurse a lot unless we have a well defined use-case. The fact that an LCAF has an AFI value. It means that an LCAF can be encoded wherever an AFI value is allowed.

This section shows a practical example and not a general one that no one would know how to use.

> This also appears to be just an example.

But a useful one that is already implemented in cisco code.

> It says minus 1. Doesn't that mean that a Key Field Num of 4 indicates

I fixed the description to be more clear.

> "low order" one? Basically at the end of the message? And the highest

Yes, I will make that more clear.

> Isn't it exactly the value n, since the length is n + 3?

Yes, you are right. Fixed.

> the lenght field. While n is the number of bytes after the JSON length.

The 2 is for the JSON length field, since it is a fixed field. The “n” is for the remaining variable length fields which include the JSON-binary-text and the AFI address. I’ll make the Length description reflect this. Thanks.

> value x, but they can differ, right? I this applies to several other

I’ll change x to y in the second occurence and a description for each.

> the types defined in this document.

That was done intentionally. The ones that are experimental are not in this section 7 list because there is no use-case document for it yet. Maybe the chairs can explain this better than me.

> I'm happy to discuss my comments if needed, and also review any

Let me know about the response where I didn’t make any changes.

Thanks again,
Dino/Dave/Job

---

From: Stig Venaas <[email-masked]>
Date: Fri, 9 Sep 2016 16:15:14 -0700

Hi

> Thanks Stig for your comments. See responses below. As well as a new version -15 not submitted yet but for your quick review. Also find a lcaf-rfcdiff.html file for easy diff viewing.

Thanks. I have some comments inline.

> Well like I said above, the AFI values defined in the AFI document are just type values and there is no length defined. So “conventional wisdom” means that if a spec says an AFI value 1 is IPv4, we…

In theory there should be standards/documents specifying this for each
of the address types, and maybe could write that people should see the
respective standards or something. I don't know if this exists for all
the types though.

> LCAF encodings and definitions do not change the length of an AFI encoded address. So I am not sure what you mean.

The last sentence says "Where the LISP protocol uses LISP Canonical
Addresses specifically, the address length definitions will be in this
specification and take precedent over any other specification.". What
if you in the future would like to write a separate specification that
defines additional LISP Canonical address formats?

> The language is precise and accurate. Let me know how you think stating what I did in this comment response can be put in without writing a lot of text about a special case.

The problem I see is that first you write "So any LCAF encoded address
will have a minimum length of 8 bytes when the Length field is 0." and
then you write "then the encoded address will have a minimum length of
6 bytes when the Length field is 0." I understand what you mean, but I
feel this is a bit confusing.

Maybe you could say something like:

When including the AFI, an LCAF encoded address will have a minimum
length of 8 bytes when the Length field is 0. Or start by saying that
the minimal length is 6, and then say that it will then be 8 when the
AFI is included.

> Always. And since 16387 is in [AFI] and the sentence above says “can be any AFI value from [AFI]” that implies it can be an LCAF AFI. If I said “including the LCAF AFI, I would have to make this ch…

I'll leave this to you, but one option could be to just mention it
here, and just say that it is true for other types as well. Or mention
somewhere before this in the draft that this is allowed. But you
already say "any AFI", so that should mean that this is allowed.

>> number of AFIs in the list. Any number > 0? It might be useful to have

How about adding a few lines of text to 4.10 saying that you can have
a variable number etc., explaining briefly what the general format is.
And then say that what follows are examples?

> Well each AFI has an implicit length per address entry and the LCAF Type length has the total length. So why would we need to have yet another length and complicate parsing even more? Please be awa…

I guess I agree with you. Only issue I see is if you at some point
need an implementation to be able to parse past an unkown AFI,
basically not knowing the expected address length of the AFI.

> Well the AFI definition in [AFI] doesn’t say how to terminate the string. But the length field will imply it. However, I wrote the “distinguished-name” draft to specify when AFI=17 is used (not onl…

Currently it says:

   Length value n:  length in bytes AFI=17 field and the null-terminated
      ASCII string (the last byte of 0 is included).

It might make sense to 0 terminate the DN in some cases, but at least
here we know the string length from the value of n, so I think it
would be better to drop it here. And as I mentioned, you cannot rely
on the 0 for parsing, to be on the safe side you would use n anyway.

> That was done intentionally. The ones that are experimental are not in this section 7 list because there is no use-case document for it yet. Maybe the chairs can explain this better than me.

I think it's still useful. If someone sees the type used, they can
look it up in the registry. It also helps avoid that someone perhaps
tries to reuse one of these types in new documents. If you really want
to use one of the code points for something else in the future, a new
document could always update the registry.

BTW, it also makes me wonder if it is worth reserving any LCAF types
for experiments.

Regards,
Stig

---

From: Dino Farinacci <[email-masked]>
Date: Mon, 12 Sep 2016 13:01:48 +0100

Stig, I'm traveling this week. I'll get you a reply over this coming weekend. 

Dino

> On Sep 10, 2016, at 12:15 AM, Stig Venaas <[email-masked]> wrote:
> 
> Hi
>

---

From: Luigi Iannone <[email-masked]>
Date: Tue, 13 Sep 2016 12:08:26 +0200

Hi Stig,

thanks for the review.

Just a quick comment on one of the last point you raised:

> On 08 Sep 2016, at 23:02, Dino Farinacci <[email-masked]> wrote:
> 

[snip]

> That was done intentionally. The ones that are experimental are not in this section 7 list because there is no use-case document for it yet. Maybe the chairs can explain this better than me.

AFAIR the rationale was that since these types are experimental proposals, with no (or at least not yet) detailed use-case, there is no need to allocate these values now.
If new documents will use them, than those documents will require allocation in their IANA section.

ciao

L.

> <draft-ietf-lisp-lcaf-15.txt>
> 
> <lcaf-rfcdiff.html>

---

From: Dino Farinacci <[email-masked]>
Date: Mon, 19 Sep 2016 08:54:55 -0700

> On Sep 9, 2016, at 4:15 PM, Stig Venaas <[email-masked]> wrote:
> 

It does not. But how about I promise you that when there is a new use-case for a specific AFI, we’ll make sure that the AFI and its length are defined in that use-case document.

Like I said, we have this for AFI=17 already. So I have already set an example.

> defines additional LISP Canonical address formats?

Okay, I have clarified that new LCAFs that are defined in other documents will specify length and formatting definitions.

> the minimal length is 6, and then say that it will then be 8 when the

Changed to add your suggestion. Thanks.

> And then say that what follows are examples?

Done.

> on the 0 for parsing, to be on the safe side you would use n anyway.

You want to terminate the string in all cases. Because there are cases where AFI=17 is not used inside an LCAF encoding. And where there is no length to convey the string length. And the Distinguished-Name use-case Internet Draft specs this for both LCAF and non-LCAF applications.

> document could always update the registry.

Did Luigi’s response satisfy your comment?

> BTW, it also makes me wonder if it is worth reserving any LCAF types

The space is large enough for FCFS. 

> Stig

See new version enclosed. Let me know when I can post it if you like the changes.

Thanks again,
Dino

---

From: Stig Venaas <[email-masked]>
Date: Mon, 19 Sep 2016 16:07:44 -0700

Hi

The changes look good.

Stig

---

From: Dino Farinacci <[email-masked]>
Date: Mon, 19 Sep 2016 17:06:43 -0700

-15 posted. Thanks a lot Stig!

Dino

> On Sep 19, 2016, at 4:07 PM, Stig Venaas <[email-masked]> wrote:
> 
> Hi
> 
> The changes look good.
> 
> Stig
> 
>