Re: [Bier] WG LC on https://datatracker.ietf.org/doc/draft-ietf-bier-pim-signaling/

["Jeffrey (Zhaohui) Zhang" <[e-mail]>]

As a co-author I support progressing the work. Obviously we'll look into the comments from Stig below and address them.

Thanks!
Jeffrey

-----Original Message-----
From: Stig Venaas <[e-mail]> 
Sent: Tuesday, September 24, 2019 1:51 PM
To: Mankamana Mishra (mankamis) <[e-mail]>
Cc: [e-mail]; Bidgoli, Hooman (Nokia - CA/Ottawa) <[e-mail]>; BIER WG <[e-mail]>; Ijsbrand Wijnands <[e-mail]>; [e-mail]; Jeffrey (Zhaohui) Zhang <[e-mail]>; Antoni Przygienda <[e-mail]>
Subject: Re: [Bier] WG LC on https://datatracker.ietf.org/doc/draft-ietf-bier-pim-signaling/

Hi

This is good work and I generally support the document. There are however some issues that need to be addressed. I also think it would be good to get this reviewed in the pim WG, perhaps you can ask for reviews on the pim list.

I mostly have editorial comments that are easy to address. But at a high level I think some more text about pim is needed.

PIM relies on hello messages to know what capabilities a router has, e.g. whether it support Join attributes, whether it support BIDIR etc. I think you need to point out what capabilities are assumed to be present. Obviously it is assumed that Join attributes are supported. It may also be good to point out that there is no J/P suppression as the J/P is only sent to the target, and not to other routers. Also point out that only J/P messages are used, and that there is no assert processing.


Below are the editorial comments. Please also check the "nits"
tool. It complains about missing references, and it cannot find the "Authors' Addresses" section. The heading is missing.

Also Section 7 must be updated. There is an IANA action, but it says there are none!

The abstract is rather long. Suggest removing some of the text that explains the general operation of BIER.

Some of the references might need some work. In section 2 it says "RFC 2119 [RFC2119]". Should it be just "[RFC2119]"? Also the reference is missing.  In 2.1 it says "[I-D. rfc8279]" which also needs to be fixed. Most of the other references look fine.
In 3.1 there seems to be a reference to 8279 without brackets though.

In 2.1, BFR definition. Missing space after ".".

BFIR definition: It says "insert the BM into the packet", but I think it might be better to say that it performs BIER encapsulation. The term BM is not defined. It says "plain" instead of "plane".

BFER defintion: Do we have to say that BFER is a BFR? In that case, we should also say that BFIR is a BFR. It says "plain" instead of "plane".

IBBR and EBBR, might it be good to include "signaling" in the name, like Ingress Signaling BIER Router and Egress Signaling BIER Router? I want it to be very clear that ingress and egress are not related to data.

In "Figure 1" I think "bfir" and "bfer" should be upper case.

In 3.1:
"weather" should be "whether".
"located on" should probably be "located in".

In 3.1 there is this paragraph that I think need some changes.

   After discovering the EBBR and its BFR-ID (flooded via IGP BIER
   extension), the IBBR will construct a PIM Join Attribute encoded as

It says that BFR-ID is flooded via IGP BIER extension. That isn't necessarily true, do we need to explain how the BFR-ID is discovered?

   TLVs into the Source Address field of the PIM Join Message as per

Isn't it just one TLV? I think you can skip saying that it is a TLV in the source address field. Just say that it includes a new PIM Join Attribute in the Join/Prune message. Also note that we should really say Join/Prune, not just Join.

   [RFC5384] and include it in PIM signaling message. Two new "BIER

I think it is better to be clear and say PIM Join/Prune message.

   IBBR" attributes are define and explained in upcoming section. The s/define/defined

   PIM Join Attribute is used on EBBR to obtain necessary bier s/bier/BIER
   information to build its multicast states. In addition the IBBR will
   change the PIM signaling packet source IP address to its BIER prefix
   address (standard PIM procedure). It will also keep the destination
   address as the well known multicast IP address. It then will
   construct the BIER header. The signaling packet, in this case the PIM
   join/prune packet, is encapsulated in the BIER header and transported
   through BIER domain to EBBR.

The language at the end here makes it sound like the PIM J/P is being forwarded and that the J/P is being modified. But J/P messages are sent hop-by-hop. Each router originates a new J/P. For instance, a router may receive a J/P for multiple sources. These sources may have different RPF neighbors on the receiving router and hence be split into separate join messages. This could happen on the IBBR as well.

The lasts paragraph in section 3 says:
   The IBBR will track all the PIM interfaces on the attached PIM domain
   which are interested in a certain (S,G). It creates multicast states
   for arriving (S,G)s from PIM domain, with incoming interface as BIER
   "tunnel" interface and outgoing interface as the PIM domain
   interface(s) on which PIM Join(s) were received on.

The IBBR is a PIM router and what you are describing is standard PIM behavior, so I think this is a bit redundant. It might be goot to stress that OIFs are adding according to standard PIM behavior. The only special here is the RPF interface.

Section 3.1.4:
In heading Pim/PIM
bier/BIER

It says "PIM Join Attribute [RFC5384] is used." I think it might be good to say that "a new PIM Join Attribute is used". And then also this sentence "The PIM Join Attribute format is as follow:" should perhaps be The new PIM Join Attribute format is defined as follows:".

s/Ipv4/IPv4
s/Ipv6/IPv6

In the format I think it might be good to not just show "BIER info", but show the formatting of prefix, SD and BFR-ID explicitly. Also, it should state clearly that these are the prefix, SD and BFR-id of the IBBR.

In 3.1.4.1 why not say PIM Join/Prune packet instead of PIM signaling?

In 3.3.:
   After receiving the BIER packet and determining this packet is a
   signaling packet, EBBR will remove the BIER header from PIM packet.
   The Received PIM join/prune Signaling packet is processed as if it
   were received from neighbors on a virtual interface, (i.e. as if the
   pim adjacency was presents, regardless of the fact there is no
   adjacency)
Wouldn't the router remove the BIER header simply because the BFR-id in the BIER header matches its own BFR-id? There are some grammar issues and a missing period in this paragraph.

In this paragraph:
   With same token the EBBR creates a multicast state with incoming
   interface as same interface that PIM join packet was forwarded and
   outgoing interfaces of BIER tunnel to BFER identified with BFIR-id
   and its corresponding Sub-Domain obtained from the BIER header or via
   PIM Join Attributes added to the PIM signaling packet by the IBBR.

I'm not sure what "With same token" means here. Also, it not should say that PIM join packet was forwarded. I would say that state is created according to the PIM specification, and just describe the BIER OIF state.
The specific OIF state may be implementation specific though. Perhaps this is sufficiently described in the paragraph that comes right after? It is also well described in 4.1.

In section 5: s/LEAFs/leaves? Should it be EBBRs?

In section 6:
s/vrf/VRF
"it is determine" should be "it is determined".
Replace "PIM signaling message" with "PIM Join/Prune message"?

Section 8:
The 4601 reference should be to RFC 7761.

Appendix A:
The header seems to not be formatted correctly.
"This section" should be "This appendix"?

In A.1: "is consist" should be "consists".

In Figure 2: s/bier/BIER
Also in the text in A.2.2.

IN A.3.1
s/Bier/BIER
s/bier/BIER
s/tlv/TLV

Stig