bgp: T3741: "parameter default no-ipv4-unicast" is now a default option

5 files changed, 154 insertions, 11 deletions

diff --git a/data/templates/frr/bgpd.frr.tmpl b/data/templates/frr/bgpd.frr.tmpl
index 27a2b98a5..a35930c93 100644
--- a/data/templates/frr/bgpd.frr.tmpl
+++ b/data/templates/frr/bgpd.frr.tmpl
@@ -230,10 +230,8 @@ router bgp {{ local_as }} {{ 'vrf ' ~ vrf if vrf is defined and vrf is not none
 {% else %}
  no bgp ebgp-requires-policy
 {% endif %}
-{% if parameters is defined and parameters.default is defined and parameters.default.no_ipv4_unicast is defined %}
 {#   Option must be set before any neighbor - see https://phabricator.vyos.net/T3463 #}
  no bgp default ipv4-unicast
-{% endif %}
 {# Workaround for T2100 until we have decided about a migration script #}
  no bgp network import-check
 {% if address_family is defined and address_family is not none %}
diff --git a/interface-definitions/include/bgp/protocol-common-config.xml.i b/interface-definitions/include/bgp/protocol-common-config.xml.i
index 30033bc50..2dfae517e 100644
--- a/interface-definitions/include/bgp/protocol-common-config.xml.i
+++ b/interface-definitions/include/bgp/protocol-common-config.xml.i
@@ -1253,12 +1253,6 @@
             </constraint>
           </properties>
         </leafNode>
-        <leafNode name="no-ipv4-unicast">
-          <properties>
-            <help>Deactivate IPv4 unicast for a peer by default</help>
-            <valueless/>
-          </properties>
-        </leafNode>
       </children>
     </node>
     <leafNode name="deterministic-med">
diff --git a/smoketest/configs/bgp-small-ipv4-unicast b/smoketest/configs/bgp-small-ipv4-unicast
new file mode 100644
index 000000000..a4dcd6218
--- /dev/null
+++ b/smoketest/configs/bgp-small-ipv4-unicast
@@ -0,0 +1,77 @@
+interfaces {
+    ethernet eth0 {
+        address 192.0.2.1
+        address 2001:db8::1/64
+    }
+    loopback lo {
+    }
+}
+protocols {
+    bgp 65001 {
+        address-family {
+            ipv4-unicast {
+                network 10.0.150.0/23 {
+                }
+            }
+            ipv6-unicast {
+                network 2001:db8:200::/40 {
+                }
+            }
+        }
+        neighbor 192.0.2.10 {
+            remote-as 65010
+        }
+        neighbor 192.0.2.11 {
+            remote-as 65011
+        }
+        neighbor 2001:db8::10 {
+            remote-as 65010
+        }
+        neighbor 2001:db8::11 {
+            remote-as 65011
+        }
+        parameters {
+            log-neighbor-changes
+        }
+    }
+}
+service {
+    ssh {
+        disable-host-validation
+        port 22
+    }
+}
+system {
+    config-management {
+        commit-revisions 200
+    }
+    console {
+        device ttyS0 {
+            speed 115200
+        }
+    }
+    domain-name vyos.net
+    host-name vyos
+    login {
+        user vyos {
+            authentication {
+                encrypted-password $6$O5gJRlDYQpj$MtrCV9lxMnZPMbcxlU7.FI793MImNHznxGoMFgm3Q6QP3vfKJyOSRCt3Ka/GzFQyW1yZS4NS616NLHaIPPFHc0
+                plaintext-password ""
+            }
+        }
+    }
+    syslog {
+        global {
+            facility all {
+                level notice
+            }
+            facility protocols {
+                level debug
+            }
+        }
+    }
+}
+
+/* Warning: Do not remove the following line. */
+/* === vyatta-config-version: "broadcast-relay@1:cluster@1:config-management@1:conntrack-sync@1:conntrack@1:dhcp-relay@2:dhcp-server@5:dns-forwarding@1:firewall@5:ipsec@5:l2tp@1:mdns@1:nat@4:ntp@1:pptp@1:qos@1:quagga@6:snmp@1:ssh@1:system@9:vrrp@2:wanloadbalance@3:webgui@1:webproxy@1:webproxy@2:zone-policy@1" === */
+/* Release version: 1.2.5 */
diff --git a/smoketest/scripts/cli/test_protocols_bgp.py b/smoketest/scripts/cli/test_protocols_bgp.py
index 29b5aa9d1..16284ed01 100755
--- a/smoketest/scripts/cli/test_protocols_bgp.py
+++ b/smoketest/scripts/cli/test_protocols_bgp.py
@@ -221,8 +221,6 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
 
         # Default local preference (higher = more preferred, default value is 100)
         self.cli_set(base_path + ['parameters', 'default', 'local-pref', local_pref])
-        # Deactivate IPv4 unicast for a peer by default
-        self.cli_set(base_path + ['parameters', 'default', 'no-ipv4-unicast'])
         self.cli_set(base_path + ['parameters', 'graceful-restart', 'stalepath-time', stalepath_time])
         self.cli_set(base_path + ['parameters', 'graceful-shutdown'])
         self.cli_set(base_path + ['parameters', 'ebgp-requires-policy'])
@@ -246,7 +244,6 @@ class TestProtocolsBGP(VyOSUnitTestSHIM.TestCase):
         self.assertIn(f' bgp router-id {router_id}', frrconfig)
         self.assertIn(f' bgp log-neighbor-changes', frrconfig)
         self.assertIn(f' bgp default local-preference {local_pref}', frrconfig)
-        self.assertIn(f' no bgp default ipv4-unicast', frrconfig)
         self.assertIn(f' bgp graceful-restart stalepath-time {stalepath_time}', frrconfig)
         self.assertIn(f' bgp graceful-shutdown', frrconfig)
         self.assertIn(f' bgp bestpath as-path multipath-relax', frrconfig)
diff --git a/src/migration-scripts/bgp/1-to-2 b/src/migration-scripts/bgp/1-to-2
new file mode 100755
index 000000000..4c6d5ceb8
--- /dev/null
+++ b/src/migration-scripts/bgp/1-to-2
@@ -0,0 +1,77 @@
+#!/usr/bin/env python3
+#
+# Copyright (C) 2021 VyOS maintainers and contributors
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 2 or later as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# T3741: no-ipv4-unicast is now enabled by default
+
+from sys import argv
+from sys import exit
+
+from vyos.configtree import ConfigTree
+from vyos.template import is_ipv4
+
+if (len(argv) < 1):
+    print("Must specify file name!")
+    exit(1)
+
+file_name = argv[1]
+
+with open(file_name, 'r') as f:
+    config_file = f.read()
+
+base = ['protocols', 'bgp']
+config = ConfigTree(config_file)
+
+if not config.exists(base):
+    # Nothing to do
+    exit(0)
+
+# This is now a default option - simply delete it.
+# As it was configured explicitly - we can also bail out early as we need to
+# do nothing!
+if config.exists(base + ['parameters', 'default', 'no-ipv4-unicast']):
+    config.delete(base + ['parameters', 'default', 'no-ipv4-unicast'])
+
+    # Check if the "default" node is now empty, if so - remove it
+    if len(config.list_nodes(base + ['parameters', 'default'])) == 0:
+        config.delete(base + ['parameters', 'default'])
+
+    # Check if the "default" node is now empty, if so - remove it
+    if len(config.list_nodes(base + ['parameters'])) == 0:
+        config.delete(base + ['parameters'])
+
+    exit(0)
+
+# As we now install a new default option into BGP we need to migrate all
+# existing BGP neighbors and restore the old behavior
+if config.exists(base + ['neighbor']):
+    for neighbor in config.list_nodes(base + ['neighbor']):
+        peer_group = base + ['neighbor', neighbor, 'peer-group']
+        if config.exists(peer_group):
+            peer_group_name = config.return_value(peer_group)
+            # peer group enables old behavior for neighbor - bail out
+            if config.exists(base + ['peer-group', peer_group_name, 'address-family', 'ipv4-unicast']):
+                continue
+
+        afi_ipv4 = base + ['neighbor', neighbor, 'address-family', 'ipv4-unicast']
+        if not config.exists(afi_ipv4):
+            config.set(afi_ipv4)
+
+try:
+    with open(file_name, 'w') as f:
+        f.write(config.to_string())
+except OSError as e:
+    print(f'Failed to save the modified config: {e}')
+    exit(1)