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

bgpd: per-neighbor dampening support #15911

Merged

Conversation

ton31337
Copy link
Member

@ton31337 ton31337 commented May 2, 2024

No description provided.

@frrbot frrbot bot added bgp bugfix documentation tests Topotests, make check, etc labels May 2, 2024
@ton31337 ton31337 force-pushed the feature/bgpd_dampening_per_neighbor branch 2 times, most recently from e80c153 to 187fd96 Compare May 2, 2024 20:29
Additional cli commands to add dampening profiles to peers / peer groups
and functions to save dampening configurations.

Signed-off-by: David Schweizer <[email protected]>
@ton31337 ton31337 force-pushed the feature/bgpd_dampening_per_neighbor branch from 187fd96 to 48c18b6 Compare May 3, 2024 06:25
davischw and others added 16 commits May 3, 2024 09:29
Changes implement dampening profiles for peers and peer groups. This is
achieved by introducing the possibility to have multible existing
dampening configurations with their own sets of parameters and lists of
associated paths.

Signed-off-by: Rafael Zalamena <[email protected]>
Signed-off-by: David Schweizer <[email protected]>
When we are cycling through all peers and looking for
dampening data to dump, do not consider non-configed
peers( dopplegangers ).

Signed-off-by: Donald Sharp <[email protected]>
… for the prefix

Description:
    clear ip bgp dampening was not triggering the route
    calculation for the prefix, Due to this prefix are not install in
    RIB(Zebra) and not adv to neighbor

Problem Description/Summary :
    clear ip bgp dampening was not triggering the route
    calculation for the prefix, Due to this prefix are not install in
    RIB(Zebra) and not adv to neighbor

    Fix: When clear ip bgp dampening, route are put for route-calculation as
    that it is install in the Zebra and adv to neighbor.

Signed-off-by: sudhanshukumar22 <[email protected]>
Seems really not necessary pointing to initial value before while loop, where
it's assigned anyway.

Signed-off-by: Donatas Abraitis <[email protected]>
bgp_damp_info_unclaim already calls bgp_reuselist_del. We must not call
it again here.

Fixes FRRouting#9046.

Signed-off-by: Igor Ryzhov <[email protected]>
This causes a crash using `clear ip bgp dampening <prefix>`.

Signed-off-by: Donatas Abraitis <[email protected]>
Signed-off-by: Donatas Abraitis <[email protected]>
One more crash in dampening code...

When bgp_damp_withdraw is called, if there's already a BDI structure,
bgp_damp_info_claim is called to re-assign the bdi->config in case it
was changed. The problem is that bgp_damp_info_claim actually removes
the BDI from the reuse list of the old config and never adds it to the
reuse list of the new config. We must do this to prevent the crash
because all the code assumes that BDI is always in some list.

Signed-off-by: Igor Ryzhov <[email protected]>
Current code is a complete misuse of SLIST structure. Instead of just
adding a SLIST_ENTRY to struct bgp_damp_info, it allocates a separate
structure to be a node in the list.

Signed-off-by: Igor Ryzhov <[email protected]>
bgp_damp_config, afi and safi are never used.

Signed-off-by: Igor Ryzhov <[email protected]>
Signed-off-by: Donatas Abraitis <[email protected]>
…thread

This fixes the crash:

```
==14759== Invalid read of size 8
==14759==    at 0x31032B: bgp_reuselist_del (bgp_damp.c:51)
==14759==    by 0x310392: bgp_damp_info_unclaim (bgp_damp.c:69)
==14759==    by 0x310CD6: bgp_damp_info_free (bgp_damp.c:387)
==14759==    by 0x311016: bgp_reuse_timer (bgp_damp.c:230)
==14759==    by 0x4F227CC: thread_call (thread.c:2008)
==14759==    by 0x4EDB7D7: frr_run (libfrr.c:1216)
==14759==    by 0x1EF748: main (bgp_main.c:525)
==14759==  Address 0x48 is not stack'd, malloc'd or (recently) free'd
==14759==
==14759==
==14759== Process terminating with default action of signal 11 (SIGSEGV)
==14759==    at 0x59CC7F5: raise (raise.c:46)
==14759==    by 0x4F10CEB: core_handler (sigevent.c:261)
==14759==    by 0x59CC97F: ??? (in /lib/x86_64-linux-gnu/libpthread-2.27.so)
==14759==    by 0x31032A: bgp_reuselist_del (bgp_damp.c:51)
==14759==    by 0x310392: bgp_damp_info_unclaim (bgp_damp.c:69)
==14759==    by 0x310CD6: bgp_damp_info_free (bgp_damp.c:387)
==14759==    by 0x311016: bgp_reuse_timer (bgp_damp.c:230)
==14759==    by 0x4F227CC: thread_call (thread.c:2008)
==14759==    by 0x4EDB7D7: frr_run (libfrr.c:1216)
==14759==    by 0x1EF748: main (bgp_main.c:525)
```

Signed-off-by: Donatas Abraitis <[email protected]>
Changes update the user documentation to include a description of the
now available commands to enable/disable route-flap dampening for peers
and peer groups.

Signed-off-by: David Schweizer <[email protected]>
Signed-off-by: Donatas Abraitis <[email protected]>
@ton31337 ton31337 force-pushed the feature/bgpd_dampening_per_neighbor branch from 48c18b6 to bf37877 Compare May 3, 2024 06:35
@ton31337 ton31337 marked this pull request as ready for review May 3, 2024 12:42
@riw777 riw777 self-requested a review May 7, 2024 14:49
Copy link
Member

@riw777 riw777 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good

@riw777 riw777 merged commit 2e02086 into FRRouting:master May 13, 2024
9 checks passed
@ton31337 ton31337 deleted the feature/bgpd_dampening_per_neighbor branch May 14, 2024 12:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants