openssl s_time: refactor benchmark loops

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

openssl s_time: refactor benchmark loops

Scott Cheloha
Hi,

s_time -- everyone's perennial pick for favorite openssl app -- has a
lot of copy & paste.  Here's a first shot at refactoring that away.

We can merge the two benchmark loops, the timing stuff, and the report
printing, as all of it is nearly identical.

I modified as little as possible when moving and merging things so it's
obvious to the reader that the behavior is completely unchanged.  There's
cleanup and minor bugfixing I'd like to do, but that belongs in a separate
diff to keep this review easy.

Thoughts & feedback?  ok?

--
Scott Cheloha

Index: usr.bin/openssl/s_time.c
===================================================================
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.24
diff -u -p -r1.24 s_time.c
--- usr.bin/openssl/s_time.c 13 Jul 2018 18:36:56 -0000 1.24
+++ usr.bin/openssl/s_time.c 9 Aug 2018 22:45:20 -0000
@@ -91,6 +91,7 @@ extern int verify_depth;
 
 static void s_time_usage(void);
 static SSL *doConnection(SSL * scon);
+static int benchmark(int);
 
 static SSL_CTX *tm_ctx = NULL;
 static const SSL_METHOD *s_time_meth = NULL;
@@ -244,13 +245,7 @@ tm_Time_F(int op)
 int
 s_time_main(int argc, char **argv)
 {
- double totalTime = 0.0;
- int nConn = 0;
- SSL *scon = NULL;
- time_t finishtime;
  int ret = 1;
- char buf[1024 * 8];
- int ver;
 
  if (single_execution) {
  if (pledge("stdio rpath inet dns", NULL) == -1) {
@@ -328,60 +323,8 @@ s_time_main(int argc, char **argv)
 
  /* Loop and time how long it takes to make connections */
 
- bytes_read = 0;
- finishtime = time(NULL) + s_time_config.maxtime;
- tm_Time_F(START);
- for (;;) {
- if (finishtime < time(NULL))
- break;
- if ((scon = doConnection(NULL)) == NULL)
- goto end;
-
- if (s_time_config.www_path != NULL) {
- int i, retval = snprintf(buf, sizeof buf,
-    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
- if ((size_t)retval >= sizeof buf) {
- fprintf(stderr, "URL too long\n");
- goto end;
- }
- SSL_write(scon, buf, strlen(buf));
- while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
- bytes_read += i;
- }
- if (s_time_config.no_shutdown)
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
-    SSL_RECEIVED_SHUTDOWN);
- else
- SSL_shutdown(scon);
-
- nConn += 1;
- if (SSL_session_reused(scon))
- ver = 'r';
- else {
- ver = SSL_version(scon);
- if (ver == TLS1_VERSION)
- ver = 't';
- else if (ver == SSL3_VERSION)
- ver = '3';
- else if (ver == SSL2_VERSION)
- ver = '2';
- else
- ver = '*';
- }
- fputc(ver, stdout);
- fflush(stdout);
-
- SSL_free(scon);
- scon = NULL;
- }
- totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
-
- printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n",
-    nConn, totalTime, ((double) nConn / totalTime), bytes_read);
- printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
-    nConn,
-    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
-    bytes_read / nConn);
+ if (benchmark(0))
+ goto end;
 
  /*
  * Now loop and time connections using the same session id over and
@@ -393,88 +336,11 @@ s_time_main(int argc, char **argv)
  goto end;
  printf("\n\nNow timing with session id reuse.\n");
 
- /* Get an SSL object so we can reuse the session id */
- if ((scon = doConnection(NULL)) == NULL) {
- fprintf(stderr, "Unable to get connection\n");
+ if (benchmark(1))
  goto end;
- }
- if (s_time_config.www_path != NULL) {
- int retval = snprintf(buf, sizeof buf,
-    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
- if ((size_t)retval >= sizeof buf) {
- fprintf(stderr, "URL too long\n");
- goto end;
- }
- SSL_write(scon, buf, strlen(buf));
- while (SSL_read(scon, buf, sizeof(buf)) > 0);
- }
- if (s_time_config.no_shutdown)
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
-    SSL_RECEIVED_SHUTDOWN);
- else
- SSL_shutdown(scon);
-
- nConn = 0;
- totalTime = 0.0;
-
- finishtime = time(NULL) + s_time_config.maxtime;
-
- printf("starting\n");
- bytes_read = 0;
- tm_Time_F(START);
-
- for (;;) {
- if (finishtime < time(NULL))
- break;
- if ((doConnection(scon)) == NULL)
- goto end;
-
- if (s_time_config.www_path) {
- int i, retval = snprintf(buf, sizeof buf,
-    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
- if ((size_t)retval >= sizeof buf) {
- fprintf(stderr, "URL too long\n");
- goto end;
- }
- SSL_write(scon, buf, strlen(buf));
- while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
- bytes_read += i;
- }
- if (s_time_config.no_shutdown)
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
-    SSL_RECEIVED_SHUTDOWN);
- else
- SSL_shutdown(scon);
-
- nConn += 1;
- if (SSL_session_reused(scon))
- ver = 'r';
- else {
- ver = SSL_version(scon);
- if (ver == TLS1_VERSION)
- ver = 't';
- else if (ver == SSL3_VERSION)
- ver = '3';
- else if (ver == SSL2_VERSION)
- ver = '2';
- else
- ver = '*';
- }
- fputc(ver, stdout);
- fflush(stdout);
- }
- totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
-
- printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n", nConn, totalTime, ((double) nConn / totalTime), bytes_read);
- printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
-    nConn,
-    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
-    bytes_read / nConn);
 
  ret = 0;
  end:
- SSL_free(scon);
-
  if (tm_ctx != NULL) {
  SSL_CTX_free(tm_ctx);
  tm_ctx = NULL;
@@ -541,4 +407,111 @@ doConnection(SSL * scon)
  return NULL;
  }
  return serverCon;
+}
+
+static int
+benchmark(int reuse_session)
+{
+ double totalTime = 0.0;
+ int nConn = 0;
+ SSL *scon = NULL;
+ time_t finishtime;
+ int ret = 1;
+ char buf[1024 * 8];
+ int ver;
+
+ if (reuse_session) {
+ /* Get an SSL object so we can reuse the session id */
+ if ((scon = doConnection(NULL)) == NULL) {
+ fprintf(stderr, "Unable to get connection\n");
+ goto end;
+ }
+ if (s_time_config.www_path != NULL) {
+ int retval = snprintf(buf, sizeof buf,
+    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
+ if ((size_t)retval >= sizeof buf) {
+ fprintf(stderr, "URL too long\n");
+ goto end;
+ }
+ SSL_write(scon, buf, strlen(buf));
+ while (SSL_read(scon, buf, sizeof(buf)) > 0);
+ }
+ if (s_time_config.no_shutdown)
+ SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
+    SSL_RECEIVED_SHUTDOWN);
+ else
+ SSL_shutdown(scon);
+ shutdown(SSL_get_fd(scon), SHUT_RDWR);
+ close(SSL_get_fd(scon));
+ printf("starting\n");
+ }
+
+ nConn = 0;
+ totalTime = 0.0;
+
+ finishtime = time(NULL) + s_time_config.maxtime;
+
+ bytes_read = 0;
+ tm_Time_F(START);
+
+ for (;;) {
+ if (finishtime < time(NULL))
+ break;
+ if ((scon = doConnection(reuse_session ? scon : NULL)) == NULL)
+ goto end;
+
+ if (s_time_config.www_path) {
+ int i, retval = snprintf(buf, sizeof buf,
+    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
+ if ((size_t)retval >= sizeof buf) {
+ fprintf(stderr, "URL too long\n");
+ goto end;
+ }
+ SSL_write(scon, buf, strlen(buf));
+ while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
+ bytes_read += i;
+ }
+ if (s_time_config.no_shutdown)
+ SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
+    SSL_RECEIVED_SHUTDOWN);
+ else
+ SSL_shutdown(scon);
+ shutdown(SSL_get_fd(scon), SHUT_RDWR);
+ close(SSL_get_fd(scon));
+
+ nConn += 1;
+ if (SSL_session_reused(scon))
+ ver = 'r';
+ else {
+ ver = SSL_version(scon);
+ if (ver == TLS1_VERSION)
+ ver = 't';
+ else if (ver == SSL3_VERSION)
+ ver = '3';
+ else if (ver == SSL2_VERSION)
+ ver = '2';
+ else
+ ver = '*';
+ }
+ fputc(ver, stdout);
+ fflush(stdout);
+
+ if (!reuse_session) {
+ SSL_free(scon);
+ scon = NULL;
+ }
+ }
+ totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
+
+ printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n",
+    nConn, totalTime, ((double) nConn / totalTime), bytes_read);
+ printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
+    nConn,
+    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
+    bytes_read / nConn);
+
+ ret = 0;
+end:
+ SSL_free(scon);
+ return ret;
 }

Reply | Threaded
Open this post in threaded view
|

Re: openssl s_time: refactor benchmark loops

kinichiro inoguchi
Hi,

I saw that 2 loops are almost identical, and I think diff tidy up these 2 loops
in 1 function.

I found these 2 lines were not in original code.

> + shutdown(SSL_get_fd(scon), SHUT_RDWR);
> + close(SSL_get_fd(scon));

Is this part needed ?

Kinichiro Inoguchi

