Subject: [Idr] Review of draft-ietf-large-community-06

From: Mach Chen <[email-masked]>
Date: Tue, 8 Nov 2016 06:22:54 +0000

Hi, 

I have review the latest version(06) of this document. The draft is well written and is basically ready for publication. I have some minor comments as bellows:

1. 

Section 2, the first sentence:
"This document creates the Large BGP Communities attribute as an..."

Although RFC1997 also uses "creates", I'd suggest to use "defines" here, it also aligns with the convention of most of the RFCs.

2.
Section 2, the last sentence:
"A receiving speaker SHOULD silently remove duplicate Large BGP
   Communities from a BGP UPDATE message."

IMHO, whether remove the duplicate communities from a BGP UPDATE message is not significant, since the life of the message is over once the message processed. The key point is that the receiving speaker should ignore the duplicate Large BGP Communities. 

How about:

"A receiving speaker SHOULD silently ignore the duplicate Large BGP
   Communities within a Large BGP Communities attribute. "

3.

In Section 5 says:
   "Operators SHOULD NOT use these Global Administrator
   values."

Later, in Section 6:
"The Large BGP Communities Global Administrator field may contain any
   value, and a Large BGP Communities attribute MUST NOT be considered
   malformed if the Global Administrator field contains an unallocated,
   unassigned or reserved ASN or is set to one of the reserved Large BGP
   Community values defined in Section 5."

The "SHOULD NOT", "may contain any value" and "MUST NOT" seems little bit confusion and self-contradiction. Since the first sentence is not a specification to the Large BGP Communities attribute, but just a recommendation to the Operators.

How about changing it as:

"It is recommended that operators do not use these Global Administrator values." 

Or using some other similar text.

Hope this comments helps!

Best regards,
Mach

---

From: Job Snijders <[email-masked]>
Date: Tue, 8 Nov 2016 12:26:16 +0100

On Tue, Nov 08, 2016 at 06:22:54AM +0000, Mach Chen wrote:
> I have review the latest version(06) of this document. The draft is
> well written and is basically ready for publication. I have some minor
> comments as bellows:
> 
> 1. 
> 
> Although RFC1997 also uses "creates", I'd suggest to use "defines"
> here, it also aligns with the convention of most of the RFCs.

Good suggestion, accepted.

> IMHO, whether remove the duplicate communities from a BGP UPDATE
> message is not significant, since the life of the message is over once
> the message processed. The key point is that the receiving speaker
> should ignore the duplicate Large BGP Communities. 
> 
> How about:
> 

After the latest round of review from Geoff Huston and Jakob Heitz the
following text was produced.

The below text is queued to be in -07 of the draft, but -07 can't be
posted yet as we are in the 'cutoff' period before the IETF meeting in
Seoul.

"""
	Duplicate BGP Large Community values SHOULD NOT be transmitted. A
	receiving speaker SHOULD silently remove duplicate BGP Large
	Community values from a BGP Large Community attribute.
"""

I think the above text aligns with your suggestion, right?

> 3.
> 
> In Section 5 says:
>    "Operators SHOULD NOT use these Global Administrator values."
> 
> Later, in Section 6:
> The "SHOULD NOT", "may contain any value" and "MUST NOT" seems little
> bit confusion and self-contradiction. Since the first sentence is not
> a specification to the Large BGP Communities attribute, but just a
> recommendation to the Operators.
> 

I don't see the contradiction, its text that gradually narrows down into
specifics for specific audiences. The text in section 5 is for
operators, the text in section 6 is for implementors and bgp developers.
I don't think that removing the 2119 language will increase the clarity.

> Hope this comments helps!

Very much appreciated!

Kind regards,

Job

---

From: "Jakob Heitz (jheitz)" <[email-masked]>
Date: Tue, 8 Nov 2016 17:02:40 +0000

> On Nov 8, 2016, at 3:26 AM, Job Snijders <[email-masked]> wrote:
> 
>> the message processed.

That's not what happens. After a route is originated, it passes through many BGP speakers and typically ends up in most routing tables on earth. Each speaker that sees the route may or may not interpret all the communities and may or may not propagate the communities. However, any BGP speaker that receives a community attribute SHOULD remove duplicate communities before propagating the attribute. Also any BGP speaker is free to propagate communities in a different order than it received them.

What actually happens in two code bases that I have worked on: A receiving speaker will sort the received communities and remove duplicates. This happens even if the receiving speaker simply propagates the communities without interpretation. This applies to legacy communities and the same behavior is carried on in my large community code.

>> The key point is that the receiving speaker

Thanks,
Jakob.

---

From: Mach Chen <[email-masked]>
Date: Wed, 9 Nov 2016 02:29:18 +0000

Hi Jakob,

Thanks for your prompt response!

Please see my reply inline...

> -----Original Message-----
> From: Jakob Heitz (jheitz) [mailto:[email-masked]]
> Sent: Wednesday, November 09, 2016 1:03 AM
> To: Job Snijders
> Cc: Mach Chen; IETF IDR WG
> Subject: Re: [Idr] Review of draft-ietf-large-community-06
> 
> 
> >> message is not significant, since the life of the message is over
> >> once the message processed.
> 
> That's not what happens. After a route is originated, it passes through many
> BGP speakers and typically ends up in most routing tables on earth. Each
> speaker that sees the route may or may not interpret all the communities and
> may or may not propagate the communities. However, any BGP speaker that
> receives a community attribute SHOULD remove duplicate communities before
> propagating the attribute. Also any BGP speaker is free to propagate
> communities in a different order than it received them.

Yes, this is true and I agree.

> 
> What actually happens in two code bases that I have worked on: A receiving
> speaker will sort the received communities and remove duplicates. This

This also aligns with my worked codes. But the point is that it does not remove the duplicate values from the received message. The duplicate values should be ignored and dropped.
Normally, a BGP message will be terminated at the receiving speaker, when it needs to send messages to other speakers, the messages are actually new generated ones. Of cause, this is more about implementation details.

I am fine with the new text Job proposed in the upcoming version 07.

Thanks,
Mach

> happens even if the receiving speaker simply propagates the communities
> without interpretation. This applies to legacy communities and the same
> behavior is carried on in my large community code.
> 
> 
> >> The key point is that the receiving speaker should ignore the
> >> duplicate Large BGP Communities.
> >>
> >> How about:
> >>

---

From: Mach Chen <[email-masked]>
Date: Wed, 9 Nov 2016 02:42:51 +0000

Hi Job,

Please see replies inline...

> From: Job Snijders [mailto:[email-masked]]
> Sent: Tuesday, November 08, 2016 7:26 PM
> To: Mach Chen
> Cc: IETF IDR WG
> The below text is queued to be in -07 of the draft, but -07 can't be posted yet as
> we are in the 'cutoff' period before the IETF meeting in Seoul.
> 
> """

Yes, I am fine with the text.

> I don't see the contradiction, its text that gradually narrows down into specifics
> for specific audiences. The text in section 5 is for operators, the text in section
> 6 is for implementors and bgp developers.

Although I am not fully convinced, I am OK to move the document forward and let's leave this to the picky IESG review :-)

Best regards,
Mach