| Message ID | 20190401112140.13212-4-cschenk@mail.uni-paderborn.de |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [Openvpn-devel,1/4] Setting adapter mtu on windows systems | expand |
Hi, On Mon, Apr 1, 2019 at 7:22 AM Christopher Schenk < cschenk@mail.uni-paderborn.de> wrote: > --- > src/openvpn/tun.c | 58 ++++++++++++----------------------------------- > 1 file changed, 14 insertions(+), 44 deletions(-) > @@ -213,7 +213,7 @@ do_set_mtu_service(const struct tuntap *tt, const short > family, const int mtu) > ack_message_t ack; > struct gc_arena gc = gc_new(); > HANDLE pipe = tt->options.msg_channel; > - > + char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4"; This should be preferably declared as const char *family_name. Also see my response to commit 2 Some general remarks follow: Now we have 4 commits, two for the initial netsh implementation, one replacing that by SetIpInterfaceEntry() and a fixup. We don't want to keep all that irrelevant history, so these have to be squashed into one commit (or two if you wish to separate the interactive service part). And edit the final commit message header and description to reflect the final version. Squashing all into one commit may be easier and that's fine in this case. We also prefer to have patches signed off. Adding -s to "git commit" or doing "git commit -s --amend" will take care of that. Finally, I'm not sure whether we still require patch authors to follow the code formatting style or we automatically run uncrustify during merge. Even if its not mandatory, its a good practice to follow the style in the rest of the code to the extent possible: for example, we use 4 spaces for indent (not 8), no tabs anywhere etc. You can run "uncrustify -c dev-tools/uncrustify.conf" on tun.c, interactive.c and openvpn_msg.h to get most of that rectified. Note that uncrustify may edit unrelated parts of the code -- if it does, do not include such changes in the patch. Thanks Selva <div dir="ltr"><div dir="ltr"> Hi,<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Mon, Apr 1, 2019 at 7:22 AM Christopher Schenk <<a href="mailto:cschenk@mail.uni-paderborn.de" target="_blank">cschenk@mail.uni-paderborn.de</a>> 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">---<br> src/openvpn/tun.c | 58 ++++++++++++-----------------------------------<br> 1 file changed, 14 insertions(+), 44 deletions(-)<br></blockquote><div><br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">@@ -213,7 +213,7 @@ do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu)<br> ack_message_t ack;<br> struct gc_arena gc = gc_new();<br> HANDLE pipe = tt->options.msg_channel;<br> -<br> + char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4";</blockquote></div><div><br>This should be preferably declared as const char *family_name. Also see my<br>response to commit 2<br></div><div><br></div><div>Some general remarks follow:<br><br></div><div>Now we have 4 commits, two for the initial netsh implementation,<br>one replacing that by SetIpInterfaceEntry() and a fixup. We don't want to<br>keep all that irrelevant history, so these have to be squashed into one commit <br>(or two if you wish to separate the interactive service part). <br>And edit the final commit message header and description to reflect the final<br>version. Squashing all into one commit may be easier and that's fine in<br>this case.<br></div><div><br></div><div>We also prefer to have patches signed off. Adding -s to "git commit" or<br>doing "git commit -s --amend" will take care of that.<br><br></div><div>Finally, I'm not sure whether we still require patch authors to follow the<br>code formatting style or we automatically run uncrustify during merge.<br>Even if its not mandatory, its a good practice to follow the style in the rest of<br>the code to the extent possible: for example, we use 4 spaces for indent<br>(not 8), no tabs anywhere etc. You can run <br>"uncrustify -c dev-tools/uncrustify.conf" on tun.c, interactive.c<br>and openvpn_msg.h to get most of that rectified. Note that uncrustify may<br>edit unrelated parts of the code -- if it does, do not include<br>such changes in the patch.<br></div><div><br></div><div>Thanks<br><br></div><div>Selva</div></div></div>
Hi, On Mon, Apr 01, 2019 at 11:59:04AM -0400, Selva Nair wrote: > Finally, I'm not sure whether we still require patch authors to follow the > code formatting style or we automatically run uncrustify during merge. We have decided that all *core* authors are required to either run uncrustify or "get it right on their own" - but Christopher is not "core", so I could do that for him. OTOH, I prefer to commit patches "as they come in on the list", so, no code changes by me, not even formatting... @Christopher: the source tree has "openvpn/dev-tools/uncrustify.conf". If you copy that (on a unixish system) to $HOME/.uncrustify.cfg, you can just call "uncrustify --replace $sourcefile" to have it change the file to our accepted coding standard. Much easier than hanggling on the list later on. gert
diff --git a/src/openvpn/tun.c b/src/openvpn/tun.c index 9fe8444f..20e661a7 100644 --- a/src/openvpn/tun.c +++ b/src/openvpn/tun.c @@ -69,7 +69,7 @@ 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_indey, +static void windows_set_mtu(const int iface_index, short family, const int mtu); @@ -213,7 +213,7 @@ do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu) ack_message_t ack; struct gc_arena gc = gc_new(); HANDLE pipe = tt->options.msg_channel; - + char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4"; set_mtu_message_t mtu_msg = { .header = { msg_set_mtu, @@ -230,31 +230,15 @@ do_set_mtu_service(const struct tuntap *tt, const short family, const int mtu) goto out; } - if (family == AF_INET) + if (ack.error_number != NO_ERROR) { - if (ack.error_number != NO_ERROR) - { - msg(M_NONFATAL, "TUN: setting IPv4 mtu using service failed: %s [status=%u if_index=%d]", - strerror_win32(ack.error_number, &gc), ack.error_number, mtu_msg.iface.index); - } - else - { - msg(M_INFO, "IPv4 MTU set to %d on interface %d using service", mtu, mtu_msg.iface.index); - ret = true; - } + 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 if (family == AF_INET6) + else { - if (ack.error_number != NO_ERROR) - { - msg(M_NONFATAL, "TUN: setting IPv6 mtu using service failed: %s [status=%u if_index=%d]", - strerror_win32(ack.error_number, &gc), ack.error_number, mtu_msg.iface.index); - } - else - { - msg(M_INFO, "IPv6 MTU set to %d on interface %d using service", mtu, mtu_msg.iface.index); - ret = true; - } + msg(M_INFO, "%s MTU set to %d on interface %d using service", family_name, mtu, mtu_msg.iface.index); + ret = true; } out: @@ -5313,35 +5297,21 @@ windows_set_mtu(const int iface_index, const short family, DWORD err = 0; struct gc_arena gc = gc_new(); MIB_IPINTERFACE_ROW row; + char *family_name = (family == AF_INET6) ? "IPv6" : "IPv4"; InitializeIpInterfaceEntry(&row); row.Family = family; row.InterfaceIndex = iface_index; row.NlMtu = mtu; err = SetIpInterfaceEntry(&row); - if (family == AF_INET) + if (err != NO_ERROR) { - if (err != NO_ERROR) - { - msg(M_WARN, "TUN: Setting IPv4 mtu failed: %s [status=%u if_index=%d]", - strerror_win32(err, &gc), err, iface_index); - } - else - { - msg(M_INFO, "Successfully set IPv4 mtu on interface %d", iface_index); - } + msg(M_WARN, "TUN: Setting %s mtu failed: %s [status=%u if_index=%d]", + family_name, strerror_win32(err, &gc), err, iface_index); } - else if (family == AF_INET6) + else { - if (err != NO_ERROR) - { - msg(M_WARN, "TUN: Setting IPv6 mtu failed: %s [status=%u if_index=%d]", - strerror_win32(err, &gc), err, iface_index); - } - else - { - msg(M_INFO, "Successfully set IPv6 mtu on interface %d", iface_index); - } + msg(M_INFO, "Successfully set %s mtu on interface %d", family_name, iface_index); } }