On Thu, Aug 09, 2018 at 06:00:23PM -0500, Scott Cheloha wrote:

> Hi,
>
> s_time -- everyone's perennial pick for favorite openssl app -- has a
> lot of copy & paste.  Here's a first shot at refactoring that away.
>
> We can merge the two benchmark loops, the timing stuff, and the report
> printing, as all of it is nearly identical.
>
> I modified as little as possible when moving and merging things so it's
> obvious to the reader that the behavior is completely unchanged.  There's
> cleanup and minor bugfixing I'd like to do, but that belongs in a separate
> diff to keep this review easy.
>
> Thoughts & feedback?  ok?
>
> --
> Scott Cheloha
>
> Index: usr.bin/openssl/s_time.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 s_time.c
> --- usr.bin/openssl/s_time.c 13 Jul 2018 18:36:56 -0000 1.24
> +++ usr.bin/openssl/s_time.c 9 Aug 2018 22:45:20 -0000
> @@ -91,6 +91,7 @@ extern int verify_depth;
>  
>  static void s_time_usage(void);
>  static SSL *doConnection(SSL * scon);
> +static int benchmark(int);
>  
>  static SSL_CTX *tm_ctx = NULL;
>  static const SSL_METHOD *s_time_meth = NULL;
> @@ -244,13 +245,7 @@ tm_Time_F(int op)
>  int
>  s_time_main(int argc, char **argv)
>  {
> - double totalTime = 0.0;
> - int nConn = 0;
> - SSL *scon = NULL;
> - time_t finishtime;
>   int ret = 1;
> - char buf[1024 * 8];
> - int ver;
>  
>   if (single_execution) {
>   if (pledge("stdio rpath inet dns", NULL) == -1) {
> @@ -328,60 +323,8 @@ s_time_main(int argc, char **argv)
>  
>   /* Loop and time how long it takes to make connections */
>  
> - bytes_read = 0;
> - finishtime = time(NULL) + s_time_config.maxtime;
> - tm_Time_F(START);
> - for (;;) {
> - if (finishtime < time(NULL))
> - break;
> - if ((scon = doConnection(NULL)) == NULL)
> - goto end;
> -
> - if (s_time_config.www_path != NULL) {
> - int i, retval = snprintf(buf, sizeof buf,
> -    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
> - if ((size_t)retval >= sizeof buf) {
> - fprintf(stderr, "URL too long\n");
> - goto end;
> - }
> - SSL_write(scon, buf, strlen(buf));
> - while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
> - bytes_read += i;
> - }
> - if (s_time_config.no_shutdown)
> - SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
> -    SSL_RECEIVED_SHUTDOWN);
> - else
> - SSL_shutdown(scon);
> -
> - nConn += 1;
> - if (SSL_session_reused(scon))
> - ver = 'r';
> - else {
> - ver = SSL_version(scon);
> - if (ver == TLS1_VERSION)
> - ver = 't';
> - else if (ver == SSL3_VERSION)
> - ver = '3';
> - else if (ver == SSL2_VERSION)
> - ver = '2';
> - else
> - ver = '*';
> - }
> - fputc(ver, stdout);
> - fflush(stdout);
> -
> - SSL_free(scon);
> - scon = NULL;
> - }
> - totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
> -
> - printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n",
> -    nConn, totalTime, ((double) nConn / totalTime), bytes_read);
> - printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
> -    nConn,
> -    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
> -    bytes_read / nConn);
> + if (benchmark(0))
> + goto end;
>  
>   /*
>   * Now loop and time connections using the same session id over and
> @@ -393,88 +336,11 @@ s_time_main(int argc, char **argv)
>   goto end;
>   printf("\n\nNow timing with session id reuse.\n");
>  
> - /* Get an SSL object so we can reuse the session id */
> - if ((scon = doConnection(NULL)) == NULL) {
> - fprintf(stderr, "Unable to get connection\n");
> + if (benchmark(1))
>   goto end;
> - }
> - if (s_time_config.www_path != NULL) {
> - int retval = snprintf(buf, sizeof buf,
> -    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
> - if ((size_t)retval >= sizeof buf) {
> - fprintf(stderr, "URL too long\n");
> - goto end;
> - }
> - SSL_write(scon, buf, strlen(buf));
> - while (SSL_read(scon, buf, sizeof(buf)) > 0);
> - }
> - if (s_time_config.no_shutdown)
> - SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
> -    SSL_RECEIVED_SHUTDOWN);
> - else
> - SSL_shutdown(scon);
> -
> - nConn = 0;
> - totalTime = 0.0;
> -
> - finishtime = time(NULL) + s_time_config.maxtime;
> -
> - printf("starting\n");
> - bytes_read = 0;
> - tm_Time_F(START);
> -
> - for (;;) {
> - if (finishtime < time(NULL))
> - break;
> - if ((doConnection(scon)) == NULL)
> - goto end;
> -
> - if (s_time_config.www_path) {
> - int i, retval = snprintf(buf, sizeof buf,
> -    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
> - if ((size_t)retval >= sizeof buf) {
> - fprintf(stderr, "URL too long\n");
> - goto end;
> - }
> - SSL_write(scon, buf, strlen(buf));
> - while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
> - bytes_read += i;
> - }
> - if (s_time_config.no_shutdown)
> - SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
> -    SSL_RECEIVED_SHUTDOWN);
> - else
> - SSL_shutdown(scon);
> -
> - nConn += 1;
> - if (SSL_session_reused(scon))
> - ver = 'r';
> - else {
> - ver = SSL_version(scon);
> - if (ver == TLS1_VERSION)
> - ver = 't';
> - else if (ver == SSL3_VERSION)
> - ver = '3';
> - else if (ver == SSL2_VERSION)
> - ver = '2';
> - else
> - ver = '*';
> - }
> - fputc(ver, stdout);
> - fflush(stdout);
> - }
> - totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
> -
> - printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n", nConn, totalTime, ((double) nConn / totalTime), bytes_read);
> - printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
> -    nConn,
> -    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
> -    bytes_read / nConn);
>  
>   ret = 0;
>   end:
> - SSL_free(scon);
> -
>   if (tm_ctx != NULL) {
>   SSL_CTX_free(tm_ctx);
>   tm_ctx = NULL;
> @@ -541,4 +407,111 @@ doConnection(SSL * scon)
>   return NULL;
>   }
>   return serverCon;
> +}
> +
> +static int
> +benchmark(int reuse_session)
> +{
> + double totalTime = 0.0;
> + int nConn = 0;
> + SSL *scon = NULL;
> + time_t finishtime;
> + int ret = 1;
> + char buf[1024 * 8];
> + int ver;
> +
> + if (reuse_session) {
> + /* Get an SSL object so we can reuse the session id */
> + if ((scon = doConnection(NULL)) == NULL) {
> + fprintf(stderr, "Unable to get connection\n");
> + goto end;
> + }
> + if (s_time_config.www_path != NULL) {
> + int retval = snprintf(buf, sizeof buf,
> +    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
> + if ((size_t)retval >= sizeof buf) {
> + fprintf(stderr, "URL too long\n");
> + goto end;
> + }
> + SSL_write(scon, buf, strlen(buf));
> + while (SSL_read(scon, buf, sizeof(buf)) > 0);
> + }
> + if (s_time_config.no_shutdown)
> + SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
> +    SSL_RECEIVED_SHUTDOWN);
> + else
> + SSL_shutdown(scon);
> + shutdown(SSL_get_fd(scon), SHUT_RDWR);
> + close(SSL_get_fd(scon));
> + printf("starting\n");
> + }
> +
> + nConn = 0;
> + totalTime = 0.0;
> +
> + finishtime = time(NULL) + s_time_config.maxtime;
> +
> + bytes_read = 0;
> + tm_Time_F(START);
> +
> + for (;;) {
> + if (finishtime < time(NULL))
> + break;
> + if ((scon = doConnection(reuse_session ? scon : NULL)) == NULL)
> + goto end;
> +
> + if (s_time_config.www_path) {
> + int i, retval = snprintf(buf, sizeof buf,
> +    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
> + if ((size_t)retval >= sizeof buf) {
> + fprintf(stderr, "URL too long\n");
> + goto end;
> + }
> + SSL_write(scon, buf, strlen(buf));
> + while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
> + bytes_read += i;
> + }
> + if (s_time_config.no_shutdown)
> + SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
> +    SSL_RECEIVED_SHUTDOWN);
> + else
> + SSL_shutdown(scon);
> + shutdown(SSL_get_fd(scon), SHUT_RDWR);
> + close(SSL_get_fd(scon));
> +
> + nConn += 1;
> + if (SSL_session_reused(scon))
> + ver = 'r';
> + else {
> + ver = SSL_version(scon);
> + if (ver == TLS1_VERSION)
> + ver = 't';
> + else if (ver == SSL3_VERSION)
> + ver = '3';
> + else if (ver == SSL2_VERSION)
> + ver = '2';
> + else
> + ver = '*';
> + }
> + fputc(ver, stdout);
> + fflush(stdout);
> +
> + if (!reuse_session) {
> + SSL_free(scon);
> + scon = NULL;
> + }
> + }
> + totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
> +
> + printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n",
> +    nConn, totalTime, ((double) nConn / totalTime), bytes_read);
> + printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
> +    nConn,
> +    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
> +    bytes_read / nConn);
> +
> + ret = 0;
> +end:
> + SSL_free(scon);
> + return ret;
>  }
>

