Hi
On Thu, Sep 8, 2016 at 2:02 PM, Dino Farinacci <[email protected]> wrote:
>> Document: draft-ietf-lisp-lcaf-14.txt
>> Reviewer: Stig Venaas
>> Review Date: 2016-09-07
>> IETF LC End Date:
>> 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.
Thanks. I have some comments inline.
>> 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,
>
> 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 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.
>
> 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.
>
>> 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.
>
> Changed.
>
>> 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
>
> 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.
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.
>> 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?
>
> 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?
>> 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
>
> No not really. It is precise up to the sentence “When the AFI is not ..”.
>
>> 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.
>
> 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.
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.
>> 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.
>
> 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.
>
>> 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.
>
> I will change to order must be in AFI value order.
>
>> 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.
>
> 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.
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.
>> 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?
>
> Good catch. NTR was an old term and the NAT-traversal draft now uses the term
> RTR. I will fix.
>
>> 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
>
> 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.
>
>> 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?
>
> 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.
>
>> It seems one will need to inspect the address itself to tell whether it
>> is a unicast or multicast address. This is probably fine.
>
> Right.
>
>> Section 4.6
>> Add description of Rsvd3.
>>
>> Section 4.7
>> Add description of Rsvd3 and Rsvd4.
>
> Changed.
>
>> 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
>
> Yes, a variable number.
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?
>> 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?
>
> 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.
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.
>> 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?
>
> 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.
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.
>> Section 4.10.4
>> I'm assuming that the recursion is more generic than this example?
>
> Yes.
>
>> It might be worth trying to explain the more generic concept and
>> 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.
>
>> Section 4.10.5
>> This also appears to be just an example.
>
> But a useful one that is already implemented in cisco code.
>
>> 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?
>
> I fixed the description to be more clear.
>
>> 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
>
> Yes, I will make that more clear.
>
>> 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?
>
> Yes, you are right. Fixed.
>
>> 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.
>
> 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.
>
>> 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.
>
> I’ll change x to y in the second occurence and a description for each.
>
>> Section 7
>> It looks like the table in the IANA considerations doesn't include all
>> 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 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
>> I'm happy to discuss my comments if needed, and also review any
>> updates to this draft.
>>
>> Stig
>
> Let me know about the response where I didn’t make any changes.
>
> Thanks again,
> Dino/Dave/Job
>
>
>
>
>
>
>
>
>
_______________________________________________
lisp mailing list
[email protected]
https://www.ietf.org/mailman/listinfo/lisp
