Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error loading configuration file when using "isis metric" on the interface #13818

Closed
1 of 2 tasks
klimkovice opened this issue Jun 20, 2023 · 8 comments
Closed
1 of 2 tasks

Comments

@klimkovice
Copy link

FRR version (compiled from source on Slackware):
FRRouting 8.5 (kompilator) on Linux(4.4.302-SMP-amd64).
Copyright 1996-2005 Kunihiro Ishiguro, et al.
configured with:
'--disable-doc' '--disable-ripd' '--disable-ripngd' '--disable-eigrpd' '--disable-pimd' '--disable-babeld' '--disable-nhrpd' '--disable-fabricd' '--enable-rpki' '--enable-multipath=2' '--prefix=/usr/local' '--sysconfdir=/usr/local/etc/frr' '--localstatedir=/var/run/frr' 'PKG_CONFIG_PATH=/usr/local/lib/pkgconfig:/usr/local/share/pkgconfig:/usr/lib/pkgconfig:/usr/share/pkgconfig'

FRR running daemons:
watchfrr, zebra, isisd, ldpd, staticd

OS: Slackware 14.2, kernel 4.4.302

Problem:
If I use command "isis metric xxx" in interface configuration, it is apply and all works fine. After saving the configuration and then restarting frr daemon, interface appears without isis configuration. If the configuration file does not contain the "isis metric xxx" command, then after the restart, the interface will load correctly with the isis configuration. If I manually edit the configuration file so that the configuration of the isis protocol is before the interfaces, everything loads correctly.

  • Did you check if this is a duplicate issue?
  • Did you test it on the latest FRRouting/frr master branch?

A snippet of the configuration:
'''
interface eth0
ip router isis 1
isis circuit-type level-2-only
isis hello-multiplier 3
isis metric 1000
isis network point-to-point
isis password md5 xxx
mpls enable
no isis hello padding
exit
!
router isis 1
is-type level-2-only
net 49.0001.1921.6800.1050.00
log-adjacency-changes
mpls ldp-sync
exit
!
'''

Summary:
If I use command "isis metric 1000" on the interface, save the configuration and then restart frr dameon, interfaces are without isis configuration.

before restart:
'''
interface eth0
ip router isis 1
isis circuit-type level-2-only
isis hello-multiplier 3
isis metric 1000
isis network point-to-point
isis password md5 xxx
mpls enable
no isis hello padding
exit

'''
after restart:
'''
interface eth0
mpls enable
exit
'''
if I don't use the "isis metric" command, everything loads fine even after a restart of daemon.

Complete example configuration:
'''
Current configuration:
!
frr version 8.5
frr defaults traditional
hostname kompilator
no ipv6 forwarding
service integrated-vtysh-config
!
interface eth0
ip router isis 1
isis circuit-type level-2-only
isis hello-multiplier 3
isis metric 1000
isis network point-to-point
isis password md5 xxxxxx
no isis hello padding
exit
!
interface dummy0
ip router isis 1
isis circuit-type level-2-only
exit
!
mpls ldp
router-id 192.168.1.50
ordered-control
!
address-family ipv4
discovery transport-address 192.168.1.50
label local allocate for ldp
!
interface eth0
exit
!
exit-address-family
!
exit
!
router isis 1
is-type level-2-only
net 49.0001.1921.6800.1050.00
log-adjacency-changes
exit
!
access-list ldp seq 5 permit 192.168.1.0/24
access-list vty seq 5 permit 127.0.0.0/8
access-list vty seq 10 deny any
!
line vty
access-class vty
exit
!
'''

@klimkovice klimkovice added the triage Needs further investigation label Jun 20, 2023
@riw777
Copy link
Member

riw777 commented Jun 21, 2023

Can you try from master, rather than 8.5?

@klimkovice
Copy link
Author

klimkovice commented Jun 27, 2023

Sorry for the delay. I tried it on the master but it behaves exactly the same.

FRRouting 9.1-dev (milan-pc) on Linux(5.19.0-42-generic).
Copyright 1996-2005 Kunihiro Ishiguro, et al.
configured with:
'--disable-doc' '--disable-ripd' '--disable-ripngd' '--disable-eigrpd' '--disable-pimd' '--disable-babeld' '--disable-nhrpd' '--disable-fabricd' '--enable-rpki' '--enable-multipath=2' '--localstatedir=/var/run/frr' '--sysconfdir=/etc/frr'

