[bmwg] Second part -- Re: FW: WGLC on New version of draft-ietf-bmwg-ngfw-performance

[Gabor LENCSE <[e-mail]>]

Dear Al, Authors and BMWG Members,

Al, thank you very much for extending the deadline. However, today I 
managed to finish the review of the draft.

I think this draft is matured enough. I could find only minor formal 
issues in the text about the benchmarking procedures. (Although I kept 
the categories of Discuss/Bugs/Nits, I did not find any significant new 
issue. My entries in the "Discuss" category are either reiterations from 
my previous review, or rather editorial ones.)

Discuss:
--------

page 25
7.1.4.2.

    [...] The test equipment SHOULD start to measure and record
    all specified KPIs and the frequency of measurements SHOULD be less
    than 2 seconds.

What does the "frequency of measurements" mean?
I would expect that the KPIs are mausered continously.
Perhaps it is the "frequency of results produced", that is, the "time 
window for collecting the results and calculating their average" should 
be less than 2 seconds?

------------------

page 26

    The client SHOULD negotiate HTTP 1.1 and close the connection with
    FIN immediately after completion of one transaction.  In each test
    iteration, client MUST send GET command requesting a fixed HTTP
    response object size.

As I mentioned before (I mean: in my previous review), I recommend the 
inclusion of HTTP/2.

------------------

page 27

    c.  During the sustain phase, traffic should be forwarded at a
        constant rate.

- How is "constant rate" defined?
   E.g. the maximum deviation SHOULD be less than 10%, as required at "d."?
- Why "should" is not capitalized here?

------------------

page 28

    The test equipment SHOULD start to measure and record all specified
    KPIs and the frequency of measurements SHOULD be less than 2 seconds.
    Continue the test until all traffic profile phases are completed.

The same comment, as for the "frequency of measurements" on page 25.

------------------

page 30

    b.  Traffic should be forwarded constantly.

The same comments as regarding the "constant rate" on page 27.
 From now on, I will not report it again...

------------------

pages 28-32

Unlike in other cases, HTTP version was not mentioned at all in Section 7.3.
However, I think it is as important here, as in other cases.

------------------

page 31

    [...] The test
    equipment SHOULD start to measure and record all specified KPIs and
    the frequency of measurements SHOULD be less than 2 seconds.

The same comment, as for the "frequency of measurements" on page 25.

 From now on, I will not report it again...

------------------

page 32

    For consistency both the single and multiple transaction test MUST be
    configured with HTTP 1.1.

As I mentioned before, I recommend the inclusion of HTTP/2.
HTTP 1.1 is mentioned again it the next two paragraphs and later many 
times. I will not report it again...


------------------------------------------------------


Bugs:
-----

page 30

                           Table 4: Mixed Objects

There is a "Table 4" on page 11.
(Should be renumbered. Please take care for their citations in the text, 
too!)

------------------

page 31

    The measured KPIs during the sustain phase MUST meet the test results
    validation criteria "a" defined in Section 7.3.3.3.

Why only "a" counts?
There are "b" and "c" too.

------------------

page 34

    results validation criteria a, b, c, d, e and f defined in
    Section 7.4.3.3.

Criterion "f" is undefined, as the last one is "e".

------------------

page 37

    During the sustain phase, the DUT/SUT SHOULD reach the "Initial
    concurrent TCP connections".  The measured KPIs during the sustain
    phase MUST meet the test results validation criteria "a" and "b"
    defined in Section 7.5.3.3.


Why only "a" and "b" counts?
There is "c" too.

------------------

page 44

    The measured KPIs during the sustain phase MUST meet the test results
    validation criteria "a" defined in Section 7.7.3.3.

Why only "a" counts?
There are "b" and "c" too.

------------------

page 47

    results validation criteria a, b, c, d, e and f defined in
    Section 7.8.3.3.

Criterion "f" is undefined, as the last one is "e".

------------------

page 50

    During the sustain phase, the DUT/SUT SHOULD reach the "Initial
    concurrent TCP connections".  The measured KPIs during the sustain
    phase MUST meet the test results validation criteria "a" and "b"
    defined in Section 7.9.3.3.

Why only "a" and "b" counts?
There is "c" too.


------------------------------------------------------


Nits:
-----

page 27

    "initial connections per second" as defined in the parameters
-->
    "Initial connections per second" as defined in the parameters

------------------

page 41


    iteration can be interrupted and the result for 64 KByte SHOULD NOT
    be reported).
-->
    iteration MAY be interrupted and the result for 64 KByte SHOULD NOT
    be reported).

(It is so in Section 7.2.4.2, thus it would be consistent to do so here, 
too)

------------------

Page 43:

Table 5 seems to have the same content as Table 4.
If it is so, it is worth including it twice?

------------------

page 44

    "initial inspected throughput" as defined in the parameters
-->
    "Initial inspected throughput" as defined in the parameters

------------------

page 53

    mode and all detected attack traffic MUST be dropped and the session
    Should be reset
-->
    mode and all detected attack traffic MUST be dropped and the session
    SHOULD be reset.

(perhaps)

------------------

    This document attempts to classify the DUT/SUT in four different four
    different categories based on its maximum supported firewall
-->
    This document attempts to classify the DUT/SUT in four different
    categories based on its maximum supported firewall

("four different" was repeated)

------------------

page 56

    Extra Small (XS) - supported throughput less than 1Gbit/s

    Small (S) - supported throughput less than 5Gbit/s

    Medium (M) - supported throughput greater than 5Gbit/s and less than
    10Gbit/s

    Large (L) - supported throughput greater than 10Gbit/s

Mathematicians could ask: and what if the supported throughput is 
exactly 10Gbit/s? ;-)

That's all.

Best regards,

Gábor


On 5/10/2021 7:34 PM, MORTON JR., AL wrote:
>
> of course Brian and Gábor, I can use the extra time myself.
>
> BMWG,
>
> We will extend the deadline for WGLC to May 21, as requested.
>
> Al
>
> bmwgco-chair
>
>