|
From: stipa (C. Review) <ge...@op...> - 2025-07-31 09:16:26
|
Attention is currently required from: flichtenheld, plaisthos.
Hello plaisthos, flichtenheld,
I'd like you to do a code review.
Please visit
http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email
to review the following change.
Change subject: Fix --dns options for TAP adapter
......................................................................
Fix --dns options for TAP adapter
Commit
2dfc4f ("dns: deal with --dhcp-options when --dns is active")
has accidentally removed setting of the DHCP_OPTIONS_DHCP_OPTIONAL
flag when copying --dns options. This flag is required to apply options
via DHCP string, which we do for TAP adapter. As a result, --dns options
stopped working for TAP.
Fix by setting this flag when copying --dns options to tuntap_options.
Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89
Signed-off-by: Lev Stipakov <le...@op...>
---
M src/openvpn/options.c
1 file changed, 3 insertions(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/20/1120/1
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 53be6f5..fde3e73 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3579,6 +3579,9 @@
{
msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
}
+#if defined(_WIN32)
+ tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
+#endif
return;
}
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89
Gerrit-Change-Number: 1120
Gerrit-PatchSet: 1
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-MessageType: newchange
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-07-31 09:30:18
|
Attention is currently required from: flichtenheld, plaisthos, stipa. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email ) Change subject: Fix --dns options for TAP adapter ...................................................................... Patch Set 1: Code-Review-1 (1 comment) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/1120/comment/d02b145c_6d172850 : PS1, Line 3582: #if defined(_WIN32) As this is inside an `#if defined(_WIN32) || defined(TARGET_ANDROID)` block and `dhcp_options` is part of the tuntap_options for both platforms, I think we can avoid that `#ifdef` -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89 Gerrit-Change-Number: 1120 Gerrit-PatchSet: 1 Gerrit-Owner: stipa <lst...@gm...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Attention: stipa <lst...@gm...> Gerrit-Comment-Date: Thu, 31 Jul 2025 09:29:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: stipa (C. Review) <ge...@op...> - 2025-07-31 09:48:31
|
Attention is currently required from: cron2, flichtenheld, plaisthos, stipa.
Hello cron2, flichtenheld, plaisthos,
I'd like you to reexamine a change. Please visit
http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email
to look at the new patch set (#2).
The following approvals got outdated and were removed:
Code-Review-1 by cron2
Change subject: Fix --dns options for TAP adapter
......................................................................
Fix --dns options for TAP adapter
Commit
2dfc4f ("dns: deal with --dhcp-options when --dns is active")
has accidentally removed setting of the DHCP_OPTIONS_DHCP_OPTIONAL
flag when copying --dns options. This flag is required to apply options
via DHCP string, which we do for TAP adapter. As a result, --dns options
stopped working for TAP.
Fix by setting this flag when copying --dns options to tuntap_options.
Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89
Signed-off-by: Lev Stipakov <le...@op...>
---
M src/openvpn/options.c
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/20/1120/2
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 53be6f5..3753810 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3579,6 +3579,7 @@
{
msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
}
+ tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
return;
}
}
--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89
Gerrit-Change-Number: 1120
Gerrit-PatchSet: 2
Gerrit-Owner: stipa <lst...@gm...>
Gerrit-Reviewer: cron2 <ge...@gr...>
Gerrit-Reviewer: flichtenheld <fr...@li...>
Gerrit-Reviewer: plaisthos <arn...@rf...>
Gerrit-CC: openvpn-devel <ope...@li...>
Gerrit-Attention: plaisthos <arn...@rf...>
Gerrit-Attention: cron2 <ge...@gr...>
Gerrit-Attention: flichtenheld <fr...@li...>
Gerrit-Attention: stipa <lst...@gm...>
Gerrit-MessageType: newpatchset
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-07-31 10:48:26
|
Attention is currently required from: flichtenheld, plaisthos, stipa. cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email ) Change subject: Fix --dns options for TAP adapter ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89 Gerrit-Change-Number: 1120 Gerrit-PatchSet: 2 Gerrit-Owner: stipa <lst...@gm...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Attention: plaisthos <arn...@rf...> Gerrit-Attention: flichtenheld <fr...@li...> Gerrit-Attention: stipa <lst...@gm...> Gerrit-Comment-Date: Thu, 31 Jul 2025 10:48:12 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment |
|
From: Gert D. <ge...@gr...> - 2025-07-31 10:48:42
|
From: Lev Stipakov <le...@op...>
Commit
2dfc4f ("dns: deal with --dhcp-options when --dns is active")
has accidentally removed setting of the DHCP_OPTIONS_DHCP_OPTIONAL
flag when copying --dns options. This flag is required to apply options
via DHCP string, which we do for TAP adapter. As a result, --dns options
stopped working for TAP.
Fix by setting this flag when copying --dns options to tuntap_options.
Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
---
This change was reviewed on Gerrit and approved by at least one
developer. I request to merge it to master.
Gerrit URL: https://gerrit.openvpn.net/c/openvpn/+/1120
This mail reflects revision 2 of this Change.
Acked-by according to Gerrit (reflected above):
Gert Doering <ge...@gr...>
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 53be6f5..3753810 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -3579,6 +3579,7 @@
{
msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP");
}
+ tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL;
return;
}
}
|
|
From: Gert D. <ge...@gr...> - 2025-07-31 10:55:41
|
I have stared at the code, and together with the findings from the
previous commit (040e0d1c) this very much looks like the right fix.
I haven't tested it myself, just let the BBs compile-test on all
platforms (especially win32 <-> android, given the #ifdefs in the
neighbourhood)
Your patch has been applied to the master branch.
commit 264fe91660fb1a7a426161a9a042ed3f9bd0a7a6
Author: Lev Stipakov
Date: Thu Jul 31 12:48:27 2025 +0200
Fix --dns options for TAP adapter
Signed-off-by: Lev Stipakov <le...@op...>
Acked-by: Gert Doering <ge...@gr...>
Message-Id: <202...@gr...>
URL: https://www.mail-archive.com/ope...@li.../msg32436.html
Signed-off-by: Gert Doering <ge...@gr...>
--
kind regards,
Gert Doering
|
|
From: cron2 (C. Review) <ge...@op...> - 2025-07-31 10:55:48
|
cron2 has uploaded a new patch set (#3) to the change originally created by stipa. ( http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email ) The following approvals got outdated and were removed: Code-Review+2 by cron2 Change subject: Fix --dns options for TAP adapter ...................................................................... Fix --dns options for TAP adapter Commit 2dfc4f ("dns: deal with --dhcp-options when --dns is active") has accidentally removed setting of the DHCP_OPTIONS_DHCP_OPTIONAL flag when copying --dns options. This flag is required to apply options via DHCP string, which we do for TAP adapter. As a result, --dns options stopped working for TAP. Fix by setting this flag when copying --dns options to tuntap_options. Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89 Signed-off-by: Lev Stipakov <le...@op...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg32436.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/options.c 1 file changed, 1 insertion(+), 0 deletions(-) git pull ssh://gerrit.openvpn.net:29418/openvpn refs/changes/20/1120/3 diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 53be6f5..3753810 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3579,6 +3579,7 @@ { msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP"); } + tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL; return; } } -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89 Gerrit-Change-Number: 1120 Gerrit-PatchSet: 3 Gerrit-Owner: stipa <lst...@gm...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: newpatchset |
|
From: cron2 (C. Review) <ge...@op...> - 2025-07-31 10:55:54
|
cron2 has submitted this change. ( http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email ) Change subject: Fix --dns options for TAP adapter ...................................................................... Fix --dns options for TAP adapter Commit 2dfc4f ("dns: deal with --dhcp-options when --dns is active") has accidentally removed setting of the DHCP_OPTIONS_DHCP_OPTIONAL flag when copying --dns options. This flag is required to apply options via DHCP string, which we do for TAP adapter. As a result, --dns options stopped working for TAP. Fix by setting this flag when copying --dns options to tuntap_options. Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89 Signed-off-by: Lev Stipakov <le...@op...> Acked-by: Gert Doering <ge...@gr...> Message-Id: <202...@gr...> URL: https://www.mail-archive.com/ope...@li.../msg32436.html Signed-off-by: Gert Doering <ge...@gr...> --- M src/openvpn/options.c 1 file changed, 1 insertion(+), 0 deletions(-) diff --git a/src/openvpn/options.c b/src/openvpn/options.c index 53be6f5..3753810 100644 --- a/src/openvpn/options.c +++ b/src/openvpn/options.c @@ -3579,6 +3579,7 @@ { msg(M_WARN, "WARNING: couldn't copy all --dns server addresses to TUN/TAP"); } + tt->dhcp_options |= DHCP_OPTIONS_DHCP_OPTIONAL; return; } } -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89 Gerrit-Change-Number: 1120 Gerrit-PatchSet: 3 Gerrit-Owner: stipa <lst...@gm...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-MessageType: merged |
|
From: stipa (C. Review) <ge...@op...> - 2025-07-31 12:42:51
|
stipa has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email ) Change subject: Fix --dns options for TAP adapter ...................................................................... Patch Set 3: (1 comment) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/1120/comment/b766abff_4d3f8bdb : PS1, Line 3582: #if defined(_WIN32) > As this is inside an `#if defined(_WIN32) || defined(TARGET_ANDROID)` block and `dhcp_options` is pa […] Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/1120?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: Id95cd14095a03afb3140a03ae96e9f5679e4fe89 Gerrit-Change-Number: 1120 Gerrit-PatchSet: 3 Gerrit-Owner: stipa <lst...@gm...> Gerrit-Reviewer: cron2 <ge...@gr...> Gerrit-Reviewer: flichtenheld <fr...@li...> Gerrit-Reviewer: plaisthos <arn...@rf...> Gerrit-CC: openvpn-devel <ope...@li...> Gerrit-Comment-Date: Thu, 31 Jul 2025 12:42:38 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: cron2 <ge...@gr...> Gerrit-MessageType: comment |