Re: [Openvpn-devel] [PATCH v2 5/5] Improve signal handling using POSIX sigaction
On Sun, Jan 8, 2023 at 2:27 PM Gert Doering wrote: > Hi, > > On Mon, Jan 02, 2023 at 02:43:22PM -0500, selva.n...@gmail.com wrote: > > From: Selva Nair > > > > Currently we use the old signal API which follows system-V or > > BSD semantics depending on the platform and/or feature-set macros. > > Further, signal has many weaknesses which makes proper masking > > (blocking) of signals during update not possible. > > Just for the record - as discussed before, this is considered too > intrusive ("needs more testing") for 2.6.0, and can go in early in > the 2.7 phase. > if no one has started reviewing 5/5, I would like to split this into two 1. signal -> sigaction affecting sig.c only 2. changes to the way signal reset is done #2 involves some decisions, affects the rest of the code, and may elicit more comments. Otherwise, will wait and see. Selva ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Cleanup route error and debug logging on Windows
Lots of good unifications, thanks, and thanks for sending the v2 quickly. I have not tested beyond "push at github", but Lev has tested, and I have stared at the changes and liked what I saw :-) - especially the interface ID change fixes "that bug" that I happened to hit when testing the v6-route-via-IPAPI patch. Thanks! (But there is always more work) Your patch has been applied to the master and release/2.6 branch. commit a45c201e2edcf5ccb6baea0a03145023db7f222b (master) commit 638082f3d92dcfd4211a213073de08a0b69fb61d (release/2.6) Author: Selva Nair Date: Fri Jan 20 04:41:00 2023 -0500 Cleanup route error and debug logging on Windows Signed-off-by: Selva Nair Acked-by: Lev Stipakov Message-Id: <20230120094100.2063883-1-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26058.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH applied] Re: Warn when pkcs11-id or pkcs11-id-management options are ignored
As I do not have a working pkcs11-anything environment (still :( ), I have not tested this beyond "looks reasonable" and "compiles on github". Your patch has been applied to the master and release/2.6 branch. commit abad04fc8ef6c1da7dc4e976bacee9f34931adea (master) commit f42b38a68e7a9b44153021a7c3864618e3864cae (release/2.6) Author: Selva Nair Date: Thu Jan 19 21:18:41 2023 -0500 Warn when pkcs11-id or pkcs11-id-management options are ignored Signed-off-by: Selva Nair Acked-by: Frank Lichtenheld Message-Id: <20230120021841.2048791-1-selva.n...@gmail.com> URL: https://www.mail-archive.com/openvpn-devel@lists.sourceforge.net/msg26056.html Signed-off-by: Gert Doering -- kind regards, Gert Doering ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] (no subject)
___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Warn when pkcs11-id or pkcs11-id-management options are ignored
On Thu, Jan 19, 2023 at 09:18:41PM -0500, selva.n...@gmail.com wrote: > From: Selva Nair > > - If there are no pkcs11-providers either directly specified or > through p11-kit-proxy made available through a build-time detection, > these options are ignored. Log a warning in such cases. > > Especially important on Windows where automatic loading of p11-kit > is not enabled in our release builds. > > - Document this behaviour. > Acked-By: Frank Lichtenheld LGTM. I was looking at the warning messages whether they are consistent with other similar messages. But since there is no consistency at all, that is impossible. Regards, -- Frank Lichtenheld ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH v2 2/2] Cleanup route error and debug logging on Windows
Hi, Stared at the code and slightly tested - works as expected. Nice that we display interface id: add_route_ipv6(2000::/3 -> 2001:db8:0:456::1 metric 200) IF 74 instead of device name: add_route_ipv6(2000::/3 -> 2001:db8:0:456::1 metric 200) dev OpenVPN Data Channel Offload Another thing which would be good to improve is a consistency between IPv4 and IPv6 log messages: 2023-01-20 11:59:56 us=265000 C:\Windows\system32\route.exe ADD 128.0.0.0 MASK 128.0.0.0 10.58.0.1 2023-01-20 11:59:56 us=281000 ROUTE: CreateIpForwardEntry succeeded with dwForwardMetric1=25 and dwForwardType=4 2023-01-20 11:59:56 us=281000 Route addition via ipapi [adaptive] succeeded 2023-01-20 11:59:56 us=281000 add_route_ipv6(2000::/3 -> 2001:db8:0:456::1 metric 200) IF 74 2023-01-20 11:59:56 us=281000 IPv6 route added using ipapi - for IPv4 we print route.exe command line (for debugging I think?, since we use IPAPI), and we don't do it for IPv6. - for IPv4 we print "Route addition via ipapi [adaptive] succeeded", for IPv6 "IPv6 route added using ipapi" Besides that, we use capitalized IPAPI spelling in delete_route. That function could probably benefit from the same refactoring as add_route. Anyway, this is not really about this patch. Acked-by: Lev Stipakov ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v2 2/2] Cleanup route error and debug logging on Windows
From: Selva Nair Use a unified logging format for various route-methods - Route add/delete errors are always logged with M_WARN, so log only additional information (succeed/exists) with D_ROUTE. - Non-windows platforms log route errors with a prefix "ERROR:" and debug info with "ROUTE:". Do the same on Windows. Do not log errors or success multiple times. - In add_route_ipv6, log the interface id instead of device name as the latter always point to the tun/tap adapter name on Windows. Log lines prefixed with a PACKAGE_NAME "ROUTE" are unchanged. They appear to use the same format on all platforms. v2: rebase to master Signed-off-by: Selva Nair --- src/openvpn/route.c | 66 +++-- 1 file changed, 46 insertions(+), 20 deletions(-) diff --git a/src/openvpn/route.c b/src/openvpn/route.c index 61a40ff1..8f54eb50 100644 --- a/src/openvpn/route.c +++ b/src/openvpn/route.c @@ -1641,17 +1641,15 @@ add_route(struct route_ipv4 *r, argv_msg(D_ROUTE, ); +const char *method = "service"; if ((flags & ROUTE_METHOD_MASK) == ROUTE_METHOD_SERVICE) { status = add_route_service(r, tt); -msg(D_ROUTE, "Route addition via service %s", -(status == RTA_SUCCESS) ? "succeeded" : "failed"); } else if ((flags & ROUTE_METHOD_MASK) == ROUTE_METHOD_IPAPI) { status = add_route_ipapi(r, tt, ai); -msg(D_ROUTE, "Route addition via IPAPI %s", -(status == RTA_SUCCESS) ? "succeeded" : "failed"); +method = "ipapi"; } else if ((flags & ROUTE_METHOD_MASK) == ROUTE_METHOD_EXE) { @@ -1660,12 +1658,12 @@ add_route(struct route_ipv4 *r, "ERROR: Windows route add command failed"); status = ret ? RTA_SUCCESS : RTA_ERROR; netcmd_semaphore_release(); +method = "route.exe"; } else if ((flags & ROUTE_METHOD_MASK) == ROUTE_METHOD_ADAPTIVE) { status = add_route_ipapi(r, tt, ai); -msg(D_ROUTE, "Route addition via IPAPI %s [adaptive]", -(status == RTA_SUCCESS) ? "succeeded" : "failed"); +method = "ipapi [adaptive]"; if (status == RTA_ERROR) { msg(D_ROUTE, "Route addition fallback to route.exe"); @@ -1674,12 +1672,18 @@ add_route(struct route_ipv4 *r, "ERROR: Windows route add command failed [adaptive]"); status = ret ? RTA_SUCCESS : RTA_ERROR; netcmd_semaphore_release(); +method = "route.exe"; } } else { ASSERT(0); } +if (status != RTA_ERROR) /* error is logged upstream */ +{ +msg(D_ROUTE, "Route addition via %s %s", method, +(status == RTA_SUCCESS) ? "succeeded" : "failed because route exists"); +} } #elif defined (TARGET_SOLARIS) @@ -1881,7 +1885,6 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, openvpn_net_ctx_t *ctx) { int status = 0; -const char *device = tt->actual_name; bool gateway_needed = false; if (!(r6->flags & RT_DEFINED) ) @@ -1893,6 +1896,7 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, struct gc_arena gc = gc_new(); #ifndef _WIN32 +const char *device = tt->actual_name; if (r6->iface != NULL) /* vpn server special route */ { device = r6->iface; @@ -1925,8 +1929,14 @@ add_route_ipv6(struct route_ipv6 *r6, const struct tuntap *tt, } #endif +#ifndef _WIN32 msg( M_INFO, "add_route_ipv6(%s/%d -> %s metric %d) dev %s", network, r6->netbits, gateway, r6->metric, device ); +#else +msg( M_INFO, "add_route_ipv6(%s/%d -> %s metric %d) IF %lu", + network, r6->netbits, gateway, r6->metric, + r6->adapter_index ? r6->adapter_index : tt->adapter_index); +#endif /* * Filter out routes which are essentially no-ops @@ -2871,14 +2881,16 @@ add_route_ipapi(const struct route_ipv4 *r, const struct tuntap *tt, DWORD adapt doublebreak: if (status != NO_ERROR) { -msg(M_WARN, "ROUTE: route addition failed using CreateIpForwardEntry: %s [status=%u if_index=%u]", -strerror_win32(status, ), -(unsigned int)status, -(unsigned int)if_index); if (status == ERROR_OBJECT_ALREADY_EXISTS) { ret = RTA_EEXIST; } +else +{ +msg(M_WARN, "ERROR: route addition failed using CreateIpForwardEntry: " +"%s [status=%u if_index=%u]", strerror_win32(status, ), +(unsigned int)status,
Re: [Openvpn-devel] [PATCH 2/2] Cleanup route error and debug logging on Windows
Hi, On Fri, Jan 13, 2023 at 06:57:54PM -0500, selva.n...@gmail.com wrote: > Use a unified logging format for various route-methods This generally looks good, but I could't get Lev to test it, because the patch does not apply anymore - v2 of the RTA_ macro patch brought "bool ret = ..." change which conflicts with this one. Can you rebase, and resend? thanks, 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