[Spice-devel,06/11] server/red_channel: support network monitoring

Submitted by Yonit Halperin on April 8, 2012, 3:43 p.m.

Details

Message ID 1333899800-1341-6-git-send-email-yhalperi@redhat.com
State New
Headers show

Not browsing as part of any series.

Commit Message

Yonit Halperin April 8, 2012, 3:43 p.m.
If the client's channel has SPICE_COMMON_CAP_QOS_QUERY, the server
channel's can send it SPICE_MSG_QOS_QUERY. The client is supposed to
respond by SPICE_MSG_QOS_ACK right after it receives the one message
that follows SPICE_MSG_QOS_QUERY.
For channels that are characterized mainly by transmissions from the
server to the client, the time it takes to send "large enough" messages
can be used to calculate the current available bandwidth of the channel.

This patch also contains the corresponding update to the spice-common
submodule.

Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
---
 server/inputs_channel.c    |    1 +
 server/main_channel.c      |    5 +-
 server/red_channel.c       |  117 ++++++++++++++++++++++++++++++++++++++++++++
 server/red_channel.h       |   23 +++++++++
 server/red_tunnel_worker.c |    1 +
 server/red_worker.c        |    2 +-
 server/smartcard.c         |    1 +
 server/spicevmc.c          |    1 +
 spice-common               |    2 +-
 9 files changed, 149 insertions(+), 4 deletions(-)

Patch hide | download patch | download mbox

diff --git a/server/inputs_channel.c b/server/inputs_channel.c
index ad247f4..cc0db94 100644
--- a/server/inputs_channel.c
+++ b/server/inputs_channel.c
@@ -500,6 +500,7 @@  static void inputs_connect(RedChannel *channel, RedClient *client,
                                                           channel,
                                                           client,
                                                           stream,
+                                                          FALSE,
                                                           num_common_caps, common_caps,
                                                           num_caps, caps);
     icc->motion_count = 0;
diff --git a/server/main_channel.c b/server/main_channel.c
index 713f121..624b56d 100644
--- a/server/main_channel.c
+++ b/server/main_channel.c
@@ -996,8 +996,9 @@  static MainChannelClient *main_channel_client_create(MainChannel *main_chan, Red
 {
     MainChannelClient *mcc = (MainChannelClient*)
                              red_channel_client_create(sizeof(MainChannelClient), &main_chan->base,
-                                                       client, stream, num_common_caps,
-                                                       common_caps, num_caps, caps);
+                                                       client, stream, FALSE,
+                                                       num_common_caps, common_caps,
+                                                       num_caps, caps);
 
     mcc->connection_id = connection_id;
     mcc->bitrate_per_sec = ~0;
diff --git a/server/red_channel.c b/server/red_channel.c
index 4858bb5..c43c233 100644
--- a/server/red_channel.c
+++ b/server/red_channel.c
@@ -24,11 +24,15 @@ 
 
 #include <stdio.h>
 #include <stdint.h>
+#include <sys/ioctl.h>
+#include <sys/socket.h>
 #include <netinet/in.h>
 #include <netinet/tcp.h>
 #include <fcntl.h>
 #include <unistd.h>
 #include <errno.h>
+#include <time.h>
+#include <spice/protocol.h>
 
 #include "common/generated_server_marshallers.h"
 #include "common/ring.h"
@@ -37,6 +41,10 @@ 
 #include "red_channel.h"
 #include "reds.h"
 
+#define NET_MONITOR_TIMEOUT_MS 15000
+#define NET_MONITOR_QOS_QUERY_SIZE 100000
+#define NET_MONITOR_QOS_QUERY_SIZE_MIN 25000
+
 static void red_channel_client_event(int fd, int event, void *data);
 static void red_client_add_channel(RedClient *client, RedChannelClient *rcc);
 static void red_client_remove_channel(RedChannelClient *rcc);
@@ -514,8 +522,28 @@  int red_channel_client_test_remote_cap(RedChannelClient *rcc, uint32_t cap)
                           cap);
 }
 
+static void red_channel_client_net_monitor_qos_timer(void *opaque)
+{
+    RedChannelClient *rcc = opaque;
+    int *qos_state = &rcc->net_monitor.qos.state;
+
+    spice_assert(*qos_state != NET_MONITOR_STATE_INVALID);
+
+    if (*qos_state == NET_MONITOR_STATE_COMPLETE) {
+        *qos_state = NET_MONITOR_STATE_PENDING;
+        rcc->net_monitor.qos.size_th = NET_MONITOR_QOS_QUERY_SIZE;
+    } else {
+        spice_assert(*qos_state == NET_MONITOR_STATE_PENDING);
+        rcc->net_monitor.qos.size_th = MAX(NET_MONITOR_QOS_QUERY_SIZE_MIN,
+                                           (rcc->net_monitor.qos.size_th - 10000));
+        rcc->channel->core->timer_start(rcc->net_monitor.qos.timer, NET_MONITOR_TIMEOUT_MS / 2);
+        spice_debug("size_th=%d", rcc->net_monitor.qos.size_th);
+    }
+}
+
 RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedClient  *client,
                                             RedsStream *stream,
