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

[Adam Montville <[e-mail]>]

Yes, I accept. Sorry about that, and thanks for reaching out again to
wrangle an answer from me.

Adam
On Mon, Jun 12, 2017 at 5:51 PM Lucien Avramov <[e-mail]> wrote:

> 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
>>>
>>>
>>>
>>
>