FortiOS: OSPF graceful restart by a-v-popov · Pull Request #3529 · ipspace/netlab

a-v-popov · 2026-06-25T10:42:05Z

Summary

Render OSPF graceful restart on Fortinet FortiOS for OSPFv2 and OSPFv3.

FortiOS uses one CLI switch, set restart-mode graceful-restart, under both
config router ospf and config router ospf6. That switch enables the
FortiOS graceful-restart behavior; FortiOS does not expose independent restart
and helper controls.

The netlab OSPF model is more explicit: ospf.gr.restart and
ospf.gr.helper are separate attributes. The FortiOS quirk therefore applies
this matrix:

`restart` \ `helper`	Undefined	True	False
Undefined	Accept: no GR	Reject: helper-only cannot be represented safely	Accept: no GR
True	Accept: enable FortiOS GR	Accept: enable FortiOS GR	Reject: restart without helper
False	Accept: no GR	Reject: helper with restart disabled	Accept: no GR

The asymmetric handling is intentional. Enabling OSPF graceful restart on
FortiOS necessarily enables helper behavior, which is compatible with the RFC
model. The reverse is not safe: a helper-only request must not enable restart,
because restart requires the device to preserve forwarding state across its own
OSPF restart.

ospf.gr.restart.grace_period maps to FortiOS restart-period.
ospf.gr.helper.grace_period is a helper-side local policy limit. RFC 3623
allows such local policy, for example to "never allow the grace period to
exceed a Time T", but FortiOS has no corresponding configuration knob, so
netlab warns when this attribute is used on FortiOS.

Testing

Transformation error fixtures cover helper-only and restart-with-helper-off
combinations.
Verified locally that ospf.gr: True and ospf.gr.restart: True transform
into ospf.gr.restart: {} and are accepted by the FortiOS quirk.
FortiOS live testing was performed with the device under test in the restart
role because the shared upstream helper-only graceful-restart test is not
directly representable on FortiOS.

jbemmel · 2026-06-25T12:46:09Z

Rather than rejecting configuration that enables an unsupported combination, I would suggest a warning stating that FortiOS cannot enable one without the other

We’re not talking about an operational commitment, most users are simply testing out capabilities and considering options

Yes it’s important to make clear what works and what doesn’t, but I would prefer defaulting to “ok, but be aware that …”

a-v-popov · 2026-06-25T13:41:16Z

Rather than rejecting configuration that enables an unsupported combination, I would suggest a warning stating that FortiOS cannot enable one without the other

Sorry, I'm not sure if I understand. And then what? Which configuration would actually be deployed?

We’re not talking about an operational commitment, most users are simply testing out capabilities and considering option

Which, in my opinion, does not reduce the need to fail fast and loudly.

Yes it’s important to make clear what works and what doesn’t, but I would prefer defaulting to “ok, but be aware that …”

I haven't seen any guidelines or built-in mechanism for handling unsupported configurations. So I'd make a judgment call based on the following loosely defined rules:

Error on critical functionality.
Warning on RFC non-compliance or violation of "normal" expectations.
Silently ignore anything else.

ipspace

The code has a distinct AI-generated feel to it. While it's technically correct, it's also unnecessarily verbose, and the same goes for caveats.

I've learned the hard way that I can't expect the code authors to take care of their code indefinitely, so I lean toward having code that's easier to understand for other humans.

