Attention is currently required from: Arne Schwabe.
Hello Arne Schwabe,
I'd like you to do a code review.
Please visit
https://gerrit.openvpn.net/c/openvpn/+/2143?usp=email
to review the following change.
Change subject: comp: share the stub compress methods with LZO and LZ4
......................................................................
comp: share the stub compress methods with LZO and LZ4
Since 7f0214c9 ("Remove support for compression on send") the LZO and
LZ4 compress methods only add the stub framing, duplicating
stub_compress() and stubv2_compress(). Export those as
comp_stub_compress() and compv2_stub_compress() and use them for all
methods. Also allocate the context only once in comp_init().
This also fixes LZO sending dropped packets. encrypt_sign() drops
packets by setting buf.len to 0 (e.g. before client-connect has
completed) and still calls the compress method. lzo_compress()
prepended NO_COMPRESS_BYTE regardless, turning the dropped packet into
a one byte packet that was sent. The stub methods return early on
empty buffers.
Change-Id: I7c9e123a458b9d3757e9de93f401e4e421741675
Signed-off-by: Razvan Cojocaru <[email protected]>
---
M doc/doxygen/doc_compression.h
M src/openvpn/comp-lz4.c
M src/openvpn/comp.c
M src/openvpn/comp.h
M src/openvpn/compstub.c
M src/openvpn/forward.h
M src/openvpn/lzo.c
7 files changed, 41 insertions(+), 81 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/43/2143/1
diff --git a/doc/doxygen/doc_compression.h b/doc/doxygen/doc_compression.h
index 1f2e93bd1..efd18bf 100644
--- a/doc/doxygen/doc_compression.h
+++ b/doc/doxygen/doc_compression.h
@@ -60,10 +60,10 @@
* Data Channel Control module\endlink and processes them according to the
* settings of the packet's VPN tunnel. The \link data_control Data
* Channel Control module\endlink uses the following interface functions:
- * - For packets which will be sent to a remote OpenVPN peer: \c
- * lzo_compress()
- * - For packets which have been received from a remote OpenVPN peer: \c
- * lzo_decompress()
+ * - For packets which will be sent to a remote OpenVPN peer: the \c
+ * compress method of \c compress_alg
+ * - For packets which have been received from a remote OpenVPN peer: the
+ * \c decompress method of \c compress_alg
*
* @par Settings that control this module's activity
* Whether or not the Data Channel Compression module is active depends on
diff --git a/src/openvpn/comp-lz4.c b/src/openvpn/comp-lz4.c
index 57c6bb3..192d1bd 100644
--- a/src/openvpn/comp-lz4.c
+++ b/src/openvpn/comp-lz4.c
@@ -55,40 +55,6 @@
{
}
-/* Doesn't do any actual compression anymore */
-static void
-lz4_compress(struct buffer *buf, struct buffer work, struct compress_context
*compctx,
- const struct frame *frame)
-{
- if (buf->len <= 0)
- {
- return;
- }
-
- uint8_t comp_head_byte = NO_COMPRESS_BYTE_SWAP;
- uint8_t *head = BPTR(buf);
- uint8_t *tail = BEND(buf);
- ASSERT(buf_safe(buf, 1));
- ++buf->len;
-
- /* move head byte of payload to tail */
- *tail = *head;
- *head = comp_head_byte;
-}
-
-/* Doesn't do any actual compression anymore */
-static void
-lz4v2_compress(struct buffer *buf, struct buffer work, struct compress_context
*compctx,
- const struct frame *frame)
-{
- if (buf->len <= 0)
- {
- return;
- }
-
- compv2_escape_data_ifneeded(buf);
-}
-
static void
do_lz4_decompress(int zlen_max, struct buffer *work, struct buffer *buf,
struct compress_context *compctx)
@@ -198,9 +164,9 @@
}
}
-const struct compress_alg lz4_alg = { "lz4", lz4_compress_init,
lz4_compress_uninit, lz4_compress,
- lz4_decompress };
+const struct compress_alg lz4_alg = { "lz4", lz4_compress_init,
lz4_compress_uninit,
+ comp_stub_compress, lz4_decompress };
const struct compress_alg lz4v2_alg = { "lz4v2", lz4v2_compress_init,
lz4_compress_uninit,
- lz4v2_compress, lz4v2_decompress };
+ compv2_stub_compress, lz4v2_decompress
};
#endif /* ENABLE_LZ4 */
diff --git a/src/openvpn/comp.c b/src/openvpn/comp.c
index 387a7c4..357eabc 100644
--- a/src/openvpn/comp.c
+++ b/src/openvpn/comp.c
@@ -38,47 +38,41 @@
struct compress_context *
comp_init(const struct compress_options *opt)
{
- struct compress_context *compctx = NULL;
+ const struct compress_alg *alg;
switch (opt->alg)
{
case COMP_ALG_STUB:
- ALLOC_OBJ_CLEAR(compctx, struct compress_context);
- compctx->flags = opt->flags;
- compctx->alg = comp_stub_alg;
+ alg = &comp_stub_alg;
break;
case COMP_ALGV2_UNCOMPRESSED:
- ALLOC_OBJ_CLEAR(compctx, struct compress_context);
- compctx->flags = opt->flags;
- compctx->alg = compv2_stub_alg;
+ alg = &compv2_stub_alg;
break;
#ifdef ENABLE_LZO
case COMP_ALG_LZO:
- ALLOC_OBJ_CLEAR(compctx, struct compress_context);
- compctx->flags = opt->flags;
- compctx->alg = lzo_alg;
+ alg = &lzo_alg;
break;
#endif
#ifdef ENABLE_LZ4
case COMP_ALG_LZ4:
- ALLOC_OBJ_CLEAR(compctx, struct compress_context);
- compctx->flags = opt->flags;
- compctx->alg = lz4_alg;
+ alg = &lz4_alg;
break;
case COMP_ALGV2_LZ4:
- ALLOC_OBJ_CLEAR(compctx, struct compress_context);
- compctx->flags = opt->flags;
- compctx->alg = lz4v2_alg;
+ alg = &lz4v2_alg;
break;
#endif
+ default:
+ return NULL;
}
- if (compctx)
- {
- (*compctx->alg.compress_init)(compctx);
- }
+
+ struct compress_context *compctx;
+ ALLOC_OBJ_CLEAR(compctx, struct compress_context);
+ compctx->flags = opt->flags;
+ compctx->alg = *alg;
+ (*compctx->alg.compress_init)(compctx);
return compctx;
}
diff --git a/src/openvpn/comp.h b/src/openvpn/comp.h
index ad80243..3f6411b 100644
--- a/src/openvpn/comp.h
+++ b/src/openvpn/comp.h
@@ -208,6 +208,14 @@
void compv2_escape_data_ifneeded(struct buffer *buf);
+/* Compress methods of the stub algorithms, also used by LZO and LZ4, which
+ * no longer compress on send */
+void comp_stub_compress(struct buffer *buf, struct buffer work, struct
compress_context *compctx,
+ const struct frame *frame);
+
+void compv2_stub_compress(struct buffer *buf, struct buffer work, struct
compress_context *compctx,
+ const struct frame *frame);
+
static inline bool
comp_enabled(const struct compress_options *info)
{
diff --git a/src/openvpn/compstub.c b/src/openvpn/compstub.c
index 2eef587..b1115c9 100644
--- a/src/openvpn/compstub.c
+++ b/src/openvpn/compstub.c
@@ -44,9 +44,9 @@
{
}
-static void
-stub_compress(struct buffer *buf, struct buffer work, struct compress_context
*compctx,
- const struct frame *frame)
+void
+comp_stub_compress(struct buffer *buf, struct buffer work, struct
compress_context *compctx,
+ const struct frame *frame)
{
if (buf->len <= 0)
{
@@ -104,9 +104,9 @@
}
-static void
-stubv2_compress(struct buffer *buf, struct buffer work, struct
compress_context *compctx,
- const struct frame *frame)
+void
+compv2_stub_compress(struct buffer *buf, struct buffer work, struct
compress_context *compctx,
+ const struct frame *frame)
{
if (buf->len <= 0)
{
@@ -154,8 +154,8 @@
}
const struct compress_alg compv2_stub_alg = { "stubv2", stub_compress_init,
stub_compress_uninit,
- stubv2_compress,
stubv2_decompress };
+ compv2_stub_compress,
stubv2_decompress };
const struct compress_alg comp_stub_alg = { "stub", stub_compress_init,
stub_compress_uninit,
- stub_compress, stub_decompress };
+ comp_stub_compress,
stub_decompress };
#endif /* USE_STUB */
diff --git a/src/openvpn/forward.h b/src/openvpn/forward.h
index 375635b..1a67319 100644
--- a/src/openvpn/forward.h
+++ b/src/openvpn/forward.h
@@ -102,7 +102,7 @@
* - Check that the client authentication has succeeded; if not, drop the
* packet.
* - If the \a comp_frag argument is true:
- * - Call \c lzo_compress() of the \link compression Data Channel Compression
+ * - Call the \c compress method of the \link compression Data Channel
Compression
* module\endlink to (possibly) compress the packet.
* - Call \c fragment_outgoing() of the \link fragmentation Data Channel
Fragmentation
* module\endlink to (possibly) fragment the packet.
@@ -191,7 +191,7 @@
* - Call \c fragment_incoming() of the \link fragmentation Data Channel
* Fragmentation module\endlink to reassemble the packet if it's
* fragmented.
- * - Call \c lzo_decompress() of the \link compression Data Channel
+ * - Call the \c decompress method of the \link compression Data Channel
* Compression module\endlink to decompress the packet if it's
* compressed.
* - Place the resulting packet in \c c->c2.to_tun so that it can be sent
diff --git a/src/openvpn/lzo.c b/src/openvpn/lzo.c
index 51cf5e8..3cf0d79 100644
--- a/src/openvpn/lzo.c
+++ b/src/openvpn/lzo.c
@@ -65,14 +65,6 @@
}
static void
-lzo_compress(struct buffer *buf, struct buffer work, struct compress_context
*compctx,
- const struct frame *frame)
-{
- uint8_t *header = buf_prepend(buf, 1);
- *header = NO_COMPRESS_BYTE;
-}
-
-static void
lzo_decompress(struct buffer *buf, struct buffer work, struct compress_context
*compctx,
const struct frame *frame)
{
@@ -120,6 +112,6 @@
}
}
-const struct compress_alg lzo_alg = { "lzo", lzo_compress_init,
lzo_compress_uninit, lzo_compress,
- lzo_decompress };
+const struct compress_alg lzo_alg = { "lzo", lzo_compress_init,
lzo_compress_uninit,
+ comp_stub_compress, lzo_decompress };
#endif /* ENABLE_LZO */
--
To view, visit https://gerrit.openvpn.net/c/openvpn/+/2143?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.openvpn.net/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I7c9e123a458b9d3757e9de93f401e4e421741675
Gerrit-Change-Number: 2143
Gerrit-PatchSet: 1
Gerrit-Owner: Răzvan Cojocaru <[email protected]>
Gerrit-Reviewer: Arne Schwabe <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: Arne Schwabe <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
