[bess] Genart early review of draft-ietf-bess-bgp-multicast-05

[Joel Halpern via Datatracker]

Reviewer: Joel Halpern
Review result: Not Ready

This is a requested genart early review of

draft-ietf-bess-bgp-multicast-05

Prepared on 23-Oct-2023

Summary: This draft is not ready, having a number of issues that need to be
addressed.

I presume this draft will be socialized with the PIM and IDR working groups
before completion (or maybe it already has been?)

Major:
While I appreciate the effort to give an overview of the operation before
providing the specification, the actual text is almost impossible to
follow.   At a guess, the authors have a coherent view of what happens, and
have then written down each "interesting" piece.   This does not give the
reader clarity. As a first step, before giving the overview, all the
terminology needs to be defined.  Including such things as the fact that
MCAST-TREE NLRI is a general holder, within which there are a number of
different NLRI (which is finally explained in section 2, after the reader
is thoroughly confused.)  All terms and abbreviations should be defined or,
when they are from other documents, expanded with a citation so the reader
knows where to look for the term.  (I did figure out what FHR and LHR were,
but the draft did not help me do so.) Secondly, the extensive use of
construction of the form ~this use of construct A is just like the use in
document B except...~ is very confusing.  The reader is left without a
coherent description of how this protocol works, exactly which pieces from
the other document must be followed, and exactly how the changes are to be
applied. Third, if the intention of the introductory material is to provide
a perspective on, but not duplicative specification of, the material in
section 2.2, then each overview should have forward references explaining
where the detailed procedures can be found.  If material in the overview is
intended to be the normative specification of the operation (as for example
when and which rotue communities are to be used, and apparently all of
section 1.2.1.1) then normative language is needed.  It is quite hard to
exgtract from these sections the required procedures.

I also note that ID-Nits ha a lot of complaints.  I will not repeat them,
but they need to be dealt with. (For example, the addresses used in various
examples are not example addresses.  They should be.)

Scaling needs to be better addressed.  While I understand the use of RT
constraints helps the avoidance of overloading the leaves of the tree, it
appears that any network using route reflectors is likely to have state
about every sender of every multicast group in every rotue reflector.  It
may be that this is acceptable.  But it should be called out explicitly.

Section 2.2.1 sates that source advertisement will be triggered only by sources
sending multicast traffic.  And will expire based on time.  Except that the
network has no idea what the suitable time interval is for a given multicast
group.  Some groups will have long inter-packet intervals, while some will be
short and some will be quite variable.  Also, some groups will have the
property that the senders will know who they are before sending, and receivers
may even want to join before the senders are active.  If the working group
wishes to exclude such use cases, then the document should be explicit about
what use cases it is covering.

Moderate:
Additional explanation is needed in section 1.2.1.2.  This section describes
how to set up a shared tree for an ANY-Source Multicast Group.  Unlike the
earlier discussion of advertising sources with a route target community to
restrict distribution, this section explicitly says that the sources sending to
the ASM Group are advertised in BGP without the restricting community.  I
presume there is some other assumed aspect that restrits the information so it
is only received by the Rendezvous points for the shared tree.  But I can not
see how this is achieved.  Please add explanation of why this approach does not
flood the whole domain with information it does not want or need.

The last paragraph (short) of section 1.2.1.2 gives a vague description of
certain cases.  Presuming it is described in more detail later, a forward
reference would be extremely helpful.

Section 1.2.1.3 has very confusing use of ingress and egress PE.  I would have
assumed ingress and egress were in terms of the direction of traffic flow (from
traffic sources to interested receivers).  But the usage in both paragraphs
seems to be exactly the opposite.

Section 1.2.3 refers to establishing information via an IGP ("IGP neighbor
discovery").  Except that the earlier descriptions indicate that the deployment
case here is for a pure BGP domain, with no IGP.  (Otherwise, there would need
to be extensive procedures as to how the multicasts traverse regions that use
the IGP instead of BGP.)

Section 1.2.4 discusses handling when 

[bess] Genart last call review of draft-ietf-bess-pbb-evpn-isid-cmacflush-07

[Joel Halpern via Datatracker]

Reviewer: Joel Halpern
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

.

Document: draft-ietf-bess-pbb-evpn-isid-cmacflush-07
Reviewer: Joel Halpern
Review Date: 2023-06-30
IETF LC End Date: 2023-07-11
IESG Telechat date: Not scheduled for a telechat

Summary: This document is ready for publication as a Proposed Standard.

reviewers note: My thanks to the authors / editors for a very helpful
introduction that made clear the problem to be solved.

Major issues: N/A

Minor issues:
I got a bit confused in section 3, where the text says:
All the other fields are set and used as defined in [RFC7623]. This document
will refer to this route as the BMAC/I-SID route, as opposed to the [RFC7623]
BMAC/0 route (BMAC route sent with Ethernet Tag ID = 0).
I got confused because when I went to RFC 7623, I could not find the string
BMAC/0, and while I tried searching for related terms, I could not find
what term is being distinguished.  The string BMAC does not occur in RFC
7623.  So this and later (e.g. 4.1) references to 7623 use of BMAC/0 is
confusing.

Nits/editorial comments: N/A


___
BESS mailing list
BESS@ietf.org
https://www.ietf.org/mailman/listinfo/bess

[bess] Genart last call review of draft-ietf-bess-evpn-lsp-ping-08

[Joel Halpern via Datatracker]

Reviewer: Joel Halpern
Review result: Ready with Nits

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

.

Document: draft-ietf-bess-evpn-lsp-ping-08
Reviewer: Joel Halpern
Review Date: 2022-10-07
IETF LC End Date: 2022-10-18
IESG Telechat date: Not scheduled for a telechat

Summary: This document is ready for publication as a Proposed Standard RFC.

Major issues: N/A

Minor issues: N/A

Nits/editorial comments:
Should the RDs in section 6.1 and 6.2 use example IP addresses instead of
1.1.1.1 and 2.2.2.2?  (ID Nits called my attention to this, and I could not
decide if it was important.  So it is a nit here.)



___
BESS mailing list
BESS@ietf.org
https://www.ietf.org/mailman/listinfo/bess

[bess] Rtgdir last call review of draft-ietf-bess-evpn-lsp-ping-07

[Joel Halpern via Datatracker]

Reviewer: Joel Halpern
Review result: Ready

This is a routing directorate review of draft-ietf-bess-evpn-lsp-ping-07
provided by Joel Halpern at the request of the routing directorate review team
leaders as input to the routing area directors.  Please treat as any other
review comments.

(Apologies for the delay.)

This document is Ready for publication as a Proposed Standard RFC.

Comments:
Major: None

Minor:
Section 4.1 describes the EVPN MAC Sub-TLV.  And states it is derived from
MAC/IP advertisement route.  I note that in RFC 7623 (PBB-EVPN) there are
restrictions on several of these fields.  Should this section note at least
that in the PBB-EVPN case those restrictions apply (probably with a
pointer, not repeating the restrictions)?

 Section 4.4 describes EVPN IP Prefix Sub-TLV.  From the wording I suspect
 that it applies only to the EVPN case and not to the PBB-EVPN case.  In
 4.3, the text was explicit about that applicability.  Could we be equally
 clear here?



___
BESS mailing list
BESS@ietf.org
https://www.ietf.org/mailman/listinfo/bess

[bess] Genart last call review of draft-ietf-bess-vpls-multihoming-05

[Joel Halpern via Datatracker]

Reviewer: Joel Halpern
Review result: Ready with Issues

I am the assigned Gen-ART reviewer for this draft. The General Area
Review Team (Gen-ART) reviews all IETF documents being processed
by the IESG for the IETF Chair.  Please treat these comments just
like any other last call comments.

For more information, please see the FAQ at

.

Document: draft-ietf-bess-vpls-multihoming-05
Reviewer: Joel Halpern
Review Date: 2020-03-18
IETF LC End Date: 2020-03-12
IESG Telechat date: Not scheduled for a telechat

Summary: This document is ready for publication as a Proposed Standard.  I
would recommend that the minor issues below be addressed before approval for
publication.

Major issues: N/A

Minor issues:
Section 1 seems to have some language oddities.   The first paragraph
appears to be technically correct that  RFC 4761 and 6074 both describe
using BGP for auto-discovery.  As written, the text seems to imply that the
same thing is defined in two places.  This is going to confuse readers and
tempt them into trying to understand the wrong problem.  Please either
simplify or explain. (I believe the distinction in the RFCs is about use
with LDP.  While the text refers to that, the English construction is not
clear.) Section 1.1  paragraph 4 is a duplicate of the latter half of
paragraph 3.

   VE (VPLS Edge) is defined in section 1.1 as being about a device.  The
   introductory text then starts talking about VE NLRI and VE-IDs.  It looks
   like the point is merely to setup the definition for  CE-NLRI.  If so, the
   text could be restructured to say:
 BGP VPLS NLRI as specified in section 3.2.2 in [RFC4761] includes a number
 of fields.  This document refers to the VE-ID, VE offset,  from that
 RFC.

   This document should reference RFC 8174 as part of BCP 14.  And should use
   upper case usage (MUST, SHOULD, ...) consistently.  It currently mixes upper
   and lower case usage in different places.  Even though some of each usage
   are clearly normative.

It is unclear why discarding CE-ID=0 (which is defined to be invalid) is a
should rather than a MUST.

The beginning of section 3.3.1 seems to say that the L2-info community may
be omitted (e.g. is optional).  Then the text says that it must (presumably
upper case) be present.   As written, this is quite confusing. Reading
later, it appears that this is intended to support backwards compatibility.
 If so, then the sentence about handling their absence should say "Although
required, this document specifies mechanisms for dealing with their absence
to support backwards compatibility."  Alternatively, it seems that the
L2-info is allways required, but that the L2-pref is not required unless
the VPLS advertisement is inter-AS.  If that is the case, say that.

Nits/editorial comments:
I found myself misreading the definition of multi-homing: "redundant
connectivity to one or more sites".  What is there is technically correct. 
It would help I think if this were split into two pieces: multi-homing is
when the provider provides redundant VPLS connectivity to a customer site. 
If the customer has more than one site, the service is considered to be
multi-homed if at least one site is multi-homed.

   Section 3.3.1, the two new flags in the L2-info community are presumably
   defined in this document rather than proposed.   Or at least, that is how
   they need to be described once this document is approved.


___
BESS mailing list
BESS@ietf.org
https://www.ietf.org/mailman/listinfo/bess