[Dynamic Neighbor]: add patch to support dynamic neighbor configuration.#14
[Dynamic Neighbor]: add patch to support dynamic neighbor configuration.#14sihuihan88 merged 6 commits intodebian/0.99.24.1from
Conversation
|
@sihuihan88, |
| peer_delete (peer); | ||
| return -1; | ||
| } | ||
|
|
There was a problem hiding this comment.
The same code 4 times. Probably it's better to introduce a static function here?
There was a problem hiding this comment.
The code is used to provide log info if debug_event is enabled. They are not exactly the same. So adding more detailed log info now for better troubleshooting.
|
|
||
| peer1 = peer_lookup (NULL, &su); | ||
|
|
||
|
|
There was a problem hiding this comment.
Probably it's better to remove it?
There was a problem hiding this comment.
Good catch. Have updated accordingly.
|
could you provide the patch info you reference? Add it to the commit message and also state what you have changed due to the patch conflicts. |
|
can you check this commit? FRRouting/frr@20eb886 |
|
and this one. FRRouting/frr@57e9ee0 |
|
and this one FRRouting/frr@2aab8d2 |
| { | ||
| if (BGP_DEBUG (events, EVENTS)) | ||
| { | ||
| zlog_debug("BGP stop: %s (dynamic neighbor) deleted", peer->host); |
There was a problem hiding this comment.
-> zlog_info
check line 467: neighbor down event is logged as info level, this is also neighbor down event, should also logged as info level.
|
next time, you need to do a fork and https://help.github.com/articles/fork-a-repo/ |
| peer_delete (peer); | ||
| return -1; | ||
| } | ||
|
|
There was a problem hiding this comment.
I think this logic can be merged into bgp_stop() as bgp_stop already has similar logic. We should enable code re-use.
|
|
||
| peer_delete (peer); | ||
| return -1; | ||
| } |
There was a problem hiding this comment.
should move into bgp_stop()
|
|
||
| peer_delete (peer); | ||
| return -1; | ||
| } |
| { | ||
| zlog_debug ("%s Dynamic Neighbor added, group %s count %d", | ||
| peer->host, group->name, dncount); | ||
| } |
There was a problem hiding this comment.
this is neighbor add event, I think it should be info level, we should print it out when we enabled log neighbor change.
| { | ||
| zlog_debug ("%s dropped from group %s, count %d", | ||
| peer->host, peer->group->name, dncount); | ||
| } |
There was a problem hiding this comment.
we should do something simliar here.
/* bgp log-neighbor-changes of neighbor Down */
if (bgp_flag_check (peer->bgp, BGP_FLAG_LOG_NEIGHBOR_CHANGES))
zlog_info ("%%ADJCHANGE: neighbor %s Down %s", peer->host,
peer_down_str [(int) peer->last_reset]);
lguohan
left a comment
There was a problem hiding this comment.
mostly logging information related
This patch bases on FRRouting/frr@f14e6fd, FRRouting/frr@20eb886, FRRouting/frr@57e9ee0, FRRouting/frr@2aab8d2
To make it work in our code base, the major additional changes are: