Re: [Openvpn-devel] [OpenVPN/openvpn-gui] UI showing green connected status despite not beeing able to create a route (#9)
Hi,
On Mon, Jan 2, 2023 at 5:51 PM Gert Doering wrote:
> Hi,
>
> On Sat, Dec 31, 2022 at 05:40:49PM +0100, Gert Doering wrote:
> > On Sat, Dec 17, 2022 at 07:09:34PM -0500, Selva Nair wrote:
> > > tldr: Can we get CONNECTED,ERROR instead of CONNECTED,SUCCESS on route
> > > errors?
> >
> > I think this makes sense. Not sure how complicated it is, and if we
> > can make this before 2.6.0 ("some time in January").
>
> So if I am reading this right, "CONNECTED," is produced
> by management_set_state() -> log_entry_print(), with the "something"
> being transported via "detail" -> "e->string".
>
> "CONNECTED" is only produced by
> init.c::initialization_sequence_completed(),
> and there already is a case for "CONNECTED,ERROR" though I don't understand
> it...
>
> if (flags & ISC_ERRORS)
> {
> detail = "ERROR";
> }
>
> (which will also log "Initialization Sequence Completed *With Errors*",
> but the rest of that branch might be a bit misleading, as it will
> tell systemd "STATUS=Failed")
>
>
> initialization_sequence_completed() is called from init.c::do_up()
> (for client/p2p), forward.c, and mtcp.c/mudp.c (Server).
>
> The "--route-delay" case goes via forward.c::check_add_routes_action(),
> which *can* signal ERROR. init.c::do_up() does not.
>
> Routes are (if no --route-delay) set up by do_init_tun(), which calls
> do_route(). So adding an "&flags" parameter to that call chain which sets
> ISC_ERROR if a route addition fails might be a viable approach.
>
I was also thinking along these lines. I think we can make add_route()
return a status instead of void, and propagate it back to where it's needed
in check_add_route_action(). Many of those functions that return void can
return an int instead. All real worker functions that set a route :
do_route_service, do_route_ipv6_servuce, do_route_ipapi,
openvpn_execve_check, net_route_v4_add, net_route_v6_add etc.
all return true or false or a status, so it's only their callers that need
to pass it on.
This is assuming "false" or error really means a serious error, not
something
trivial like a route already exists. And, there's the rub... I can't see
how to get
around this on all platforms. On Windows when using service or IPAPI we
can check the error code for ERROR_OBJECT_ALREADY_EXISTS, on Linux
using SITNL already ignores EEXIST, so those two cases are good to go.
But what to do with openvpn_execve?
I'm tempted to take a short-cut and implement this only for Windows and
Linux
when using SITNL, and leave others to continue reporting CONNECTED,SUCCESS
on route errors.
That would be easy to do as the only change required is: update netsh calls
used
for ipv6 routes on Windows to use IPAPI (that's an easy change worth doing
anyway).
As for those using route-method = exe on Windows, we may occasionally report
CONNECTED,ERROR even for trivial "route already exists" cases. I think that
is
tolerable as no one should be using route-method=exe. The whole route-method
option should have been deprecated long ago. Same with ip-win32 (just retain
the manual option to please the demons).
> ... and then I found ROUTE_BEFORE_TUN, ROUTE_AFTER_TUN... which adds
> yet another call chain, which is totally irrelevant but would need that
> extra argument as well...
> And, --route-delay, I need to understand better what the code is doing
> there...
>
I haven't looked into this either.
Selva
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [OpenVPN/openvpn-gui] UI showing green connected status despite not beeing able to create a route (#9)
Hi,
On Sat, Dec 31, 2022 at 05:40:49PM +0100, Gert Doering wrote:
> On Sat, Dec 17, 2022 at 07:09:34PM -0500, Selva Nair wrote:
> > tldr: Can we get CONNECTED,ERROR instead of CONNECTED,SUCCESS on route
> > errors?
>
> I think this makes sense. Not sure how complicated it is, and if we
> can make this before 2.6.0 ("some time in January").
So if I am reading this right, "CONNECTED," is produced
by management_set_state() -> log_entry_print(), with the "something"
being transported via "detail" -> "e->string".
"CONNECTED" is only produced by init.c::initialization_sequence_completed(),
and there already is a case for "CONNECTED,ERROR" though I don't understand
it...
if (flags & ISC_ERRORS)
{
detail = "ERROR";
}
(which will also log "Initialization Sequence Completed *With Errors*",
but the rest of that branch might be a bit misleading, as it will
tell systemd "STATUS=Failed")
initialization_sequence_completed() is called from init.c::do_up()
(for client/p2p), forward.c, and mtcp.c/mudp.c (Server).
The "--route-delay" case goes via forward.c::check_add_routes_action(),
which *can* signal ERROR. init.c::do_up() does not.
Routes are (if no --route-delay) set up by do_init_tun(), which calls
do_route(). So adding an "&flags" parameter to that call chain which sets
ISC_ERROR if a route addition fails might be a viable approach.
... and then I found ROUTE_BEFORE_TUN, ROUTE_AFTER_TUN... which adds
yet another call chain, which is totally irrelevant but would need that
extra argument as well...
And, --route-delay, I need to understand better what the code is doing
there...
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [OpenVPN/openvpn-gui] UI showing green connected status despite not beeing able to create a route (#9)
Hi,
On Sat, Dec 17, 2022 at 07:09:34PM -0500, Selva Nair wrote:
> tldr: Can we get CONNECTED,ERROR instead of CONNECTED,SUCCESS on route
> errors?
I think this makes sense. Not sure how complicated it is, and if we
can make this before 2.6.0 ("some time in January").
gert
--
"If was one thing all people took for granted, was conviction that if you
feed honest figures into a computer, honest figures come out. Never doubted
it myself till I met a computer with a sense of humor."
Robert A. Heinlein, The Moon is a Harsh Mistress
Gert Doering - Munich, Germany g...@greenie.muc.de
signature.asc
Description: PGP signature
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [OpenVPN/openvpn-gui] UI showing green connected status despite not beeing able to create a route (#9)
Hi, Trying to resurrect this thread, as we still have this deficiency in OpenVPN core that makes it difficult for UI's to report routing errors. tldr: Can we get CONNECTED,ERROR instead of CONNECTED,SUCCESS on route errors? See also https://github.com/OpenVPN/openvpn-gui/issues/9 On Fri, Jul 6, 2018 at 3:38 PM Jonathan K. Bullard wrote: > Hi, > > On Fri, Jul 6, 2018 at 3:24 PM, Selva Nair wrote: > > > > Hi, > > > > Copying the devel list as a reminder that "we" have been asking for this > change for a long time :) > > > > On Fri, Jul 6, 2018 at 2:48 PM, Gert Doering > wrote: > >> > >> Hi, > >> > >> On Fri, Jul 06, 2018 at 08:25:02AM -0700, Selva Nair wrote: > >> > Can we do something about this in openvpn core, so that the GUI can > be report route errors? > >> > >> Currently when we get CONNECTED,SUCCESS we turn the icon green. If we > get > >> > >> CONENCTED,ERROR we keep it amber and the state as "connecting.." even if > >> > >> the initialization sequence completes. > >> > > >> > So route errors can still be treated as non-fatal but we want to get > CONNECTED,ERROR in such cases. > > +1 to routing errors causing CONNECTED,ERROR (I didn't even know that > was possible). This confuses many Tunnelblick users. > > I'd be happy for routing errors to cause a FATAL error as long as the > errors weren't caused by "route already exists" or something like > that. But I assume that's too much trouble. > > > Best regards, > > Jon > Selva ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [OpenVPN/openvpn-gui] UI showing green connected status despite not beeing able to create a route (#9)
Hi, On Fri, Jul 6, 2018 at 3:24 PM, Selva Nair wrote: > > Hi, > > Copying the devel list as a reminder that "we" have been asking for this > change for a long time :) > > On Fri, Jul 6, 2018 at 2:48 PM, Gert Doering wrote: >> >> Hi, >> >> On Fri, Jul 06, 2018 at 08:25:02AM -0700, Selva Nair wrote: >> > Can we do something about this in openvpn core, so that the GUI can be >> > report route errors? >> >> Currently when we get CONNECTED,SUCCESS we turn the icon green. If we get >> >> CONENCTED,ERROR we keep it amber and the state as "connecting.." even if >> >> the initialization sequence completes. >> > >> > So route errors can still be treated as non-fatal but we want to get >> > CONNECTED,ERROR in such cases. +1 to routing errors causing CONNECTED,ERROR (I didn't even know that was possible). This confuses many Tunnelblick users. I'd be happy for routing errors to cause a FATAL error as long as the errors weren't caused by "route already exists" or something like that. But I assume that's too much trouble. Best regards, Jon -- Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [OpenVPN/openvpn-gui] UI showing green connected status despite not beeing able to create a route (#9)
Hi,
Copying the devel list as a reminder that "we" have been asking for this
change for a long time :)
On Fri, Jul 6, 2018 at 2:48 PM, Gert Doering
wrote:
> Hi,
>
> On Fri, Jul 06, 2018 at 08:25:02AM -0700, Selva Nair wrote:
> > Can we do something about this in openvpn core, so that the GUI can be
> report route errors?
Currently when we get CONNECTED,SUCCESS we turn the icon green. If we get
CONENCTED,ERROR we keep it amber and the state as "connecting.." even if
the initialization sequence completes.
> >
> > So route errors can still be treated as non-fatal but we want to get
> CONNECTED,ERROR in such cases.
>
> I still wonder why IPv4 route addition failures are considered
> non-fatal...
>
As failure in test_routes is considered an error (waiting for TAP to come
up..,
though not fatal) and leads to CONNECTED,ERROR, I guess that
CONNECTED,SUCCESS would imply the TUN/TAP is up and has got its
on-link route to the vpn subnet.
>
> But this aside ("it has always been that way", before I got involved),
> I can have look into how this signalling is done.
To make all route errors propagate to initilaization_sequence_completed()
one needs to return errors back up from a deep call chain:
check_add_routes_dowork -> check_add_routes_action -> do_route->
add_routes->etc.. Each of those calls returns void right now..
Selva
--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
