Attention is currently required from: ralf_lici. plaisthos has posted comments on this change. ( http://gerrit.openvpn.net/c/openvpn/+/685?usp=email )
Change subject: Add support for HAProxy's PROXY protocol ...................................................................... Patch Set 1: Code-Review-1 (9 comments) Patchset: PS1: I think this needs more overhaul File CMakeLists.txt: http://gerrit.openvpn.net/c/openvpn/+/685/comment/59d24823_1499669a : PS1, Line 494: src/openvpn/proxy_protocol.h haproxy_protocol.c/haproxy_protol.h Currently it looks like they are related to the other proxy code which they are not. File doc/proxy-protocol.txt: http://gerrit.openvpn.net/c/openvpn/+/685/comment/4932a48c_3085af39 : PS1, Line 8: https://www.haproxy.org/download/2.4/doc/proxy-protocol.txt Is this just a copy and paste of the document or is this addition to that document? It would be good to clarify the relationship between these docuemnts. http://gerrit.openvpn.net/c/openvpn/+/685/comment/4a84e43a_90eb3cad : PS1, Line 11: a binary version (v2). Is this protocol TCP only? I assume so, so would be good to mention this. http://gerrit.openvpn.net/c/openvpn/+/685/comment/1508ed20_2e564dcb : PS1, Line 26: --------- Why are supporting two new protocls? Is there a need to introduce an already deprecated protocol to OpenVPN? http://gerrit.openvpn.net/c/openvpn/+/685/comment/f26ed461_54c16e70 : PS1, Line 97: The v2 signature of the protocol: "\x0D\x0A\x0D\x0A\x00\x0D\x0A\x51\x55\x49\x54\x0A\x20". Really? The singnature is '\r\n\r\n\x00\r\nQUIT\n'? http://gerrit.openvpn.net/c/openvpn/+/685/comment/28038db2_443fb0fb : PS1, Line 108: What does this mean in the openvpn context? http://gerrit.openvpn.net/c/openvpn/+/685/comment/aff6de14_525b6ce9 : PS1, Line 115: DGRAM (0x2). if this is dgram, is the protocol still over tcp or does it use UDP then? File src/openvpn/socket.c: http://gerrit.openvpn.net/c/openvpn/+/685/comment/9d712c5d_d6c36d0f : PS1, Line 2684: /* check if it's a PROXY protocol header */ I don't think this is the right place to add this. This will be basically called on every link_socket_read_tcp and not just on the first packets. Also it seems to be enabled always so this might have side effects on non-haproxy operation. -- To view, visit http://gerrit.openvpn.net/c/openvpn/+/685?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: I9766c68feaba71279f26c3310eee4b5ca215e6ac Gerrit-Change-Number: 685 Gerrit-PatchSet: 1 Gerrit-Owner: ralf_lici <r...@mandelbit.com> Gerrit-Reviewer: flichtenheld <fr...@lichtenheld.com> Gerrit-Reviewer: plaisthos <arne-open...@rfc2549.org> Gerrit-CC: openvpn-devel <openvpn-devel@lists.sourceforge.net> Gerrit-Attention: ralf_lici <r...@mandelbit.com> Gerrit-Comment-Date: Fri, 26 Jul 2024 11:45:33 +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