Reply | Threaded
Open this post in threaded view
|

Re: openssl s_time: refactor benchmark loops

Scott Cheloha
On Sat, Aug 11, 2018 at 11:22:09PM +0900, Kinichiro Inoguchi wrote:

> Hi,
>
> I saw that 2 loops are almost identical, and I think diff tidy up these 2 loops
> in 1 function.
>
> I found these 2 lines were not in original code.
>
> > + shutdown(SSL_get_fd(scon), SHUT_RDWR);
> > + close(SSL_get_fd(scon));
>
> Is this part needed ?

Nope, good catch.  Those are leftovers from an older
revision of this patch and would be a regression.

Updated diff below.

Index: usr.bin/openssl/s_time.c
===================================================================
RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
retrieving revision 1.24
diff -u -p -r1.24 s_time.c
--- usr.bin/openssl/s_time.c 13 Jul 2018 18:36:56 -0000 1.24
+++ usr.bin/openssl/s_time.c 11 Aug 2018 15:48:02 -0000
@@ -91,6 +91,7 @@ extern int verify_depth;
 
 static void s_time_usage(void);
 static SSL *doConnection(SSL * scon);
+static int benchmark(int);
 
 static SSL_CTX *tm_ctx = NULL;
 static const SSL_METHOD *s_time_meth = NULL;
@@ -244,13 +245,7 @@ tm_Time_F(int op)
 int
 s_time_main(int argc, char **argv)
 {
- double totalTime = 0.0;
- int nConn = 0;
- SSL *scon = NULL;
- time_t finishtime;
  int ret = 1;
- char buf[1024 * 8];
- int ver;
 
  if (single_execution) {
  if (pledge("stdio rpath inet dns", NULL) == -1) {
@@ -328,60 +323,8 @@ s_time_main(int argc, char **argv)
 
  /* Loop and time how long it takes to make connections */
 
- bytes_read = 0;
- finishtime = time(NULL) + s_time_config.maxtime;
- tm_Time_F(START);
- for (;;) {
- if (finishtime < time(NULL))
- break;
- if ((scon = doConnection(NULL)) == NULL)
- goto end;
-
- if (s_time_config.www_path != NULL) {
- int i, retval = snprintf(buf, sizeof buf,
-    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
- if ((size_t)retval >= sizeof buf) {
- fprintf(stderr, "URL too long\n");
- goto end;
- }
- SSL_write(scon, buf, strlen(buf));
- while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
- bytes_read += i;
- }
- if (s_time_config.no_shutdown)
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
-    SSL_RECEIVED_SHUTDOWN);
- else
- SSL_shutdown(scon);
-
- nConn += 1;
- if (SSL_session_reused(scon))
- ver = 'r';
- else {
- ver = SSL_version(scon);
- if (ver == TLS1_VERSION)
- ver = 't';
- else if (ver == SSL3_VERSION)
- ver = '3';
- else if (ver == SSL2_VERSION)
- ver = '2';
- else
- ver = '*';
- }
- fputc(ver, stdout);
- fflush(stdout);
-
- SSL_free(scon);
- scon = NULL;
- }
- totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
-
- printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n",
-    nConn, totalTime, ((double) nConn / totalTime), bytes_read);
- printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
-    nConn,
-    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
-    bytes_read / nConn);
+ if (benchmark(0))
+ goto end;
 
  /*
  * Now loop and time connections using the same session id over and
@@ -393,88 +336,11 @@ s_time_main(int argc, char **argv)
  goto end;
  printf("\n\nNow timing with session id reuse.\n");
 
- /* Get an SSL object so we can reuse the session id */
- if ((scon = doConnection(NULL)) == NULL) {
- fprintf(stderr, "Unable to get connection\n");
+ if (benchmark(1))
  goto end;
- }
- if (s_time_config.www_path != NULL) {
- int retval = snprintf(buf, sizeof buf,
-    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
- if ((size_t)retval >= sizeof buf) {
- fprintf(stderr, "URL too long\n");
- goto end;
- }
- SSL_write(scon, buf, strlen(buf));
- while (SSL_read(scon, buf, sizeof(buf)) > 0);
- }
- if (s_time_config.no_shutdown)
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
-    SSL_RECEIVED_SHUTDOWN);
- else
- SSL_shutdown(scon);
-
- nConn = 0;
- totalTime = 0.0;
-
- finishtime = time(NULL) + s_time_config.maxtime;
-
- printf("starting\n");
- bytes_read = 0;
- tm_Time_F(START);
-
- for (;;) {
- if (finishtime < time(NULL))
- break;
- if ((doConnection(scon)) == NULL)
- goto end;
-
- if (s_time_config.www_path) {
- int i, retval = snprintf(buf, sizeof buf,
-    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
- if ((size_t)retval >= sizeof buf) {
- fprintf(stderr, "URL too long\n");
- goto end;
- }
- SSL_write(scon, buf, strlen(buf));
- while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
- bytes_read += i;
- }
- if (s_time_config.no_shutdown)
- SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
-    SSL_RECEIVED_SHUTDOWN);
- else
- SSL_shutdown(scon);
-
- nConn += 1;
- if (SSL_session_reused(scon))
- ver = 'r';
- else {
- ver = SSL_version(scon);
- if (ver == TLS1_VERSION)
- ver = 't';
- else if (ver == SSL3_VERSION)
- ver = '3';
- else if (ver == SSL2_VERSION)
- ver = '2';
- else
- ver = '*';
- }
- fputc(ver, stdout);
- fflush(stdout);
- }
- totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
-
- printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n", nConn, totalTime, ((double) nConn / totalTime), bytes_read);
- printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
-    nConn,
-    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
-    bytes_read / nConn);
 
  ret = 0;
  end:
