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

["Bidgoli, Hooman (Nokia - CA/Ottawa)" <[e-mail]>]

Hi Nabeel

Much appreciate your comments, all your comments have been incorporated in the new version of the doc.

One exception is the format of “Appendix A.” I am using xml2rfc editor and the format of Appendix is hardcoded in the tool.

I will upload the new doc as soon as ietf  submission tool opens

Regards
Hooman

From: Nabeel Cocker <[e-mail]>
Sent: Sunday, November 8, 2020 5:24 PM
To: Mankamana Mishra (mankamis) <mankamis=[e-mail]>; Bidgoli, Hooman (Nokia - CA/Ottawa) <[e-mail]>; Jeffrey (Zhaohui) Zhang <[e-mail]>; Ijsbrand Wijnands <[e-mail]>; Stig Venaas <[e-mail]>
Cc: Greg Shepherd <[e-mail]>; BIER WG <[e-mail]>; Antoni Przygienda <[e-mail]>; [e-mail]
Subject: Re: [Bier] WG LC on https://datatracker.ietf.org/doc/draft-ietf-bier-pim-signaling/

Shepherds Review:

Authors, I started on the review of the document and have made my comments in the attached google document:

https://docs.google.com/document/d/1exSjh3ZFRl1LfWSmfv8BI_XzRewmsBPksOUKio4Rkvk/edit?usp=sharing

My comments are within that doc.  The document is in pretty good shape, it has addressed all the comments I have seen from the mailing list.

The main feedback is really on a few editorial/grammar/spellings...

If you prefer, I can download the google doc and send that as an attachment as well.

thanks
Nabeel

On Tue, Sep 15, 2020 at 2:57 PM Mankamana Mishra (mankamis) <mankamis=[e-mail]<mailto:[e-mail]>> wrote:
Yes, I think its good to move document to next step

From: Greg Shepherd <[e-mail]<mailto:[e-mail]>>
Date: Tuesday, September 15, 2020 at 10:39 AM
To: "Bidgoli, Hooman (Nokia - CA/Ottawa)" <[e-mail]<mailto:[e-mail]>>
Cc: Stig Venaas <[e-mail]<mailto:[e-mail]>>, "Mankamana Mishra (mankamis)" <[e-mail]<mailto:[e-mail]>>, BIER WG <[e-mail]<mailto:[e-mail]>>, Ijsbrand Wijnands <[e-mail]<mailto:[e-mail]>>, "[e-mail]<mailto:[e-mail]>" <[e-mail]<mailto:[e-mail]>>, "Jeffrey (Zhaohui) Zhang" <[e-mail]<mailto:[e-mail]>>, Antoni Przygienda <[e-mail]<mailto:[e-mail]>>
Subject: Re: [Bier] WG LC on https://datatracker.ietf.org/doc/draft-ietf-bier-pim-signaling/

Thanks! If all the authors agree then let’s release the Doc Shepherd for the write-up.

- Shep
Sent from my iPhone

On Sep 15, 2020, at 08:52, Bidgoli, Hooman (Nokia - CA/Ottawa) <[e-mail]<mailto:[e-mail]>> wrote:
Hi

Yes the agreed changes are there, again these were mostly cleanup rather then any significant procedural changes.

I think they were included in the previous last call.

Thanks
Hooman

From: Greg Shepherd <[e-mail]<mailto:[e-mail]>>
Sent: Monday, September 14, 2020 4:30 PM
To: Bidgoli, Hooman (Nokia - CA/Ottawa) <[e-mail]<mailto:[e-mail]>>
Cc: Stig Venaas <[e-mail]<mailto:[e-mail]>>; Mankamana Mishra (mankamis) <[e-mail]<mailto:[e-mail]>>; BIER WG <[e-mail]<mailto:[e-mail]>>; Ijsbrand Wijnands <[e-mail]<mailto:[e-mail]>>; [e-mail]<mailto:[e-mail]>; Jeffrey (Zhaohui) Zhang <[e-mail]<mailto:[e-mail]>>; Antoni Przygienda <[e-mail]<mailto:[e-mail]>>
Subject: Re: [Bier] WG LC on https://datatracker.ietf.org/doc/draft-ietf-bier-pim-signaling/

It appears that issues have been addressed as per the list discussion of July 28th. The latest rev was posted July 29th, I assume with the agreed changes, correct?

Let's do a quick 1 week WGLC to confirm the latest rev.

Please respond to this thread by Sept 27th.

Thanks,
Shep
(chairs)

On Tue, Jul 28, 2020 at 2:35 PM Bidgoli, Hooman (Nokia - CA/Ottawa) <[e-mail]<mailto:[e-mail]>> wrote:
Thanks Stig!

Inline HB>

Regards

Hooman

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

Hi

It looks like most of the issues I found have been resolved now. But most importantly, the IANA Considerations still say that there are none. This needs to be fixed!

HB> Done, Add it for the join attribute TLV that you got away from in the other draft ;)

I see a couple of places where it says PIM signaling message or PIM signaling packet. I think it should say PIM join/prune message, or PIM join/prune packet.,

HB>
Changed to PIM join/prune in section 3 because this was the PIM domain join/prune. "it will generate a PIM join/prune packet toward its attached PIM domain."
Any where else that the pim signaling is left is actually the BIER signaling the join/prune through the bier network as such I wanted it to be clear that these are signalling packet and not extending the PIM over the BIER domain.

:


Regards,
Stig

On Tue, Sep 24, 2019 at 10:51 AM Stig Venaas <[e-mail]<mailto:[e-mail]>> wrote:
>
> 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
_______________________________________________
BIER mailing list
[e-mail]<mailto:[e-mail]>
https://www.ietf.org/mailman/listinfo/bier