[Openvpn-devel] Set the correct mtu on windows based systems

Message ID 20190404111656.14916-1-cschenk@mail.uni-paderborn.de
State Superseded
Headers show
Series [Openvpn-devel] Set the correct mtu on windows based systems | expand

Commit Message

Christopher Schenk April 4, 2019, 12:16 a.m. UTC
Signed-off-by: Christopher Schenk <cschenk@mail.uni-paderborn.de>
---
 include/openvpn-msg.h         |  8 ++++
 src/openvpn/tun.c             | 89 +++++++++++++++++++++++++++++++++++
 src/openvpnserv/interactive.c | 31 ++++++++++++
 3 files changed, 128 insertions(+)

Comments

Christopher Schenk April 17, 2019, 11:14 p.m. UTC | #1
Hi,

what do you think about this patch? Is anything missing?

Best regards

Christopher

On 4/4/19 1:16 PM, Christopher Schenk wrote:
> Signed-off-by: Christopher Schenk <cschenk@mail.uni-paderborn.de>
> ---
>   include/openvpn-msg.h         |  8 ++++
>   src/openvpn/tun.c             | 89 +++++++++++++++++++++++++++++++++++
>   src/openvpnserv/interactive.c | 31 ++++++++++++
>   3 files changed, 128 insertions(+)
>
> diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
> index 66177a21..10cd68ac 100644
> --- a/include/openvpn-msg.h
> +++ b/include/openvpn-msg.h
> @@ -39,6 +39,7 @@ typedef enum {
>       msg_del_block_dns,
>       msg_register_dns,
>       msg_enable_dhcp,
> +    msg_set_mtu,
>   } message_type_t;
>   
>   typedef struct {
> @@ -117,4 +118,11 @@ typedef struct {
>       interface_t iface;
>   } enable_dhcp_message_t;
>   
> +typedef struct {
> +    message_header_t header;
> +    interface_t iface;
> +    short family;
> +    int mtu;
> +} set_mtu_message_t;
> +
>   #endif /* ifndef OPENVPN_MSG_H_ */
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index 48a8fdf7..3895e421 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -69,6 +69,10 @@ static void netsh_ifconfig(const struct tuntap_options *to,
>                              const in_addr_t netmask,
>                              const unsigned int flags);
>   
> +static void windows_set_mtu(const int iface_index,
> +                            const short family,
> +                            const int mtu);
> +
>   static void netsh_set_dns6_servers(const struct in6_addr *addr_list,
>                                      const int addr_len,
>                                      const char *flex_name);
> @@ -201,6 +205,47 @@ out:
>       return ret;
>   }
>   
> +static bool
> +do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu)
> +{
> +    DWORD len;
> +    bool ret = false;
> +    ack_message_t ack;
> +    struct gc_arena gc = gc_new();
> +    HANDLE pipe = tt->options.msg_channel;
> +    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
> +    set_mtu_message_t mtu_msg = {
> +        .header = {
> +            msg_set_mtu,
> +            sizeof(set_mtu_message_t),
> +            0
> +        },
> +        .iface = {.index = tt->adapter_index,.name = tt->actual_name },
> +        .mtu = mtu,
> +        .family = family
> +    };
> +
> +    if (!send_msg_iservice(pipe, &mtu_msg, sizeof(mtu_msg), &ack, "Set_mtu"))
> +    {
> +        goto out;
> +    }
> +
> +    if (ack.error_number != NO_ERROR)
> +    {
> +        msg(M_NONFATAL, "TUN: setting %s mtu using service failed: %s [status=%u if_index=%d]",
> +            family_name, strerror_win32(ack.error_number, &gc), ack.error_number, mtu_msg.iface.index);
> +    }
> +    else
> +    {
> +        msg(M_INFO, "%s MTU set to %d on interface %d using service", family_name, mtu, mtu_msg.iface.index);
> +        ret = true;
> +    }
> +
> +out:
> +    gc_free(&gc);
> +    return ret;
> +}
> +
>   #endif /* ifdef _WIN32 */
>   
>   #ifdef TARGET_SOLARIS
> @@ -984,6 +1029,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char *ifname, int tun_mtu,
>       {
>           do_address_service(true, AF_INET6, tt);
>           do_dns6_service(true, tt);
> +        do_set_mtu_service(tt, AF_INET6, tun_mtu);
>       }
>       else
>       {
> @@ -1000,6 +1046,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char *ifname, int tun_mtu,
>           netsh_command(&argv, 4, M_FATAL);
>           /* set ipv6 dns servers if any are specified */
>           netsh_set_dns6_servers(tt->options.dns6, tt->options.dns6_len, ifname);
> +        windows_set_mtu(tt->adapter_index, AF_INET6, tun_mtu);
>       }
>   
>       /* explicit route needed */
> @@ -1394,6 +1441,14 @@ do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
>   
>                   break;
>           }
> +        if (tt->options.msg_channel)
> +        {
> +            do_set_mtu_service(tt, AF_INET, tun_mtu);
> +        }
> +        else
> +        {
> +            windows_set_mtu(tt->adapter_index, AF_INET, tun_mtu);
> +        }
>       }
>   
>   #else  /* if defined(TARGET_LINUX) */
> @@ -5236,6 +5291,40 @@ out:
>       return ret;
>   }
>   
> +static void
> +windows_set_mtu(const int iface_index, const short family,
> +                const int mtu)
> +{
> +    DWORD err = 0;
> +    struct gc_arena gc = gc_new();
> +    MIB_IPINTERFACE_ROW ipiface;
> +    InitializeIpInterfaceEntry(&ipiface);
> +    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
> +    ipiface.Family = family;
> +    ipiface.InterfaceIndex = iface_index;
> +    err = GetIpInterfaceEntry(&ipiface);
> +    if (err == NO_ERROR)
> +    {
> +        if (family == AF_INET)
> +        {
> +            ipiface.SitePrefixLength = 0;
> +        }
> +        ipiface.NlMtu = mtu;
> +        err = SetIpInterfaceEntry(&ipiface);
> +    }
> +
> +    if (err != NO_ERROR)
> +    {
> +        msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u if_index=%d]",
> +            family_name, strerror_win32(err, &gc), err, iface_index);
> +    }
> +    else
> +    {
> +        msg(M_INFO, "Successfully set %s mtu on interface %d", family_name, iface_index);
> +    }
> +}
> +
> +
>   /*
>    * Return a TAP name for netsh commands.
>    */
> diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
> index 623c3ff7..c24cb22b 100644
> --- a/src/openvpnserv/interactive.c
> +++ b/src/openvpnserv/interactive.c
> @@ -1198,6 +1198,29 @@ HandleEnableDHCPMessage(const enable_dhcp_message_t *dhcp)
>       return err;
>   }
>   
> +static DWORD
> +HandleMTUMessage(const set_mtu_message_t *mtu)
> +{
> +    DWORD err = 0;
> +    MIB_IPINTERFACE_ROW ipiface;
> +    InitializeIpInterfaceEntry(&ipiface);
> +    ipiface.Family = mtu->family;
> +    ipiface.InterfaceIndex = mtu->iface.index;
> +    err = GetIpInterfaceEntry(&ipiface);
> +    if (err != NO_ERROR)
> +    {
> +        return err;
> +    }
> +    if (mtu->family == AF_INET)
> +    {
> +        ipiface.SitePrefixLength = 0;
> +    }
> +    ipiface.NlMtu = mtu->mtu;
> +
> +    err = SetIpInterfaceEntry(&ipiface);
> +    return err;
> +}
> +
>   static VOID
>   HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists_t *lists)
>   {
> @@ -1210,6 +1233,7 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists
>           block_dns_message_t block_dns;
>           dns_cfg_message_t dns;
>           enable_dhcp_message_t dhcp;
> +        set_mtu_message_t mtu;
>       } msg;
>       ack_message_t ack = {
>           .header = {
> @@ -1277,6 +1301,13 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists
>               }
>               break;
>   
> +        case msg_set_mtu:
> +            if (msg.header.size == sizeof(msg.mtu))
> +            {
> +                ack.error_number = HandleMTUMessage(&msg.mtu);
> +            }
> +            break;
> +
>           default:
>               ack.error_number = ERROR_MESSAGE_TYPE;
>               MsgToEventLog(MSG_FLAGS_ERROR, TEXT("Unknown message type %d"), msg.header.type);
Selva Nair April 18, 2019, 5:47 a.m. UTC | #2
Hi,

Thanks for the new version. I somehow missed it as it
went into a new thread.

For the record, this is an updated version of the series
https://patchwork.openvpn.net/project/openvpn2/list/?series=471
with requested changes:

(i) tabs removed + some good white-space changes
(ii) GetIpInterfaceEntry() called first to read-in the current
parameters before calling SetIpInterfaceEntry()

Summary:
The  patch sets the tun_mtu value (either the default of 1500 or
user-specified --tun-mtu xxx) on the tun/tap  interface on Windows
using the IP helper API, preferably via the interactive service.

Looks good now and a quick test on WIndows 7 behaves
as expected.

I'm ACKing this, but one remark: the patch does not revert the
mtu back to the previous value on disconnect. This is an issue
only (?) if we want to support clean downgrades to an earlier
version without recreating the tun/tap interfaces.

On Thu, Apr 4, 2019 at 7:18 AM Christopher Schenk <
cschenk@mail.uni-paderborn.de> wrote:

> Signed-off-by: Christopher Schenk <cschenk@mail.uni-paderborn.de>
> ---
>  include/openvpn-msg.h         |  8 ++++
>  src/openvpn/tun.c             | 89 +++++++++++++++++++++++++++++++++++
>  src/openvpnserv/interactive.c | 31 ++++++++++++
>  3 files changed, 128 insertions(+)
>
> diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
> index 66177a21..10cd68ac 100644
> --- a/include/openvpn-msg.h
> +++ b/include/openvpn-msg.h
> @@ -39,6 +39,7 @@ typedef enum {
>      msg_del_block_dns,
>      msg_register_dns,
>      msg_enable_dhcp,
> +    msg_set_mtu,
>  } message_type_t;
>
>  typedef struct {
> @@ -117,4 +118,11 @@ typedef struct {
>      interface_t iface;
>  } enable_dhcp_message_t;
>
> +typedef struct {
> +    message_header_t header;
> +    interface_t iface;
> +    short family;
> +    int mtu;
> +} set_mtu_message_t;
> +
>  #endif /* ifndef OPENVPN_MSG_H_ */
> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> index 48a8fdf7..3895e421 100644
> --- a/src/openvpn/tun.c
> +++ b/src/openvpn/tun.c
> @@ -69,6 +69,10 @@ static void netsh_ifconfig(const struct tuntap_options
> *to,
>                             const in_addr_t netmask,
>                             const unsigned int flags);
>
> +static void windows_set_mtu(const int iface_index,
> +                            const short family,
> +                            const int mtu);
> +
>  static void netsh_set_dns6_servers(const struct in6_addr *addr_list,
>                                     const int addr_len,
>                                     const char *flex_name);
> @@ -201,6 +205,47 @@ out:
>      return ret;
>  }
>
> +static bool
> +do_set_mtu_service(const struct tuntap *tt, const short family, const int
> mtu)
> +{
> +    DWORD len;
> +    bool ret = false;
> +    ack_message_t ack;
> +    struct gc_arena gc = gc_new();
> +    HANDLE pipe = tt->options.msg_channel;
> +    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
> +    set_mtu_message_t mtu_msg = {
> +        .header = {
> +            msg_set_mtu,
> +            sizeof(set_mtu_message_t),
> +            0
> +        },
> +        .iface = {.index = tt->adapter_index,.name = tt->actual_name },
> +        .mtu = mtu,
> +        .family = family
> +    };
> +
> +    if (!send_msg_iservice(pipe, &mtu_msg, sizeof(mtu_msg), &ack,
> "Set_mtu"))
> +    {
> +        goto out;
> +    }
> +
> +    if (ack.error_number != NO_ERROR)
> +    {
> +        msg(M_NONFATAL, "TUN: setting %s mtu using service failed: %s
> [status=%u if_index=%d]",
> +            family_name, strerror_win32(ack.error_number, &gc),
> ack.error_number, mtu_msg.iface.index);
>
+    }
> +    else
> +    {
> +        msg(M_INFO, "%s MTU set to %d on interface %d using service",
> family_name, mtu, mtu_msg.iface.index);
> +        ret = true;
> +    }
> +
> +out:
> +    gc_free(&gc);
> +    return ret;
> +}
> +
>  #endif /* ifdef _WIN32 */
>
>  #ifdef TARGET_SOLARIS
> @@ -984,6 +1029,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char
> *ifname, int tun_mtu,
>      {
>          do_address_service(true, AF_INET6, tt);
>          do_dns6_service(true, tt);
> +        do_set_mtu_service(tt, AF_INET6, tun_mtu);
>      }
>      else
>      {
> @@ -1000,6 +1046,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char
> *ifname, int tun_mtu,
>          netsh_command(&argv, 4, M_FATAL);
>          /* set ipv6 dns servers if any are specified */
>          netsh_set_dns6_servers(tt->options.dns6, tt->options.dns6_len,
> ifname);
> +        windows_set_mtu(tt->adapter_index, AF_INET6, tun_mtu);
>      }
>
>      /* explicit route needed */
> @@ -1394,6 +1441,14 @@ do_ifconfig_ipv4(struct tuntap *tt, const char
> *ifname, int tun_mtu,
>
>                  break;
>          }
> +        if (tt->options.msg_channel)
> +        {
> +            do_set_mtu_service(tt, AF_INET, tun_mtu);
> +        }
> +        else
> +        {
> +            windows_set_mtu(tt->adapter_index, AF_INET, tun_mtu);
> +        }
>      }
>
>  #else  /* if defined(TARGET_LINUX) */
> @@ -5236,6 +5291,40 @@ out:
>      return ret;
>  }
>
> +static void
> +windows_set_mtu(const int iface_index, const short family,
> +                const int mtu)
> +{
> +    DWORD err = 0;
> +    struct gc_arena gc = gc_new();
> +    MIB_IPINTERFACE_ROW ipiface;
> +    InitializeIpInterfaceEntry(&ipiface);
> +    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
> +    ipiface.Family = family;
> +    ipiface.InterfaceIndex = iface_index;
> +    err = GetIpInterfaceEntry(&ipiface);
> +    if (err == NO_ERROR)
> +    {
> +        if (family == AF_INET)
> +        {
> +            ipiface.SitePrefixLength = 0;
> +        }
> +        ipiface.NlMtu = mtu;
> +        err = SetIpInterfaceEntry(&ipiface);
> +    }
> +
> +    if (err != NO_ERROR)
> +    {
> +        msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u
> if_index=%d]",
> +            family_name, strerror_win32(err, &gc), err, iface_index);
> +    }
> +    else
> +    {
> +        msg(M_INFO, "Successfully set %s mtu on interface %d",
> family_name, iface_index);
> +    }
> +}
> +
> +
>  /*
>   * Return a TAP name for netsh commands.
>   */
> diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
> index 623c3ff7..c24cb22b 100644
> --- a/src/openvpnserv/interactive.c
> +++ b/src/openvpnserv/interactive.c
> @@ -1198,6 +1198,29 @@ HandleEnableDHCPMessage(const enable_dhcp_message_t
> *dhcp)
>      return err;
>  }
>
> +static DWORD
> +HandleMTUMessage(const set_mtu_message_t *mtu)
> +{
> +    DWORD err = 0;
> +    MIB_IPINTERFACE_ROW ipiface;
> +    InitializeIpInterfaceEntry(&ipiface);
> +    ipiface.Family = mtu->family;
> +    ipiface.InterfaceIndex = mtu->iface.index;
> +    err = GetIpInterfaceEntry(&ipiface);
> +    if (err != NO_ERROR)
> +    {
> +        return err;
> +    }
> +    if (mtu->family == AF_INET)
> +    {
> +        ipiface.SitePrefixLength = 0;
> +    }
> +    ipiface.NlMtu = mtu->mtu;
> +
> +    err = SetIpInterfaceEntry(&ipiface);
> +    return err;
> +}
> +
>  static VOID
>  HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events,
> undo_lists_t *lists)
>  {
> @@ -1210,6 +1233,7 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD count,
> LPHANDLE events, undo_lists
>          block_dns_message_t block_dns;
>          dns_cfg_message_t dns;
>          enable_dhcp_message_t dhcp;
> +        set_mtu_message_t mtu;
>      } msg;
>      ack_message_t ack = {
>          .header = {
> @@ -1277,6 +1301,13 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD
> count, LPHANDLE events, undo_lists
>              }
>              break;
>
> +        case msg_set_mtu:
> +            if (msg.header.size == sizeof(msg.mtu))
> +            {
> +                ack.error_number = HandleMTUMessage(&msg.mtu);
> +            }
> +            break;
> +
>          default:
>              ack.error_number = ERROR_MESSAGE_TYPE;
>              MsgToEventLog(MSG_FLAGS_ERROR, TEXT("Unknown message type
> %d"), msg.header.type);
>
>
Acked-by: Selva Nair <selva.nair@gmail.com>

Selva
<div dir="ltr"><div><div><div><div><div dir="ltr"><div><div><div>Hi,<br><br></div>Thanks for the new version. I somehow missed it as it<br>went into a new thread.<br><br></div>For the record, this is an updated version of the series<br><a href="https://patchwork.openvpn.net/project/openvpn2/list/?series=471" target="_blank">https://patchwork.openvpn.net/project/openvpn2/list/?series=471</a><br></div><div>with requested changes:<br><br></div><div>(i) tabs removed + some good white-space changes<br></div><div>(ii) GetIpInterfaceEntry() called first to read-in the current<br>parameters before calling SetIpInterfaceEntry()<br><br>Summary:<br>The  patch sets the tun_mtu value (either the default of 1500 or<br>user-specified --tun-mtu xxx) on the tun/tap  interface on Windows<br>using the IP helper API, preferably via the interactive service.<br><br>Looks good now and a quick test on WIndows 7 behaves<br>as expected.<br><br></div></div></div></div>I&#39;m ACKing this, but one remark: the patch does not revert the<br>mtu back to the previous value on disconnect. This is an issue<br>only (?) if we want to support clean downgrades to an earlier<br>version without recreating the tun/tap interfaces.<br></div></div><div><div><br><div><div><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Thu, Apr 4, 2019 at 7:18 AM Christopher Schenk &lt;<a href="mailto:cschenk@mail.uni-paderborn.de" target="_blank">cschenk@mail.uni-paderborn.de</a>&gt; wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Signed-off-by: Christopher Schenk &lt;<a href="mailto:cschenk@mail.uni-paderborn.de" target="_blank">cschenk@mail.uni-paderborn.de</a>&gt;<br>
---<br>
 include/openvpn-msg.h         |  8 ++++<br>
 src/openvpn/tun.c             | 89 +++++++++++++++++++++++++++++++++++<br>
 src/openvpnserv/interactive.c | 31 ++++++++++++<br>
 3 files changed, 128 insertions(+)<br>
<br>
diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h<br>
index 66177a21..10cd68ac 100644<br>
--- a/include/openvpn-msg.h<br>
+++ b/include/openvpn-msg.h<br>
@@ -39,6 +39,7 @@ typedef enum {<br>
     msg_del_block_dns,<br>
     msg_register_dns,<br>
     msg_enable_dhcp,<br>
+    msg_set_mtu,<br>
 } message_type_t;<br>
<br>
 typedef struct {<br>
@@ -117,4 +118,11 @@ typedef struct {<br>
     interface_t iface;<br>
 } enable_dhcp_message_t;<br>
<br>
+typedef struct {<br>
+    message_header_t header;<br>
+    interface_t iface;<br>
+    short family;<br>
+    int mtu;<br>
+} set_mtu_message_t;<br>
+<br>
 #endif /* ifndef OPENVPN_MSG_H_ */<br>
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c<br>
index 48a8fdf7..3895e421 100644<br>
--- a/src/openvpn/tun.c<br>
+++ b/src/openvpn/tun.c<br>
@@ -69,6 +69,10 @@ static void netsh_ifconfig(const struct tuntap_options *to,<br>
                            const in_addr_t netmask,<br>
                            const unsigned int flags);<br>
<br>
+static void windows_set_mtu(const int iface_index,<br>
+                            const short family,<br>
+                            const int mtu);<br>
+<br>
 static void netsh_set_dns6_servers(const struct in6_addr *addr_list,<br>
                                    const int addr_len,<br>
                                    const char *flex_name);<br>
@@ -201,6 +205,47 @@ out:<br>
     return ret;<br>
 }<br>
<br>
+static bool<br>
+do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu)<br>
+{<br>
+    DWORD len;<br>
+    bool ret = false;<br>
+    ack_message_t ack;<br>
+    struct gc_arena gc = gc_new();<br>
+    HANDLE pipe = tt-&gt;options.msg_channel;<br>
+    const char *family_name = (family == AF_INET6) ? &quot;IPv6&quot; : &quot;IPv4&quot;;<br>
+    set_mtu_message_t mtu_msg = {<br>
+        .header = {<br>
+            msg_set_mtu,<br>
+            sizeof(set_mtu_message_t),<br>
+            0<br>
+        },<br>
+        .iface = {.index = tt-&gt;adapter_index,.name = tt-&gt;actual_name },<br>
+        .mtu = mtu,<br>
+        .family = family<br>
+    };<br>
+<br>
+    if (!send_msg_iservice(pipe, &amp;mtu_msg, sizeof(mtu_msg), &amp;ack, &quot;Set_mtu&quot;))<br>
+    {<br>
+        goto out;<br>
+    }<br>
+<br>
+    if (ack.error_number != NO_ERROR)<br>
+    {<br>
+        msg(M_NONFATAL, &quot;TUN: setting %s mtu using service failed: %s [status=%u if_index=%d]&quot;,<br>
+            family_name, strerror_win32(ack.error_number, &amp;gc), ack.error_number, mtu_msg.iface.index);<br></blockquote><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
+    }<br>
+    else<br>
+    {<br>
+        msg(M_INFO, &quot;%s MTU set to %d on interface %d using service&quot;, family_name, mtu, mtu_msg.iface.index);<br>
+        ret = true;<br>
+    }<br>
+<br>
+out:<br>
+    gc_free(&amp;gc);<br>
+    return ret;<br>
+}<br>
+<br>
 #endif /* ifdef _WIN32 */<br>
<br>
 #ifdef TARGET_SOLARIS<br>
@@ -984,6 +1029,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char *ifname, int tun_mtu,<br>
     {<br>
         do_address_service(true, AF_INET6, tt);<br>
         do_dns6_service(true, tt);<br>
+        do_set_mtu_service(tt, AF_INET6, tun_mtu);<br>
     }<br>
     else<br>
     {<br>
@@ -1000,6 +1046,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char *ifname, int tun_mtu,<br>
         netsh_command(&amp;argv, 4, M_FATAL);<br>
         /* set ipv6 dns servers if any are specified */<br>
         netsh_set_dns6_servers(tt-&gt;options.dns6, tt-&gt;options.dns6_len, ifname);<br>
+        windows_set_mtu(tt-&gt;adapter_index, AF_INET6, tun_mtu);<br>
     }<br>
<br>
     /* explicit route needed */<br>
@@ -1394,6 +1441,14 @@ do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,<br>
<br>
                 break;<br>
         }<br>
+        if (tt-&gt;options.msg_channel)<br>
+        {<br>
+            do_set_mtu_service(tt, AF_INET, tun_mtu);<br>
+        }<br>
+        else<br>
+        {<br>
+            windows_set_mtu(tt-&gt;adapter_index, AF_INET, tun_mtu);<br>
+        }<br>
     }<br>
<br>
 #else  /* if defined(TARGET_LINUX) */<br>
@@ -5236,6 +5291,40 @@ out:<br>
     return ret;<br>
 }<br>
<br>
+static void<br>
+windows_set_mtu(const int iface_index, const short family,<br>
+                const int mtu)<br>
+{<br>
+    DWORD err = 0;<br>
+    struct gc_arena gc = gc_new();<br>
+    MIB_IPINTERFACE_ROW ipiface;<br>
+    InitializeIpInterfaceEntry(&amp;ipiface);<br>
+    const char *family_name = (family == AF_INET6) ? &quot;IPv6&quot; : &quot;IPv4&quot;;<br>
+    ipiface.Family = family;<br>
+    ipiface.InterfaceIndex = iface_index;<br>
+    err = GetIpInterfaceEntry(&amp;ipiface);<br>
+    if (err == NO_ERROR)<br>
+    {<br>
+        if (family == AF_INET)<br>
+        {<br>
+            ipiface.SitePrefixLength = 0;<br>
+        }<br>
+        ipiface.NlMtu = mtu;<br>
+        err = SetIpInterfaceEntry(&amp;ipiface);<br>
+    }<br>
+<br>
+    if (err != NO_ERROR)<br>
+    {<br>
+        msg(M_WARN, &quot;TUN: Setting %s mtu failed: %s [status=%u if_index=%d]&quot;,<br>
+            family_name, strerror_win32(err, &amp;gc), err, iface_index);<br>
+    }<br>
+    else<br>
+    {<br>
+        msg(M_INFO, &quot;Successfully set %s mtu on interface %d&quot;, family_name, iface_index);<br>
+    }<br>
+}<br>
+<br>
+<br>
 /*<br>
  * Return a TAP name for netsh commands.<br>
  */<br>
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c<br>
index 623c3ff7..c24cb22b 100644<br>
--- a/src/openvpnserv/interactive.c<br>
+++ b/src/openvpnserv/interactive.c<br>
@@ -1198,6 +1198,29 @@ HandleEnableDHCPMessage(const enable_dhcp_message_t *dhcp)<br>
     return err;<br>
 }<br>
