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

[Lucien Avramov <[e-mail]>]

Thanks Adam!

Appreciate your feedback!!

On Mon, Jun 12, 2017 at 3:52 PM, Adam Montville <[e-mail]>
wrote:

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