Re: [Bier] Benjamin Kaduk's Discuss on draft-ietf-bier-te-arch-10: (with DISCUSS and COMMENT)

Benjamin Kaduk <kaduk@mit.edu> Fri, 24 December 2021 23:09 UTC

Return-Path: <kaduk@mit.edu>
X-Original-To: bier@ietfa.amsl.com
Delivered-To: bier@ietfa.amsl.com
Received: from localhost (localhost [127.0.0.1]) by ietfa.amsl.com (Postfix) with ESMTP id CF8B53A00E1; Fri, 24 Dec 2021 15:09:36 -0800 (PST)
X-Virus-Scanned: amavisd-new at amsl.com
X-Spam-Flag: NO
X-Spam-Score: -1.496
X-Spam-Level:
X-Spam-Status: No, score=-1.496 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, KHOP_HELO_FCRDNS=0.399, RCVD_IN_MSPIKE_H3=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_NONE=0.001, URIBL_BLOCKED=0.001] autolearn=no autolearn_force=no
Received: from mail.ietf.org ([4.31.198.44]) by localhost (ietfa.amsl.com [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id aS6GxMaIabcG; Fri, 24 Dec 2021 15:09:32 -0800 (PST)
Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ietfa.amsl.com (Postfix) with ESMTPS id 1AF243A00E0; Fri, 24 Dec 2021 15:09:30 -0800 (PST)
Received: from mit.edu ([24.16.140.251]) (authenticated bits=56) (User authenticated as kaduk@ATHENA.MIT.EDU) by outgoing.mit.edu (8.14.7/8.12.4) with ESMTP id 1BON909d032177 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT); Fri, 24 Dec 2021 18:09:06 -0500
Date: Fri, 24 Dec 2021 15:08:59 -0800
From: Benjamin Kaduk <kaduk@mit.edu>
To: Toerless Eckert <tte@cs.fau.de>
Cc: The IESG <iesg@ietf.org>, draft-ietf-bier-te-arch@ietf.org, bier-chairs@ietf.org, bier@ietf.org, Xuesong Geng <gengxuesong@huawei.com>, aretana.ietf@gmail.com
Message-ID: <20211224230859.GD11486@mit.edu>
References: <162996119973.19003.11174569450531796563@ietfa.amsl.com> <YZNfIH1vSf5sQGXN@faui48e.informatik.uni-erlangen.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <YZNfIH1vSf5sQGXN@faui48e.informatik.uni-erlangen.de>
Archived-At: <https://mailarchive.ietf.org/arch/msg/bier/h0egMFomWxQyMrET4ZzayVODpcQ>
Subject: Re: [Bier] Benjamin Kaduk's Discuss on draft-ietf-bier-te-arch-10: (with DISCUSS and COMMENT)
X-BeenThere: bier@ietf.org
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: "\"Bit Indexed Explicit Replication discussion list\"" <bier.ietf.org>
List-Unsubscribe: <https://www.ietf.org/mailman/options/bier>, <mailto:bier-request@ietf.org?subject=unsubscribe>
List-Archive: <https://mailarchive.ietf.org/arch/browse/bier/>
List-Post: <mailto:bier@ietf.org>
List-Help: <mailto:bier-request@ietf.org?subject=help>
List-Subscribe: <https://www.ietf.org/mailman/listinfo/bier>, <mailto:bier-request@ietf.org?subject=subscribe>
X-List-Received-Date: Fri, 24 Dec 2021 23:09:37 -0000

Hi Toerless,

Alas, I had some time contention as well.
But responses now, as the contention has eased...

On Tue, Nov 16, 2021 at 08:34:56AM +0100, Toerless Eckert wrote:
> Hi Ben
> 
> Thanks a lot for the review, especially also the long list of nits to
> help improve text quality..
> 
> I have some time contention in the coming weeks, so i wanted to at least finish
> your review, so i pushed -11 for the fixes from your review.
> 
> The DISCUSS was actually just one wrong character.
> 
> Answers to your comments and fixes inline below.
> 
> diff here:
> 
> http://tools.ietf.org/rfcdiff?url1=https://www.ietf.org/archive/id/draft-ietf-bier-te-arch-10.txt&url2=https://www.ietf.org/archive/id/draft-ietf-bier-te-arch-11.txt
> 
> Next rev with fixes for the other IESG reviewer comments somewhat later. Sorry.
> 
> Cheers
>     Toerless
> 
> On Wed, Aug 25, 2021 at 11:59:59PM -0700, Benjamin Kaduk via Datatracker wrote:
> > Benjamin Kaduk has entered the following ballot position for
> > draft-ietf-bier-te-arch-10: Discuss
> > 
> > When responding, please keep the subject line intact and reply to all
> > email addresses included in the To and CC lines. (Feel free to cut this
> > introductory paragraph, however.)
> > 
> > 
> > Please refer to https://www.ietf.org/iesg/statement/discuss-criteria.html
> > for more information about DISCUSS and COMMENT positions.
> > 
> > 
> > The document, along with other ballot positions, can be found here:
> > https://datatracker.ietf.org/doc/draft-ietf-bier-te-arch/
> > 
> > 
> > 
> > ----------------------------------------------------------------------
> > DISCUSS:
> > ----------------------------------------------------------------------
> > 
> > I think there's an issue with the pseudocode in Figure 6.  While I
> > understand that it's pseudocode, any reasonable interpretation I can
> > come up with for the "~" and "&=" operators seems to result in
> > performing an operation logically equivalent to:
> > 
> > X = AdjacentBits[SI]
> > Packet->BitString = Packet->BitString & ~X & X
> > 
> > that can be optimized to
> > 
> > Packet->BitString = 0
> > 
> > and I do not think that the only bits that are supposed to be set in the
> > outgoing packet/packet copy are the ones for which DNC is set -- bits
> > that we did not find in our BIFT should remain set in outgoing packets.
> > (Slightly more detail in the COMMENT.)
> 
> great catch
> 
> AdjacentBits = Packet->BitString &=  ~AdjacentBits[SI];
>                                   ^
> No idea how i overlooked that extraneous = character.
> 
> I also inserted two comment lines to make the purpose of the lines more obvious:
> 
>      // Set variable for looping across only adjacent bits
>      AdjacentBits = Packet->BitString & ~AdjacentBits[SI];
>      // Clear adjacent bits in Packet header to avoid loops
>      Packet->BitString &= ~AdjacentBits[SI];

The comments help a great deal, though I'm still not convinced it's quite
right.
The clearing of Packet->BitString in the second line seems okay now, but in
the first line we are supposed to be grabbing the set of bits that were set
in the packet and represent adjacencies relevant for this router.  I think
that should be:

    AdjacentBits = Packet->BitString & AdjacentBits[SI];

(with no "~") so that we loop over the intended bits later on.

I'd also suggest using a different name than "AdjacentBits" for this local
variable.  What the behavior is when a local variable has the same name as
an array variable is unlikely to be an intuitive part of pseudocode, and
using a different name provides a clear separation.

(I also note that Carsten had some more comments on the pseudocode, which
seem worth thinking about; the criteria I used in my original review
wouldn't have made such items Discuss-worthy, so from my perspective they
are non-blocking comments.)

> > ----------------------------------------------------------------------
> > COMMENT:
> > ----------------------------------------------------------------------
> > 
> > I think it would be helpful to have a clear statement early on that the
> > DNC bit is applicable only to "forward_connected()" adjacencies and not
> > to "forward_routed()", that is, earlier than the guidance in §5.2.1.
> 
> Added to 4.2.1 (first specification style mentioning of DNC):
> 
> <t>For safety against loops under misconfiguration (see <xref target="Loops"/>),
> DNC is only permissible for forward_connected() adjacencies. No need or benefit
> of DNC for other type of adjacencies was identified and their risk was not analyzed.</t>

Thank you!

> > It's easy for a reader in a hurry to miss the distinction that just
> > "BIER" is "regular BIER" as opposed to the "BIER-TE" that this document
> > specifies.  It might be worth adding a qualifier in at least some
> > instances about "non-TE BIER" or similar, especially where we have a
> > paragraph that talks about BIER as a set-up for a comparison with
> > BIER-TE.
> 
> Hmm. I have added (non-TE) to several places , first mentioning of BIER
> in sections. To give it a try. Will ask the WG if they like it, else remove them later again.
> I would assume people not familiar with BIER may prefer this, while those
> are may not. I don't think i have ever seen this for RSVP vs. RSVP-TE...

Thanks for giving it a try.  If the WG doesn't like it, I am happy to defer
to them.

> > Thanks for the conversation with the gen-art reviewer; I think it's important
> > to get in the clarifications that were proposed in that thread.
> > 
> > Section 2.3
> > 
> >    BIER-TE is also intended as an option to expand the BIER architecture
> >    into deployments where BIER may not be the best fit, [...]
> > 
> > It's fairly surprising to advocate for doing something even when there
> > are known better options to achieve the same goal.
> 
> Ok. That actually a very good place to add (non-TE) before "BIER may not.."

Oh!  With that, it is a much more interesting statement :)

