Re: [secdir] SECDIR Review of: draft-ietf-bmwg-dcbench-terminology

[Lucien Avramov <[e-mail]>]

Hi Adam,

Just wanted to check in with you, to see if you accept the way I addressed
your comments.

Thanks in advance!
Lucien

On Thu, Jun 8, 2017 at 10:10 PM, Lucien Avramov <[e-mail]> wrote:

> Hi Adam!
>
> Many thanks for the review! I addressed your comments and published -10 of
> the draft (draft-ietf-bmwg-dcbench-terminology-10).
>
> Please see inline for some comments.
>
> Thanks a lot for the detail feedback!
>
> Lucien
>
> On Mon, Jun 5, 2017 at 10:32 AM, Adam Montville <
> [e-mail]> wrote:
>
>> I have reviewed this document as part of the security directorate's
>> ongoing effort to review all IETF documents being processed by the IESG.
>> These comments were written primarily for the benefit of the security area
>> directors. Document editors and WG chairs should treat these comments just
>> like any other last call comments.
>>
>> Bottom Line: Almost Ready
>>
>> About Security Considerations
>> For your security considerations phrases like "are limited to" and "will
>> be", and I would suggest that these are your MUSTs or SHOULDs. Given the
>> "MUST NOT" you have in the second paragraph, it seems that "MUST" should be
>> used instead of "SHOULD". In that sense, then, I would suggest the
>> following statements be updated: 1) "Benchmarking activities as described
>> in this memo MUST be limited to technology characterization using
>> controlled stimuli...", "The benchmarking network topology MUST be an
>> independent test setup..."
>>
>> That said, you might consider softening the "MUST NOT" and consequent
>> "MUST" statements to "SHOULD NOT" and "SHOULD", respectively. This because
>> there may well be circumstances where testing in a production environment
>> is necessary.
>>
>>
>> *These are the standard security considerations all BMWG uses for all the
> drafts. We want to stay with the considerations we chose. *
>
>
>> About Rest of Draft
>> The draft describes well-understood concepts of FILO, FIFO, LILO, and
>> LIFO. It then goes on to recast these definitions as "first bit last bit"
>> (or "FL"), and "last bit last bit" (or "LL"), and so on. However, the draft
>> does not really make use of these alternate expressions for the
>> aforementioned well-understood concepts. I recommend picking one of these
>> representations and using it consistently throughout; moreover, I recommend
>> sticking with FILO, FIFO, LILO, and LIFO.
>>
>
> *We do want to stay with FILO, FIFO, LILO and LIFO. The FL, LL, etc were
> added as this was sometimes referred to as, not to as FL, LL, etc.*
>
>
>>
>> There's a sentence in 2.2 which reads, "Data Center DUTs are frequently
>> store-and-forward for smaller packet sizes and then adopting a cut-through
>> behavior." This feels incomplete. When does it adopt cut-through behavior?
>> Is that adopted for larger packet sizes? Is it worthwhile to talk at all
>> about the change threshold?
>>
>
> *I addressed your comment by updating the text in the publication to state
> that the threshold change is not matter, what matters is that the test FILO
> is key, in order to catch these differences of behaviors. *
>
>
>
>>
>> Section 1.2 describes the generally expected definition format. However,
>> section 2 seems to immediately depart from that to some degree by placing
>> terms as top-level sections (i.e. "2. Latency") with children for
>> definition, discussion, and measurement units. To compound this confusion,
>> section 6 further departs from the expectation set in 1.2 by using the
>> top-level section as a grouping construct for "Buffering" (without defining
>> "Buffering"). Buffering has two children which are "buffer" and "incast".
>> Buffer, itself is never defined, but instead has a "definition" subsection
>>  consisting of many other terms. The same sort of thing happens with "6.2
>> Incast". Please pick a format and make clear what things are terms, and
>> which are not terms. Then rewrite section 1.2, so that it more properly
>> describes what the reader should expect.
>>
>
> *This is just nesting of the format, Incast is a sub of Buffering. We can
> change them to top level with Buffering – Buffer and Buffering – Incast,
> however we find that less readable for the user and want to keep the
> current display/text the way it is.*
>
>
>>
>> There are, at several locations, uses of the capitalized word "CAN" in a
>> way that suggests normative-type intent. "CAN" does not seem to be an
>> RFC2119 keyword. In one case, it seems that "CAN" may actually mean a lower
>> case "may" or "can" (see 6.1.1 on page 12).
>>
>
>
> *This is fixed, thank you!*
>
>
>> Nits
>> Throughout the draft sometimes terms are capitalized, sometimes they are
>> not. I believe they should more often not be capitalized, but if they are,
>> do so consistently.
>>
>> Generally speaking (this may be stylistic) the first word after a colon
>> (':') should be capitalized. Example: This is.
>>
>
> *This is fixed, thank you!*
>
>>
>> Find acronyms/initializations and ensure they're expanded at least on the
>> first occurrence (i.e. "PDV" on page 6)
>>
>
> Thanks!
>
>>
>> In the abstract s/as it is to/as to/
>>
>> *This is fixed, thank you!*
>
>
>> In 2.1 s/in unit of/in units of/ and s/store forward/store and forward/
>>
>
> *This is fixed, thank you!*
>
>>
>> In 2.3 s/follow:/follows:/
>>
> *This is fixed, thank you!*
>
>>
>> In 2.3, item 2: Is "proceed data" a common term?
>>
> *This is fixed, thank you!*
>
>>
>> In 4.1 s/test on/tests on/
>>
> *This is fixed, thank you!*
>
>>
>> In 4.3 s/of :/of:/
>>
> *This is fixed, thank you!*
>
>>
>> In 4.3 s/[4.1s]/section 4.1/
>>
> *This is fixed, thank you!*
>
>>
>> In 5.3, if "CAN" is changed to "can", then s/line rate can measured/line
>> rate can be measured/
>>
>
> *This is fixed, thank you!*
>
>
>> In 6.1.1 s/expressed in Byte;/B (bytes),/ or s/expressed in Byte;/Bytes,/
>>
> *This is fixed, thank you!*
>
>
>>
>> In 6.1.1 s/amount of buffer a single/amount of buffer for a single/
>>
> *This is fixed, thank you!*
>
>
>>
>> In 6.1.1 recommend striking "it is a burst"
>>
> *This is fixed, thank you!*
>
>
>>
>> In 6.2.1 s/by synchronous/by synchronous arrival time/
>>
> *This is fixed, thank you!*
>
>
>>
>> In 6.2.2 s/In a ingress/In an ingress/
>>
> *This is fixed, thank you!*
>
>
>>
>> In 6.2.2 s/In either cases/In either case/
>>
> *This is fixed, thank you!*
>
>
>>
>> In 6.2.3 s/non null/non-null/
>>
> *This is fixed, thank you!*
>
>
>>
>> In 7.3 the first paragraph could be improved by simply writing, "When S
>> represents the payload bytes, which does not include packet or TCP headers,
>> and Ft is the Finishing Time of the last sender, the Goodput, G, is then
>> measured by the following formula..."
>>
> *This is fixed, thank you!*
>
>
>>
>>
>> Kind regards,
>>
>> Adam
>>
>>
>>
>