Re: [tsvwg] Benjamin Kaduk's No Objection on draft-ietf-tsvwg-rlc-fec-scheme-14: (with COMMENT)

Vincent Roca <> Tue, 18 June 2019 08:46 UTC

Return-Path: <>
Received: from localhost (localhost []) by (Postfix) with ESMTP id 375C21200DF; Tue, 18 Jun 2019 01:46:00 -0700 (PDT)
X-Virus-Scanned: amavisd-new at
X-Spam-Flag: NO
X-Spam-Score: -6.899
X-Spam-Status: No, score=-6.899 tagged_above=-999 required=5 tests=[BAYES_00=-1.9, HTML_MESSAGE=0.001, RCVD_IN_DNSWL_HI=-5, SPF_HELO_NONE=0.001, SPF_PASS=-0.001] autolearn=ham autolearn_force=no
Received: from ([]) by localhost ( []) (amavisd-new, port 10024) with ESMTP id J4PaTMvO7S1s; Tue, 18 Jun 2019 01:45:56 -0700 (PDT)
Received: from ( []) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by (Postfix) with ESMTPS id 510C41200F8; Tue, 18 Jun 2019 01:45:55 -0700 (PDT)
X-IronPort-AV: E=Sophos;i="5.63,388,1557180000"; d="scan'208,217";a="387903451"
Received: from ([]) by with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 18 Jun 2019 10:45:53 +0200
From: Vincent Roca <>
Message-Id: <>
Content-Type: multipart/alternative; boundary="Apple-Mail=_CF94A7F0-2347-4F46-8DD2-2D425CB19F22"
Mime-Version: 1.0 (Mac OS X Mail 12.4 \(3445.104.11\))
Date: Tue, 18 Jun 2019 10:45:52 +0200
In-Reply-To: <359EC4B99E040048A7131E0F4E113AFC01B33926C6@marathon>
Cc: Vincent Roca <>, The IESG <>, "" <>, "" <>, "" <>
To: Roman Danyliw <>, Benjamin Kaduk <kaduk@MIT.EDU>
References: <> <> <359EC4B99E040048A7131E0F4E113AFC01B33926C6@marathon>
X-Mailer: Apple Mail (2.3445.104.11)
Archived-At: <>
Subject: Re: [tsvwg] Benjamin Kaduk's No Objection on draft-ietf-tsvwg-rlc-fec-scheme-14: (with COMMENT)
X-Mailman-Version: 2.1.29
Precedence: list
List-Id: Transport Area Working Group <>
List-Unsubscribe: <>, <>
List-Archive: <>
List-Post: <>
List-Help: <>
List-Subscribe: <>, <>
X-List-Received-Date: Tue, 18 Jun 2019 08:46:00 -0000

Hi Roman, Benjamin, all,

Back to the RLC I-D.
I have copied below the Discuss/Comments made for the -14 version of the document.
Many thanks, the document quality has been further improved.

   Vincent and Belkacem


> Roman Danyliw Discuss
> Discuss (2019-05-28)
> A few code nits for Section 3.6 so that the code compiles:
> ** To make the combination of this source code and that in draft-ietf-tsvwg-tinymt32 compile requires that the directive “#include <string.h>” be added (for the memset).
> ** The final return in Section 3.6 is missing a semicolon:
> s/return 0/return 0;/

[VR] Done. I usually don't add system headers to focus on key aspects, but it does not hurt. And thanks for the ";".


> Comment (2019-05-28)
> (1) Section 3.5  Is there any guidance that needs to be provided on the value of the seed to tinymt32_init?

[VR] Good point. Anything is accepted, which is now clearly stated. Since we already discuss Repair_Key management (this repair_key is used as seed), we refer to section 6.1.

   With the FEC Schemes defined in this document, the seed is in
   practice restricted to a value between 0 and 0xFFFF inclusive (note
   that this PRNG accepts a seed value equal to 0), since this is the
   Repair_Key 16-bit field value of the Repair FEC Payload ID
   (Section 4.1.3).  In practice, how to manage the seed and Repair_Key
   values (both are equal) is left to the implementer, using a
   monotonically increasing counter being one possibility (Section 6.1).

> (2) Section 3.6.  In the code comments for cc_nb, I recommend explicitly stating:
> s/number of entries in the table/
> number of entries in the cc_tab[] table/

[VR] Done.

> (3) Section 3.6.  Multiple C code fragments are used in the text.  Somewhere the text should state that the examples are C code and made a reference to what version -- consider C99 (ISO/IEC 9899:1999), C11 (ISO/IEC 9899:2011), C18 (ISO/IEC 9899:2018).

[VR] Yes. I added the same C99 normative reference as in the TinyMT32 I-D
and included some text to clearly state it. I did it for both Sections, 3.5
and 3.6.

NEW (3.6):
   Figure 3 shows the reference generate_coding_coefficients()
   implementation.  This is a C language implementation, written for C99

> (4) Other References Nits
> Section 1.  Please include a reference for FECFRAME on first introduction.

[VR] Done.

> Section 1.1.  What is the reference for Raptor/RaptorQ?

[VR] Added RFC6681.

> (6) Editorial Nits
> Abstract.  No references are permitted in the abstract

[VR] Removed.

> Section 1.1.  Multiple Typos.  s/either a end-/either an end-/g
> Section 1.2.  Extra space. s/constraints) ./constraints)./
> Section 1.4.  Multiple Typos.  s/signalling/signaling/g
> Section 6.2.  Typo.  s/Section Section 3.6/Section 3.6/

[VR] Done. Thanks.


>  Benjamin Kaduk (was Discuss) No Objection
> Comment (2019-05-28)
> Thank you for resolving my Discuss points!  Additionally,
> I really like the reorganization of the document(s) -- it seems to do a
> great job at putting the core protocol/algorithm elements in the
> spotlight, while retaining the interesting discussion about use cases
> and parameter derivation in a less-prominent location.  The introductory
> material for Appendix C also does a great job to frame the subsequent
> discussion.  Thank you!

[VR] You're welcome.
> I agree with Roman that the code snippets should be more clearly marked
> as (a specific version of) C and in the case of
> generate_coding_coefficeints() as a normative part of the protocol
> specification.

[VR] Done.

> Section 3.1
>    Appendix C proposes non normative technics to derive those
>    parameters, depending on the use-case specificities.
> nit: I think "techniques" would be more conventional usage than
> "technics" here.

[VR] Changed.

> Section 3.5
>    Any implementation of this PRNG MUST fulfill three validation
>    criteria: the one described in [tinymt32] (for the TinyMT32 32-bit
>    unsigned integer generator), and the two others detailed in
>    Appendix A (for the mapping to 4-bit and 8-bit intervals).  [...]
> As I noted on the tinymt32 document, this phrasing is a bit odd, though
> it feels less out of place here than in the tinymt32 document.

[VR] Yes, and I updated this I-D in the same way.

   Any implementation of this PRNG MUST have the same output as that
   provided by the reference implementation of [tinymt32].  In order to
   increase the compliancy confidence, three criteria are proposed: the
   one described in [tinymt32] (for the TinyMT32 32-bit unsigned integer
   generator), and the two others detailed in Appendix A (for the
   mapping to 4-bit and 8-bit intervals). [...]

> Section 3.6
>     * (in/out) cc_tab[]  pointer to a table of the right size to store
>     *                    coding coefficients. All coefficients are
>     *                    stored as bytes, regardless of the m parameter,
>     *                    upon return of this function.
> Assuming this is C, it's perhaps a bit jarring to spell the function
> parameter as "cc_tab[]" but describe it as a pointer.  (In either case,
> the encoding for the function call will be that of a pointer, of course,
> so the array syntax seems needlessly confusing.)

[VR] "uint8_t* cc_tab" and "uint8_t cc_tab[]" declarations are equivalent.
        Both are pointers to uint8_t elements so there's nothing wrong.
        However I see your point and changed for the explicit pointer

> Section 3.7.1
>    The two RLC FEC Schemes specified in this document reuse the Finite
>    Fields defined in [RFC5510], section 8.1.  More specifically, the
>    [...]
>    The following irreducible polynomial MUST be used for GF(2^^8):
>       x^^8 + x^^4 + x^^3 + x^^2 + 1
> Given the former chunk, the normative "MUST" in the second chunk is
> redundant, since the authoritative source for this polynomial is the
> cited portion of RFC 5510.

