Skip to contentSkip to conversation

Implement the compression V2 data format for stub and lz4.

PATCH Superseded Arne Schwabe 3 participants 12 messages Oct 11, 2015 – Jan 4, 2016

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 Implement the compression V2 data format for stub and lz4.

It has been tested against v3 server and again itself. From James Mail:

Compression V2

I have observed that compression in many cases, even when enabled, often does not produce packet size reduction because much of the packet data typically generated by web sessions is already compressed. Further, the single byte that precedes the packet and indicates whether or not compression occurred has the unfortunate side effect of misaligning the IP packet in cases where compression did not occur. To remedy this, I propose a Compression V2 header that is optimized for the case where compression does not occur.

a. No compression occurred and first byte of IP/Ethernet packet
is NOT 0x50 (0 bytes of overhead and maintains alignment):

[ uncompressed IP/Ethernet packet ]

b. No compression occurred and first byte of IP/Ethernet packet

      is 0x50 (2 bytes of overhead but unlikely since no known
      IP packet can begin with 0x50):

        [ 0x50 ] [ 0x00 ] [ uncompressed IP/Ethernet packet ]

c. Compression occurred (2 bytes of overhead):

        [ 0x50 ] [ compression Alg ID ] [ compressed IP/Ethernet packet ]

      Compression Alg ID is one-byte algorithm identifier
      for LZ4 (0x1), LZO (0x2), or Snappy (0x3).

This approach has several beneficial effects:

1. In the common case where compression does not occur, no compression op is required, therefore there is zero overhead.

2. When compression does not occur, the IP/Ethernet packet alignment is retained.

3. This technique does not require any byte swapping with the tail of the packet which can potentially incur an expensive cache miss.

---
 src/openvpn/comp-lz4.c | 191 ++++++++++++++++++++++++++++++++++++++-----------
 src/openvpn/comp-lz4.h |   1 +
 src/openvpn/comp.c     |  81 ++++++++++++++++-----
 src/openvpn/comp.h     |  13 ++++
 src/openvpn/compstub.c |  53 ++++++++++++++
 src/openvpn/options.c  |  10 +++
 6 files changed, 290 insertions(+), 59 deletions(-)
src/openvpn/comp-lz4.c +151 -40
index 4651148..3144717 100644
@@ -51,7 +51,7 @@ static void
 lz4_compress_init (struct compress_context *compctx)
 {
   msg (D_INIT_MEDIUM, "LZ4 compression initializing");
-  ASSERT(compctx->flags & COMP_F_SWAP);
+  ASSERT(compctx->flags & COMP_F_SWAP || compctx->flags & COMP_F_COMPV2);
 }
 
 static void
@@ -59,16 +59,12 @@ lz4_compress_uninit (struct compress_context *compctx)
 {
 }
 
-static void
-lz4_compress (struct buffer *buf, struct buffer work,
-	       struct compress_context *compctx,
-	       const struct frame* frame)
+static bool
+do_lz4_compress (struct buffer *buf,
+		 struct buffer *work,
+		 struct compress_context *compctx,
+		 const struct frame* frame)
 {
-  bool compressed = false;
-
-  if (buf->len <= 0)
-    return;
-
   /*
    * In order to attempt compression, length must be at least COMPRESS_THRESHOLD.
    */
@@ -78,33 +74,50 @@ lz4_compress (struct buffer *buf, struct buffer work,
       int zlen_max = ps + COMP_EXTRA_BUFFER (ps);
       int zlen;
 
-      ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
-      ASSERT (buf_safe (&work, zlen_max));
+      ASSERT (buf_init (work, FRAME_HEADROOM (frame)));
+      ASSERT (buf_safe (work, zlen_max));
 
       if (buf->len > ps)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression buffer overflow");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(&work), BLEN(buf), zlen_max );
+      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(work), BLEN(buf), zlen_max );
 
       if (zlen <= 0)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression error");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      ASSERT (buf_safe (&work, zlen));
-      work.len = zlen;
-      compressed = true;
+      ASSERT (buf_safe (work, zlen));
+      work->len = zlen;
+
 
-      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work.len);
+      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work->len);
       compctx->pre_compress += buf->len;
-      compctx->post_compress += work.len;
+      compctx->post_compress += work->len;
+      return true;
     }
+  return false;
+}
+
+
+static void
+lz4_compress (struct buffer *buf, struct buffer work,
+	      struct compress_context *compctx,
+	      const struct frame* frame)
+{
+  if (buf->len <= 0)
+    return;
+  bool compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
 
   /* did compression save us anything? */
   {
@@ -128,13 +141,73 @@ lz4_compress (struct buffer *buf, struct buffer work,
   }
 }
 
+
+static void
+lz4v2_compress (struct buffer *buf, struct buffer work,
+		struct compress_context *compctx,
+		const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+   compressed = do_lz4_compress(buf, &work, compctx, frame);
+  if (buf->len <= 0)
+    return;
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything? */
+  {
+    if (compressed && work.len < buf->len)
+      {
+	ASSERT(buf_prepend(&work, 2));
+	uint8_t *head = BPTR (&work);
+	head[0] = COMP_ALGV2_INDICATOR_BYTE;
+	head[1] = COMP_ALGV2_LZ4;
+	*buf = work;
+      }
+    else
+      {
+	compv2_escape_data_ifneeded(buf);
+      }
+  }
+}
+
+void
+do_lz4_decompress(size_t zlen_max,
+		  struct buffer *work,
+		  struct buffer *buf,
+		  struct compress_context *compctx)
+{
+  int uncomp_len;
+  ASSERT (buf_safe (work, zlen_max));
+  uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(work), (size_t)BLEN(buf), zlen_max);
+  if (uncomp_len <= 0)
+    {
+      dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
+      buf->len = 0;
+      return;
+    }
+
+  ASSERT (buf_safe (work, uncomp_len));
+  work->len = uncomp_len;
+
+  dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work->len);
+  compctx->pre_decompress += buf->len;
+  compctx->post_decompress += work->len;
+
+  *buf = *work;
+}
+
 static void
 lz4_decompress (struct buffer *buf, struct buffer work,
-		 struct compress_context *compctx,
-		 const struct frame* frame)
+		struct compress_context *compctx,
+		const struct frame* frame)
 {
   size_t zlen_max = EXPANDED_SIZE (frame);
-  int uncomp_len;
   uint8_t c;		/* flag indicating whether or not our peer compressed */
 
   if (buf->len <= 0)
@@ -149,30 +222,60 @@ lz4_decompress (struct buffer *buf, struct buffer work,
     --buf->len;
     *head = *BEND (buf);
   }
+  buf_advance(buf,1);
+  c = *BPTR(buf);
 
-  if (c == LZ4_COMPRESS_BYTE)	/* packet was compressed */
+  if (c == COMP_ALGV2_LZ4)	/* packet was compressed */
     {
-      ASSERT (buf_safe (&work, zlen_max));
-      uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(&work), (size_t)BLEN(buf), zlen_max);
-      if (uncomp_len <= 0)
-	{
-	  dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
-	  buf->len = 0;
-	  return;
-	}
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
+    }
+  else
+    {
+      dmsg (D_COMP_ERRORS, "Bad LZ4 decompression header byte: %d", c);
+      buf->len = 0;
+    }
+}
 
-      ASSERT (buf_safe (&work, uncomp_len));
-      work.len = uncomp_len;
 
-      dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work.len);
-      compctx->pre_decompress += buf->len;
-      compctx->post_decompress += work.len;
 
-      *buf = work;
+static void
+lz4v2_decompress (struct buffer *buf, struct buffer work,
+		  struct compress_context *compctx,
+		  const struct frame* frame)
+{
+  size_t zlen_max = EXPANDED_SIZE (frame);
+  uint8_t c;		/* flag indicating whether or not our peer compressed */
+
+  if (buf->len <= 0)
+    return;
+
+  ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
+
+  /* do unframing/swap (assumes buf->len > 0) */
+  uint8_t *head = BPTR (buf);
+  c = *head;
+
+  /* Not compressed */
+  if (c != COMP_ALGV2_INDICATOR_BYTE) {
+    return;
+  }
+
+  /* Packet to short to make sense */
+  if (buf->len <= 1)
+    {
+      buf->len=0;
+      return;
+    }
+
+  c = head[1];
+  if (c == COMP_ALGV2_LZ4)	/* packet was compressed */
+    {
+      buf_advance(buf,2);
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
     }
-  else if (c == NO_COMPRESS_BYTE_SWAP)	/* packet was not compressed */
+  else if (c == COMP_ALGV2_UNCOMPRESSED)
     {
-      ;
+      buf_advance(buf,2);
     }
   else
     {
@@ -189,6 +292,14 @@ const struct compress_alg lz4_alg = {
   lz4_decompress
 };
 
+const struct compress_alg lz4v2_alg = {
+  "lz4v2",
+  lz4_compress_init,
+  lz4_compress_uninit,
+  lz4v2_compress,
+  lz4v2_decompress
+};
+
 #else
 static void dummy(void) {}
 #endif /* ENABLE_LZ4 */
src/openvpn/comp-lz4.h +1 -0
index ca1dfa9..9d3c664 100644
@@ -31,6 +31,7 @@
 #include "buffer.h"
 
 extern const struct compress_alg lz4_alg;
+extern const struct compress_alg lz4v2_alg;
 
 struct lz4_workspace
 {
src/openvpn/comp.c +62 -19

108 lines not on this page · raw message

src/openvpn/comp.h +13 -0
index 716b1c0..2cfabef 100644
@@ -43,11 +43,21 @@
 #define COMP_ALG_SNAPPY 3 /* Snappy algorithm (no longer supported) */
 #define COMP_ALG_LZ4    4 /* LZ4 algorithm */
 
+
+/* algorithm v2 */
+#define COMP_ALGV2_INDICATOR_BYTE 0x50
+#define COMP_ALGV2_UNCOMPRESSED 0
+#define COMP_ALGV2_LZ4	    1
+#define COMP_ALGV2_LZO	    2
+#define COMP_ALGV2_SNAPPY   3
+
 /* Compression flags */
 #define COMP_F_ADAPTIVE   (1<<0) /* COMP_ALG_LZO only */
 #define COMP_F_ASYM       (1<<1) /* only downlink is compressed, not uplink */
 #define COMP_F_SWAP       (1<<2) /* initial command byte is swapped with last byte in buffer to preserve payload alignment */
 #define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support compression stubs */
+#define COMP_F_COMPV2	  (1<<8) /* Use the V2 compression methode that only uses opcodes if compression is actually needed */
+
 
 /*
  * Length of prepended prefix on compressed packets
@@ -145,6 +155,7 @@ struct compress_context
 };
 
 extern const struct compress_alg comp_stub_alg;
+extern const struct compress_alg compv2_stub_alg;
 
 struct compress_context *comp_init(const struct compress_options *opt);
 
@@ -157,6 +168,8 @@ void comp_print_stats (const struct compress_context *compctx, struct status_out
 
 void comp_generate_peer_info_string(const struct compress_options *opt, struct buffer *out);
 
+void compv2_escape_data_ifneeded (struct buffer *buf);
+
 static inline bool
 comp_enabled(const struct compress_options *info)
 {
src/openvpn/compstub.c +53 -0

65 lines not on this page · raw message

src/openvpn/options.c +10 -0
index cfba728..6823167 100644
@@ -6286,6 +6286,11 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_STUB;
 	      options->comp.flags = (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
 	    }
+	  else if (streq(p[1], "stub-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
+	      options->comp.flags = (COMP_F_COMPV2);
+	    }
 #if defined(ENABLE_LZO)
 	  else if (streq (p[1], "lzo"))
 	    {
@@ -6299,6 +6304,11 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_LZ4;
 	      options->comp.flags = COMP_F_SWAP;
 	    }
+	  else if (streq (p[1], "lz4-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_LZ4;
+	      options->comp.flags = COMP_F_COMPV2;
+	    }
 #endif
 	  else
 	    {
Signature · 1 line
1.9.5 (Apple Git-50.3)
#1 lore raw

PATCH v2 1/1 Implement the compression V2 data format for stub and lz4.

V2: Fix an unintended change in the old lz4 decompress code.

It has been tested against v3 server and again itself. From James Mail:

Compression V2

I have observed that compression in many cases, even when enabled, often does not produce packet size reduction because much of the packet data typically generated by web sessions is already compressed. Further, the single byte that precedes the packet and indicates whether or not compression occurred has the unfortunate side effect of misaligning the IP packet in cases where compression did not occur. To remedy this, I propose a Compression V2 header that is optimized for the case where compression does not occur.

a. No compression occurred and first byte of IP/Ethernet packet
is NOT 0x50 (0 bytes of overhead and maintains alignment):

[ uncompressed IP/Ethernet packet ]

b. No compression occurred and first byte of IP/Ethernet packet

      is 0x50 (2 bytes of overhead but unlikely since no known
      IP packet can begin with 0x50):

        [ 0x50 ] [ 0x00 ] [ uncompressed IP/Ethernet packet ]

c. Compression occurred (2 bytes of overhead):

        [ 0x50 ] [ compression Alg ID ] [ compressed IP/Ethernet packet ]

      Compression Alg ID is one-byte algorithm identifier
      for LZ4 (0x1), LZO (0x2), or Snappy (0x3).

This approach has several beneficial effects:

1. In the common case where compression does not occur, no compression op is required, therefore there is zero overhead.

2. When compression does not occur, the IP/Ethernet packet alignment is retained.

3. This technique does not require any byte swapping with the tail of the packet which can potentially incur an expensive cache miss.

---
 src/openvpn/comp-lz4.c | 195 +++++++++++++++++++++++++++++++++++++++----------
 src/openvpn/comp-lz4.h |   1 +
 src/openvpn/comp.c     |  81 +++++++++++++++-----
 src/openvpn/comp.h     |  13 ++++
 src/openvpn/compstub.c |  52 +++++++++++++
 src/openvpn/options.c  |  10 +++
 6 files changed, 293 insertions(+), 59 deletions(-)
src/openvpn/comp-lz4.c +155 -40
index 4651148..91ab236 100644
@@ -51,7 +51,7 @@ static void
 lz4_compress_init (struct compress_context *compctx)
 {
   msg (D_INIT_MEDIUM, "LZ4 compression initializing");
-  ASSERT(compctx->flags & COMP_F_SWAP);
+  ASSERT(compctx->flags & COMP_F_SWAP || compctx->flags & COMP_F_COMPV2);
 }
 
 static void
@@ -59,16 +59,12 @@ lz4_compress_uninit (struct compress_context *compctx)
 {
 }
 
-static void
-lz4_compress (struct buffer *buf, struct buffer work,
-	       struct compress_context *compctx,
-	       const struct frame* frame)
+static bool
+do_lz4_compress (struct buffer *buf,
+		 struct buffer *work,
+		 struct compress_context *compctx,
+		 const struct frame* frame)
 {
-  bool compressed = false;
-
-  if (buf->len <= 0)
-    return;
-
   /*
    * In order to attempt compression, length must be at least COMPRESS_THRESHOLD.
    */
@@ -78,33 +74,50 @@ lz4_compress (struct buffer *buf, struct buffer work,
       int zlen_max = ps + COMP_EXTRA_BUFFER (ps);
       int zlen;
 
-      ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
-      ASSERT (buf_safe (&work, zlen_max));
+      ASSERT (buf_init (work, FRAME_HEADROOM (frame)));
+      ASSERT (buf_safe (work, zlen_max));
 
       if (buf->len > ps)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression buffer overflow");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(&work), BLEN(buf), zlen_max );
+      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(work), BLEN(buf), zlen_max );
 
       if (zlen <= 0)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression error");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      ASSERT (buf_safe (&work, zlen));
-      work.len = zlen;
-      compressed = true;
+      ASSERT (buf_safe (work, zlen));
+      work->len = zlen;
+
 
-      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work.len);
+      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work->len);
       compctx->pre_compress += buf->len;
-      compctx->post_compress += work.len;
+      compctx->post_compress += work->len;
+      return true;
     }
