Re: [pim] AD Review of draft-ietf-pim-hierarchicaljoinattr-05

[Stig Venaas <[e-mail]>]

Hi

It's about time I try to pick this up.

On Thu, Jan 21, 2016 at 10:00 AM, Alvaro Retana (aretana)
<[e-mail]> wrote:
> Hi!
>
> I just finished reading this document.  I think that the description of the
> general idea and of the operation when attributes are in fact included at
> different levels should be made a lot clearer than what it is now.  The
> document only presents an idea of how to include attributes at different
> levels, but not what to do with them once they're in.  I would have liked to
> see sections with more details about how to process attributes to be sent
> and/or received (similar to RFC5384 and 5496..).

The meaning and processing of attributes is basically the same as
before. The only new part is how the attributes are included in the
J/P. The last to paragraphs of section 3 try to explain what you do
with the attributes etc.

   The complete set of attributes that apply to a given source is
   obtained by combining the message wide attributes, the attributes of
   the group set that the source belongs to, and the source specific
   attributes.  However, if the same attribute is specified at multiple
   levels, then the one at the most specific level overrides the other
   instances of the attribute.

   Note that Join/Prune attributes are still applied to sources as
   specified in [RFC5384].  This document does not change the meaning of
   any attributes, it is simply a more compact way of encoding an
   attribute when the same attribute and value applies to multiple
   sources.

Join attributes as currently defined are in the encoded source
address. For say a given (S,G) you would have a set of attributes that
apply to S. Now when you process, for this (S,G) the set of attributes
for the source is the union of the set of attributes that apply to the
entire message, the set of attributes for the group, and the
attributes of the source. Except that if the same attribute is in
multiple sets, the one from the more specific set is used. So group
attribute would override a message wide attribute. And a source
attribute override message and group wide attributes.

Are you looking for some kind of example here? Maybe I can do some
kind of pseudo code or an example algorithm.

Please see more comments inline.

> Clarifying the text, including the IANA Considerations, and updating the
> RFC4601 reference are all improvements that I would like to see in the
> document before starting the IETF Last Call.  Please see my detailed
> comments below.
>
> Thanks!
>
> Alvaro.
>
>
> Major:
>
> Replace the RFC4601 references for draft-ietf-pim-rfc4601bis.
> General Idea.  After reading the text a couple of times, I think I got it!
> :-)  If the new "Hierarchical Join/Prune Attribute Hello Option" is included
> in the PIM Hello, then it means that the PIM Attribute from RFC5384 can now
> also be encoded in the Upstream Neighbor Address and the Group Address.
> Right?  [Note that some of the comments below are really minor on their own,
> but add up to not clearly explaining what is intended, so including a
> succinct explanation would go a long way.]
>
> "This document provides a hierarchical way of encoding attributes and their
> values in a Join/Prune message…"  This document doesn't define a new
> encoding, the encoding from RFC5384 is still used.  This document does
> define a mechanism to include attributes in a hierarchical may…
> "This document extends this by specifying the same encoding type also for
> Encoded-Unicast and Encoded-Group formats."  "This" what?   This document

It would be better to write "This document extends the encoding
defined in RFC5384" I suppose.

> allows the use of type 1…so that Encoded-…may contain a sequence of
> attributes…  The last paragraph in the Introduction is somewhat redundant,
> but it does a much better job at explaining what is going on.

I can try to rewrite it a bit so there is less repetition.

