MERAprojects/ops-build#21: Implementation of local-as command #32
Conversation
lyubov-popova
changed the title
| if (!ovs_bgp_neighbor) { | ||
| ABORT_DB_TXN(txn, BGP_ERR_NEIGHBOR_IS_NOT_CONFIGURED); | ||
| } | ||
|
|
There was a problem hiding this comment.
Missing check for peer-group membership for "no neighbor local-as" command?
if(ovs_bgp_neighbor->bgp_peer_group) {
ABORT_DB_TXN(txn, BGP_ERR_INVALID_FOR_PEER_GROUP_MEMBER);}
vtysh/bgp_vty.c
Outdated
| void | ||
| remove_parameters_invalid_for_ibgp_peer(const struct ovsrec_bgp_neighbor *ovs_bgp_neighbor) | ||
| { | ||
| if (ovs_bgp_neighbor->remove_private_as) { |
There was a problem hiding this comment.
I suggest instead of operation with a pointer used with a counter operation.
In this case "ovs_bgp_neighbor->remove_private_as" replace "ovs_bgp_neighbor->n_remove_private_as"
vtysh/bgp_vty.c
Outdated
| ovsrec_bgp_neighbor_set_remove_private_as(ovs_bgp_neighbor, NULL, 0); | ||
| } | ||
|
|
||
| if (ovs_bgp_neighbor->local_as) { |
There was a problem hiding this comment.
Similarly suggest instead of operation with a pointer used with a counter operation.,
| ABORT_DB_TXN(txn, BGP_ERR_LOCAL_AS_ALLOWED_ONLY_FOR_EBGP); | ||
| } | ||
|
|
||
| if (local_as == asn) { |
There was a problem hiding this comment.
Suggest аdd analysis of the impossibility of assigning "local_as" values are equal "remote as" for neighbor. If the neighbor is a group that respectively need analysis for all members of the group.
There was a problem hiding this comment.
Added below for peer-group (BGP_ERR_CANNOT_HAVE_LOCAL_AS_SAME_REMOTE_AS) and all neighbors
vtysh/bgp_vty.c
Outdated
| (int64_t *)&tim_val, ovs_bgp_peer_group->n_timers); | ||
| } | ||
|
|
||
| if (ovs_bgp_peer_group->local_as) { |
There was a problem hiding this comment.
Similarly suggest instead of operation with a pointer used with a counter operation
vtysh/vtysh_ovsdb_router_context.c
Outdated
| } | ||
| } | ||
|
|
||
| if (nbr_table->local_as) { |
There was a problem hiding this comment.
Similarly suggest instead of operation with a pointer used with a counter operation
There was a problem hiding this comment.
Do you suggest to do it for all the cases in this function or just local-as?
To keep the common style we the proper way is to replace all such checks in this file(or at least in this function) or let it be as it is now. I see no major point to apply change that you've suggested.
There was a problem hiding this comment.
Yes, I propose to maintain this style everywhere. In my opinion the number of records is more adequate criterion, than the value of the pointer.
lyubov-popova
force-pushed
the
meraswitch/devel/issue_21
branch
2 times, most recently
from
1527b69 to
411200e
Compare
|
Hi!
|
lyubov-popova
force-pushed
the
meraswitch/devel/issue_21
branch
from
411200e to
0dd382f
Compare
|
Hi! |
lyubov-popova
force-pushed
the
meraswitch/devel/issue_21
branch
from
0dd382f to
e53cc58
Compare
|
Replaced local_as == *ovs_bgp_neighbor->remote_as to compare_neighbor_remote_as_with_asn(ovs_bgp_neighbor, local_as) in e53cc58 |
Change-Id: Id6a0f974276ee66bdd6251dab5e7f2055b39f4f3
Change-Id: Iad717eab3f16a1c722d79f55bec0008bf0529e2c
MERAprojects/ops-build#21
Following BGP commands implemented in commit:
neighbor x.x.x.x local-as <1-4294967295>
no neighbor x.x.x.x local-as <1-4294967295>