[VR] Removed the second upper case MUST.

   The following irreducible polynomial is used for GF(2^^8):
      x^^8 + x^^4 + x^^3 + x^^2 + 1

> Section 3.7.2
>              For each byte of position i in each source and the repair
>    symbol, where i belongs to {0; E-1}, compute:
> nit: do we expect the usage of the curly braces {} to indicate a closed
> interval to be well-understood by the readership (as opposed to square
> brackets [])?

[VR] Well it depends. In France, it seems preferable to use {...} for an
enumeration, and [] for a span, but conventions may differ in other
countries. Given you comment, I've changed for [].

>                                       The XOR sum of the byte of
>    position i in each source is computed and stored in the corresponding
>    byte of the repair symbol, where i belongs to {0; E-1}.  […]
> (ditto)

[VR] Done.

> Section 6.1
> I might add a note that the scheme for chosing the repair key is
> entirely at the discretion of the sender, since it is communicated in
> the wire format to the receiver(s), and that the sequential counter is
> presented as the simplest method that ensures new coefficients for each
> repair packet.  As already mentioned, other schemes are possible, and we
> definitely don't want any receivers hardcoding the assumption of
> sequentially incrementing counter.

[VR] Yes, definitively. It even didn't come to my mind that a receiver
may hardcode the way the repair key is managed. I've further reinforced

   In any case, chosing the repair key
   is entirely at the discretion of the sender, since it is communicated
   to the receiver(s) in each Repair FEC Payload ID.  A receiver should
   not make any assumption on the way the repair key is managed.

> Section 9.1
>                                               In case of small encoding
>    windows, the associated processing overhead is not an issue (e.g., we
>    measured decoding speeds between 745 Mbps and 2.8 Gbps on an ARM
>    Cortex-A15 embedded board in [Roca17] for an encoding window of size
>    18 or 23 symbols).  [...]
> Just to double-check: does the 2.8 Gbps correspond to the window of 18
> symbols or 23?

[VR] Not exactly, it essentially depends on the code rate (the higher,
the faster) and channel conditions, in addition to the ew_size that is
determined by the code rate.
It's well described in the paper and I don't want to put all details here,
however this is misleading and I've changed the wording.

   In case of small encoding
   windows, the associated processing overhead is not an issue (e.g., we
   measured decoding speeds between 745 Mbps and 2.8 Gbps on an ARM
   Cortex-A15 embedded board in [Roca17] depending on the code rate and
   the channel conditions, using an encoding window of size 18 or 23
   symbols; see the above article for the details).

> Appendix A
> Please note whether the numbers should be read out column-first or
> row-first.

[VR] Done.

> I refer again to my comment on the tinymt32 document regarding the
> normative "MUST" language here vs. stating that these are test vectors.

[VR] Done.

> Appendix B
> side note: I don't propose to have you redo the analysis, but looking at
> the small sequences generated using seed values from 0 to 1,000,000 - 1
> is not terribly representative of the FEC usage, since the seed values
> there are limited to the range from 0 to 16k.  Comprehensive statistics
> on the 16k possible sequences (corresponding to the 16k possible repair
> keys) of 20 pseudo-random numbers would be more compelling.
> Also, for the scenario presented in Figure 12, I'm not sure I agree with
> the claim that "although the distribution is not perfect, there is no
> major bias".  Not in that I think there is bias, but in that I think
> that we should not expect the average occurrences column to be a
> constant -- the numbers here seem largely within the expected sort of
> variances for this type of sampling.

[VR] I need to give it a second thought... I've submitted -15 without
considering this comment and will see how to adress it in -16.

> Appendix D
>    Finally, it is worth noting that a receiver that benefits from an FEC
>    protection significantly higher than what is required to recover from
>    packet losses, can choose to reduce the ls_max_size.  In that case
>    lost ADUs will be recovered without relying on this optimization.
> nit(?): It's not entirely clear to me that this statement is well-placed
> in its current location.

[VR] I think it's well placed. We talk about ls_max_size and here we
say that using a too large value is not necessarily meaningful.