Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
From: Stefan Eissing <stefan.eissing@greenbytes.de>
Content-Type: multipart/mixed;
 boundary="Apple-Mail=_DE6DDAC6-82DE-43B3-854B-747808ED20AF"
Subject: mod_status, server, protocol
Message-Id: <34A6947A-0068-4D0E-A533-03CBD02F8A1B@greenbytes.de>
Date: Wed, 20 Jan 2016 17:26:18 +0100
To: dev@httpd.apache.org
Mime-Version: 1.0 (Mac OS X Mail 9.2 \(3112\))


--Apple-Mail=_DE6DDAC6-82DE-43B3-854B-747808ED20AF
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=us-ascii

I'd like some feedback to a small scoreboard/mod_status patch. It makes =
the following changes:
- new method ap_update_child_status_from_server(ap_sb_handle_t *sbh, int =
status, conn_rec *c, server_rec *s); in the API
- mod_ssl uses that to announce servers selected by TLS servername =
*before* the first request comes in for vhost
- worker_score has an additional member char protoccol[16] that records =
ap_get_protocol(c) when a connection is given in an update
- mod_status introduces a new columne 'Protocol' for displaying the =
connection protocol used.
- vhost is no longer NULLed when a request ends
- status SERVER_READY explicitly clears client/vhost/request/protocol

1. I am not sure if it is sensitive to add a columns to mod_status =
output. It is html after all.
2. 16 bytes for protocol seems much, but 8 is too little for http/1.1 =
and websocket would also need more...
3. http/1.1 clear and http/1.1+tls could be worth a differentiation, but =
not done for now
4. I was considering to show some HTTP/2 information in the "Request" =
field, but maybe that is only confusing?

Feedback appreciated.

-Stefan


--Apple-Mail=_DE6DDAC6-82DE-43B3-854B-747808ED20AF
Content-Disposition: attachment;
	filename=proto_status_trunk.patch
Content-Type: application/octet-stream;
	name="proto_status_trunk.patch"
Content-Transfer-Encoding: 7bit

Index: include/scoreboard.h
===================================================================
--- include/scoreboard.h	(revision 1725693)
+++ include/scoreboard.h	(working copy)
@@ -115,6 +115,7 @@
     char client[40];            /* Keep 'em small... but large enough to hold an IPv6 address */
     char request[64];           /* We just want an idea... */
     char vhost[32];             /* What virtual host is being accessed? */
+    char protocol[16];          /* What protocol is used on the connection? */
 };
 
 typedef struct {
@@ -181,6 +182,7 @@
 AP_DECLARE(int) ap_update_child_status_from_indexes(int child_num, int thread_num,
                                                     int status, request_rec *r);
 AP_DECLARE(int) ap_update_child_status_from_conn(ap_sb_handle_t *sbh, int status, conn_rec *c);
+AP_DECLARE(int) ap_update_child_status_from_server(ap_sb_handle_t *sbh, int status, conn_rec *c, server_rec *s);
 AP_DECLARE(void) ap_time_process_request(ap_sb_handle_t *sbh, int status);
 
 AP_DECLARE(worker_score *) ap_get_scoreboard_worker(ap_sb_handle_t *sbh);
Index: modules/generators/mod_status.c
===================================================================
--- modules/generators/mod_status.c	(revision 1725693)
+++ modules/generators/mod_status.c	(working copy)
@@ -662,7 +662,7 @@
 #endif
                      "<th>SS</th><th>Req</th>"
                      "<th>Conn</th><th>Child</th><th>Slot</th>"
-                     "<th>Client</th><th>VHost</th>"
+                     "<th>Client</th><th>Protocol</th><th>VHost</th>"
                      "<th>Request</th></tr>\n\n", r);
 
         for (i = 0; i < server_limit; ++i) {
@@ -776,7 +776,7 @@
                     format_byte_out(r, bytes);
                     ap_rputs(")\n", r);
                     ap_rprintf(r,
-                               " <i>%s {%s}</i> <b>[%s]</b><br />\n\n",
+                               " <i>%s {%s}</i> <i>(%s)</i><b>[%s]</b><br />\n\n",
                                ap_escape_html(r->pool,
                                               ws_record->client),
                                ap_escape_html(r->pool,
@@ -783,6 +783,8 @@
                                               ap_escape_logitem(r->pool,
                                                                 ws_record->request)),
                                ap_escape_html(r->pool,
+                                              ws_record->protocol),
+                               ap_escape_html(r->pool,
                                               ws_record->vhost));
                 }
                 else { /* !no_table_report */
@@ -860,11 +862,13 @@
                                (float)conn_bytes / KBYTE, (float) my_bytes / MBYTE,
                                (float)bytes / MBYTE);
 
