Initial run of topotests on my machine takes ~210 seconds
With these changes we are at ~40 seconds
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
bgp_attr_intern(attr) takes an attribute, duplicates it, and inserts it
into the attribute hash table, returning the inserted attr. This is done
when processing a bgp update. We store the returned attribute in the
path info struct. However, later on we modify one of the fields of the
attribute. This field is inspected by attrhash_cmp, the function that
allows the hash table to select the correct item from the hash chain for
a given key when doing a lookup on an item. By modifying the field after
it's been inserted, we open the possibility that two items in the same
chain that at insertion time were differential by attrhash_cmp becomes
equal according to that function. When performing subsequent hash
lookups, it is then indeterminate which of the equivalent items the hash
table will select from the chain (in practice it is the first one but
this may not be the one we want). Thus, it is illegal to modify
data used by a hash comparison function after inserting that data into
a hash table.
In fact this is occurring for attributes. We insert two attributes that
hash to the same key and thus end up in the same hash chain. Then we
modify one of them such that the two items now compare equal. Later one
we want to release the second item from the chain before XFREE()'ing it,
but since the two items compare equal we get the first item back, then
free the second one, which constitutes two bugs, the first being the
wrong attribute removed from the hash table and the second being a
dangling pointer stored in the hash table.
To rectify this we need to perform any modifications to an attr before
it is inserted into the table, i.e., before calling bgp_attr_intern().
This patch does that by moving the sole modification to the attr that
occurs after the insert (that I have seen) before that call.
Signed-off-by: Quentin Young <qlyoung@nvidia.com>
The bgpTrapBackwardTransition callback was being called only during
bgp_stop and only under the condition that peer status was Established.
The MIB defines that the event should be generated for every transition
of the BGP FSM from a higher to a lower state.
Signed-off-by: Babis Chalios <mail@bchalios.io>
1. Minor change in distribute-list update timer based on lsa flag.
Co-authored-by: harios <hari@niralnetworks.com>
Signed-off-by: Kaushik <kaushik@niralnetworks.com>
1. Added test to verify bgp vrf dynamic route leak functionality
2. Total execution time is ~8 mins
3. Added kernel version check, these script would be run for kernel version >= 4.19
Signed-off-by: Kuldeep Kashyap <kashyapk@vmware.com>
Given a received nexthop update, only send down an update to the
relevant nexthop group. Avoid sending down superfluous updates
Signed-off-by: Wesley Coakley <wcoakley@nvidia.com>
1. Removed the VRF_DEFAULT dependency from ospf6d.
2. The dependency on show command still exist
will be fixed when the ospf6 master is available.
Co-authored-by: Harios <hari@niralnetworks.com>
Signed-off-by: Kaushik <kaushik@niralnetworks.com>
When deleting a dynamic peer, unsetting md5 password would cause
it to be unset on the listener allowing unauthenticated connections
from any peer in the range.
Check for dynamic peers in peer delete and avoid this.
Signed-off-by: Pat Ruddy <pat@voltanet.io>
When setting authentication on a BGP peer in a VRF the listener is
looked up from a global list. However there is no check that the
listener is the one associated with the VRF being configured. This
can result in the wrong listener beiong configured with a password,
leaving the intended listener in an open authentication state.
To simplify this lookup stash a pointer to the bgp instance in
the listener on creating (in the same way as is done for NS-based
VRFS).
Signed-off-by: Pat Ruddy <pat@voltanet.io>
1. Topotest for isis-vrf is added for ipv4 and ipv6.
2. Test case for checking isis topology.
3. Test case for checking zebra isis routes.
4. Test case for checking linux vrf routes.
5. 2 new API's written in topotest/lib for checking vrf routes.
Co-authored-by: Kaushik <kaushik@niralnetworks.com>"
Signed-off-by: harios_niral <hari@niralnetworks.com>
1. Added isis with different vrf and it's dependecies.
2. Added new vrf leaf in yang.
3. A minor change for IF_DOWN_FROM_Z passing argrument is
replaced with ifp pointer in api "isis_if_delete_hook()".
4. Minor fix in the isisd spf unit test.
Co-authored-by: Kaushik <kaushik@niralnetworks.com>"
Signed-off-by: harios_niral <hari@niralnetworks.com>
When not using the transactional CLI mode, do not display a
warning when a YANG-modeled commmand doesn't perform any effective
configuration change.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
1. Added new API for add/delete acl with route map notify.
Co-authored-by: harios <hari@niralnetworks.com>
Signed-off-by: Kaushik <kaushik@niralnetworks.com>
since the addition of srte_color to the comparison for bgp nexthops
it is possible to have several nexthops per prefix but since zebra
only sores a per prefix registration we should not unregister for
nh notifications for a prefix unti all the nexthops for that prefix
have been deleted. Otherwise we can get into a deadlock situation
where BGP thinks we have registered but we have unregistered from zebra.
Signed-off-by: Pat Ruddy <pat@voltanet.io>
Extend the NHT code so that only the affected BGP routes are affected
whenever an SR-policy is updated on zebra.
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
Example configuration:
route-map SET_SR_POLICY permit 10
set sr-te color 1
!
router bgp 1
bgp router-id 1.1.1.1
neighbor 2.2.2.2 remote-as 1
neighbor 2.2.2.2 update-source lo
address-family ipv4 unicast
neighbor 2.2.2.2 next-hop-self
neighbor 2.2.2.2 route-map SET_SR_POLICY in
exit-address-family
!
!
Learned BGP routes from 2.2.2.2 are mapped to the SR-TE Policy
which is uniquely determined by the BGP nexthop (2.2.2.2 in this
case) and the SR-TE color in the route-map.
Co-authored-by: Renato Westphal <renato@opensourcerouting.org>
Co-authored-by: GalaxyGorilla <sascha@netdef.org>
Co-authored-by: Sebastien Merle <sebastien@netdef.org>
Signed-off-by: Sebastien Merle <sebastien@netdef.org>
Fist, routing tables aren't the most appropriate data structure
to store nexthops and imported routes since we don't need to do
longest prefix matches with that information.
Second, by converting the NHT code to use rb-trees, we can index
the nexthops using additional information, not only the destination
address. This will be useful later to index bgpd's nexthops by
both destination and SR-TE color.
Co-authored-by: Sebastien Merle <sebastien@netdef.org>
Signed-off-by: Renato Westphal <renato@opensourcerouting.org>
1. MAC ref of a zero ESI was accidentally creating a new ES with zero
ES id.
2. When an ES was deleted and re-added the ES was not being sent to BGP
because of a stale flag that suppressed the update as a dup.
Signed-off-by: Anuradha Karuppiah <anuradhak@cumulusnetworks.com>
When we get a rib deletion event and we already have
that particular route node in the queue to be reprocessed,
just note that someone from kernel land has done us dirty
and allow it to be cleaned up by normal processing
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Imagine a situation where a interface is bouncing up/down.
The interface comes up and daemons like pbr will get a nht
tracking callback for a connected interface up and will install
the routes down to zebra. At this same time the interface can
go down. But since zebra is busy handling route changes ( from pbr )
it has not read the netlink message and can get into a situation
where the route resolves properly and then we attempt to install
it into the kernel( which is rejected ). If the interface
bounces back up fast at this point, the down then up netlink
message will be read and create two route entries off the connected
route node. Zebra will then enqueue both route entries for future processing.
After this processing happens the down/up is collapsed into an up
and nexthop tracking sees no changes and does not inform any upper
level protocol( in this case pbr ) that nexthop tracking has changed.
So pbr still believes the nexthops are good but the routes are not
installed since pbr has taken no action.
Fix this by immediately running rnh when we signal a connected
route entry is scheduled for removal. This should cause
upper level protocols to get a rnh notification for the small
amount of time that the connected route was bouncing around like
a madman.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
Avoid unnecessary use of StringIO in one place, use version-
dependent method in another. Remove a couple of other py2->py3
problems.
Signed-off-by: Mark Stapp <mjs@voltanet.io>
The pnhc->nexthop was a pointer copy. Causing issues
with the ability to move pointers around for the
different pnhc since the pnhc mirrored the nexthop
caches. When we received a vrf change if we shared
pointers it was impossible to know if we had
already updated the code.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
We had multiple pnhc cache entries with the same nexthop
pointer. This causes some large amount of confusion.
Fixup the code to handle this situation better.
Ticket: CM-31044
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
If we have an interface configured in a daemon on shutdown
store the old ifindex value for retrieval on when it is
possibly recreated.
This is especially important for nexthop groups as that we
had at one point in time the ability to restore the
configuration but it was lost when we started deleting
all deleted interfaces. We need the nexthop group subsystem
to also mark that it has configured an interface.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
PBR needs the ability to allow ephermeal interfaces( bonds,
vrfs, dummy, bridges, etc ) to be destroyed and then
recreated and at the same time keep track of them and
rebuild state as appropriate when we get a change.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
The nexthop_group_write_nexthop_simple function outputs the
interface name, because we've stored the ifindex. The problem
is that there are ephermeal interfaces in linux that can be
destroyed/recreated. Allow us to keep that data and do something
a bit smarter to allow show run's and other show commands to continue
to work when the interface is deleted.
Signed-off-by: Donald Sharp <sharpd@cumulusnetworks.com>
