Re: [Openvpn-devel] [PATCH 14/21] Add mtu paramter to --fragment and change fragment calculation

2021-12-10 Thread Frank Lichtenheld
> Arne Schwabe  hat am 07.12.2021 18:02 geschrieben:
> 
> diff --git a/doc/man-sections/link-options.rst 
> b/doc/man-sections/link-options.rst
> index f41c0c4f1..b71656e0b 100644
> --- a/doc/man-sections/link-options.rst
> +++ b/doc/man-sections/link-options.rst
> @@ -24,13 +24,18 @@ the local and the remote host.
>from any address, not only the address which was specified in the
>``--remote`` option.
>  
> ---fragment max
> +--fragment max [mtu]
>Enable internal datagram fragmentation so that no UDP datagrams are sent
>which are larger than ``max`` bytes.
>  
> -  The ``max`` parameter is interpreted in the same way as the
> -  ``--link-mtu`` parameter, i.e. the UDP packet size after encapsulation
> -  overhead has been added in, but not including the UDP header itself.
> +  If the :code:`mtu` parameter is present the ``max`` parameter is
> +  interpreted to include IP and UDP encapsulation overhead. The
> +  :code:`mtu` parameter is introduced in OpenVPN version 2.6.0.
> +
> +  If the :code:`mtu` parameter is absent, the ``max`` parameter is
> +  interpreted in the same way as the ``--link-mtu`` parameter, i.e.
> +  the UDP packet size after encapsulation overhead has been added in,
> +  but not including the UDP header itself.

The same comment as for mssfix applies here: "--fragment max [mtu]" is not 
valid rst syntax.
Even "--fragment max mtu" is invalid. You need to do something like

--fragment args
  Valid syntaxes::

 fragment max
 fragment max mtu

Like we do for other parameters.

Regards,
--
Frank Lichtenheld


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


Re: [Openvpn-devel] [PATCH 14/21] Add mtu paramter to --fragment and change fragment calculation

2021-12-09 Thread Frank Lichtenheld



> Arne Schwabe  hat am 07.12.2021 18:02 geschrieben:
> diff --git a/Changes.rst b/Changes.rst
> index cf6a2f86d..c673196fa 100644
> --- a/Changes.rst
> +++ b/Changes.rst
> @@ -63,10 +63,11 @@ Optional ciphers in ``--data-ciphers``
>  those as optional and only use them if the SSL library supports them.
>  
>  
> -Improved ``--mssfix`` calculation
> -The ``--mssfix`` option now allows an optional :code:`mtu` parameter to 
> specify
> -that different overhead for IPv4/IPv6 should taken into account and the 
> resulting
> -size is specified as the total size of the VPN packets including IP and 
> UDP headers.
> +Improved ``--mssfix`` and ``--fragement`` calculation

Typo, should be "--fragment"

> +The ``--mssfix`` and ``--fragment`` options now allow an optional 
> :code:`mtu`
> +parameter to specify that different overhead for IPv4/IPv6 should taken 
> into
> +account and the resulting size is specified as the total size of the VPN 
> packets
> +including IP and UDP headers.

Regards,
  Frank

--
Frank Lichtenheld


___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel


[Openvpn-devel] [PATCH 14/21] Add mtu paramter to --fragment and change fragment calculation

2021-12-07 Thread Arne Schwabe
Instead relying on the link_mtu_dynamic field and its calculation
in the frame struct, add a new field max_fragment_size and add
a calculation of it similar to mssfix.

Also whenever mssfix value is calculated, we also want to calculate
the values for fragment as both options need to be calculated from
the real overhead.

Signed-off-by: Arne Schwabe 
---
 Changes.rst   |   9 +--
 doc/man-sections/link-options.rst |  13 ++--
 src/openvpn/forward.c |   3 +-
 src/openvpn/fragment.c|   4 +-
 src/openvpn/init.c|  23 +++
 src/openvpn/mss.c | 101 +++---
 src/openvpn/mss.h |  13 +++-
 src/openvpn/mtu.c |  66 +--
 src/openvpn/mtu.h |  22 +++
 src/openvpn/options.c |  12 +++-
 src/openvpn/options.h |   2 +
 src/openvpn/socket.c  |  11 
 src/openvpn/socket.h  |   2 -
 src/openvpn/ssl.c |  20 ++
 14 files changed, 157 insertions(+), 144 deletions(-)

diff --git a/Changes.rst b/Changes.rst
index cf6a2f86d..c673196fa 100644
--- a/Changes.rst
+++ b/Changes.rst
@@ -63,10 +63,11 @@ Optional ciphers in ``--data-ciphers``
 those as optional and only use them if the SSL library supports them.
 
 
-Improved ``--mssfix`` calculation
-The ``--mssfix`` option now allows an optional :code:`mtu` parameter to 
specify
-that different overhead for IPv4/IPv6 should taken into account and the 
resulting
-size is specified as the total size of the VPN packets including IP and 
UDP headers.
+Improved ``--mssfix`` and ``--fragement`` calculation
+The ``--mssfix`` and ``--fragment`` options now allow an optional 
:code:`mtu`
+parameter to specify that different overhead for IPv4/IPv6 should taken 
into
+account and the resulting size is specified as the total size of the VPN 
packets
+including IP and UDP headers.
 
 Deprecated features
 ---
diff --git a/doc/man-sections/link-options.rst 
b/doc/man-sections/link-options.rst
index f41c0c4f1..b71656e0b 100644
--- a/doc/man-sections/link-options.rst
+++ b/doc/man-sections/link-options.rst
@@ -24,13 +24,18 @@ the local and the remote host.
   from any address, not only the address which was specified in the
   ``--remote`` option.
 
---fragment max
+--fragment max [mtu]
   Enable internal datagram fragmentation so that no UDP datagrams are sent
   which are larger than ``max`` bytes.
 
-  The ``max`` parameter is interpreted in the same way as the
-  ``--link-mtu`` parameter, i.e. the UDP packet size after encapsulation
-  overhead has been added in, but not including the UDP header itself.
+  If the :code:`mtu` parameter is present the ``max`` parameter is
+  interpreted to include IP and UDP encapsulation overhead. The
+  :code:`mtu` parameter is introduced in OpenVPN version 2.6.0.
+
+  If the :code:`mtu` parameter is absent, the ``max`` parameter is
+  interpreted in the same way as the ``--link-mtu`` parameter, i.e.
+  the UDP packet size after encapsulation overhead has been added in,
+  but not including the UDP header itself.
 
   The ``--fragment`` option only makes sense when you are using the UDP
   protocol (``--proto udp``).
diff --git a/src/openvpn/forward.c b/src/openvpn/forward.c
index 6de6b4d49..3f362e95d 100644
--- a/src/openvpn/forward.c
+++ b/src/openvpn/forward.c
@@ -482,8 +482,7 @@ check_fragment(struct context *c)
 /* OS MTU Hint? */
 if (lsi->mtu_changed && lsi->lsa)
 {
-frame_adjust_path_mtu(>c2.frame_fragment, c->c2.link_socket->mtu,
-  lsi->lsa->actual.dest.addr.sa.sa_family, 
lsi->proto);
+frame_adjust_path_mtu(c);
 lsi->mtu_changed = false;
 }
 
diff --git a/src/openvpn/fragment.c b/src/openvpn/fragment.c
index 6f8fb4476..ce8cd3489 100644
--- a/src/openvpn/fragment.c
+++ b/src/openvpn/fragment.c
@@ -335,12 +335,12 @@ fragment_outgoing(struct fragment_master *f, struct 
buffer *buf,
 msg(D_FRAG_ERRORS, "FRAG: outgoing buffer is not empty, 
len=[%d,%d]",
 buf->len, f->outgoing.len);
 }
-if (buf->len > PAYLOAD_SIZE_DYNAMIC(frame)) /* should we fragment? */
+if (buf->len > frame->max_fragment_size) /* should we fragment? */
 {
 /*
  * Send the datagram as a series of 2 or more fragments.
  */
-f->outgoing_frag_size = optimal_fragment_size(buf->len, 
PAYLOAD_SIZE_DYNAMIC(frame));
+f->outgoing_frag_size = optimal_fragment_size(buf->len, 
frame->max_fragment_size);
 if (buf->len > f->outgoing_frag_size * MAX_FRAGS)
 {
 FRAG_ERR("too many fragments would be required to send 
datagram");
diff --git a/src/openvpn/init.c b/src/openvpn/init.c
index b3653971c..31ae250bc 100644
--- a/src/openvpn/init.c
+++ b/src/openvpn/init.c
@@ -2535,14 +2535,6 @@