+                                            int monitor_net,
                                             int num_common_caps, uint32_t *common_caps,
                                             int num_caps, uint32_t *caps)
 {
@@ -570,6 +598,29 @@  RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedCl
     rcc->id = channel->clients_num;
     red_channel_add_client(channel, rcc);
     red_client_add_channel(client, rcc);
+
+
+    if (monitor_net) {
+        spice_assert(channel->core->timer_add);
+        spice_assert(channel->core->timer_remove);
+        spice_assert(channel->core->timer_start);
+        spice_assert(channel->core->timer_cancel);
+
+        if (red_channel_client_test_remote_common_cap(rcc, SPICE_COMMON_CAP_QOS_QUERY)) {
+            rcc->net_monitor.qos.timer = channel->core->timer_add(
+                red_channel_client_net_monitor_qos_timer, rcc);
+            spice_debug("qos monitoring active");
+            rcc->net_monitor.qos.state = NET_MONITOR_STATE_PENDING;
+            rcc->net_monitor.qos.size_th = NET_MONITOR_QOS_QUERY_SIZE;
+            channel->core->timer_start(rcc->net_monitor.qos.timer, NET_MONITOR_TIMEOUT_MS);
+        } else {
+            spice_printerr("channel %d id %d client %p: qos cap unavailable",
+                           rcc->channel->type,
+                           rcc->channel->id,
+                           rcc->client);
+        }
+    }
+
     return rcc;
 error:
     free(rcc);
@@ -906,6 +957,11 @@  static void red_channel_client_init_outgoing_messages_window(RedChannelClient *r
     red_channel_client_push(rcc);
 }
 
+uint64_t red_channel_client_get_qos_bit_rate(RedChannelClient *rcc)
+{
+    return rcc->net_monitor.qos.bit_rate;
+}
+
 // TODO: this function doesn't make sense because the window should be client (WAN/LAN)
 // specific
 void red_channel_init_outgoing_messages_window(RedChannel *channel)
@@ -944,6 +1000,28 @@  static void red_channel_handle_migrate_data(RedChannelClient *rcc, uint32_t size
     rcc->channel->channel_cbs.handle_migrate_data(rcc, size, message);
 }
 
+static void red_channel_client_handle_qos_ack(RedChannelClient *rcc)
+{
+    struct timespec now;
+    uint64_t send_time;
+
+    spice_debug(NULL);
+    clock_gettime(CLOCK_MONOTONIC, &now);
+    send_time = (now.tv_sec * 1000000000LL + now.tv_nsec) - rcc->net_monitor.qos.start_time;
+    rcc->net_monitor.qos.bit_rate = ((rcc->net_monitor.qos.size * 8) * 1000 * 1000 * 1000) /
+                                     send_time;
+    rcc->net_monitor.qos.timestamp = now.tv_sec * 1000 + (now.tv_nsec / 1000 /1000);
+
+    spice_debug("QOS    %lu (%p, %d, %d): bit rate %.2f (Mbps), size %lu time %lu (ns)",
+                rcc->net_monitor.qos.timestamp,
+                rcc->client, rcc->channel->type,
+                rcc->channel->id, (rcc->net_monitor.qos.bit_rate + 0.0) / 1024 / 1024,
+                rcc->net_monitor.qos.size,
+                send_time);
+    rcc->net_monitor.qos.state = NET_MONITOR_STATE_COMPLETE;
+    rcc->channel->core->timer_start(rcc->net_monitor.qos.timer, NET_MONITOR_TIMEOUT_MS);
+}
+
 int red_channel_client_handle_message(RedChannelClient *rcc, uint32_t size,
                                       uint16_t type, void *message)
 {
@@ -969,6 +1047,9 @@  int red_channel_client_handle_message(RedChannelClient *rcc, uint32_t size,
     case SPICE_MSGC_MIGRATE_DATA:
         red_channel_handle_migrate_data(rcc, size, message);
         break;
+    case SPICE_MSGC_QOS_ACK:
+        red_channel_client_handle_qos_ack(rcc);
+        break;
     default:
         spice_printerr("invalid message type %u", type);
         return FALSE;
@@ -999,6 +1080,30 @@  void red_channel_client_init_send_data(RedChannelClient *rcc, uint16_t msg_type,
     }
 }
 
+static void red_channel_client_send_qos_query(RedChannelClient *rcc, int size)
+{
+    struct timespec now;
+    int so_unsend_size = 0;
+
+    /* retrieving the occupied size of the socket's tcp snd buffer */
+    if (ioctl(rcc->stream->socket, TIOCOUTQ, &so_unsend_size) == -1) {
+        spice_printerr("ioctl(TIOCOUTQ) failed, %s", strerror(errno));
+        so_unsend_size = 0;
+    }
+
+    spice_debug(NULL);
+    clock_gettime(CLOCK_MONOTONIC, &now);
+    rcc->net_monitor.qos.state = NET_MONITOR_STATE_STARTED;
+    rcc->net_monitor.qos.start_time = now.tv_sec *1000 *1000 *1000 + now.tv_nsec;
+    spice_debug("new message size %d occupied tcp snd buf size %d", size, so_unsend_size);
+    rcc->net_monitor.qos.size = size + so_unsend_size;
+    rcc->channel->core->timer_cancel(rcc->net_monitor.qos.timer);
+
+    red_channel_client_switch_to_urgent_sender(rcc);
+    red_channel_client_init_send_data(rcc, SPICE_MSG_QOS_QUERY, NULL);
+    red_channel_client_begin_send_message(rcc);
+}
+
 void red_channel_client_begin_send_message(RedChannelClient *rcc)
 {
     SpiceMarshaller *m = rcc->send_data.marshaller;
@@ -1008,6 +1113,15 @@  void red_channel_client_begin_send_message(RedChannelClient *rcc)
         spice_printerr("BUG: header->type == 0");
         return;
     }
+
+    if (spice_marshaller_get_total_size(m) > rcc->net_monitor.qos.size_th) {
+        if (rcc->net_monitor.qos.state == NET_MONITOR_STATE_PENDING &&
+            !red_channel_client_urgent_marshaller_is_active(rcc)) {
+            red_channel_client_send_qos_query(rcc, spice_marshaller_get_total_size(m));
+            return;
+        }
+    }
+
     spice_marshaller_flush(m);
     rcc->send_data.size = spice_marshaller_get_total_size(m);
     rcc->send_data.header.set_msg_size(&rcc->send_data.header,
@@ -1199,6 +1313,9 @@  void red_channel_client_disconnect(RedChannelClient *rcc)
     }
     reds_stream_free(rcc->stream);
     rcc->stream = NULL;
+    if (rcc->net_monitor.qos.timer) {
+        rcc->channel->core->timer_remove(rcc->net_monitor.qos.timer);
+    }
     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 5418210..5c718fb 100644
--- a/server/red_channel.h
+++ b/server/red_channel.h
@@ -219,6 +219,13 @@  typedef struct RedChannelCapabilities {
 
 int test_capabilty(uint32_t *caps, int num_caps, uint32_t cap);
 
+enum NetMonitorState {
+    NET_MONITOR_STATE_INVALID,
+    NET_MONITOR_STATE_PENDING,
+    NET_MONITOR_STATE_STARTED,
+    NET_MONITOR_STATE_COMPLETE,
+};
+
 struct RedChannelClient {
     RingItem channel_link;
     RingItem client_link;
@@ -252,6 +259,18 @@  struct RedChannelClient {
         } urgent;
     } send_data;
 
+    struct {
+        struct {
+            int state;
+            uint64_t bit_rate;
+            uint64_t timestamp;
+            uint64_t start_time;
+            uint64_t size;
+            int size_th;
+            SpiceTimer *timer;
+        } qos;
+    } net_monitor;
+
     OutgoingHandler outgoing;
     IncomingHandler incoming;
     int during_send;
@@ -328,6 +347,7 @@  void red_channel_set_data(RedChannel *channel, void *data);
 
 RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedClient *client,
                                             RedsStream *stream,
+                                            int monitor_net,
                                             int num_common_caps, uint32_t *common_caps,
                                             int num_caps, uint32_t *caps);
 // TODO: tmp, for channels that don't use RedChannel yet (e.g., snd channel), but
@@ -393,6 +413,9 @@  void red_channel_client_begin_send_message(RedChannelClient *rcc);
  */
 SpiceMarshaller *red_channel_client_switch_to_urgent_sender(RedChannelClient *rcc);
 
+/* returns 0 if the bit rate has never been estimated */
+uint64_t red_channel_client_get_qos_bit_rate(RedChannelClient *rcc);
+
 void red_channel_pipe_item_init(RedChannel *channel, PipeItem *item, int type);
 
 // TODO: add back the channel_pipe_add functionality - by adding reference counting
diff --git a/server/red_tunnel_worker.c b/server/red_tunnel_worker.c
index 384c36d..9d77e5a 100644
--- a/server/red_tunnel_worker.c
+++ b/server/red_tunnel_worker.c
@@ -3451,6 +3451,7 @@  static void handle_tunnel_channel_link(RedChannel *channel, RedClient *client,
 
     tcc = (TunnelChannelClient*)red_channel_client_create(sizeof(TunnelChannelClient),
                                                           channel, client, stream,
+                                                          FALSE,
                                                           0, NULL, 0, NULL);
 
     tcc->worker = worker;
diff --git a/server/red_worker.c b/server/red_worker.c
index fd0e32e..9fa3afd 100644
--- a/server/red_worker.c
+++ b/server/red_worker.c
@@ -9720,7 +9720,7 @@  static CommonChannelClient *common_channel_client_create(int size,
 {
     MainChannelClient *mcc = red_client_get_main(client);
     RedChannelClient *rcc =
-        red_channel_client_create(size, &common->base, client, stream,
+        red_channel_client_create(size, &common->base, client, stream, FALSE,
                                   num_common_caps, common_caps, num_caps, caps);
     CommonChannelClient *common_cc = (CommonChannelClient*)rcc;
     common_cc->worker = common->worker;
diff --git a/server/smartcard.c b/server/smartcard.c
index 894053e..cd51d19 100644
--- a/server/smartcard.c
+++ b/server/smartcard.c
@@ -499,6 +499,7 @@  static void smartcard_connect(RedChannel *channel, RedClient *client,
     RedChannelClient *rcc;
 
     rcc = red_channel_client_create(sizeof(RedChannelClient), channel, client, stream,
+                                    FALSE,
                                     num_common_caps, common_caps,
                                     num_caps, caps);
     red_channel_client_ack_zero_messages_window(rcc);
diff --git a/server/spicevmc.c b/server/spicevmc.c
index 27123d4..67ede47 100644
--- a/server/spicevmc.c
+++ b/server/spicevmc.c
@@ -232,6 +232,7 @@  static void spicevmc_connect(RedChannel *channel, RedClient *client,
     }
 
     rcc = red_channel_client_create(sizeof(RedChannelClient), channel, client, stream,
+                                    FALSE,
                                     num_common_caps, common_caps,
                                     num_caps, caps);
     if (!rcc) {
diff --git a/spice-common b/spice-common
index cc613c0..5b11895 160000
--- a/spice-common
+++ b/spice-common
@@ -1 +1 @@ 
-Subproject commit cc613c0c3c1ba3fe7017528d0da998a210a35d75
+Subproject commit 5b1189540ad62f2c79815040e93e2ad567304cdb

Comments

On Sun, Apr 08, 2012 at 06:43:15PM +0300, Yonit Halperin wrote:
> If the client's channel has SPICE_COMMON_CAP_QOS_QUERY, the server
> channel's can send it SPICE_MSG_QOS_QUERY. The client is supposed to
> respond by SPICE_MSG_QOS_ACK right after it receives the one message
> that follows SPICE_MSG_QOS_QUERY.
> For channels that are characterized mainly by transmissions from the
> server to the client, the time it takes to send "large enough" messages
> can be used to calculate the current available bandwidth of the channel.
> 
> This patch also contains the corresponding update to the spice-common
> submodule.
> 
> Signed-off-by: Yonit Halperin <yhalperi@redhat.com>
> ---
>  server/inputs_channel.c    |    1 +
>  server/main_channel.c      |    5 +-
>  server/red_channel.c       |  117 ++++++++++++++++++++++++++++++++++++++++++++
>  server/red_channel.h       |   23 +++++++++
>  server/red_tunnel_worker.c |    1 +
>  server/red_worker.c        |    2 +-
>  server/smartcard.c         |    1 +
>  server/spicevmc.c          |    1 +
>  spice-common               |    2 +-
>  9 files changed, 149 insertions(+), 4 deletions(-)
> 
> diff --git a/server/inputs_channel.c b/server/inputs_channel.c
> index ad247f4..cc0db94 100644
> --- a/server/inputs_channel.c
> +++ b/server/inputs_channel.c
> @@ -500,6 +500,7 @@ static void inputs_connect(RedChannel *channel, RedClient *client,
>                                                            channel,
>                                                            client,
>                                                            stream,
> +                                                          FALSE,
>                                                            num_common_caps, common_caps,
>                                                            num_caps, caps);
>      icc->motion_count = 0;
> diff --git a/server/main_channel.c b/server/main_channel.c
> index 713f121..624b56d 100644
> --- a/server/main_channel.c
> +++ b/server/main_channel.c
> @@ -996,8 +996,9 @@ static MainChannelClient *main_channel_client_create(MainChannel *main_chan, Red
>  {
>      MainChannelClient *mcc = (MainChannelClient*)
>                               red_channel_client_create(sizeof(MainChannelClient), &main_chan->base,
> -                                                       client, stream, num_common_caps,
> -                                                       common_caps, num_caps, caps);
> +                                                       client, stream, FALSE,
> +                                                       num_common_caps, common_caps,
> +                                                       num_caps, caps);
>  
>      mcc->connection_id = connection_id;
>      mcc->bitrate_per_sec = ~0;
> diff --git a/server/red_channel.c b/server/red_channel.c
> index 4858bb5..c43c233 100644
> --- a/server/red_channel.c
> +++ b/server/red_channel.c
> @@ -24,11 +24,15 @@
>  
>  #include <stdio.h>
>  #include <stdint.h>
> +#include <sys/ioctl.h>
> +#include <sys/socket.h>
>  #include <netinet/in.h>
>  #include <netinet/tcp.h>
>  #include <fcntl.h>
>  #include <unistd.h>
>  #include <errno.h>
> +#include <time.h>
> +#include <spice/protocol.h>
>  
>  #include "common/generated_server_marshallers.h"
>  #include "common/ring.h"
> @@ -37,6 +41,10 @@
>  #include "red_channel.h"
>  #include "reds.h"
>  
> +#define NET_MONITOR_TIMEOUT_MS 15000
> +#define NET_MONITOR_QOS_QUERY_SIZE 100000
> +#define NET_MONITOR_QOS_QUERY_SIZE_MIN 25000
> +
>  static void red_channel_client_event(int fd, int event, void *data);
>  static void red_client_add_channel(RedClient *client, RedChannelClient *rcc);
>  static void red_client_remove_channel(RedChannelClient *rcc);
> @@ -514,8 +522,28 @@ int red_channel_client_test_remote_cap(RedChannelClient *rcc, uint32_t cap)
>                            cap);
>  }
>  
> +static void red_channel_client_net_monitor_qos_timer(void *opaque)
> +{
> +    RedChannelClient *rcc = opaque;
> +    int *qos_state = &rcc->net_monitor.qos.state;
> +
> +    spice_assert(*qos_state != NET_MONITOR_STATE_INVALID);
> +
> +    if (*qos_state == NET_MONITOR_STATE_COMPLETE) {
> +        *qos_state = NET_MONITOR_STATE_PENDING;
> +        rcc->net_monitor.qos.size_th = NET_MONITOR_QOS_QUERY_SIZE;

size_th ? what does that mean?

> +    } else {
> +        spice_assert(*qos_state == NET_MONITOR_STATE_PENDING);
> +        rcc->net_monitor.qos.size_th = MAX(NET_MONITOR_QOS_QUERY_SIZE_MIN,
> +                                           (rcc->net_monitor.qos.size_th - 10000));

Why the - 10000?

> +        rcc->channel->core->timer_start(rcc->net_monitor.qos.timer, NET_MONITOR_TIMEOUT_MS / 2);

Why half the timeout?

> +        spice_debug("size_th=%d", rcc->net_monitor.qos.size_th);
> +    }
> +}
> +
>  RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedClient  *client,
>                                              RedsStream *stream,
> +                                            int monitor_net,
>                                              int num_common_caps, uint32_t *common_caps,
>                                              int num_caps, uint32_t *caps)
>  {
> @@ -570,6 +598,29 @@ RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedCl
>      rcc->id = channel->clients_num;
>      red_channel_add_client(channel, rcc);
>      red_client_add_channel(client, rcc);
> +
> +
> +    if (monitor_net) {
> +        spice_assert(channel->core->timer_add);
> +        spice_assert(channel->core->timer_remove);
> +        spice_assert(channel->core->timer_start);
> +        spice_assert(channel->core->timer_cancel);
> +
> +        if (red_channel_client_test_remote_common_cap(rcc, SPICE_COMMON_CAP_QOS_QUERY)) {
> +            rcc->net_monitor.qos.timer = channel->core->timer_add(
> +                red_channel_client_net_monitor_qos_timer, rcc);
> +            spice_debug("qos monitoring active");
> +            rcc->net_monitor.qos.state = NET_MONITOR_STATE_PENDING;
> +            rcc->net_monitor.qos.size_th = NET_MONITOR_QOS_QUERY_SIZE;
> +            channel->core->timer_start(rcc->net_monitor.qos.timer, NET_MONITOR_TIMEOUT_MS);
> +        } else {
> +            spice_printerr("channel %d id %d client %p: qos cap unavailable",
> +                           rcc->channel->type,
> +                           rcc->channel->id,
> +                           rcc->client);
> +        }
> +    }
> +
>      return rcc;
>  error:
>      free(rcc);
> @@ -906,6 +957,11 @@ static void red_channel_client_init_outgoing_messages_window(RedChannelClient *r
>      red_channel_client_push(rcc);
>  }
>  
> +uint64_t red_channel_client_get_qos_bit_rate(RedChannelClient *rcc)
> +{
> +    return rcc->net_monitor.qos.bit_rate;
> +}
> +
>  // TODO: this function doesn't make sense because the window should be client (WAN/LAN)
>  // specific
>  void red_channel_init_outgoing_messages_window(RedChannel *channel)
> @@ -944,6 +1000,28 @@ static void red_channel_handle_migrate_data(RedChannelClient *rcc, uint32_t size
>      rcc->channel->channel_cbs.handle_migrate_data(rcc, size, message);
>  }
>  
> +static void red_channel_client_handle_qos_ack(RedChannelClient *rcc)
> +{
> +    struct timespec now;
> +    uint64_t send_time;
> +
> +    spice_debug(NULL);
> +    clock_gettime(CLOCK_MONOTONIC, &now);
> +    send_time = (now.tv_sec * 1000000000LL + now.tv_nsec) - rcc->net_monitor.qos.start_time;
> +    rcc->net_monitor.qos.bit_rate = ((rcc->net_monitor.qos.size * 8) * 1000 * 1000 * 1000) /
> +                                     send_time;
> +    rcc->net_monitor.qos.timestamp = now.tv_sec * 1000 + (now.tv_nsec / 1000 /1000);
> +
> +    spice_debug("QOS    %lu (%p, %d, %d): bit rate %.2f (Mbps), size %lu time %lu (ns)",
> +                rcc->net_monitor.qos.timestamp,
> +                rcc->client, rcc->channel->type,
> +                rcc->channel->id, (rcc->net_monitor.qos.bit_rate + 0.0) / 1024 / 1024,
> +                rcc->net_monitor.qos.size,
> +                send_time);
> +    rcc->net_monitor.qos.state = NET_MONITOR_STATE_COMPLETE;
> +    rcc->channel->core->timer_start(rcc->net_monitor.qos.timer, NET_MONITOR_TIMEOUT_MS);
> +}
> +
>  int red_channel_client_handle_message(RedChannelClient *rcc, uint32_t size,
>                                        uint16_t type, void *message)
>  {
> @@ -969,6 +1047,9 @@ int red_channel_client_handle_message(RedChannelClient *rcc, uint32_t size,
>      case SPICE_MSGC_MIGRATE_DATA:
>          red_channel_handle_migrate_data(rcc, size, message);
>          break;
> +    case SPICE_MSGC_QOS_ACK:
> +        red_channel_client_handle_qos_ack(rcc);
> +        break;
>      default:
>          spice_printerr("invalid message type %u", type);
>          return FALSE;
> @@ -999,6 +1080,30 @@ void red_channel_client_init_send_data(RedChannelClient *rcc, uint16_t msg_type,
>      }
>  }
>  
> +static void red_channel_client_send_qos_query(RedChannelClient *rcc, int size)
> +{
> +    struct timespec now;
> +    int so_unsend_size = 0;
> +
> +    /* retrieving the occupied size of the socket's tcp snd buffer */
> +    if (ioctl(rcc->stream->socket, TIOCOUTQ, &so_unsend_size) == -1) {
> +        spice_printerr("ioctl(TIOCOUTQ) failed, %s", strerror(errno));
> +        so_unsend_size = 0;

I think it's so_unsent_size
> +    }
> +
> +    spice_debug(NULL);
> +    clock_gettime(CLOCK_MONOTONIC, &now);
> +    rcc->net_monitor.qos.state = NET_MONITOR_STATE_STARTED;
> +    rcc->net_monitor.qos.start_time = now.tv_sec *1000 *1000 *1000 + now.tv_nsec;
> +    spice_debug("new message size %d occupied tcp snd buf size %d", size, so_unsend_size);
> +    rcc->net_monitor.qos.size = size + so_unsend_size;
> +    rcc->channel->core->timer_cancel(rcc->net_monitor.qos.timer);
> +
> +    red_channel_client_switch_to_urgent_sender(rcc);
> +    red_channel_client_init_send_data(rcc, SPICE_MSG_QOS_QUERY, NULL);
> +    red_channel_client_begin_send_message(rcc);
> +}
> +
>  void red_channel_client_begin_send_message(RedChannelClient *rcc)
>  {
>      SpiceMarshaller *m = rcc->send_data.marshaller;
> @@ -1008,6 +1113,15 @@ void red_channel_client_begin_send_message(RedChannelClient *rcc)
>          spice_printerr("BUG: header->type == 0");
>          return;
>      }
> +
> +    if (spice_marshaller_get_total_size(m) > rcc->net_monitor.qos.size_th) {
> +        if (rcc->net_monitor.qos.state == NET_MONITOR_STATE_PENDING &&
> +            !red_channel_client_urgent_marshaller_is_active(rcc)) {
> +            red_channel_client_send_qos_query(rcc, spice_marshaller_get_total_size(m));
> +            return;
> +        }
> +    }
> +
>      spice_marshaller_flush(m);
>      rcc->send_data.size = spice_marshaller_get_total_size(m);
>      rcc->send_data.header.set_msg_size(&rcc->send_data.header,
> @@ -1199,6 +1313,9 @@ void red_channel_client_disconnect(RedChannelClient *rcc)
>      }
>      reds_stream_free(rcc->stream);
>      rcc->stream = NULL;
> +    if (rcc->net_monitor.qos.timer) {
> +        rcc->channel->core->timer_remove(rcc->net_monitor.qos.timer);
> +    }
>      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 5418210..5c718fb 100644
> --- a/server/red_channel.h
> +++ b/server/red_channel.h
> @@ -219,6 +219,13 @@ typedef struct RedChannelCapabilities {
>  
>  int test_capabilty(uint32_t *caps, int num_caps, uint32_t cap);
>  
> +enum NetMonitorState {
> +    NET_MONITOR_STATE_INVALID,
> +    NET_MONITOR_STATE_PENDING,
> +    NET_MONITOR_STATE_STARTED,
> +    NET_MONITOR_STATE_COMPLETE,
> +};
> +
>  struct RedChannelClient {
>      RingItem channel_link;
>      RingItem client_link;
> @@ -252,6 +259,18 @@ struct RedChannelClient {
>          } urgent;
>      } send_data;
>  
> +    struct {
> +        struct {
> +            int state;
> +            uint64_t bit_rate;
> +            uint64_t timestamp;
> +            uint64_t start_time;
> +            uint64_t size;
> +            int size_th;

This could be unsigned I think.

Christophe

> +            SpiceTimer *timer;
> +        } qos;
> +    } net_monitor;
> +
>      OutgoingHandler outgoing;
>      IncomingHandler incoming;
>      int during_send;
> @@ -328,6 +347,7 @@ void red_channel_set_data(RedChannel *channel, void *data);
>  
>  RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedClient *client,
>                                              RedsStream *stream,
> +                                            int monitor_net,
>                                              int num_common_caps, uint32_t *common_caps,
>                                              int num_caps, uint32_t *caps);
>  // TODO: tmp, for channels that don't use RedChannel yet (e.g., snd channel), but
> @@ -393,6 +413,9 @@ void red_channel_client_begin_send_message(RedChannelClient *rcc);
>   */
>  SpiceMarshaller *red_channel_client_switch_to_urgent_sender(RedChannelClient *rcc);
>  
> +/* returns 0 if the bit rate has never been estimated */
> +uint64_t red_channel_client_get_qos_bit_rate(RedChannelClient *rcc);
> +
>  void red_channel_pipe_item_init(RedChannel *channel, PipeItem *item, int type);
>  
>  // TODO: add back the channel_pipe_add functionality - by adding reference counting
> diff --git a/server/red_tunnel_worker.c b/server/red_tunnel_worker.c
> index 384c36d..9d77e5a 100644
> --- a/server/red_tunnel_worker.c
> +++ b/server/red_tunnel_worker.c
> @@ -3451,6 +3451,7 @@ static void handle_tunnel_channel_link(RedChannel *channel, RedClient *client,
>  
>      tcc = (TunnelChannelClient*)red_channel_client_create(sizeof(TunnelChannelClient),
>                                                            channel, client, stream,
> +                                                          FALSE,
>                                                            0, NULL, 0, NULL);
>  
>      tcc->worker = worker;
> diff --git a/server/red_worker.c b/server/red_worker.c
> index fd0e32e..9fa3afd 100644
> --- a/server/red_worker.c
> +++ b/server/red_worker.c
> @@ -9720,7 +9720,7 @@ static CommonChannelClient *common_channel_client_create(int size,
>  {
>      MainChannelClient *mcc = red_client_get_main(client);
>      RedChannelClient *rcc =
> -        red_channel_client_create(size, &common->base, client, stream,
> +        red_channel_client_create(size, &common->base, client, stream, FALSE,
>                                    num_common_caps, common_caps, num_caps, caps);
>      CommonChannelClient *common_cc = (CommonChannelClient*)rcc;
>      common_cc->worker = common->worker;
> diff --git a/server/smartcard.c b/server/smartcard.c
> index 894053e..cd51d19 100644
> --- a/server/smartcard.c
> +++ b/server/smartcard.c
> @@ -499,6 +499,7 @@ static void smartcard_connect(RedChannel *channel, RedClient *client,
>      RedChannelClient *rcc;
>  
>      rcc = red_channel_client_create(sizeof(RedChannelClient), channel, client, stream,
> +                                    FALSE,
>                                      num_common_caps, common_caps,
>                                      num_caps, caps);
>      red_channel_client_ack_zero_messages_window(rcc);
> diff --git a/server/spicevmc.c b/server/spicevmc.c
> index 27123d4..67ede47 100644
> --- a/server/spicevmc.c
> +++ b/server/spicevmc.c
> @@ -232,6 +232,7 @@ static void spicevmc_connect(RedChannel *channel, RedClient *client,
>      }
>  
>      rcc = red_channel_client_create(sizeof(RedChannelClient), channel, client, stream,
> +                                    FALSE,
>                                      num_common_caps, common_caps,
>                                      num_caps, caps);
>      if (!rcc) {
> diff --git a/spice-common b/spice-common
> index cc613c0..5b11895 160000
> --- a/spice-common
> +++ b/spice-common
> @@ -1 +1 @@
> -Subproject commit cc613c0c3c1ba3fe7017528d0da998a210a35d75
> +Subproject commit 5b1189540ad62f2c79815040e93e2ad567304cdb
> -- 
> 1.7.7.6
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel@lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel
On 04/10/2012 03:34 PM, Christophe Fergeau wrote:
> On Sun, Apr 08, 2012 at 06:43:15PM +0300, Yonit Halperin wrote:
>> If the client's channel has SPICE_COMMON_CAP_QOS_QUERY, the server
>> channel's can send it SPICE_MSG_QOS_QUERY. The client is supposed to
>> respond by SPICE_MSG_QOS_ACK right after it receives the one message
>> that follows SPICE_MSG_QOS_QUERY.
>> For channels that are characterized mainly by transmissions from the
>> server to the client, the time it takes to send "large enough" messages
>> can be used to calculate the current available bandwidth of the channel.
>>
>> This patch also contains the corresponding update to the spice-common
>> submodule.
>>
>> Signed-off-by: Yonit Halperin<yhalperi@redhat.com>
>> ---
>>   server/inputs_channel.c    |    1 +
>>   server/main_channel.c      |    5 +-
>>   server/red_channel.c       |  117 ++++++++++++++++++++++++++++++++++++++++++++
>>   server/red_channel.h       |   23 +++++++++
>>   server/red_tunnel_worker.c |    1 +
>>   server/red_worker.c        |    2 +-
>>   server/smartcard.c         |    1 +
>>   server/spicevmc.c          |    1 +
>>   spice-common               |    2 +-
>>   9 files changed, 149 insertions(+), 4 deletions(-)
>>
>> diff --git a/server/inputs_channel.c b/server/inputs_channel.c
>> index ad247f4..cc0db94 100644
>> --- a/server/inputs_channel.c
>> +++ b/server/inputs_channel.c
>> @@ -500,6 +500,7 @@ static void inputs_connect(RedChannel *channel, RedClient *client,
>>                                                             channel,
>>                                                             client,
>>                                                             stream,
>> +                                                          FALSE,
>>                                                             num_common_caps, common_caps,
>>                                                             num_caps, caps);
>>       icc->motion_count = 0;
>> diff --git a/server/main_channel.c b/server/main_channel.c
>> index 713f121..624b56d 100644
>> --- a/server/main_channel.c
>> +++ b/server/main_channel.c
>> @@ -996,8 +996,9 @@ static MainChannelClient *main_channel_client_create(MainChannel *main_chan, Red
>>   {
>>       MainChannelClient *mcc = (MainChannelClient*)
>>                                red_channel_client_create(sizeof(MainChannelClient),&main_chan->base,
>> -                                                       client, stream, num_common_caps,
>> -                                                       common_caps, num_caps, caps);
>> +                                                       client, stream, FALSE,
>> +                                                       num_common_caps, common_caps,
>> +                                                       num_caps, caps);
>>
>>       mcc->connection_id = connection_id;
>>       mcc->bitrate_per_sec = ~0;
>> diff --git a/server/red_channel.c b/server/red_channel.c
>> index 4858bb5..c43c233 100644
>> --- a/server/red_channel.c
>> +++ b/server/red_channel.c
>> @@ -24,11 +24,15 @@
>>
>>   #include<stdio.h>
>>   #include<stdint.h>
>> +#include<sys/ioctl.h>
>> +#include<sys/socket.h>
>>   #include<netinet/in.h>
>>   #include<netinet/tcp.h>
>>   #include<fcntl.h>
>>   #include<unistd.h>
>>   #include<errno.h>
>> +#include<time.h>
>> +#include<spice/protocol.h>
>>
>>   #include "common/generated_server_marshallers.h"
>>   #include "common/ring.h"
>> @@ -37,6 +41,10 @@
>>   #include "red_channel.h"
>>   #include "reds.h"
>>
>> +#define NET_MONITOR_TIMEOUT_MS 15000
>> +#define NET_MONITOR_QOS_QUERY_SIZE 100000
>> +#define NET_MONITOR_QOS_QUERY_SIZE_MIN 25000
>> +
>>   static void red_channel_client_event(int fd, int event, void *data);
>>   static void red_client_add_channel(RedClient *client, RedChannelClient *rcc);
>>   static void red_client_remove_channel(RedChannelClient *rcc);
>> @@ -514,8 +522,28 @@ int red_channel_client_test_remote_cap(RedChannelClient *rcc, uint32_t cap)
>>                             cap);
>>   }
>>
>> +static void red_channel_client_net_monitor_qos_timer(void *opaque)
>> +{
>> +    RedChannelClient *rcc = opaque;
>> +    int *qos_state =&rcc->net_monitor.qos.state;
>> +
>> +    spice_assert(*qos_state != NET_MONITOR_STATE_INVALID);
>> +
>> +    if (*qos_state == NET_MONITOR_STATE_COMPLETE) {
>> +        *qos_state = NET_MONITOR_STATE_PENDING;
>> +        rcc->net_monitor.qos.size_th = NET_MONITOR_QOS_QUERY_SIZE;
>
> size_th ? what does that mean?
>
th=threshold. The minimal size we require for sending QOS_QUERY
>> +    } else {
>> +        spice_assert(*qos_state == NET_MONITOR_STATE_PENDING);
>> +        rcc->net_monitor.qos.size_th = MAX(NET_MONITOR_QOS_QUERY_SIZE_MIN,
>> +                                           (rcc->net_monitor.qos.size_th - 10000));
>
> Why the - 10000?
>
Since the timeout reached, and we no message reached the size limit, I 
chose to decrease the limit (for example, if the resolution is very 
small, we will never reach the threshold). 10000 was arbitrary choice.
>> +        rcc->channel->core->timer_start(rcc->net_monitor.qos.timer, NET_MONITOR_TIMEOUT_MS / 2);
>
> Why half the timeout?
>
In order not to wait too much time.
>> +        spice_debug("size_th=%d", rcc->net_monitor.qos.size_th);
>> +    }
>> +}
>> +
>>   RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedClient  *client,
>>                                               RedsStream *stream,
>> +                                            int monitor_net,
>>                                               int num_common_caps, uint32_t *common_caps,
>>                                               int num_caps, uint32_t *caps)
>>   {
>> @@ -570,6 +598,29 @@ RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedCl
>>       rcc->id = channel->clients_num;
>>       red_channel_add_client(channel, rcc);
>>       red_client_add_channel(client, rcc);
>> +
>> +
>> +    if (monitor_net) {
>> +        spice_assert(channel->core->timer_add);
>> +        spice_assert(channel->core->timer_remove);
>> +        spice_assert(channel->core->timer_start);
>> +        spice_assert(channel->core->timer_cancel);
>> +
>> +        if (red_channel_client_test_remote_common_cap(rcc, SPICE_COMMON_CAP_QOS_QUERY)) {
>> +            rcc->net_monitor.qos.timer = channel->core->timer_add(
>> +                red_channel_client_net_monitor_qos_timer, rcc);
>> +            spice_debug("qos monitoring active");
>> +            rcc->net_monitor.qos.state = NET_MONITOR_STATE_PENDING;
>> +            rcc->net_monitor.qos.size_th = NET_MONITOR_QOS_QUERY_SIZE;
>> +            channel->core->timer_start(rcc->net_monitor.qos.timer, NET_MONITOR_TIMEOUT_MS);
>> +        } else {
>> +            spice_printerr("channel %d id %d client %p: qos cap unavailable",
>> +                           rcc->channel->type,
>> +                           rcc->channel->id,
>> +                           rcc->client);
>> +        }
>> +    }
>> +
>>       return rcc;
>>   error:
>>       free(rcc);
>> @@ -906,6 +957,11 @@ static void red_channel_client_init_outgoing_messages_window(RedChannelClient *r
>>       red_channel_client_push(rcc);
>>   }
>>
>> +uint64_t red_channel_client_get_qos_bit_rate(RedChannelClient *rcc)
>> +{
>> +    return rcc->net_monitor.qos.bit_rate;
>> +}
>> +
>>   // TODO: this function doesn't make sense because the window should be client (WAN/LAN)
>>   // specific
>>   void red_channel_init_outgoing_messages_window(RedChannel *channel)
>> @@ -944,6 +1000,28 @@ static void red_channel_handle_migrate_data(RedChannelClient *rcc, uint32_t size
>>       rcc->channel->channel_cbs.handle_migrate_data(rcc, size, message);
>>   }
>>
>> +static void red_channel_client_handle_qos_ack(RedChannelClient *rcc)
>> +{
>> +    struct timespec now;
>> +    uint64_t send_time;
>> +
>> +    spice_debug(NULL);
>> +    clock_gettime(CLOCK_MONOTONIC,&now);
>> +    send_time = (now.tv_sec * 1000000000LL + now.tv_nsec) - rcc->net_monitor.qos.start_time;
>> +    rcc->net_monitor.qos.bit_rate = ((rcc->net_monitor.qos.size * 8) * 1000 * 1000 * 1000) /
>> +                                     send_time;
>> +    rcc->net_monitor.qos.timestamp = now.tv_sec * 1000 + (now.tv_nsec / 1000 /1000);
>> +
>> +    spice_debug("QOS    %lu (%p, %d, %d): bit rate %.2f (Mbps), size %lu time %lu (ns)",
>> +                rcc->net_monitor.qos.timestamp,
>> +                rcc->client, rcc->channel->type,
>> +                rcc->channel->id, (rcc->net_monitor.qos.bit_rate + 0.0) / 1024 / 1024,
>> +                rcc->net_monitor.qos.size,
>> +                send_time);
>> +    rcc->net_monitor.qos.state = NET_MONITOR_STATE_COMPLETE;
>> +    rcc->channel->core->timer_start(rcc->net_monitor.qos.timer, NET_MONITOR_TIMEOUT_MS);
>> +}
>> +
>>   int red_channel_client_handle_message(RedChannelClient *rcc, uint32_t size,
>>                                         uint16_t type, void *message)
>>   {
>> @@ -969,6 +1047,9 @@ int red_channel_client_handle_message(RedChannelClient *rcc, uint32_t size,
>>       case SPICE_MSGC_MIGRATE_DATA:
>>           red_channel_handle_migrate_data(rcc, size, message);
>>           break;
>> +    case SPICE_MSGC_QOS_ACK:
>> +        red_channel_client_handle_qos_ack(rcc);
>> +        break;
>>       default:
>>           spice_printerr("invalid message type %u", type);
>>           return FALSE;
>> @@ -999,6 +1080,30 @@ void red_channel_client_init_send_data(RedChannelClient *rcc, uint16_t msg_type,
>>       }
>>   }
>>
>> +static void red_channel_client_send_qos_query(RedChannelClient *rcc, int size)
>> +{
>> +    struct timespec now;
>> +    int so_unsend_size = 0;
>> +
>> +    /* retrieving the occupied size of the socket's tcp snd buffer */
>> +    if (ioctl(rcc->stream->socket, TIOCOUTQ,&so_unsend_size) == -1) {
>> +        spice_printerr("ioctl(TIOCOUTQ) failed, %s", strerror(errno));
>> +        so_unsend_size = 0;
>
> I think it's so_unsent_size
>> +    }
>> +
>> +    spice_debug(NULL);
>> +    clock_gettime(CLOCK_MONOTONIC,&now);
>> +    rcc->net_monitor.qos.state = NET_MONITOR_STATE_STARTED;
>> +    rcc->net_monitor.qos.start_time = now.tv_sec *1000 *1000 *1000 + now.tv_nsec;
>> +    spice_debug("new message size %d occupied tcp snd buf size %d", size, so_unsend_size);
>> +    rcc->net_monitor.qos.size = size + so_unsend_size;
>> +    rcc->channel->core->timer_cancel(rcc->net_monitor.qos.timer);
>> +
>> +    red_channel_client_switch_to_urgent_sender(rcc);
>> +    red_channel_client_init_send_data(rcc, SPICE_MSG_QOS_QUERY, NULL);
>> +    red_channel_client_begin_send_message(rcc);
>> +}
>> +
>>   void red_channel_client_begin_send_message(RedChannelClient *rcc)
>>   {
>>       SpiceMarshaller *m = rcc->send_data.marshaller;
>> @@ -1008,6 +1113,15 @@ void red_channel_client_begin_send_message(RedChannelClient *rcc)
>>           spice_printerr("BUG: header->type == 0");
>>           return;
>>       }
>> +
>> +    if (spice_marshaller_get_total_size(m)>  rcc->net_monitor.qos.size_th) {
>> +        if (rcc->net_monitor.qos.state == NET_MONITOR_STATE_PENDING&&
>> +            !red_channel_client_urgent_marshaller_is_active(rcc)) {
>> +            red_channel_client_send_qos_query(rcc, spice_marshaller_get_total_size(m));
>> +            return;
>> +        }
>> +    }
>> +
>>       spice_marshaller_flush(m);
>>       rcc->send_data.size = spice_marshaller_get_total_size(m);
>>       rcc->send_data.header.set_msg_size(&rcc->send_data.header,
>> @@ -1199,6 +1313,9 @@ void red_channel_client_disconnect(RedChannelClient *rcc)
>>       }
>>       reds_stream_free(rcc->stream);
>>       rcc->stream = NULL;
>> +    if (rcc->net_monitor.qos.timer) {
>> +        rcc->channel->core->timer_remove(rcc->net_monitor.qos.timer);
>> +    }
>>       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 5418210..5c718fb 100644
>> --- a/server/red_channel.h
>> +++ b/server/red_channel.h
>> @@ -219,6 +219,13 @@ typedef struct RedChannelCapabilities {
>>
>>   int test_capabilty(uint32_t *caps, int num_caps, uint32_t cap);
>>
>> +enum NetMonitorState {
>> +    NET_MONITOR_STATE_INVALID,
>> +    NET_MONITOR_STATE_PENDING,
>> +    NET_MONITOR_STATE_STARTED,
>> +    NET_MONITOR_STATE_COMPLETE,
>> +};
>> +
>>   struct RedChannelClient {
>>       RingItem channel_link;
>>       RingItem client_link;
>> @@ -252,6 +259,18 @@ struct RedChannelClient {
>>           } urgent;
>>       } send_data;
>>
>> +    struct {
>> +        struct {
>> +            int state;
>> +            uint64_t bit_rate;
>> +            uint64_t timestamp;
>> +            uint64_t start_time;
>> +            uint64_t size;
>> +            int size_th;
>
> This could be unsigned I think.
>
> Christophe
>
>> +            SpiceTimer *timer;
>> +        } qos;
>> +    } net_monitor;
>> +
>>       OutgoingHandler outgoing;
>>       IncomingHandler incoming;
>>       int during_send;
>> @@ -328,6 +347,7 @@ void red_channel_set_data(RedChannel *channel, void *data);
>>
>>   RedChannelClient *red_channel_client_create(int size, RedChannel *channel, RedClient *client,
>>                                               RedsStream *stream,
>> +                                            int monitor_net,
>>                                               int num_common_caps, uint32_t *common_caps,
>>                                               int num_caps, uint32_t *caps);
>>   // TODO: tmp, for channels that don't use RedChannel yet (e.g., snd channel), but
>> @@ -393,6 +413,9 @@ void red_channel_client_begin_send_message(RedChannelClient *rcc);
>>    */
>>   SpiceMarshaller *red_channel_client_switch_to_urgent_sender(RedChannelClient *rcc);
>>
>> +/* returns 0 if the bit rate has never been estimated */
>> +uint64_t red_channel_client_get_qos_bit_rate(RedChannelClient *rcc);
>> +
>>   void red_channel_pipe_item_init(RedChannel *channel, PipeItem *item, int type);
>>
>>   // TODO: add back the channel_pipe_add functionality - by adding reference counting
>> diff --git a/server/red_tunnel_worker.c b/server/red_tunnel_worker.c
>> index 384c36d..9d77e5a 100644
>> --- a/server/red_tunnel_worker.c
>> +++ b/server/red_tunnel_worker.c
>> @@ -3451,6 +3451,7 @@ static void handle_tunnel_channel_link(RedChannel *channel, RedClient *client,
>>
>>       tcc = (TunnelChannelClient*)red_channel_client_create(sizeof(TunnelChannelClient),
>>                                                             channel, client, stream,
>> +                                                          FALSE,
>>                                                             0, NULL, 0, NULL);
>>
>>       tcc->worker = worker;
>> diff --git a/server/red_worker.c b/server/red_worker.c
>> index fd0e32e..9fa3afd 100644
>> --- a/server/red_worker.c
>> +++ b/server/red_worker.c
>> @@ -9720,7 +9720,7 @@ static CommonChannelClient *common_channel_client_create(int size,
>>   {
>>       MainChannelClient *mcc = red_client_get_main(client);
>>       RedChannelClient *rcc =
>> -        red_channel_client_create(size,&common->base, client, stream,
>> +        red_channel_client_create(size,&common->base, client, stream, FALSE,
>>                                     num_common_caps, common_caps, num_caps, caps);
>>       CommonChannelClient *common_cc = (CommonChannelClient*)rcc;
>>       common_cc->worker = common->worker;
>> diff --git a/server/smartcard.c b/server/smartcard.c
>> index 894053e..cd51d19 100644
>> --- a/server/smartcard.c
>> +++ b/server/smartcard.c
>> @@ -499,6 +499,7 @@ static void smartcard_connect(RedChannel *channel, RedClient *client,
>>       RedChannelClient *rcc;
>>
>>       rcc = red_channel_client_create(sizeof(RedChannelClient), channel, client, stream,
>> +                                    FALSE,
>>                                       num_common_caps, common_caps,
>>                                       num_caps, caps);
>>       red_channel_client_ack_zero_messages_window(rcc);
>> diff --git a/server/spicevmc.c b/server/spicevmc.c
>> index 27123d4..67ede47 100644
>> --- a/server/spicevmc.c
>> +++ b/server/spicevmc.c
>> @@ -232,6 +232,7 @@ static void spicevmc_connect(RedChannel *channel, RedClient *client,
>>       }
>>
>>       rcc = red_channel_client_create(sizeof(RedChannelClient), channel, client, stream,
>> +                                    FALSE,
>>                                       num_common_caps, common_caps,
>>                                       num_caps, caps);
>>       if (!rcc) {
>> diff --git a/spice-common b/spice-common
>> index cc613c0..5b11895 160000
>> --- a/spice-common
>> +++ b/spice-common
>> @@ -1 +1 @@
>> -Subproject commit cc613c0c3c1ba3fe7017528d0da998a210a35d75
>> +Subproject commit 5b1189540ad62f2c79815040e93e2ad567304cdb
>> --
>> 1.7.7.6
>>
>> _______________________________________________
>> Spice-devel mailing list
>> Spice-devel@lists.freedesktop.org
>> http://lists.freedesktop.org/mailman/listinfo/spice-devel
On Tue, Apr 10, 2012 at 03:53:17PM +0300, Yonit Halperin wrote:
> On 04/10/2012 03:34 PM, Christophe Fergeau wrote:
> >On Sun, Apr 08, 2012 at 06:43:15PM +0300, Yonit Halperin wrote:
> >>+static void red_channel_client_net_monitor_qos_timer(void *opaque)
> >>+{
> >>+    RedChannelClient *rcc = opaque;
> >>+    int *qos_state =&rcc->net_monitor.qos.state;
> >>+
> >>+    spice_assert(*qos_state != NET_MONITOR_STATE_INVALID);
> >>+
> >>+    if (*qos_state == NET_MONITOR_STATE_COMPLETE) {
> >>+        *qos_state = NET_MONITOR_STATE_PENDING;
> >>+        rcc->net_monitor.qos.size_th = NET_MONITOR_QOS_QUERY_SIZE;
> >
> >size_th ? what does that mean?
> >
> th=threshold. The minimal size we require for sending QOS_QUERY

I'd use the full name then, same in one of the followup patches.

> >>+    } else {
> >>+        spice_assert(*qos_state == NET_MONITOR_STATE_PENDING);
> >>+        rcc->net_monitor.qos.size_th = MAX(NET_MONITOR_QOS_QUERY_SIZE_MIN,
> >>+                                           (rcc->net_monitor.qos.size_th - 10000));
> >
> >Why the - 10000?
> >
> Since the timeout reached, and we no message reached the size limit,
> I chose to decrease the limit (for example, if the resolution is
> very small, we will never reach the threshold). 10000 was arbitrary
> choice.
> >>+        rcc->channel->core->timer_start(rcc->net_monitor.qos.timer, NET_MONITOR_TIMEOUT_MS / 2);
> >
> >Why half the timeout?
> >
> In order not to wait too much time.


I'd add a comment explaining these 2 things then since they are arbitrary
choices, and things would probably work without these changes.

Christophe