> > Section 4.3
> > 
> > It seems that these coexistence schemes rely on something like a central
> > controller entity to configure all BFRs with the criteria for assessing
> > whether a given packet is in "BIER mode" or "BIER-TE mode".
> 
> It more like configuring SD values to be BIER or BIER-TE. Think of
> being able to configure if an IP address prefix is unicast or multicast
> instead of standardizing the address plan.
> 
> > It seems
> > prudent to consider the risk that this information is not distributed
> > uniformly in time (especially in the case of dynamic updates) or
> > otherwise becomes stale on some particular node(s).  Would the network
> > be "self-healing" in the face of a node that persistently
> > mis-forwards/replicates a given class of packet (by virtue of treating
> > it as BIER rather than BIER-TE, or vice versa)?
> 
> The self-healing has to come from the control plane. (non-TE) BIER
> was specified very much with the assumption that one uses an IGP as
> the control plane, but the desire is now to also be able to use a central
> controller for deployments that don't like IGP. BIER-TE is written very
> much assuming there is a controller, but if someone comes up with a
> disributed control plane, that would be great too.
> 
> IMHO, a central controller has an easier job achieving consistency as
> an IGP because it can employ more intricate multi-phase commit procedures.
> Of course, a lazily deveoped controller could be worse than an IGP.
> 
> With there was an RFC to reference these fundamental routing topics,
> but in the absence of it i wouldn't want to venture writing down
> any of this in a mere "application" of those mechanisms.

Ok.  Thanks for sharing the extra background.

> > 
> > Section 4.4
> > 
> >         AdjacentBits = Packet->BitString &= ~AdjacentBits[SI];
> > 
> > The "A = B &= C" construct might be overly clever for what is intended
> > to be illustrative pseudocode...
> > 
> >         Packet->BitString &= AdjacentBits[SI];
> > 
> > ... and the combination of "Packet->BitString &= ~X" and
> > "Packet->BitString &= X" seems to result in all bits getting cleared, if
> > I'm not mistaken.  Surely that's not the intent (per the Discuss)...
> 
> Done.
> 
> > Section 4.5
> > 
> > I think the example that walks through the packet flow and talks about
> > which bits are cleared along the way is a helpful example.
> 
> Thanks
> 
> > Section 5.1.3
> > 
> >    All leaf-BFERs in a BIER-TE domain can share a single bit position.
> >    This is possible because the bit position for the adjacency to reach
> >    the BFER can be used to distinguish whether or not packets should
> >    reach the BFER.
> > 
> > I'm not sure I understand this properly.  I think that this single
> > shared bit is used both to indicate "send along this adjacency" and
> > (implicitly?) "local_decap when it gets here".  It seems like this could
> > result in spurious packet copies egressing from some BFERs.  Consider
> > (using the left topology from Figure 10) a case where only egress from
> > BFER2 is needed, but some other constraints meant that the only possible
> > path from BFIR to BFER2 involved traversing BFR1 on the way to BFR2.
> > Even though BFR1 only needs to be a transit router for this flow, it
> > will still send a packet copy to BFER1 (where it gets decapsulated and
> > sent out).
> 
> Routing , including "spurious routing" impacts only forward_routed(),
> not forward_connected(), so it could only impact a packet that
> was encapsulated for forward_routed by some other BFR with destination
> being BFER2, and this packet passes unexpectedly via BFR1. But it doesn't
> matter which routers/BFR it passes through, the BIER header will not
> be examined there, its just unicast routed.
> 
> > Even worse, if DNC is not set, it seems that the copy that
> > leaves BFR1 and makes its way to BFR2 will have the bit cleared that's
> > used to indicate "leaf BFER".  So, either I'm confused about how this
> > works (something that's quite plausible!), or there's some other
> > undocumented constraints here either on the topology or the use of DNC.
> 
> The BitString is intended to express a delivery tree. Only nodes that
> will always be leaves in any tree (because of their location in the topolocy)
> will have a share Leaf BFR bit set. The fact that they clear this
> bit when they process the packet does thereofre not hurt further
> BIER forwarding of the packet, because by being leaf there is the implication
> that no BitString will be built in which they need to forward a receive
> BIER-TE pto a further BFR.
> 
> > Reading forward to the summary in §5.1.10, perhaps my assumption about
> > indicating "send along this adjacency" was incorrect.  Still, I
> > shouldn't need to read the later section in order to understand the
> > earlier one, so adding a few more words here may be in order.
> 
> I wouldn't know what to write to clear up this confusion.
> If i managed correctly guess what you where confused about,
> maybe you want to suggest text ?

I am still a little uncertain whether I understand correctly, though less
so than when I wrote the original comment.

When you say that "nodes ... will have a share Leaf BFR bit set", is this
referring to when "the BIER-TE Controller then pushes the
BitPositions/adjacenties to the BIFT of the BFRs" (to quote §3.2.1.1),
which would (roughly?) correspond to having a bit set in the local BIFT for
a given adjacency?

Assuming that it is, then when I transition to the context of how a packet
where that bit is set gets propagated, that seems to imply that the "share
Leaf BFR bit" is not relevant to the processing that BFR1 does in my
original example.  Going all the way back to my phrasing with a split into
"send along this adjacency" and "local_decap when it gets here", that was
intended to refer to BFR1 performing the "send along this adjacency"
function, and in light of the subsequent discussion I no longer think that
the single shared bit would perform that function.

I think the source of my confusion is the sentence "All leaf-BFERs in a
BIER-TE domain can share a single bit position", in that I can read that
sentence as saying either/both of "single bit position for 'send this
packet out the interface that exits the BIER-TE domain'" and "single bit
position for 'a packet should get sent to this BFER'".  It sounds like only
the former is the intent, so maybe adding a few words to this sentence
would help, e.g., "All leaf-BFERs in a BIER-TE domain can share a single
bit position to indicate that a packet should be processed and sent onwards
out of the BIER-TE domain".


> > Section 5.1.5
> > 
> >    In a setup with a hub and multiple spokes connected via separate p2p
> >    links to the hub, all p2p links can share the same bit position.  The
> > 
> > Is this for the adjacency to the hub, from the hub, or the same bit for
> > both?
> 
> Good catch. Changed to:
> 
> | In a setup with a hub and multiple spokes connected via separate
> | p2p links to the hub, all p2p adjacencies from the hub to the spokes links can share the same bit position.
> 
> Aka: this is for hop->spoke traffic only.
> 
> Note that this section does not describe that you may also want to
> use a single (but separate) BP for all spoke->hub adjacencies,
> because designing how to do this has more considerations, and
> this section was just written to give a sufficiently good reasoning
> why we want to support a list of adjacencies for a BP.

Understood.

> > Section 5.2.1
> > 
> >    Link layers without port unique link layer addresses should not be
> >    used with the DNC flag set.
> > 
> > "should not" or "MUST NOT"?
> 
> First of all, it is lower case because i have tried to not
> create normative requirements against controller behavior (section 5),
>  because i have not seen this being done elsewhere in documents for
> comparable text/guidelines.
> 
> Secondly, it is "should" and not "must/MUST" because the loops
> in question would happen under miscabling (and then the link layer
> address protects), but there are of course networks that would
> break down from miscabling anyhow - e.g.: in embedded 
> environments, planes, trains, automobiles. So on those type
> of networks, one can expect that there is never miscabling
> (because there is always complete cabling validation before
>  operations). In these type of networks one can likely forego link
> layer verification of cabling.
> 
> > Section 5.2.2
> > 
> >    When links are incorrectly physically re-connected before the BIER-TE
> >    Controller updates BitStrings in BFIRs, duplicates can happen.  Like
> >    loops, these can be inhibited by link layer addressing in
> >    forward_connected() adjacencies.
> > 
> > (Likewise.)
> 
> Answer also likewise ;-)
> 
> > Section 5.3.2
> > 
> >    and the amount of alternative paths in it.  The higher the percentage
> >    of non-BFER bits, the higher the likelihood, that those topology bits
> >    are not just BIER-TE overhead without additional benefit, but instead
> >    that they will allow to express desirable path steering alternatives.
> > 
> > This assessment of likelihood seems unsupportable without some
> > additional assumptions or preconditions.  If the sampling domain is
> > "all possible representations of subtopologies" I think the statement is
> > false.  It only seems likely to hold if the sampling domain is limited
> > to topology descriptions crafted by BIER-TE experts.
> 
> Nice. Indeed.
> 
> | In a BIER-TE topology crafted by a BIER-TE expert, the higher the percentage...
> 
> > Section 5.3.3
> > 
> >    In BIER-TE, for a non-leaf BFER, there is usually a single BP for
> >    that BFER with a local_decap() adjacency on the BFER.  The BFR-id for
> >    such a BFER can therefore equally be determined as in BIER: BFR-id =
> >    SI * BitStringLength + BP.
> > 
> > I don't think I understand why this constraint is necessary.  What
> > component is going to know which BFRs are BFERs and rely on this
> > property of teh BFR-id?
> 
> This is not a constraint, but an explanation how to calculate the
> bift-id of a BFER in BIER-TE. the bift-id is an identifier used
> in various BIER routing underlay signaling (which i think we don't
> need for BIER-TE, but someone might want to), and Multicast Flow
> overlay signaling (which we do want to reuse). In BIER, this
> bift is BFR-id = SI * BitStringLength + BP, where BP is the BP
> explicitly assigned to the BFER.
> 
> In BIER-TE we do not necessarily have a single BP designated
> to a BFER, so the controller has various options of how he
> assigns unambiguous bift-ids to BFER, and this text explains
> one option that would make it closests to (non-te) BIER.

I think the "equally be determined" was what tripped me up, as it implies
that there is some other (unstated) primary way to determine the BFR-id,
which is equivalent to this procedure.  Maybe "equally" is not quite the
right word, or we should say "can be determined using the same procedure as
in [(non-TE)] BIER"?

> > Section 5.3.5
> > 
> >    It is not currently determined if a single sub-domain could or should
> >    be allowed to forward both BIER and BIER-TE packets.  If this should
> >    be supported, there are two options:
> > 
> > This sounds kind of Experimental.  Perhaps this content is more
> > appropriate in an Appendix?
> 
> Not really experimental. I'd call it an option (MAY) vendor implementation option.
> Ultimately this relates to the encap, so like in the BIER architecture (RFC8279),
> which is not normative about encap but gives guidance, this is guidance 
> might result in normative text in some update of RFC8296.
> 
> >    this approach.  Depending on topology, only the same 20%..80% of bits
> >    as possible for BIER-TE can be used for BIER.
> > 
> > Where does the 20-80% range come from?
> 
> remove sentence. I originally had this rough estimate (20..80%) in a 
> prior paragraph, but that was negotiated away by another reviewer,
> and this one was left over (hence also the meaningless "only the same"...
> - which was referring to the prior, alredy removed occurance of this
> estimate).
> 
> > Section 5.3.6.1
> > 
> >    Consider a network setup with a BSL of 256 for a network topology as
> >    shown in Figure 20.  The network has 6 areas, each with 170 BFRs,
> >    connecting via a core with 4 (core) BFRs.  To address all BFERs with
> >    BIER, 4 SIs are required.  To send a BIER packet to all BFER in the
> > 
> > How many BFERs per area?
> 
> replaced "170 BFRs" with "170 BFERs".
> 
> > What calculation produces the "4 SIs are required" result?
> 
> 170 * 6 / 265 = 3.84 => 4 * 265 bitstrings (SI) needed.
> 
> > Section 5.3.6.2
> > 
> >    On all BFIRs in an area j|j=2...6, bia in each BIFT:SI is populated
> >    with the same forward_routed(BFRja), and bib with
> >    forward_routed(BFRjb).  On all area edge BFR, bea in
> >    BIFT:SI=k|k=2...6 is populated with forward_routed(BFRka) and beb in
> > 
> > Why do j and k range from 2 through 6, excluding 1?  Where is area 1
> > handled?
> 
> Added at end of paragraph:
> 
> | For this setup we do not consider
> | area 1 because we assume the BIER-TE setup is just for sending traffic from
> | area 1 into area 2...6, for example bcause the broadcast headends are
> | in area 1 for an IPTV BIER-TE setup.
> 
> > Section 6
> > 
> >    Adjacency scope could be global, but then every BFR would need an
> >    adjacency for this BP, for example a forward_routed() adjacency with
> >    encapsulation to the global SR SID of the destination.  Such a BP
> >    would always result in ingress replication though.  The first BFR
> > 
> > I don't think I know what "ingress replication" is.
> 
> Added "(as in <xref target="RFC7988"/>)" as pointer defining ingress
> replicartion.
> 
> > Are we trying to say that all the replication that occurs for traffic to
> > this BP is created starting at the ingress node and there can be no
> > efficiency gains by deferring replication until later on in the path(s)?
> 
> Yes. This is not meant to describe something ideal but to explain
> how SR terms would map to BIER-TE options.
> 
> >    Both BIER and BIER-TE allow BFIR to "opportunistically" copy packets
> >    to a set of desired BFER on a packet-by-packet basis.  In BIER, this
> >    is done by OR'ing the BP for the desired BFER.  In BIER-TE this can
> >    be done by OR'ing for each desired BFER a BitString using the
> >    "independent branches" approach described in Section 5.3.3 and
> >    therefore also indicating the engineered path towards each desired
> >    BFER.  This is the approach that
> >    [I-D.ietf-bier-multicast-http-response] relies on.
> > 
> > It's a bit late in the day here (so I could be missing something), but
> > it seems to me that this paragraph is not really needed in this
> > document.  If it's helpful content, it could appear in the referenced
> > document, to which it seems more directly applicable.
> 
> Nope. sharp. This is duplicate text too, thats i probably originally
> felt would make sense to repeat in the context of SR, but not i also
> don't see a good linkage to SR. Removed paragraph.
> 
> > Section 7
> > 
> > It would be great if (as Roman mentions) we could say that any
> > BFR/Controller connections used for BIER-TE MUST be protected by TLS or
> > security protections at least as strong as TLS.
> > 
> > We might consider something like "the discussion in [3272bis] is also
> > relevant for BIER-TE".
> 
> I added after the existing sentence:
> 
> |for example by using Secure SHell (SSH), <xref target="RFC4253"/> for NetConf (<xref target="RFC6241"/>), or via Transport Layer Security (TLS), such as <xref target="RFC8253"/> for PCEP, <xref target="RFC5440"/>, or <xref target="RFC7589"/> for NetConf.
> 
> | BIER-TE controllers SHOULD use security equal to or better than these mechanisms.
> 
> I am only using SHOULD because i dont see a risk of getting less security
> in any "IT" network (service provider, enterprise, data-center), TLS is
> the obviouos choice, and if somebody uses something different from the above
> list like SSH, then the deployment has some more fundamental ops challenges.
> 
> But with MUST, we might create an inappropriate burden for "OT" networks
> (inside trains, planes, automobiles, to repeat my prior example). 
> In many cases you might have physically removed "provisioning ports".
> 
> Think of hidden i2c ports on devices through which you configure the
> device, then you remove and never touch again. Aka: physical security.
>  Is that equal or better than TLS ? 

