[Spice-devel] [RFC spice-gtk 2/5] qos: introduce quality of service to channels
David Jaša
djasa at redhat.com
Wed Apr 12 12:21:28 UTC 2017
When we have overall notion of available bandwidth and the bandwidth is
constrained, we should be able to adjust Opus settings accordingly:
https://opus-codec.org/examples/#bitrate-scalability
The QoS may be as important for uplinks as is for downlinks because
many last-mile connections have asymmetrical bandwidth/latency/jitter.
David
On Fri, 2017-04-07 at 15:19 +0200, Victor Toso wrote:
> From: Victor Toso <me at victortoso.com>
>
> The QOS over spice channels is splitted in a few patches in order to
> easy review.
>
> This patch introduces 4 new functions that are only internally
> exposed
> to SpiceChannels, they are:
>
> * spice_session_qos_can_channel_write()
> * spice_session_qos_can_channel_read()
> * spice_session_qos_channel_has_write_nbytes()
> * spice_session_qos_channel_has_read_nbytes()
>
> Prior to each read or write, the SpiceChannel should query if it is
> allowed to perform the IO with spice_session_qos_can_channel_write
> (or
> read).
>
> After each successful read or write, the SpiceChannel should inform
> the QOS system how many bytes it has write/read.
>
> These functions should perform as fast as possible to not introduce
> delays in each IO.
>
> The logic behind QOS will be introduced in follow up patches.
>
> Related: https://bugs.freedesktop.org/show_bug.cgi?id=96598
>
> Signed-off-by: Victor Toso <victortoso at redhat.com>
> ---
> src/spice-channel.c | 28 ++++++++++++
> src/spice-option.c | 5 ++
> src/spice-session-priv.h | 5 ++
> src/spice-session.c | 117
> +++++++++++++++++++++++++++++++++++++++++++++++
> 4 files changed, 155 insertions(+)
>
> diff --git a/src/spice-channel.c b/src/spice-channel.c
> index b3fd521..f49950d 100644
> --- a/src/spice-channel.c
> +++ b/src/spice-channel.c
> @@ -837,6 +837,13 @@ static gint
> spice_channel_flush_wire_nonblocking(SpiceChannel *channel,
> return ret;
> }
>
> +static gboolean wait_qos_can_flush(gpointer data)
> +{
> + SpiceChannel *channel = SPICE_CHANNEL(data);
> + SpiceChannelPrivate *c = channel->priv;
> + return spice_session_qos_can_channel_write(c->session, channel);
> +}
> +
> /*
> * Write all 'data' of length 'datalen' bytes out to
> * the wire
> @@ -856,6 +863,12 @@ static void
> spice_channel_flush_wire(SpiceChannel *channel,
>
> if (c->has_error) return;
>
> + if (!g_coroutine_condition_wait(&c->coroutine,
> wait_qos_can_flush, channel)) {
> + CHANNEL_DEBUG(channel, "Closing the channel:
> spice_channel_flush failed qos");
> + c->has_error = TRUE;
> + return;
> + }
> +
> ret = spice_channel_flush_wire_nonblocking(channel,
> ptr+offset, datalen-offset, &cond);
> if (ret == -1) {
> if (cond != 0) {
> @@ -875,6 +888,7 @@ static void spice_channel_flush_wire(SpiceChannel
> *channel,
> }
> offset += ret;
> c->total_write_bytes += ret;
> + spice_session_qos_channel_has_write_nbytes(c->session,
> channel, ret);
> }
> }
>
> @@ -1063,6 +1077,13 @@ static int
> spice_channel_read_wire_nonblocking(SpiceChannel *channel,
> return ret;
> }
>
> +static gboolean wait_qos_can_read(gpointer data)
> +{
> + SpiceChannel *channel = SPICE_CHANNEL(data);
> + SpiceChannelPrivate *c = channel->priv;
> + return spice_session_qos_can_channel_read(c->session, channel);
> +}
> +
> /*
> * Read at least 1 more byte of data straight off the wire
> * into the requested buffer.
> @@ -1081,6 +1102,12 @@ static int
> spice_channel_read_wire(SpiceChannel *channel, void *data, size_t len
> return 0;
> }
>
> + if (!g_coroutine_condition_wait(&c->coroutine,
> wait_qos_can_read, channel)) {
> + CHANNEL_DEBUG(channel, "Closing the channel:
> spice_channel_read failed qos");
> + c->has_error = TRUE;
> + return 0;
> + }
> +
> ret = spice_channel_read_wire_nonblocking(channel, data,
> len, &cond);
>
> if (ret == -1) {
> @@ -1099,6 +1126,7 @@ static int spice_channel_read_wire(SpiceChannel
> *channel, void *data, size_t len
> return 0;
> }
>
> + spice_session_qos_channel_has_read_nbytes(c->session,
> channel, ret);
> return ret;
> }
> }
> diff --git a/src/spice-option.c b/src/spice-option.c
> index c04e978..bf81dea 100644
> --- a/src/spice-option.c
> +++ b/src/spice-option.c
> @@ -36,6 +36,7 @@ static char *usbredir_redirect_on_connect = NULL;
> static gboolean smartcard = FALSE;
> static gboolean disable_audio = FALSE;
> static gboolean disable_usbredir = FALSE;
> +static gboolean disable_qos = FALSE;
> static gint cache_size = 0;
> static gint glz_window_size = 0;
> static gchar *secure_channels = NULL;
> @@ -203,6 +204,8 @@ GOptionGroup* spice_get_option_group(void)
> N_("Subject of the host certificate (field=value pairs
> separated by commas)"), N_("<host-subject>") },
> { "spice-disable-audio", '\0', 0, G_OPTION_ARG_NONE,
> &disable_audio,
> N_("Disable audio support"), NULL },
> + { "spice-disable-qos", '\0', 0, G_OPTION_ARG_NONE,
> &disable_qos,
> + N_("Disable quality of service"), NULL },
> { "spice-smartcard", '\0', 0, G_OPTION_ARG_NONE, &smartcard,
> N_("Enable smartcard support"), NULL },
> { "spice-smartcard-certificates", '\0', 0,
> G_OPTION_ARG_STRING, &smartcard_certificates,
> @@ -312,6 +315,8 @@ void spice_set_session_option(SpiceSession
> *session)
> g_object_set(session, "enable-usbredir", FALSE, NULL);
> if (disable_audio)
> g_object_set(session, "enable-audio", FALSE, NULL);
> + if (disable_qos)
> + g_object_set(session, "enable-qos", FALSE, NULL);
> if (cache_size)
> g_object_set(session, "cache-size", cache_size, NULL);
> if (glz_window_size)
> diff --git a/src/spice-session-priv.h b/src/spice-session-priv.h
> index 03005aa..a65a7c0 100644
> --- a/src/spice-session-priv.h
> +++ b/src/spice-session-priv.h
> @@ -100,6 +100,11 @@ void spice_session_set_main_channel(SpiceSession
> *session, SpiceChannel *channel
> gboolean spice_session_set_migration_session(SpiceSession *session,
> SpiceSession *mig_session);
> SpiceAudio *spice_audio_get(SpiceSession *session, GMainContext
> *context);
> const gchar* spice_audio_data_mode_to_string(gint mode);
> +
> +gboolean spice_session_qos_can_channel_write(SpiceSession *session,
> SpiceChannel *channel);
> +gboolean spice_session_qos_can_channel_read(SpiceSession *session,
> SpiceChannel *channel);
> +void spice_session_qos_channel_has_write_nbytes(SpiceSession
> *session, SpiceChannel *channel, gssize nbytes);
> +void spice_session_qos_channel_has_read_nbytes(SpiceSession
> *session, SpiceChannel *channel, gssize nbytes);
> G_END_DECLS
>
> #endif /* __SPICE_CLIENT_SESSION_PRIV_H__ */
> diff --git a/src/spice-session.c b/src/spice-session.c
> index 6f8cf5e..f7c35b4 100644
> --- a/src/spice-session.c
> +++ b/src/spice-session.c
> @@ -68,6 +68,9 @@ struct _SpiceSessionPrivate {
> /* whether to enable smartcard event forwarding to the server */
> gboolean smartcard;
>
> + /* whether to enable quality of service */
> + gboolean qos_enabled;
> +
> /* list of certificates to use for the software smartcard reader
> if
> * enabled. For now, it has to contain exactly 3 certificates
> for
> * the software reader to be functional
> @@ -123,8 +126,15 @@ struct _SpiceSessionPrivate {
> SpiceUsbDeviceManager *usb_manager;
> SpicePlaybackChannel *playback_channel;
> PhodavServer *webdav;
> +
> + /* QoS */
> + GHashTable *qos_table;
> };
>
> +typedef struct session_channel_qos {
> + bool throttling_write_enabled;
> + bool throttling_read_enabled;
> +} session_channel_qos;
>
> /**
> * SECTION:spice-session
> @@ -204,6 +214,7 @@ enum {
> PROP_USERNAME,
> PROP_UNIX_PATH,
> PROP_PREF_COMPRESSION,
> + PROP_QOS,
> };
>
> /* signals */
> @@ -295,6 +306,7 @@ static void spice_session_init(SpiceSession
> *session)
> s->glz_window = glz_decoder_window_new();
> update_proxy(session, NULL);
>
> + s->qos_table = g_hash_table_new_full(g_direct_hash,
> g_direct_equal, NULL, g_free);
> s->usb_manager = spice_usb_device_manager_get(session, &err);
> if (err != NULL) {
> SPICE_DEBUG("Could not initialize SpiceUsbDeviceManager -
> %s", err->message);
> @@ -382,6 +394,7 @@ spice_session_finalize(GObject *gobject)
>
> g_clear_pointer(&s->pubkey, g_byte_array_unref);
> g_clear_pointer(&s->ca, g_byte_array_unref);
> + g_clear_pointer(&s->qos_table, g_hash_table_unref);
>
> /* Chain up to the parent class */
> if (G_OBJECT_CLASS(spice_session_parent_class)->finalize)
> @@ -696,6 +709,9 @@ static void
> spice_session_get_property(GObject *gobject,
> case PROP_PREF_COMPRESSION:
> g_value_set_enum(value, s->preferred_compression);
> break;
> + case PROP_QOS:
> + g_value_set_boolean(value, s->qos_enabled);
> + break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> break;
> @@ -835,6 +851,9 @@ static void
> spice_session_set_property(GObject *gobject,
> case PROP_PREF_COMPRESSION:
> s->preferred_compression = g_value_get_enum(value);
> break;
> + case PROP_QOS:
> + s->qos_enabled = g_value_get_boolean(value);
> + break;
> default:
> G_OBJECT_WARN_INVALID_PROPERTY_ID(gobject, prop_id, pspec);
> break;
> @@ -1260,6 +1279,22 @@ static void
> spice_session_class_init(SpiceSessionClass *klass)
> G_PARAM_READWRITE |
> G_PARAM_STATIC_STRINGS));
>
> + /**
> + * SpiceSession:enable-qos:
> + *
> + * If set to TRUE, the quality of service for channels will be
> enabled.
> + *
> + * Since: 0.34
> + **/
> + g_object_class_install_property
> + (gobject_class, PROP_QOS,
> + g_param_spec_boolean("enable-qos",
> + "Enable quality of service",
> + "Enable quality of service",
> + TRUE,
> + G_PARAM_READWRITE | G_PARAM_CONSTRUCT |
> + G_PARAM_STATIC_STRINGS));
> +
>
> /**
> * SpiceSession::channel-new:
> @@ -1529,6 +1564,7 @@ SpiceSession
> *spice_session_new_from_session(SpiceSession *session)
> "enable-smartcard", &c->smartcard,
> "enable-audio", &c->audio,
> "enable-usbredir", &c->usbredir,
> + "enable-qos", &c->qos_enabled,
> "ca", &c->ca,
> NULL);
>
> @@ -2226,12 +2262,17 @@ void spice_session_channel_new(SpiceSession
> *session, SpiceChannel *channel)
>
> SpiceSessionPrivate *s = session->priv;
> struct channel *item;
> + session_channel_qos *qos;
>
>
> item = g_new0(struct channel, 1);
> item->channel = channel;
> ring_add(&s->channels, &item->link);
>
> + qos = g_new0(session_channel_qos, 1);
> + qos->throttling_write_enabled = qos->throttling_read_enabled =
> false;
> + g_hash_table_insert(s->qos_table, channel, qos);
> +
> if (SPICE_IS_MAIN_CHANNEL(channel)) {
> gboolean all = spice_strv_contains(s->disable_effects,
> "all");
>
> @@ -2805,3 +2846,79 @@ gboolean
> spice_session_set_migration_session(SpiceSession *session,
> SpiceSession
>
> return TRUE;
> }
> +
> +G_GNUC_INTERNAL
> +gboolean spice_session_qos_can_channel_write(SpiceSession *session,
> + SpiceChannel *channel)
> +{
> + session_channel_qos *qos;
> + SpiceSessionPrivate *s;
> +
> + g_return_val_if_fail(SPICE_IS_SESSION(session), FALSE);
> + g_return_val_if_fail(SPICE_IS_CHANNEL(channel), FALSE);
> +
> + s = session->priv;
> + if (!s->qos_enabled)
> + return true;
> +
> + qos = g_hash_table_lookup(s->qos_table, channel);
> + g_return_val_if_fail(qos != NULL, FALSE);
> +
> + return !qos->throttling_write_enabled;
> +}
> +
> +G_GNUC_INTERNAL
> +gboolean spice_session_qos_can_channel_read(SpiceSession *session,
> + SpiceChannel *channel)
> +{
> + session_channel_qos *qos;
> + SpiceSessionPrivate *s;
> +
> + g_return_val_if_fail(SPICE_IS_SESSION(session), FALSE);
> + g_return_val_if_fail(SPICE_IS_CHANNEL(channel), FALSE);
> +
> + s = session->priv;
> + if (!s->qos_enabled)
> + return true;
> +
> + qos = g_hash_table_lookup(s->qos_table, channel);
> + g_return_val_if_fail(qos != NULL, FALSE);
> +
> + return !qos->throttling_read_enabled;
> +}
> +
> +G_GNUC_INTERNAL
> +void spice_session_qos_channel_has_read_nbytes(SpiceSession
> *session,
> + SpiceChannel
> *channel,
> + gssize nbytes)
> +{
> + SpiceSessionPrivate *s;
> +
> + g_return_if_fail(SPICE_IS_SESSION(session));
> + g_return_if_fail(SPICE_IS_CHANNEL(channel));
> +
> + s = session->priv;
> + if (!s->qos_enabled)
> + return;
> +
> + /* TODO */
> + return;
> +}
> +
> +G_GNUC_INTERNAL
> +void spice_session_qos_channel_has_write_nbytes(SpiceSession
> *session,
> + SpiceChannel
> *channel,
> + gssize nbytes)
> +{
> + SpiceSessionPrivate *s;
> +
> + g_return_if_fail(SPICE_IS_SESSION(session));
> + g_return_if_fail(SPICE_IS_CHANNEL(channel));
> +
> + s = session->priv;
> + if (!s->qos_enabled)
> + return;
> +
> + /* TODO */
> + return;
> +}
More information about the Spice-devel
mailing list