- SSL_free(scon);
-
  if (tm_ctx != NULL) {
  SSL_CTX_free(tm_ctx);
  tm_ctx = NULL;
@@ -541,4 +407,107 @@ doConnection(SSL * scon)
  return NULL;
  }
  return serverCon;
+}
+
+static int
+benchmark(int reuse_session)
+{
+ double totalTime = 0.0;
+ int nConn = 0;
+ SSL *scon = NULL;
+ time_t finishtime;
+ int ret = 1;
+ char buf[1024 * 8];
+ int ver;
+
+ if (reuse_session) {
+ /* Get an SSL object so we can reuse the session id */
+ if ((scon = doConnection(NULL)) == NULL) {
+ fprintf(stderr, "Unable to get connection\n");
+ goto end;
+ }
+ if (s_time_config.www_path != NULL) {
+ int retval = snprintf(buf, sizeof buf,
+    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
+ if ((size_t)retval >= sizeof buf) {
+ fprintf(stderr, "URL too long\n");
+ goto end;
+ }
+ SSL_write(scon, buf, strlen(buf));
+ while (SSL_read(scon, buf, sizeof(buf)) > 0);
+ }
+ if (s_time_config.no_shutdown)
+ SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
+    SSL_RECEIVED_SHUTDOWN);
+ else
+ SSL_shutdown(scon);
+ printf("starting\n");
+ }
+
+ nConn = 0;
+ totalTime = 0.0;
+
+ finishtime = time(NULL) + s_time_config.maxtime;
+
+ bytes_read = 0;
+ tm_Time_F(START);
+
+ for (;;) {
+ if (finishtime < time(NULL))
+ break;
+ if ((scon = doConnection(reuse_session ? scon : NULL)) == NULL)
+ goto end;
+
+ if (s_time_config.www_path != NULL) {
+ int i, retval = snprintf(buf, sizeof buf,
+    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
+ if ((size_t)retval >= sizeof buf) {
+ fprintf(stderr, "URL too long\n");
+ goto end;
+ }
+ SSL_write(scon, buf, strlen(buf));
+ while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
+ bytes_read += i;
+ }
+ if (s_time_config.no_shutdown)
+ SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
+    SSL_RECEIVED_SHUTDOWN);
+ else
+ SSL_shutdown(scon);
+
+ nConn += 1;
+ if (SSL_session_reused(scon))
+ ver = 'r';
+ else {
+ ver = SSL_version(scon);
+ if (ver == TLS1_VERSION)
+ ver = 't';
+ else if (ver == SSL3_VERSION)
+ ver = '3';
+ else if (ver == SSL2_VERSION)
+ ver = '2';
+ else
+ ver = '*';
+ }
+ fputc(ver, stdout);
+ fflush(stdout);
+
+ if (!reuse_session) {
+ SSL_free(scon);
+ scon = NULL;
+ }
+ }
+ totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
+
+ printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n",
+    nConn, totalTime, ((double) nConn / totalTime), bytes_read);
+ printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
+    nConn,
+    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
+    bytes_read / nConn);
+
+ ret = 0;
+ end:
+ SSL_free(scon);
+ return ret;
 }

Reply | Threaded
Open this post in threaded view
|

Re: openssl s_time: refactor benchmark loops

Theo Buehler-5
On Sat, Aug 11, 2018 at 10:51:12AM -0500, Scott Cheloha wrote:

> On Sat, Aug 11, 2018 at 11:22:09PM +0900, Kinichiro Inoguchi wrote:
> > Hi,
> >
> > I saw that 2 loops are almost identical, and I think diff tidy up these 2 loops
> > in 1 function.
> >
> > I found these 2 lines were not in original code.
> >
> > > + shutdown(SSL_get_fd(scon), SHUT_RDWR);
> > > + close(SSL_get_fd(scon));
> >
> > Is this part needed ?
>
> Nope, good catch.  Those are leftovers from an older
> revision of this patch and would be a regression.
>
> Updated diff below.

