bgpd: Notify "Peer De-configured" after entering 'no neighbor <neighb… #5285

ton31337 · 2019-11-05T17:32:11Z

…or> cmd'

Before changes:

~# vtysh -c 'show ip bgp neighbors 192.168.0.2 json' |
jq '."192.168.0.2".lastNotificationReason'
null

After changes:

~# vtysh -c 'show ip bgp neighbors 192.168.0.2 json' |
jq '."192.168.0.2".lastNotificationReason'
"Cease/Peer Unconfigured"

Closes #5262

Signed-off-by: Donatas Abraitis donatas.abraitis@gmail.com

LabN-CI · 2019-11-05T17:57:22Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/5285 `95a2aab`
Date	11/05/2019
Start	12:35:44
Finish	12:57:19
Run-Time	21:35
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-11-05-12:35:44.txt
Log	autoscript-2019-11-05-12:36:36.log.bz2
Memory	433 424 361

For details, please contact louberger

bisdhdh

We need to take care of two more commands

no_neighbor_interface_config_cmd
no_neighbor_set_peer_group

bgpd/bgp_vty.c

bgpd/bgpd.c

NetDEF-CI · 2019-11-05T20:03:05Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9604/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings

Report for bgpd.c | 2 issues
===============================================
< ERROR: open brace '{' following function declarations go on the next line
< #2565: FILE: /tmp/f1-1288/bgpd.c:2565:

CLANG Static Analyzer Summary

Github Pull Request 5285, comparing to Git base SHA 3bc6be2
Base image data for Git 3bc6be2 does not exist - compare skipped

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9604/artifact/shared/static_analysis/index.html

ton31337 · 2019-11-05T20:26:35Z

@bisdhdh regarding no_neighbor_interface_config_cmd and no_neighbor_set_peer_group, I doubt notification needed there. Right now it's handled with BGP_NOTIFY_CEASE_CONFIG_CHANGE, do you think we need to send BGP_NOTIFY_CEASE_PEER_UNCONFIG there?

LabN-CI · 2019-11-05T20:47:26Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/5285 `a30da03`
Date	11/05/2019
Start	15:25:43
Finish	15:47:24
Run-Time	21:41
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-11-05-15:25:43.txt
Log	autoscript-2019-11-05-15:26:39.log.bz2
Memory	430 431 360

For details, please contact louberger

NetDEF-CI · 2019-11-05T23:14:31Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9609/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

CLANG Static Analyzer Summary

Github Pull Request 5285, comparing to Git base SHA b3bce3a

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9609/artifact/shared/static_analysis/index.html

bisdhdh · 2019-11-06T08:00:32Z

@bisdhdh regarding no_neighbor_interface_config_cmd and no_neighbor_set_peer_group, I doubt notification needed there. Right now it's handled with BGP_NOTIFY_CEASE_CONFIG_CHANGE, do you think we need to send BGP_NOTIFY_CEASE_PEER_UNCONFIG there?

@ton31337

no_neighbor_interface_config_cmd - For this command i donot see any notification being sent
no_neighbor_set_peer_group also calls peer_delete() and it doesnot send any notification.

If i am missing something pls let me know.

bgpd/bgpd.c

LabN-CI · 2019-11-06T12:27:23Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/5285 `1fd9ee5`
Date	11/06/2019
Start	07:05:41
Finish	07:27:21
Run-Time	21:40
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-11-06-07:05:41.txt
Log	autoscript-2019-11-06-07:06:35.log.bz2
Memory	432 433 360

For details, please contact louberger

NetDEF-CI · 2019-11-06T14:53:12Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9613/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Warnings Generated during build:

Checkout code: Successful with additional warnings

Report for bgp_vty.c | 4 issues
===============================================
< ERROR: else should follow close brace '}'
< #3159: FILE: /tmp/f1-4445/bgp_vty.c:3159:
< ERROR: else should follow close brace '}'
< #3236: FILE: /tmp/f1-4445/bgp_vty.c:3236:

CLANG Static Analyzer Summary

Github Pull Request 5285, comparing to Git base SHA b3bce3a

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9613/artifact/shared/static_analysis/index.html

…or> cmd' Before changes: ~# vtysh -c 'show ip bgp neighbors 192.168.0.2 json' | \ jq '."192.168.0.2".lastNotificationReason' null After changes: ~# vtysh -c 'show ip bgp neighbors 192.168.0.2 json' | \ jq '."192.168.0.2".lastNotificationReason' "Cease/Peer Unconfigured" Signed-off-by: Donatas Abraitis <donatas.abraitis@gmail.com>

riw777 · 2019-11-06T18:57:55Z

@bisdhdh regarding no_neighbor_interface_config_cmd and no_neighbor_set_peer_group, I doubt notification needed there. Right now it's handled with BGP_NOTIFY_CEASE_CONFIG_CHANGE, do you think we need to send BGP_NOTIFY_CEASE_PEER_UNCONFIG there?

I don't think we need to notify on a neighbor being pulled from a peer group, but we probably do on unconfig. If these are already handled, but that code isn't working correctly, then we probably need to open a separate issue?

LabN-CI · 2019-11-06T19:02:22Z

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

Results table

_	_
Result	SUCCESS git merge/5285 `4e2786d`
Date	11/06/2019
Start	13:40:43
Finish	14:02:20
Run-Time	21:37
Total	1815
Pass	1815
Fail	0
Valgrind-Errors	0
Valgrind-Loss	0
Details	vncregress-2019-11-06-13:40:43.txt
Log	autoscript-2019-11-06-13:41:35.log.bz2
Memory	435 432 359

For details, please contact louberger

NetDEF-CI · 2019-11-06T19:04:57Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9616/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Failed

Ubuntu 18.04 ppc64le build: Failed (click for details)

Ubuntu 18.04 ppc64le build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9616/artifact/U1804PPC64LEBUILD/ErrorLog/ Ubuntu 18.04 ppc64le build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9616/artifact/U1804PPC64LEBUILD/config.status/ Ubuntu 18.04 ppc64le build: config.log output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9616/artifact/U1804PPC64LEBUILD/config.log/ Ubuntu 18.04 ppc64le build: No useful log found

Successful on other platforms

Debian 9 amd64 build
Ubuntu 12.04 amd64 build
Ubuntu 16.04 i386 build
Fedora 29 amd64 build
NetBSD 6 amd64 build
CentOS 7 amd64 build
Debian 8 amd64 build
Debian 10 amd64 build
Ubuntu 14.04 amd64 build
FreeBSD 12 amd64 build
OpenBSD 6 amd64 build
NetBSD 7 amd64 build
Ubuntu 16.04 amd64 build
FreeBSD 11 amd64 build
Ubuntu 18.04 amd64 build

ton31337 · 2019-11-06T19:07:05Z

@bisdhdh regarding no_neighbor_interface_config_cmd and no_neighbor_set_peer_group, I doubt notification needed there. Right now it's handled with BGP_NOTIFY_CEASE_CONFIG_CHANGE, do you think we need to send BGP_NOTIFY_CEASE_PEER_UNCONFIG there?

I don't think we need to notify on a neighbor being pulled from a peer group, but we probably do on unconfig. If these are already handled, but that code isn't working correctly, then we probably need to open a separate issue?

@riw777 thanks for the review. At the moment I modified the code that such commands send cease/unconfig:

no neighbor eth1
no neighbor eth1 interface
no neighbor 1.1.1.1
no neighbor 2a02::1
no neighbor A1
no neighbor A1 peer-group
no neighbor 1.1.1.1 peer-group A1

ton31337 · 2019-11-06T19:08:07Z

ci:rerun

NetDEF-CI · 2019-11-06T19:46:22Z

Continuous Integration Result: FAILED

See below for issues.
CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9618/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

Get source / Pull Request: Successful

Building Stage: Failed

Ubuntu 18.04 ppc64le build: Failed (click for details)

Ubuntu 18.04 ppc64le build: Unknown Log URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9618/artifact/U1804PPC64LEBUILD/ErrorLog/ Ubuntu 18.04 ppc64le build: config.status output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9618/artifact/U1804PPC64LEBUILD/config.status/ Ubuntu 18.04 ppc64le build: config.log output from configure script can be found at https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9618/artifact/U1804PPC64LEBUILD/config.log/ Ubuntu 18.04 ppc64le build: No useful log found

Successful on other platforms

Ubuntu 12.04 amd64 build
Ubuntu 16.04 i386 build
Fedora 29 amd64 build
Debian 10 amd64 build
Ubuntu 18.04 amd64 build
CentOS 7 amd64 build
FreeBSD 12 amd64 build
Ubuntu 14.04 amd64 build
Debian 8 amd64 build
Debian 9 amd64 build
OpenBSD 6 amd64 build
NetBSD 7 amd64 build
Ubuntu 16.04 amd64 build
FreeBSD 11 amd64 build
NetBSD 6 amd64 build

NetDEF-CI · 2019-11-06T23:30:24Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9616/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

CLANG Static Analyzer Summary

Github Pull Request 5285, comparing to Git base SHA 566c5d2

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9616/artifact/shared/static_analysis/index.html

NetDEF-CI · 2019-11-07T00:17:46Z

Continuous Integration Result: SUCCESSFUL

Congratulations, this patch passed basic tests

Tested-by: NetDEF / OpenSourceRouting.org CI System

CI System Testrun URL: https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9618/

This is a comment from an automated CI system.
For questions and feedback in regards to this CI system, please feel free to email
Martin Winter - mwinter (at) opensourcerouting.org.

CLANG Static Analyzer Summary

Github Pull Request 5285, comparing to Git base SHA 566c5d2

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

See details at
https://ci1.netdef.org/browse/FRR-FRRPULLREQ-9618/artifact/shared/static_analysis/index.html

bgpd/bgpd.c

polychaeta added the bgp label Nov 5, 2019

bisdhdh suggested changes Nov 5, 2019

View reviewed changes

bgpd/bgp_vty.c Outdated Show resolved Hide resolved

bgpd/bgpd.c Outdated Show resolved Hide resolved

bisdhdh suggested changes Nov 6, 2019

View reviewed changes

bgpd/bgpd.c Show resolved Hide resolved

bisdhdh suggested changes Nov 7, 2019

View reviewed changes

bgpd/bgpd.c Show resolved Hide resolved

bisdhdh suggested changes Nov 8, 2019

View reviewed changes

bgpd/bgpd.c Show resolved Hide resolved

bisdhdh approved these changes Nov 12, 2019

View reviewed changes

riw777 approved these changes Nov 19, 2019

View reviewed changes

riw777 merged commit 20a4c5f into FRRouting:master Nov 19, 2019

ton31337 deleted the fix/send_BGP_NOTIFY_CEASE_PEER_UNCONFIG_after_no_neighbor branch November 19, 2019 18:29

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

bgpd: Notify "Peer De-configured" after entering 'no neighbor <neighb… #5285

bgpd: Notify "Peer De-configured" after entering 'no neighbor <neighb… #5285

ton31337 commented Nov 5, 2019

LabN-CI commented Nov 5, 2019

bisdhdh left a comment

NetDEF-CI commented Nov 5, 2019

ton31337 commented Nov 5, 2019

LabN-CI commented Nov 5, 2019

NetDEF-CI commented Nov 5, 2019

bisdhdh commented Nov 6, 2019

LabN-CI commented Nov 6, 2019

NetDEF-CI commented Nov 6, 2019

riw777 commented Nov 6, 2019

LabN-CI commented Nov 6, 2019

NetDEF-CI commented Nov 6, 2019

ton31337 commented Nov 6, 2019 •

edited

Loading

ton31337 commented Nov 6, 2019

NetDEF-CI commented Nov 6, 2019

NetDEF-CI commented Nov 6, 2019

NetDEF-CI commented Nov 7, 2019

bgpd: Notify "Peer De-configured" after entering 'no neighbor <neighb… #5285

bgpd: Notify "Peer De-configured" after entering 'no neighbor <neighb… #5285

Conversation

ton31337 commented Nov 5, 2019

LabN-CI commented Nov 5, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

bisdhdh left a comment

Choose a reason for hiding this comment

NetDEF-CI commented Nov 5, 2019

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

CLANG Static Analyzer Summary

1 Static Analyzer issues remaining.

ton31337 commented Nov 5, 2019

LabN-CI commented Nov 5, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Nov 5, 2019

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

bisdhdh commented Nov 6, 2019

LabN-CI commented Nov 6, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Nov 6, 2019

Continuous Integration Result: SUCCESSFUL

Warnings Generated during build:

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

riw777 commented Nov 6, 2019

LabN-CI commented Nov 6, 2019

💚 Basic BGPD CI results: SUCCESS, 0 tests failed

NetDEF-CI commented Nov 6, 2019

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Failed

ton31337 commented Nov 6, 2019 • edited Loading

ton31337 commented Nov 6, 2019

NetDEF-CI commented Nov 6, 2019

Continuous Integration Result: FAILED

Get source / Pull Request: Successful

Building Stage: Failed

NetDEF-CI commented Nov 6, 2019

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

NetDEF-CI commented Nov 7, 2019

Continuous Integration Result: SUCCESSFUL

CLANG Static Analyzer Summary

No Changes in Static Analysis warnings compared to base

1 Static Analyzer issues remaining.

ton31337 commented Nov 6, 2019 •

edited

Loading