[Spice-devel] [PATCH] Use non-zero data for initial ping

[Alon Levy]

On Mon, Feb 13, 2012 at 01:48:41PM -0600, Dan McGee wrote:
> This prevents compression over things such as VPN links misleading us on
> available bandwidth. The page used before was 4K worth of zeroes, which
> isn't a very realistic test at all.
> 
> We now use a generated sequence of bytes that vary in value and offset
> between consecutive values, causing compression programs to have a much
> harder time reducing our ping packets to nothing.
> 
> Here are some comparisons of before/after using a small standalone
> program that generates bytes in the same fashion:
> 
> Before:
>     4096 zerodata
>     43   zerodata.bz2
>     38   zerodata.gz
>     92   zerodata.xz
> 
> After:
>     4096 seqdata
>     1213 seqdata.bz2
>     4119 seqdata.gz
>     3208 seqdata.xz
> 
> Signed-off-by: Dan McGee <dpmcgee at gmail.com>
> ---
> 
> This was a TODO item on the following page:
> http://spice-space.org/page/Features/Bandwidth_monitoring
> 
> The standalone test program is below if anyone wants to try it out; simply pass
> '4096' or similar as the first argument and you will get 4096 "random" bytes on
> stdout that you can play with.
> 
> #include <stdio.h>
> #include <stdlib.h>
> 
> int main(int argc, char *argv[])
> {
> 	int i, total;
> 
> 	total = atoi(argv[1]);
> 
> 	for(i = 0; i < total; i++) {
> 		div_t result = div(i, 256);
> 		if(result.quot % 2 == 0) {
> 			fputc((result.quot + 1) * result.rem, stdout);
> 		} else {
> 			fputc(512 - (result.quot * result.rem), stdout);
> 		}
> 	}
> 	return 0;
> }
> 
> 
>  server/main_channel.c |   24 ++++++++++++++++--------
>  1 files changed, 16 insertions(+), 8 deletions(-)
> 
> diff --git a/server/main_channel.c b/server/main_channel.c
> index f7e1ab0..2ce20c0 100644
> --- a/server/main_channel.c
> +++ b/server/main_channel.c
> @@ -22,6 +22,7 @@
>  #include <inttypes.h>
>  #include <stdint.h>
>  #include <stdio.h>
> +#include <stdlib.h>
>  #include <unistd.h>
>  #include <sys/socket.h>
>  #include <netinet/in.h>
> @@ -45,8 +46,6 @@
>  #include "red_channel.h"
>  #include "generated_marshallers.h"
>  
> -#define ZERO_BUF_SIZE 4096
> -
>  #define REDS_MAX_SEND_IOVEC 100
>  
>  #define NET_TEST_WARMUP_BYTES 0
> @@ -54,8 +53,6 @@
>  
>  #define PING_INTERVAL (1000 * 10)
>  
> -static uint8_t zero_page[ZERO_BUF_SIZE] = {0};
> -
>  typedef struct RedsOutItem RedsOutItem;
>  struct RedsOutItem {
>      PipeItem base;
> @@ -335,17 +332,28 @@ static void main_channel_marshall_ping(SpiceMarshaller *m, int size, int ping_id
>  {
>      struct timespec time_space;
>      SpiceMsgPing ping;
> +    uint8_t *ping_data;
> +    int i;
>  
>      ping.id = ping_id;
>      clock_gettime(CLOCK_MONOTONIC, &time_space);
>      ping.timestamp = time_space.tv_sec * 1000000LL + time_space.tv_nsec / 1000LL;
>      spice_marshall_msg_ping(m, &ping);
>  
> -    while (size > 0) {
> -        int now = MIN(ZERO_BUF_SIZE, size);
> -        size -= now;
> -        spice_marshaller_add_ref(m, zero_page, now);
> +    ping_data = spice_malloc(size);

Are you freeing this anywhere? Why not leave the static above, just
rename it since it's no longer a zero_page? Note that you should then
keep the spice_marshaller_add_ref. spice_marshaller_add doesn't free for
you (I checked).

Other then that looks good, and like you say probably better - although
I'm not convinced that by checking several common desktop compression
programs you are checking whatever compression scheme vpn's are using.
But it's probably a good indication of the compressability.

> +    /* this produces ascending and descending byte runs which vary in offset
> +     * every 512 bytes, preventing prevents compression from being able to
> +     * influence the resulting size of the ping data too much */
> +    for(i = 0; i < size; i++) {
> +        div_t result = div(i, 256);
> +        if(result.quot % 2 == 0) {
> +            ping_data[i] = (result.quot + 1) * result.rem;
> +        } else {
> +            ping_data[i] = 512 - (result.quot * result.rem);
> +        }
>      }
> +
> +    spice_marshaller_add(m, ping_data, size);
>  }
>  
>  void main_channel_push_mouse_mode(MainChannel *main_chan, int current_mode,
> -- 
> 1.7.9
> 
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/spice-devel