[Spice-commits] 4 commits - server/main_channel.c server/main_channel.h server/red_channel.c server/red_channel.h server/reds.c server/spice_timer_queue.c

Yonit Halperin yhalperi at kemper.freedesktop.org
Wed Aug 14 10:35:34 PDT 2013


 server/main_channel.c      |   27 +++++++--
 server/main_channel.h      |    2 
 server/red_channel.c       |  126 +++++++++++++++++++++++++++++++++++++++++++++
 server/red_channel.h       |   16 +++++
 server/reds.c              |    3 -
 server/spice_timer_queue.c |    4 -
 6 files changed, 166 insertions(+), 12 deletions(-)

New commits:
commit ed1f70c6d16ff55adf73a08f063f5d7955f4c488
Author: Yonit Halperin <yhalperi at redhat.com>
Date:   Wed Aug 14 10:56:44 2013 -0400

    main_channel: monitoring client connection status
    
    rhbz#994175
    
    Start monitoring if the client connection is alive after completing
    the bit-rate test.

diff --git a/server/main_channel.c b/server/main_channel.c
index fe032a6..54718ba 100644
--- a/server/main_channel.c
+++ b/server/main_channel.c
@@ -55,6 +55,8 @@
 
 #define PING_INTERVAL (1000 * 10)
 
+#define CLIENT_CONNECTIVITY_TIMEOUT (30*1000) // 30 seconds
+
 static uint8_t zero_page[ZERO_BUF_SIZE] = {0};
 
 enum {
@@ -201,16 +203,20 @@ RedClient *main_channel_get_client_by_link_id(MainChannel *main_chan, uint32_t c
 
 static int main_channel_client_push_ping(MainChannelClient *mcc, int size);
 
-void main_channel_client_start_net_test(MainChannelClient *mcc)
+void main_channel_client_start_net_test(MainChannelClient *mcc, int test_rate)
 {
     if (!mcc || mcc->net_test_id) {
         return;
     }
-    if (main_channel_client_push_ping(mcc, NET_TEST_WARMUP_BYTES)
-        && main_channel_client_push_ping(mcc, 0)
-        && main_channel_client_push_ping(mcc, NET_TEST_BYTES)) {
-        mcc->net_test_id = mcc->ping_id - 2;
-        mcc->net_test_stage = NET_TEST_STAGE_WARMUP;
+    if (test_rate) {
+        if (main_channel_client_push_ping(mcc, NET_TEST_WARMUP_BYTES)
+            && main_channel_client_push_ping(mcc, 0)
+            && main_channel_client_push_ping(mcc, NET_TEST_BYTES)) {
+            mcc->net_test_id = mcc->ping_id - 2;
+            mcc->net_test_stage = NET_TEST_STAGE_WARMUP;
+        }
+    } else {
+        red_channel_client_start_connectivity_monitoring(&mcc->base, CLIENT_CONNECTIVITY_TIMEOUT);
     }
 }
 
@@ -970,6 +976,8 @@ static int main_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint
                     spice_printerr("net test: invalid values, latency %" PRIu64
                                " roundtrip %" PRIu64 ". assuming high"
                                "bandwidth", mcc->latency, roundtrip);
+                    red_channel_client_start_connectivity_monitoring(&mcc->base,
+                                                                     CLIENT_CONNECTIVITY_TIMEOUT);
                     break;
                 }
                 mcc->bitrate_per_sec = (uint64_t)(NET_TEST_BYTES * 8) * 1000000
@@ -980,6 +988,8 @@ static int main_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint
                            mcc->bitrate_per_sec,
                            (double)mcc->bitrate_per_sec / 1024 / 1024,
                            main_channel_client_is_low_bandwidth(mcc) ? " LOW BANDWIDTH" : "");
+                red_channel_client_start_connectivity_monitoring(&mcc->base,
+                                                                 CLIENT_CONNECTIVITY_TIMEOUT);
                 break;
             default:
                 spice_printerr("invalid net test stage, ping id %d test id %d stage %d",
@@ -989,6 +999,11 @@ static int main_channel_handle_parsed(RedChannelClient *rcc, uint32_t size, uint
                 mcc->net_test_stage = NET_TEST_STAGE_INVALID;
             }
             break;
+        } else {
+            /*
+             * channel client monitors the connectivity using ping-pong messages
+             */
+            red_channel_client_handle_message(rcc, size, type, message);
         }
 #ifdef RED_STATISTICS
         reds_update_stat_value(roundtrip);
diff --git a/server/main_channel.h b/server/main_channel.h
index 27367a4..29eb8d4 100644
--- a/server/main_channel.h
+++ b/server/main_channel.h
@@ -54,7 +54,7 @@ void main_channel_push_agent_disconnected(MainChannel *main_chan);
 void main_channel_client_push_agent_tokens(MainChannelClient *mcc, uint32_t num_tokens);
 void main_channel_client_push_agent_data(MainChannelClient *mcc, uint8_t* data, size_t len,
                                          spice_marshaller_item_free_func free_data, void *opaque);
-void main_channel_client_start_net_test(MainChannelClient *mcc);
+void main_channel_client_start_net_test(MainChannelClient *mcc, int test_rate);
 // TODO: huge. Consider making a reds_* interface for these functions
 // and calling from main.
 void main_channel_push_init(MainChannelClient *mcc, int display_channels_hint,
diff --git a/server/reds.c b/server/reds.c
index ae87c90..0f81a32 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -1718,11 +1718,10 @@ static void reds_handle_main_link(RedLinkInfo *link)
             main_channel_push_name(mcc, spice_name);
         if (spice_uuid_is_set)
             main_channel_push_uuid(mcc, spice_uuid);
-
-        main_channel_client_start_net_test(mcc);
     } else {
         reds_mig_target_client_add(client);
     }
+    main_channel_client_start_net_test(mcc, !mig_target);
 }
 
 #define RED_MOUSE_STATE_TO_LOCAL(state)     \
commit c8b808bb8211e756a5e2280da173010984b71680
Author: Yonit Halperin <yhalperi at redhat.com>
Date:   Wed Aug 14 10:10:37 2013 -0400

    red_channel: add option to monitor whether a channel client is alive
    
    rhbz#994175
    
    When a client connection is closed surprisingly (i.e., without a FIN
    segment), we cannot identify it by a socket error (which is the only
    way by which we identified disconnections so far).
    This patch allows a channel client to periodically check the state of
    the connection and identify surprise disconnections.

diff --git a/server/red_channel.c b/server/red_channel.c
index bcf5a60..961c36c 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -64,6 +64,13 @@ enum QosPingState {
     PING_STATE_LATENCY,
 };
 
+enum ConnectivityState {
+    CONNECTIVITY_STATE_CONNECTED,
+    CONNECTIVITY_STATE_BLOCKED,
+    CONNECTIVITY_STATE_WAIT_PONG,
+    CONNECTIVITY_STATE_DISCONNECTED,
+};
+
 static void red_channel_client_start_ping_timer(RedChannelClient *rcc, uint32_t timeout);
 static void red_channel_client_cancel_ping_timer(RedChannelClient *rcc);
 static void red_channel_client_restart_ping_timer(RedChannelClient *rcc);
@@ -73,6 +80,7 @@ static void red_client_add_channel(RedClient *client, RedChannelClient *rcc);
 static void red_client_remove_channel(RedChannelClient *rcc);
 static RedChannelClient *red_client_get_channel(RedClient *client, int type, int id);
 static void red_channel_client_restore_main_sender(RedChannelClient *rcc);
+static inline int red_channel_client_waiting_for_ack(RedChannelClient *rcc);
 
 /*
  * Lifetime of RedChannel, RedChannelClient and RedClient:
@@ -389,11 +397,19 @@ static void red_channel_client_on_output(void *opaque, int n)
 {
     RedChannelClient *rcc = opaque;
 
+    if (rcc->connectivity_monitor.timer) {
+        rcc->connectivity_monitor.out_bytes += n;
+    }
     stat_inc_counter(rcc->channel->out_bytes_counter, n);
 }
 
 static void red_channel_client_on_input(void *opaque, int n)
 {
+    RedChannelClient *rcc = opaque;
+
+    if (rcc->connectivity_monitor.timer) {
+        rcc->connectivity_monitor.in_bytes += n;
+    }
 }
 
 static void red_channel_client_default_peer_on_error(RedChannelClient *rcc)
@@ -759,6 +775,97 @@ static void red_channel_client_ping_timer(void *opaque)
 #endif /* ifdef HAVE_LINUX_SOCKIOS_H */
 }
 