+  return false;
+}
+
+
+static void
+lz4_compress (struct buffer *buf, struct buffer work,
+	      struct compress_context *compctx,
+	      const struct frame* frame)
+{
+  if (buf->len <= 0)
+    return;
+  bool compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
 
   /* did compression save us anything? */
   {
@@ -128,13 +141,73 @@ lz4_compress (struct buffer *buf, struct buffer work,
   }
 }
 
+
+static void
+lz4v2_compress (struct buffer *buf, struct buffer work,
+		struct compress_context *compctx,
+		const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+   compressed = do_lz4_compress(buf, &work, compctx, frame);
+  if (buf->len <= 0)
+    return;
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything? */
+  {
+    if (compressed && work.len < buf->len)
+      {
+	ASSERT(buf_prepend(&work, 2));
+	uint8_t *head = BPTR (&work);
+	head[0] = COMP_ALGV2_INDICATOR_BYTE;
+	head[1] = COMP_ALGV2_LZ4;
+	*buf = work;
+      }
+    else
+      {
+	compv2_escape_data_ifneeded(buf);
+      }
+  }
+}
+
+void
+do_lz4_decompress(size_t zlen_max,
+		  struct buffer *work,
+		  struct buffer *buf,
+		  struct compress_context *compctx)
+{
+  int uncomp_len;
+  ASSERT (buf_safe (work, zlen_max));
+  uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(work), (size_t)BLEN(buf), zlen_max);
+  if (uncomp_len <= 0)
+    {
+      dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
+      buf->len = 0;
+      return;
+    }
+
+  ASSERT (buf_safe (work, uncomp_len));
+  work->len = uncomp_len;
+
+  dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work->len);
+  compctx->pre_decompress += buf->len;
+  compctx->post_decompress += work->len;
+
+  *buf = *work;
+}
+
 static void
 lz4_decompress (struct buffer *buf, struct buffer work,
-		 struct compress_context *compctx,
-		 const struct frame* frame)
+		struct compress_context *compctx,
+		const struct frame* frame)
 {
   size_t zlen_max = EXPANDED_SIZE (frame);
-  int uncomp_len;
   uint8_t c;		/* flag indicating whether or not our peer compressed */
 
   if (buf->len <= 0)
@@ -152,23 +225,7 @@ lz4_decompress (struct buffer *buf, struct buffer work,
 
   if (c == LZ4_COMPRESS_BYTE)	/* packet was compressed */
     {
-      ASSERT (buf_safe (&work, zlen_max));
-      uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(&work), (size_t)BLEN(buf), zlen_max);
-      if (uncomp_len <= 0)
-	{
-	  dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
-	  buf->len = 0;
-	  return;
-	}
-
-      ASSERT (buf_safe (&work, uncomp_len));
-      work.len = uncomp_len;
-
-      dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work.len);
-      compctx->pre_decompress += buf->len;
-      compctx->post_decompress += work.len;
-
-      *buf = work;
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
     }
   else if (c == NO_COMPRESS_BYTE_SWAP)	/* packet was not compressed */
     {
@@ -176,7 +233,57 @@ lz4_decompress (struct buffer *buf, struct buffer work,
     }
   else
     {
+        struct gc_arena gc = gc_new ();
       dmsg (D_COMP_ERRORS, "Bad LZ4 decompression header byte: %d", c);
+      msg (M_INFO, "PKT_DUMP: %s", format_hex (BPTR (buf), buf->len, 0, &gc));
+      buf->len = 0;
+    }
+}
+
+
+
+static void
+lz4v2_decompress (struct buffer *buf, struct buffer work,
+		  struct compress_context *compctx,
+		  const struct frame* frame)
+{
+  size_t zlen_max = EXPANDED_SIZE (frame);
+  uint8_t c;		/* flag indicating whether or not our peer compressed */
+
+  if (buf->len <= 0)
+    return;
+
+  ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
+
+  /* do unframing/swap (assumes buf->len > 0) */
+  uint8_t *head = BPTR (buf);
+  c = *head;
+
+  /* Not compressed */
+  if (c != COMP_ALGV2_INDICATOR_BYTE) {
+    return;
+  }
+
+  /* Packet to short to make sense */
+  if (buf->len <= 1)
+    {
+      buf->len=0;
+      return;
+    }
+
+  c = head[1];
+  if (c == COMP_ALGV2_LZ4)	/* packet was compressed */
+    {
+      buf_advance(buf,2);
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
+    }
+  else if (c == COMP_ALGV2_UNCOMPRESSED)
+    {
+      buf_advance(buf,2);
+    }
+  else
+    {
+      dmsg (D_COMP_ERRORS, "Bad LZ4v2 decompression header byte: %d", c);
       buf->len = 0;
     }
 }
@@ -189,6 +296,14 @@ const struct compress_alg lz4_alg = {
   lz4_decompress
 };
 
+const struct compress_alg lz4v2_alg = {
+  "lz4v2",
+  lz4_compress_init,
+  lz4_compress_uninit,
+  lz4v2_compress,
+  lz4v2_decompress
+};
+
 #else
 static void dummy(void) {}
 #endif /* ENABLE_LZ4 */
src/openvpn/comp-lz4.h +1 -0
index ca1dfa9..9d3c664 100644
@@ -31,6 +31,7 @@
 #include "buffer.h"
 
 extern const struct compress_alg lz4_alg;
+extern const struct compress_alg lz4v2_alg;
 
 struct lz4_workspace
 {
src/openvpn/comp.c +62 -19

108 lines not on this page · raw message

src/openvpn/comp.h +13 -0
index 716b1c0..2cfabef 100644
@@ -43,11 +43,21 @@
 #define COMP_ALG_SNAPPY 3 /* Snappy algorithm (no longer supported) */
 #define COMP_ALG_LZ4    4 /* LZ4 algorithm */
 
+
+/* algorithm v2 */
+#define COMP_ALGV2_INDICATOR_BYTE 0x50
+#define COMP_ALGV2_UNCOMPRESSED 0
+#define COMP_ALGV2_LZ4	    1
+#define COMP_ALGV2_LZO	    2
+#define COMP_ALGV2_SNAPPY   3
+
 /* Compression flags */
 #define COMP_F_ADAPTIVE   (1<<0) /* COMP_ALG_LZO only */
 #define COMP_F_ASYM       (1<<1) /* only downlink is compressed, not uplink */
 #define COMP_F_SWAP       (1<<2) /* initial command byte is swapped with last byte in buffer to preserve payload alignment */
 #define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support compression stubs */
+#define COMP_F_COMPV2	  (1<<8) /* Use the V2 compression methode that only uses opcodes if compression is actually needed */
+
 
 /*
  * Length of prepended prefix on compressed packets
@@ -145,6 +155,7 @@ struct compress_context
 };
 
 extern const struct compress_alg comp_stub_alg;
+extern const struct compress_alg compv2_stub_alg;
 
 struct compress_context *comp_init(const struct compress_options *opt);
 
@@ -157,6 +168,8 @@ void comp_print_stats (const struct compress_context *compctx, struct status_out
 
 void comp_generate_peer_info_string(const struct compress_options *opt, struct buffer *out);
 
+void compv2_escape_data_ifneeded (struct buffer *buf);
+
 static inline bool
 comp_enabled(const struct compress_options *info)
 {
src/openvpn/compstub.c +52 -0

64 lines not on this page · raw message

src/openvpn/options.c +10 -0
index 36290a0..709a446 100644
@@ -6323,6 +6323,11 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_STUB;
 	      options->comp.flags = (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
 	    }
+	  else if (streq(p[1], "stub-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
+	      options->comp.flags = (COMP_F_COMPV2);
+	    }
 #if defined(ENABLE_LZO)
 	  else if (streq (p[1], "lzo"))
 	    {
@@ -6336,6 +6341,11 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_LZ4;
 	      options->comp.flags = COMP_F_SWAP;
 	    }
+	  else if (streq (p[1], "lz4-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_LZ4;
+	      options->comp.flags = COMP_F_COMPV2;
+	    }
 #endif
 	  else
 	    {
Signature · 1 line
2.5.4 (Apple Git-61)
#2 lore raw

Hi Arne,

Some comments after a first review:

src/openvpn/comp-lz4.c lz4_compress (struct buffer *buf, struct buffer work, · line 116

Quoted 12 lines · Arne Schwabe

V2: Fix an unintended change in the old lz4 decompress code.

[..snip...]

src/openvpn/comp-lz4.c +8 -0
+static void
+lz4_compress (struct buffer *buf, struct buffer work,
+             struct compress_context *compctx,
+             const struct frame* frame)
+{
+  if (buf->len <= 0)
+    return;
+  bool compressed = do_lz4_compress(buf, &work, compctx, frame);

This looks like it will blow up on compilers that still not properly support C99 (ie MSVC); declaration-after-statement.

Quoted 11 lines
src/openvpn/comp-lz4.c +7 -0
@@ -128,13 +141,73 @@ lz4_compress (struct buffer *buf, struct buffer work,
   }
 }
 
+
+static void
+lz4v2_compress (struct buffer *buf, struct buffer work,
+               struct compress_context *compctx,
+               const struct frame* frame)
+{
+  bool compressed;

src/openvpn/comp-lz4.c lz4_compress (struct buffer *buf, struct buffer work, · line 160

Quoted 10 lines
src/openvpn/comp-lz4.c +10 -0
+  if (buf->len <= 0)
+    return;
+
+   compressed = do_lz4_compress(buf, &work, compctx, frame);
+  if (buf->len <= 0)
+    return;
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;

A space too many before compressed. And the second check on buf->len seems pointless.

src/openvpn/comp-lz4.c lz4_compress (struct buffer *buf, struct buffer work, · line 164

Quoted 3 lines
src/openvpn/comp-lz4.c +3 -0
+  /* did compression save us anything? */
+  {
+    if (compressed && work.len < buf->len)

We could also check against work.len + 2, since we will be adding two bytes of we are going to use compression. Should save some more bytes.

src/openvpn/comp-lz4.c lz4_decompress (struct buffer *buf, struct buffer work, · line 241

Quoted 10 lines
src/openvpn/comp-lz4.c +5 -0
@@ -176,7 +233,57 @@ lz4_decompress (struct buffer *buf, struct buffer work,
     }
   else
     {
+        struct gc_arena gc = gc_new ();
       dmsg (D_COMP_ERRORS, "Bad LZ4 decompression header byte: %d", c);
+      msg (M_INFO, "PKT_DUMP: %s", format_hex (BPTR (buf), buf->len, 0, &gc));
+      buf->len = 0;
+    }
+}

Indenting looks off. And don't forget to call gc_free().

src/openvpn/compstub.c stub_compress (struct buffer *buf, struct buffer work, · line 77

Quoted 10 lines
src/openvpn/compstub.c +2 -0
index 2ab7163..0f21a6f 100644
@@ -73,6 +73,8 @@ stub_compress (struct buffer *buf, struct buffer work,
     }
 }
 
+
+

Pure whitespace change?

-Steffan

#3 lore raw

PATCH v2 1/1 Implement the compression V2 data format for stub and lz4.

Patch V2: Fix minor issues found by Steffan

It has been tested against v3 server and again itself. From James Mail:

Compression V2

I have observed that compression in many cases, even when enabled, often does not produce packet size reduction because much of the packet data typically generated by web sessions is already compressed. Further, the single byte that precedes the packet and indicates whether or not compression occurred has the unfortunate side effect of misaligning the IP packet in cases where compression did not occur. To remedy this, I propose a Compression V2 header that is optimized for the case where compression does not occur.

a. No compression occurred and first byte of IP/Ethernet packet
is NOT 0x50 (0 bytes of overhead and maintains alignment):

[ uncompressed IP/Ethernet packet ]

b. No compression occurred and first byte of IP/Ethernet packet

      is 0x50 (2 bytes of overhead but unlikely since no known
      IP packet can begin with 0x50):

        [ 0x50 ] [ 0x00 ] [ uncompressed IP/Ethernet packet ]

c. Compression occurred (2 bytes of overhead):

        [ 0x50 ] [ compression Alg ID ] [ compressed IP/Ethernet packet ]

      Compression Alg ID is one-byte algorithm identifier
      for LZ4 (0x1), LZO (0x2), or Snappy (0x3).

This approach has several beneficial effects:

1. In the common case where compression does not occur, no compression op is required, therefore there is zero overhead.

2. When compression does not occur, the IP/Ethernet packet alignment is retained.

3. This technique does not require any byte swapping with the tail of the packet which can potentially incur an expensive cache miss.

---
 src/openvpn/comp-lz4.c | 198 ++++++++++++++++++++++++++++++++++++++-----------
 src/openvpn/comp-lz4.h |   1 +
 src/openvpn/comp.c     |  81 +++++++++++++++-----
 src/openvpn/comp.h     |  13 ++++
 src/openvpn/compstub.c |  52 +++++++++++++
 src/openvpn/options.c  |  10 +++
 6 files changed, 294 insertions(+), 61 deletions(-)
src/openvpn/comp-lz4.c +156 -42
index 4651148..9274535 100644
@@ -51,7 +51,7 @@ static void
 lz4_compress_init (struct compress_context *compctx)
 {
   msg (D_INIT_MEDIUM, "LZ4 compression initializing");
-  ASSERT(compctx->flags & COMP_F_SWAP);
+  ASSERT(compctx->flags & COMP_F_SWAP || compctx->flags & COMP_F_COMPV2);
 }
 
 static void
@@ -59,16 +59,12 @@ lz4_compress_uninit (struct compress_context *compctx)
 {
 }
 
-static void
-lz4_compress (struct buffer *buf, struct buffer work,
-	       struct compress_context *compctx,
-	       const struct frame* frame)
+static bool
+do_lz4_compress (struct buffer *buf,
+		 struct buffer *work,
+		 struct compress_context *compctx,
+		 const struct frame* frame)
 {
-  bool compressed = false;
-
-  if (buf->len <= 0)
-    return;
-
   /*
    * In order to attempt compression, length must be at least COMPRESS_THRESHOLD.
    */
@@ -78,38 +74,58 @@ lz4_compress (struct buffer *buf, struct buffer work,
       int zlen_max = ps + COMP_EXTRA_BUFFER (ps);
       int zlen;
 
-      ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
-      ASSERT (buf_safe (&work, zlen_max));
+      ASSERT (buf_init (work, FRAME_HEADROOM (frame)));
+      ASSERT (buf_safe (work, zlen_max));
 
       if (buf->len > ps)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression buffer overflow");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(&work), BLEN(buf), zlen_max );
+      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(work), BLEN(buf), zlen_max );
 
       if (zlen <= 0)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression error");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      ASSERT (buf_safe (&work, zlen));
-      work.len = zlen;
-      compressed = true;
+      ASSERT (buf_safe (work, zlen));
+      work->len = zlen;
+
 
-      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work.len);
+      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work->len);
       compctx->pre_compress += buf->len;
-      compctx->post_compress += work.len;
+      compctx->post_compress += work->len;
+      return true;
     }
+  return false;
+}
 
-  /* did compression save us anything? */
+
+static void
+lz4_compress (struct buffer *buf, struct buffer work,
+	      struct compress_context *compctx,
+	      const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything? Include 2 byte compression header
+     in calculation */
   {
     uint8_t comp_head_byte = NO_COMPRESS_BYTE_SWAP;
-    if (compressed && work.len < buf->len)
+    if (compressed && work.len +2 < buf->len)
       {
 	*buf = work;
 	comp_head_byte = LZ4_COMPRESS_BYTE;
@@ -128,13 +144,73 @@ lz4_compress (struct buffer *buf, struct buffer work,
   }
 }
 
+
+static void
+lz4v2_compress (struct buffer *buf, struct buffer work,
+		struct compress_context *compctx,
+		const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+  if (buf->len <= 0)
+    return;
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything? */
+  {
+    if (compressed && work.len < buf->len)
+      {
+	ASSERT(buf_prepend(&work, 2));
+	uint8_t *head = BPTR (&work);
+	head[0] = COMP_ALGV2_INDICATOR_BYTE;
+	head[1] = COMP_ALGV2_LZ4;
+	*buf = work;
+      }
+    else
+      {
+	compv2_escape_data_ifneeded(buf);
+      }
+  }
+}
+
+void
+do_lz4_decompress(size_t zlen_max,
+		  struct buffer *work,
+		  struct buffer *buf,
+		  struct compress_context *compctx)
+{
+  int uncomp_len;
+  ASSERT (buf_safe (work, zlen_max));
+  uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(work), (size_t)BLEN(buf), zlen_max);
+  if (uncomp_len <= 0)
+    {
+      dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
+      buf->len = 0;
+      return;
+    }
+
+  ASSERT (buf_safe (work, uncomp_len));
+  work->len = uncomp_len;
+
+  dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work->len);
+  compctx->pre_decompress += buf->len;
+  compctx->post_decompress += work->len;
+
+  *buf = *work;
+}
+
 static void
 lz4_decompress (struct buffer *buf, struct buffer work,
-		 struct compress_context *compctx,
-		 const struct frame* frame)
+		struct compress_context *compctx,
+		const struct frame* frame)
 {
   size_t zlen_max = EXPANDED_SIZE (frame);
-  int uncomp_len;
   uint8_t c;		/* flag indicating whether or not our peer compressed */
 
   if (buf->len <= 0)
@@ -152,23 +228,7 @@ lz4_decompress (struct buffer *buf, struct buffer work,
 
   if (c == LZ4_COMPRESS_BYTE)	/* packet was compressed */
     {
-      ASSERT (buf_safe (&work, zlen_max));
-      uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(&work), (size_t)BLEN(buf), zlen_max);
-      if (uncomp_len <= 0)
-	{
-	  dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
-	  buf->len = 0;
-	  return;
-	}
-
-      ASSERT (buf_safe (&work, uncomp_len));
-      work.len = uncomp_len;
-
-      dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work.len);
-      compctx->pre_decompress += buf->len;
-      compctx->post_decompress += work.len;
-
-      *buf = work;
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
     }
   else if (c == NO_COMPRESS_BYTE_SWAP)	/* packet was not compressed */
     {
@@ -181,6 +241,52 @@ lz4_decompress (struct buffer *buf, struct buffer work,
     }
 }
 
+static void
+lz4v2_decompress (struct buffer *buf, struct buffer work,
+		  struct compress_context *compctx,
+		  const struct frame* frame)
+{
+  size_t zlen_max = EXPANDED_SIZE (frame);
+  uint8_t c;		/* flag indicating whether or not our peer compressed */
+
+  if (buf->len <= 0)
+    return;
+
+  ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
+
+  /* do unframing/swap (assumes buf->len > 0) */
+  uint8_t *head = BPTR (buf);
+  c = *head;
+
+  /* Not compressed */
+  if (c != COMP_ALGV2_INDICATOR_BYTE) {
+    return;
+  }
+
+  /* Packet to short to make sense */
+  if (buf->len <= 1)
+    {
+      buf->len=0;
+      return;
+    }
+
+  c = head[1];
+  if (c == COMP_ALGV2_LZ4)	/* packet was compressed */
+    {
+      buf_advance(buf,2);
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
+    }
+  else if (c == COMP_ALGV2_UNCOMPRESSED)
+    {
+      buf_advance(buf,2);
+    }
+  else
+    {
+      dmsg (D_COMP_ERRORS, "Bad LZ4v2 decompression header byte: %d", c);
+      buf->len = 0;
+    }
+}
+
 const struct compress_alg lz4_alg = {
   "lz4",
   lz4_compress_init,
@@ -189,6 +295,14 @@ const struct compress_alg lz4_alg = {
   lz4_decompress
 };
 
+const struct compress_alg lz4v2_alg = {
+  "lz4v2",
+  lz4_compress_init,
+  lz4_compress_uninit,
+  lz4v2_compress,
+  lz4v2_decompress
+};
+
 #else
 static void dummy(void) {}
 #endif /* ENABLE_LZ4 */
src/openvpn/comp-lz4.h +1 -0
index ca1dfa9..9d3c664 100644
@@ -31,6 +31,7 @@
 #include "buffer.h"
 
 extern const struct compress_alg lz4_alg;
+extern const struct compress_alg lz4v2_alg;
 
 struct lz4_workspace
 {
src/openvpn/comp.c +62 -19

108 lines not on this page · raw message

src/openvpn/comp.h +13 -0
index 716b1c0..2cfabef 100644
@@ -43,11 +43,21 @@
 #define COMP_ALG_SNAPPY 3 /* Snappy algorithm (no longer supported) */
 #define COMP_ALG_LZ4    4 /* LZ4 algorithm */
 
+
+/* algorithm v2 */
+#define COMP_ALGV2_INDICATOR_BYTE 0x50
+#define COMP_ALGV2_UNCOMPRESSED 0
+#define COMP_ALGV2_LZ4	    1
+#define COMP_ALGV2_LZO	    2
+#define COMP_ALGV2_SNAPPY   3
+
 /* Compression flags */
 #define COMP_F_ADAPTIVE   (1<<0) /* COMP_ALG_LZO only */
 #define COMP_F_ASYM       (1<<1) /* only downlink is compressed, not uplink */
 #define COMP_F_SWAP       (1<<2) /* initial command byte is swapped with last byte in buffer to preserve payload alignment */
 #define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support compression stubs */
+#define COMP_F_COMPV2	  (1<<8) /* Use the V2 compression methode that only uses opcodes if compression is actually needed */
+
 
 /*
  * Length of prepended prefix on compressed packets
@@ -145,6 +155,7 @@ struct compress_context
 };
 
 extern const struct compress_alg comp_stub_alg;
+extern const struct compress_alg compv2_stub_alg;
 
 struct compress_context *comp_init(const struct compress_options *opt);
 
@@ -157,6 +168,8 @@ void comp_print_stats (const struct compress_context *compctx, struct status_out
 
 void comp_generate_peer_info_string(const struct compress_options *opt, struct buffer *out);
 
+void compv2_escape_data_ifneeded (struct buffer *buf);
+
 static inline bool
 comp_enabled(const struct compress_options *info)
 {
src/openvpn/compstub.c +52 -0

64 lines not on this page · raw message

src/openvpn/options.c +10 -0
index 5a83897..2b29704 100644
@@ -6323,6 +6323,11 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_STUB;
 	      options->comp.flags = (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
 	    }
+	  else if (streq(p[1], "stub-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
+	      options->comp.flags = (COMP_F_COMPV2);
+	    }
 #if defined(ENABLE_LZO)
 	  else if (streq (p[1], "lzo"))
 	    {
@@ -6336,6 +6341,11 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_LZ4;
 	      options->comp.flags = COMP_F_SWAP;
 	    }
+	  else if (streq (p[1], "lz4-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_LZ4;
+	      options->comp.flags = COMP_F_COMPV2;
+	    }
 #endif
 	  else
 	    {
Signature · 1 line
2.5.4 (Apple Git-61)
#4 lore raw

PATCH v3 1/1 Implement the compression V2 data format for stub and lz4.

In reply to Steffan Karger

Patch V2: Fix minor issues found by Steffan
Patch V3: split wire codes and compression flags

It has been tested against v3 server and again itself. From James Mail:

Compression V2

I have observed that compression in many cases, even when enabled, often does not produce packet size reduction because much of the packet data typically generated by web sessions is already compressed. Further, the single byte that precedes the packet and indicates whether or not compression occurred has the unfortunate side effect of misaligning the IP packet in cases where compression did not occur. To remedy this, I propose a Compression V2 header that is optimized for the case where compression does not occur.

a. No compression occurred and first byte of IP/Ethernet packet
is NOT 0x50 (0 bytes of overhead and maintains alignment):

[ uncompressed IP/Ethernet packet ]

b. No compression occurred and first byte of IP/Ethernet packet

      is 0x50 (2 bytes of overhead but unlikely since no known
      IP packet can begin with 0x50):

        [ 0x50 ] [ 0x00 ] [ uncompressed IP/Ethernet packet ]

c. Compression occurred (2 bytes of overhead):

        [ 0x50 ] [ compression Alg ID ] [ compressed IP/Ethernet packet ]

      Compression Alg ID is one-byte algorithm identifier
      for LZ4 (0x1), LZO (0x2), or Snappy (0x3).

This approach has several beneficial effects:

1. In the common case where compression does not occur, no compression op is required, therefore there is zero overhead.

2. When compression does not occur, the IP/Ethernet packet alignment is retained.

3. This technique does not require any byte swapping with the tail of the packet which can potentially incur an expensive cache miss.

---
 src/openvpn/comp-lz4.c | 203 ++++++++++++++++++++++++++++++++++++++-----------
 src/openvpn/comp-lz4.h |   1 +
 src/openvpn/comp.c     |  31 ++++++++
 src/openvpn/comp.h     |  25 ++++++
 src/openvpn/compstub.c |  52 +++++++++++++
 src/openvpn/lzo.c      |   3 -
 src/openvpn/options.c  |   8 ++
 7 files changed, 277 insertions(+), 46 deletions(-)
src/openvpn/comp-lz4.c +160 -43
index 4651148..dad123b 100644
@@ -44,9 +44,6 @@
 
 #include "memdbg.h"
 
-/* Initial command byte to tell our peer if we compressed */
-#define LZ4_COMPRESS_BYTE 0x69
-
 static void
 lz4_compress_init (struct compress_context *compctx)
 {
@@ -55,20 +52,22 @@ lz4_compress_init (struct compress_context *compctx)
 }
 
 static void
-lz4_compress_uninit (struct compress_context *compctx)
+lz4_compress_initv2 (struct compress_context *compctx)
 {
+  msg (D_INIT_MEDIUM, "LZ4 compression initializing");
 }
 
 static void
-lz4_compress (struct buffer *buf, struct buffer work,
-	       struct compress_context *compctx,
-	       const struct frame* frame)
+lz4_compress_uninit (struct compress_context *compctx)
 {
-  bool compressed = false;
-
-  if (buf->len <= 0)
-    return;
+}
 
+static bool
+do_lz4_compress (struct buffer *buf,
+		 struct buffer *work,
+		 struct compress_context *compctx,
+		 const struct frame* frame)
+{
   /*
    * In order to attempt compression, length must be at least COMPRESS_THRESHOLD.
    */
@@ -78,38 +77,58 @@ lz4_compress (struct buffer *buf, struct buffer work,
       int zlen_max = ps + COMP_EXTRA_BUFFER (ps);
       int zlen;
 
-      ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
-      ASSERT (buf_safe (&work, zlen_max));
+      ASSERT (buf_init (work, FRAME_HEADROOM (frame)));
+      ASSERT (buf_safe (work, zlen_max));
 
       if (buf->len > ps)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression buffer overflow");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(&work), BLEN(buf), zlen_max );
+      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(work), BLEN(buf), zlen_max );
 
       if (zlen <= 0)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression error");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      ASSERT (buf_safe (&work, zlen));
-      work.len = zlen;
-      compressed = true;
+      ASSERT (buf_safe (work, zlen));
+      work->len = zlen;
+
 
-      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work.len);
+      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work->len);
       compctx->pre_compress += buf->len;
-      compctx->post_compress += work.len;
+      compctx->post_compress += work->len;
+      return true;
     }
+  return false;
+}
 
-  /* did compression save us anything? */
+
+static void
+lz4_compress (struct buffer *buf, struct buffer work,
+	      struct compress_context *compctx,
+	      const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On error do_lz4_compress sets buf len to zero, just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything? Include 2 byte compression header
+     in calculation */
   {
     uint8_t comp_head_byte = NO_COMPRESS_BYTE_SWAP;
-    if (compressed && work.len < buf->len)
+    if (compressed && work.len +2 < buf->len)
       {
 	*buf = work;
 	comp_head_byte = LZ4_COMPRESS_BYTE;
@@ -128,13 +147,73 @@ lz4_compress (struct buffer *buf, struct buffer work,
   }
 }
 
+
+static void
+lz4v2_compress (struct buffer *buf, struct buffer work,
+		struct compress_context *compctx,
+		const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+  if (buf->len <= 0)
+    return;
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything? */
+  {
+    if (compressed && work.len < buf->len)
+      {
+	ASSERT(buf_prepend(&work, 2));
+	uint8_t *head = BPTR (&work);
+	head[0] = COMP_ALGV2_INDICATOR_BYTE;
+	head[1] = COMP_ALGV2_LZ4_BYTE;
+	*buf = work;
+      }
+    else
+      {
+	compv2_escape_data_ifneeded(buf);
+      }
+  }
+}
+
+void
+do_lz4_decompress(size_t zlen_max,
+		  struct buffer *work,
+		  struct buffer *buf,
+		  struct compress_context *compctx)
+{
+  int uncomp_len;
+  ASSERT (buf_safe (work, zlen_max));
+  uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(work), (size_t)BLEN(buf), zlen_max);
+  if (uncomp_len <= 0)
+    {
+      dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
+      buf->len = 0;
+      return;
+    }
+
+  ASSERT (buf_safe (work, uncomp_len));
+  work->len = uncomp_len;
+
+  dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work->len);
+  compctx->pre_decompress += buf->len;
+  compctx->post_decompress += work->len;
+
+  *buf = *work;
+}
+
 static void
 lz4_decompress (struct buffer *buf, struct buffer work,
-		 struct compress_context *compctx,
-		 const struct frame* frame)
+		struct compress_context *compctx,
+		const struct frame* frame)
 {
   size_t zlen_max = EXPANDED_SIZE (frame);
-  int uncomp_len;
   uint8_t c;		/* flag indicating whether or not our peer compressed */
 
   if (buf->len <= 0)
@@ -152,23 +231,7 @@ lz4_decompress (struct buffer *buf, struct buffer work,
 
   if (c == LZ4_COMPRESS_BYTE)	/* packet was compressed */
     {
-      ASSERT (buf_safe (&work, zlen_max));
-      uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(&work), (size_t)BLEN(buf), zlen_max);
-      if (uncomp_len <= 0)
-	{
-	  dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
-	  buf->len = 0;
-	  return;
-	}
-
-      ASSERT (buf_safe (&work, uncomp_len));
-      work.len = uncomp_len;
-
-      dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work.len);
-      compctx->pre_decompress += buf->len;
-      compctx->post_decompress += work.len;
-
-      *buf = work;
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
     }
   else if (c == NO_COMPRESS_BYTE_SWAP)	/* packet was not compressed */
     {
@@ -181,6 +244,52 @@ lz4_decompress (struct buffer *buf, struct buffer work,
     }
 }
 
