Merge pull request #694 from c-po/t3236-ospf

T3236: Rewrite of OSPF in XML and Python notation

10 files changed, 710 insertions, 99 deletions

diff --git a/Makefile b/Makefile
index a063d1df6..66b1e8bb7 100644
--- a/Makefile
+++ b/Makefile
@@ -47,7 +47,6 @@ interface_definitions: $(config_xml_obj)
 	rm -f $(TMPL_DIR)/vpn/node.def
 	rm -f $(TMPL_DIR)/vpn/ipsec/node.def
 	rm -rf $(TMPL_DIR)/vpn/nipsec
-	rm -rf $(TMPL_DIR)/protocols/nospf
 
 	# XXX: required until OSPF and RIP is migrated from vyatta-cfg-quagga to vyos-1x
 	mkdir $(TMPL_DIR)/interfaces/loopback/node.tag/ipv6
diff --git a/data/configd-include.json b/data/configd-include.json
index 8369f0e13..318ab0e14 100644
--- a/data/configd-include.json
+++ b/data/configd-include.json
@@ -34,6 +34,7 @@
 "protocols_igmp.py",
 "protocols_isis.py",
 "protocols_mpls.py",
+"protocols_ospf.py",
 "protocols_pim.py",
 "protocols_rip.py",
 "protocols_static_multicast.py",
diff --git a/data/templates/frr/ospf.frr.tmpl b/data/templates/frr/ospf.frr.tmpl
index 465034f15..07699290c 100644
--- a/data/templates/frr/ospf.frr.tmpl
+++ b/data/templates/frr/ospf.frr.tmpl
@@ -1,2 +1,131 @@
 !
+router ospf
+{% if access_list is defined and access_list is not none %}
+{%   for acl, acl_config in access_list.items() %}
+{%     for protocol in acl_config.export if acl_config.export is defined %}
+ distribute-list {{ acl }} out {{ protocol }}
+{%     endfor %}
+{%   endfor %}
+{% endif %}
+{% if area is defined and area is not none %}
+{%   for area_id, area_config in area.items() %}
+{%     if area_config.area_type is defined and area_config.area_type is not none %}
+{%       for type, type_config in area_config.area_type.items() if type != 'normal' %}
+ area {{ area_id }} {{ type }} {{ 'no-summary' if type_config.no_summary is defined }}
+{%         if type_config.default_cost is defined and type_config.default_cost is not none %}
+ area {{ area_id }} default-cost {{ type_config.default_cost }}
+{%         endif %}
+{%       endfor %}
+{%     endif %}
+{%     if area_config.authentication is defined and area_config.authentication is not none %}
+ area {{ area_id }} authentication {{ 'message-digest' if area_config.authentication == 'md5' }}
+{%     endif %}
+{%     for network in area_config.network if area_config.network is defined %}
+ network {{ network }} area {{ area_id }}
+{%     endfor %}
+{%     if area_config.range is defined and area_config.range is not none %}
+{%       for range, range_config in area_config.range.items() %}
+{%         if range_config.cost is defined and range_config.cost is not none %}
+ area {{ area_id }} range {{ range }} cost {{ range_config.cost }}
+{%         endif %}
+{%         if range_config.not_advertise is defined %}
+ area {{ area_id }} range {{ range }} not-advertise
+{%         endif %}
+{%         if range_config.substitute is defined and range_config.substitute is not none %}
+ area {{ area_id }} range {{ range }} substitute {{ range_config.substitute }}
+{%         endif %}
+{%       endfor %}
+{%     endif %}
+{%     if area_config.shortcut is defined and area_config.shortcut is not none %}
+ area {{ area_id }} shortcut {{ area_config.shortcut }}
+{%     endif %}
+{%     if area_config.virtual_link is defined and area_config.virtual_link is not none %}
+{%       for link, link_config in area_config.virtual_link.items() %}
+{%         if link_config.authentication is defined and link_config.authentication is not none %}
+{%           if link_config.authentication.plaintext_password is defined and link_config.authentication.plaintext_password is not none %}
+ area {{ area_id }} virtual-link {{ link }} authentication-key {{ link_config.authentication.plaintext_password }}
+{%           elif link_config.authentication.md5 is defined and link_config.authentication.md5.key_id is defined and link_config.authentication.md5.key_id is not none %}
+{%             for key, key_config in link_config.authentication.md5.key_id.items() %}
+ area {{ area_id }} virtual-link {{ link }} message-digest-key {{ key }} md5 {{ key_config.md5_key }}
+{%             endfor %}
+{%           endif %}
+{%         endif %}
+{%         if link_config.dead_interval is defined and link_config.dead_interval is not none %}
+{#           The following values are default values #}
+ area {{ area_id }} virtual-link {{ link }} hello-interval {{ link_config.hello_interval }} retransmit-interval {{ link_config.retransmit_interval }} transmit-delay {{ link_config.retransmit_interval }} dead-interval {{ link_config.dead_interval }}
+{%         endif %}
+{%       endfor %}
+{%     endif %}
+{%   endfor %}
+{% endif %}
+{% if auto_cost is defined and auto_cost.reference_bandwidth is defined and auto_cost.reference_bandwidth is not none %}
+ auto-cost reference-bandwidth {{ auto_cost.reference_bandwidth }}
+{% endif %}
+{% if default_information is defined and default_information.originate is defined and default_information.originate is not none %}
+ default-information originate {{ 'always' if default_information.originate.always is defined }} {{ 'metric ' + default_information.originate.metric if default_information.originate.metric is defined }} {{ 'metric-type ' + default_information.originate.metric_type if default_information.originate.metric_type is defined }} {{ 'route-map ' + default_information.originate.route_map if default_information.originate.route_map is defined }}
+{% endif %}
+{% if default_metric is defined and default_metric is not none %}
+ default-metric {{ default_metric }}
+{% endif %}
+{% if distance is defined and distance is not none %}
+{%   if distance.global is defined and distance.global is not none %}
+ distance {{ distance.global }}
+{%   endif %}
+{%   if distance.ospf is defined and distance.ospf is not none %}
+ distance ospf {{ 'intra-area ' + distance.ospf.intra_area if distance.ospf.intra_area is defined }} {{ 'inter-area ' + distance.ospf.inter_area if distance.ospf.inter_area is defined }} {{ 'external ' + distance.ospf.external if distance.ospf.external is defined }}
+{%   endif %}
+{% endif %}
+{% if log_adjacency_changes is defined %}
+ log-adjacency-changes {{ "detail" if log_adjacency_changes.detail is defined }}
+{% endif %}
+{% if max_metric is defined and max_metric.router_lsa is defined and max_metric.router_lsa is not none %}
+{%   if max_metric.router_lsa.administrative is defined %}
+ max-metric router-lsa administrative
+{%   endif %}
+{%   if max_metric.router_lsa.on_shutdown is defined and max_metric.router_lsa.on_shutdown is not none %}
+ max-metric router-lsa on-shutdown {{ max_metric.router_lsa.on_shutdown }}
+{%   endif %}
+{%   if max_metric.router_lsa.on_startup is defined and max_metric.router_lsa.on_startup is not none %}
+ max-metric router-lsa on-startup {{ max_metric.router_lsa.on_startup }}
+{%   endif %}
+{% endif %}
+{% if mpls_te is defined and mpls_te.enable is defined %}
+ mpls-te on
+ mpls-te router-address {{ mpls_te.router_address }}
+{% endif %}
+{% if neighbor is defined and neighbor is not none%}
+{%   for address, address_config in neighbor.items() %}
+ neighbor {{ address }} {{ 'priority ' + address_config.priority if address_config.priority is defined }} {{ 'poll-interval ' + address_config.poll_interval if address_config.poll_interval is defined }}
+{%   endfor %}
+{% endif %}
+{% if parameters is defined and parameters is not none %}
+{%   if parameters.abr_type is defined and parameters.abr_type is not none %}
+ ospf abr-type {{ parameters.abr_type }}
+{%   endif %}
+{%   if parameters.router_id is defined and parameters.router_id is not none %}
+ ospf router-id {{ parameters.router_id }}
+{%   endif %}
+{% endif %}
+{% for interface in passive_interface if passive_interface is defined %}
+ passive-interface {{ interface }}
+{% endfor %}
+{% for interface in passive_interface_exclude if passive_interface_exclude is defined %}
+ no passive-interface {{ interface }}
+{% endfor %}
+{% if redistribute is defined and redistribute is not none %}
+{%   for protocol, options in redistribute.items() %}
+ redistribute {{ protocol }} {{ 'metric ' + options.metric if options.metric is defined }} {{ 'metric-type ' + options.metric_type if options.metric_type is defined }} {{ 'route-map ' + options.route_map if options.route_map is defined }}
+{%   endfor %}
+{% endif %}
+{% if refresh is defined and refresh.timers is defined and refresh.timers is not none %}
+ refresh timer {{ refresh.timers }}
+{% endif %}
+{% if timers is defined and timers.throttle is defined and timers.throttle.spf is defined and timers.throttle.spf is not none %}
+{#   Timer values have default values #}
+ timers throttle spf {{ timers.throttle.spf.delay }} {{ timers.throttle.spf.initial_holdtime }} {{ timers.throttle.spf.max_holdtime }}
+{% endif %}
+!
+{% if route_map is defined and route_map is not none %}
+ip protocol ospf route-map {{ route_map }}
+{% endif %}
 !
diff --git a/interface-definitions/include/ospf-metric-type.xml.i b/interface-definitions/include/ospf-metric-type.xml.i
new file mode 100644
index 000000000..50f11960c
--- /dev/null
+++ b/interface-definitions/include/ospf-metric-type.xml.i
@@ -0,0 +1,15 @@
+<!-- included start from ospf-metric-type.xml.i -->
+<leafNode name="metric-type">
+  <properties>
+    <help>OSPF metric type for default routes (default: 2)</help>
+    <valueHelp>
+      <format>u32:1-2</format>
+      <description>Metric type for default routes</description>
+    </valueHelp>
+    <constraint>
+      <validator name="numeric" argument="--range 1-2"/>
+    </constraint>
+  </properties>
+  <defaultValue>2</defaultValue>
+</leafNode>
+<!-- included end -->
diff --git a/interface-definitions/include/ospf-metric.xml.i b/interface-definitions/include/ospf-metric.xml.i
index 771fda02d..3ce12e877 100644
--- a/interface-definitions/include/ospf-metric.xml.i
+++ b/interface-definitions/include/ospf-metric.xml.i
@@ -11,23 +11,4 @@
     </constraint>
   </properties>
 </leafNode>
-<leafNode name="metric-type">
-  <properties>
-    <help>OSPF metric type for default routes</help>
-    <valueHelp>
-      <format>u32:1-2</format>
-      <description>Metric type for default routes (default 2)</description>
-    </valueHelp>
-    <constraint>
-      <validator name="numeric" argument="--range 1-2"/>
-    </constraint>
-  </properties>
-</leafNode>
-<leafNode name="route-map">
-  <properties>
-    <help>Route map reference</help>
-    <completionHelp>
-      <path>policy route-map</path>
-    </completionHelp>
-  </properties>
-</leafNode>
+<!-- included end -->
diff --git a/interface-definitions/include/ospf-route-map.xml.i b/interface-definitions/include/ospf-route-map.xml.i
new file mode 100644
index 000000000..8dc5b37da
--- /dev/null
+++ b/interface-definitions/include/ospf-route-map.xml.i
@@ -0,0 +1,10 @@
+<!-- included start from ospf-route-map.xml.i -->
+<leafNode name="route-map">
+  <properties>
+    <help>Route map reference</help>
+    <completionHelp>
+      <path>policy route-map</path>
+    </completionHelp>
+  </properties>
+</leafNode>
+<!-- included end -->
diff --git a/interface-definitions/protocols-ospf.xml.in b/interface-definitions/protocols-ospf.xml.in
index 4d5b84be0..074d0db63 100644
--- a/interface-definitions/protocols-ospf.xml.in
+++ b/interface-definitions/protocols-ospf.xml.in
@@ -3,15 +3,18 @@
 <interfaceDefinition>
   <node name="protocols">
     <children>
-      <node name="nospf" owner="${vyos_conf_scripts_dir}/protocols_ospf.py">
+      <node name="ospf" owner="${vyos_conf_scripts_dir}/protocols_ospf.py">
         <properties>
-          <help>Open Shortest Path First protocol (OSPF) parameters</help>
+          <help>Open Shortest Path First (OSPF)</help>
           <priority>620</priority>
         </properties>
         <children>
           <tagNode name="access-list">
             <properties>
               <help>Access list to filter networks in routing updates</help>
+              <completionHelp>
+                <path>policy access-list</path>
+              </completionHelp>
               <valueHelp>
                 <format>u32</format>
                 <description>Access-list number</description>
@@ -109,7 +112,7 @@
                       </leafNode>
                       <leafNode name="translate">
                         <properties>
-                          <help>Nssa-abr</help>
+                          <help>Configure NSSA-ABR (default: candidate)</help>
                           <completionHelp>
                             <list>always candidate never</list>
                           </completionHelp>
@@ -129,6 +132,7 @@
                             <regex>^(always|candidate|never)$</regex>
                           </constraint>
                         </properties>
+                        <defaultValue>candidate</defaultValue>
                       </leafNode>
                     </children>
                   </node>
@@ -194,6 +198,13 @@
               <tagNode name="range">
                 <properties>
                   <help>Summarize routes matching prefix (border routers only)</help>
+                  <valueHelp>
+                    <format>ipv4net</format>
+                    <description>Area range prefix</description>
+                  </valueHelp>
+                  <constraint>
+                    <validator name="ipv4-prefix"/>
+                  </constraint>
                 </properties>
                 <children>
                   <leafNode name="cost">
@@ -254,6 +265,14 @@
               <tagNode name="virtual-link">
                 <properties>
                   <help>Virtual link</help>
+                  <valueHelp>
+                    <format>ipv4</format>
+                    <description>OSPF area in dotted decimal notation</description>
+                  </valueHelp>
+                  <constraint>
+                    <validator name="numeric" argument="--range 0-4294967295"/>
+                    <validator name="ip-address"/>
+                  </constraint>
                 </properties>
                 <children>
                   <node name="authentication">
@@ -301,7 +320,7 @@
                   </node>
                   <leafNode name="dead-interval">
                     <properties>
-                      <help>Interval after which a neighbor is declared dead</help>
+                      <help>Interval after which a neighbor is declared dead (default: 40)</help>
                       <valueHelp>
                         <format>u32:1-65535</format>
                         <description>Neighbor dead interval (seconds)</description>
@@ -310,10 +329,11 @@
                         <validator name="numeric" argument="--range 1-65535"/>
                       </constraint>
                     </properties>
+                    <defaultValue>40</defaultValue>
                   </leafNode>
                   <leafNode name="hello-interval">
                     <properties>
-                      <help>Interval between hello packets</help>
+                      <help>Interval between hello packets (default: 10)</help>
                       <valueHelp>
                         <format>u32:1-65535</format>
                         <description>Hello interval (seconds)</description>
@@ -322,10 +342,11 @@
                         <validator name="numeric" argument="--range 1-65535"/>
                       </constraint>
                     </properties>
+                    <defaultValue>10</defaultValue>
                   </leafNode>
                   <leafNode name="retransmit-interval">
                     <properties>
-                      <help>Interval between retransmitting lost link state advertisements</help>
+                      <help>Interval between retransmitting lost link state advertisements (default: 5)</help>
                       <valueHelp>
                         <format>u32:1-65535</format>
                         <description>Retransmit interval (seconds)</description>
@@ -334,10 +355,11 @@
                         <validator name="numeric" argument="--range 1-65535"/>
                       </constraint>
                     </properties>
+                    <defaultValue>5</defaultValue>
                   </leafNode>
                   <leafNode name="transmit-delay">
                     <properties>
-                      <help>Link state transmit delay</help>
+                      <help>Link state transmit delay (default: 1)</help>
                       <valueHelp>
                         <format>u32:1-65535</format>
                         <description>Link state transmit delay (seconds)</description>
@@ -346,6 +368,7 @@
                         <validator name="numeric" argument="--range 1-65535"/>
                       </constraint>
                     </properties>
+                    <defaultValue>1</defaultValue>
                   </leafNode>
                 </children>
               </tagNode>
@@ -353,7 +376,7 @@
           </tagNode>
           <node name="auto-cost">
             <properties>
-              <help>Calculate OSPF interface cost according to bandwidth</help>
+              <help>Calculate OSPF interface cost according to bandwidth (default: 100)</help>
             </properties>
             <children>
               <leafNode name="reference-bandwidth">
@@ -361,12 +384,13 @@
                   <help>Reference bandwidth method to assign OSPF cost</help>
                   <valueHelp>
                     <format>u32:1-4294967</format>
-                    <description>Reference bandwidth cost in Mbits/sec (default 100)</description>
+                    <description>Reference bandwidth cost in Mbits/sec</description>
                   </valueHelp>
                   <constraint>
                     <validator name="numeric" argument="--range 1-4294967"/>
                   </constraint>
                 </properties>
+                <defaultValue>100</defaultValue>
               </leafNode>
             </children>
           </node>
@@ -386,38 +410,9 @@
                       <valueless/>
                     </properties>
                   </leafNode>
-                  <leafNode name="metric">
-                    <properties>
-                      <help>OSPF default metric</help>
-                      <valueHelp>
-                        <format>u32:0-16777214</format>
-                        <description>Default metric</description>
-                      </valueHelp>
-                      <constraint>
-                        <validator name="numeric" argument="--range 0-16777214"/>
-                      </constraint>
-                    </properties>
-                  </leafNode>
-                  <leafNode name="metric-type">
-                    <properties>
-                      <help>OSPF metric type for default routes</help>
-                      <valueHelp>
-                        <format>u32:1-2</format>
-                        <description>Metric type for default routes (default 2)</description>
-                      </valueHelp>
-                      <constraint>
-                        <validator name="numeric" argument="--range 1-2"/>
-                      </constraint>
-                    </properties>
-                  </leafNode>
-                  <leafNode name="route-map">
-                    <properties>
-                      <help>Route map reference</help>
-                      <completionHelp>
-                        <path>policy route-map</path>
-                      </completionHelp>
-                    </properties>
-                  </leafNode>
+                  #include <include/ospf-metric.xml.i>
+                  #include <include/ospf-metric-type.xml.i>
+                  #include <include/ospf-route-map.xml.i>
                 </children>
               </node>
             </children>
@@ -529,11 +524,11 @@
                     <properties>
                       <help>Advertise stub-router prior to full shutdown of OSPF</help>
                       <valueHelp>
-                        <format>u32:5-86400</format>
+                        <format>u32:5-100</format>
                         <description>Time (seconds) to advertise self as stub-router</description>
                       </valueHelp>
                       <constraint>
-                        <validator name="numeric" argument="--range 5-86400"/>
+                        <validator name="numeric" argument="--range 5-100"/>
                       </constraint>
                     </properties>
                   </leafNode>
@@ -575,41 +570,47 @@
                     <validator name="ipv4-address"/>
                   </constraint>
                 </properties>
+                <defaultValue>0.0.0.0</defaultValue>
               </leafNode>
             </children>
           </node>
           <tagNode name="neighbor">
             <properties>
-              <help>Neighbor IP address</help>
+              <help>Specify neighbor router</help>
               <valueHelp>
                 <format>ipv4</format>
                 <description>Neighbor IP address</description>
               </valueHelp>
+              <constraint>
+                <validator name="ipv4-address"/>
+              </constraint>
             </properties>
             <children>
               <leafNode name="poll-interval">
                 <properties>
-                  <help>Dead neighbor polling interval</help>
+                  <help>Dead neighbor polling interval (default: 60)</help>
                   <valueHelp>
                     <format>u32:1-65535</format>
-                    <description>Seconds between dead neighbor polling interval (default 60)</description>
+                    <description>Seconds between dead neighbor polling interval</description>
                   </valueHelp>
                   <constraint>
                     <validator name="numeric" argument="--range 1-65535"/>
                   </constraint>
                 </properties>
+                <defaultValue>60</defaultValue>
               </leafNode>
               <leafNode name="priority">
                 <properties>
-                  <help>Neighbor priority in seconds</help>
+                  <help>Neighbor priority in seconds (default: 0)</help>
                   <valueHelp>
                     <format>u32:0-255</format>
-                    <description>Neighbor priority (default 0)</description>
+                    <description>Neighbor priority</description>
                   </valueHelp>
                   <constraint>
                     <validator name="numeric" argument="--range 0-255"/>
                   </constraint>
                 </properties>
+                <defaultValue>0</defaultValue>
               </leafNode>
             </children>
           </tagNode>
@@ -620,7 +621,7 @@
             <children>
               <leafNode name="abr-type">
                 <properties>
-                  <help>OSPF ABR type</help>
+                  <help>OSPF ABR type (default: cisco)</help>
                   <completionHelp>
                     <list>cisco ibm shortcut standard</list>
                   </completionHelp>
@@ -644,6 +645,7 @@
                     <regex>^(cisco|ibm|shortcut|standard)$</regex>
                   </constraint>
                 </properties>
+                <defaultValue>cisco</defaultValue>
               </leafNode>
               <leafNode name="opaque-lsa">
                 <properties>
@@ -674,31 +676,37 @@
           <leafNode name="passive-interface">
             <properties>
               <help>Suppress routing updates on an interface</help>
+              <completionHelp>
+                <list>default</list>
+                <script>${vyos_completion_dir}/list_interfaces.py</script>
+              </completionHelp>
               <valueHelp>
-                <format>&lt;interface&gt;</format>
+                <format>txt</format>
                 <description>Interface to be passive (i.e. suppress routing updates)</description>
               </valueHelp>
               <valueHelp>
                 <format>default</format>
                 <description>Default to suppress routing updates on all interfaces</description>
               </valueHelp>
-              <completionHelp>
-                <list>default</list>
-                <script>${vyos_completion_dir}/list_interfaces.py</script>
-              </completionHelp>
+              <constraint>
+                <regex>^(br|bond|dum|en|eth|gnv|peth|tun|vti|vxlan|wg|wlan)[0-9]+|lo|default$</regex>
+              </constraint>
               <multi/>
             </properties>
           </leafNode>
           <leafNode name="passive-interface-exclude">
             <properties>
               <help>Interface to exclude when using 'passive-interface default'</help>
-              <valueHelp>
-                <format>&lt;interface&gt;</format>
-                <description>Interface to be passive (i.e. suppress routing updates)</description>
-              </valueHelp>
               <completionHelp>
                 <script>${vyos_completion_dir}/list_interfaces.py</script>
               </completionHelp>
+              <valueHelp>
+                <format>txt</format>
+                <description>Interface to be passive (i.e. suppress routing updates)</description>
+              </valueHelp>
+              <constraint>
+                <regex>^(br|bond|dum|en|eth|gnv|peth|tun|vti|vxlan|wg|wlan)[0-9]+|lo$</regex>
+              </constraint>
               <multi/>
             </properties>
           </leafNode>
@@ -713,6 +721,8 @@
                 </properties>
                 <children>
                   #include <include/ospf-metric.xml.i>
+                  #include <include/ospf-metric-type.xml.i>
+                  #include <include/ospf-route-map.xml.i>
                 </children>
               </node>
               <node name="connected">
@@ -721,6 +731,8 @@
                 </properties>
                 <children>
                   #include <include/ospf-metric.xml.i>
+                  #include <include/ospf-metric-type.xml.i>
+                  #include <include/ospf-route-map.xml.i>
                 </children>
               </node>
               <node name="kernel">
@@ -729,6 +741,8 @@
                 </properties>
                 <children>
                   #include <include/ospf-metric.xml.i>
+                  #include <include/ospf-metric-type.xml.i>
+                  #include <include/ospf-route-map.xml.i>
                 </children>
               </node>
               <node name="rip">
@@ -737,6 +751,8 @@
                 </properties>
                 <children>
                   #include <include/ospf-metric.xml.i>
+                  #include <include/ospf-metric-type.xml.i>
+                  #include <include/ospf-route-map.xml.i>
                 </children>
               </node>
               <node name="static">
@@ -745,6 +761,8 @@
                 </properties>
                 <children>
                   #include <include/ospf-metric.xml.i>
+                  #include <include/ospf-metric-type.xml.i>
+                  #include <include/ospf-route-map.xml.i>
                 </children>
               </node>
             </children>
@@ -768,14 +786,7 @@
               </leafNode>
             </children>
           </node>
-          <leafNode name="route-map">
-            <properties>
-              <help>Filter routes installed in local route map</help>
-              <completionHelp>
-                <path>policy route-map</path>
-              </completionHelp>
-            </properties>
-          </leafNode>
+          #include <include/ospf-route-map.xml.i>
           <node name="timers">
             <properties>
               <help>Adjust routing timers</help>
@@ -793,39 +804,42 @@
                     <children>
                       <leafNode name="delay">
                         <properties>
-                          <help>Delay (msec) from first change received till SPF calculation</help>
+                          <help>Delay from first change received till SPF calculation (default: 200)</help>
                           <valueHelp>
                             <format>u32:0-600000</format>
-                            <description>Delay in msec (default 200)</description>
+                            <description>Delay in milliseconds</description>
                           </valueHelp>
                           <constraint>
                             <validator name="numeric" argument="--range 0-600000"/>
                           </constraint>
                         </properties>
+                        <defaultValue>200</defaultValue>
                       </leafNode>
                       <leafNode name="initial-holdtime">
                         <properties>
-                          <help>Initial hold time(msec) between consecutive SPF calculations</help>
+                          <help>Initial hold time between consecutive SPF calculations (default: 1000)</help>
                           <valueHelp>
                             <format>u32:0-600000</format>
-                            <description>Initial hold time in msec (default 1000)</description>
+                            <description>Initial hold time in milliseconds</description>
                           </valueHelp>
                           <constraint>
                             <validator name="numeric" argument="--range 0-600000"/>
                           </constraint>
                         </properties>
+                        <defaultValue>1000</defaultValue>
                       </leafNode>
                       <leafNode name="max-holdtime">
                         <properties>
-                          <help>Maximum hold time (msec)</help>
+                          <help>Maximum hold time (default: 10000)</help>
                           <valueHelp>
                             <format>u32:0-600000</format>
-                            <description>Max hold time in msec (default 10000)</description>
+                            <description>Max hold time in milliseconds</description>
                           </valueHelp>
                           <constraint>
                             <validator name="numeric" argument="--range 0-600000"/>
                           </constraint>
                         </properties>
+                        <defaultValue>10000</defaultValue>
                       </leafNode>
                     </children>
                   </node>
diff --git a/smoketest/configs/ospf-config b/smoketest/configs/ospf-config
new file mode 100644
index 000000000..9e18e6155
--- /dev/null
+++ b/smoketest/configs/ospf-config
@@ -0,0 +1,115 @@
+interfaces {
+    dummy dum0 {
+        address 172.18.254.201/32
+    }
+    ethernet eth0 {
+        duplex auto
+        smp-affinity auto
+        speed auto
+        vif 201 {
+            address 172.18.201.10/24
+            ip {
+                ospf {
+                    authentication {
+                        md5 {
+                            key-id 10 {
+                                md5-key OSPFVyOSNET
+                            }
+                        }
+                    }
+                    dead-interval 40
+                    hello-interval 10
+                    priority 1
+                    retransmit-interval 5
+                    transmit-delay 1
+                }
+            }
+        }
+    }
+    ethernet eth1 {
+        duplex auto
+        smp-affinity auto
+        speed auto
+    }
+}
+protocols {
+    ospf {
+        area 0 {
+            network 172.18.201.0/24
+            network 172.18.254.201/32
+        }
+        log-adjacency-changes {
+        }
+        parameters {
+            abr-type cisco
+            router-id 172.18.254.201
+        }
+        passive-interface default
+        passive-interface-exclude eth0.201
+    }
+    static {
+        route 0.0.0.0/0 {
+            next-hop 172.18.201.254 {
+                distance 10
+            }
+        }
+    }
+}
+service {
+    lldp {
+        interface all {
+        }
+    }
+    snmp {
+        community public {
+            authorization ro
+            network 172.16.100.0/24
+        }
+        contact "VyOS maintainers and contributors <maintainers@vyos.io>"
+        location "Jenkins"
+    }
+    ssh {
+        disable-host-validation
+        port 22
+    }
+}
+system {
+    config-management {
+        commit-revisions 200
+    }
+    domain-name vyos.net
+    host-name vyos
+    login {
+        user vyos {
+            authentication {
+                encrypted-password $6$2Ta6TWHd/U$NmrX0x9kexCimeOcYK1MfhMpITF9ELxHcaBU/znBq.X2ukQOj61fVI2UYP/xBzP4QtiTcdkgs7WOQMHWsRymO/
+                plaintext-password ""
+            }
+            level admin
+        }
+    }
+    name-server 172.16.254.30
+    ntp {
+        server 0.pool.ntp.org {
+        }
+        server 1.pool.ntp.org {
+        }
+        server 2.pool.ntp.org {
+        }
+    }
+    syslog {
+        global {
+            facility all {
+                level info
+            }
+            facility protocols {
+                level debug
+            }
+        }
+    }
+    time-zone Europe/Berlin
+}
+
+/* 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.6 */
diff --git a/smoketest/scripts/cli/test_protocols_ospf.py b/smoketest/scripts/cli/test_protocols_ospf.py
new file mode 100755
index 000000000..f5fc75084
--- /dev/null
+++ b/smoketest/scripts/cli/test_protocols_ospf.py
@@ -0,0 +1,296 @@
+#!/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/>.
+
+import os
+import unittest
+
+from vyos.configsession import ConfigSession
+from vyos.ifconfig import Section
+from vyos.util import cmd
+from vyos.util import process_named_running
+
+PROCESS_NAME = 'ospfd'
+base_path = ['protocols', 'ospf']
+
+route_map = 'foobarbaz'
+route_map_seq = '10'
+
+def getFRROSPFconfig():
+    return cmd('vtysh -c "show run" | sed -n "/router ospf/,/^!/p"')
+
+class TestProtocolsOSPF(unittest.TestCase):
+    def setUp(self):
+        self.session = ConfigSession(os.getpid())
+        self.session.set(['policy', 'route-map', route_map, 'rule', route_map_seq, 'action', 'permit'])
+
+    def tearDown(self):
+        self.session.delete(['policy', 'route-map', route_map])
+        self.session.delete(base_path)
+        self.session.commit()
+        del self.session
+
+    def test_ospf_01_defaults(self):
+        # commit changes
+        self.session.set(base_path)
+        self.session.commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f'router ospf', frrconfig)
+        self.assertIn(f' auto-cost reference-bandwidth 100', frrconfig)
+        self.assertIn(f' timers throttle spf 200 1000 10000', frrconfig) # defaults
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
+    def test_ospf_02_simple(self):
+        router_id = '127.0.0.1'
+        abr_type = 'ibm'
+        bandwidth = '1000'
+        metric = '123'
+
+        self.session.set(base_path + ['auto-cost', 'reference-bandwidth', bandwidth])
+        self.session.set(base_path + ['parameters', 'router-id', router_id])
+        self.session.set(base_path + ['parameters', 'abr-type', abr_type])
+        self.session.set(base_path + ['log-adjacency-changes', 'detail'])
+        self.session.set(base_path + ['default-metric', metric])
+
+        # commit changes
+        self.session.commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f'router ospf', frrconfig)
+        self.assertIn(f' auto-cost reference-bandwidth {bandwidth}', frrconfig)
+        self.assertIn(f' ospf router-id {router_id}', frrconfig)
+        self.assertIn(f' ospf abr-type {abr_type}', frrconfig)
+        self.assertIn(f' timers throttle spf 200 1000 10000', frrconfig) # defaults
+        self.assertIn(f' default-metric {metric}', frrconfig)
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
+    def test_ospf_03_access_list(self):
+        acl = '100'
+        seq = '10'
+        protocols = ['bgp', 'connected', 'kernel', 'rip', 'static']
+
+        self.session.set(['policy', 'access-list', acl, 'rule', seq, 'action', 'permit'])
+        self.session.set(['policy', 'access-list', acl, 'rule', seq, 'source', 'any'])
+        self.session.set(['policy', 'access-list', acl, 'rule', seq, 'destination', 'any'])
+        for ptotocol in protocols:
+            self.session.set(base_path + ['access-list', acl, 'export', ptotocol])
+
+        # commit changes
+        self.session.commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f'router ospf', frrconfig)
+        self.assertIn(f' timers throttle spf 200 1000 10000', frrconfig) # defaults
+        for ptotocol in protocols:
+            self.assertIn(f' distribute-list {acl} out {ptotocol}', frrconfig) # defaults
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
+        self.session.delete(['policy', 'access-list', acl])
+
+    def test_ospf_04_default_originate(self):
+        seq = '100'
+        metric = '50'
+        metric_type = '1'
+
+        self.session.set(base_path + ['default-information', 'originate', 'metric', metric])
+        self.session.set(base_path + ['default-information', 'originate', 'metric-type', metric_type])
+        self.session.set(base_path + ['default-information', 'originate', 'route-map', route_map])
+
+        # commit changes
+        self.session.commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f'router ospf', frrconfig)
+        self.assertIn(f' timers throttle spf 200 1000 10000', frrconfig) # defaults
+        self.assertIn(f' default-information originate metric {metric} metric-type {metric_type} route-map {route_map}', frrconfig)
+
+        # Now set 'always'
+        self.session.set(base_path + ['default-information', 'originate', 'always'])
+        self.session.commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f' default-information originate always metric {metric} metric-type {metric_type} route-map {route_map}', frrconfig)
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
+    def test_ospf_05_options(self):
+        global_distance = '128'
+        intra_area = '100'
+        inter_area = '110'
+        external = '120'
+        on_startup = '30'
+        on_shutdown = '60'
+        refresh = '50'
+
+        self.session.set(base_path + ['distance', 'global', global_distance])
+        self.session.set(base_path + ['distance', 'ospf', 'external', external])
+        self.session.set(base_path + ['distance', 'ospf', 'intra-area', intra_area])
+
+        self.session.set(base_path + ['max-metric', 'router-lsa', 'on-startup', on_startup])
+        self.session.set(base_path + ['max-metric', 'router-lsa', 'on-shutdown', on_shutdown])
+
+        self.session.set(base_path + ['mpls-te', 'enable'])
+        self.session.set(base_path + ['refresh', 'timers', refresh])
+
+        # commit changes
+        self.session.commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f'router ospf', frrconfig)
+        self.assertIn(f' mpls-te on', frrconfig)
+        self.assertIn(f' mpls-te router-address 0.0.0.0', frrconfig) # default
+        self.assertIn(f' distance {global_distance}', frrconfig)
+        self.assertIn(f' distance ospf intra-area {intra_area} external {external}', frrconfig)
+        self.assertIn(f' max-metric router-lsa on-startup {on_startup}', frrconfig)
+        self.assertIn(f' max-metric router-lsa on-shutdown {on_shutdown}', frrconfig)
+        self.assertIn(f' refresh timer {refresh}', frrconfig)
+
+
+        # enable inter-area
+        self.session.set(base_path + ['distance', 'ospf', 'inter-area', inter_area])
+        self.session.commit()
+
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f' distance ospf intra-area {intra_area} inter-area {inter_area} external {external}', frrconfig)
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
+    def test_ospf_06_neighbor(self):
+        priority = '10'
+        poll_interval = '20'
+        neighbors = ['1.1.1.1', '2.2.2.2', '3.3.3.3']
+        for neighbor in neighbors:
+            self.session.set(base_path + ['neighbor', neighbor, 'priority', priority])
+            self.session.set(base_path + ['neighbor', neighbor, 'poll-interval', poll_interval])
+
+        # commit changes
+        self.session.commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f'router ospf', frrconfig)
+        for neighbor in neighbors:
+            self.assertIn(f' neighbor {neighbor} priority {priority} poll-interval {poll_interval}', frrconfig) # default
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
+    def test_ospf_07_passive_interface(self):
+        self.session.set(base_path + ['passive-interface', 'default'])
+        interfaces = Section.interfaces('ethernet')
+        for interface in interfaces:
+            self.session.set(base_path + ['passive-interface-exclude', interface])
+
+        # commit changes
+        self.session.commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f'router ospf', frrconfig)
+        self.assertIn(f' passive-interface default', frrconfig) # default
+        for interface in interfaces:
+            self.assertIn(f' no passive-interface {interface}', frrconfig) # default
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
+    def test_ospf_08_redistribute(self):
+        metric = '15'
+        metric_type = '1'
+        redistribute = ['bgp', 'connected', 'kernel', 'rip', 'static']
+
+        for protocol in redistribute:
+            self.session.set(base_path + ['redistribute', protocol, 'metric', metric])
+            self.session.set(base_path + ['redistribute', protocol, 'route-map', route_map])
+            if protocol not in ['kernel', 'static']:
+                self.session.set(base_path + ['redistribute', protocol, 'metric-type', metric_type])
+
+        # commit changes
+        self.session.commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f'router ospf', frrconfig)
+        for protocol in redistribute:
+            if protocol in ['kernel', 'static']:
+                self.assertIn(f' redistribute {protocol} metric {metric} route-map {route_map}', frrconfig)
+            else:
+                self.assertIn(f' redistribute {protocol} metric {metric} metric-type {metric_type} route-map {route_map}', frrconfig)
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
+    def test_ospf_09_area(self):
+        area = '0'
+        networks = ['10.0.0.0/8', '172.16.0.0/12', '192.168.0.0/16']
+        for network in networks:
+            self.session.set(base_path + ['area', area, 'network', network])
+
+        # commit changes
+        self.session.commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f'router ospf', frrconfig)
+        for network in networks:
+            self.assertIn(f' network {network} area {area}', frrconfig)
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
+    def test_ospf_10_virtual_link(self):
+        area = '10'
+        shortcut = 'enable'
+        virtual_link = '192.0.2.1'
+        hello = '6'
+        retransmit = '5'
+        transmit = '5'
+        dead = '40'
+
+        self.session.set(base_path + ['area', area, 'shortcut', shortcut])
+        self.session.set(base_path + ['area', area, 'virtual-link', virtual_link, 'hello-interval', hello])
+        self.session.set(base_path + ['area', area, 'virtual-link', virtual_link, 'retransmit-interval', retransmit])
+        self.session.set(base_path + ['area', area, 'virtual-link', virtual_link, 'transmit-delay', transmit])
+        self.session.set(base_path + ['area', area, 'virtual-link', virtual_link, 'dead-interval', dead])
+
+        # commit changes
+        self.session.commit()
+
+        # Verify FRR ospfd configuration
+        frrconfig = getFRROSPFconfig()
+        self.assertIn(f'router ospf', frrconfig)
+        self.assertIn(f' area {area} shortcut {shortcut}', frrconfig)
+        self.assertIn(f' area {area} virtual-link {virtual_link} hello-interval {hello} retransmit-interval {retransmit} transmit-delay {transmit} dead-interval {dead}', frrconfig)
+
+        # Check for running process
+        self.assertTrue(process_named_running(PROCESS_NAME))
+
+if __name__ == '__main__':
+    unittest.main(verbosity=2)
diff --git a/src/conf_mode/protocols_ospf.py b/src/conf_mode/protocols_ospf.py
index 73c244571..f43929b46 100644..100755
--- a/src/conf_mode/protocols_ospf.py
+++ b/src/conf_mode/protocols_ospf.py
@@ -24,12 +24,14 @@ from vyos.template import render
 from vyos.template import render_to_string
 from vyos.util import call
 from vyos.util import dict_search
+from vyos.xml import defaults
 from vyos import ConfigError
 from vyos import frr
 from vyos import airbag
 airbag.enable()
 
 config_file = r'/tmp/ospf.frr'
+frr_daemon = 'ospfd'
 
 DEBUG = os.path.exists('/tmp/ospf.debug')
 if DEBUG:
@@ -39,10 +41,58 @@ if DEBUG:
     ch = logging.StreamHandler()
     lg.addHandler(ch)
 
-def get_config():
-    conf = Config()
+def get_config(config=None):
+    if config:
+        conf = config
+    else:
+        conf = Config()
     base = ['protocols', 'ospf']
     ospf = conf.get_config_dict(base, key_mangling=('-', '_'), get_first_key=True)
+
+    # Bail out early if configuration tree does not exist
+    if not conf.exists(base):
+        return ospf
+
+    # We have gathered the dict representation of the CLI, but there are default
+    # options which we need to update into the dictionary retrived.
+    default_values = defaults(base)
+
+    # We have to cleanup the default dict, as default values could enable features
+    # which are not explicitly enabled on the CLI. Example: default-information
+    # originate comes with a default metric-type of 2, which will enable the
+    # entire default-information originate tree, even when not set via CLI so we
+    # need to check this first and probably drop that key.
+    if dict_search('default_information.originate', ospf) is None:
+        del default_values['default_information']
+    if dict_search('area.area_type.nssa', ospf) is None:
+        del default_values['area']['area_type']['nssa']
+    if 'mpls_te' not in ospf:
+        del default_values['mpls_te']
+    for protocol in ['bgp', 'connected', 'kernel', 'rip', 'static']:
+        if dict_search(f'redistribute.{protocol}', ospf) is None:
+            del default_values['redistribute'][protocol]
+    # XXX: T2665: we currently have no nice way for defaults under tag nodes,
+    # clean them out and add them manually :(
+    del default_values['neighbor']
+    del default_values['area']['virtual_link']
+
+    # merge in remaining default values
+    ospf = dict_merge(default_values, ospf)
+
+    if 'neighbor' in ospf:
+        default_values = defaults(base + ['neighbor'])
+        for neighbor in ospf['neighbor']:
+            ospf['neighbor'][neighbor] = dict_merge(default_values, ospf['neighbor'][neighbor])
+
+    if 'area' in ospf:
+        default_values = defaults(base + ['area', 'virtual-link'])
+        for area, area_config in ospf['area'].items():
+            if 'virtual_link' in area_config:
+                print(default_values)
+                for virtual_link in area_config['virtual_link']:
+                    ospf['area'][area]['virtual_link'][virtual_link] = dict_merge(
+                        default_values, ospf['area'][area]['virtual_link'][virtual_link])
+
     return ospf
 
 def verify(ospf):
@@ -65,8 +115,9 @@ def generate(ospf):
 def apply(ospf):
     # Save original configuration prior to starting any commit actions
     frr_cfg = frr.FRRConfig()
-    frr_cfg.load_configuration(daemon='ospfd')
-    frr_cfg.modify_section(f'router ospf', '')
+    frr_cfg.load_configuration(frr_daemon)
+    frr_cfg.modify_section('router ospf', '')
+    frr_cfg.add_before(r'(ip prefix-list .*|route-map .*|line vty)', ospf['new_frr_config'])
 
     # Debugging
     if DEBUG:
@@ -82,13 +133,13 @@ def apply(ospf):
         print(f'Modified config:\n')
         print(f'{frr_cfg}')
 
-    frr_cfg.commit_configuration(daemon='ospfd')
+    frr_cfg.commit_configuration(frr_daemon)
 
     # If FRR config is blank, rerun the blank commit x times due to frr-reload
     # behavior/bug not properly clearing out on one commit.
     if ospf['new_frr_config'] == '':
         for a in range(5):
-            frr_cfg.commit_configuration(daemon='ospfd')
+            frr_cfg.commit_configuration(frr_daemon)
 
     return None