+/*
+ * When a connection is not alive (and we can't detect it via a socket error), we
+ * reach one of these 2 states:
+ * (1) Sending msgs is blocked: either writes return EAGAIN
+ *     or we are missing MSGC_ACK from the client.
+ * (2) MSG_PING was sent without receiving a MSGC_PONG in reply.
+ *
+ * The connectivity_timer callback tests if the channel's state matches one of the above.
+ * In case it does, on the next time the timer is called, it checks if the connection has
+ * been idle during the time that passed since the previous timer call. If the connection
+ * has been idle, we consider the client as disconnected.
+ */
+static void red_channel_client_connectivity_timer(void *opaque)
+{
+    RedChannelClient *rcc = opaque;
+    RedChannelClientConnectivityMonitor *monitor = &rcc->connectivity_monitor;
+    int is_alive = TRUE;
+
+    if (monitor->state == CONNECTIVITY_STATE_BLOCKED) {
+        if (monitor->in_bytes == 0 && monitor->out_bytes == 0) {
+            if (!rcc->send_data.blocked && !red_channel_client_waiting_for_ack(rcc)) {
+                spice_error("mismatch between rcc-state and connectivity-state");
+            }
+            spice_debug("rcc is blocked; connection is idle");
+            is_alive = FALSE;
+        }
+    } else if (monitor->state == CONNECTIVITY_STATE_WAIT_PONG) {
+        if (monitor->in_bytes == 0) {
+            if (rcc->latency_monitor.state != PING_STATE_WARMUP &&
+                rcc->latency_monitor.state != PING_STATE_LATENCY) {
+                spice_error("mismatch between rcc-state and connectivity-state");
+            }
+            spice_debug("rcc waits for pong; connection is idle");
+            is_alive = FALSE;
+        }
+    }
+
+    if (is_alive) {
+        monitor->in_bytes = 0;
+        monitor->out_bytes = 0;
+        if (rcc->send_data.blocked || red_channel_client_waiting_for_ack(rcc)) {
+            monitor->state = CONNECTIVITY_STATE_BLOCKED;
+        } else if (rcc->latency_monitor.state == PING_STATE_WARMUP ||
+                   rcc->latency_monitor.state == PING_STATE_LATENCY) {
+            monitor->state = CONNECTIVITY_STATE_WAIT_PONG;
+        } else {
+             monitor->state = CONNECTIVITY_STATE_CONNECTED;
+        }
+        rcc->channel->core->timer_start(rcc->connectivity_monitor.timer,
+                                        rcc->connectivity_monitor.timeout);
+    } else {
+        monitor->state = CONNECTIVITY_STATE_DISCONNECTED;
+        spice_debug("rcc %p has not been responsive for more than %u ms, disconnecting",
+                    rcc, monitor->timeout);
+        red_channel_client_disconnect(rcc);
+    }
+}
+
+void red_channel_client_start_connectivity_monitoring(RedChannelClient *rcc, uint32_t timeout_ms)
+{
+    if (!red_channel_client_is_connected(rcc)) {
+        return;
+    }
+    spice_debug(NULL);
+    spice_assert(timeout_ms > 0);
+    /*
+     * If latency_monitor is not active, we activate it in order to enable
+     * periodic ping messages so that we will be be able to identify a disconnected
+     * channel-client even if there are no ongoing channel specific messages
+     * on this channel.
+     */
+    if (rcc->latency_monitor.timer == NULL) {
+        rcc->latency_monitor.timer = rcc->channel->core->timer_add(
+            red_channel_client_ping_timer, rcc);
+        if (!rcc->client->during_target_migrate) {
+            red_channel_client_start_ping_timer(rcc, PING_TEST_IDLE_NET_TIMEOUT_MS);
+        }
+        rcc->latency_monitor.roundtrip = -1;
+    }
+    if (rcc->connectivity_monitor.timer == NULL) {
+        rcc->connectivity_monitor.state = CONNECTIVITY_STATE_CONNECTED;
+        rcc->connectivity_monitor.timer = rcc->channel->core->timer_add(
+            red_channel_client_connectivity_timer, rcc);
+        rcc->connectivity_monitor.timeout = timeout_ms;
+        if (!rcc->client->during_target_migrate) {
+           rcc->channel->core->timer_start(rcc->connectivity_monitor.timer,
+                                           rcc->connectivity_monitor.timeout);
+        }
+    }
+}
+
 RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedClient  *client,
                                             RedsStream *stream,
                                             int monitor_latency,
@@ -859,6 +966,10 @@ static void red_channel_client_seamless_migration_done(RedChannelClient *rcc)
         if (rcc->latency_monitor.timer) {
             red_channel_client_start_ping_timer(rcc, PING_TEST_IDLE_NET_TIMEOUT_MS);
         }
+        if (rcc->connectivity_monitor.timer) {
+            rcc->channel->core->timer_start(rcc->connectivity_monitor.timer,
+                                            rcc->connectivity_monitor.timeout);
+        }
     }
     pthread_mutex_unlock(&rcc->client->lock);
 }
@@ -905,6 +1016,10 @@ void red_channel_client_default_migrate(RedChannelClient *rcc)
         rcc->channel->core->timer_remove(rcc->latency_monitor.timer);
         rcc->latency_monitor.timer = NULL;
     }
+    if (rcc->connectivity_monitor.timer) {
+       rcc->channel->core->timer_remove(rcc->connectivity_monitor.timer);
+        rcc->connectivity_monitor.timer = NULL;
+    }
     red_channel_client_pipe_add_type(rcc, PIPE_ITEM_TYPE_MIGRATE);
 }
 
@@ -1752,6 +1867,10 @@ void red_channel_client_disconnect(RedChannelClient *rcc)
         rcc->channel->core->timer_remove(rcc->latency_monitor.timer);
         rcc->latency_monitor.timer = NULL;
     }
+    if (rcc->connectivity_monitor.timer) {
+        rcc->channel->core->timer_remove(rcc->connectivity_monitor.timer);
+        rcc->connectivity_monitor.timer = NULL;
+    }
     red_channel_remove_client(rcc);
     rcc->channel->channel_cbs.on_disconnect(rcc);
 }
diff --git a/server/red_channel.h b/server/red_channel.h
index 1592896..676e1ef 100644
--- a/server/red_channel.h
+++ b/server/red_channel.h
@@ -236,6 +236,14 @@ typedef struct RedChannelClientLatencyMonitor {
     int64_t roundtrip;
 } RedChannelClientLatencyMonitor;
 
+typedef struct RedChannelClientConnectivityMonitor {
+    int state;
+    uint32_t out_bytes;
+    uint32_t in_bytes;
+    uint32_t timeout;
+    SpiceTimer *timer;
+} RedChannelClientConnectivityMonitor;
+
 struct RedChannelClient {
     RingItem channel_link;
     RingItem client_link;
@@ -289,6 +297,7 @@ struct RedChannelClient {
     int wait_migrate_flush_mark;
 
     RedChannelClientLatencyMonitor latency_monitor;
+    RedChannelClientConnectivityMonitor connectivity_monitor;
 };
 
 struct RedChannel {
@@ -448,6 +457,11 @@ SpiceMarshaller *red_channel_client_switch_to_urgent_sender(RedChannelClient *rc
 /* returns -1 if we don't have an estimation */
 int red_channel_client_get_roundtrip_ms(RedChannelClient *rcc);
 
+/*
+ * Checks periodically if the connection is still alive
+ */
+void red_channel_client_start_connectivity_monitoring(RedChannelClient *rcc, uint32_t timeout_ms);
+
 void red_channel_pipe_item_init(RedChannel *channel, PipeItem *item, int type);
 
 // TODO: add back the channel_pipe_add functionality - by adding reference counting
commit d1e7142a0f90e2b977d2a73d26dc5b09d7771826
Author: Yonit Halperin <yhalperi at redhat.com>
Date:   Wed Aug 14 09:38:12 2013 -0400

    red_channel: add on_input callback for tracing incoming bytes
    
    The callback will be used in the next patch.

diff --git a/server/red_channel.c b/server/red_channel.c
index 37b0c1c..bcf5a60 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -251,6 +251,7 @@ static void red_peer_handle_incoming(RedsStream *stream, IncomingHandler *handle
                 handler->cb->on_error(handler->opaque);
                 return;
             }
+            handler->cb->on_input(handler->opaque, bytes_read);
             handler->header_pos += bytes_read;
 
             if (handler->header_pos != handler->header.header_size) {
@@ -278,6 +279,7 @@ static void red_peer_handle_incoming(RedsStream *stream, IncomingHandler *handle
                 handler->cb->on_error(handler->opaque);
                 return;
             }
+            handler->cb->on_input(handler->opaque, bytes_read);
             handler->msg_pos += bytes_read;
             if (handler->msg_pos != msg_size) {
                 return;
@@ -390,6 +392,10 @@ static void red_channel_client_on_output(void *opaque, int n)
     stat_inc_counter(rcc->channel->out_bytes_counter, n);
 }
 
+static void red_channel_client_on_input(void *opaque, int n)
+{
+}
+
 static void red_channel_client_default_peer_on_error(RedChannelClient *rcc)
 {
     red_channel_client_disconnect(rcc);
@@ -935,6 +941,7 @@ RedChannel *red_channel_create(int size,
     channel->incoming_cb.handle_message = (handle_message_proc)handle_message;
     channel->incoming_cb.on_error =
         (on_incoming_error_proc)red_channel_client_default_peer_on_error;
+    channel->incoming_cb.on_input = red_channel_client_on_input;
     channel->outgoing_cb.get_msg_size = red_channel_client_peer_get_out_msg_size;
     channel->outgoing_cb.prepare = red_channel_client_peer_prepare_out_msg;
     channel->outgoing_cb.on_block = red_channel_client_peer_on_out_block;
diff --git a/server/red_channel.h b/server/red_channel.h
index 9021b3f..1592896 100644
--- a/server/red_channel.h
+++ b/server/red_channel.h
@@ -74,6 +74,7 @@ typedef uint8_t *(*alloc_msg_recv_buf_proc)(void *opaque, uint16_t type, uint32_
 typedef void (*release_msg_recv_buf_proc)(void *opaque,
                                           uint16_t type, uint32_t size, uint8_t *msg);
 typedef void (*on_incoming_error_proc)(void *opaque);
+typedef void (*on_input_proc)(void *opaque, int n);
 
 typedef struct IncomingHandlerInterface {
     handle_message_proc handle_message;
@@ -83,6 +84,7 @@ typedef struct IncomingHandlerInterface {
     // The following is an optional alternative to handle_message, used if not null
     spice_parse_channel_func_t parser;
     handle_parsed_proc handle_parsed;
+    on_input_proc on_input;
 } IncomingHandlerInterface;
 
 typedef struct IncomingHandler {
commit c1c08c289883455f025836f14eda7bfd86442ed7
Author: Yonit Halperin <yhalperi at redhat.com>
Date:   Tue Aug 13 15:40:16 2013 -0400

    spice_timer_queue: don't call timers repeatedly
    
    For channels that don't run as part of the main loop, we use
    spice_timer_queue, while for the other channels we use
    qemu timers support. The callbacks for setting timers are supplied to
    red_channel via SpiceCoreInterface, and their behavior should be
    consistent. qemu timers are called only once per each call to
    timer_start. This patch assigns the same behaviour to spice_timer_queue.

diff --git a/server/spice_timer_queue.c b/server/spice_timer_queue.c
index 833ab1d..8f6e9c8 100644
--- a/server/spice_timer_queue.c
+++ b/server/spice_timer_queue.c
@@ -261,9 +261,7 @@ void spice_timer_queue_cb(void)
             break;
         } else {
             timer->func(timer->opaque);
-            if (timer->is_active) {
-                _spice_timer_set(timer, timer->ms, now_ms);
-            }
+            spice_timer_cancel(timer);
         }
     }
 }


More information about the Spice-commits mailing list