[Openvpn-devel] [PATCH v4 2/7] pool: allow to configure an IPv6-only ifconfig-pool
From: Antonio Quartulli
With this change a server is allowed to allocate an
IPv6-only pool. This is required to make it capable
of managing an IPv6-only tunnel.
Trac: #208
Signed-off-by: Antonio Quartulli
---
Changes from v3:
- properly compute pool size taking into account the actual base address
src/openvpn/multi.c | 10 ++-
src/openvpn/pool.c | 181
src/openvpn/pool.h | 8 +-
3 files changed, 146 insertions(+), 53 deletions(-)
diff --git a/src/openvpn/multi.c b/src/openvpn/multi.c
index 7f61350d..2fbbe9ec 100644
--- a/src/openvpn/multi.c
+++ b/src/openvpn/multi.c
@@ -388,7 +388,8 @@ multi_init(struct multi_context *m, struct context *t, bool
tcp_mode, int thread
* differently based on whether a tun or tap style
* tunnel.
*/
-if (t->options.ifconfig_pool_defined)
+if (t->options.ifconfig_pool_defined
+|| t->options.ifconfig_ipv6_pool_defined)
{
int pool_type = IFCONFIG_POOL_INDIV;
@@ -397,7 +398,8 @@ multi_init(struct multi_context *m, struct context *t, bool
tcp_mode, int thread
pool_type = IFCONFIG_POOL_30NET;
}
-m->ifconfig_pool = ifconfig_pool_init(pool_type,
+m->ifconfig_pool = ifconfig_pool_init(t->options.ifconfig_pool_defined,
+ pool_type,
t->options.ifconfig_pool_start,
t->options.ifconfig_pool_end,
t->options.duplicate_cn,
@@ -1495,7 +1497,9 @@ multi_select_virtual_addr(struct multi_context *m, struct
multi_instance *mi)
const int tunnel_topology = TUNNEL_TOPOLOGY(mi->context.c1.tuntap);
msg( M_INFO, "MULTI_sva: pool returned IPv4=%s, IPv6=%s",
- print_in_addr_t( remote, 0, ),
+ (mi->context.options.ifconfig_pool_defined
+ ? print_in_addr_t(remote, 0, )
+ : "(Not enabled)"),
(mi->context.options.ifconfig_ipv6_pool_defined
? print_in6_addr( remote_ipv6, 0, )
: "(Not enabled)") );
diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
index 6dd72bb9..48f3ec95 100644
--- a/src/openvpn/pool.c
+++ b/src/openvpn/pool.c
@@ -58,6 +58,29 @@ ifconfig_pool_entry_free(struct ifconfig_pool_entry *ipe,
bool hard)
}
}
+static const int
+ifconfig_pool_size(const struct ifconfig_pool *pool)
+{
+int min = INT_MAX;
+
+if (!pool->ipv4.enabled && !pool->ipv6.enabled)
+{
+return 0;
+}
+
+if (pool->ipv4.enabled)
+{
+min = pool->ipv4.size;
+}
+
+if (pool->ipv6.enabled && pool->ipv6.size < min)
+{
+min = pool->ipv6.size;
+}
+
+return min;
+}
+
static int
ifconfig_pool_find(struct ifconfig_pool *pool, const char *common_name)
{
@@ -65,8 +88,9 @@ ifconfig_pool_find(struct ifconfig_pool *pool, const char
*common_name)
time_t earliest_release = 0;
int previous_usage = -1;
int new_usage = -1;
+int pool_size = ifconfig_pool_size(pool);
-for (i = 0; i < pool->ipv4.size; ++i)
+for (i = 0; i < pool_size; ++i)
{
struct ifconfig_pool_entry *ipe = >list[i];
if (!ipe->in_use)
@@ -147,34 +171,43 @@ ifconfig_pool_verify_range(const int msglevel, const
in_addr_t start, const in_a
}
struct ifconfig_pool *
-ifconfig_pool_init(enum pool_type type, in_addr_t start, in_addr_t end,
- const bool duplicate_cn,
+ifconfig_pool_init(const bool ipv4_pool, enum pool_type type, in_addr_t start,
+ in_addr_t end, const bool duplicate_cn,
const bool ipv6_pool, const struct in6_addr ipv6_base,
const int ipv6_netbits )
{
struct gc_arena gc = gc_new();
struct ifconfig_pool *pool = NULL;
+int pool_size = -1;
ASSERT(start <= end && end - start < IFCONFIG_POOL_MAX);
ALLOC_OBJ_CLEAR(pool, struct ifconfig_pool);
-pool->ipv4.type = type;
pool->duplicate_cn = duplicate_cn;
-switch (pool->ipv4.type)
+pool->ipv4.enabled = ipv4_pool;
+
+if (pool->ipv4.enabled)
{
-case IFCONFIG_POOL_30NET:
-pool->ipv4.base = start & ~3;
-pool->ipv4.size = (((end | 3) + 1) - pool->ipv4.base) >> 2;
-break;
+pool->ipv4.type = type;
+switch (pool->ipv4.type)
+{
+case IFCONFIG_POOL_30NET:
+pool->ipv4.base = start & ~3;
+pool->ipv4.size = (((end | 3) + 1) - pool->ipv4.base) >> 2;
+break;
-case IFCONFIG_POOL_INDIV:
-pool->ipv4.base = start;
-pool->ipv4.size = end - start + 1;
-break;
+case IFCONFIG_POOL_INDIV:
+pool->ipv4.base = start;
+pool->ipv4.size = end - start + 1;
+break;
-default:
-
[Openvpn-devel] [PATCH v4 7/7] ipv6-pool: get rid of size constraint
Signed-off-by: Antonio Quartulli
---
src/openvpn/helper.c | 7 +++
src/openvpn/options.c | 13 +
src/openvpn/pool.c| 12
3 files changed, 24 insertions(+), 8 deletions(-)
diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c
index 277e6972..2174b580 100644
--- a/src/openvpn/helper.c
+++ b/src/openvpn/helper.c
@@ -198,12 +198,11 @@ helper_client_server(struct options *o)
print_in6_addr( add_in6_addr( o->server_network_ipv6, 2), 0,
>gc );
o->ifconfig_ipv6_netbits = o->server_netbits_ipv6;
-/* pool starts at "base address + 0x1000" - leave enough room */
-ASSERT( o->server_netbits_ipv6 <= 112 );/* want 16 bits */
+/* basic sanity check */
+ASSERT(o->server_netbits_ipv6 >= 64 && o->server_netbits_ipv6 <= 124);
o->ifconfig_ipv6_pool_defined = true;
-o->ifconfig_ipv6_pool_base =
-add_in6_addr( o->server_network_ipv6, 0x1000 );
+o->ifconfig_ipv6_pool_base = add_in6_addr(o->server_network_ipv6, 2);
o->ifconfig_ipv6_pool_netbits = o->server_netbits_ipv6;
push_option( o, "tun-ipv6", M_USAGE );
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 3798731e..327207bd 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6714,9 +6714,12 @@ add_option(struct options *options,
msg(msglevel, "error parsing --server-ipv6 parameter");
goto err;
}
-if (netbits < 64 || netbits > 112)
+if (netbits < 64 || netbits > 124)
{
-msg( msglevel, "--server-ipv6 settings: only /64../112 supported
right now (not /%d)", netbits );
+msg(msglevel,
+"--server-ipv6 settings: network must be between /64 and /124
(not /%d)",
+netbits);
+
goto err;
}
options->server_ipv6_defined = true;
@@ -6836,9 +6839,11 @@ add_option(struct options *options,
msg(msglevel, "error parsing --ifconfig-ipv6-pool parameters");
goto err;
}
-if (netbits < 64 || netbits > 112)
+if (netbits < 64 || netbits > 124)
{
-msg( msglevel, "--ifconfig-ipv6-pool settings: only /64../112
supported right now (not /%d)", netbits );
+msg(msglevel,
+"--ifconfig-ipv6-pool settings: network must be between /64
and /124 (not /%d)",
+netbits);
goto err;
}
diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
index 42c6a38b..62cb0413 100644
--- a/src/openvpn/pool.c
+++ b/src/openvpn/pool.c
@@ -207,6 +207,12 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type
type, in_addr_t start,
ASSERT(0);
}
+if (pool->ipv4.size < 2)
+{
+msg(M_FATAL, "IPv4 pool size is too small (%d), must be at least
2",
+pool->ipv4.size);
+}
+
msg(D_IFCONFIG_POOL, "IFCONFIG POOL: base=%s size=%d",
print_in_addr_t(pool->ipv4.base, 0, ), pool->ipv4.size);
}
@@ -245,6 +251,12 @@ ifconfig_pool_init(const bool ipv4_pool, enum pool_type
type, in_addr_t start,
? (1 << (128 - ipv6_netbits)) - base
: IFCONFIG_POOL_MAX;
+if (pool->ipv6.size < 2)
+{
+msg(M_FATAL, "IPv6 pool size is too small (%d), must be at least
2",
+pool->ipv6.size);
+}
+
msg(D_IFCONFIG_POOL, "IFCONFIG POOL IPv6: base=%s size=%d netbits=%d",
print_in6_addr(pool->ipv6.base, 0, ), pool->ipv6.size,
ipv6_netbits);
--
2.26.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v4 3/7] allow usage of --server-ipv6 even when no --server is specified
From: Antonio Quartulli
Until now OpenVPN has not allowed to specify --server-ipv6
if no --server was also set. This constraint comes from the
fact that most of the IPv6 logic (i.e. ifconfig-pool handling)
relied on IPv4 components to be activated and configured as
well.
Now that the IPv6 code path has been made independent from
IPv4, it is finally possible to to relax the constraint
mentioned above and make it possible for the user to have a
configurations with --server-ipv6 only.
Trac: #208
Signed-off-by: Antonio Quartulli
---
Changes from v4:
- fix typ0 in message
Changes from v3:
- patchset rebased on top of pre-ipv6-only patchset
Changes from v4:
- change commit subject/message
- move ifconfig-ipv6-pool check change into 4/8
src/openvpn/helper.c | 8 ++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/openvpn/helper.c b/src/openvpn/helper.c
index 34cfa6bd..277e6972 100644
--- a/src/openvpn/helper.c
+++ b/src/openvpn/helper.c
@@ -174,10 +174,11 @@ helper_client_server(struct options *o)
*/
if (o->server_ipv6_defined)
{
-if (!o->server_defined)
+if (o->client)
{
-msg(M_USAGE, "--server-ipv6 must be used together with --server");
+msg(M_USAGE, "--server-ipv6 and --client cannot be used together");
}
+
if (o->server_flags & SF_NOPOOL)
{
msg( M_USAGE, "--server-ipv6 is incompatible with 'nopool' option"
);
@@ -187,6 +188,9 @@ helper_client_server(struct options *o)
msg( M_USAGE, "--server-ipv6 already defines an
ifconfig-ipv6-pool, so you can't also specify --ifconfig-pool explicitly");
}
+o->mode = MODE_SERVER;
+o->tls_server = true;
+
/* local ifconfig is "base address + 1" and "+2" */
o->ifconfig_ipv6_local =
print_in6_addr( add_in6_addr( o->server_network_ipv6, 1), 0,
>gc );
--
2.26.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v4 6/7] options: enable IPv4 redirection logic only if really required
From: Antonio Quartulli
If no IPv4 redirection flag is set, do not enable the IPv4
redirection logic at all so that it won't bother adding any
useless IPv4 route.
Trac: #208
Signed-off-by: Antonio Quartulli
---
Changes from v4:
- move error message modification to previous patch
Changes from v3:
- patchset rebased on top of pre-ipv6-only patchset
src/openvpn/options.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 7556e7ee..3798731e 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -6591,7 +6591,14 @@ add_option(struct options *options,
/* we need this here to handle pushed --redirect-gateway */
remap_redirect_gateway_flags(options);
#endif
-options->routes->flags |= RG_ENABLE;
+/* enable IPv4 redirection logic only if at least one IPv4 flag is set.
+ * For instance, when "redirect-gateway !ipv4 ipv6" is specified no
+ * IPv4 redirection should be activated.
+ */
+if (options->routes->flags)
+{
+options->routes->flags |= RG_ENABLE;
+}
}
else if (streq(p[0], "block-ipv6") && !p[1])
{
--
2.26.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v4 4/7] pool: add support for ifconfig-pool-persist with IPv6 only
From: Antonio Quartulli
Without altering the pool logic, this patch enables using
a persistent IP pool also when the server is configured
with IPv6 only.
Trac: #208
Signed-off-by: Antonio Quartulli
---
Changes from v3:
- patchset rebased on top of pre-ipv6-only patchset
src/openvpn/options.c | 7 ++-
src/openvpn/pool.c| 140 --
2 files changed, 114 insertions(+), 33 deletions(-)
diff --git a/src/openvpn/options.c b/src/openvpn/options.c
index 6c0fc0ed..7556e7ee 100644
--- a/src/openvpn/options.c
+++ b/src/openvpn/options.c
@@ -2346,9 +2346,12 @@ options_postprocess_verify_ce(const struct options
*options, const struct connec
{
msg(M_USAGE, "--up-delay cannot be used with --mode server");
}
-if (!options->ifconfig_pool_defined &&
options->ifconfig_pool_persist_filename)
+if (!options->ifconfig_pool_defined
+&& !options->ifconfig_ipv6_pool_defined
+&& options->ifconfig_pool_persist_filename)
{
-msg(M_USAGE, "--ifconfig-pool-persist must be used with
--ifconfig-pool");
+msg(M_USAGE,
+"--ifconfig-pool-persist must be used with --ifconfig-pool or
--ifconfig-ipv6-pool");
}
if (options->ifconfig_ipv6_pool_defined &&
!options->ifconfig_ipv6_local)
{
diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
index 48f3ec95..42c6a38b 100644
--- a/src/openvpn/pool.c
+++ b/src/openvpn/pool.c
@@ -34,6 +34,7 @@
#include "error.h"
#include "socket.h"
#include "otime.h"
+#include "options.h"
#include "memdbg.h"
@@ -391,12 +392,52 @@ ifconfig_pool_ip_base_to_handle(const struct
ifconfig_pool *pool, const in_addr_
return ret;
}
+static ifconfig_pool_handle
+ifconfig_pool_ipv6_base_to_handle(const struct ifconfig_pool *pool,
+ const struct in6_addr *in_addr)
+{
+ifconfig_pool_handle ret;
+uint32_t base, addr;
+
+/* IPv6 pool is always IFCONFIG_POOL_INDIV.
+ *
+ * We assume the offset can't be larger than 2^32-1, therefore we compute
+ * the difference only among the last 4 bytes like if they were two 32bit
+ * long integers. The rest of the address must match.
+ */
+for (int i = 0; i < (12); i++)
+{
+if (pool->ipv6.base.s6_addr[i] != in_addr->s6_addr[i])
+{
+return -1;
+}
+}
+
+base = (pool->ipv6.base.s6_addr[12] << 24)
+ | (pool->ipv6.base.s6_addr[13] << 16)
+ | (pool->ipv6.base.s6_addr[14] << 8)
+ | pool->ipv6.base.s6_addr[15];
+
+addr = (in_addr->s6_addr[12] << 24)
+ | (in_addr->s6_addr[13] << 16)
+ | (in_addr->s6_addr[14] << 8)
+ | in_addr->s6_addr[15];
+
+ret = addr - base;
+if (ret < 0 || ret >= pool->ipv6.size)
+{
+ret = -1;
+}
+
+return ret;
+}
+
static in_addr_t
ifconfig_pool_handle_to_ip_base(const struct ifconfig_pool *pool,
ifconfig_pool_handle hand)
{
in_addr_t ret = 0;
-if (hand >= 0 && hand < pool->ipv4.size)
+if (pool->ipv4.enabled && hand >= 0 && hand < pool->ipv4.size)
{
switch (pool->ipv4.type)
{
@@ -426,7 +467,7 @@ ifconfig_pool_handle_to_ipv6_base(const struct
ifconfig_pool *pool, ifconfig_poo
struct in6_addr ret = in6addr_any;
/* IPv6 pools are always INDIV (--linear) */
-if (hand >= 0 && hand < pool->ipv6.size)
+if (pool->ipv6.enabled && hand >= 0 && hand < pool->ipv6.size)
{
ret = add_in6_addr( pool->ipv6.base, hand );
}
@@ -434,9 +475,34 @@ ifconfig_pool_handle_to_ipv6_base(const struct
ifconfig_pool *pool, ifconfig_poo
}
static void
-ifconfig_pool_set(struct ifconfig_pool *pool, const char *cn, const in_addr_t
addr, const bool fixed)
+ifconfig_pool_set(struct ifconfig_pool *pool, const char *cn,
+ const in_addr_t addr, const struct in6_addr *addr6,
+ const bool fixed)
{
-ifconfig_pool_handle h = ifconfig_pool_ip_base_to_handle(pool, addr);
+ifconfig_pool_handle h = -1, h6 = -1;
+
+if (pool->ipv6.enabled)
+{
+h = h6 = ifconfig_pool_ipv6_base_to_handle(pool, addr6);
+}
+
+if (pool->ipv4.enabled)
+{
+h = ifconfig_pool_ip_base_to_handle(pool, addr);
+/* at the moment IPv4 and IPv6 share the same pool, therefore offsets
+ * have to match for the same client
+ */
+if ((pool->ipv6.enabled) && (h != h6))
+{
+struct gc_arena gc = gc_new();
+msg(M_WARN,
+"pool: IPv4 (%s) and IPv6 (%s) have different offsets! Relying
on IPv4",
+print_in_addr_t(addr, 0, ),
+print_in6_addr(*addr6, 0, ));
+gc_free();
+}
+}
+
if (h >= 0)
{
struct ifconfig_pool_entry *e = >list[h];
@@ -459,23 +525,26 @@ ifconfig_pool_list(const struct ifconfig_pool *pool,
struct status_output *out)
for
[Openvpn-devel] [PATCH v4 1/7] pool: prevent IPv6 pools to be larger than 2^16 addresses
Signed-off-by: Antonio Quartulli
---
src/openvpn/pool.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/openvpn/pool.c b/src/openvpn/pool.c
index 5d503a33..6dd72bb9 100644
--- a/src/openvpn/pool.c
+++ b/src/openvpn/pool.c
@@ -183,7 +183,7 @@ ifconfig_pool_init(enum pool_type type, in_addr_t start,
in_addr_t end,
if (pool->ipv6.enabled)
{
pool->ipv6.base = ipv6_base;
-pool->ipv6.size = ipv6_netbits > 96 ? (1 << (128 - ipv6_netbits))
+pool->ipv6.size = ipv6_netbits > 112 ? (1 << (128 - ipv6_netbits))
: IFCONFIG_POOL_MAX;
msg( D_IFCONFIG_POOL, "IFCONFIG POOL IPv6: (IPv4) size=%d,
size_ipv6=%d, netbits=%d, base_ipv6=%s",
--
2.26.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v4 5/7] route: warn on IPv4 routes installation when no IPv4 is configured
From: Antonio Quartulli
Same as already happens for IPv6, it is useful for the user to throw a
warning when an IPv4 route is about to be installed and the tun interface
has no IPv4 configured.
The twin message for IPv4 is adapted to have the same format.
The warning is not fatal, becuase the route might actually be external
to the tun interface and therefore it may still work.
At the same time, modify the error message used for a route
installation failure to explicitly mention "IPv4" since this it is
used in the IPv4 code path only.
Trac: #208
Signed-off-by: Antonio Quartulli
---
Changes from v4:
- edit commit message
- modify IPv4 warning as well (moved from next patch)
Changes from v3:
- patchset rebased on top of pre-ipv6-only patchset
src/openvpn/route.c | 16
1 file changed, 12 insertions(+), 4 deletions(-)
diff --git a/src/openvpn/route.c b/src/openvpn/route.c
index 51f76318..9ff36359 100644
--- a/src/openvpn/route.c
+++ b/src/openvpn/route.c
@@ -987,7 +987,7 @@ redirect_default_route_to_vpn(struct route_list *rl, const
struct tuntap *tt,
unsigned int flags, const struct env_set *es,
openvpn_net_ctx_t *ctx)
{
-const char err[] = "NOTE: unable to redirect default gateway --";
+const char err[] = "NOTE: unable to redirect IPv4 default gateway --";
if (rl && rl->flags & RG_ENABLE)
{
@@ -1192,6 +1192,14 @@ add_routes(struct route_list *rl, struct route_ipv6_list
*rl6,
{
struct route_ipv4 *r;
+if (rl->routes && !tt->did_ifconfig_setup)
+{
+msg(M_INFO, "WARNING: OpenVPN was configured to add an IPv4 "
+"route. However, no IPv4 has been configured for %s, "
+"therefore the route installation may fail or may not work "
+"as expected.", tt->actual_name);
+}
+
#ifdef ENABLE_MANAGEMENT
if (management && rl->routes)
{
@@ -1223,9 +1231,9 @@ add_routes(struct route_list *rl, struct route_ipv6_list
*rl6,
if (!tt->did_ifconfig_ipv6_setup)
{
msg(M_INFO, "WARNING: OpenVPN was configured to add an IPv6 "
-"route over %s. However, no IPv6 has been configured for "
-"this interface, therefore the route installation may "
-"fail or may not work as expected.", tt->actual_name);
+"route. However, no IPv6 has been configured for %s, "
+"therefore the route installation may fail or may not work "
+"as expected.", tt->actual_name);
}
for (r = rl6->routes_ipv6; r; r = r->next)
--
2.26.2
___
Openvpn-devel mailing list
Openvpn-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
[Openvpn-devel] [PATCH v4 0/7] Allow IPv6-only tunnels
This patchset allows servers to work with a tunnel configured with IPv6 only (client code has already been merged). This new v4 is the result of some more reviews, including an hardcore reviewing session that me and Gert had yesterday afternoon. We found some interesting bugs and agreed on some previously-not-well-defined behaviours. The patchset is now ready for broader testing and potential final vetting. In a nutshell, after applying this patchset, it will be possible to configure a server with just --server-ipv6 and no IPv4 at all (in the tunnel). Happy hacking! (Note: this code is also available at [1] in the ipv6-only branch) Cheers, Trac: #208 [1] https://gitlab.com/ordex986/openvpn Antonio Quartulli (7): pool: prevent IPv6 pools to be larger than 2^16 addresses pool: allow to configure an IPv6-only ifconfig-pool allow usage of --server-ipv6 even when no --server is specified pool: add support for ifconfig-pool-persist with IPv6 only route: warn on IPv4 routes installation when no IPv4 is configured options: enable IPv4 redirection logic only if really required ipv6-pool: get rid of size constraint src/openvpn/helper.c | 15 +- src/openvpn/multi.c | 10 +- src/openvpn/options.c | 29 +++- src/openvpn/pool.c| 317 -- src/openvpn/pool.h| 8 +- src/openvpn/route.c | 16 ++- 6 files changed, 302 insertions(+), 93 deletions(-) -- 2.26.2 ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Add .git-blame-ignore-revs with reformat commits
Hi, On 29/05/2020 16:21, Arne Schwabe wrote: > Am 29.05.20 um 15:51 schrieb David Sommerseth: >> On 29/05/2020 01:54, Arne Schwabe wrote: >>> +# Merge 'reformatting' branch into master >>> +1f004b2f06e987d73e48f7fd7b96b0b248274f58 >> >> Are you sure this "merge" commit shouldn't be commit 81d882d5 instead? >> > > I am not sure to be honest, that is why I included both commits as > ignoring the merge commit might be instead then showing the other commit. merge commits are normally there to solve conflicts..or to do nothing. The conflict might be related to the reformatting (so you think we may want to hide it)...but I think we should not ignore it in any case as resolving a conflict is potentially a source of bugs. Regards, -- Antonio Quartulli ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Add .git-blame-ignore-revs with reformat commits
Am 29.05.20 um 15:51 schrieb David Sommerseth: > On 29/05/2020 01:54, Arne Schwabe wrote: >> This allows git blame to ignore reformatting changes and instead >> shows the previous commit that changed the line. >> >> To avoid manually building the list of commits this commit >> adds a file with a list of reformatting commits. I might have >> missed a few but this should be a good start. To use the file >> use: >> >>git blame --ignore-revs-file=.git-blame-ignore-revs file >> >> or to automatically always use the file >> >>git config blame.ignoreRevsFile .git-blame-ignore-revs >> >> Naming the file .git-blame-ignore-revs is a convention. >> >> Some more details in this random blog post: >> >> https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame >> > > Feature-ACK ... the commit message could also mention that git 2.23 is needed > as well. > >> Signed-off-by: Arne Schwabe >> --- >> .git-blame-ignore-revs| 29 +++ >> .../managent-demo/management-demo-server.py | 0 >> contrib/ovpnkeys.py | 0 >> 3 files changed, 29 insertions(+) >> create mode 100644 .git-blame-ignore-revs >> create mode 100644 contrib/managent-demo/management-demo-server.py >> create mode 100644 contrib/ovpnkeys.py >> >> diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs >> new file mode 100644 >> index ..3cc811cf >> --- /dev/null >> +++ b/.git-blame-ignore-revs >> @@ -0,0 +1,29 @@ >> +# Uncrustify the tests/unit_tests/ part of our tree. >> +da1574ef7826d73f01e120cbd1ba40ce39a305b7 >> + >> +# Another round of uncrustify code cleanup. >> +9cf7b4925a54d93fbea1cadcf3dc0e11f3ce358f >> + >> +# networking_sitnl.c: uncrustify file >> +2c45d268ca65c522fbabb7c4dab5e721296b4623 >> + >> +# Uncrustify tapctl and openvpnmsica >> +6280d3d5536174934ee22d3840457d61896e0e3a > > Missing a blank line here, for consistency > >> +# tun.c: uncrustify >> +baef44fc8769bbd99f4d699ce9f63180c29a5455 >> + >> +# networking_sitnl.c: uncrustify file >> +2c45d268ca65c522fbabb7c4dab5e721296b4623 >> + >> +# uncrustify openvpn sources >> +f57431cdc88f22fa4d7962946f0d3187fe058539 >> + >> +# More broadly enforce Allman style and braces-around-conditionals >> +4cd4899e8e80efae03c584a760fd107251735723 >> + >> +# Merge 'reformatting' branch into master >> +1f004b2f06e987d73e48f7fd7b96b0b248274f58 > > Are you sure this "merge" commit shouldn't be commit 81d882d5 instead? > I am not sure to be honest, that is why I included both commits as ignoring the merge commit might be instead then showing the other commit. Arne signature.asc Description: OpenPGP digital signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [PATCH] Add .git-blame-ignore-revs with reformat commits
On 29/05/2020 01:54, Arne Schwabe wrote: > This allows git blame to ignore reformatting changes and instead > shows the previous commit that changed the line. > > To avoid manually building the list of commits this commit > adds a file with a list of reformatting commits. I might have > missed a few but this should be a good start. To use the file > use: > >git blame --ignore-revs-file=.git-blame-ignore-revs file > > or to automatically always use the file > >git config blame.ignoreRevsFile .git-blame-ignore-revs > > Naming the file .git-blame-ignore-revs is a convention. > > Some more details in this random blog post: > > https://www.moxio.com/blog/43/ignoring-bulk-change-commits-with-git-blame > Feature-ACK ... the commit message could also mention that git 2.23 is needed as well. > Signed-off-by: Arne Schwabe > --- > .git-blame-ignore-revs| 29 +++ > .../managent-demo/management-demo-server.py | 0 > contrib/ovpnkeys.py | 0 > 3 files changed, 29 insertions(+) > create mode 100644 .git-blame-ignore-revs > create mode 100644 contrib/managent-demo/management-demo-server.py > create mode 100644 contrib/ovpnkeys.py > > diff --git a/.git-blame-ignore-revs b/.git-blame-ignore-revs > new file mode 100644 > index ..3cc811cf > --- /dev/null > +++ b/.git-blame-ignore-revs > @@ -0,0 +1,29 @@ > +# Uncrustify the tests/unit_tests/ part of our tree. > +da1574ef7826d73f01e120cbd1ba40ce39a305b7 > + > +# Another round of uncrustify code cleanup. > +9cf7b4925a54d93fbea1cadcf3dc0e11f3ce358f > + > +# networking_sitnl.c: uncrustify file > +2c45d268ca65c522fbabb7c4dab5e721296b4623 > + > +# Uncrustify tapctl and openvpnmsica > +6280d3d5536174934ee22d3840457d61896e0e3a Missing a blank line here, for consistency > +# tun.c: uncrustify > +baef44fc8769bbd99f4d699ce9f63180c29a5455 > + > +# networking_sitnl.c: uncrustify file > +2c45d268ca65c522fbabb7c4dab5e721296b4623 > + > +# uncrustify openvpn sources > +f57431cdc88f22fa4d7962946f0d3187fe058539 > + > +# More broadly enforce Allman style and braces-around-conditionals > +4cd4899e8e80efae03c584a760fd107251735723 > + > +# Merge 'reformatting' branch into master > +1f004b2f06e987d73e48f7fd7b96b0b248274f58 Are you sure this "merge" commit shouldn't be commit 81d882d5 instead? > + > +# The Great Reformatting - first phase > +81d882d5302b8b647202a6893b57dfdc61fd6df2 > + > diff --git a/contrib/managent-demo/management-demo-server.py > b/contrib/managent-demo/management-demo-server.py > new file mode 100644 This file shouldn't be included in this change. > index ..e69de29b > diff --git a/contrib/ovpnkeys.py b/contrib/ovpnkeys.py > new file mode 100644 > index ..e69de29b > So a few minor things to look into, otherwise this is really valuable. I would probably also consider to also add a note about this git feature in README. It's not the most ideal file, but I struggle to find a better place currently (I might have overlooked something). We would probably benefit having a separate developers README with all the hints, tips and tricks developers would need to care about. That is, however, out of scope for this change. -- kind regards, David Sommerseth OpenVPN Inc signature.asc Description: OpenPGP digital signature ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
Re: [Openvpn-devel] [BUG] test_ncp.c failing
On 29-05-2020 01:46, James Bottomley wrote: > The problem seems to be openssl uses a mixed case name for the cipher > and EVP_CIPHER_name() is case sensitive. Applying the patch below > fixes this for openssl and gets make check to pass all tests, but I > rather wonder why this isn't part of cipher_kt_name() to prevent this > type of problem? This was my first thought when looking at the patch too. Probably because I wrote the cipher name translation code as one of my very first OpenVPN contributions, and didn't get the abstractions right at the time. Would be good to see if this can be refactored. Slightly annoying to test if that won't break corner cases though, because I think this part is not sufficiently covered by automated tests. -Steffan ___ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