+static void
+lz4v2_decompress (struct buffer *buf, struct buffer work,
+		  struct compress_context *compctx,
+		  const struct frame* frame)
+{
+  size_t zlen_max = EXPANDED_SIZE (frame);
+  uint8_t c;		/* flag indicating whether or not our peer compressed */
+
+  if (buf->len <= 0)
+    return;
+
+  ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
+
+  /* do unframing/swap (assumes buf->len > 0) */
+  uint8_t *head = BPTR (buf);
+  c = *head;
+
+  /* Not compressed */
+  if (c != COMP_ALGV2_INDICATOR_BYTE) {
+    return;
+  }
+
+  /* Packet to short to make sense */
+  if (buf->len <= 1)
+    {
+      buf->len=0;
+      return;
+    }
+
+  c = head[1];
+  if (c == COMP_ALGV2_LZ4_BYTE) /* packet was compressed */
+    {
+      buf_advance(buf,2);
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
+    }
+  else if (c == COMP_ALGV2_UNCOMPRESSED_BYTE)
+    {
+      buf_advance(buf,2);
+    }
+  else
+    {
+      dmsg (D_COMP_ERRORS, "Bad LZ4v2 decompression header byte: %d", c);
+      buf->len = 0;
+    }
+}
+
 const struct compress_alg lz4_alg = {
   "lz4",
   lz4_compress_init,
@@ -189,6 +298,14 @@ const struct compress_alg lz4_alg = {
   lz4_decompress
 };
 
+const struct compress_alg lz4v2_alg = {
+  "lz4v2",
+  lz4_compress_initv2,
+  lz4_compress_uninit,
+  lz4v2_compress,
+  lz4v2_decompress
+};
+
 #else
 static void dummy(void) {}
 #endif /* ENABLE_LZ4 */
src/openvpn/comp-lz4.h +1 -0
index ca1dfa9..9d3c664 100644
@@ -31,6 +31,7 @@
 #include "buffer.h"
 
 extern const struct compress_alg lz4_alg;
+extern const struct compress_alg lz4v2_alg;
 
 struct lz4_workspace
 {
src/openvpn/comp.c +31 -0
index 706ad7e..809c83d 100644
@@ -50,6 +50,11 @@ comp_init(const struct compress_options *opt)
       compctx->alg = comp_stub_alg;
       (*compctx->alg.compress_init)(compctx);
       break;
+    case COMP_ALGV2_UNCOMPRESSED:
+      ALLOC_OBJ_CLEAR (compctx, struct compress_context);
+      compctx->flags = opt->flags;
+      compctx->alg = compv2_stub_alg;
+      break;
 #ifdef ENABLE_LZO
     case COMP_ALG_LZO:
       ALLOC_OBJ_CLEAR (compctx, struct compress_context);
@@ -65,11 +70,35 @@ comp_init(const struct compress_options *opt)
       compctx->alg = lz4_alg;
       (*compctx->alg.compress_init)(compctx);
       break;
+    case COMP_ALGV2_LZ4:
+      ALLOC_OBJ_CLEAR (compctx, struct compress_context);
+      compctx->flags = opt->flags;
+      compctx->alg = lz4v2_alg;
+      break;
 #endif
     }
   return compctx;
 }
 
+/* In the v2 compression schmemes, an uncompressed packet has
+ * has no opcode in front, unless the first byte is 0x50. In this
+ * case the packet needs to be escaped */
+void
+compv2_escape_data_ifneeded (struct buffer *buf)
+{
+    uint8_t *head = BPTR (buf);
+    if (head[0] != COMP_ALGV2_INDICATOR_BYTE)
+	return;
+
+    /* Header is 0x50 */
+    ASSERT(buf_prepend(buf, 2));
+
+    head = BPTR (buf);
+    head[0] = COMP_ALGV2_INDICATOR_BYTE;
+    head[1] = COMP_ALGV2_UNCOMPRESSED;
+}
+
+
 void
 comp_uninit(struct compress_context *compctx)
 {
@@ -120,6 +149,7 @@ comp_generate_peer_info_string(const struct compress_options *opt, struct buffer
 	{
 #if defined(ENABLE_LZ4)
 	  buf_printf (out, "IV_LZ4=1\n");
+	  buf_printf (out, "IV_LZ4v2=1\n");
 #endif
 #if defined(ENABLE_LZO)
 	  buf_printf (out, "IV_LZO=1\n");
@@ -129,6 +159,7 @@ comp_generate_peer_info_string(const struct compress_options *opt, struct buffer
       if (!lzo_avail)
 	buf_printf (out, "IV_LZO_STUB=1\n");
       buf_printf (out, "IV_COMP_STUB=1\n");
+      buf_printf (out, "IV_COMP_STUBv2=1\n");
     }
 }
 
src/openvpn/comp.h +25 -0
index 716b1c0..56590b7 100644
@@ -43,12 +43,22 @@
 #define COMP_ALG_SNAPPY 3 /* Snappy algorithm (no longer supported) */
 #define COMP_ALG_LZ4    4 /* LZ4 algorithm */
 
+
+/* algorithm v2 */
+#define COMP_ALGV2_UNCOMPRESSED 10
+#define COMP_ALGV2_LZ4	    11
+/*
+#define COMP_ALGV2_LZO	    12
+#define COMP_ALGV2_SNAPPY   13
+*/
+
 /* Compression flags */
 #define COMP_F_ADAPTIVE   (1<<0) /* COMP_ALG_LZO only */
 #define COMP_F_ASYM       (1<<1) /* only downlink is compressed, not uplink */
 #define COMP_F_SWAP       (1<<2) /* initial command byte is swapped with last byte in buffer to preserve payload alignment */
 #define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support compression stubs */
 
+
 /*
  * Length of prepended prefix on compressed packets
  */
@@ -57,9 +67,21 @@
 /*
  * Prefix bytes
  */
+
+/* V1 on wire codes */
+/* Initial command byte to tell our peer if we compressed */
+#define LZO_COMPRESS_BYTE 0x66
+#define LZ4_COMPRESS_BYTE 0x69
 #define NO_COMPRESS_BYTE      0xFA
 #define NO_COMPRESS_BYTE_SWAP 0xFB /* to maintain payload alignment, replace this byte with last byte of packet */
 
+/* V2 on wire code */
+#define COMP_ALGV2_INDICATOR_BYTE	0x50
+#define COMP_ALGV2_UNCOMPRESSED_BYTE	1
+#define COMP_ALGV2_LZ4_BYTE		1
+#define COMP_ALGV2_LZO_BYTE		2
+#define COMP_ALGV2_SNAPPY_BYTE		3
+
 /*
  * Compress worst case size expansion (for any algorithm)
  *
@@ -145,6 +167,7 @@ struct compress_context
 };
 
 extern const struct compress_alg comp_stub_alg;
+extern const struct compress_alg compv2_stub_alg;
 
 struct compress_context *comp_init(const struct compress_options *opt);
 
@@ -157,6 +180,8 @@ void comp_print_stats (const struct compress_context *compctx, struct status_out
 
 void comp_generate_peer_info_string(const struct compress_options *opt, struct buffer *out);
 
+void compv2_escape_data_ifneeded (struct buffer *buf);
+
 static inline bool
 comp_enabled(const struct compress_options *info)
 {
src/openvpn/compstub.c +52 -0

64 lines not on this page · raw message

src/openvpn/lzo.c +0 -3
index daa02ed..25a839b 100644
@@ -42,9 +42,6 @@
 
 #include "memdbg.h"
 
-/* Initial command byte to tell our peer if we compressed */
-#define LZO_COMPRESS_BYTE 0x66
-
 /**
  * Perform adaptive compression housekeeping.
  *
src/openvpn/options.c +8 -0
index 5a83897..eb4942b 100644
@@ -6323,6 +6323,10 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_STUB;
 	      options->comp.flags = (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
 	    }
+	  else if (streq(p[1], "stub-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
+	    }
 #if defined(ENABLE_LZO)
 	  else if (streq (p[1], "lzo"))
 	    {
@@ -6336,6 +6340,10 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_LZ4;
 	      options->comp.flags = COMP_F_SWAP;
 	    }
+	  else if (streq (p[1], "lz4-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_LZ4;
+	    }
 #endif
 	  else
 	    {
Signature · 1 line
2.5.4 (Apple Git-61)
#5 lore raw

PATCH v4 1/1 Implement the compression V2 data format for stub and lz4.

In reply to Steffan Karger

Patch V2: Fix minor issues found by Steffan
Patch V3: split wire codes and compression flags
Patch V4: Fix further issues reported by Gert

It has been tested against v3 server and again itself. From James Mail:

Compression V2

I have observed that compression in many cases, even when enabled, often does not produce packet size reduction because much of the packet data typically generated by web sessions is already compressed. Further, the single byte that precedes the packet and indicates whether or not compression occurred has the unfortunate side effect of misaligning the IP packet in cases where compression did not occur. To remedy this, I propose a Compression V2 header that is optimized for the case where compression does not occur.

a. No compression occurred and first byte of IP/Ethernet packet
is NOT 0x50 (0 bytes of overhead and maintains alignment):

[ uncompressed IP/Ethernet packet ]

b. No compression occurred and first byte of IP/Ethernet packet

      is 0x50 (2 bytes of overhead but unlikely since no known
      IP packet can begin with 0x50):

        [ 0x50 ] [ 0x00 ] [ uncompressed IP/Ethernet packet ]

c. Compression occurred (2 bytes of overhead):

        [ 0x50 ] [ compression Alg ID ] [ compressed IP/Ethernet packet ]

      Compression Alg ID is one-byte algorithm identifier
      for LZ4 (0x1), LZO (0x2), or Snappy (0x3).

This approach has several beneficial effects:

1. In the common case where compression does not occur, no compression op is required, therefore there is zero overhead.

2. When compression does not occur, the IP/Ethernet packet alignment is retained.

3. This technique does not require any byte swapping with the tail of the packet which can potentially incur an expensive cache miss.

---
 src/openvpn/comp-lz4.c | 203 ++++++++++++++++++++++++++++++++++++++-----------
 src/openvpn/comp-lz4.h |   1 +
 src/openvpn/comp.c     |  31 ++++++++
 src/openvpn/comp.h     |  25 ++++++
 src/openvpn/compstub.c |  51 +++++++++++++
 src/openvpn/lzo.c      |   3 -
 src/openvpn/options.c  |  10 ++-
 7 files changed, 277 insertions(+), 47 deletions(-)
src/openvpn/comp-lz4.c +160 -43
index 4651148..1d3dc20 100644
@@ -44,9 +44,6 @@
 
 #include "memdbg.h"
 
-/* Initial command byte to tell our peer if we compressed */
-#define LZ4_COMPRESS_BYTE 0x69
-
 static void
 lz4_compress_init (struct compress_context *compctx)
 {
@@ -55,20 +52,22 @@ lz4_compress_init (struct compress_context *compctx)
 }
 
 static void
-lz4_compress_uninit (struct compress_context *compctx)
+lz4v2_compress_init (struct compress_context *compctx)
 {
+  msg (D_INIT_MEDIUM, "LZ4v2 compression initializing");
 }
 
 static void
-lz4_compress (struct buffer *buf, struct buffer work,
-	       struct compress_context *compctx,
-	       const struct frame* frame)
+lz4_compress_uninit (struct compress_context *compctx)
 {
-  bool compressed = false;
-
-  if (buf->len <= 0)
-    return;
+}
 
+static bool
+do_lz4_compress (struct buffer *buf,
+		 struct buffer *work,
+		 struct compress_context *compctx,
+		 const struct frame* frame)
+{
   /*
    * In order to attempt compression, length must be at least COMPRESS_THRESHOLD.
    */
@@ -78,38 +77,58 @@ lz4_compress (struct buffer *buf, struct buffer work,
       int zlen_max = ps + COMP_EXTRA_BUFFER (ps);
       int zlen;
 
-      ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
-      ASSERT (buf_safe (&work, zlen_max));
+      ASSERT (buf_init (work, FRAME_HEADROOM (frame)));
+      ASSERT (buf_safe (work, zlen_max));
 
       if (buf->len > ps)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression buffer overflow");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(&work), BLEN(buf), zlen_max );
+      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(work), BLEN(buf), zlen_max );
 
       if (zlen <= 0)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression error");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      ASSERT (buf_safe (&work, zlen));
-      work.len = zlen;
-      compressed = true;
+      ASSERT (buf_safe (work, zlen));
+      work->len = zlen;
+
 
-      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work.len);
+      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work->len);
       compctx->pre_compress += buf->len;
-      compctx->post_compress += work.len;
+      compctx->post_compress += work->len;
+      return true;
     }
+  return false;
+}
 
-  /* did compression save us anything? */
+
+static void
+lz4_compress (struct buffer *buf, struct buffer work,
+	      struct compress_context *compctx,
+	      const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On error do_lz4_compress sets buf len to zero, just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything? Include 2 byte compression header
+     in calculation */
   {
     uint8_t comp_head_byte = NO_COMPRESS_BYTE_SWAP;
-    if (compressed && work.len < buf->len)
+    if (compressed && work.len +2 < buf->len)
       {
 	*buf = work;
 	comp_head_byte = LZ4_COMPRESS_BYTE;
@@ -128,13 +147,73 @@ lz4_compress (struct buffer *buf, struct buffer work,
   }
 }
 
+
+static void
+lz4v2_compress (struct buffer *buf, struct buffer work,
+		struct compress_context *compctx,
+		const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+  if (buf->len <= 0)
+    return;
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything? */
+  {
+    if (compressed && work.len < buf->len)
+      {
+	ASSERT(buf_prepend(&work, 2));
+	uint8_t *head = BPTR (&work);
+	head[0] = COMP_ALGV2_INDICATOR_BYTE;
+	head[1] = COMP_ALGV2_LZ4_BYTE;
+	*buf = work;
+      }
+    else
+      {
+	compv2_escape_data_ifneeded(buf);
+      }
+  }
+}
+
+void
+do_lz4_decompress(size_t zlen_max,
+		  struct buffer *work,
+		  struct buffer *buf,
+		  struct compress_context *compctx)
+{
+  int uncomp_len;
+  ASSERT (buf_safe (work, zlen_max));
+  uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(work), (size_t)BLEN(buf), zlen_max);
+  if (uncomp_len <= 0)
+    {
+      dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
+      buf->len = 0;
+      return;
+    }
+
+  ASSERT (buf_safe (work, uncomp_len));
+  work->len = uncomp_len;
+
+  dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work->len);
+  compctx->pre_decompress += buf->len;
+  compctx->post_decompress += work->len;
+
+  *buf = *work;
+}
+
 static void
 lz4_decompress (struct buffer *buf, struct buffer work,
-		 struct compress_context *compctx,
-		 const struct frame* frame)
+		struct compress_context *compctx,
+		const struct frame* frame)
 {
   size_t zlen_max = EXPANDED_SIZE (frame);
-  int uncomp_len;
   uint8_t c;		/* flag indicating whether or not our peer compressed */
 
   if (buf->len <= 0)
@@ -152,23 +231,7 @@ lz4_decompress (struct buffer *buf, struct buffer work,
 
   if (c == LZ4_COMPRESS_BYTE)	/* packet was compressed */
     {
-      ASSERT (buf_safe (&work, zlen_max));
-      uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(&work), (size_t)BLEN(buf), zlen_max);
-      if (uncomp_len <= 0)
-	{
-	  dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
-	  buf->len = 0;
-	  return;
-	}
-
-      ASSERT (buf_safe (&work, uncomp_len));
-      work.len = uncomp_len;
-
-      dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work.len);
-      compctx->pre_decompress += buf->len;
-      compctx->post_decompress += work.len;
-
-      *buf = work;
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
     }
   else if (c == NO_COMPRESS_BYTE_SWAP)	/* packet was not compressed */
     {
@@ -181,6 +244,52 @@ lz4_decompress (struct buffer *buf, struct buffer work,
     }
 }
 