ok tb

>
> Index: usr.bin/openssl/s_time.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/openssl/s_time.c,v
> retrieving revision 1.24
> diff -u -p -r1.24 s_time.c
> --- usr.bin/openssl/s_time.c 13 Jul 2018 18:36:56 -0000 1.24
> +++ usr.bin/openssl/s_time.c 11 Aug 2018 15:48:02 -0000
> @@ -91,6 +91,7 @@ extern int verify_depth;
>  
>  static void s_time_usage(void);
>  static SSL *doConnection(SSL * scon);
> +static int benchmark(int);
>  
>  static SSL_CTX *tm_ctx = NULL;
>  static const SSL_METHOD *s_time_meth = NULL;
> @@ -244,13 +245,7 @@ tm_Time_F(int op)
>  int
>  s_time_main(int argc, char **argv)
>  {
> - double totalTime = 0.0;
> - int nConn = 0;
> - SSL *scon = NULL;
> - time_t finishtime;
>   int ret = 1;
> - char buf[1024 * 8];
> - int ver;
>  
>   if (single_execution) {
>   if (pledge("stdio rpath inet dns", NULL) == -1) {
> @@ -328,60 +323,8 @@ s_time_main(int argc, char **argv)
>  
>   /* Loop and time how long it takes to make connections */
>  
> - bytes_read = 0;
> - finishtime = time(NULL) + s_time_config.maxtime;
> - tm_Time_F(START);
> - for (;;) {
> - if (finishtime < time(NULL))
> - break;
> - if ((scon = doConnection(NULL)) == NULL)
> - goto end;
> -
> - if (s_time_config.www_path != NULL) {
> - int i, retval = snprintf(buf, sizeof buf,
> -    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
> - if ((size_t)retval >= sizeof buf) {
> - fprintf(stderr, "URL too long\n");
> - goto end;
> - }
> - SSL_write(scon, buf, strlen(buf));
> - while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
> - bytes_read += i;
> - }
> - if (s_time_config.no_shutdown)
> - SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
> -    SSL_RECEIVED_SHUTDOWN);
> - else
> - SSL_shutdown(scon);
> -
> - nConn += 1;
> - if (SSL_session_reused(scon))
> - ver = 'r';
> - else {
> - ver = SSL_version(scon);
> - if (ver == TLS1_VERSION)
> - ver = 't';
> - else if (ver == SSL3_VERSION)
> - ver = '3';
> - else if (ver == SSL2_VERSION)
> - ver = '2';
> - else
> - ver = '*';
> - }
> - fputc(ver, stdout);
> - fflush(stdout);
> -
> - SSL_free(scon);
> - scon = NULL;
> - }
> - totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
> -
> - printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n",
> -    nConn, totalTime, ((double) nConn / totalTime), bytes_read);
> - printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
> -    nConn,
> -    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
> -    bytes_read / nConn);
> + if (benchmark(0))
> + goto end;
>  
>   /*
>   * Now loop and time connections using the same session id over and
> @@ -393,88 +336,11 @@ s_time_main(int argc, char **argv)
>   goto end;
>   printf("\n\nNow timing with session id reuse.\n");
>  
> - /* Get an SSL object so we can reuse the session id */
> - if ((scon = doConnection(NULL)) == NULL) {
> - fprintf(stderr, "Unable to get connection\n");
> + if (benchmark(1))
>   goto end;
> - }
> - if (s_time_config.www_path != NULL) {
> - int retval = snprintf(buf, sizeof buf,
> -    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
> - if ((size_t)retval >= sizeof buf) {
> - fprintf(stderr, "URL too long\n");
> - goto end;
> - }
> - SSL_write(scon, buf, strlen(buf));
> - while (SSL_read(scon, buf, sizeof(buf)) > 0);
> - }
> - if (s_time_config.no_shutdown)
> - SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
> -    SSL_RECEIVED_SHUTDOWN);
> - else
> - SSL_shutdown(scon);
> -
> - nConn = 0;
> - totalTime = 0.0;
> -
> - finishtime = time(NULL) + s_time_config.maxtime;
> -
> - printf("starting\n");
> - bytes_read = 0;
> - tm_Time_F(START);
> -
> - for (;;) {
> - if (finishtime < time(NULL))
> - break;
> - if ((doConnection(scon)) == NULL)
> - goto end;
> -
> - if (s_time_config.www_path) {
> - int i, retval = snprintf(buf, sizeof buf,
> -    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
> - if ((size_t)retval >= sizeof buf) {
> - fprintf(stderr, "URL too long\n");
> - goto end;
> - }
> - SSL_write(scon, buf, strlen(buf));
> - while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
> - bytes_read += i;
> - }
> - if (s_time_config.no_shutdown)
> - SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
> -    SSL_RECEIVED_SHUTDOWN);
> - else
> - SSL_shutdown(scon);
> -
> - nConn += 1;
> - if (SSL_session_reused(scon))
> - ver = 'r';
> - else {
> - ver = SSL_version(scon);
> - if (ver == TLS1_VERSION)
> - ver = 't';
> - else if (ver == SSL3_VERSION)
> - ver = '3';
> - else if (ver == SSL2_VERSION)
> - ver = '2';
> - else
> - ver = '*';
> - }
> - fputc(ver, stdout);
> - fflush(stdout);
> - }
> - totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
> -
> - printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n", nConn, totalTime, ((double) nConn / totalTime), bytes_read);
> - printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
> -    nConn,
> -    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
> -    bytes_read / nConn);
>  
>   ret = 0;
>   end:
> - SSL_free(scon);
> -
>   if (tm_ctx != NULL) {
>   SSL_CTX_free(tm_ctx);
>   tm_ctx = NULL;
> @@ -541,4 +407,107 @@ doConnection(SSL * scon)
>   return NULL;
>   }
>   return serverCon;
> +}
> +
> +static int
> +benchmark(int reuse_session)
> +{
> + double totalTime = 0.0;
> + int nConn = 0;
> + SSL *scon = NULL;
> + time_t finishtime;
> + int ret = 1;
> + char buf[1024 * 8];
> + int ver;
> +
> + if (reuse_session) {
> + /* Get an SSL object so we can reuse the session id */
> + if ((scon = doConnection(NULL)) == NULL) {
> + fprintf(stderr, "Unable to get connection\n");
> + goto end;
> + }
> + if (s_time_config.www_path != NULL) {
> + int retval = snprintf(buf, sizeof buf,
> +    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
> + if ((size_t)retval >= sizeof buf) {
> + fprintf(stderr, "URL too long\n");
> + goto end;
> + }
> + SSL_write(scon, buf, strlen(buf));
> + while (SSL_read(scon, buf, sizeof(buf)) > 0);
> + }
> + if (s_time_config.no_shutdown)
> + SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
> +    SSL_RECEIVED_SHUTDOWN);
> + else
> + SSL_shutdown(scon);
> + printf("starting\n");
> + }
> +
> + nConn = 0;
> + totalTime = 0.0;
> +
> + finishtime = time(NULL) + s_time_config.maxtime;
> +
> + bytes_read = 0;
> + tm_Time_F(START);
> +
> + for (;;) {
> + if (finishtime < time(NULL))
> + break;
> + if ((scon = doConnection(reuse_session ? scon : NULL)) == NULL)
> + goto end;
> +
> + if (s_time_config.www_path != NULL) {
> + int i, retval = snprintf(buf, sizeof buf,
> +    "GET %s HTTP/1.0\r\n\r\n", s_time_config.www_path);
> + if ((size_t)retval >= sizeof buf) {
> + fprintf(stderr, "URL too long\n");
> + goto end;
> + }
> + SSL_write(scon, buf, strlen(buf));
> + while ((i = SSL_read(scon, buf, sizeof(buf))) > 0)
> + bytes_read += i;
> + }
> + if (s_time_config.no_shutdown)
> + SSL_set_shutdown(scon, SSL_SENT_SHUTDOWN |
> +    SSL_RECEIVED_SHUTDOWN);
> + else
> + SSL_shutdown(scon);
> +
> + nConn += 1;
> + if (SSL_session_reused(scon))
> + ver = 'r';
> + else {
> + ver = SSL_version(scon);
> + if (ver == TLS1_VERSION)
> + ver = 't';
> + else if (ver == SSL3_VERSION)
> + ver = '3';
> + else if (ver == SSL2_VERSION)
> + ver = '2';
> + else
> + ver = '*';
> + }
> + fputc(ver, stdout);
> + fflush(stdout);
> +
> + if (!reuse_session) {
> + SSL_free(scon);
> + scon = NULL;
> + }
> + }
> + totalTime += tm_Time_F(STOP); /* Add the time for this iteration */
> +
> + printf("\n\n%d connections in %.2fs; %.2f connections/user sec, bytes read %ld\n",
> +    nConn, totalTime, ((double) nConn / totalTime), bytes_read);
> + printf("%d connections in %lld real seconds, %ld bytes read per connection\n",
> +    nConn,
> +    (long long)(time(NULL) - finishtime + s_time_config.maxtime),
> +    bytes_read / nConn);
> +
> + ret = 0;
> + end:
> + SSL_free(scon);
> + return ret;
>  }