Attention is currently required from: cron2, flichtenheld, plaisthos. stipa has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/815?usp=email )
Change subject: dco-win: multipeer support ...................................................................... Patch Set 19: (12 comments) File src/openvpn/dco_win.h: http://gerrit.openvpn.net/c/openvpn/+/815/comment/4fc86de9_75204ca5 : PS18, Line 42: dco_mode_type mode; > I'm not really happy with that. […] Acknowledged File src/openvpn/dco_win.c: http://gerrit.openvpn.net/c/openvpn/+/815/comment/958d5b8c_a18c12a8 : PS18, Line 127: > A few words of comment somewhere on the call chain "who calls what, in which > sequence" for p2p and m […] Acknowledged http://gerrit.openvpn.net/c/openvpn/+/815/comment/23adf9fa_be8de950 : PS18, Line 132: dco->mode = (mode == MODE_POINT_TO_POINT) ? dco_mode_p2p : dco_mode_mp; > that comment is slightly confusing - if it is "already set", then why do we > set it again? Acknowledged http://gerrit.openvpn.net/c/openvpn/+/815/comment/7778dea2_f293c54f : PS18, Line 146: OVPN_MODE m = OVPN_MODE_MP; > here we have a new define for communication between driver and userland, > which does the same as `dco […] Yeah, this one is from the driver API. But I see the frustration. http://gerrit.openvpn.net/c/openvpn/+/815/comment/0dc53053_22601180 : PS18, Line 291: if (out.ListenAddress.Addr4.sin_family == AF_INET) > This part confuses me. What is `out`? Why is it put into `actual. […] The driver writes to out the local transport address of the socket after the bind() call. But we already know the local bind address, except maybe the port, we probably don't want to bind to port 0. It seems that there is no use of this data, so I'll remove those lines. http://gerrit.openvpn.net/c/openvpn/+/815/comment/26e4fbd9_488525de : PS18, Line 393: { > I do wonder why this is necessary. […] I see. At the moment driver doesn't use local transport address of the peer, although this is a part of API. I will remove this code. File src/openvpn/forward.c: http://gerrit.openvpn.net/c/openvpn/+/815/comment/ea8ee09a_b0ad999b : PS18, Line 1326: bool prepend_sa = c->options.mode == MODE_SERVER && dco_enabled(&c->options); > we do have `SF_PREPEND_SA` set on the sockflags - is that available here? Actually we cannot end up here when using DCO. So prepend_sa should be false and I will add an ASSERT(!dco_enabled(&c->options)); File src/openvpn/ovpn_dco_win.h: http://gerrit.openvpn.net/c/openvpn/+/815/comment/98441252_eacace58 : PS18, Line 120: } OVPN_CRYPTO_DATA_V2, * POVPN_CRYPTO_DATA_V2; > these two are not part of *this* patch set, and might need revisiting for > "the new epoch thing" Acknowledged File src/openvpn/socket.c: http://gerrit.openvpn.net/c/openvpn/+/815/comment/9f6896eb_6e2a0bdf : PS18, Line 2256: else > since there is a "goto done" in the `if()` we could get rid of the `else` and > the dangling brackets […] Acknowledged http://gerrit.openvpn.net/c/openvpn/+/815/comment/be0ef538_110ecee4 : PS18, Line 3857: sa_len = sizeof(struct sockaddr_in); > the repetition of `sizeof(struct sockaddr_in)` makes my eyes hurt... […] Acknowledged http://gerrit.openvpn.net/c/openvpn/+/815/comment/c8f0d980_977d5293 : PS18, Line 3867: sa_len = sizeof(struct sockaddr_in6); > same here Acknowledged http://gerrit.openvpn.net/c/openvpn/+/815/comment/69e4d1f0_887d9531 : PS18, Line 3957: read_sockaddr_from_overlapped(io, &from->dest.addr.sa, ret); > I do not understand these two `sh.is_handle` cases. […] SH could be either file descriptor (in case of non-dco link) or handle (non-dco tun and dco link). `from` is NULL in case of non-dco tun and read tcp link - but my patch hasn't changed that. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/815?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: Ia267276d61fa1425ba205f54ba6eb89021f32dba Gerrit-Change-Number: 815 Gerrit-PatchSet: 19 Gerrit-Owner: stipa <lstipa...@gmail.com> Gerrit-Reviewer: cron2 <g...@greenie.muc.de> Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.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-Comment-Date: Thu, 06 Feb 2025 18:45:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: cron2 <g...@greenie.muc.de> Gerrit-MessageType: comment
_______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