+static void
+lz4v2_decompress (struct buffer *buf, struct buffer work,
+		  struct compress_context *compctx,
+		  const struct frame* frame)
+{
+  size_t zlen_max = EXPANDED_SIZE (frame);
+  uint8_t c;		/* flag indicating whether or not our peer compressed */
+
+  if (buf->len <= 0)
+    return;
+
+  ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
+
+  /* do unframing/swap (assumes buf->len > 0) */
+  uint8_t *head = BPTR (buf);
+  c = *head;
+
+  /* Not compressed */
+  if (c != COMP_ALGV2_INDICATOR_BYTE) {
+    return;
+  }
+
+  /* Packet to short to make sense */
+  if (buf->len <= 1)
+    {
+      buf->len=0;
+      return;
+    }
+
+  c = head[1];
+  if (c == COMP_ALGV2_LZ4_BYTE) /* packet was compressed */
+    {
+      buf_advance(buf,2);
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
+    }
+  else if (c == COMP_ALGV2_UNCOMPRESSED_BYTE)
+    {
+      buf_advance(buf,2);
+    }
+  else
+    {
+      dmsg (D_COMP_ERRORS, "Bad LZ4v2 decompression header byte: %d", c);
+      buf->len = 0;
+    }
+}
+
 const struct compress_alg lz4_alg = {
   "lz4",
   lz4_compress_init,
@@ -189,6 +298,14 @@ const struct compress_alg lz4_alg = {
   lz4_decompress
 };
 
+const struct compress_alg lz4v2_alg = {
+  "lz4v2",
+  lz4v2_compress_init,
+  lz4_compress_uninit,
+  lz4v2_compress,
+  lz4v2_decompress
+};
+
 #else
 static void dummy(void) {}
 #endif /* ENABLE_LZ4 */
src/openvpn/comp-lz4.h +1 -0
index ca1dfa9..9d3c664 100644
@@ -31,6 +31,7 @@
 #include "buffer.h"
 
 extern const struct compress_alg lz4_alg;
+extern const struct compress_alg lz4v2_alg;
 
 struct lz4_workspace
 {
src/openvpn/comp.c +31 -0
index 706ad7e..809c83d 100644
@@ -50,6 +50,11 @@ comp_init(const struct compress_options *opt)
       compctx->alg = comp_stub_alg;
       (*compctx->alg.compress_init)(compctx);
       break;
+    case COMP_ALGV2_UNCOMPRESSED:
+      ALLOC_OBJ_CLEAR (compctx, struct compress_context);
+      compctx->flags = opt->flags;
+      compctx->alg = compv2_stub_alg;
+      break;
 #ifdef ENABLE_LZO
     case COMP_ALG_LZO:
       ALLOC_OBJ_CLEAR (compctx, struct compress_context);
@@ -65,11 +70,35 @@ comp_init(const struct compress_options *opt)
       compctx->alg = lz4_alg;
       (*compctx->alg.compress_init)(compctx);
       break;
+    case COMP_ALGV2_LZ4:
+      ALLOC_OBJ_CLEAR (compctx, struct compress_context);
+      compctx->flags = opt->flags;
+      compctx->alg = lz4v2_alg;
+      break;
 #endif
     }
   return compctx;
 }
 
+/* In the v2 compression schmemes, an uncompressed packet has
+ * has no opcode in front, unless the first byte is 0x50. In this
+ * case the packet needs to be escaped */
+void
+compv2_escape_data_ifneeded (struct buffer *buf)
+{
+    uint8_t *head = BPTR (buf);
+    if (head[0] != COMP_ALGV2_INDICATOR_BYTE)
+	return;
+
+    /* Header is 0x50 */
+    ASSERT(buf_prepend(buf, 2));
+
+    head = BPTR (buf);
+    head[0] = COMP_ALGV2_INDICATOR_BYTE;
+    head[1] = COMP_ALGV2_UNCOMPRESSED;
+}
+
+
 void
 comp_uninit(struct compress_context *compctx)
 {
@@ -120,6 +149,7 @@ comp_generate_peer_info_string(const struct compress_options *opt, struct buffer
 	{
 #if defined(ENABLE_LZ4)
 	  buf_printf (out, "IV_LZ4=1\n");
+	  buf_printf (out, "IV_LZ4v2=1\n");
 #endif
 #if defined(ENABLE_LZO)
 	  buf_printf (out, "IV_LZO=1\n");
@@ -129,6 +159,7 @@ comp_generate_peer_info_string(const struct compress_options *opt, struct buffer
       if (!lzo_avail)
 	buf_printf (out, "IV_LZO_STUB=1\n");
       buf_printf (out, "IV_COMP_STUB=1\n");
+      buf_printf (out, "IV_COMP_STUBv2=1\n");
     }
 }
 
src/openvpn/comp.h +25 -0
index 716b1c0..9ed9532 100644
@@ -43,12 +43,22 @@
 #define COMP_ALG_SNAPPY 3 /* Snappy algorithm (no longer supported) */
 #define COMP_ALG_LZ4    4 /* LZ4 algorithm */
 
+
+/* algorithm v2 */
+#define COMP_ALGV2_UNCOMPRESSED 10
+#define COMP_ALGV2_LZ4	    11
+/*
+#define COMP_ALGV2_LZO	    12
+#define COMP_ALGV2_SNAPPY   13
+*/
+
 /* Compression flags */
 #define COMP_F_ADAPTIVE   (1<<0) /* COMP_ALG_LZO only */
 #define COMP_F_ASYM       (1<<1) /* only downlink is compressed, not uplink */
 #define COMP_F_SWAP       (1<<2) /* initial command byte is swapped with last byte in buffer to preserve payload alignment */
 #define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support compression stubs */
 
+
 /*
  * Length of prepended prefix on compressed packets
  */
@@ -57,9 +67,21 @@
 /*
  * Prefix bytes
  */
+
+/* V1 on wire codes */
+/* Initial command byte to tell our peer if we compressed */
+#define LZO_COMPRESS_BYTE 0x66
+#define LZ4_COMPRESS_BYTE 0x69
 #define NO_COMPRESS_BYTE      0xFA
 #define NO_COMPRESS_BYTE_SWAP 0xFB /* to maintain payload alignment, replace this byte with last byte of packet */
 
+/* V2 on wire code */
+#define COMP_ALGV2_INDICATOR_BYTE	0x50
+#define COMP_ALGV2_UNCOMPRESSED_BYTE	0
+#define COMP_ALGV2_LZ4_BYTE		1
+#define COMP_ALGV2_LZO_BYTE		2
+#define COMP_ALGV2_SNAPPY_BYTE		3
+
 /*
  * Compress worst case size expansion (for any algorithm)
  *
@@ -145,6 +167,7 @@ struct compress_context
 };
 
 extern const struct compress_alg comp_stub_alg;
+extern const struct compress_alg compv2_stub_alg;
 
 struct compress_context *comp_init(const struct compress_options *opt);
 
@@ -157,6 +180,8 @@ void comp_print_stats (const struct compress_context *compctx, struct status_out
 
 void comp_generate_peer_info_string(const struct compress_options *opt, struct buffer *out);
 
+void compv2_escape_data_ifneeded (struct buffer *buf);
+
 static inline bool
 comp_enabled(const struct compress_options *info)
 {
src/openvpn/compstub.c +51 -0

57 lines not on this page · raw message

src/openvpn/lzo.c +0 -3
index daa02ed..25a839b 100644
@@ -42,9 +42,6 @@
 
 #include "memdbg.h"
 
-/* Initial command byte to tell our peer if we compressed */
-#define LZO_COMPRESS_BYTE 0x66
-
 /**
  * Perform adaptive compression housekeeping.
  *
src/openvpn/options.c +9 -1
index 5a83897..b00059a 100644
@@ -6318,16 +6318,20 @@ add_option (struct options *options,
       VERIFY_PERMISSION (OPT_P_COMP);
       if (p[1])
 	{
+	  options->comp.flags = 0;
 	  if (streq (p[1], "stub"))
 	    {
 	      options->comp.alg = COMP_ALG_STUB;
 	      options->comp.flags = (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
 	    }
+	  else if (streq(p[1], "stub-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
+	    }
 #if defined(ENABLE_LZO)
 	  else if (streq (p[1], "lzo"))
 	    {
 	      options->comp.alg = COMP_ALG_LZO;
-	      options->comp.flags = 0;
 	    }
 #endif
 #if defined(ENABLE_LZ4)
@@ -6336,6 +6340,10 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_LZ4;
 	      options->comp.flags = COMP_F_SWAP;
 	    }
+	  else if (streq (p[1], "lz4-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_LZ4;
+	    }
 #endif
 	  else
 	    {
Signature · 1 line
2.5.4 (Apple Git-61)
#6 lore raw

PATCH v5 1/1 Implement the compression V2 data format for stub and lz4.

In reply to Steffan Karger

Patch V2: Fix minor issues found by Steffan
Patch V3: split wire codes and compression flags Patch V4: Fix further issues reported by Gert Patch V5: really fix the issues that should be fixed in v2

It has been tested against v3 server and again itself. From James Mail:

Compression V2

I have observed that compression in many cases, even when enabled, often does not produce packet size reduction because much of the packet data typically generated by web sessions is already compressed. Further, the single byte that precedes the packet and indicates whether or not compression occurred has the unfortunate side effect of misaligning the IP packet in cases where compression did not occur. To remedy this, I propose a Compression V2 header that is optimized for the case where compression does not occur.

a. No compression occurred and first byte of IP/Ethernet packet
is NOT 0x50 (0 bytes of overhead and maintains alignment):

[ uncompressed IP/Ethernet packet ]

b. No compression occurred and first byte of IP/Ethernet packet

      is 0x50 (2 bytes of overhead but unlikely since no known
      IP packet can begin with 0x50):

        [ 0x50 ] [ 0x00 ] [ uncompressed IP/Ethernet packet ]

c. Compression occurred (2 bytes of overhead):

        [ 0x50 ] [ compression Alg ID ] [ compressed IP/Ethernet packet ]

      Compression Alg ID is one-byte algorithm identifier
      for LZ4 (0x1), LZO (0x2), or Snappy (0x3).

This approach has several beneficial effects:

1. In the common case where compression does not occur, no compression op is required, therefore there is zero overhead.

2. When compression does not occur, the IP/Ethernet packet alignment is retained.

3. This technique does not require any byte swapping with the tail of the packet which can potentially incur an expensive cache miss.

---
 src/openvpn/comp-lz4.c | 199 ++++++++++++++++++++++++++++++++++++++-----------
 src/openvpn/comp-lz4.h |   1 +
 src/openvpn/comp.c     |  31 ++++++++
 src/openvpn/comp.h     |  25 +++++++
 src/openvpn/compstub.c |  51 +++++++++++++
 src/openvpn/lzo.c      |   3 -
 src/openvpn/options.c  |  10 ++-
 7 files changed, 274 insertions(+), 46 deletions(-)
src/openvpn/comp-lz4.c +157 -42
index 4651148..ddd8e28 100644
@@ -44,9 +44,6 @@
 
 #include "memdbg.h"
 
-/* Initial command byte to tell our peer if we compressed */
-#define LZ4_COMPRESS_BYTE 0x69
-
 static void
 lz4_compress_init (struct compress_context *compctx)
 {
@@ -55,20 +52,22 @@ lz4_compress_init (struct compress_context *compctx)
 }
 
 static void
-lz4_compress_uninit (struct compress_context *compctx)
+lz4v2_compress_init (struct compress_context *compctx)
 {
+  msg (D_INIT_MEDIUM, "LZ4v2 compression initializing");
 }
 
 static void
-lz4_compress (struct buffer *buf, struct buffer work,
-	       struct compress_context *compctx,
-	       const struct frame* frame)
+lz4_compress_uninit (struct compress_context *compctx)
 {
-  bool compressed = false;
-
-  if (buf->len <= 0)
-    return;
+}
 
+static bool
+do_lz4_compress (struct buffer *buf,
+		 struct buffer *work,
+		 struct compress_context *compctx,
+		 const struct frame* frame)
+{
   /*
    * In order to attempt compression, length must be at least COMPRESS_THRESHOLD.
    */
@@ -78,38 +77,57 @@ lz4_compress (struct buffer *buf, struct buffer work,
       int zlen_max = ps + COMP_EXTRA_BUFFER (ps);
       int zlen;
 
-      ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
-      ASSERT (buf_safe (&work, zlen_max));
+      ASSERT (buf_init (work, FRAME_HEADROOM (frame)));
+      ASSERT (buf_safe (work, zlen_max));
 
       if (buf->len > ps)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression buffer overflow");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(&work), BLEN(buf), zlen_max );
+      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(work), BLEN(buf), zlen_max );
 
       if (zlen <= 0)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression error");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      ASSERT (buf_safe (&work, zlen));
-      work.len = zlen;
-      compressed = true;
+      ASSERT (buf_safe (work, zlen));
+      work->len = zlen;
+
 
-      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work.len);
+      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work->len);
       compctx->pre_compress += buf->len;
-      compctx->post_compress += work.len;
+      compctx->post_compress += work->len;
+      return true;
     }
+  return false;
+}
+
+
+static void
+lz4_compress (struct buffer *buf, struct buffer work,
+	      struct compress_context *compctx,
+	      const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On error do_lz4_compress sets buf len to zero, just return */
+  if (buf->len == 0)
+    return;
 
   /* did compression save us anything? */
   {
     uint8_t comp_head_byte = NO_COMPRESS_BYTE_SWAP;
-    if (compressed && work.len < buf->len)
+    if (compressed && work.len +2 < buf->len)
       {
 	*buf = work;
 	comp_head_byte = LZ4_COMPRESS_BYTE;
@@ -128,13 +146,72 @@ lz4_compress (struct buffer *buf, struct buffer work,
   }
 }
 
+
+static void
+lz4v2_compress (struct buffer *buf, struct buffer work,
+		struct compress_context *compctx,
+		const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything?  Include 2 byte compression header
+     in calculation */
+  {
+    if (compressed && work.len + 2 < buf->len)
+      {
+	ASSERT(buf_prepend(&work, 2));
+	uint8_t *head = BPTR (&work);
+	head[0] = COMP_ALGV2_INDICATOR_BYTE;
+	head[1] = COMP_ALGV2_LZ4_BYTE;
+	*buf = work;
+      }
+    else
+      {
+	compv2_escape_data_ifneeded(buf);
+      }
+  }
+}
+
+void
+do_lz4_decompress(size_t zlen_max,
+		  struct buffer *work,
+		  struct buffer *buf,
+		  struct compress_context *compctx)
+{
+  int uncomp_len;
+  ASSERT (buf_safe (work, zlen_max));
+  uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(work), (size_t)BLEN(buf), zlen_max);
+  if (uncomp_len <= 0)
+    {
+      dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
+      buf->len = 0;
+      return;
+    }
+
+  ASSERT (buf_safe (work, uncomp_len));
+  work->len = uncomp_len;
+
+  dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work->len);
+  compctx->pre_decompress += buf->len;
+  compctx->post_decompress += work->len;
+
+  *buf = *work;
+}
+
 static void
 lz4_decompress (struct buffer *buf, struct buffer work,
-		 struct compress_context *compctx,
-		 const struct frame* frame)
+		struct compress_context *compctx,
+		const struct frame* frame)
 {
   size_t zlen_max = EXPANDED_SIZE (frame);
-  int uncomp_len;
   uint8_t c;		/* flag indicating whether or not our peer compressed */
 
   if (buf->len <= 0)
@@ -152,23 +229,7 @@ lz4_decompress (struct buffer *buf, struct buffer work,
 
   if (c == LZ4_COMPRESS_BYTE)	/* packet was compressed */
     {
-      ASSERT (buf_safe (&work, zlen_max));
-      uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(&work), (size_t)BLEN(buf), zlen_max);
-      if (uncomp_len <= 0)
-	{
-	  dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
-	  buf->len = 0;
-	  return;
-	}
-
-      ASSERT (buf_safe (&work, uncomp_len));
-      work.len = uncomp_len;
-
-      dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work.len);
-      compctx->pre_decompress += buf->len;
-      compctx->post_decompress += work.len;
-
-      *buf = work;
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
     }
   else if (c == NO_COMPRESS_BYTE_SWAP)	/* packet was not compressed */
     {
@@ -181,6 +242,52 @@ lz4_decompress (struct buffer *buf, struct buffer work,
     }
 }
 
+static void
+lz4v2_decompress (struct buffer *buf, struct buffer work,
+		  struct compress_context *compctx,
+		  const struct frame* frame)
+{
+  size_t zlen_max = EXPANDED_SIZE (frame);
+  uint8_t c;		/* flag indicating whether or not our peer compressed */
+
+  if (buf->len <= 0)
+    return;
+
+  ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
+
+  /* do unframing/swap (assumes buf->len > 0) */
+  uint8_t *head = BPTR (buf);
+  c = *head;
+
+  /* Not compressed */
+  if (c != COMP_ALGV2_INDICATOR_BYTE) {
+    return;
+  }
+
+  /* Packet to short to make sense */
+  if (buf->len <= 1)
+    {
+      buf->len=0;
+      return;
+    }
+
+  c = head[1];
+  if (c == COMP_ALGV2_LZ4_BYTE) /* packet was compressed */
+    {
+      buf_advance(buf,2);
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
+    }
+  else if (c == COMP_ALGV2_UNCOMPRESSED_BYTE)
+    {
+      buf_advance(buf,2);
+    }
+  else
+    {
+      dmsg (D_COMP_ERRORS, "Bad LZ4v2 decompression header byte: %d", c);
+      buf->len = 0;
+    }
+}
+
 const struct compress_alg lz4_alg = {
   "lz4",
   lz4_compress_init,
@@ -189,6 +296,14 @@ const struct compress_alg lz4_alg = {
   lz4_decompress
 };
 
+const struct compress_alg lz4v2_alg = {
+  "lz4v2",
+  lz4v2_compress_init,
+  lz4_compress_uninit,
+  lz4v2_compress,
+  lz4v2_decompress
+};
+
 #else
 static void dummy(void) {}
 #endif /* ENABLE_LZ4 */
src/openvpn/comp-lz4.h +1 -0
index ca1dfa9..9d3c664 100644
@@ -31,6 +31,7 @@
 #include "buffer.h"
 
 extern const struct compress_alg lz4_alg;
+extern const struct compress_alg lz4v2_alg;
 
 struct lz4_workspace
 {
src/openvpn/comp.c +31 -0
index 706ad7e..809c83d 100644
@@ -50,6 +50,11 @@ comp_init(const struct compress_options *opt)
       compctx->alg = comp_stub_alg;
       (*compctx->alg.compress_init)(compctx);
       break;
+    case COMP_ALGV2_UNCOMPRESSED:
+      ALLOC_OBJ_CLEAR (compctx, struct compress_context);
+      compctx->flags = opt->flags;
+      compctx->alg = compv2_stub_alg;
+      break;
 #ifdef ENABLE_LZO
     case COMP_ALG_LZO:
       ALLOC_OBJ_CLEAR (compctx, struct compress_context);
@@ -65,11 +70,35 @@ comp_init(const struct compress_options *opt)
       compctx->alg = lz4_alg;
       (*compctx->alg.compress_init)(compctx);
       break;
+    case COMP_ALGV2_LZ4:
+      ALLOC_OBJ_CLEAR (compctx, struct compress_context);
+      compctx->flags = opt->flags;
+      compctx->alg = lz4v2_alg;
+      break;
 #endif
     }
   return compctx;
 }
 