<br>
+static DWORD<br>
+HandleMTUMessage(const set_mtu_message_t *mtu)<br>
+{<br>
+    DWORD err = 0;<br>
+    MIB_IPINTERFACE_ROW ipiface;<br>
+    InitializeIpInterfaceEntry(&amp;ipiface);<br>
+    ipiface.Family = mtu-&gt;family;<br>
+    ipiface.InterfaceIndex = mtu-&gt;iface.index;<br>
+    err = GetIpInterfaceEntry(&amp;ipiface);<br>
+    if (err != NO_ERROR)<br>
+    {<br>
+        return err;<br>
+    }<br>
+    if (mtu-&gt;family == AF_INET)<br>
+    {<br>
+        ipiface.SitePrefixLength = 0;<br>
+    }<br>
+    ipiface.NlMtu = mtu-&gt;mtu;<br>
+<br>
+    err = SetIpInterfaceEntry(&amp;ipiface);<br>
+    return err;<br>
+}<br>
+<br>
 static VOID<br>
 HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists_t *lists)<br>
 {<br>
@@ -1210,6 +1233,7 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists<br>
         block_dns_message_t block_dns;<br>
         dns_cfg_message_t dns;<br>
         enable_dhcp_message_t dhcp;<br>
+        set_mtu_message_t mtu;<br>
     } msg;<br>
     ack_message_t ack = {<br>
         .header = {<br>
@@ -1277,6 +1301,13 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists<br>
             }<br>
             break;<br>
<br>
+        case msg_set_mtu:<br>
+            if (msg.header.size == sizeof(msg.mtu))<br>
+            {<br>
+                ack.error_number = HandleMTUMessage(&amp;msg.mtu);<br>
+            }<br>
+            break;<br>
+<br>
         default:<br>
             ack.error_number = ERROR_MESSAGE_TYPE;<br>
             MsgToEventLog(MSG_FLAGS_ERROR, TEXT(&quot;Unknown message type %d&quot;), msg.header.type);<br><br></blockquote><div><br></div><div>Acked-by: Selva Nair &lt;<a href="mailto:selva.nair@gmail.com" target="_blank">selva.nair@gmail.com</a>&gt;<br><br></div><div>Selva<br></div><div> <br></div></div></div></div></div></div></div>
Christopher Schenk April 23, 2019, 5:35 a.m. UTC | #3
Hi,

i think this is not a big issue since the mtu is only set until the 
system gets rebooted. So it behaves exactly like my netsh version where 
i specified a "store=active" in the set command.

Christopher

On 18/04/2019 17:47, Selva Nair wrote:
> Hi,
>
> Thanks for the new version. I somehow missed it as it
> went into a new thread.
>
> For the record, this is an updated version of the series
> https://patchwork.openvpn.net/project/openvpn2/list/?series=471
> with requested changes:
>
> (i) tabs removed + some good white-space changes
> (ii) GetIpInterfaceEntry() called first to read-in the current
> parameters before calling SetIpInterfaceEntry()
>
> Summary:
> The  patch sets the tun_mtu value (either the default of 1500 or
> user-specified --tun-mtu xxx) on the tun/tap  interface on Windows
> using the IP helper API, preferably via the interactive service.
>
> Looks good now and a quick test on WIndows 7 behaves
> as expected.
>
> I'm ACKing this, but one remark: the patch does not revert the
> mtu back to the previous value on disconnect. This is an issue
> only (?) if we want to support clean downgrades to an earlier
> version without recreating the tun/tap interfaces.
>
> On Thu, Apr 4, 2019 at 7:18 AM Christopher Schenk <
> cschenk@mail.uni-paderborn.de> wrote:
>
>> Signed-off-by: Christopher Schenk <cschenk@mail.uni-paderborn.de>
>> ---
>>   include/openvpn-msg.h         |  8 ++++
>>   src/openvpn/tun.c             | 89 +++++++++++++++++++++++++++++++++++
>>   src/openvpnserv/interactive.c | 31 ++++++++++++
>>   3 files changed, 128 insertions(+)
>>
>> diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
>> index 66177a21..10cd68ac 100644
>> --- a/include/openvpn-msg.h
>> +++ b/include/openvpn-msg.h
>> @@ -39,6 +39,7 @@ typedef enum {
>>       msg_del_block_dns,
>>       msg_register_dns,
>>       msg_enable_dhcp,
>> +    msg_set_mtu,
>>   } message_type_t;
>>
>>   typedef struct {
>> @@ -117,4 +118,11 @@ typedef struct {
>>       interface_t iface;
>>   } enable_dhcp_message_t;
>>
>> +typedef struct {
>> +    message_header_t header;
>> +    interface_t iface;
>> +    short family;
>> +    int mtu;
>> +} set_mtu_message_t;
>> +
>>   #endif /* ifndef OPENVPN_MSG_H_ */
>> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
>> index 48a8fdf7..3895e421 100644
>> --- a/src/openvpn/tun.c
>> +++ b/src/openvpn/tun.c
>> @@ -69,6 +69,10 @@ static void netsh_ifconfig(const struct tuntap_options
>> *to,
>>                              const in_addr_t netmask,
>>                              const unsigned int flags);
>>
>> +static void windows_set_mtu(const int iface_index,
>> +                            const short family,
>> +                            const int mtu);
>> +
>>   static void netsh_set_dns6_servers(const struct in6_addr *addr_list,
>>                                      const int addr_len,
>>                                      const char *flex_name);
>> @@ -201,6 +205,47 @@ out:
>>       return ret;
>>   }
>>
>> +static bool
>> +do_set_mtu_service(const struct tuntap *tt, const short family, const int
>> mtu)
>> +{
>> +    DWORD len;
>> +    bool ret = false;
>> +    ack_message_t ack;
>> +    struct gc_arena gc = gc_new();
>> +    HANDLE pipe = tt->options.msg_channel;
>> +    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
>> +    set_mtu_message_t mtu_msg = {
>> +        .header = {
>> +            msg_set_mtu,
>> +            sizeof(set_mtu_message_t),
>> +            0
>> +        },
>> +        .iface = {.index = tt->adapter_index,.name = tt->actual_name },
>> +        .mtu = mtu,
>> +        .family = family
>> +    };
>> +
>> +    if (!send_msg_iservice(pipe, &mtu_msg, sizeof(mtu_msg), &ack,
>> "Set_mtu"))
>> +    {
>> +        goto out;
>> +    }
>> +
>> +    if (ack.error_number != NO_ERROR)
>> +    {
>> +        msg(M_NONFATAL, "TUN: setting %s mtu using service failed: %s
>> [status=%u if_index=%d]",
>> +            family_name, strerror_win32(ack.error_number, &gc),
>> ack.error_number, mtu_msg.iface.index);
>>
> +    }
>> +    else
>> +    {
>> +        msg(M_INFO, "%s MTU set to %d on interface %d using service",
>> family_name, mtu, mtu_msg.iface.index);
>> +        ret = true;
>> +    }
>> +
>> +out:
>> +    gc_free(&gc);
>> +    return ret;
>> +}
>> +
>>   #endif /* ifdef _WIN32 */
>>
>>   #ifdef TARGET_SOLARIS
>> @@ -984,6 +1029,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char
>> *ifname, int tun_mtu,
>>       {
>>           do_address_service(true, AF_INET6, tt);
>>           do_dns6_service(true, tt);
>> +        do_set_mtu_service(tt, AF_INET6, tun_mtu);
>>       }
>>       else
>>       {
>> @@ -1000,6 +1046,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char
>> *ifname, int tun_mtu,
>>           netsh_command(&argv, 4, M_FATAL);
>>           /* set ipv6 dns servers if any are specified */
>>           netsh_set_dns6_servers(tt->options.dns6, tt->options.dns6_len,
>> ifname);
>> +        windows_set_mtu(tt->adapter_index, AF_INET6, tun_mtu);
>>       }
>>
>>       /* explicit route needed */
>> @@ -1394,6 +1441,14 @@ do_ifconfig_ipv4(struct tuntap *tt, const char
>> *ifname, int tun_mtu,
>>
>>                   break;
>>           }
>> +        if (tt->options.msg_channel)
>> +        {
>> +            do_set_mtu_service(tt, AF_INET, tun_mtu);
>> +        }
>> +        else
>> +        {
>> +            windows_set_mtu(tt->adapter_index, AF_INET, tun_mtu);
>> +        }
>>       }
>>
>>   #else  /* if defined(TARGET_LINUX) */
>> @@ -5236,6 +5291,40 @@ out:
>>       return ret;
>>   }
>>
>> +static void
>> +windows_set_mtu(const int iface_index, const short family,
>> +                const int mtu)
>> +{
>> +    DWORD err = 0;
>> +    struct gc_arena gc = gc_new();
>> +    MIB_IPINTERFACE_ROW ipiface;
>> +    InitializeIpInterfaceEntry(&ipiface);
>> +    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
>> +    ipiface.Family = family;
>> +    ipiface.InterfaceIndex = iface_index;
>> +    err = GetIpInterfaceEntry(&ipiface);
>> +    if (err == NO_ERROR)
>> +    {
>> +        if (family == AF_INET)
>> +        {
>> +            ipiface.SitePrefixLength = 0;
>> +        }
>> +        ipiface.NlMtu = mtu;
>> +        err = SetIpInterfaceEntry(&ipiface);
>> +    }
>> +
>> +    if (err != NO_ERROR)
>> +    {
>> +        msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u
>> if_index=%d]",
>> +            family_name, strerror_win32(err, &gc), err, iface_index);
>> +    }
>> +    else
>> +    {
>> +        msg(M_INFO, "Successfully set %s mtu on interface %d",
>> family_name, iface_index);
>> +    }
>> +}
>> +
>> +
>>   /*
>>    * Return a TAP name for netsh commands.
>>    */
>> diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
>> index 623c3ff7..c24cb22b 100644
>> --- a/src/openvpnserv/interactive.c
>> +++ b/src/openvpnserv/interactive.c
>> @@ -1198,6 +1198,29 @@ HandleEnableDHCPMessage(const enable_dhcp_message_t
>> *dhcp)
>>       return err;
>>   }
>>
>> +static DWORD
>> +HandleMTUMessage(const set_mtu_message_t *mtu)
>> +{
>> +    DWORD err = 0;
>> +    MIB_IPINTERFACE_ROW ipiface;
>> +    InitializeIpInterfaceEntry(&ipiface);
>> +    ipiface.Family = mtu->family;
>> +    ipiface.InterfaceIndex = mtu->iface.index;
>> +    err = GetIpInterfaceEntry(&ipiface);
>> +    if (err != NO_ERROR)
>> +    {
>> +        return err;
>> +    }
>> +    if (mtu->family == AF_INET)
>> +    {
>> +        ipiface.SitePrefixLength = 0;
>> +    }
>> +    ipiface.NlMtu = mtu->mtu;
>> +
>> +    err = SetIpInterfaceEntry(&ipiface);
>> +    return err;
>> +}
>> +
>>   static VOID
>>   HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events,
>> undo_lists_t *lists)
>>   {
>> @@ -1210,6 +1233,7 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD count,
>> LPHANDLE events, undo_lists
>>           block_dns_message_t block_dns;
>>           dns_cfg_message_t dns;
>>           enable_dhcp_message_t dhcp;
>> +        set_mtu_message_t mtu;
>>       } msg;
>>       ack_message_t ack = {
>>           .header = {
>> @@ -1277,6 +1301,13 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD
>> count, LPHANDLE events, undo_lists
>>               }
>>               break;
>>
>> +        case msg_set_mtu:
>> +            if (msg.header.size == sizeof(msg.mtu))
>> +            {
>> +                ack.error_number = HandleMTUMessage(&msg.mtu);
>> +            }
>> +            break;
>> +
>>           default:
>>               ack.error_number = ERROR_MESSAGE_TYPE;
>>               MsgToEventLog(MSG_FLAGS_ERROR, TEXT("Unknown message type
>> %d"), msg.header.type);
>>
>>
> Acked-by: Selva Nair <selva.nair@gmail.com>
>
> Selva
>
Christopher Schenk May 18, 2019, 2:38 a.m. UTC | #4
Hi,

any other thoughts on this patch? Any chance to getting it applied?

Christopher

On 4/23/19 5:35 PM, Christopher Schenk wrote:
> Hi,
> 
> i think this is not a big issue since the mtu is only set until the 
> system gets rebooted. So it behaves exactly like my netsh version where 
> i specified a "store=active" in the set command.
> 
> Christopher
> 
> On 18/04/2019 17:47, Selva Nair wrote:
>> Hi,
>>
>> Thanks for the new version. I somehow missed it as it
>> went into a new thread.
>>
>> For the record, this is an updated version of the series
>> https://patchwork.openvpn.net/project/openvpn2/list/?series=471
>> with requested changes:
>>
>> (i) tabs removed + some good white-space changes
>> (ii) GetIpInterfaceEntry() called first to read-in the current
>> parameters before calling SetIpInterfaceEntry()
>>
>> Summary:
>> The  patch sets the tun_mtu value (either the default of 1500 or
>> user-specified --tun-mtu xxx) on the tun/tap  interface on Windows
>> using the IP helper API, preferably via the interactive service.
>>
>> Looks good now and a quick test on WIndows 7 behaves
>> as expected.
>>
>> I'm ACKing this, but one remark: the patch does not revert the
>> mtu back to the previous value on disconnect. This is an issue
>> only (?) if we want to support clean downgrades to an earlier
>> version without recreating the tun/tap interfaces.
>>
>> On Thu, Apr 4, 2019 at 7:18 AM Christopher Schenk <
>> cschenk@mail.uni-paderborn.de> wrote:
>>
>>> Signed-off-by: Christopher Schenk <cschenk@mail.uni-paderborn.de>
>>> ---
>>>   include/openvpn-msg.h         |  8 ++++
>>>   src/openvpn/tun.c             | 89 +++++++++++++++++++++++++++++++++++
>>>   src/openvpnserv/interactive.c | 31 ++++++++++++
>>>   3 files changed, 128 insertions(+)
>>>
>>> diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
>>> index 66177a21..10cd68ac 100644
>>> --- a/include/openvpn-msg.h
>>> +++ b/include/openvpn-msg.h
>>> @@ -39,6 +39,7 @@ typedef enum {
>>>       msg_del_block_dns,
>>>       msg_register_dns,
>>>       msg_enable_dhcp,
>>> +    msg_set_mtu,
>>>   } message_type_t;
>>>
>>>   typedef struct {
>>> @@ -117,4 +118,11 @@ typedef struct {
>>>       interface_t iface;
>>>   } enable_dhcp_message_t;
>>>
>>> +typedef struct {
>>> +    message_header_t header;
>>> +    interface_t iface;
>>> +    short family;
>>> +    int mtu;
>>> +} set_mtu_message_t;
>>> +
>>>   #endif /* ifndef OPENVPN_MSG_H_ */
>>> diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
>>> index 48a8fdf7..3895e421 100644
>>> --- a/src/openvpn/tun.c
>>> +++ b/src/openvpn/tun.c
>>> @@ -69,6 +69,10 @@ static void netsh_ifconfig(const struct 
>>> tuntap_options
>>> *to,
>>>                              const in_addr_t netmask,
>>>                              const unsigned int flags);
>>>
>>> +static void windows_set_mtu(const int iface_index,
>>> +                            const short family,
>>> +                            const int mtu);
>>> +
>>>   static void netsh_set_dns6_servers(const struct in6_addr *addr_list,
>>>                                      const int addr_len,
>>>                                      const char *flex_name);
>>> @@ -201,6 +205,47 @@ out:
>>>       return ret;
>>>   }
>>>
>>> +static bool
>>> +do_set_mtu_service(const struct tuntap *tt, const short family, 
>>> const int
>>> mtu)
>>> +{
>>> +    DWORD len;
>>> +    bool ret = false;
>>> +    ack_message_t ack;
>>> +    struct gc_arena gc = gc_new();
>>> +    HANDLE pipe = tt->options.msg_channel;
>>> +    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
>>> +    set_mtu_message_t mtu_msg = {
>>> +        .header = {
>>> +            msg_set_mtu,
>>> +            sizeof(set_mtu_message_t),
>>> +            0
>>> +        },
>>> +        .iface = {.index = tt->adapter_index,.name = tt->actual_name },
>>> +        .mtu = mtu,
>>> +        .family = family
>>> +    };
>>> +
>>> +    if (!send_msg_iservice(pipe, &mtu_msg, sizeof(mtu_msg), &ack,
>>> "Set_mtu"))
>>> +    {
>>> +        goto out;
>>> +    }
>>> +
>>> +    if (ack.error_number != NO_ERROR)
>>> +    {
>>> +        msg(M_NONFATAL, "TUN: setting %s mtu using service failed: %s
>>> [status=%u if_index=%d]",
>>> +            family_name, strerror_win32(ack.error_number, &gc),
>>> ack.error_number, mtu_msg.iface.index);
>>>
>> +    }
>>> +    else
>>> +    {
>>> +        msg(M_INFO, "%s MTU set to %d on interface %d using service",
>>> family_name, mtu, mtu_msg.iface.index);
>>> +        ret = true;
>>> +    }
>>> +
>>> +out:
>>> +    gc_free(&gc);
>>> +    return ret;
>>> +}
>>> +
>>>   #endif /* ifdef _WIN32 */
>>>
>>>   #ifdef TARGET_SOLARIS
>>> @@ -984,6 +1029,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char
>>> *ifname, int tun_mtu,
>>>       {
>>>           do_address_service(true, AF_INET6, tt);
>>>           do_dns6_service(true, tt);
>>> +        do_set_mtu_service(tt, AF_INET6, tun_mtu);
>>>       }
>>>       else
>>>       {
>>> @@ -1000,6 +1046,7 @@ do_ifconfig_ipv6(struct tuntap *tt, const char
>>> *ifname, int tun_mtu,
>>>           netsh_command(&argv, 4, M_FATAL);
>>>           /* set ipv6 dns servers if any are specified */
>>>           netsh_set_dns6_servers(tt->options.dns6, tt->options.dns6_len,
>>> ifname);
>>> +        windows_set_mtu(tt->adapter_index, AF_INET6, tun_mtu);
>>>       }
>>>
>>>       /* explicit route needed */
>>> @@ -1394,6 +1441,14 @@ do_ifconfig_ipv4(struct tuntap *tt, const char
>>> *ifname, int tun_mtu,
>>>
>>>                   break;
>>>           }
>>> +        if (tt->options.msg_channel)
>>> +        {
>>> +            do_set_mtu_service(tt, AF_INET, tun_mtu);
>>> +        }
>>> +        else
>>> +        {
>>> +            windows_set_mtu(tt->adapter_index, AF_INET, tun_mtu);
>>> +        }
>>>       }
>>>
>>>   #else  /* if defined(TARGET_LINUX) */
>>> @@ -5236,6 +5291,40 @@ out:
>>>       return ret;
>>>   }
>>>
>>> +static void
>>> +windows_set_mtu(const int iface_index, const short family,
>>> +                const int mtu)
>>> +{
>>> +    DWORD err = 0;
>>> +    struct gc_arena gc = gc_new();
>>> +    MIB_IPINTERFACE_ROW ipiface;
>>> +    InitializeIpInterfaceEntry(&ipiface);
>>> +    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
>>> +    ipiface.Family = family;
>>> +    ipiface.InterfaceIndex = iface_index;
>>> +    err = GetIpInterfaceEntry(&ipiface);
>>> +    if (err == NO_ERROR)
>>> +    {
>>> +        if (family == AF_INET)
>>> +        {
>>> +            ipiface.SitePrefixLength = 0;
>>> +        }
>>> +        ipiface.NlMtu = mtu;
>>> +        err = SetIpInterfaceEntry(&ipiface);
>>> +    }
>>> +
>>> +    if (err != NO_ERROR)
>>> +    {
>>> +        msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u
>>> if_index=%d]",
>>> +            family_name, strerror_win32(err, &gc), err, iface_index);
>>> +    }
>>> +    else
>>> +    {
>>> +        msg(M_INFO, "Successfully set %s mtu on interface %d",
>>> family_name, iface_index);
>>> +    }
>>> +}
>>> +
>>> +
>>>   /*
>>>    * Return a TAP name for netsh commands.
>>>    */
>>> diff --git a/src/openvpnserv/interactive.c 
>>> b/src/openvpnserv/interactive.c
>>> index 623c3ff7..c24cb22b 100644
>>> --- a/src/openvpnserv/interactive.c
>>> +++ b/src/openvpnserv/interactive.c
>>> @@ -1198,6 +1198,29 @@ HandleEnableDHCPMessage(const 
>>> enable_dhcp_message_t
>>> *dhcp)
>>>       return err;
>>>   }
>>>
>>> +static DWORD
>>> +HandleMTUMessage(const set_mtu_message_t *mtu)
>>> +{
>>> +    DWORD err = 0;
>>> +    MIB_IPINTERFACE_ROW ipiface;
>>> +    InitializeIpInterfaceEntry(&ipiface);
>>> +    ipiface.Family = mtu->family;
>>> +    ipiface.InterfaceIndex = mtu->iface.index;
>>> +    err = GetIpInterfaceEntry(&ipiface);
>>> +    if (err != NO_ERROR)
>>> +    {
>>> +        return err;
>>> +    }
>>> +    if (mtu->family == AF_INET)
>>> +    {
>>> +        ipiface.SitePrefixLength = 0;
>>> +    }
>>> +    ipiface.NlMtu = mtu->mtu;
>>> +
>>> +    err = SetIpInterfaceEntry(&ipiface);
>>> +    return err;
>>> +}
>>> +
>>>   static VOID
>>>   HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events,
>>> undo_lists_t *lists)
>>>   {
>>> @@ -1210,6 +1233,7 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD 
>>> count,
>>> LPHANDLE events, undo_lists
>>>           block_dns_message_t block_dns;
>>>           dns_cfg_message_t dns;
>>>           enable_dhcp_message_t dhcp;
>>> +        set_mtu_message_t mtu;
>>>       } msg;
>>>       ack_message_t ack = {
>>>           .header = {
>>> @@ -1277,6 +1301,13 @@ HandleMessage(HANDLE pipe, DWORD bytes, DWORD
>>> count, LPHANDLE events, undo_lists
>>>               }
>>>               break;
>>>
>>> +        case msg_set_mtu:
>>> +            if (msg.header.size == sizeof(msg.mtu))
>>> +            {
>>> +                ack.error_number = HandleMTUMessage(&msg.mtu);
>>> +            }
>>> +            break;
>>> +
>>>           default:
>>>               ack.error_number = ERROR_MESSAGE_TYPE;
>>>               MsgToEventLog(MSG_FLAGS_ERROR, TEXT("Unknown message type
>>> %d"), msg.header.type);
>>>
>>>
>> Acked-by: Selva Nair <selva.nair@gmail.com>
>>
>> Selva
>>
> 
> 
> _______________________________________________
> Openvpn-devel mailing list
> Openvpn-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Gert Doering May 18, 2019, 3:43 a.m. UTC | #5
Hi,

On Sat, May 18, 2019 at 02:38:29PM +0200, Christopher Schenk wrote:
> any other thoughts on this patch? Any chance to getting it applied?

It got an ACK, so it's in my work queue :-)  - there is a given chance
that I might find something on my "double check before merging", but
usually when Selva says something is good it is so.

Lack of time, mostly... sorry for that.

gert
Christopher Schenk Nov. 11, 2019, 10:59 p.m. UTC | #6
Hi,

any updates on this? I would really love to see this upstream.

Regards,
Christopher

On 5/18/19 3:43 PM, Gert Doering wrote:
> Hi,
> 
> On Sat, May 18, 2019 at 02:38:29PM +0200, Christopher Schenk wrote:
>> any other thoughts on this patch? Any chance to getting it applied?
> 
> It got an ACK, so it's in my work queue :-)  - there is a given chance
> that I might find something on my "double check before merging", but
> usually when Selva says something is good it is so.
> 
> Lack of time, mostly... sorry for that.
> 
> gert
>
Gert Doering April 15, 2020, 8:27 a.m. UTC | #7
Hi,

On Thu, Apr 04, 2019 at 01:16:56PM +0200, Christopher Schenk wrote:
> Signed-off-by: Christopher Schenk <cschenk@mail.uni-paderborn.de>
> ---
>  include/openvpn-msg.h         |  8 ++++
>  src/openvpn/tun.c             | 89 +++++++++++++++++++++++++++++++++++
>  src/openvpnserv/interactive.c | 31 ++++++++++++
>  3 files changed, 128 insertions(+)

So, we have this nice patch, and I got a very pointed reminder today
that I messed merging it up.

The patch has an ACK, so everything is good, but Lev and Simon whacked
tun.c a lot in between, and someone else added message types, so it does
not apply anymore.

Applying: Set the correct mtu on windows based systems
error: patch failed: include/openvpn-msg.h:39
error: include/openvpn-msg.h: patch does not apply
error: patch failed: src/openvpn/tun.c:984
error: src/openvpn/tun.c: patch does not apply
error: patch failed: src/openvpnserv/interactive.c:1198
error: src/openvpnserv/interactive.c: patch does not apply
Patch failed at 0001 Set the correct mtu on windows based systems


Would you accept an apology for me dragging my feet on this for so long,
and send a rebased v3 to current master?

(And if that does not get traction, feel free to send Max my way on
a weekly basis)

gert

Patch

diff --git a/include/openvpn-msg.h b/include/openvpn-msg.h
index 66177a21..10cd68ac 100644
--- a/include/openvpn-msg.h
+++ b/include/openvpn-msg.h
@@ -39,6 +39,7 @@  typedef enum {
     msg_del_block_dns,
     msg_register_dns,
     msg_enable_dhcp,
+    msg_set_mtu,
 } message_type_t;
 
 typedef struct {
@@ -117,4 +118,11 @@  typedef struct {
     interface_t iface;
 } enable_dhcp_message_t;
 
+typedef struct {
+    message_header_t header;
+    interface_t iface;
+    short family;
+    int mtu;
+} set_mtu_message_t;
+
 #endif /* ifndef OPENVPN_MSG_H_ */
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
index 48a8fdf7..3895e421 100644
--- a/src/openvpn/tun.c
+++ b/src/openvpn/tun.c
@@ -69,6 +69,10 @@  static void netsh_ifconfig(const struct tuntap_options *to,
                            const in_addr_t netmask,
                            const unsigned int flags);
 
+static void windows_set_mtu(const int iface_index,
+                            const short family,
+                            const int mtu);
+
 static void netsh_set_dns6_servers(const struct in6_addr *addr_list,
                                    const int addr_len,
                                    const char *flex_name);
@@ -201,6 +205,47 @@  out:
     return ret;
 }
 
+static bool
+do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu)
+{
+    DWORD len;
+    bool ret = false;
+    ack_message_t ack;
+    struct gc_arena gc = gc_new();
+    HANDLE pipe = tt->options.msg_channel;
+    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
+    set_mtu_message_t mtu_msg = {
+        .header = {
+            msg_set_mtu,
+            sizeof(set_mtu_message_t),
+            0
+        },
+        .iface = {.index = tt->adapter_index,.name = tt->actual_name },
+        .mtu = mtu,
+        .family = family
+    };
+
+    if (!send_msg_iservice(pipe, &mtu_msg, sizeof(mtu_msg), &ack, "Set_mtu"))
+    {
+        goto out;
+    }
+
+    if (ack.error_number != NO_ERROR)
+    {
+        msg(M_NONFATAL, "TUN: setting %s mtu using service failed: %s [status=%u if_index=%d]",
+            family_name, strerror_win32(ack.error_number, &gc), ack.error_number, mtu_msg.iface.index);
+    }
+    else
+    {
+        msg(M_INFO, "%s MTU set to %d on interface %d using service", family_name, mtu, mtu_msg.iface.index);
+        ret = true;
+    }
+
+out:
+    gc_free(&gc);
+    return ret;
+}
+
 #endif /* ifdef _WIN32 */
 
 #ifdef TARGET_SOLARIS
@@ -984,6 +1029,7 @@  do_ifconfig_ipv6(struct tuntap *tt, const char *ifname, int tun_mtu,
     {
         do_address_service(true, AF_INET6, tt);
         do_dns6_service(true, tt);
+        do_set_mtu_service(tt, AF_INET6, tun_mtu);
     }
     else
     {
@@ -1000,6 +1046,7 @@  do_ifconfig_ipv6(struct tuntap *tt, const char *ifname, int tun_mtu,
         netsh_command(&argv, 4, M_FATAL);
         /* set ipv6 dns servers if any are specified */
         netsh_set_dns6_servers(tt->options.dns6, tt->options.dns6_len, ifname);
+        windows_set_mtu(tt->adapter_index, AF_INET6, tun_mtu);
     }
 
     /* explicit route needed */
@@ -1394,6 +1441,14 @@  do_ifconfig_ipv4(struct tuntap *tt, const char *ifname, int tun_mtu,
 
                 break;
         }
+        if (tt->options.msg_channel)
+        {
+            do_set_mtu_service(tt, AF_INET, tun_mtu);
+        }
+        else
+        {
+            windows_set_mtu(tt->adapter_index, AF_INET, tun_mtu);
+        }
     }
 
 #else  /* if defined(TARGET_LINUX) */
@@ -5236,6 +5291,40 @@  out:
     return ret;
 }
 
+static void
+windows_set_mtu(const int iface_index, const short family,
+                const int mtu)
+{
+    DWORD err = 0;
+    struct gc_arena gc = gc_new();
+    MIB_IPINTERFACE_ROW ipiface;
+    InitializeIpInterfaceEntry(&ipiface);
+    const char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";
+    ipiface.Family = family;
+    ipiface.InterfaceIndex = iface_index;
+    err = GetIpInterfaceEntry(&ipiface);
+    if (err == NO_ERROR)
+    {
+        if (family == AF_INET)
+        {
+            ipiface.SitePrefixLength = 0;
+        }
+        ipiface.NlMtu = mtu;
+        err = SetIpInterfaceEntry(&ipiface);
+    }
+
+    if (err != NO_ERROR)
+    {
+        msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u if_index=%d]",
+            family_name, strerror_win32(err, &gc), err, iface_index);
+    }
+    else
+    {
+        msg(M_INFO, "Successfully set %s mtu on interface %d", family_name, iface_index);
+    }
+}
+
+
 /*
  * Return a TAP name for netsh commands.
  */
diff --git a/src/openvpnserv/interactive.c b/src/openvpnserv/interactive.c
index 623c3ff7..c24cb22b 100644
--- a/src/openvpnserv/interactive.c
+++ b/src/openvpnserv/interactive.c
@@ -1198,6 +1198,29 @@  HandleEnableDHCPMessage(const enable_dhcp_message_t *dhcp)
     return err;
 }
 
+static DWORD
+HandleMTUMessage(const set_mtu_message_t *mtu)
+{
+    DWORD err = 0;
+    MIB_IPINTERFACE_ROW ipiface;
+    InitializeIpInterfaceEntry(&ipiface);
+    ipiface.Family = mtu->family;
+    ipiface.InterfaceIndex = mtu->iface.index;
+    err = GetIpInterfaceEntry(&ipiface);
+    if (err != NO_ERROR)
+    {
+        return err;
+    }
+    if (mtu->family == AF_INET)
+    {
+        ipiface.SitePrefixLength = 0;
+    }
+    ipiface.NlMtu = mtu->mtu;
+
+    err = SetIpInterfaceEntry(&ipiface);
+    return err;
+}
+
 static VOID
 HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists_t *lists)
 {
@@ -1210,6 +1233,7 @@  HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists
         block_dns_message_t block_dns;
         dns_cfg_message_t dns;
         enable_dhcp_message_t dhcp;
+        set_mtu_message_t mtu;
     } msg;
     ack_message_t ack = {
         .header = {
@@ -1277,6 +1301,13 @@  HandleMessage(HANDLE pipe, DWORD bytes, DWORD count, LPHANDLE events, undo_lists
             }
             break;
 
+        case msg_set_mtu:
+            if (msg.header.size == sizeof(msg.mtu))
+            {
+                ack.error_number = HandleMTUMessage(&msg.mtu);
+            }
+            break;
+
         default:
             ack.error_number = ERROR_MESSAGE_TYPE;
             MsgToEventLog(MSG_FLAGS_ERROR, TEXT("Unknown message type %d"), msg.header.type);