Attention is currently required from: cron2, flichtenheld, ordex, plaisthos.

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

Change subject: PUSH_UPDATE server: remove old IP(s) from vhash after sending a 
message containing ifconfig(-ipv6)
......................................................................


Patch Set 6:

(2 comments)

File src/openvpn/multi.c:

http://gerrit.openvpn.net/c/openvpn/+/1253/comment/fc5a283e_68bf4b30?usp=email :
PS5, Line 4390:             mi->context.c2.push_ifconfig_local = 
htonl(new_addr.s_addr);
> something is weird here. […]
yes I printed bit by bit and they are the same and I don't understand why htonl 
fixes things and I don't understand why (addr.s_addr == htonl(add.s_addr)) 
statement is false.

uint32

2025-10-14 17:40:06 us=422285 MANAGEMENT: CMD 'push-update-cid 0 "ifconfig 
10.10.10.55 255.255.255.0"'
2025-10-14 17:40:06 us=422386 SENT CONTROL [client0]: 'PUSH_UPDATE,ifconfig 
10.10.10.55 255.255.255.0' (status=1)
2025-10-14 17:40:06 us=422409 MULTI: Unlearn: 10.10.10.2 -> 
client0/udp6:92.168.3.1:37467
2025-10-14 17:40:06 us=422422 MULTI: Learn: 10.10.10.55 -> 
client0/udp6:92.168.3.1:37467


2025-10-14 17:40:06 us=422432 New IPv4 Address Analysis:
2025-10-14 17:40:06 us=422437   Original s_addr:     0x370a0a0a = 923404810 = 
00110111000010100000101000001010
2025-10-14 17:40:06 us=422442   ntohl(s_addr):       0x0a0a0a37 = 168430135 = 
00001010000010100000101000110111
2025-10-14 17:40:06 us=422448   htonl(s_addr):       0x0a0a0a37 = 168430135 = 
00001010000010100000101000110111
2025-10-14 17:40:06 us=422454   As IP address: 10.10.10.55

uint64

2025-10-14 17:49:28 us=407391 MANAGEMENT: CMD 'push-update-cid 0 "ifconfig 
10.10.10.55 255.255.255.0"'
2025-10-14 17:49:28 us=407531 SENT CONTROL [client0]: 'PUSH_UPDATE,ifconfig 
10.10.10.55 255.255.255.0' (status=1)
2025-10-14 17:49:28 us=407565 MULTI: Unlearn: 10.10.10.2 -> 
client0/udp6:92.168.3.1:37923
2025-10-14 17:49:28 us=407581 MULTI: Learn: 10.10.10.55 -> 
client0/udp6:92.168.3.1:37923


2025-10-14 17:49:28 us=407592 New IPv4 Address Analysis:
2025-10-14 17:49:28 us=407600   Original s_addr:     0x370a0a0a = 923404810 = 
0000000000000000000000000000000000110111000010100000101000001010
2025-10-14 17:49:28 us=407606   ntohl(s_addr):       0x0a0a0a37 = 168430135 = 
0000000000000000000000000000000000001010000010100000101000110111
2025-10-14 17:49:28 us=407613   htonl(s_addr):       0x0a0a0a37 = 168430135 = 
0000000000000000000000000000000000001010000010100000101000110111
2025-10-14 17:49:28 us=407619   As IP address: 10.10.10.55


File src/openvpn/push_util.c:

http://gerrit.openvpn.net/c/openvpn/+/1253/comment/bd5cee56_ee31112f?usp=email :
PS6, Line 165:     o.ifconfig_local = canary;
> in other parts of the code we use having a bool named like `$option_defined` 
> (in this case it would  […]
the canary is mostly to be less invasive as possible. I know it's a bit weird 
but it works well and minimizes the scope for this logic without having to 
touch other functions that are also used in other parts of the code in 
different logic. Adding ifconfig_local_defined to the options struct and 
modifying add_option() and remove_option(), just to handle -ifconfig(-ipv6) in 
the server looks a little overkill to me.



--
To view, visit http://gerrit.openvpn.net/c/openvpn/+/1253?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: I07a8ddd9026eef64b6f5abde98702a9801616a5f
Gerrit-Change-Number: 1253
Gerrit-PatchSet: 6
Gerrit-Owner: mrbff <[email protected]>
Gerrit-Reviewer: cron2 <[email protected]>
Gerrit-Reviewer: flichtenheld <[email protected]>
Gerrit-Reviewer: plaisthos <[email protected]>
Gerrit-CC: openvpn-devel <[email protected]>
Gerrit-CC: ordex <[email protected]>
Gerrit-Attention: plaisthos <[email protected]>
Gerrit-Attention: cron2 <[email protected]>
Gerrit-Attention: flichtenheld <[email protected]>
Gerrit-Attention: ordex <[email protected]>
Gerrit-Comment-Date: Tue, 14 Oct 2025 16:05:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: ordex <[email protected]>
_______________________________________________
Openvpn-devel mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/openvpn-devel

Reply via email to