Attention is currently required from: cron2, flichtenheld, ordex, plaisthos.
its_Giaan 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 8: (9 comments) File doc/man-sections/link-options.rst: http://gerrit.openvpn.net/c/openvpn/+/436/comment/8dd8c81c_1c28df37 : PS7, Line 114: open a socket. > This text is not clear to me. What does "*" do? (This interacts with #761 > on `::`). […] Done, for the text. (--proto) It will not interact with --proto, I tried to avoid checking it as it is fishy on the server side since we may have multiple sockets of different protocols, instead we will check the protocol for each socket when possible, otherwise we rely on the mode. File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/436/comment/3c4e7f07_331b4f56 : PS5, Line 6247: if (strcmp(p[1], "*") != 0) > should we issue a warning if someone specifies "--local *" without port > number that this is a NOOP? Acknowledged File src/openvpn/options.c: http://gerrit.openvpn.net/c/openvpn/+/436/comment/d9af2a74_f67140b0 : 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 Done http://gerrit.openvpn.net/c/openvpn/+/436/comment/b04415ed_9d0e4ae8 : 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? No, it will not, even without specifying any '--local' we're gonna create a local_entry based on the global port, see options.c:3818 for details. http://gerrit.openvpn.net/c/openvpn/+/436/comment/750168cd_967ca0a0 : PS7, Line 2406: } > I would not check on `ce->remote` but on `mode == SERVER` here, and change > the wording to `multiple […] Done for the check. See answer above for --local crash question. http://gerrit.openvpn.net/c/openvpn/+/436/comment/fb946320_525f1ec3 : 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`? Extended this with checking for the local_list, (if not NULL we're going to bind). http://gerrit.openvpn.net/c/openvpn/+/436/comment/d52d560f_2dcb23ff : PS7, Line 3198: if (o->client && !need_bind) > is this intentional or a rebase artefact? This looks a bit unrelated Done http://gerrit.openvpn.net/c/openvpn/+/436/comment/6ecec3d0_fc5af621 : 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.. […] We handle everything as a list, but later on there's a specific sanity check that in the case of clients ensures that the list has only one element. http://gerrit.openvpn.net/c/openvpn/+/436/comment/3a5e4b2a_9eed652f : PS7, Line 6250: e->bind_local = true; > I do not understand this `bind_local` variable. […] I removed the bind_local variable on the local_entry and rely on the o->ce.bind_local which has all the sanity checks of the case. -- 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: 8 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: cron2 <g...@greenie.muc.de> Gerrit-Attention: flichtenheld <fr...@lichtenheld.com> Gerrit-Attention: ordex <a...@unstable.cc> Gerrit-Comment-Date: Thu, 12 Dec 2024 09:42:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: cron2 <g...@greenie.muc.de> Comment-In-Reply-To: flichtenheld <fr...@lichtenheld.com> Gerrit-MessageType: comment
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
