Update gNOI.bgp.proto #214
Conversation
Current gNOI.BGP proto lacks guidance on how the ClearBGPNeighborRequest must be handled. As per RFC
Pull Request Test Coverage Report for Build 11077491977Details
💛 - Coveralls |
Current gNOI.BGP proto lacks guidance on how the ClearBGPNeighborRequest.Hard must be handled. As per RFC4486, code6 subcode 4, ```If a BGP speaker decides to administratively reset the peering with a neighbor, then the speaker SHOULD send a NOTIFICATION message with the Error Code Cease and the Error Subcode "Administrative Reset".``` which basically leaves implementations to make a choice if they should "reset TCP connection" and "Flush all routes OR not Flush all routes". Hence in rfc8538#section-3.2 for GR a new subcode 9 called `HARD RESET` was introduced to clear such ambiguities so the GR process can be handled more predictively. Having said that, the gNOI.ClearBGPNeighborRequest API must also have 2 options for reseting a TCP connection to match the RFC. The `GRACEFUL` option introduced in this pull expects sending subcode 4 and the existing `HARD` option expects sending subcode 9 so the GR behavior can be accurately handled.
Update to be more accurate and match the RFC requirements in rfc8538#section-3.1 for subcode9.
This is to define a new Test for ERR (Extended Route retention). This test demonstrates a new behavior for Graceful restart which is an extension to the existing behavior explained in RFC4724 and RFC8538. This test also relies on, - Changes proposed to the gNOI.bgp proto in openconfig/gnoi#214 - Also relies on some OC paths for ERR which arent available yet.
| @@ -39,7 +39,12 @@ message ClearBGPNeighborRequest { | |||
| enum Mode { | |||
| SOFT = 0; // Send route-refresh and reapply policy. | |||
| SOFTIN = 1; // Re-apply inbound policy on stored Adj-RIB-In. | |||
| HARD = 2; // Teardown and restart TCP connection. | |||
| HARD = 2; // Teardown, restart TCP connection, Send Cease | |||
There was a problem hiding this comment.
This is a (slightly) backward-incompatible change.
There could be existing implementations that send codes 4/9 unconditionally.
There was a problem hiding this comment.
Can you please elaborate this with examples? What does unconditionally mean?
Error code 6 subcode 9 was introduced to handle GR cases when the "N" bit in the RESET flag of Capability 64 is set. So what other conditions would you send this subcode?
Following also from RFC8538 section#4:
A BGP speaker SHOULD NOT send a Hard Reset to a peer from which it
has not received the "N" bit. We note, however, that if it did so,
the effect would be as desired in any case because, according to
[RFC4271] and [RFC4724], any NOTIFICATION message, whether recognized
or not, results in a session reset. Thus, the only negative effect
to be expected from sending the Hard Reset to a peer that hasn't
advertised compliance to this specification would be that the peer
would be unable to properly log the associated information.
There was a problem hiding this comment.
Scenario 1) an implementation doesn't know about the RFC & code 9 and always uses code 4. (and receives the notification bit from its peer, which implies, according to the rules you specified, code 9)
Scenario 2) an implementation knows about the RFC and code 9 and still always uses code 4, because nobody said that it cannot do so for this specific RPC call
Scenario 3) an implementation knows about code 9 and always uses it, ignoring the quoted recommendation
There are a number of implementations in the wild, and any of these are possible. A safer way to introduce new (strict) requirements is to add a new option instead of redefining the existing one.
There was a problem hiding this comment.
Scenario 1) an implementation doesn't know about the RFC & code 9 and always uses code 4. (and receives the notification bit from its peer, which implies, according to the rules you specified, code 9)"
Response: Right, which means the receiving speaker either isnt supporting RFC8538 or isnt configured for one. Expectations are that when the implementation does so, do follow the RFC requirement for "HARD RESET" when the gNOI.ClearBGPNeighborRequest.HARD is executed.
Following from the RFC8538
A BGP speaker SHOULD NOT send a Hard Reset to a peer from which it has not received the "N" bit. We note, however, that if it did so, the effect would be as desired in any case because, according to [RFC4271] and [RFC4724], any NOTIFICATION message, whether recognized or not, results in a session reset. Thus, the only negative effect to be expected from sending the Hard Reset to a peer that hasn't advertised compliance to this specification would be that the peer would be unable to properly log the associated information.
Scenario 2) An implementation knows about the RFC and code 9 and still always uses code 4, because nobody said that it cannot do so for this specific RPC call.
Response: Understood, which is why additional guidance now. For your example, following can happen
a. The implementation doesnt support RFC8538 and hence hasnt set "N" bit in the initial Capability 64 negotiation. Therefore, the receipt of gNOI.ClearBGPNeighborRequest.HARD command would result in the implementation sending error code 6 subcode 4. This MUST result in TCP connection reset and route FLUSH. In this scenario, gNOI.ClearBGPNeighborRequest.GRACEFUL will also result in sending error code 6 subcode 4. Since bit "N" of the RESET flag is "0", the recieving speaker would handle the subcode4 receipt accordingly resulting in route flush.
b. If the "N" bit was set originally then for "gNOI.ClearBGPNeighborRequest.HARD", the expectations are that the original Code6 Subcode 4 is encapsulated inside the errorcode 6 subcode9. If the implementation doesnt do so and continues to send error code 6 subcode 4, then this wont be a HARD_RESET signal for the receiving Speaker of the NOTIFICATION message. In this case the TCP session can go down but the routes wouldnt be flushed. This is also the RFC 8538 expectation for Subcode 4.
Adding a 3rd option in the proto keeping "HARD" intact will be confusing. The issue is, the currently defined "HARD" wasnt comprehensive enough to cover for GR scenarios. This pull is intending to close that gap.
Scenario 3) an implementation knows about code 9 and always uses it, ignoring the quoted recommendation
Response: What does that mean? Subcode 9 was introduced in the RFC8538 for GR. If the implementation is using that all the time then they arent compliant with the original BGP CEASE message (rfc4486).
There was a problem hiding this comment.
Adding a 3rd option in the proto keeping "HARD" intact will be confusing. The issue is, the currently defined "HARD" wasnt comprehensive enough to cover for GR scenarios. This pull is intending to close that gap.
The previous option can be deprecated.
There was a problem hiding this comment.
Just for my knowledge, how is that approach of deprecating "HARD" in the favor of a new attribute to handle GR and non-GR situation for "HARD" reset better than updating the existing "HARD" attribute itself?
There was a problem hiding this comment.
It doesn't change the end goal, the issue is just a matter of backward-compatibility.
As you correctly noted, in the current proto the behavior is not fully defined, so there could be differences in existing implementations (we can debate what is correct and what is not correct according to the rfc and so on, but that's not the main point).
A new option (for example let's call it HARD_RFC8538) provides an API guarantee to a client: if the client calls it, he gets the exact, strictly defined behavior in all cases (subcode 9 if supported by the receiver, subcode 4 otherwise).
Updating the existing option doesn't provide such guarantees: the client will get this behavior if NOS was updated according to the latest changes in proto, but if it wasn't, the behavior is not really deterministic and maybe the NOS will continue sending subcode 4 while the gnoi client wanted to send code 9.
(if you have a strong opinion, I'm fine with updating the existing option. But in my view a new option is a bit cleaner.)
Current gNOI.BGP proto lacks guidance on how the ClearBGPNeighborRequest.Hard must be handled. As per RFC4486, code6 subcode 4,
If a BGP speaker decides to administratively reset the peering with a neighbor, then the speaker SHOULD send a NOTIFICATION message with the Error Code Cease and the Error Subcode "Administrative Reset".which basically leaves implementations to make a choice if they should "reset TCP connection" and "Flush all routes OR not Flush all routes". Hence in rfc8538#section-3.2 for GR a new subcode 9 called
HARD RESETwas introduced to clear such ambiguities so the GR process can be handled more predictively.Having said that, the gNOI.ClearBGPNeighborRequest API must also have 2 options for reseting a TCP connection to match the RFC. The
GRACEFULoption introduced in this pull expects sending subcode 4 and the existingHARDoption expects sending subcode 9 so the GR behavior can be accurately handled by expecting implementations to flush routes post reseting the TCP connection.