Before restart (in configuration file):
interface enp2s0
ip router isis 1
isis circuit-type level-2-only
isis hello-multiplier 3
isis metric 100
isis network point-to-point
isis password md5 xxx
mpls enable
no isis hello padding
exit

After restart of daemon (in running configuration):
interface enp2s0
mpls enable
exit

@riw777
Copy link
Member

riw777 commented Jun 27, 2023

it does look like this is a bug ... we'll see if we can find someone to take a look at it

@riw777 riw777 added bug isis and removed triage Needs further investigation labels Jun 27, 2023
@clay584
Copy link

clay584 commented Aug 3, 2023

This is affecting us as well on version 8.5.2. It seems to randomly remove other ISIS configurations on the interface.

Version 8.3.1 is also affected.

@jcquln
Copy link

jcquln commented Sep 26, 2023

Tested with v9.0.0 and, recently, with v9.0.1. Same issue still occurring.
FRR running config below:

log file /var/log/frr/isisd.log debug
!
interface ens224
 description Just a Test
 ip address 10.0.0.10/31
 ip router isis Test-1
 isis bfd
 isis bfd profile STANDARD
 isis circuit-type level-2
 isis fast-reroute lfa
 isis metric level-2 1000
 isis network point-to-point
 no isis hello padding
exit
!
router isis Test-1
 is-type level-2
 metric-style wide
 log-adjacency-changes
exit
!
debug zebra events
debug isis events
!

The issue arises when saving the above config to startup and restarting FRR.
From the logs:

ISIS: libyang: Must condition ". < 64 or /frr-isisd:isis/instance[area-tag = current()/../../area-tag]/metric-style = 'wide'" not satisfied. (Data location "/frr-interface:lib/interface[name='ens224']/frr-isisd:isis/metric/level-2".)
ISIS: [H68KZ-12QEF][EC 100663340] nb_candidate_commit_prepare: failed to validate candidate configuration
STATIC: [VTVCM-Y2NW3] Configuration Read in Took: 00:00:00

As previously mentioned, if the stanza order is switched around, i.e., the IS-IS router instance defined before the IS-IS interface instance, the startup config loads just fine. However, this is not the order written when saving the running config to startup.

The YANG validation above seems to check the metric style value on a per IS-IS routing object basis, which, if not mistaken, seems to only be instantiated afterwards.
Since the default metric-style is set to wide, I attempted to change the condition above to check the XPath of the default metric style - /frr-isisd:isis/instance/metric-style - instead, however, not getting anywhere, as I'm not familiar with libyang or YANG data modelling in general.

Any suggestions would be greatly appreciated :)

@rwestphal
Copy link
Member

@jcquln Thanks for the detailed bug report.

As you pointed out, the "must" condition is failing because the IS-IS routing instance doesn't exist by the time the interface-level isis metric command is entered. This is an unfortunate side effect of having the IS-IS interface configuration not nested within the IS-IS routing instance configuration.

Looking at commit be49219, we can see that isisd had the same validation check before the YANG conversion. But the validation only occurred if the corresponding IS-IS routing instance existed, otherwise any metric was accepted.

Commit e0df320 shows, however, that metric-style narrow validated the metric of all configured interfaces and ensured they were smaller or equal to MAX_NARROW_LINK_METRIC, otherwise that command would be rejected [1]. That validation check was removed.

What we could do to solve this problem is to restore the old behavior, which is this:

  • For isis metric, validate the metric only if the IS-IS instance exists.
  • For metric-style narrow, validate the metric of all IS-IS interfaces.

Since these validations checks are non-trivial, I think it'd be better to perform them using northbound validation callbacks instead of using complicated YANG constraints.

Another option would be to accept all metric values, but impose an upper cap of MAX_NARROW_LINK_METRIC (63) when narrow metrics are configured. In that case, we could add a comment in the "sh run" output telling users when the configured metric isn't compatible with narrow metrics. I'm not sure if this is a good idea though. It would be interesting to see how other implementations handle this situation.

[1] e0df320#diff-b18edadfcd0a803a4fbcb9455a4f009a6fa457c71a59d60acd8bb1bbf9fc7043L481-L496

Copy link

This issue is stale because it has been open 180 days with no activity. Comment or remove the autoclose label in order to avoid having this issue closed.

@frrbot
Copy link

frrbot bot commented Mar 28, 2024

This issue will be automatically closed in the specified period unless there is further activity.

@frrbot frrbot bot closed this as completed Apr 4, 2024
@frrbot frrbot bot removed the autoclose label Apr 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants