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: Add an ability to filter UPDATEs using neighbor with prefix-list #14819

Conversation

ton31337
Copy link
Member

No description provided.

@eqvinox
Copy link
Contributor

eqvinox commented Nov 17, 2023

I don't think it's a good idea to use route-maps for this; while they can do all kinds of random things, the problem is also that they can do all kinds of random things.

The filters for debug statements can end up being invoked a lot. Prefix lists are heavily optimized, single function, and are limited enough in scope to be "fully understandable" in terms of side effects.

⇒ just use prefix lists directly.

(no need to bother with access lists)

@ton31337
Copy link
Member Author

I don't think it's a good idea to use route-maps for this; while they can do all kinds of random things, the problem is also that they can do all kinds of random things.

The filters for debug statements can end up being invoked a lot. Prefix lists are heavily optimized, single function, and are limited enough in scope to be "fully understandable" in terms of side effects.

⇒ just use prefix lists directly.

(no need to bother with access lists)

Will change, thanks.

@ton31337 ton31337 force-pushed the feature/bgp_debug_updates_with_route-map branch from 2ce9f6c to c2d832a Compare November 17, 2023 21:13
@ton31337 ton31337 changed the title bgpd: Add an ability to filter UPDATEs using neighbor with route-maps bgpd: Add an ability to filter UPDATEs using neighbor with prefix-list Nov 18, 2023
Copy link
Contributor

@eqvinox eqvinox left a comment

Choose a reason for hiding this comment

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

Gotta go fast

bgpd/bgp_debug.c Outdated Show resolved Hide resolved
@frrbot frrbot bot added the libfrr label Nov 20, 2023
@ton31337 ton31337 requested a review from eqvinox November 20, 2023 19:16
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

Copy link
Contributor

@eqvinox eqvinox left a comment

Choose a reason for hiding this comment

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

one more nit, sorry :)

lib/command.h Show resolved Hide resolved
Before this patch we didn't have an option to filter debug UPDATE messages
by specifying an arbitrary prefix, prefix-list or so. We had/have only an option
to specify:

```
* debug bgp updates in 10.0.0.1
* debug bgp updates prefix 10.0.1.0/24
```

Now adding:

```
* debug bgp updates <in|out> 10.0.0.1 prefix-list plist
```

CLI output:

```
r2# show debugging
MGMT debugging status:

Zebra debugging status:

BGP debugging status:
  BGP updates debugging is on (inbound) for 192.168.2.6 with prefix-list debug

Staticd debugging status

r2# show running-config | include prefix-list debug
debug bgp updates in 192.168.2.6 prefix-list debug
r2#
```

Logs:

```
BGP: [PCFFM-WMARW] 192.168.2.3(r3) rcvd UPDATE wlen 0 attrlen 28 alen 5
BGP: [PCFFM-WMARW] 192.168.2.3(r3) rcvd UPDATE wlen 0 attrlen 28 alen 4
BGP: [PCFFM-WMARW] 192.168.2.3(r3) rcvd UPDATE wlen 0 attrlen 0 alen 0
BGP: [M59KS-A3ZXZ] bgp_update_receive: rcvd End-of-RIB for IPv4 Unicast from 192.168.2.3 in vrf default
BGP: [PCFFM-WMARW] 192.168.2.4(r4) rcvd UPDATE wlen 0 attrlen 28 alen 5
BGP: [PCFFM-WMARW] 192.168.2.4(r4) rcvd UPDATE wlen 0 attrlen 28 alen 4
BGP: [PCFFM-WMARW] 192.168.2.4(r4) rcvd UPDATE wlen 0 attrlen 0 alen 0
BGP: [M59KS-A3ZXZ] bgp_update_receive: rcvd End-of-RIB for IPv4 Unicast from 192.168.2.4 in vrf default
BGP: [PCFFM-WMARW] 192.168.1.1(r1) rcvd UPDATE wlen 0 attrlen 29 alen 5
BGP: [PCFFM-WMARW] 192.168.2.6(r6) rcvd UPDATE wlen 0 attrlen 28 alen 5
BGP: [XXWBM-V772F] 192.168.2.6(r6) rcvd UPDATE w/ attr: nexthop 192.168.2.6, origin ?, metric 0, path 65006
BGP: [YCKEM-GB33T] 192.168.2.6(r6) rcvd 172.16.16.254/32 IPv4 unicast <<<<<<<<<<<<
BGP: [PCFFM-WMARW] 192.168.2.6(r6) rcvd UPDATE wlen 0 attrlen 28 alen 4
BGP: [PCFFM-WMARW] 192.168.2.6(r6) rcvd UPDATE wlen 0 attrlen 0 alen 0
BGP: [M59KS-A3ZXZ] bgp_update_receive: rcvd End-of-RIB for IPv4 Unicast from 192.168.2.6 in vrf default
BGP: [PCFFM-WMARW] 192.168.2.5(r5) rcvd UPDATE wlen 0 attrlen 28 alen 5
BGP: [PCFFM-WMARW] 192.168.2.5(r5) rcvd UPDATE wlen 0 attrlen 28 alen 4
BGP: [PCFFM-WMARW] 192.168.2.5(r5) rcvd UPDATE wlen 0 attrlen 0 alen 0
BGP: [M59KS-A3ZXZ] bgp_update_receive: rcvd End-of-RIB for IPv4 Unicast from 192.168.2.5 in vrf default
BGP: [PCFFM-WMARW] 192.168.1.1(r1) rcvd UPDATE wlen 0 attrlen 29 alen 5
BGP: [PCFFM-WMARW] 192.168.7.7(r7) rcvd UPDATE wlen 0 attrlen 0 alen 0
BGP: [M59KS-A3ZXZ] bgp_update_receive: rcvd End-of-RIB for IPv4 Unicast from 192.168.7.7 in vrf default
```

Signed-off-by: Donatas Abraitis <[email protected]>
Pass prefix-list pointers at configuration time.

Signed-off-by: Donatas Abraitis <[email protected]>
Without this if we enter something like `debug bgp updates in x.x.x.x prefix-list y`,
prefix-list can't be lookup up, because when we read the config, debug does not know
anything about this prefix-list.

Signed-off-by: Donatas Abraitis <[email protected]>
@ton31337 ton31337 force-pushed the feature/bgp_debug_updates_with_route-map branch from beea2f8 to 9d8e384 Compare November 22, 2023 07:13
@ton31337 ton31337 requested a review from eqvinox November 22, 2023 07:14
@riw777 riw777 merged commit 1fcc3d2 into FRRouting:master Nov 28, 2023
77 checks passed
@ton31337 ton31337 deleted the feature/bgp_debug_updates_with_route-map branch November 28, 2023 13:43
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.

3 participants