Bundle Protocol Version 7
draft-ietf-dtn-bpbis-30

Summary: Has enough positions to pass.

(Mirja Kühlewind) Discuss

Discuss (2020-02-03 for -21)
I looked up RFC 4838 and there is a section on congestion control, however it only says:
"Congestion control is an ongoing research topic."
Unfortunately this document also doesn't give any further advise about congestion control but as a PS it really should. I understand that the bundle protocol is basically an application layer protocol on top of a transport that should care about congestion control, however, the document doesn't talk much about anything related to that underlying protocol. It would be important to specify requirements for the underlying transport protocol, indicating that is must be congestion controlled or rate limited (see RFC8085 as a reference for rate limiting of (uni-direction) UDP-based protocols).

Further this sentence in Sec 5.1 needs more clarification:
“For this reason, the generation of status reports MUST be
   disabled by default and enabled only when the risk of excessive
   network traffic is deemed acceptable.”
It is not clear when it makes sense to enable and if enables one should probably also implement some filtering and rate limiting.
Comment (2020-02-03 for -21)
1) One general comment on the style of the document: I found it actually quite hard to read. I think it would help to add some overview text from time to time instead of going into all the details right away. E.g. maybe provide some kind of short summary/overview at the beginning of section 5 like the “Life cycle of a bundle“. In contrast I think you could cut down the section on concepts a bit (move some the implementation related notes to the appendix). Further, compared to the previous RFC, now that CBOR is used the text because actually more abstract. I think it would actually help to put CDDL fragment in body of document.

2) Sec 4.1.3 and 4.1.4: Maybe use normative language here for all or most of the musts, e.g. 
s/then all status report request flag values must be zero/then all status report request flag values MUST be zero/

3) Quick question on Sec 4.2.2:
“Version number SHALL be
   represented as a CBOR unsigned integer item.”
The document says that this version is not backward compatible. Is the version number the only field that can be used to identify the bundle protocol?

Magnus Westerlund (was Discuss, Yes) Yes

Deborah Brungard No Objection

Alissa Cooper No Objection

Comment (2020-02-06 for -22)
I support Benjamin's first DISCUSS point.

The registration templates in 10.7 and 10.8 should include iesg@ietf.org as the contact information for the change controller.

Roman Danyliw (was Discuss) No Objection

Comment (2020-02-24 for -23)
No email
send info
Thank you for addressing my DISCUSS and COMMENTs items.

Martin Duke No Objection

Comment (2020-11-24 for -29)
Sec 4.1.5.1.2 I though this section was all about endpoint IDs. So what are the "all other purposes" that involve ASCII representations?

Sec 7. Please add "congestion control" to the list of services the CL provides.

Sec 10. There are many instances in these registries of a codepoint only applying to Version 6, but including 'RFC-to-be' as a reference. Is this a mistake, or am I missing something?

Benjamin Kaduk (was Discuss) No Objection

Comment (2020-12-02 for -29)
No email
send info
[updating my position for the -29, but no further actions are expected
after my comments on the -27]

Section 10.7

I assume that as part of the IANA processing, we will have this document 
added as a reference for the updated "dtn" registration.  (Whether or
not that change is specifically mentioned in the final RFC is probably
not of great import.)

Erik Kline No Objection

Comment (2020-12-02 for -29)
I was going to ballot as Discuss, but I lack some context on the history
of this document and its evolution.  Nevertheless, hopefully the "discuss"
points below leave a record of things about which I had some concern.


[[ discuss ]]

[ section 4.1.5.1.2 ]

* The encoding considerations mentions "dtn" scheme but this is the "ipn"
  scheme section so...should this be "ipn"?

[ section 5.2 ]

* Should step 2 be step 1 of 5.3 (not 5.4)?

[ section 5.4.2 ]

* It seems to me that the behaviour in step 1, which I think does make some
  sense, pretty easily sets up the potential for ping-ponging (AIUI).

  If true, should there be some text (perhaps in 5.4) acknowledging that
  forwarding a given bundle to a node for the 2nd time might warrant, at least,
  some reassessment of the routing/reachability state in the node?

  I fully understand that extensive routing discussion is out of scope
  for this document.

[ section 5.6 ]

* Does "cannot process" include "does not understand"?

  If so it might be good to use a different reason value from
  "Block unintelligible" so that some other node can understand that, for
  example, the CRC (if sent) was valid.

  Basically, consider differentiating between "understood but malformed" and
  "not understood".

[ section 5.9 ]

* The mention of non-overlapping fragments brings a few issues to mind that
  have been encountered in IPv6 when considering fragment handling:

    [1] How are overlapping fragments to be handled?  Are they ignored
    and/or cause any specific error to be generated? (vis. IPv6: RFC 5722)

    [2] What about "atomic fragments", i.e. a fragment that starts at
    offset zero and has a total payload length equal to the actual length
    of the payload (i.e. a fragmented payload consisting of a single fragment)?
    (vis. IPv6: RFC 6949)

    I'm guessing these should be silently accepted, but there might be some
    text saying they MUST/SHOULD NOT be generated.


[[ comments ]]

[ section 4.2.2 ]

* The Creation Timestamp element description seems to copy a bunch of text
  from 4.1.7.  Might be able to shorten the document a bit by referring to
  4.1.7 for extended discussion of creation timestamp operations.


[[ questions ]]

[ section 4.3.3 ]

* What's the value of using a {limit, incrementing_value} pair like this over
  just a single decrements_to_zero integer?  I'm not suggesting this be
  changed (it's probably too late anyway), but I was just curious as to the
  motivation.

(Suresh Krishnan) No Objection

Warren Kumari No Objection

Barry Leiba No Objection

Comment (2020-02-05 for -22)
I support the Sec ADs’ DISCUSS and comment points.  I have only a small thing to add to what’s already been said:

— Section 1 —
You mention RFC5050 several times here, but it isn’t actually cited as a reference until Section 10.1.  You would normally put it in as a citation the first time, at the beginning of Section 1, and I think you should.

(Alexey Melnikov) (was Discuss) No Objection

Comment (2020-02-20 for -23)
No email
send info
I am clearing my DISCUSS with understanding that future work will be done in this area:

1) Retry strategies are listed as out-of-scope for this document. This is not sufficient for writing an interoperable implementation, unless there is a separate document that provides such details.
Below I describe 1 remaining relevant place in the text and suggest some possible ways of addressing my DISCUSS:

   Step 5: When all selected convergence layer adapters have informed
   the bundle protocol agent that they have concluded their data
   sending procedures with regard to this bundle, processing may depend
   on the results of those procedures.  If completion of the data
   sending procedures by all selected convergence layer adapters has
   not resulted in successful forwarding of the bundle (an
   implementation-specific determination that is beyond the scope of
   this specification), then the bundle protocol agent MAY choose (in
   an implementation-specific manner, again beyond the scope of this
   specification) to initiate another attempt to forward the bundle.

Similar to the above: retries affect interoperability and should be documented
as description of a convergence layer document.
I think you need to add this as a requirement to section 7 ("Services Required of the Convergence Layer").

   In that event, processing proceeds from Step 4 of Section 5.4.


**********************************************************************
* Note, that I am conducting an experiment when people aspiring to be*
* Area Directors get exposed to AD work ("AD shadowing experiment"). *
* As a part of this experiment they get to review documents on IESG  *
* telechats according to IESG Discuss criteria document and their    *
* comments get relayed pretty much verbatim to relevant editors/WGs. *
* As an AD I retain responsibility in defending their position when  *
* I agree with it.                                                   *
* Recipients of these reviews are encouraged to reply to me directly * 
* about perceived successes or failures of this experiment.          *
**********************************************************************

I also have some comments on Benjamin's comments below marked with "[[Alexey]]:"


The following comments were provided by Benjamin Schwartz <bemasc@google.com>:

Benjamin would have balloted *DISCUSS* on this document. He wrote:
This draft’s content seems sound but the text is difficult to understand and needs some editorial improvements.


## Abstract

Nit: Abstract should be before status.
Nit: “specification for Bundle Protocol” -> “for the Bundle Protocol”.

## Section 1

Title: If this document doesn’t describe the operation, routing, or management of bundles, maybe it doesn’t describe a complete protocol.  Consider retitling to “Bundle Protocol Version 7: Format and Architecture”

[[Alexey]]: This relates to my DISCUSS points, but my DISCUSS position is stronger than this statement.

## Section 4.1.3

Nit: It would be clearer to move the reserved and unassigned flags to an IANA considerations section, here and throughout.

## Section 4

Phrasing: “The last item of the array ... SHALL be a CBOR "break" stop code.”  This seems like a misuse of the word “item”.  I would suggest “The array SHALL be terminated by a CBOR “break” stop code.”  Similarly in Section 4.2.1.

## Section 4.1.4

Clarity: The “(Boolean)” specifiers seem redundant.  What is a non-boolean flag?

Clarity: The two lists here seem redundant.  Can they be collapsed?

Clarity: The phrasing “...the value of the "Transmit status report if block can't be processed" flag in every canonical block…” suggests that these flags should be named or numbered before attempting to describe them.

## Section 4.1.5.1

These two sentences seem contradictory:

* “Any scheme conforming to [URIREG] may be used in a bundle protocol endpoint ID.”
* “The first item of the array SHALL be the code number identifying the endpoint's URI scheme [URI], as defined in the registry of URI scheme code numbers”

Please rephrase.

[[Alexey]]: This is similar to Roman's DISCUSS.

## Section 5.4.2

Consistency: This section relies on the presence of a Previous Node block, but nothing in the forwarding procedure instructs any agent to add a Previous Node block.

Correctness: If two nodes both opt to return failed bundles, how are they to avoid a ping-pong loop?

[[Alexey]]: I included these in the DISCUSS portion of my ballot.

##Section 5.6

Error handling: What about CBOR parsing failures?

[[Alexey]]: I included these in the DISCUSS portion of my ballot.

## Section 5.8

Clarity: The insistence, here and earlier, that a bundle is only fragmented into two packets, and that this can then be performed recursively, seems unhelpful.  Bundles can be fragmented into arbitrarily many pieces, and this binary subdivision concept is not actually present in the protocol.  I suggest removing it from the text as well.

## Section 6.1

Formatting: This table is excessive for a single value.

Clarity: The text specifies the contents twice, which is unhelpful for comprehension.

## Section 10

Formatting: The extra line breaks in the vertical dividers are distracting.  Please fill them in or remove the breaks if possible.

Alvaro Retana (was Discuss) No Objection

(Adam Roach) No Objection

Éric Vyncke No Objection

Comment (2020-02-03 for -21)
Thank you for the work put into this document. And it is a really interesting topic !

The document is not really easy to read with some long and convoluted sentence such as
  "An application data unit is the unit
   of data whose conveyance to the bundle's destination is the purpose
   for the transmission of some bundle that is not a fragment (as
   defined below)."

Another one:
  "The payload for a bundle
   forwarded in response to reception of a bundle is the payload of the
   received bundle."

The above sentences and others make the reading really difficult. Probably too late to change now... 

The section 3.2 go a little too deep IMHO in details (do we care whether the BPA is general-purpose computer?)

As a side note, I would be interested by the potential use of this bundle protocol to transport the future webpack ;-)

Please find below some non-blocking COMMENTs.
C1) is there any reason why the reserved bits in section 4.3.1 are not specified with the usual 'transmitted as 0 and ignored by receiver' ?
C2) in section 4.1.5, is there a registry or a mechanism to avoid / detect identifier collisions ?
C3) just curious, why is it version 7 ? 
C4) in section 4.3.2, if the bundle age is expressed in microseconds, then I would suggest to define exactly 'most recently forwarded': by which component of the bundle node ? is it measured at the completion of the action (e.g., layer-2 serialization is finished). Section 5.4 adds some more information but not enough to my taste (is it when the forwarding decision is made or when the convergence layer has accepted it).
C5) in section 4.3.3., why not following the usual method of hop-limit starting at a non-zero value and being decremented at each forwarding operation ? it is like using DTN time from 2000 rather than the usual timestamp definition. Nothing bad or blocking, but, I wonder why re-inventing the wheel if there is no real reason
C6) the possibility of having a single bundle to generate multiple reports along its path to the 'report-to' endpoint ID per section 5.6 could possibly be used for an amplification attacks + resource exhaustion attack on traversed nodes. Should a rate limiting be mentionned ?
C7) section 5.9, should the re-assembly process time out ? E.g., taking into account the max bundle age ?
C8) section 8, I have hard time to parse the following "the Bundle Protocol version 7 specification draft (version 6)" even if I think I got it right
C9) section 8, an interop status (if any) between the three existing implementations would be welcome

I hope that this helps to improve the document,

Regards,

-éric

Robert Wilton No Objection

Comment (2020-12-01 for -29)
Hi,

Thank you for this document.

Just a few minor observations/comments.  Quite possibly only the last comment is actionable, and the rest are just observations.

1) Figure 2: Components of a Bundle Node

It would be nice if this diagram is not split across page boundaries, hopefully the RFC editor can sort this out.

2) 4. Bundle Format

   Each bundle SHALL be a concatenated sequence of at least two blocks,
   represented as a CBOR indefinite-length array
   
It wasn't immediate obvious why this restriction was here, I presume that this is to ensure that there is a canonical representation?

4.1.1. CRC Type
I was surprised to see that CRC-16 is also supported.  I presume that reasoning for this is because blocks could be small, and hence the overhead from always using CRC32 was regarded as being too much?
 
4.1.2. CRC
I was also slightly surprised that these are encoded as fixed length byte strings rather than as unsigned integers.  I presume that this is done to make them fixed length and also easier to zero out when performing the CRC calculation?

It looks like RFC 2119 language used to define the bundle format both in section 4 and 4.2.1.  Possibly it would be better to not use RFC 2119 language in the intro of section 4, and instead refer to 4.2.1 for the normative definition?

Regards,
Rob

Murray Kucherawy No Record

Martin Vigoureux No Record