Also, keep things simple. FortiOS cannot configure GR + GR helper functionality separately. No big deal (it's probably not the only device with that approach), but there's no need to make a detailed drama out of it.

ipspace · 2026-06-25T13:43:27Z

+  device cannot honour, leaving the restart role (which a helper rides along with) to render.
+  """
+  for o_data,_,vname in _routing.rp_data(node,'ospf'):
+    gr = o_data.get('gr',None)


I would just continue to the next instance if "not gr"

ipspace · 2026-06-25T13:43:47Z

+  """
+  for o_data,_,vname in _routing.rp_data(node,'ospf'):
+    gr = o_data.get('gr',None)
+    restart_on = isinstance(gr,dict) and 'restart' in gr             # restart role explicitly enabled


Unnecessary, check it once and be done with it.

ipspace · 2026-06-25T13:46:34Z

+    gr = o_data.get('gr',None)
+    restart_on = isinstance(gr,dict) and 'restart' in gr             # restart role explicitly enabled
+    helper_on = isinstance(gr,dict) and 'helper' in gr               # helper role explicitly enabled
+    helper_off = removed_attributes(o_data,'gr.helper')              # helper role explicitly disabled


After figuring out that the user wanted OSPF GR, I would just check if 'helper' and 'restart' are both present (and the validation code takes care of the details).

If one of them is missing, you have two options:

Display an error saying "both have to be enabled for OSPF GR to work"

Display a warning saying "we need both, so I have enabled one of them", enable it, and move on.

ipspace · 2026-06-25T13:50:25Z

 * FortiOS accepts the graceful restart timers in the 1-3600 second range. A value of 0 (allowed by the _netlab_ schema) is rejected by the device during configuration deployment.
 * Enabling graceful restart on any BGP neighbor also enables the global FortiOS **graceful-restart** switch, which preserves the forwarding state across a routing restart. Advertising the capability without it would announce forwarding-state preservation the device does not deliver.

+OSPF graceful restart (**ospf.gr**):


We don't need to turn caveats into a novel. A simple bullet along the lines of "FortiOS enables OSPF Graceful Restart and helper functionality at the same time" would be enough.

We need to look at more platforms, but if it turns out that helper.grace_period is rare (although it makes sense from an implementation perspective, just like BGP weights do), then we have to document that in the OSPF document.

jbemmel · 2026-06-25T14:13:09Z

Rather than rejecting configuration that enables an unsupported combination, I would suggest a warning stating that FortiOS cannot enable one without the other

Sorry, I'm not sure if I understand. And then what? Which configuration would actually be deployed?

We’re not talking about an operational commitment, most users are simply testing out capabilities and considering option

Which, in my opinion, does not reduce the need to fail fast and loudly.

Yes it’s important to make clear what works and what doesn’t, but I would prefer defaulting to “ok, but be aware that …”

I haven't seen any guidelines or built-in mechanism for handling unsupported configurations. So I'd make a judgment call based on the following loosely defined rules:

Error on critical functionality.

Warning on RFC non-compliance or violation of "normal" expectations.

Silently ignore anything else.

Imagine a user who configures ospf.gr: True with the intent to enable Graceful Restart. That gets transformed into 'ospf': { 'gr': { 'restart': 300 } }. For FortiOS, the current template would error out with "unsupported" because it cannot enable restart without also enabling helper mode.

I think the average user would be fine with enabling both in that situation.

"unsupported" is too harsh in this situation, IMHO. Power users could still explicitly configure ospf.gr.helper: False if they wanted to, and in that case yes, the template for FortiOS should report an error. But not in the common casual user case

ipspace · 2026-06-25T14:19:10Z

Imagine a user who configures ospf.gr: True with the intent to enable Graceful Restart. That gets transformed into 'ospf': { 'gr': { 'restart': 300 } }.

Not any longer, now it would be 'ospf': { 'gr': { 'restart': {} } }.

"unsupported" is too harsh in this situation, IMHO. Power users could still explicitly configure ospf.gr.helper: False if they wanted to, and in that case yes, the template for FortiOS should report an error. But not in the common casual user case

I think it's important to let people know what's going on behind the scenes. If we can make things work without a major impact, then we should create a warning (which can be disabled). If a device has a limitation that would result in a configuration error or some such, then we should throw an error.

However, while I think it's crucial we never do unexpected stuff behind the scenes, I wouldn't worry too much about generating just-the-right error message when a more generic one is understandable (and easier on us). In this example, "we have to enable GR and GR helper on FortiOS" is probably good enough, and we don't need "you enabled GR, but I enabled GR helper for you" together with "you enabled GR helper, but I can't do that without enabling GR, so I did that".

Renders the OSPF graceful restart configuration introduced by the core ospf.gr attribute (ipspace#3490) on FortiOS, for both OSPFv2 and OSPFv3. FortiOS configures graceful restart with a single keyword, 'set restart-mode graceful-restart', in each of its two OSPF processes -- 'config router ospf' (OSPFv2) and 'config router ospf6' (OSPFv3), which netlab addresses as the IPv4 and IPv6 address families of its OSPF module. That one switch makes the device both a restarting router and a graceful-restart helper; the two roles cannot be configured independently. Because the roles are coupled, a device quirk rejects the two combinations FortiOS cannot honour: - helper enabled without restart: FortiOS would have to make the restart promise (preserve forwarding state across its own restart) that the user did not request. A helper is benign assistance to a neighbour, but restart is a commitment, so the helper role must not silently pull in restart. - restart enabled with helper explicitly disabled: the roles share one switch, so the request is unsatisfiable. A node that enables ospf.gr.restart therefore also acts as a helper. The quirk reads the explicit-disable intent through data.removed_attributes, since _remove_when_false records a cleared sub-key in _removed_attr rather than discarding it. ospf.gr.restart.grace_period maps to 'restart-period' (device range 1-3600, default 120), the grace period the restarting router advertises in its Grace-LSA. ospf.gr.helper.grace_period has no standard counterpart: RFC 3623 and RFC 5187 define only the restarting router's grace period, not a helper-side limit (that is an FRR-specific knob), so it is not rendered. restart-on-topology-change is left at the FortiOS default. The device feature ospf.gr is declared for both address families ([ ipv4, ipv6 ]) so the core check_gr validation admits FortiOS nodes, and docs/module/ospf.md marks FortiOS as supporting OSPF graceful restart. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

a-v-popov · 2026-06-25T17:17:32Z

I replaced PR description, hope it is more readable.

I made some changes through a separate commit, not sure if it is objectively better.

I removed the new FortiOS OSPF graceful-restart caveat.

The previous text tried to explain the restart/helper distinction, the FortiOS
single-switch implementation, and the helper grace-period limitation. That made
the caveat longer than the surrounding device caveats, and I do not want to add
an opinionated caveat unless the maintainers want a specific short wording.

The essential FortiOS fact is already captured by the implementation and the PR
description: FortiOS configures OSPF graceful restart and helper behavior with
one switch, so netlab cannot configure the two roles independently.

ipspace requested changes Jun 25, 2026

View reviewed changes

a-v-popov and others added 2 commits June 25, 2026 16:41

Refine FortiOS OSPF GR handling

ad59bc0

a-v-popov force-pushed the fos-ospf-gr branch from b23e414 to ad59bc0 Compare June 25, 2026 17:09

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FortiOS: OSPF graceful restart#3529

FortiOS: OSPF graceful restart#3529
a-v-popov wants to merge 2 commits into
ipspace:devfrom
a-v-popov:fos-ospf-gr

a-v-popov commented Jun 25, 2026 •

edited

Loading

Uh oh!

jbemmel commented Jun 25, 2026

Uh oh!

a-v-popov commented Jun 25, 2026

Uh oh!

ipspace left a comment

Uh oh!

ipspace Jun 25, 2026

Uh oh!

ipspace Jun 25, 2026

Uh oh!

ipspace Jun 25, 2026

Uh oh!

ipspace Jun 25, 2026

Uh oh!

jbemmel commented Jun 25, 2026

Uh oh!

ipspace commented Jun 25, 2026

Uh oh!

a-v-popov commented Jun 25, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

a-v-popov commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Testing

Uh oh!

jbemmel commented Jun 25, 2026

Uh oh!

a-v-popov commented Jun 25, 2026

Uh oh!

ipspace left a comment

Choose a reason for hiding this comment

Uh oh!

ipspace Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

ipspace Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

ipspace Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

ipspace Jun 25, 2026

Choose a reason for hiding this comment

Uh oh!

jbemmel commented Jun 25, 2026

Uh oh!

ipspace commented Jun 25, 2026

Uh oh!

a-v-popov commented Jun 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

a-v-popov commented Jun 25, 2026 •

edited

Loading

a-v-popov commented Jun 25, 2026 •

edited

Loading