Attention is currently required from: flichtenheld, its_Giaan, ordex, plaisthos.
cron2 has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/436?usp=email ) Change subject: allow user to specify 'local' multiple times in config files ...................................................................... Patch Set 7: Code-Review-1 (9 comments) Patchset: PS7: This is messier than it should be - having the `local` list in the CE structure, while it is only useful for servers (that do not use CE). But maybe this cannot be fixed if we do want to permit a (single) `local` entry inside a `<connection>` (which makes sense). But besides this, there's questions... File doc/man-sections/link-options.rst: http://gerrit.openvpn.net/c/openvpn/+/436/comment/2ee16be6_972b5621 : PS7, Line 114: open a socket. This text is not clear to me. What does "*" do? (This interacts with #761 on `::`). Maybe: ``` On a client, or in point-to-point mode, this can only be specified once (1 socket). On an OpenVPN setup running as `--server`, this can be specified multiple times to open multiple listening sockets on different addresses and/or different ports. In order to specify multiple listen ports without specifying an address, use '*' to signal "use what the operating system gives you as default". For "all IPv4 addresses" use '0.0.0.0', for "all IPv6 addresses" use '::'. `--local` implies `--bind`. ``` (It's also slightly unclear how this interacts with `--proto`? Maybe this is added in a later patch of the series?) File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/436/comment/c96e1fb3_88b2e29c : PS7, Line 133: " a server listen on multiple sockets at the same time.\n" when there is agreement on the wording in the manpage, this needs to be addressed as well http://gerrit.openvpn.net/c/openvpn/+/436/comment/1ff60b54_20cb34b1 : PS7, Line 998: setenv_str_i(es, "local", e->local_list->array[0]->local, i); will this crash if `--local` has not been used at all? http://gerrit.openvpn.net/c/openvpn/+/436/comment/24119bb9_af8f4bf9 : PS7, Line 2406: } I would not check on `ce->remote` but on `mode == SERVER` here, and change the wording to `multiple --local statements only allowed in --server mode`. Also, will this crash if no `--local` statement has been used (aka is the list always initialized?) http://gerrit.openvpn.net/c/openvpn/+/436/comment/8fdc0098_59259f5f : PS7, Line 3185: bool need_bind = ce->local_port_defined || ce->bind_defined; this looks incomplete - shouldn't the existence of a `--local` entry also set `need_bind = true`? http://gerrit.openvpn.net/c/openvpn/+/436/comment/f467c398_a6c43f2d : PS7, Line 3198: if (o->client && !need_bind) is this intentional or a rebase artefact? This looks a bit unrelated http://gerrit.openvpn.net/c/openvpn/+/436/comment/18bf97b8_6adc2bc6 : PS7, Line 3827: /* use the same listen list for every outgoing connection */ well... for outgoing connections we do not want to use a *list* at all, as we can only bind() once...? (but I need to read up on o->ce vs. o->connection_list) http://gerrit.openvpn.net/c/openvpn/+/436/comment/42fa7611_2d03d61e : PS7, Line 6250: e->bind_local = true; I do not understand this `bind_local` variable. Of course we bind(), otherwise the whole setting of `local` does not make sense? And even on "*" we do want to `bind()`...? -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/436?usp=email To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings Gerrit-Project: openvpn Gerrit-Branch: master Gerrit-Change-Id: I4d1c96662c5a8c750d883e3b20adde09529e2764 Gerrit-Change-Number: 436 Gerrit-PatchSet: 7 Gerrit-Owner: ordex <a...@unstable.cc> Gerrit-Reviewer: cron2 <g...@greenie.muc.de> Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com> Gerrit-Reviewer: its_Giaan <gianma...@mandelbit.com> Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org> Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net> Gerrit-Attention: plaisthos <arne-open...@rfc2549.org> Gerrit-Attention: its_Giaan <gianma...@mandelbit.com> Gerrit-Attention: flichtenheld <fr...@lichtenheld.com> Gerrit-Attention: ordex <a...@unstable.cc> Gerrit-Comment-Date: Wed, 27 Nov 2024 18:19:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
