Page MenuHomeVyOS Platform

BGP route adverisement wih checks rib
Open, NormalPublicFEATURE REQUEST

Description

By default, the BGP prefix is advertised even if it's not present in the routing table.
It's related only for param " ipv4-unicast network x.x.x.x/x"

set protocols bgp 65001 address-family ipv4-unicast network 10.55.55.0/24
set protocols bgp 65001 neighbor 10.0.0.2 remote-as '65002'
vyos@zab01:~$ sh ip bgp neighbors 10.0.0.2 advertised-routes 

   Network          Next Hop            Metric LocPrf Weight Path
*> 10.55.55.0/24    0.0.0.0                  0         32768 i

vyos@zab01:~$ show ip route  10.55.55.0/24
% Network not in table

Frr by default don't check it, but have command "bgp network import-check" to prevent it. Check BGP network route exists in IGP.

vtysh

zab01# conf t
zab01(config)# router bgp 65001
zab01(config-router)# bgp network import-check

We need to add this checker as an option in BGP configuration (vyos CLI).

After option (import-check), the prefix will be advertised only if it present in the routing table.

vyos@zab01:~$ show ip bgp neighbors 10.0.0.2  advertised-routes 
vyos@zab01:~$

Details

Difficulty level
Unknown (require assessment)
Version
-
Why the issue appeared?
Will be filled on close
Is it a breaking change?
Unspecified (possibly destroys the router)

Event Timeline

Viacheslav added a comment.EditedMar 5 2020, 8:41 AM

We have this feature but with wrong logic.
https://github.com/vyos/vyatta-cfg-quagga/blob/current/scripts/bgp/vyatta-bgp.pl#L723

Be default frr don't check this param.
So to enable this checks we need set and delete it again. It don't have any sense.

set protocols bgp 65001 parameters disable-network-import-check
commit
del protocols bgp 65001 parameters disable-network-import-check

vyos@zab01# commit
vyos@zab01# vtysh -c "show run"
Building configuration...

Current configuration:
!
router bgp 65001
 bgp network import-check
 neighbor 10.0.0.2 remote-as 65002

How about replace

parameters disable-network-import-check

to

parameters enable-network-import-check
pasik added a subscriber: pasik.Mar 5 2020, 5:14 PM
syncer changed the task status from Open to Needs testing.Mar 11 2020, 1:48 AM
syncer reassigned this task from Viacheslav to jestabro.
syncer triaged this task as Normal priority.
syncer moved this task from Need Triage to In Progress on the VyOS 1.3 Equuleus board.
syncer updated the task description. (Show Details)

Thanks, @Viacheslav. We will need to add a migration script for the previous setting; that is simple in this case, since, as you observed, it was a no-op, and can just be dropped. If you are busy, I can add it.

While you are working on this I'd suggest the default behavior to be to check if IGP routes exist by default. The reason most implementations check IGP is described in my initial bug submission along with diagrams. Since advertising unconditionally breaks dynamic routing it may make sense to make this a default.

Ben Russell
Chief Technology Officer
Stratus Networks
(309)370-3174
CCIE R&S #35267
MEF CECP #25743X2
CCDP

[logo]

This e-mail, and any files transmitted with it are the property of Stratus Networks, Inc. and/or its affiliates, are confidential, and are intended solely for the use of the individual or entity to whom this e-mail is addressed. If you are not one of the named recipient(s) or otherwise have reason to believe that you have received this message in error, please notify the sender at 309-370-3174 and delete this message immediately from your computer. Any other use, retention, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited

We don't can do it as default behavior.
Frr documentation, frr has profiles

datacenter - reflects a single administrative domain with intradomain links using aggressive timers.

http://docs.frrouting.org/en/latest/basic.html#profiles

With this profile default import check is "on"

#define DFLT_BGP_IMPORT_CHECK			1

https://github.com/rtrlib/frr-1/blob/master/defaults.h#L28

But it has some aggressive timers.
The profile must be the same across all daemons. Mismatches may result in undefined behavior.
I don't think it safe for production (changing profiles).

protocols bgp var parameters network-import-check

I would leave this as an option.

DFLT_BGP_IMPORT_CHECK can only be set by changing the profile? It can't be set directly? We don't need to change the default timers, just this parameter.

Ben Russell
Chief Technology Officer
Stratus Networks
(309)370-3174
CCIE R&S #35267
MEF CECP #25743X2
CCDP

[logo]

This e-mail, and any files transmitted with it are the property of Stratus Networks, Inc. and/or its affiliates, are confidential, and are intended solely for the use of the individual or entity to whom this e-mail is addressed. If you are not one of the named recipient(s) or otherwise have reason to believe that you have received this message in error, please notify the sender at 309-370-3174 and delete this message immediately from your computer. Any other use, retention, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited

Why we can't enable this feature by default.
A lot of customers don't use it, and announce their BGP prefix with "network x.x.x.x"
Imagine if you don't have configuration "redistribute connected" or "redistribute static".
If this feature enabled by default in the new release - you update the VyOS, reboot it and lose access to the router.
Because there are no routes /24 as directly connected. Also, you can use more-spec prefixes (/28 /29 /25), not /24.
Prefixes will disappear from the announcements ISPs.
It's impossible to figure out quickly what happened.

jestabro closed this task as Resolved.Apr 2 2020, 9:24 PM

As far as I remember, originally in our Quagga days, it was the case: nothing was advertised if it wasn't present in the RIB. So if you wanted to advertise e.g. 192.0.2.0/24 but had it split into /25's, you'd need both set protocols bgp ... network 192.0.2.0/24 and set protocols static route 192.0.2.0/24 blackhole.

So I'm, with @brussell here: let's make it the default again.
I also agree that we should make a highly visible advisory to the release notes though.

@dmbaturin to enable it by default we need rewrite BGP to new python/XML format and use template, where this feature will be by default because FRR by default doesn't check routes in the RIB.

It would be interesting to hear the reason the FRR chose this behavior as default for one of their profiles. It causes major reconvergence issues for BGP networks and I don't see an obvious benefit.

syncer reopened this task as Open.May 4 2020, 6:05 PM
syncer added subscribers: zsdc, syncer.

@brussell @Viacheslav @dmbaturin @zsdc
FRR devs agree to change the default behavior in 7.4
so we need to prepare change on our side and also document that change in FRR docs

FRR 7.4 has been released, and the default behaviour has been changed, commit 62282e8379. @Viacheslav, when we update to this version, I can work with you to update the migration script.

Sounds good thanks.

Ben Russell
Chief Technology Officer
Stratus Networks
(309)370-3174
CCIE R&S #35267
MEF CECP #25743X2
CCDP | VCNE

[logo]

This e-mail, and any files transmitted with it are the property of Stratus Networks, Inc. and/or its affiliates, are confidential, and are intended solely for the use of the individual or entity to whom this e-mail is addressed. If you are not one of the named recipient(s) or otherwise have reason to believe that you have received this message in error, please notify the sender at 309-370-3174 and delete this message immediately from your computer. Any other use, retention, dissemination, forwarding, printing, or copying of this e-mail is strictly prohibited