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

[[e-mail]]

Hi Gabor,

 

Apologies for the tardy reply. All of the comments/suggestions/nits you
submitted look fine, with one exception. In your original e-mail you said:

 

“page 16

   [...]  The behavior of the
   client is to sweep through the given server IP space, sequentially
   generating a recognizable service by the DUT.  

“I am not sure if sequential sweep is the right choice for two reasons:
- I feel that the spirit of RFC 4814 would rather suggest pseudorandom. To
surely include all of them, I recommend random permutation.
- According to my experience, iptables has shown significantly different
maximum connection establishment rate, when I used pseudorandom port numbers
or when I enumerated the port numbers in increasing order. (Details
available upon request.)”

 

We feel that in order to meet the goal of reproducible results that
introducing randomness is not something we want to do.  While we do
understand your position, we hope you can support our desire to no make this
change.

 

Brian



 

From: Gabor LENCSE <[e-mail]> 
Sent: Friday, May 14, 2021 12:56 PM
To: MORTON JR., AL <[e-mail]>; [e-mail]
Cc: 'Bala Balarajah' <[e-mail]>; 'Bala Balarajah'
<[e-mail]>; [e-mail]; 'MORTON, ALFRED C (AL)'
<[e-mail]>; 'Sarah Banks' <[e-mail]>; Gábor LENCSE
<[e-mail]>
Subject: Second part -- Re: [bmwg] FW: WGLC on New version of
draft-ietf-bmwg-ngfw-performance

 

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

bmwg co-chair