Message ID | 1521522224-18829-2-git-send-email-selva.nair@gmail.com |
---|---|
State | Superseded |
Headers | show |
Series | [Openvpn-devel,1/2] Persist management-query-remote and proxy prompts | expand |
Ref: https://patchwork.openvpn.net/project/openvpn2/list/?series=201 Hi, These patches were meant to help implement choosing the remote through the GUI. I may not find time for that but the patches by themselves are still relevant. If there is some interest I'll rebase to master. Selva
Hi, On Sun, Jun 09, 2019 at 03:33:55PM -0400, Selva Nair wrote: > Ref: https://patchwork.openvpn.net/project/openvpn2/list/?series=201 > > These patches were meant to help implement choosing the remote through > the GUI. I may not find time for that but the patches by themselves > are still relevant. > > If there is some interest I'll rebase to master. I'm working my way through the patch queue these days, and now I'm at this one :-) Can you elaborate a bit how this would work, and how much work on the GUI side would be needed? (And, yes, a rebased patch :) ). gert
Hi, On Wed, May 13, 2020 at 12:36 PM Gert Doering <gert@greenie.muc.de> wrote: > > Hi, > > On Sun, Jun 09, 2019 at 03:33:55PM -0400, Selva Nair wrote: > > Ref: https://patchwork.openvpn.net/project/openvpn2/list/?series=201 > > > > These patches were meant to help implement choosing the remote through > > the GUI. I may not find time for that but the patches by themselves > > are still relevant. > > > > If there is some interest I'll rebase to master. > > I'm working my way through the patch queue these days, and now I'm > at this one :-) > > Can you elaborate a bit how this would work, and how much work on the > GUI side would be needed? (And, yes, a rebased patch :) ). From what I can recall... Two points to note: (i) With multiple remotes, openvpn exits if no successful connection could be made after two cycles through all remotes (undocumented?) . (ii) When --management-query-remote is used, the core presents one remote at a time and the user has to make a choice to skip, accept or replace without knowing which remotes are available. Now, for a user-friendly implementation of selecting the remote from a GUI dialog, the plan is to silently cycle through all remotes, make a list and then allow the user to make a selection from the list. This will be aided by having a safe way to cycle through all remotes multiple times without the core exiting --- arguably, one cycle is enough to make a list and the list building is complete when the second cycle starts. But it would be much easier to do this without having to worry about the core exiting unexpectedly. The GUI knows how to restart or terminate the core exit if need be. The behaviour is unchanged if management-query-remote is not in use. The patch also changes the way failed connections are counted: skipped remotes should not be counted as failed as that count is used in the back-off logic. Selva
Am 13.05.20 um 19:44 schrieb Selva Nair: > Hi, > > On Wed, May 13, 2020 at 12:36 PM Gert Doering <gert@greenie.muc.de> wrote: >> >> Hi, >> >> On Sun, Jun 09, 2019 at 03:33:55PM -0400, Selva Nair wrote: >>> Ref: https://patchwork.openvpn.net/project/openvpn2/list/?series=201 >>> >>> These patches were meant to help implement choosing the remote through >>> the GUI. I may not find time for that but the patches by themselves >>> are still relevant. >>> >>> If there is some interest I'll rebase to master. >> >> I'm working my way through the patch queue these days, and now I'm >> at this one :-) >> >> Can you elaborate a bit how this would work, and how much work on the >> GUI side would be needed? (And, yes, a rebased patch :) ). > > From what I can recall... > > Two points to note: > > (i) With multiple remotes, openvpn exits if no successful connection > could be made after two cycles through all remotes (undocumented?) . > (ii) When --management-query-remote is used, the core presents one > remote at a time and the user has to make a choice to skip, accept or > replace without knowing which remotes are available. > > Now, for a user-friendly implementation of selecting the remote from a > GUI dialog, the plan is to silently cycle through all remotes, make a > list and then allow the user to make a selection from the list. This > will be aided by having a safe way to cycle through all remotes > multiple times without the core exiting --- arguably, one cycle is > enough to make a list and the list building is complete when the > second cycle starts. But it would be much easier to do this without > having to worry about the core exiting unexpectedly. The GUI knows how > to restart or terminate the core exit if need be. > > The behaviour is unchanged if management-query-remote is not in use. I do not like this change. If you want to list all remotes adding an option to the management interface that list all remotes is a better idea than to piggyback on skipping and implmenenting this as hack to list them all. Something like like-remotes. Also I want skip to count a failure since a UI might have some knowlege that certain remotes cannot work and skipping therefore should also count as "have tried this remote". Arne
diff --git a/src/openvpn/init.c b/src/openvpn/init.c index 413563a..c63dc67 100644 --- a/src/openvpn/init.c +++ b/src/openvpn/init.c @@ -468,14 +468,6 @@ next_connection_entry(struct context *c) c->c1.link_socket_addr.remote_list; } - /* - * Increase the number of connection attempts - * If this is connect-retry-max * size(l) - * OpenVPN will quit - */ - - c->options.unsuccessful_attempts++; - if (++l->current >= l->len) { @@ -501,6 +493,9 @@ next_connection_entry(struct context *c) { /* allow management interface to override connection entry details */ ce_defined = ce_management_query_remote(c); + + /* ignore cycles when management-query-remote is in use */ + n_cycles = 0; if (IS_SIG(c)) { break; @@ -517,6 +512,7 @@ next_connection_entry(struct context *c) #endif } while (!ce_defined); + c->options.unsuccessful_attempts++; /* Check if this connection attempt would bring us over the limit */ if (c->options.connect_retry_max > 0 && c->options.unsuccessful_attempts > (l->len * c->options.connect_retry_max))