+/* In the v2 compression schmemes, an uncompressed packet has
+ * has no opcode in front, unless the first byte is 0x50. In this
+ * case the packet needs to be escaped */
+void
+compv2_escape_data_ifneeded (struct buffer *buf)
+{
+    uint8_t *head = BPTR (buf);
+    if (head[0] != COMP_ALGV2_INDICATOR_BYTE)
+	return;
+
+    /* Header is 0x50 */
+    ASSERT(buf_prepend(buf, 2));
+
+    head = BPTR (buf);
+    head[0] = COMP_ALGV2_INDICATOR_BYTE;
+    head[1] = COMP_ALGV2_UNCOMPRESSED;
+}
+
+
 void
 comp_uninit(struct compress_context *compctx)
 {
@@ -120,6 +149,7 @@ comp_generate_peer_info_string(const struct compress_options *opt, struct buffer
 	{
 #if defined(ENABLE_LZ4)
 	  buf_printf (out, "IV_LZ4=1\n");
+	  buf_printf (out, "IV_LZ4v2=1\n");
 #endif
 #if defined(ENABLE_LZO)
 	  buf_printf (out, "IV_LZO=1\n");
@@ -129,6 +159,7 @@ comp_generate_peer_info_string(const struct compress_options *opt, struct buffer
       if (!lzo_avail)
 	buf_printf (out, "IV_LZO_STUB=1\n");
       buf_printf (out, "IV_COMP_STUB=1\n");
+      buf_printf (out, "IV_COMP_STUBv2=1\n");
     }
 }
 
src/openvpn/comp.h +25 -0
index 716b1c0..9ed9532 100644
@@ -43,12 +43,22 @@
 #define COMP_ALG_SNAPPY 3 /* Snappy algorithm (no longer supported) */
 #define COMP_ALG_LZ4    4 /* LZ4 algorithm */
 
+
+/* algorithm v2 */
+#define COMP_ALGV2_UNCOMPRESSED 10
+#define COMP_ALGV2_LZ4	    11
+/*
+#define COMP_ALGV2_LZO	    12
+#define COMP_ALGV2_SNAPPY   13
+*/
+
 /* Compression flags */
 #define COMP_F_ADAPTIVE   (1<<0) /* COMP_ALG_LZO only */
 #define COMP_F_ASYM       (1<<1) /* only downlink is compressed, not uplink */
 #define COMP_F_SWAP       (1<<2) /* initial command byte is swapped with last byte in buffer to preserve payload alignment */
 #define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support compression stubs */
 
+
 /*
  * Length of prepended prefix on compressed packets
  */
@@ -57,9 +67,21 @@
 /*
  * Prefix bytes
  */
+
+/* V1 on wire codes */
+/* Initial command byte to tell our peer if we compressed */
+#define LZO_COMPRESS_BYTE 0x66
+#define LZ4_COMPRESS_BYTE 0x69
 #define NO_COMPRESS_BYTE      0xFA
 #define NO_COMPRESS_BYTE_SWAP 0xFB /* to maintain payload alignment, replace this byte with last byte of packet */
 
+/* V2 on wire code */
+#define COMP_ALGV2_INDICATOR_BYTE	0x50
+#define COMP_ALGV2_UNCOMPRESSED_BYTE	0
+#define COMP_ALGV2_LZ4_BYTE		1
+#define COMP_ALGV2_LZO_BYTE		2
+#define COMP_ALGV2_SNAPPY_BYTE		3
+
 /*
  * Compress worst case size expansion (for any algorithm)
  *
@@ -145,6 +167,7 @@ struct compress_context
 };
 
 extern const struct compress_alg comp_stub_alg;
+extern const struct compress_alg compv2_stub_alg;
 
 struct compress_context *comp_init(const struct compress_options *opt);
 
@@ -157,6 +180,8 @@ void comp_print_stats (const struct compress_context *compctx, struct status_out
 
 void comp_generate_peer_info_string(const struct compress_options *opt, struct buffer *out);
 
+void compv2_escape_data_ifneeded (struct buffer *buf);
+
 static inline bool
 comp_enabled(const struct compress_options *info)
 {
src/openvpn/compstub.c +51 -0

57 lines not on this page · raw message

src/openvpn/lzo.c +0 -3
index daa02ed..25a839b 100644
@@ -42,9 +42,6 @@
 
 #include "memdbg.h"
 
-/* Initial command byte to tell our peer if we compressed */
-#define LZO_COMPRESS_BYTE 0x66
-
 /**
  * Perform adaptive compression housekeeping.
  *
src/openvpn/options.c +9 -1
index 5a83897..b00059a 100644
@@ -6318,16 +6318,20 @@ add_option (struct options *options,
       VERIFY_PERMISSION (OPT_P_COMP);
       if (p[1])
 	{
+	  options->comp.flags = 0;
 	  if (streq (p[1], "stub"))
 	    {
 	      options->comp.alg = COMP_ALG_STUB;
 	      options->comp.flags = (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
 	    }
+	  else if (streq(p[1], "stub-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
+	    }
 #if defined(ENABLE_LZO)
 	  else if (streq (p[1], "lzo"))
 	    {
 	      options->comp.alg = COMP_ALG_LZO;
-	      options->comp.flags = 0;
 	    }
 #endif
 #if defined(ENABLE_LZ4)
@@ -6336,6 +6340,10 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_LZ4;
 	      options->comp.flags = COMP_F_SWAP;
 	    }
+	  else if (streq (p[1], "lz4-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_LZ4;
+	    }
 #endif
 	  else
 	    {
Signature · 1 line
2.5.4 (Apple Git-61)
#7 lore raw

Hi Arne,

Some final comments (mostly just nitpicking). Other than this, I think the patch is ready to be merged. I tested it against James' openvpn 3 test server.

Quoted 10 lines · Arne Schwabe
src/openvpn/comp-lz4.c +10 -0
+static void
+lz4_compress (struct buffer *buf, struct buffer work,
+	      struct compress_context *compctx,
+	      const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);

src/openvpn/comp-lz4.c lz4_compress (struct buffer *buf, struct buffer work, · line 130

Quoted 10 lines
src/openvpn/comp-lz4.c +5 -1
+
+  /* On error do_lz4_compress sets buf len to zero, just return */
+  if (buf->len == 0)
+    return;
 
    /* did compression save us anything? */
    {
      uint8_t comp_head_byte = NO_COMPRESS_BYTE_SWAP;
-    if (compressed && work.len < buf->len)
+    if (compressed && work.len +2 < buf->len)

This +2 needs to go too - it only makes sense for the v2 format.

Quoted 21 lines
src/openvpn/comp-lz4.c +21 -0
+static void
+lz4v2_compress (struct buffer *buf, struct buffer work,
+		struct compress_context *compctx,
+		const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything?  Include 2 byte compression header
+     in calculation */
+  {
+    if (compressed && work.len + 2 < buf->len)
+      {
+	ASSERT(buf_prepend(&work, 2));

src/openvpn/comp-lz4.c lz4_compress (struct buffer *buf, struct buffer work, · line 180

Quoted 10 lines
src/openvpn/comp-lz4.c +10 -0
+	uint8_t *head = BPTR (&work);
+	head[0] = COMP_ALGV2_INDICATOR_BYTE;
+	head[1] = COMP_ALGV2_LZ4_BYTE;
+	*buf = work;
+      }
+    else
+      {
+	compv2_escape_data_ifneeded(buf);
+      }
+  }

This extra pair of scoping braces is not needed.

Quoted 7 lines
src/openvpn/options.c +0 -0
index 5a83897..b00059a 100644
@@ -6318,16 +6318,20 @@ add_option (struct options *options,
        VERIFY_PERMISSION (OPT_P_COMP);
        if (p[1])

src/openvpn/options.c add_option (struct options *options, · line 6329

Quoted 10 lines
src/openvpn/options.c +4 -0
  	{
+	  options->comp.flags = 0;
  	  if (streq (p[1], "stub"))
  	    {
  	      options->comp.alg = COMP_ALG_STUB;
  	      options->comp.flags = (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
  	    }
+	  else if (streq(p[1], "stub-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_UNCOMPRESSED;

I think this also needs

    options->comp.flags = COMP_F_ADVERTISE_STUBS_ONLY;

-Steffan

#8 lore raw

PATCH v5 1/1 Implement the compression V2 data format for stub and lz4.

In reply to Steffan Karger

Patch V2: Fix minor issues found by Steffan
Patch V3: split wire codes and compression flags Patch V4: Fix further issues reported by Gert Patch V5: really fix the issues that should be fixed in v2

It has been tested against v3 server and again itself. From James Mail:

Compression V2

I have observed that compression in many cases, even when enabled, often does not produce packet size reduction because much of the packet data typically generated by web sessions is already compressed. Further, the single byte that precedes the packet and indicates whether or not compression occurred has the unfortunate side effect of misaligning the IP packet in cases where compression did not occur. To remedy this, I propose a Compression V2 header that is optimized for the case where compression does not occur.

a. No compression occurred and first byte of IP/Ethernet packet
is NOT 0x50 (0 bytes of overhead and maintains alignment):

[ uncompressed IP/Ethernet packet ]

b. No compression occurred and first byte of IP/Ethernet packet

      is 0x50 (2 bytes of overhead but unlikely since no known
      IP packet can begin with 0x50):

        [ 0x50 ] [ 0x00 ] [ uncompressed IP/Ethernet packet ]

c. Compression occurred (2 bytes of overhead):

        [ 0x50 ] [ compression Alg ID ] [ compressed IP/Ethernet packet ]

      Compression Alg ID is one-byte algorithm identifier
      for LZ4 (0x1), LZO (0x2), or Snappy (0x3).

This approach has several beneficial effects:

1. In the common case where compression does not occur, no compression op is required, therefore there is zero overhead.

2. When compression does not occur, the IP/Ethernet packet alignment is retained.

3. This technique does not require any byte swapping with the tail of the packet which can potentially incur an expensive cache miss.

---
 src/openvpn/comp-lz4.c | 199 ++++++++++++++++++++++++++++++++++++++-----------
 src/openvpn/comp-lz4.h |   1 +
 src/openvpn/comp.c     |  31 ++++++++
 src/openvpn/comp.h     |  25 +++++++
 src/openvpn/compstub.c |  51 +++++++++++++
 src/openvpn/lzo.c      |   3 -
 src/openvpn/options.c  |  10 ++-
 7 files changed, 274 insertions(+), 46 deletions(-)
src/openvpn/comp-lz4.c +157 -42
index 4651148..ddd8e28 100644
@@ -44,9 +44,6 @@
 
 #include "memdbg.h"
 
-/* Initial command byte to tell our peer if we compressed */
-#define LZ4_COMPRESS_BYTE 0x69
-
 static void
 lz4_compress_init (struct compress_context *compctx)
 {
@@ -55,20 +52,22 @@ lz4_compress_init (struct compress_context *compctx)
 }
 
 static void
-lz4_compress_uninit (struct compress_context *compctx)
+lz4v2_compress_init (struct compress_context *compctx)
 {
+  msg (D_INIT_MEDIUM, "LZ4v2 compression initializing");
 }
 
 static void
-lz4_compress (struct buffer *buf, struct buffer work,
-	       struct compress_context *compctx,
-	       const struct frame* frame)
+lz4_compress_uninit (struct compress_context *compctx)
 {
-  bool compressed = false;
-
-  if (buf->len <= 0)
-    return;
+}
 
+static bool
+do_lz4_compress (struct buffer *buf,
+		 struct buffer *work,
+		 struct compress_context *compctx,
+		 const struct frame* frame)
+{
   /*
    * In order to attempt compression, length must be at least COMPRESS_THRESHOLD.
    */
@@ -78,38 +77,57 @@ lz4_compress (struct buffer *buf, struct buffer work,
       int zlen_max = ps + COMP_EXTRA_BUFFER (ps);
       int zlen;
 
-      ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
-      ASSERT (buf_safe (&work, zlen_max));
+      ASSERT (buf_init (work, FRAME_HEADROOM (frame)));
+      ASSERT (buf_safe (work, zlen_max));
 
       if (buf->len > ps)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression buffer overflow");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(&work), BLEN(buf), zlen_max );
+      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(work), BLEN(buf), zlen_max );
 
       if (zlen <= 0)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression error");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      ASSERT (buf_safe (&work, zlen));
-      work.len = zlen;
-      compressed = true;
+      ASSERT (buf_safe (work, zlen));
+      work->len = zlen;
+
 
-      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work.len);
+      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work->len);
       compctx->pre_compress += buf->len;
-      compctx->post_compress += work.len;
+      compctx->post_compress += work->len;
+      return true;
     }
+  return false;
+}
+
+
+static void
+lz4_compress (struct buffer *buf, struct buffer work,
+	      struct compress_context *compctx,
+	      const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On error do_lz4_compress sets buf len to zero, just return */
+  if (buf->len == 0)
+    return;
 
   /* did compression save us anything? */
   {
     uint8_t comp_head_byte = NO_COMPRESS_BYTE_SWAP;
-    if (compressed && work.len < buf->len)
+    if (compressed && work.len +2 < buf->len)
       {
 	*buf = work;
 	comp_head_byte = LZ4_COMPRESS_BYTE;
@@ -128,13 +146,72 @@ lz4_compress (struct buffer *buf, struct buffer work,
   }
 }
 
+
+static void
+lz4v2_compress (struct buffer *buf, struct buffer work,
+		struct compress_context *compctx,
+		const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything?  Include 2 byte compression header
+     in calculation */
+  {
+    if (compressed && work.len + 2 < buf->len)
+      {
+	ASSERT(buf_prepend(&work, 2));
+	uint8_t *head = BPTR (&work);
+	head[0] = COMP_ALGV2_INDICATOR_BYTE;
+	head[1] = COMP_ALGV2_LZ4_BYTE;
+	*buf = work;
+      }
+    else
+      {
+	compv2_escape_data_ifneeded(buf);
+      }
+  }
+}
+
+void
+do_lz4_decompress(size_t zlen_max,
+		  struct buffer *work,
+		  struct buffer *buf,
+		  struct compress_context *compctx)
+{
+  int uncomp_len;
+  ASSERT (buf_safe (work, zlen_max));
+  uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(work), (size_t)BLEN(buf), zlen_max);
+  if (uncomp_len <= 0)
+    {
+      dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
+      buf->len = 0;
+      return;
+    }
+
+  ASSERT (buf_safe (work, uncomp_len));
+  work->len = uncomp_len;
+
+  dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work->len);
+  compctx->pre_decompress += buf->len;
+  compctx->post_decompress += work->len;
+
+  *buf = *work;
+}
+
 static void
 lz4_decompress (struct buffer *buf, struct buffer work,
-		 struct compress_context *compctx,
-		 const struct frame* frame)
+		struct compress_context *compctx,
+		const struct frame* frame)
 {
   size_t zlen_max = EXPANDED_SIZE (frame);
-  int uncomp_len;
   uint8_t c;		/* flag indicating whether or not our peer compressed */
 
   if (buf->len <= 0)
@@ -152,23 +229,7 @@ lz4_decompress (struct buffer *buf, struct buffer work,
 
   if (c == LZ4_COMPRESS_BYTE)	/* packet was compressed */
     {
-      ASSERT (buf_safe (&work, zlen_max));
-      uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(&work), (size_t)BLEN(buf), zlen_max);
-      if (uncomp_len <= 0)
-	{
-	  dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
-	  buf->len = 0;
-	  return;
-	}
-
-      ASSERT (buf_safe (&work, uncomp_len));
-      work.len = uncomp_len;
-
-      dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work.len);
-      compctx->pre_decompress += buf->len;
-      compctx->post_decompress += work.len;
-
-      *buf = work;
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
     }
   else if (c == NO_COMPRESS_BYTE_SWAP)	/* packet was not compressed */
     {
@@ -181,6 +242,52 @@ lz4_decompress (struct buffer *buf, struct buffer work,
     }
 }
 
+static void
+lz4v2_decompress (struct buffer *buf, struct buffer work,
+		  struct compress_context *compctx,
+		  const struct frame* frame)
+{
+  size_t zlen_max = EXPANDED_SIZE (frame);
+  uint8_t c;		/* flag indicating whether or not our peer compressed */
+
+  if (buf->len <= 0)
+    return;
+
+  ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
+
+  /* do unframing/swap (assumes buf->len > 0) */
+  uint8_t *head = BPTR (buf);
+  c = *head;
+
+  /* Not compressed */
+  if (c != COMP_ALGV2_INDICATOR_BYTE) {
+    return;
+  }
+
+  /* Packet to short to make sense */
+  if (buf->len <= 1)
+    {
+      buf->len=0;
+      return;
+    }
+
+  c = head[1];
+  if (c == COMP_ALGV2_LZ4_BYTE) /* packet was compressed */
+    {
+      buf_advance(buf,2);
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
+    }
+  else if (c == COMP_ALGV2_UNCOMPRESSED_BYTE)
+    {
+      buf_advance(buf,2);
+    }
+  else
+    {
+      dmsg (D_COMP_ERRORS, "Bad LZ4v2 decompression header byte: %d", c);
+      buf->len = 0;
+    }
+}
+
 const struct compress_alg lz4_alg = {
   "lz4",
   lz4_compress_init,
@@ -189,6 +296,14 @@ const struct compress_alg lz4_alg = {
   lz4_decompress
 };
 
+const struct compress_alg lz4v2_alg = {
+  "lz4v2",
+  lz4v2_compress_init,
+  lz4_compress_uninit,
+  lz4v2_compress,
+  lz4v2_decompress
+};
+
 #else
 static void dummy(void) {}
 #endif /* ENABLE_LZ4 */
src/openvpn/comp-lz4.h +1 -0
index ca1dfa9..9d3c664 100644
@@ -31,6 +31,7 @@
 #include "buffer.h"
 
 extern const struct compress_alg lz4_alg;
+extern const struct compress_alg lz4v2_alg;
 
 struct lz4_workspace
 {
src/openvpn/comp.c +31 -0
index 706ad7e..809c83d 100644
@@ -50,6 +50,11 @@ comp_init(const struct compress_options *opt)
       compctx->alg = comp_stub_alg;
       (*compctx->alg.compress_init)(compctx);
       break;
+    case COMP_ALGV2_UNCOMPRESSED:
+      ALLOC_OBJ_CLEAR (compctx, struct compress_context);
+      compctx->flags = opt->flags;
+      compctx->alg = compv2_stub_alg;
+      break;
 #ifdef ENABLE_LZO
     case COMP_ALG_LZO:
       ALLOC_OBJ_CLEAR (compctx, struct compress_context);
@@ -65,11 +70,35 @@ comp_init(const struct compress_options *opt)
       compctx->alg = lz4_alg;
       (*compctx->alg.compress_init)(compctx);
       break;
+    case COMP_ALGV2_LZ4:
+      ALLOC_OBJ_CLEAR (compctx, struct compress_context);
+      compctx->flags = opt->flags;
+      compctx->alg = lz4v2_alg;
+      break;
 #endif
     }
   return compctx;
 }
 
+/* In the v2 compression schmemes, an uncompressed packet has
+ * has no opcode in front, unless the first byte is 0x50. In this
+ * case the packet needs to be escaped */
+void
+compv2_escape_data_ifneeded (struct buffer *buf)
+{
+    uint8_t *head = BPTR (buf);
+    if (head[0] != COMP_ALGV2_INDICATOR_BYTE)
+	return;
+
+    /* Header is 0x50 */
+    ASSERT(buf_prepend(buf, 2));
+
+    head = BPTR (buf);
+    head[0] = COMP_ALGV2_INDICATOR_BYTE;
+    head[1] = COMP_ALGV2_UNCOMPRESSED;
+}
+
+
 void
 comp_uninit(struct compress_context *compctx)
 {
@@ -120,6 +149,7 @@ comp_generate_peer_info_string(const struct compress_options *opt, struct buffer
 	{
 #if defined(ENABLE_LZ4)
 	  buf_printf (out, "IV_LZ4=1\n");
+	  buf_printf (out, "IV_LZ4v2=1\n");
 #endif
 #if defined(ENABLE_LZO)
 	  buf_printf (out, "IV_LZO=1\n");
@@ -129,6 +159,7 @@ comp_generate_peer_info_string(const struct compress_options *opt, struct buffer
       if (!lzo_avail)
 	buf_printf (out, "IV_LZO_STUB=1\n");
       buf_printf (out, "IV_COMP_STUB=1\n");
+      buf_printf (out, "IV_COMP_STUBv2=1\n");
     }
 }
 
src/openvpn/comp.h +25 -0
index 716b1c0..9ed9532 100644
@@ -43,12 +43,22 @@
 #define COMP_ALG_SNAPPY 3 /* Snappy algorithm (no longer supported) */
 #define COMP_ALG_LZ4    4 /* LZ4 algorithm */
 
+
+/* algorithm v2 */
+#define COMP_ALGV2_UNCOMPRESSED 10
+#define COMP_ALGV2_LZ4	    11
+/*
+#define COMP_ALGV2_LZO	    12
+#define COMP_ALGV2_SNAPPY   13
+*/
+
 /* Compression flags */
 #define COMP_F_ADAPTIVE   (1<<0) /* COMP_ALG_LZO only */
 #define COMP_F_ASYM       (1<<1) /* only downlink is compressed, not uplink */
 #define COMP_F_SWAP       (1<<2) /* initial command byte is swapped with last byte in buffer to preserve payload alignment */
 #define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support compression stubs */
 
+
 /*
  * Length of prepended prefix on compressed packets
  */
@@ -57,9 +67,21 @@
 /*
  * Prefix bytes
  */
+
+/* V1 on wire codes */
+/* Initial command byte to tell our peer if we compressed */
+#define LZO_COMPRESS_BYTE 0x66
+#define LZ4_COMPRESS_BYTE 0x69
 #define NO_COMPRESS_BYTE      0xFA
 #define NO_COMPRESS_BYTE_SWAP 0xFB /* to maintain payload alignment, replace this byte with last byte of packet */
 
+/* V2 on wire code */
+#define COMP_ALGV2_INDICATOR_BYTE	0x50
+#define COMP_ALGV2_UNCOMPRESSED_BYTE	0
+#define COMP_ALGV2_LZ4_BYTE		1
+#define COMP_ALGV2_LZO_BYTE		2
+#define COMP_ALGV2_SNAPPY_BYTE		3
+
 /*
  * Compress worst case size expansion (for any algorithm)
  *
@@ -145,6 +167,7 @@ struct compress_context
 };
 
 extern const struct compress_alg comp_stub_alg;
+extern const struct compress_alg compv2_stub_alg;
 
 struct compress_context *comp_init(const struct compress_options *opt);
 
@@ -157,6 +180,8 @@ void comp_print_stats (const struct compress_context *compctx, struct status_out
 
 void comp_generate_peer_info_string(const struct compress_options *opt, struct buffer *out);
 
+void compv2_escape_data_ifneeded (struct buffer *buf);
+
 static inline bool
 comp_enabled(const struct compress_options *info)
 {
src/openvpn/compstub.c +51 -0

57 lines not on this page · raw message

src/openvpn/lzo.c +0 -3
index daa02ed..25a839b 100644
@@ -42,9 +42,6 @@
 
 #include "memdbg.h"
 
-/* Initial command byte to tell our peer if we compressed */
-#define LZO_COMPRESS_BYTE 0x66
-
 /**
  * Perform adaptive compression housekeeping.
  *
src/openvpn/options.c +9 -1
index 5a83897..b00059a 100644
@@ -6318,16 +6318,20 @@ add_option (struct options *options,
       VERIFY_PERMISSION (OPT_P_COMP);
       if (p[1])
 	{
+	  options->comp.flags = 0;
 	  if (streq (p[1], "stub"))
 	    {
 	      options->comp.alg = COMP_ALG_STUB;
 	      options->comp.flags = (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
 	    }
+	  else if (streq(p[1], "stub-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
+	    }
 #if defined(ENABLE_LZO)
 	  else if (streq (p[1], "lzo"))
 	    {
 	      options->comp.alg = COMP_ALG_LZO;
-	      options->comp.flags = 0;
 	    }
 #endif
 #if defined(ENABLE_LZ4)
@@ -6336,6 +6340,10 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_LZ4;
 	      options->comp.flags = COMP_F_SWAP;
 	    }
+	  else if (streq (p[1], "lz4-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_LZ4;
+	    }
 #endif
 	  else
 	    {
Signature · 1 line
2.5.4 (Apple Git-61)
#9 lore raw

PATCH v6 1/1 Implement the compression V2 data format for stub and lz4.

In reply to Steffan Karger

Patch V2: Fix minor issues found by Steffan
Patch V3: split wire codes and compression flags Patch V4: Fix further issues reported by Gert Patch V5: really fix the issues that should be fixed in v2 Patch V6: fix more minor things

It has been tested against v3 server and again itself. From James Mail:

Compression V2

I have observed that compression in many cases, even when enabled, often does not produce packet size reduction because much of the packet data typically generated by web sessions is already compressed. Further, the single byte that precedes the packet and indicates whether or not compression occurred has the unfortunate side effect of misaligning the IP packet in cases where compression did not occur. To remedy this, I propose a Compression V2 header that is optimized for the case where compression does not occur.

a. No compression occurred and first byte of IP/Ethernet packet
is NOT 0x50 (0 bytes of overhead and maintains alignment):

[ uncompressed IP/Ethernet packet ]

b. No compression occurred and first byte of IP/Ethernet packet

      is 0x50 (2 bytes of overhead but unlikely since no known
      IP packet can begin with 0x50):

        [ 0x50 ] [ 0x00 ] [ uncompressed IP/Ethernet packet ]

c. Compression occurred (2 bytes of overhead):

        [ 0x50 ] [ compression Alg ID ] [ compressed IP/Ethernet packet ]

      Compression Alg ID is one-byte algorithm identifier
      for LZ4 (0x1), LZO (0x2), or Snappy (0x3).

This approach has several beneficial effects:

1. In the common case where compression does not occur, no compression op is required, therefore there is zero overhead.

2. When compression does not occur, the IP/Ethernet packet alignment is retained.

3. This technique does not require any byte swapping with the tail of the packet which can potentially incur an expensive cache miss.

---
 src/openvpn/comp-lz4.c | 195 ++++++++++++++++++++++++++++++++++++++-----------
 src/openvpn/comp-lz4.h |   1 +
 src/openvpn/comp.c     |  31 ++++++++
 src/openvpn/comp.h     |  25 +++++++
 src/openvpn/compstub.c |  51 +++++++++++++
 src/openvpn/lzo.c      |   3 -
 src/openvpn/options.c  |  11 ++-
 7 files changed, 272 insertions(+), 45 deletions(-)
src/openvpn/comp-lz4.c +154 -41
index 4651148..88e4ef7 100644
@@ -44,9 +44,6 @@
 
 #include "memdbg.h"
 
-/* Initial command byte to tell our peer if we compressed */
-#define LZ4_COMPRESS_BYTE 0x69
-
 static void
 lz4_compress_init (struct compress_context *compctx)
 {
@@ -55,20 +52,22 @@ lz4_compress_init (struct compress_context *compctx)
 }
 
 static void
-lz4_compress_uninit (struct compress_context *compctx)
+lz4v2_compress_init (struct compress_context *compctx)
 {
+  msg (D_INIT_MEDIUM, "LZ4v2 compression initializing");
 }
 
 static void
-lz4_compress (struct buffer *buf, struct buffer work,
-	       struct compress_context *compctx,
-	       const struct frame* frame)
+lz4_compress_uninit (struct compress_context *compctx)
 {
-  bool compressed = false;
-
-  if (buf->len <= 0)
-    return;
+}
 
+static bool
+do_lz4_compress (struct buffer *buf,
+		 struct buffer *work,
+		 struct compress_context *compctx,
+		 const struct frame* frame)
+{
   /*
    * In order to attempt compression, length must be at least COMPRESS_THRESHOLD.
    */
@@ -78,33 +77,52 @@ lz4_compress (struct buffer *buf, struct buffer work,
       int zlen_max = ps + COMP_EXTRA_BUFFER (ps);
       int zlen;
 
-      ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
-      ASSERT (buf_safe (&work, zlen_max));
+      ASSERT (buf_init (work, FRAME_HEADROOM (frame)));
+      ASSERT (buf_safe (work, zlen_max));
 
       if (buf->len > ps)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression buffer overflow");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(&work), BLEN(buf), zlen_max );
+      zlen = LZ4_compress_limitedOutput((const char *)BPTR(buf), (char *)BPTR(work), BLEN(buf), zlen_max );
 
       if (zlen <= 0)
 	{
 	  dmsg (D_COMP_ERRORS, "LZ4 compression error");
 	  buf->len = 0;
-	  return;
+	  return false;
 	}
 
-      ASSERT (buf_safe (&work, zlen));
-      work.len = zlen;
-      compressed = true;
+      ASSERT (buf_safe (work, zlen));
+      work->len = zlen;
+
 
-      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work.len);
+      dmsg (D_COMP, "LZ4 compress %d -> %d", buf->len, work->len);
       compctx->pre_compress += buf->len;
-      compctx->post_compress += work.len;
+      compctx->post_compress += work->len;
+      return true;
     }
+  return false;
+}
+
+
+static void
+lz4_compress (struct buffer *buf, struct buffer work,
+	      struct compress_context *compctx,
+	      const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On error do_lz4_compress sets buf len to zero, just return */
+  if (buf->len == 0)
+    return;
 
   /* did compression save us anything? */
   {
@@ -128,13 +146,70 @@ lz4_compress (struct buffer *buf, struct buffer work,
   }
 }
 
+
+static void
+lz4v2_compress (struct buffer *buf, struct buffer work,
+		struct compress_context *compctx,
+		const struct frame* frame)
+{
+  bool compressed;
+  if (buf->len <= 0)
+    return;
+
+  compressed = do_lz4_compress(buf, &work, compctx, frame);
+
+  /* On Error just return */
+  if (buf->len == 0)
+    return;
+
+  /* did compression save us anything?  Include 2 byte compression header
+     in calculation */
+  {
+    if (compressed && work.len + 2 < buf->len)
+      {
+	ASSERT(buf_prepend(&work, 2));
+	uint8_t *head = BPTR (&work);
+	head[0] = COMP_ALGV2_INDICATOR_BYTE;
+	head[1] = COMP_ALGV2_LZ4_BYTE;
+	*buf = work;
+      }
+    else
+      compv2_escape_data_ifneeded(buf);
+  }
+}
+
+void
+do_lz4_decompress(size_t zlen_max,
+		  struct buffer *work,
+		  struct buffer *buf,
+		  struct compress_context *compctx)
+{
+  int uncomp_len;
+  ASSERT (buf_safe (work, zlen_max));
+  uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(work), (size_t)BLEN(buf), zlen_max);
+  if (uncomp_len <= 0)
+    {
+      dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
+      buf->len = 0;
+      return;
+    }
+
+  ASSERT (buf_safe (work, uncomp_len));
+  work->len = uncomp_len;
+
+  dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work->len);
+  compctx->pre_decompress += buf->len;
+  compctx->post_decompress += work->len;
+
+  *buf = *work;
+}
+
 static void
 lz4_decompress (struct buffer *buf, struct buffer work,
-		 struct compress_context *compctx,
-		 const struct frame* frame)
+		struct compress_context *compctx,
+		const struct frame* frame)
 {
   size_t zlen_max = EXPANDED_SIZE (frame);
-  int uncomp_len;
   uint8_t c;		/* flag indicating whether or not our peer compressed */
 
   if (buf->len <= 0)
@@ -152,23 +227,7 @@ lz4_decompress (struct buffer *buf, struct buffer work,
 
   if (c == LZ4_COMPRESS_BYTE)	/* packet was compressed */
     {
-      ASSERT (buf_safe (&work, zlen_max));
-      uncomp_len = LZ4_decompress_safe((const char *)BPTR(buf), (char *)BPTR(&work), (size_t)BLEN(buf), zlen_max);
-      if (uncomp_len <= 0)
-	{
-	  dmsg (D_COMP_ERRORS, "LZ4 decompression error: %d", uncomp_len);
-	  buf->len = 0;
-	  return;
-	}
-
-      ASSERT (buf_safe (&work, uncomp_len));
-      work.len = uncomp_len;
-
-      dmsg (D_COMP, "LZ4 decompress %d -> %d", buf->len, work.len);
-      compctx->pre_decompress += buf->len;
-      compctx->post_decompress += work.len;
-
-      *buf = work;
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
     }
   else if (c == NO_COMPRESS_BYTE_SWAP)	/* packet was not compressed */
     {
@@ -181,6 +240,52 @@ lz4_decompress (struct buffer *buf, struct buffer work,
     }
 }
 
+static void
+lz4v2_decompress (struct buffer *buf, struct buffer work,
+		  struct compress_context *compctx,
+		  const struct frame* frame)
+{
+  size_t zlen_max = EXPANDED_SIZE (frame);
+  uint8_t c;		/* flag indicating whether or not our peer compressed */
+
+  if (buf->len <= 0)
+    return;
+
+  ASSERT (buf_init (&work, FRAME_HEADROOM (frame)));
+
+  /* do unframing/swap (assumes buf->len > 0) */
+  uint8_t *head = BPTR (buf);
+  c = *head;
+
+  /* Not compressed */
+  if (c != COMP_ALGV2_INDICATOR_BYTE) {
+    return;
+  }
+
+  /* Packet to short to make sense */
+  if (buf->len <= 1)
+    {
+      buf->len=0;
+      return;
+    }
+
+  c = head[1];
+  if (c == COMP_ALGV2_LZ4_BYTE) /* packet was compressed */
+    {
+      buf_advance(buf,2);
+      do_lz4_decompress(zlen_max, &work, buf, compctx);
+    }
+  else if (c == COMP_ALGV2_UNCOMPRESSED_BYTE)
+    {
+      buf_advance(buf,2);
+    }
+  else
+    {
+      dmsg (D_COMP_ERRORS, "Bad LZ4v2 decompression header byte: %d", c);
+      buf->len = 0;
+    }
+}
+
 const struct compress_alg lz4_alg = {
   "lz4",
   lz4_compress_init,
@@ -189,6 +294,14 @@ const struct compress_alg lz4_alg = {
   lz4_decompress
 };
 
+const struct compress_alg lz4v2_alg = {
+  "lz4v2",
+  lz4v2_compress_init,
+  lz4_compress_uninit,
+  lz4v2_compress,
+  lz4v2_decompress
+};
+
 #else
 static void dummy(void) {}
 #endif /* ENABLE_LZ4 */
src/openvpn/comp-lz4.h +1 -0
index ca1dfa9..9d3c664 100644
@@ -31,6 +31,7 @@
 #include "buffer.h"
 
 extern const struct compress_alg lz4_alg;
+extern const struct compress_alg lz4v2_alg;
 
 struct lz4_workspace
 {
src/openvpn/comp.c +31 -0
index 706ad7e..809c83d 100644
@@ -50,6 +50,11 @@ comp_init(const struct compress_options *opt)
       compctx->alg = comp_stub_alg;
       (*compctx->alg.compress_init)(compctx);
       break;
+    case COMP_ALGV2_UNCOMPRESSED:
+      ALLOC_OBJ_CLEAR (compctx, struct compress_context);
+      compctx->flags = opt->flags;
+      compctx->alg = compv2_stub_alg;
+      break;
 #ifdef ENABLE_LZO
     case COMP_ALG_LZO:
       ALLOC_OBJ_CLEAR (compctx, struct compress_context);
@@ -65,11 +70,35 @@ comp_init(const struct compress_options *opt)
       compctx->alg = lz4_alg;
       (*compctx->alg.compress_init)(compctx);
       break;
+    case COMP_ALGV2_LZ4:
+      ALLOC_OBJ_CLEAR (compctx, struct compress_context);
+      compctx->flags = opt->flags;
+      compctx->alg = lz4v2_alg;
+      break;
 #endif
     }
   return compctx;
 }
 
+/* In the v2 compression schmemes, an uncompressed packet has
+ * has no opcode in front, unless the first byte is 0x50. In this
+ * case the packet needs to be escaped */
+void
+compv2_escape_data_ifneeded (struct buffer *buf)
+{
+    uint8_t *head = BPTR (buf);
+    if (head[0] != COMP_ALGV2_INDICATOR_BYTE)
+	return;
+
+    /* Header is 0x50 */
+    ASSERT(buf_prepend(buf, 2));
+
+    head = BPTR (buf);
+    head[0] = COMP_ALGV2_INDICATOR_BYTE;
+    head[1] = COMP_ALGV2_UNCOMPRESSED;
+}
+
+
 void
 comp_uninit(struct compress_context *compctx)
 {
@@ -120,6 +149,7 @@ comp_generate_peer_info_string(const struct compress_options *opt, struct buffer
 	{
 #if defined(ENABLE_LZ4)
 	  buf_printf (out, "IV_LZ4=1\n");
+	  buf_printf (out, "IV_LZ4v2=1\n");
 #endif
 #if defined(ENABLE_LZO)
 	  buf_printf (out, "IV_LZO=1\n");
@@ -129,6 +159,7 @@ comp_generate_peer_info_string(const struct compress_options *opt, struct buffer
       if (!lzo_avail)
 	buf_printf (out, "IV_LZO_STUB=1\n");
       buf_printf (out, "IV_COMP_STUB=1\n");
+      buf_printf (out, "IV_COMP_STUBv2=1\n");
     }
 }
 
src/openvpn/comp.h +25 -0
index 716b1c0..9ed9532 100644
@@ -43,12 +43,22 @@
 #define COMP_ALG_SNAPPY 3 /* Snappy algorithm (no longer supported) */
 #define COMP_ALG_LZ4    4 /* LZ4 algorithm */
 
+
+/* algorithm v2 */
+#define COMP_ALGV2_UNCOMPRESSED 10
+#define COMP_ALGV2_LZ4	    11
+/*
+#define COMP_ALGV2_LZO	    12
+#define COMP_ALGV2_SNAPPY   13
+*/
+
 /* Compression flags */
 #define COMP_F_ADAPTIVE   (1<<0) /* COMP_ALG_LZO only */
 #define COMP_F_ASYM       (1<<1) /* only downlink is compressed, not uplink */
 #define COMP_F_SWAP       (1<<2) /* initial command byte is swapped with last byte in buffer to preserve payload alignment */
 #define COMP_F_ADVERTISE_STUBS_ONLY (1<<3) /* tell server that we only support compression stubs */
 
+
 /*
  * Length of prepended prefix on compressed packets
  */
@@ -57,9 +67,21 @@
 /*
  * Prefix bytes
  */
+
+/* V1 on wire codes */
+/* Initial command byte to tell our peer if we compressed */
+#define LZO_COMPRESS_BYTE 0x66
+#define LZ4_COMPRESS_BYTE 0x69
 #define NO_COMPRESS_BYTE      0xFA
 #define NO_COMPRESS_BYTE_SWAP 0xFB /* to maintain payload alignment, replace this byte with last byte of packet */
 
+/* V2 on wire code */
+#define COMP_ALGV2_INDICATOR_BYTE	0x50
+#define COMP_ALGV2_UNCOMPRESSED_BYTE	0
+#define COMP_ALGV2_LZ4_BYTE		1
+#define COMP_ALGV2_LZO_BYTE		2
+#define COMP_ALGV2_SNAPPY_BYTE		3
+
 /*
  * Compress worst case size expansion (for any algorithm)
  *
@@ -145,6 +167,7 @@ struct compress_context
 };
 
 extern const struct compress_alg comp_stub_alg;
+extern const struct compress_alg compv2_stub_alg;
 
 struct compress_context *comp_init(const struct compress_options *opt);
 
@@ -157,6 +180,8 @@ void comp_print_stats (const struct compress_context *compctx, struct status_out
 
 void comp_generate_peer_info_string(const struct compress_options *opt, struct buffer *out);
 
+void compv2_escape_data_ifneeded (struct buffer *buf);
+
 static inline bool
 comp_enabled(const struct compress_options *info)
 {
src/openvpn/compstub.c +51 -0

57 lines not on this page · raw message

src/openvpn/lzo.c +0 -3
index daa02ed..25a839b 100644
@@ -42,9 +42,6 @@
 
 #include "memdbg.h"
 
-/* Initial command byte to tell our peer if we compressed */
-#define LZO_COMPRESS_BYTE 0x66
-
 /**
  * Perform adaptive compression housekeeping.
  *
src/openvpn/options.c +10 -1
index 5a83897..af60850 100644
@@ -6318,16 +6318,21 @@ add_option (struct options *options,
       VERIFY_PERMISSION (OPT_P_COMP);
       if (p[1])
 	{
+	  options->comp.flags = 0;
 	  if (streq (p[1], "stub"))
 	    {
 	      options->comp.alg = COMP_ALG_STUB;
 	      options->comp.flags = (COMP_F_SWAP|COMP_F_ADVERTISE_STUBS_ONLY);
 	    }
+	  else if (streq(p[1], "stub-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_UNCOMPRESSED;
+	      options->comp.flags = COMP_F_ADVERTISE_STUBS_ONLY;
+	    }
 #if defined(ENABLE_LZO)
 	  else if (streq (p[1], "lzo"))
 	    {
 	      options->comp.alg = COMP_ALG_LZO;
-	      options->comp.flags = 0;
 	    }
 #endif
 #if defined(ENABLE_LZ4)
@@ -6336,6 +6341,10 @@ add_option (struct options *options,
 	      options->comp.alg = COMP_ALG_LZ4;
 	      options->comp.flags = COMP_F_SWAP;
 	    }
+	  else if (streq (p[1], "lz4-v2"))
+	    {
+	      options->comp.alg = COMP_ALGV2_LZ4;
+	    }
 #endif
 	  else
 	    {
Signature · 1 line
2.5.4 (Apple Git-61)
#10 lore raw
Quoted 4 lines · Arne Schwabe
src/openvpn/comp-lz4.c +4 -0
+  /* did compression save us anything?  Include 2 byte compression header
+     in calculation */
+  {
+    if (compressed && work.len + 2 < buf->len)

src/openvpn/comp-lz4.c lz4_compress (struct buffer *buf, struct buffer work, · line 178

Quoted 10 lines
src/openvpn/comp-lz4.c +10 -0
+      {
+       ASSERT(buf_prepend(&work, 2));
+       uint8_t *head = BPTR (&work);
+       head[0] = COMP_ALGV2_INDICATOR_BYTE;
+       head[1] = COMP_ALGV2_LZ4_BYTE;
+       *buf = work;
+      }
+    else
+      compv2_escape_data_ifneeded(buf);
+  }

I almost don't dare to say it - but I meant the outer braces surrounding this entire if () {} else {}-block, not the braces of the else-block (which I consider good practice). But, as just discussed on IRC, Gert could just take that when applying :)

So, ACK for everything except the braces ;)

-Steffan

#11 lore raw
In reply to Arne Schwabe

ACK from me as well (and thanks for bearing with Steffan and me).

I have stared at the code, and ran my test suite - I have not yet run end-to-end lz4v2 or stubv2, but my test server will pick up the change tomorrow and we'll see... :-)

Your patch has been applied to the master branch.

The {} brackets in lz4v2_compress() have been moved around as suggested by Steffan. Also, I have fixed a typo ("compression schmemes") that managed to evade earlier review.

commit a75bb2e40a431e053ea1ef328ec022aaf851ccc0 (master)

Author: Arne Schwabe
Date:   Sun Jan 3 18:27:46 2016 +0100
     Implement the compression V2 data format for stub and lz4.

     Acked-by: Steffan Karger <[email protected]...>
     Acked-by: Gert Doering <[email protected]...>
     Message-Id: <[email protected]...>
     URL: http://article.gmane.org/gmane.network.openvpn.devel/10925
     Signed-off-by: Gert Doering <[email protected]...>

--
kind regards,

Gert Doering