It might depend on what attacker capabilities are in the threat model you
care about.  If you only care about some attacker far away on the internet
... probably.

Which is to say, I will not argue about the SHOULD, here.


And all the other stuff (above and below) seems okay; no further comments.
Thanks again for the updates and explanations,

Ben

> > Section 11.2
> > 
> > If "it is expected that the reader be familiar with" RFC 8296, it should
> > probably be classified as normative.
> 
> Done.
> 
> > NITS
> > 
> > By no means an exhaustive collection, but hopefully they will help.
> > 
> > Section 2.2
> > 
> >    may be called an "overlay" BIER-TE topology.  A BIER-TE topology will
> >    both "forward_connected" and "forward_routed" adjacencies may be
> >    called a "hybrid" BIER-TE topology.
> > 
> > s/will/with/
> 
> Ack.
> 
> > Section 2.3
> > 
> >                                                                 In BIER-
> >            TE, bits in the BitString of a BIER packet header indicate an
> >            adjacency in the BIER-TE topology, and only the BFRs that are
> >            upstream of this adjacency have this bit populated with the
> >            adjacency in their BIFT.
> > 
> > I think that s/are upstream/are the upstream endpoint/ would greatly aid
> > readability.
> 
> Ack.
> 
> > 
> >        2.  In BIER, the implied reference option for the core part of
> >            the BIER layer control plane is the BIER extension to
> >            distributed routing protocol, such as standardized in ISIS/
> >            OSPF extensions for BIER, [RFC8401] and [RFC8444].  [...]
> > 
> > There seems to be some singular/plural mismatch(es) going on here.
> > Maybe "are the BIER extensions to distributed routing protocols, such as
> > those standardized in"?
> 
> yes.
> 
> >    3.  The following element/functions described in the BIER
> >        architecture are not required by the BIER-TE architecture:
> > 
> > Probably best to use "elements/functions" to match plurality.
> 
> ack.
> 
> >        1.  BIRTs on BFR for BIER-TE are not required when using a BIER-
> > 
> > I think "BFRs" plural.  (Probably the plural is best for the next few
> > instances of "BFR" (not "BFR-id") in this section, but I will not try to
> > note them all explicitly.)
> 
> ack
> 
> > Section 3.2
> > 
> > I think s/BFIR/BFIRs/ in all cases in this section (excluding
> > subsections) other than item 2.3, and possibly 2.1 if the intent is "for
> > each multicast overlay flow".
> 
> sounds right.
> > 
> >        3.  Install state on the BFIR to imposition the desired BIER
> >            packet header(s) for packets of the overlay flow.
> > 
> > s/imposition/impose/
> 
> ack.
> 
> > Section 3.2.1
> 
> >    o  Data-models and protocols to communicate between controller and
> >       BFR in step 1, such YANG/Netconf/RestConf.
> > 
> > probably s/BFR/BFRs/ and s/such/such as/.
> 
> ack.
> 
> > Section 3.2.1.1
> > 
> >    When the topology is determined, the BIER-TE Controller then pushes
> >    the BitPositions/adjacencies to the BIFT of the BFRs, populating only
> >    those SI:BitPositions to the BIFT of each BFR to which that BFR
> >    should be able to send packets to - adjacencies connecting to this
> >    BFR.
> > 
> > This sentence should probably be rewritten and split up; it may have
> > multiple valid parse trees but regardless is pretty hard to parse.
> > Having separate terms for "the BFR receiving BIFT updates" and "the BFR
> > to which a given bit says to send a packet" would help a lot.
> 
> Yeah, weird complex. simplified now:
> 
> <t>When the BIER-TE topology is determined, the BIER-TE Controller then pushes
> the BitPositions/adjacencies to the BIFT of the BFRs. On each BFR only those SI:BitPositions
> are populated that are adjacencies to other BFRs in the BIER-TE topology.</t>
> 
> > Section 3.2.1.2
> > 
> >    encoded as the same BitString.  In BIER-TE, the BitString used to
> >    reach the same set of BFER in the same sub-domain can be different
> >    for different overlay flows because the BitString encodes the paths
> >    towards the BFER, so the BitStrings from different BFIR to the same
> >    set of BFER will often be different, and the BitString from the same
> >    BFIR to the same set of BFER can different for different overlay
> >    flows for policy reasons such as shortest path trees, Steiner trees
> >    (minimum cost trees), diverse path trees for redundancy and so on.
> > 
> > I suggest breaking the sentence before "and the BitString from the same
> > BFIR".
> 
> ack
> > 
> > Also, s/can different/can be different/
> 
> ack
> > 
> >    See also [I-D.ietf-bier-multicast-http-response] for a solution
> >    describing this interaction.
> > 
> > I'm not sure that "solution" is the best word here ("solution to which
> > problem?").
> 
> See also <xref target="I-D.ietf-bier-multicast-http-response"/> for an application  
> leveraging BIER-TE engineered trees.
> 
> > Section 3.2.1.4
> > 
> >    When link or nodes fail or recover in the topology, BIER-TE could
> >    quickly respond with out-of-scope FRR procedures such as
> >    [I-D.eckert-bier-te-frr].  [...]
> > 
> > Is the intent to say "could quickly respond with FRR procedures [...]
> > the details of which are out of scope for this document"?
> 
> ack.
> 
> > Section 3.3
> > 
> >    1.  On BFIR imposition of BIER header for packets from overlay flows.
> > 
> > comma after BFIR
> 
> ack
> > 
> >    3.  On BFER removal of BIER header and dispatching of the payload
> > 
> > comma after BFER
> 
> ack
> > 
> > Section 3.5
> > 
> >    available at each BFR and for each BP when it is providing congestion
> >    loss free services such as Rate Controlled Service Disciplines
> > 
> > hyphenate "congestion-loss-free"
> 
> ack
> > 
> >    control protocol such as Netconf or any other protocol commonly used
> >    by a PCE to understand the resources of the network it operates on.
> > 
> > I think s/PCE/Controller/ since this is the only instance of "PCE"
> > that's not part of "PCEP", in this document.
> 
> Ack.
> 
> >    the BIER-TE defined steering of packets.  This includes allocation of
> >    buffers to guarantee the worst case requirements of admitted RCSD
> >    traffic and potential policing and/or rate-shaping mechanisms,
> > 
> > I'm not sure, but possibly s/potential/potentially/
> 
> yes.
> 
> > Section 4.2.1
> > 
> >    for copies of the packet made towards other adjacencies.  This can be
> >    used for example in ring topologies as explained below.
> > 
> > "Below" could become a reference to §5.1.6 as was done in §4.1.
> 
> ack.
> 
> > Section 4.2.3
> > 
> >    packet is copied onto such an ECMP adjacency, an implementation
> >    specific so-called hash function will select one out of the lists
> >    adjacencies to which the packet is forwarded.  This ECMP hash
> > 
> > s/lists/list's/
> 
> ack
> 
> > Section 4.2.4
> > 
> >    packets.  Local_decap() adjacencies require the BFER to support
> >    routing or switching for NextProto to determine how to further
> >    process the packet.
> > 
> > I'm not finding a protocol element to map to "NextProto" in this
> > context, so maybe writing out in long form something like "the next
> > protocol layer" would be preferred.
> 
> Resolved by introducing term in prior sentence:
> 
> A "local_decap" adjacency passes a copy of the payload of
> the BIER-TE packet to the protocol ("NextProto") within the BFR (IPv4/IPv6, Ethernet,...) responsible for
> that payload
> 
> NextProto is also used in the Pseudocode.
> 
> > Section 4.3
> > 
> >    With MPLS, it is also possible to reuse the same SD space for both
> >    BIER-TE and BIER, so that the same SD has both a BIER BIFT and
> >    according range of BIFT-ids and a disjoint BIER-TE BIFT and non-
> >    overlapping range of BIFT-ids.
> > 
> > I think s/according/corresponding/
> 
> ack.
> 
> >    "forward_routed" requires an encapsulation permitting to unicast
> >    BIER-TE packets to a specific interface address on a target BFR.
> > 
> > "encapsulation that permits directing unicast BIER-TE packets"
> 
> ack.
> 
> > Section 4.4
> > 
> >       void ForwardBitMaskPacket_withTE (Packet)
> >       {
> >           SI=GetPacketSI(Packet);
> >           Offset=SI*BitStringLength;
> >           for (Index = GetFirstbit position(Packet->BitString); Index ;
> >                Index = GetNextbit position(Packet->BitString, Index)) {
> >               F-BM = BIFT[Index+Offset]->F-BM;
> >               if (!F-BM) continue;                            [3]
> >               BFR-NBR = BIFT[Index+Offset]->BFR-NBR;
> >               PacketCopy = Copy(Packet);
> >               PacketCopy->BitString &= F-BM;                  [2]
> >               PacketSend(PacketCopy, BFR-NBR);
> >               // The following must not be done for BIER-TE:
> >               // Packet->BitString &= ~F-BM;                  [1]
> >           }
> >       }
> > 
> > GetFirstBitPosition and GetNextBitPosition are the spellings used in the
> > RFC 8279 pseudocode; I don't see reason to diverge from them.  (The same
> > comment applies to the pseudocode in Figure 6 as well.)
> 
> Indeed. strange. fixed.
> 
> >    To solve both [1] and [2] for BIER, the F-BM of each bit needs to
> >    have all bits set that this BFR wants to route across BFR-NBR. [2]
> > 
> > Expanded out, "the F-BM of each bit needs to have" doesn't seem to make
> > much sense; would s/bit/bit index/ make more sense?
> 
> Yes.
> 
> >       *  The packets BitString is masked with those AdjacentBits before
> >          the loop to avoid doing this repeatedly for every PacketCopy.
> > 
> > s/packets/packet's/
> 
> ack
> 
> >                     case forward_routed({VRF},neighbor):
> >                         SendToL3(PacketCopy,{VRF,}l3-neighbor);
> > 
> > Should the first line use "l3-neighbor" as well as the second one?
> 
> yes.
> 
> > Section 4.5
> > 
> >    BFR3 sees a BitString of p5,p7,p8,p10,p11,p12.  For those BPs, it has
> >    only adjacencies for p7,p8.  It creates a copy of the packet to BFER1
> >    (due to p7) and one to BFR4 (due to p8).  It clears p7, p8 before
> >    sending.
> > 
> > I think "clears both p7 and p8 in both copies before sending" is more
> > clear.
> 
> ack.
> 
> > Section 4.6
> > 
> >    clear DNC flag, forward_routed() and local_decap.  As explained in
> >    Section 4.4, these REQUIRED BIER-TE forwarding functions can be
> >    implement via the same Forwarding Pseudocode as BIER forwarding
> > 
> > s/implement/implemented/
> > 
> > Section 5.1.4
> > 
> >    opposite LANs.  Adjacencies of such BFRs into their LAN still need a
> >    separate bit position.
> > 
> > Is this something like "such multi-homed BFRs"?
> 
> That is name calling, and i usually hear the term only in
> conjunction with hosts, not routers, so i think it would not be helpfull.
> 
> > Section 5.1.9
> > 
> >                           Figure 17: Reuse of BP
> > 
> >    Reuse may also save BPs in larger topologies.  Consider the topology
> >    shown in Figure 20.  A BFIR/sender (e.g.: video headend) is attached
> > 
> > I suspect that s/20/17/ is intended.
> 
> Yes.
> 
> > Section 5.1.10
> > 
> >    o  BP can generally be reused across nodes that do not need to be
> >       consecutive in paths, but depending on scenario, this may limit
> > 
> > "consecutive in paths" might imply "directly one after the other" to
> > some readers.  Perhaps "where it can be guaranteed that no path will
> > ever need to traverse more than one node of the set"?
> 
> Right. thanks.
> 
> > Section 5.3.3
> > 
> >    BIER-TE forwarding does not use the BFR-id, not does it require for
> > 
> > s/not does/nor does/
> 
> fixed.
> 
> > Section 5.3.6.2
> > 
> >    In addition, we use 4 bits in each SI: bia, bib, bea, beb: bit
> >    ingress a, bit ingress b, bit egress a, bit egress b.  These bits
> > 
> > It's probably worth spending a few more words to connect these "a" and
> > "b" back to the 6 different BFR1a/BFR1b through BFR6a/BFR6b of Figure
> > 20.  (Figure 20, which appears in §5.3.6.1, is not mentioned at all from
> > this section at present.)
> 
> Added reference to picture, highlighted how abbreviations are derived:
> 
>     In addition, we use 4 bits in each SI: bia, bib, bea, beb: (b)it (i)ngress (a),
>     (b)it (i)ngress (b), (b)it (e)gress (a), (b)it (e)gress (b).
> > 
> >    legs: 1) BFIR to ingress are edge and 2) core to egress area edge.
> > 
> > s/are edge/area edge/
> 
> Ack.
> 
> > Section 6
> > 
> >    hop to each destination Node-SID.  What BIER does not allow is to
> >    indicate intermediate hops, or terms of SR the ability to indicate a
> > 
> > s/terms of SR/in terms of SR/
> 
> ack
> 
> > Section 7
> > 
> >    In BIER, the standardized methods for the routing underlays as well
> >    as to distribute BFR-ids and BFR-prefixes are IGPs such as specified
> >    in [RFC8401] for IS-IS and in [RFC8444] for OSPF.  Attacking the
> > 
> > I can't make a parse tree for this sentence.  I think it's just trying
> > to say that RFCs 8401 and 8444 specify how to distribute BFR-ids and
> > BFR-prefixes over the respective IGP, and the respective IGP inherently
> > distributes information needed for routing underlays, but I could be
> > wrong.
> 
> 
> <t>In BIER, the standardized methods for the routing underlays are IGPs with extensions to distribute BFR-ids and BFR-prefixes.  <xref target="RFC8401"/> specifies the extensions for IS-IS and <xref target="RFC8444"/> specifies the extensions for OSPF.
> > 
> >    against the results of the routing protocol, enabling DoS attacks
> >    against paths or addressing (BFR-id, BFR-prefixes) used by BIER.
> > 
> > I think s/addressing/the addressing/
> 
> ack.
> > 
> >    adjacencies, then this is still an attack vector as in BIER, but only
> >    for BIER-TE forward_routed() adjacencies, and no other adjacencies.
> > 
> > s/and no/and not/
> 
> ack
> 
> >    the BIER-TE controller/control-plane can and are much more commonly
> > 
> > s/can/can be/
> 
> ack
> > 
> >    forwarding rules are defined to be as strict in clearing bits as they
> >    are.  The clearing of all bits with an adjacency on a BFR prohibits
> > 
> > I think just "defined to be as strict in clearing bits as possible" --
> > "defined to be as <anything> as they are" is basically a tautology.
> 
> true. fixed.
> 
> >    that a looping packet creates additional packet amplification through
> >    the misconfigured loop on the packets second or further times around
> > 
> > s/packets/packet's/
> 
> fixed.
> > 
> >    Deployments especially where BIER-TE would likely be beneficial may
> >    include operational models where actual configuration changes from
> > 
> > This "especially" seems out of place (I think the sentence would work if
> > it's just dropped, though maybe moving it is preferred).
> 
> hmm.. dropped it.
> > 
> >    the controller are only required during non-productive phases of the
> > 
> > Maybe s/productive/production/?
> 
> yes.
> > 
> >    networks life-cycle, such as in embedded networks or in manufacturing
> > 
> > s/networks/network's/ (just the first one)
> 
> ack
> 
> >    reverting the network/installation into non-productive state.  Such
> > 
> > ("production" again)
> > 
> > 
> 
> -- 
> ---
> tte@cs.fau.de