-                    ap_rprintf(r, "</td><td>%s</td><td nowrap>%s</td>"
+                    ap_rprintf(r, "</td><td>%s</td><td>%s</td><td nowrap>%s</td>"
                                   "<td nowrap>%s</td></tr>\n\n",
                                ap_escape_html(r->pool,
                                               ws_record->client),
                                ap_escape_html(r->pool,
+                                              ws_record->protocol),
+                               ap_escape_html(r->pool,
                                               ws_record->vhost),
                                ap_escape_html(r->pool,
                                               ap_escape_logitem(r->pool,
Index: modules/ssl/ssl_engine_kernel.c
===================================================================
--- modules/ssl/ssl_engine_kernel.c	(revision 1725693)
+++ modules/ssl/ssl_engine_kernel.c	(working copy)
@@ -31,6 +31,7 @@
 #include "ssl_private.h"
 #include "mod_ssl.h"
 #include "util_md5.h"
+#include "scoreboard.h"
 
 static void ssl_configure_env(request_rec *r, SSLConnRec *sslconn);
 #ifdef HAVE_TLSEXT
@@ -2218,6 +2219,7 @@
         sslcon->server = s;
         sslcon->cipher_suite = sc->server->auth.cipher_suite;
         
+        ap_update_child_status_from_server(c->sbh, SERVER_BUSY_READ, c, s);
         /*
          * There is one special filter callback, which is set
          * very early depending on the base_server's log level.
Index: server/scoreboard.c
===================================================================
--- server/scoreboard.c	(revision 1725693)
+++ server/scoreboard.c	(working copy)
@@ -32,6 +32,7 @@
 #include "http_main.h"
 #include "http_core.h"
 #include "http_config.h"
+#include "http_protocol.h"
 #include "ap_mpm.h"
 
 #include "scoreboard.h"
@@ -457,6 +458,7 @@
                                         int thread_num,
                                         int status,
                                         conn_rec *c,
+                                        server_rec *s,
                                         request_rec *r)
 {
     int old_status;
@@ -490,28 +492,42 @@
             ws->conn_bytes = 0;
             ws->last_used = apr_time_now();
         }
-        if (r) {
-            const char *client = ap_get_remote_host(c, r->per_dir_config,
-                                 REMOTE_NOLOOKUP, NULL);
-            if (!client || !strcmp(client, c->client_ip)) {
-                apr_cpystrn(ws->client, r->useragent_ip, sizeof(ws->client));
+        if (status == SERVER_READY) {
+            ws->client[0]='\0';
+            ws->vhost[0]='\0';
+            ws->request[0]='\0';
+            ws->protocol[0]='\0';
+        }
+        else {
+            if (r) {
+                const char *client = ap_get_remote_host(c, r->per_dir_config,
+                                                        REMOTE_NOLOOKUP, NULL);
+                if (!client || !strcmp(client, c->client_ip)) {
+                    apr_cpystrn(ws->client, r->useragent_ip, sizeof(ws->client));
+                }
+                else {
+                    apr_cpystrn(ws->client, client, sizeof(ws->client));
+                }
+                copy_request(ws->request, sizeof(ws->request), r);
             }
-            else {
-                apr_cpystrn(ws->client, client, sizeof(ws->client));
+            else if (c) {
+                apr_cpystrn(ws->client, ap_get_remote_host(c, NULL,
+                                                           REMOTE_NOLOOKUP, NULL), sizeof(ws->client));
+                ws->request[0]='\0';
             }
-            copy_request(ws->request, sizeof(ws->request), r);
-            if (r->server) {
-                apr_snprintf(ws->vhost, sizeof(ws->vhost), "%s:%d",
-                             r->server->server_hostname,
-                             r->connection->local_addr->port);
+            if (s) {
+                if (c) {
+                    apr_snprintf(ws->vhost, sizeof(ws->vhost), "%s:%d",
+                                 s->server_hostname, c->local_addr->port);
+                }
+                else {
+                    apr_cpystrn(ws->vhost, s->server_hostname, sizeof(ws->vhost));
+                }
             }
+            if (c) {
+                apr_cpystrn(ws->protocol, ap_get_protocol(c), sizeof(ws->protocol));
+            }
         }
-        else if (c) {
-            apr_cpystrn(ws->client, ap_get_remote_host(c, NULL,
-                        REMOTE_NOLOOKUP, NULL), sizeof(ws->client));
-            ws->request[0]='\0';
-            ws->vhost[0]='\0';
-        }
     }
 
     return old_status;
@@ -528,6 +544,7 @@
 
     return update_child_status_internal(child_num, thread_num, status,
                                         r ? r->connection : NULL,
+                                        r ? r->server : NULL,
                                         r);
 }
 
@@ -540,19 +557,30 @@
     return update_child_status_internal(sbh->child_num, sbh->thread_num,
                                         status,
                                         r ? r->connection : NULL,
+                                        r ? r->server : NULL,
                                         r);
 }
 
 AP_DECLARE(int) ap_update_child_status_from_conn(ap_sb_handle_t *sbh, int status,
-                                       conn_rec *c)
+                                                 conn_rec *c)
 {
     if (!sbh || (sbh->child_num < 0))
         return -1;
 
     return update_child_status_internal(sbh->child_num, sbh->thread_num,
-                                        status, c, NULL);
+                                        status, c, NULL, NULL);
 }
 
+AP_DECLARE(int) ap_update_child_status_from_server(ap_sb_handle_t *sbh, int status, 
+                                                   conn_rec *c, server_rec *s)
+{
+    if (!sbh || (sbh->child_num < 0))
+        return -1;
+
+    return update_child_status_internal(sbh->child_num, sbh->thread_num,
+                                        status, c, s, NULL);
+}
+
 AP_DECLARE(void) ap_time_process_request(ap_sb_handle_t *sbh, int status)
 {
     worker_score *ws;
@@ -605,6 +633,7 @@
     dest->client[sizeof(dest->client) - 1] = '\0';
     dest->request[sizeof(dest->request) - 1] = '\0';
     dest->vhost[sizeof(dest->vhost) - 1] = '\0';
+    dest->protocol[sizeof(dest->protocol) - 1] = '\0';
 }
 
 AP_DECLARE(process_score *) ap_get_scoreboard_process(int x)

--Apple-Mail=_DE6DDAC6-82DE-43B3-854B-747808ED20AF--