[M] Change in openvpn[master]: multipeer: introduce asymmetric peer-id

"cron2 \(Code Review\) via Openvpn-devel" <[email protected]>
Newsgroups gmane.network.openvpn.devel
Message-ID <7c1275e4fa13744c6ff76220b878e4f6f1a3d663-EmailReviewComments-HTML@gerrit.openvpn.net>
Attention is currently required from: flichtenheld, its_Giaan.

cron2 has posted comments on this change by its_Giaan. ( http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email )

Change subject: multipeer: introduce asymmetric peer-id
......................................................................


Patch Set 18:

(6 comments)

Patchset:

PS18: 
this needs more work to make the logic not even worse than it is already


File src/openvpn/dco.c:

http://gerrit.openvpn.net/c/openvpn/+/1089/comment/e24ee2fa_4cf1aa90?usp=email :
PS18, Line 693:         dco_win_add_iroute_ipv6(&c->c1.tuntap->dco, addr->v6.addr, addr->netbits, c->c2.tls_multi->rx_peer_id);
is this unwrapping intentional?


File src/openvpn/init.c:

http://gerrit.openvpn.net/c/openvpn/+/1089/comment/0f1d8f42_e233cf1b?usp=email :
PS18, Line 2695:             c->c2.tls_multi->tx_peer_id = c->options.peer_id;
why set `tx_peer_id` twice, to the same value?


File src/openvpn/push.c:

http://gerrit.openvpn.net/c/openvpn/+/1089/comment/ddd6871f_ae9c28e6?usp=email :
PS18, Line 667:                         tls_multi->rx_peer_id);
was this wrapping intentional? it makes reading the diff harder...


File src/openvpn/ssl.c:

http://gerrit.openvpn.net/c/openvpn/+/1089/comment/446bf172_079ab262?usp=email :
PS18, Line 2079:                   : write_empty_string(buf)))
please do not call functions with side effects as part of a ternary condition in an if() - it's really hard to follow what this does


File src/openvpn/ssl_ncp.c:

http://gerrit.openvpn.net/c/openvpn/+/1089/comment/b36f67ed_37e3a663?usp=email :
PS18, Line 457:             multi->tx_peer_id = 0x76706e; /* 'v' 'p' 'n' */
setting the magic number for p2p peer-id should be done once.  The way this is written now, with the new condition further up, it's increasingly confusing when and why this would be set.



-- 
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1089?usp=email
To unsubscribe, or for help writing mail filters, visit http://gerrit.openvpn.net/settings?usp=email

Gerrit-MessageType: comment
Gerrit-Project: openvpn
Gerrit-Branch: master
Gerrit-Change-Id: I0a13ee90b6706acf20eabcee3bab3f2dff639bf9
Gerrit-Change-Number: 1089
Gerrit-PatchSet: 18
Gerrit-Owner: its_Giaan <[email protected]>
Gerrit-Reviewer: cron2 <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-Attention: its_Giaan <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-Comment-Date: Fri, 17 Jul 2026 21:31:48 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No

_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.