> Section 4. (PIM Address Encoding Types)
>
> I'm not sure what the purpose of this section is (not to mention some of the
> speculative language: "it is possible", "one could have").  It seems to
> rehash information that is already in RFC5384 and draft-ietf-pim-rfc4601bis
> (BTW, it wouldn't hurt to put a reference related to type 0).  The only
> conclusion seems to be the renaming of the registry.  Is that the purpose,
> to justify the renaming of the registry?

It is not just a renaming. The problem is that there is nothing in
4601 or 5384 saying that the meaning of the encoding types for
unicast, group and source encodings are the same. 4601 just had type 0
(native encoding) which applied to all. 5384 created a registry for
source encoding, because that was what was needed for join attributes.
But it makes sense to have a single encoding type registry for all of
these. This section tries to justify that.

This sentence is where I try to justify that:

   While it
   is possible to have the same encoding type value indicate different
   encodings depending on whether it is a Unicast, Group or Source
   address, it is simpler to have the same encoding type value indicate
   the same encoding independent of where it is used.

Perhaps it makes sense to remove the justification. I definitely don't
want to give people the idea that encoding types are defined
differently for each of these address encodings.


> Section 3. (Hierarchical Join/Prune Attribute Definition) "In this document
> we make use of this to allow Join/Prune attributes in each of these
> addresses, using the encoding in Section 4."  What is "this"?  I don't see
> any encoding defined in Section 4 (or anywhere else).

Perhaps it could say "In this document we make use of these encoded
address fields to allow..."

> Operation when multiple attributes are present in the hierarchy.

The next to last paragraph of section 3 tries to explain this.

> The hierarchy seems straight forward: attributes in the Upstream Neighbor
> Address take precedence over ones in the Group Address (maybe use "Multicast
> Group Address n" to be consistent with the packet), which then take
> precedence over the source ones (Encoded-Source Address).  That is fine, but

It's the other way around. Say one for the source overrides the same
in the group. This means that if say all sources should have the same
attributes except one, then for the one source you would specify the
attribute unique to that source.

> this sentence in the last paragraph seems to imply that the attributes in
> the source are always applied (regardless of the hierarchy): "Note that
> Join/Prune attributes are still applied to sources as specified in
> [RFC5384]."
> I'm assuming that the rules specified in RFC5384 for the Encoded-Source
> Address apply to the Upstream and Group addresses in this document.  For
> example, RFC5384 says that a "type 1 Encoded-Source Address MUST contain at
> least one Join Attribute.  The way to specify that there are no Join
> Attributes for a particular tree is to use the type 0 Encoded-Source
> Address."  Please explicitly indicate whether the procedures in RFC5384
> apply here or not...or if some do and others don't…or if some of the rules
> change.

The rules regarding type 1 and type 0 above are exactly the same as in
5384, no change. Attributes apply to sources as before, the semantics
of the attributes are not changing. When you process the message you
process each of the sources separately, applying the attributes for
that source. It's just that instead of requiring all the attributes
for the source being encoded in the source address, you can put some
of the attributes for the source in the upstream neighbor field or
group field. The goal is simply that if you have an attribute that
applies to all the sources, you don't have to encode it for every
single source.

> Should the behavior for higher level attributes affect the lower levels?
> One of the reasons I'm asking is because RFC5496 (The RPF Vector TLV) says
> in Section 3.3.2. (Processing a Received Vector Attribute) that "a received
> PIM Join that contains a Vector Attribute, a router MUST first check to see
> if the Vector IP address is one of its own IP addresses.  If so, the Vector
> Attribute is discarded, and not passed further upstream."  If this attribute
> is included so that it affects all sources, should all other Vector
> Attributes be discarded from the lower levels?

The way I think it should be done is that you first build the set of
attributes that apply to the source (which means taking the union of
the 3 sets). Once you have the set you process the attributes exactly
like you do today. This means that say RPF vector is in the upstream
neighbor field, you will walk through all the sources. For each source
you process the set. For each source you would do the validation and
discard the attribute.

Let me try to explain something here by example. Assume you have RPF
vector TLV A in the upstream neighbor field and then for a source you
have RPF vector TLV B. For that source you B with override A according
to the rules specified. Then after you have built the set you do the
validation and you might find that B should be discarded per 5496
3.3.2. This does not mean that TLV A should be used. I think it is
crucial that you first construct the set of attributes that apply to
the source, and then you process the set exactly as if the full set
had been included in the encoded source address.

>
> Disclaimer: I haven't looked at the other attribute RFCs, but I think that
> other similar issues may come up.

That is why I try to keep the semantics. That it is just a more
efficient way of storing the attributes in a J/P message. The
attributes still apply to the sources exactly as in 5384 and the
individual attribute RFCs.

> Related question:  are there cases where it doesn't make sense to include a
> specific attribute at a specific level?  What happens then?

A version of the draft had a mechanism for that, but it makes the
processing fairly complex and I'm not convinced there is a need. In
such a case you can always fall back to specifying the attribute for
each of the sources that need it.

> IANA Considerations
>
> The Introduction says that this "document defines a new IANA registry for
> PIM encoding types", but the IANA Considerations Section seems to just be
> renaming the existing registry.  So it is not a new registry, just a new
> name.
> "the more correct name Join/Prune attributes"  Shouldn't this result in
> renaming the "PIM Join Attribute Types" registry too?

In a way it is a different registry, or at least the scope is wider.
But I agree it is better to say we rename it.

> Security Considerations.  Can including an attribute at a specific level
> (maybe where it doesn't make sense — think not just of current attributes)
> cause an unintended consequence?  Knowing that the attributes can be changed
> (from RFC5384), what are the risks?  You should at least point to the
> security considerations in RFC5384, even though that doesn't say much.

Since the semantics are the same, it is just a more efficient encoding
scheme, I do not see how there can be any new security concerns. I can
try to say that and refer to 5384.

>
> Minor:
>
> The Abstract should have a sentence about the update to RFC5384.  Something
> simple: "This document updates RFC5384 by…"  Also, similar text should also
> appear in the Introduction (with maybe some more details).  The 3rd
> paragraph in the Introduction tries, but it is not crisp enough (see above).

OK.

> I think that the update is just related to the registries.

That is true. I'll say that.

> The hierarchical construct is just an extension, not an update.
>
> Section 1. (Introduction):  Please add a reference to
> draft-ietf-pim-rfc4601bis when mentioning "Encoded-Unicast and
> Encoded-Group".
> Hello Option.  The use of this term is not consistent.  Sometimes it is
> referred to as just "Hello Option", but the full name seems to be
> "Hierarchical Join/Prune Attribute Hello Option".  This is specially
> important in Section 7. (IANA Considerations), where the registry should be
> specifically called out too.

Agree.

> Nits:
>
> Section 3 uses "we" several times.  3rd person would be better.
> I'm a little surprised that only one RFC2119 keyword is used.  I'm not
> suggesting that you add more, as the text can be made clear without them.

OK, I'll change the language a bit and think about whether 2119
keywords may be appropriate more places.

Stig

> Just an observation..
> I am also surprised at the minimal changes between
> draft-venaas-pim-hierarchicaljoinattr-00 and the current version [1] and how
> little discussion happened on the list.  Again, just an observation.
>
> [1]
> https://www.ietf.org/rfcdiff?url1=draft-venaas-pim-hierarchicaljoinattr-00&url2=draft-ietf-pim-hierarchicaljoinattr-05