Hi
On Thu, Mar 28, 2019 at 9:13 AM Arne Schwabe <a...@rfc2549.org> wrote:
> Am 28.03.19 um 13:27 schrieb Christopher Schenk:
> > From: Christopher Schenk <christopher.sch...@hotmail.com>
> >
> > ---
> > src/openvpn/tun.c | 39 ++++++++++++++++++++++++++++++++++++++-
> > 1 file changed, 38 insertions(+), 1 deletion(-)
> >
> > diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c
> > index 48a8fdf7..93d028c8 100644
> > --- a/src/openvpn/tun.c
> > +++ b/src/openvpn/tun.c
> > @@ -69,6 +69,12 @@ static void netsh_ifconfig(const struct
> tuntap_options *to,
> > const in_addr_t netmask,
> > const unsigned int flags);
> >
> > +static void netsh_set_mtu_ipv4(const char *flex_name,
> > + const int mtu);
> > +
> > +static void netsh_set_mtu_ipv6(const char *flex_name,
> > + const int mtu);
> > +
> > static void netsh_set_dns6_servers(const struct in6_addr *addr_list,
> > const int addr_len,
> > const char *flex_name);
> > @@ -1000,6 +1006,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);
> > + netsh_set_mtu_ipv6(ifname, tun_mtu);
> > }
> >
> > /* explicit route needed */
> > @@ -1391,9 +1398,9 @@ do_ifconfig_ipv4(struct tuntap *tt, const char
> *ifname, int tun_mtu,
> > case IPW32_SET_NETSH:
> > netsh_ifconfig(&tt->options, ifname, tt->local,
> > tt->adapter_netmask,
> NI_IP_NETMASK|NI_OPTIONS);
> > -
> > break;
> > }
> > + netsh_set_mtu_ipv4(ifname, tun_mtu);
> > }
> >
> > #else /* if defined(TARGET_LINUX) */
> > @@ -5236,6 +5243,36 @@ out:
> > return ret;
> > }
> >
> > +static void
> > +netsh_set_mtu_ipv4(const char *flex_name,
> > + const int mtu)
> > +{
> > + struct argv argv = argv_new();
> > + argv_printf(&argv, "%s%sc interface ipv4 set subinterface %s mtu =
> %d store=active",
>
> The spacing here looks a bit odd. I would have expected mtu=%d instead.
> Is this necessary or was there a reason for doing this?
>
> Patch looks okay enough to ACK but:
>
> In general, this patch adds a missing feature (setting MTU) with one
> windows interface only (netsh). And more commonly used interface
> (interactive service)would be different then leading to harder to debug
> problems.
>
I would go a step further to say we should not add new features that
do not work when started using the interactive service.
Secondly, we should avoid the old style use of netsh and instead use the
iphelper API as far as possible. It should be possible to
set MTU using the SetIfEntry() or SetIpInterfaceEnrty() function -- I
haven't
checked which one is appropriate here.
Selva
_______________________________________________
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
