Attention is currently required from: cron2, ordex, plaisthos. its_Giaan has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/434?usp=email )
Change subject: Adapt socket handling to support listening on multiple sockets ...................................................................... Patch Set 7: (16 comments) File src/openvpn/forward.c: http://gerrit.openvpn.net/c/openvpn/+/434/comment/5c3d2260_ff08a995 : PS7, Line 1133: /* all sockets are of the same type, so just check the first one */ > this confuses me. […] Done File src/openvpn/init.c: http://gerrit.openvpn.net/c/openvpn/+/434/comment/c8bba806_c5354795 : PS7, Line 533: { > this is a behavioural change, not checking `advance_next_remote` anymore. […] Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/0274c355_3b2e8c19 : PS7, Line 1959: ASSERT(c->c2.link_sockets); > this is not doing the same thing as before... […] Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/9e623ad9_318662fe : PS7, Line 2015: do_ifconfig(c->c1.tuntap, c->c1.tuntap->actual_name, c->c2.frame.tun_mtu, c->c2.es, &c->net_ctx); > line length exceeded, and the old version wasn't that bad (the unwrapping > above was reasonable, with […] Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/83e1f2b3_82aeffb6 : PS7, Line 3819: do_init_socket_1(struct context *c) > I think we should stick to `phase1` as function name, otherwise it reads as > if this is "init socket […] Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/983be341_eb72cb9c : PS7, Line 4011: if (c->c2.link_sockets && c->c2.link_socket_owned) > what happened to the `is_dco_win()` part here? This looks like a rebase > accident Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/0dd0c10f_cbddc037 : PS7, Line 4040: CLEAR(c->c1.link_socket_addrs[0].actual); > this looks inconsistent - freeaddrinfo() is called in the loop, while the > CLEAR() is only called for […] Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/8c4402d6_a5c9227d : PS7, Line 4719: do_init_socket_1(c); > mmmh, see comment above, maybe name this `do_init_link_sockets_phase1()`? […] Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/90c989e7_e8b47bfa : PS7, Line 4762: do_init_socket_2(c); > `_phase2()` Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/da819cb1_03b64f6a : PS7, Line 4968: &dest->gc); > is this wrapping necessary? Comparing to the lines below it does not seem > like it Acknowledged File src/openvpn/mudp.c: http://gerrit.openvpn.net/c/openvpn/+/434/comment/d1d68fc6_23406680 : PS7, Line 384: for (int i = 0; i < m->top.c1.link_sockets_num; i++) > this comment opened a little pandora's box... […] In the next patches we will have a common server loop for both TCP and UDP however the I/O processing will remain separate, for this specific case we decided to pass directly the link_socket to avoid to walk through all of them, in the final patch we will handle directly the link_socket returned by the event_set. File src/openvpn/socket.c: http://gerrit.openvpn.net/c/openvpn/+/434/comment/6534f7ed_de52f295 : PS7, Line 1858: c->c2.link_sockets[sock_index]->local_host = c->options.ce.local; > Can this be changed to `*sock = &c->c2. […] Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/b247da87_2274fb46 : PS7, Line 2222: #if defined (_WIN32) > unrelated whitespace change, please do not (also, this extra space is not how > it's done elsewhere) Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/58256377_1c734eb8 : PS7, Line 2259: sig_info->signal_received = SIGUSR1; > I think this is a rebase artefact, the multisocket patch colliding with > Selva's signal handler rewor […] Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/72e83319_3a6cdcc6 : PS7, Line 2271: sig_info); > please do not wrap. The line is not long and not changed by this patch. Done http://gerrit.openvpn.net/c/openvpn/+/434/comment/93ac21b3_d36b9855 : PS7, Line 2297: if (sig_save) > again, please thoroughly double check. […] Done -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/434?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: Ia0a889e800f0b36aed770ee36e31afeec5df6084 Gerrit-Change-Number: 434 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: cron2 <g...@greenie.muc.de> Gerrit-Attention: ordex <a...@unstable.cc> Gerrit-Comment-Date: Tue, 10 Dec 2024 13:10:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: cron2 <g...@greenie.muc.de> Comment-In-Reply-To: ordex <a...@unstable.cc> Gerrit-MessageType: comment
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
