diff mbox series

[Openvpn-devel,v3,04/14] Fix datagram_overhead and assorted functions

Message ID 20220101162532.2251835-5-arne@rfc2549.org
State Accepted
Headers show
Series Big buffer/frame refactoring patch set v3 | expand

Commit Message

Arne Schwabe Jan. 1, 2022, 5:25 a.m. UTC 
This function is supposed to calculate the overhead of the protocol
header (IP/IPv6 + TCP/UDP). But at some point the index that used
to index the array proto_overhead and the associated PROTO_N went
completely out of sync. This fixed the function and related caller
to again calculate the overhead as intended.

Note that IPv6 mapped IPv4 addresses still have the wrong overhead calculated
as they treated as IPv6 addresses (0:0:0:0:0:ffff::/96)

Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
 src/openvpn/forward.c | 10 ++++++----
 src/openvpn/socket.c  | 16 +++-------------
 src/openvpn/socket.h  | 17 ++++++-----------
 3 files changed, 15 insertions(+), 28 deletions(-)

Comments

Gert Doering Feb. 2, 2022, 2:39 a.m. UTC | #1 
Acked-by: Gert Doering <gert@greenie.muc.de>

For reference, this was 12/21 in the first patch series, and got an
ACK from Frank Lichtenheld there.

Stared at the code.  Looks bigger than it really is :-) - what it
really does is pass on the socket address family down the call
chain to the "datagram_overhead()" function, which will then know
if it's v4 or v6.  

Should we ever support SCTP, we'll only have to adjust a single
function here, no mixture of functions and tables... :-)

Tested client and server side.  P2P NCP is still broken (waiting for 
13/14), Rest works.

Your patch has been applied to the master branch.

commit a63a727b020ef42c475bd861b960200686359b2d
Author: Arne Schwabe
Date:   Sat Jan 1 17:25:22 2022 +0100

     Fix datagram_overhead and assorted functions

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


--
kind regards,

Gert Doering
diff mbox series

Patch

diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index c971c6bd..6de6b4d4 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -480,10 +480,10 @@  check_fragment(struct context *c)
     struct link_socket_info *lsi = get_link_socket_info(c);
 
     /* OS MTU Hint? */
-    if (lsi->mtu_changed)
+    if (lsi->mtu_changed && lsi->lsa)
     {
         frame_adjust_path_mtu(&c->c2.frame_fragment, c->c2.link_socket->mtu,
-                              c->options.ce.proto);
+                              lsi->lsa->actual.dest.addr.sa.sa_family, lsi->proto);
         lsi->mtu_changed = false;
     }
 
@@ -1565,8 +1565,10 @@  process_outgoing_link(struct context *c)
              */
             if (c->options.shaper)
             {
-                shaper_wrote_bytes(&c->c2.shaper, BLEN(&c->c2.to_link)
-                                   + datagram_overhead(c->options.ce.proto));
+                int overhead = datagram_overhead(c->c2.to_link_addr->dest.addr.sa.sa_family,
+                                                 c->options.ce.proto);
+                shaper_wrote_bytes(&c->c2.shaper,
+                                   BLEN(&c->c2.to_link) + overhead);
             }
 
             /*
diff --git a/src/openvpn/socket.c b/src/openvpn/socket.c
index df736746..93d2e61e 100644
--- a/src/openvpn/socket.c
+++ b/src/openvpn/socket.c
@@ -44,17 +44,6 @@ 
 
 #include "memdbg.h"
 
-const int proto_overhead[] = { /* indexed by PROTO_x */
-    0,
-    IPv4_UDP_HEADER_SIZE, /* IPv4 */
-    IPv4_TCP_HEADER_SIZE,
-    IPv4_TCP_HEADER_SIZE,
-    IPv6_UDP_HEADER_SIZE, /* IPv6 */
-    IPv6_TCP_HEADER_SIZE,
-    IPv6_TCP_HEADER_SIZE,
-    IPv6_TCP_HEADER_SIZE,
-};
-
 /*
  * Convert sockflags/getaddr_flags into getaddr_flags
  */
@@ -1660,9 +1649,10 @@  socket_frame_init(const struct frame *frame, struct link_socket *sock)
  * to us by the OS.
  */
 void
-frame_adjust_path_mtu(struct frame *frame, int pmtu, int proto)
+frame_adjust_path_mtu(struct frame *frame, int pmtu, sa_family_t af, int proto)
 {
-    frame_set_mtu_dynamic(frame, pmtu - datagram_overhead(proto), SET_MTU_UPPER_BOUND);
+    frame_set_mtu_dynamic(frame, pmtu - datagram_overhead(af, proto),
+                          SET_MTU_UPPER_BOUND);
 }
 
 static void
diff --git a/src/openvpn/socket.h b/src/openvpn/socket.h
index cc1e0c36..936ef262 100644
--- a/src/openvpn/socket.h
+++ b/src/openvpn/socket.h
@@ -300,7 +300,7 @@  void do_preresolve(struct context *c);
 
 void socket_adjust_frame_parameters(struct frame *frame, int proto);
 
-void frame_adjust_path_mtu(struct frame *frame, int pmtu, int proto);
+void frame_adjust_path_mtu(struct frame *frame, int pmtu, sa_family_t af, int proto);
 
 void link_socket_close(struct link_socket *sock);
 
@@ -579,18 +579,13 @@  const char *addr_family_name(int af);
 /*
  * Overhead added to packets by various protocols.
  */
-#define IPv4_UDP_HEADER_SIZE              28
-#define IPv4_TCP_HEADER_SIZE              40
-#define IPv6_UDP_HEADER_SIZE              48
-#define IPv6_TCP_HEADER_SIZE              60
-
-extern const int proto_overhead[];
-
 static inline int
-datagram_overhead(int proto)
+datagram_overhead(sa_family_t af, int proto)
 {
-    ASSERT(proto >= 0 && proto < PROTO_N);
-    return proto_overhead [proto];
+    int overhead = 0;
+    overhead += (proto == PROTO_UDP) ? 8 : 20;
+    overhead += (af == AF_INET) ? 20 : 40;
+    return overhead;
 }
 
 /*