[Openvpn-devel,06/21] Remove post_open_mtu code
Commit Message
This code is probably from a time when we could not set the MTU on
the Windows tap6 driver. Nowadays we can set the MTU on this device,
so this code is a noop now.
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
---
src/openvpn/init.c | 10 ----------
src/openvpn/ssl.c | 2 +-
src/openvpn/tun.c | 1 -
src/openvpn/tun.h | 4 ----
4 files changed, 1 insertion(+), 16 deletions(-)
Comments
> Arne Schwabe <arne@rfc2549.org> hat am 07.12.2021 18:01 geschrieben:
> diff --git a/src/openvpn/ssl.c b/src/openvpn/ssl.c
> index 8cbb129d2..303e3fe8f 100644
> --- a/src/openvpn/ssl.c
> +++ b/src/openvpn/ssl.c
> @@ -1897,7 +1897,7 @@ tls_session_update_crypto_params_do_work(struct tls_session *session,
> frame_print(frame, D_MTU_INFO, "Data Channel MTU parms");
>
> /*
> - * mssfix uses data channel framing, which at this point contains
> + * mssfix uses data channel framing, which at this poipnt contains
Spurious change
Regards,
Frank
--
Frank Lichtenheld
> Arne Schwabe <arne@rfc2549.org> hat am 07.12.2021 18:01 geschrieben:
> This code is probably from a time when we could not set the MTU on
> the Windows tap6 driver. Nowadays we can set the MTU on this device,
> so this code is a noop now.
[...]
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index 75d5eaf7b..12bdd2005 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -6071,7 +6071,6 @@ tuntap_get_mtu(struct tuntap *tt)
> &mtu, sizeof(mtu),
> &mtu, sizeof(mtu), &len, NULL))
> {
> - tt->post_open_mtu = (int)mtu;
> msg(D_MTU_INFO, "TAP-Windows MTU=%d", (int)mtu);
> }
> }
This change makes no sense to me. Why would you not remove the complete tuntap_get_mtu call? If the whole point was to get the MTU, and now it will be the one we set, why even to the ioctl at all?
Regards,
Frank
--
Frank Lichtenheld
Am 10.12.21 um 17:17 schrieb Frank Lichtenheld:
>> Arne Schwabe <arne@rfc2549.org> hat am 07.12.2021 18:01 geschrieben:
>> This code is probably from a time when we could not set the MTU on
>> the Windows tap6 driver. Nowadays we can set the MTU on this device,
>> so this code is a noop now.
> [...]
>> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
>> index 75d5eaf7b..12bdd2005 100644
>> --- a/src/openvpn/tun.c
>> +++ b/src/openvpn/tun.c
>> @@ -6071,7 +6071,6 @@ tuntap_get_mtu(struct tuntap *tt)
>> &mtu, sizeof(mtu),
>> &mtu, sizeof(mtu), &len, NULL))
>> {
>> - tt->post_open_mtu = (int)mtu;
>> msg(D_MTU_INFO, "TAP-Windows MTU=%d", (int)mtu);
>> }
>> }
>
> This change makes no sense to me. Why would you not remove the complete tuntap_get_mtu call? If the whole point was to get the MTU, and now it will be the one we set, why even to the ioctl at all?
I wanted to keep the message that is currently printed at verb 1. If we
decide that the message can be removed I can remove the calling of the
ioctl as well.
Arne
Acked-by: Gert Doering <gert@greenie.muc.de>
I follow the reasoning - since we set the MTU now, we do not need to
adjust the frame afterwards. Logging the MTU might be useful (Windows
is weird, maybe it does not always "stick"...), so let's keep this in
for the time being.
Spurious comment change in ssl.c removed from the patch.
I like the comment
/* Some TUN/TAP drivers like to be ioctled for mtu after open */
"like to be ioctled", hah :-) - but why this is there is lost lore from
the long gone past...
I have test compiled this on mingw.
Your patch has been applied to the master branch.
commit c27868bfc2ad6ee21c28f4addb1c2fc003bfa61a
Author: Arne Schwabe
Date: Tue Dec 7 18:01:56 2021 +0100
Remove post_open_mtu code
Signed-off-by: Arne Schwabe <arne@rfc2549.org>
Acked-by: Gert Doering <gert@greenie.muc.de>
Message-Id: <20211207170211.3275837-7-arne@rfc2549.org>
URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg23327.html
Signed-off-by: Gert Doering <gert@greenie.muc.de>
--
kind regards,
Gert Doering
@@ -1809,16 +1809,6 @@ do_open_tun(struct context *c)
c->c1.tuntap, c->plugins, c->c2.es, &c->net_ctx);
}
- /*
- * Did tun/tap driver give us an MTU?
- */
- if (c->c1.tuntap->post_open_mtu)
- {
- frame_set_mtu_dynamic(&c->c2.frame,
- c->c1.tuntap->post_open_mtu,
- SET_MTU_TUN | SET_MTU_UPPER_BOUND);
- }
-
ret = true;
static_context = c;
#ifndef TARGET_ANDROID
@@ -1897,7 +1897,7 @@ tls_session_update_crypto_params_do_work(struct tls_session *session,
frame_print(frame, D_MTU_INFO, "Data Channel MTU parms");
/*
- * mssfix uses data channel framing, which at this point contains
+ * mssfix uses data channel framing, which at this poipnt contains
* actual overhead. Fragmentation logic uses frame_fragment, which
* still contains worst case overhead. Replace it with actual overhead
* to prevent unneeded fragmentation.
@@ -6071,7 +6071,6 @@ tuntap_get_mtu(struct tuntap *tt)
&mtu, sizeof(mtu),
&mtu, sizeof(mtu), &len, NULL))
{
- tt->post_open_mtu = (int)mtu;
msg(D_MTU_INFO, "TAP-Windows MTU=%d", (int)mtu);
}
}
@@ -214,10 +214,6 @@ struct tuntap
#endif
/* used for printing status info only */
unsigned int rwflags_debug;
-
- /* Some TUN/TAP drivers like to be ioctled for mtu
- * after open */
- int post_open_mtu;
};
static inline bool