Skip to content

Conversation

@stoops
Copy link

@stoops stoops commented Nov 19, 2025

There was an update pushed earlier this year which undid some important logic in terms of learning/unlearning the ifconfig IP address. The older original code would check that the new/old IPs don't match first before performing that logic. I believe this was important and correct because it could cause connection interruptions during the learn/unlearn process if the IP address has stayed the same!

commit 5e4c9a69eaf9d32e85613bd71ed219fdbb062d34
Author: Marco Baffo <marco@mandelbit.com>
Date:   Fri Oct 17 22:19:12 2025 +0200
...
-update_vhash(struct multi_context *m, struct multi_instance *mi, const char *old_ip, const char *old_ipv6)
+update_vhash(struct multi_context *m, struct multi_instance *mi, const char *new_ip, const char *new_ipv6)
 {
-    struct in_addr addr;
-    struct in6_addr new_ipv6;
-
-    if ((mi->context.options.ifconfig_local && (!old_ip || strcmp(old_ip, mi->context.options.ifconfig_local)))
-        && inet_pton(AF_INET, mi->context.options.ifconfig_local, &addr) == 1)
+    if (new_ip)
     {

@cron2
Copy link
Contributor

cron2 commented Nov 19, 2025

As it has been discussed on the mailing list, this is currently not seen as a desirable patch, in the pre-2.7 runup.

This said, to consider this in the port-2.7 phase, it would need to be reworked

  • the repetition of if (addr_stat == 1 && new_addr_t != old_addr_t does not make anyhting more readable
  • it might actually not work correctly, if there is no new_ip at all (PUSH_REPLY -ifconfig) and addr_stat ends up being 0, so not removing the old ip - this is a valid use case
  • IPv6 is not covered at all

@stoops
Copy link
Author

stoops commented Nov 19, 2025

As it has been discussed on the mailing list, this is currently not seen as a desirable patch, in the pre-2.7 runup.

This said, to consider this in the port-2.7 phase, it would need to be reworked

  • the repetition of if (addr_stat == 1 && new_addr_t != old_addr_t does not make anyhting more readable
  • it might actually not work correctly, if there is no new_ip at all (PUSH_REPLY -ifconfig) and addr_stat ends up being 0, so not removing the old ip - this is a valid use case
  • IPv6 is not covered at all

Thanks, I did miss those cases you mentioned - I admit I am not an IPv6 expert also but anyway, its worth a try for me to re-work the logic to see if I can make it better (or if the devs there who know more than me could redo it in a more proper style than what I could do). It might take me some time. :)

@stoops stoops force-pushed the old-new branch 2 times, most recently from d8115b7 to dbbcd77 Compare November 19, 2025 17:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants