Open cron2 opened 3 months ago
So if init_tun()
is called with strict_warn == true
then we warn when the second argument doesn't look like a netmask.
But the problem with that check is that it actually uses topology to detect whether we're in P2P mode:
bool
is_tun_p2p(const struct tuntap *tt)
{
bool tun = false;
if (tt->type == DEV_TYPE_TAP
|| (tt->type == DEV_TYPE_TUN && tt->topology == TOP_SUBNET)
|| tt->type == DEV_TYPE_NULL)
{
tun = false;
}
else if (tt->type == DEV_TYPE_TUN)
{
tun = true;
}
else
{
msg(M_FATAL, "Error: problem with tun vs. tap setting"); /* JYFIXME -- needs to be caught earlier, in ini\
t_tun? */
}
return tun;
}
So by changing the default we probably broke this check.
Nevermind, in current master we do not actually use is_tun_p2p
. This is only added by my patch https://gerrit.openvpn.net/c/openvpn/+/380. Master uses tt->type == DEV_TYPE_TUN
which is just not correct, so the wrong checks are done probably.
Changing the default only for --server
would probably look something like this:
TOP_UNDEF
instead of TOP_SUBNET
helper_client_server
check whether topology is still TOP_UNDEF
, if yes change to TOP_SUBNET
helper_client_server
set topology to TOP_NET30
if it is still TOP_UNDEF
After
helper_client_server
set topology toTOP_NET30
if it is stillTOP_UNDEF
honestly this feels a bit hacky: i.e. using what we have in a dirty way to properly achieve what we need.
How about explicitly adding a TOP_P2P ? after all this is not NET30, but it's a truly different way of assigning the IPs (local+remote vs /30).
This way we can then explicitly check if topology == TOP_P2P and act accordingly.
As discussed on IRC: There is already a TOP_P2P
which does something slightly different again, although most of the code treats it as alias for TOP_NET30
.
So the topology
default has been taken care of, but the other aspect of passing on -1
as netbits after parsing is still not really helpful error handling.
So the
topology
default has been taken care of, but the other aspect of passing on-1
as netbits after parsing is still not really helpful error handling.
@cron2 Please take a look at https://gerrit.openvpn.net/c/openvpn/+/380, my earlier comments seem to indicate that it should improve that part.
so there's a p2p tun config with
and with commit 32e6586687 the new default is now
topology subnet
. This leads to the instance no longer starting, withso it seems "something" is trying to convert the second argument to a netmask/netbits, failing, assigning "-1" to "something" and passing that to sitnl...
This is a config with no
client
orserver
, just plain p2p udp, so it surprised me a bit thattopology
would be relevant here - but quite obviously it changes the interpretation ofifconfig
.So there's two questions here
net30
for point-to-point configs (noserver
)? Which is, of course, much more work than just changing the global default for all...@ordex for the parser, @flichtenheld for the
topology
default.