From 7a1c1fef7677e68d5a92c8538eda74a45b47a3c9 Mon Sep 17 00:00:00 2001
From: Jernej Jakob <jernej.jakob@gmail.com>
Date: Tue, 5 May 2020 15:48:47 +0200
Subject: configdict: T2427: do not remove all addresses when disabling
 interface

Commit 3fdf0093a introduced code that removed all addresses from an
interface when that interface is disabled. This is wrong, as other
configured services may be listening on these addresses and may fail to
start if their configured address isn't present.

It also caused a commit error when applying dhcp-server configuration:

    DHCP server configuration error!
    None of configured DHCP subnets does not have appropriate
    primary IP address on any broadcast interface.

This commit reverts it to prior behavior, which was to just put the
interface admin down and leave all addresses configured, other than the
IPv6 'fe80::EUI-64/64' link-local, which it deletes, as the interface
may not have a MAC if it's put down.
---
 python/vyos/configdict.py | 55 ++++++++++++++++++-----------------------------
 1 file changed, 21 insertions(+), 34 deletions(-)

diff --git a/python/vyos/configdict.py b/python/vyos/configdict.py
index 5ab831258..ceacf86fc 100644
--- a/python/vyos/configdict.py
+++ b/python/vyos/configdict.py
@@ -201,10 +201,6 @@ def intf_to_dict(conf, default):
     intf = deepcopy(default)
     intf['intf'] = ifname_from_config(conf)
 
-    # retrieve configured interface addresses
-    if conf.exists('address'):
-        intf['address'] = conf.return_values('address')
-
     # retrieve interface description
     if conf.exists('description'):
         intf['description'] = conf.return_value('description')
@@ -258,10 +254,6 @@ def intf_to_dict(conf, default):
     if conf.exists('ipv6 address autoconf'):
         intf['ipv6_autoconf'] = 1
 
-    # Get prefixes for IPv6 addressing based on MAC address (EUI-64)
-    if conf.exists('ipv6 address eui64'):
-        intf['ipv6_eui64_prefix'] = conf.return_values('ipv6 address eui64')
-
     # Disable IPv6 forwarding on this interface
     if conf.exists('ipv6 disable-forwarding'):
         intf['ipv6_forwarding'] = 0
@@ -304,49 +296,44 @@ def intf_to_dict(conf, default):
         if intf['ingress_qos'] != conf.return_effective_value('ingress-qos'):
             intf['ingress_qos_changed'] = True
 
-    disabled = disable_state(conf)
+    # Get the interface addresses
+    intf['address'] = conf.return_values('address')
 
-    # Get the interface IPs
-    eff_addr = conf.return_effective_values('address')
-    act_addr = conf.return_values('address')
+    # addresses to remove - difference between effective and working config
+    intf['address_remove'] = list_diff(
+            conf.return_effective_values('address'),
+            intf['address']
+            )
 
     # Get prefixes for IPv6 addressing based on MAC address (EUI-64)
-    eff_eui = conf.return_effective_values('ipv6 address eui64')
-    act_eui = conf.return_values('ipv6 address eui64')
+    intf['ipv6_eui64_prefix'] = conf.return_values('ipv6 address eui64')
+
+    # EUI64 to remove - difference between effective and working config
+    intf['ipv6_eui64_prefix_remove'] = list_diff(
+            conf.return_effective_values('ipv6 address eui64'),
+            intf['ipv6_eui64_prefix']
+            )
 
-    # Determine what should stay or be removed
+    # Determine if the interface should be disabled
+    disabled = disable_state(conf)
     if disabled == disable.both:
         # was and is still disabled
         intf['disable'] = True
-        intf['address'] = []
-        intf['address_remove'] = []
-        intf['ipv6_eui64_prefix'] = []
-        intf['ipv6_eui64_prefix_remove'] = []
     elif disabled == disable.now:
         # it is now disable but was not before
         intf['disable'] = True
-        intf['address'] = []
-        intf['address_remove'] = eff_addr
-        intf['ipv6_eui64_prefix'] = []
-        intf['ipv6_eui64_prefix_remove'] = eff_eui
     elif disabled == disable.was:
         # it was disable but not anymore
         intf['disable'] = False
-        intf['address'] = act_addr
-        intf['address_remove'] = []
-        intf['ipv6_eui64_prefix'] = act_eui
-        intf['ipv6_eui64_prefix_remove'] = []
     else:
         # normal change
         intf['disable'] = False
-        intf['address'] = act_addr
-        intf['address_remove'] = list_diff(eff_addr, act_addr)
-        intf['ipv6_eui64_prefix'] = act_eui
-        intf['ipv6_eui64_prefix_remove'] = list_diff(eff_eui, act_eui)
 
-    # Remove the default link-local address if set or if member of a bridge
+    # Remove the default link-local address if no-default-link-local is set,
+    # if member of a bridge or if disabled (it may not have a MAC if it's down)
     if ( conf.exists('ipv6 address no-default-link-local')
-            or intf.get('is_bridge_member') ):
+            or intf.get('is_bridge_member')
+            or intf['disable'] ):
         intf['ipv6_eui64_prefix_remove'].append('fe80::/64')
     else:
         # add the link-local by default to make IPv6 work
@@ -358,7 +345,7 @@ def intf_to_dict(conf, default):
         if intf['mac'] and intf['mac'] != interface.get_mac():
             intf['ipv6_eui64_prefix_remove'] += intf['ipv6_eui64_prefix']
     except Exception:
-        # If the interface does not exists, it can not have changed
+        # If the interface does not exist, it could not have changed
         pass
 
     return intf, disable
-- 
cgit v1.2.3