diff mbox series

[Openvpn-devel,v4,5/8] Use new frame header methods to calculate OCC_MTU_LOAD payload size

Message ID 20220210162632.3309974-5-arne@rfc2549.org
State Accepted
Headers show
Series [Openvpn-devel,v4,1/8] Replace TUN_MTU_SIZE with frame->tun_mtu | expand

Commit Message

Arne Schwabe Feb. 10, 2022, 5:26 a.m. UTC 
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/occ.c | 31 +++++++++++++++++++++++--------
 1 file changed, 23 insertions(+), 8 deletions(-)

Comments

Gert Doering Feb. 12, 2022, 11:36 p.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

.. though I'm not sure I understand any of this code, or how to trigger
it.  So I've only run client/server tests, and stared at the code changes
for a bit.

I think it actually breaks the original intent, as the "changes per
iteration" variable "entry->delta" is gone now - so this is a semi-NAK,
as in "we need to come back here and fix this".  But I want to move
forward with the frame stuff, and 7/8 can not go in without this.

Can anyone tell me how to actually trigger an "OCC MTU test"?  --mtu-disc 
is doing socket level stuff ("only supported on OSes ... necessary system
call") - so maybe this was broken / inactive already before...

Your patch has been applied to the master branch.

commit fb76954a232decc86e071a76861c1e4fdd2507ab
Author: Arne Schwabe
Date:   Thu Feb 10 17:26:29 2022 +0100

     Use new frame header methods to calculate OCC_MTU_LOAD payload size

     Signed-off-by: Arne Schwabe <arne@rfc2549.org>
     Acked-by: Gert Doering <gert@greenie.muc.de>
     Message-Id: <20220210162632.3309974-5-arne@rfc2549.org>
     URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23750.html
     Signed-off-by: Gert Doering <gert@greenie.muc.de>


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/occ.c b/src/openvpn/occ.c
index 6fc5e003..b7670356 100644
--- a/src/openvpn/occ.c
+++ b/src/openvpn/occ.c
@@ -199,8 +199,11 @@  check_send_occ_load_test_dowork(struct context *c)
         if (entry->op >= 0)
         {
             c->c2.occ_op = entry->op;
-            c->c2.occ_mtu_load_size =
-                EXPANDED_SIZE(&c->c2.frame) + entry->delta;
+            size_t payload_size = frame_calculate_payload_size(&c->c2.frame,
+                                                               &c->options, &c->c1.ks.key_type);
+            size_t header_size = frame_calculate_protocol_header_size(&c->c1.ks.key_type, &c->options, false);
+
+            c->c2.occ_mtu_load_size = payload_size + header_size;
         }
         else
         {
@@ -298,10 +301,21 @@  check_send_occ_msg_dowork(struct context *c)
             {
                 break;
             }
-            need_to_add = min_int(c->c2.occ_mtu_load_size, EXPANDED_SIZE(&c->c2.frame))
+            size_t proto_hdr, payload_hdr;
+            const struct key_type *kt = &c->c1.ks.key_type;
+
+            /* OCC message have comp/fragment headers but not ethernet headers */
+            payload_hdr = frame_calculate_payload_overhead(&c->c2.frame, &c->options,
+                                                           kt, false);
+
+            /* Since we do not know the payload size we just pass 0 as size here */
+            proto_hdr = frame_calculate_protocol_header_size(kt, &c->options, false);
+
+            need_to_add = min_int(c->c2.occ_mtu_load_size, c->c2.frame.buf.payload_size)
                           - OCC_STRING_SIZE
-                          - sizeof(uint8_t)
-                          - EXTRA_FRAME(&c->c2.frame);
+                          - sizeof(uint8_t)     /* occ opcode */
+                          - payload_hdr
+                          - proto_hdr;
 
             while (need_to_add > 0)
             {
@@ -314,12 +328,13 @@  check_send_occ_msg_dowork(struct context *c)
                 }
                 --need_to_add;
             }
-            dmsg(D_PACKET_CONTENT, "SENT OCC_MTU_LOAD min_int(%d-%d-%d-%d,%d) size=%d",
+            dmsg(D_PACKET_CONTENT, "SENT OCC_MTU_LOAD min_int(%d,%d)-%d-%d-%d-%d) size=%d",
                  c->c2.occ_mtu_load_size,
+                 c->c2.frame.buf.payload_size,
                  OCC_STRING_SIZE,
                  (int) sizeof(uint8_t),
-                 EXTRA_FRAME(&c->c2.frame),
-                 c->c2.frame.buf.payload_size,
+                 (int) payload_hdr,
+                 (int) proto_hdr,
                  BLEN(&c->c2.buf));
             doit = true;
         }