search

bgp / autopeer

Apache License 2.0
52 stars 6 forks source link

Preferences for modified request acceptance #23

Open benblaustein opened 5 months ago

benblaustein commented 5 months ago

Hi, I wanted to suggest an idea I had a while ago and may or may not have brought up in a meeting.

Instead of designing and implementing a state machine to negotiate modified requests (which I think has the potential to need some fairly complex logic, new endpoints, etc.), can we include flags or some other options in the request to indicate whether or not the caller will accept modifications? This way, the request is guaranteed to be either accepted (fully or with modifications, depending on the preferences) or rejected after only one call.

Example flags to illustrate:

A slightly simpler alternative:

With either of these implementations, in case of a disagreement where client has not agreed to accept changes, return an error explaining what the mismatch was or which options need to be enabled.

I'm not that familiar with these types of negotiations, so if there are scenarios this wouldn't cover, please bring them up and maybe there is a way to account for them.

grizz commented 5 months ago

Thanks - I like the idea, but I think it still is going to require the full state machine, since the client may not configure additional sessions for other reasons (i.e., political).

I could see adding reject-on-mismatch as an easier way to describe requirements, but if we need to add a flag for that, I'm not sure we're actually gaining much.

It's definitely interesting, might be helpful to look at this along side some reference code. :)

What do others think?

benblaustein commented 5 months ago

Thanks for the reply @grizz. I might have misunderstood what we've been referring to as the "state machine", this is just intended to replace the request handshake.

If the caller has political restrictions they can set reject-on-mismatch=true and follow up manually or with a new request in case of a disagreement, but my hope was that most would be more flexible and willing to accept modifications made by the server. You probably have a better idea than I do of how realistic that is.

I suppose if the request was rejected and we wanted to enable the caller to automatically trigger another request based on the mismatched sessions, then we would still need a handshake protocol though.

jramseyer commented 5 months ago

In general, I like this idea, as it makes it easier for the client and server to do more peering

For a v0, this additional handshaking may add more complexity that we want to outline in the doc, but I think our spec is open enough that we could add this in a v1 or future version of the spec.

The current way to do it is, if the server wants to request additional sessions, it files a separate request to the client.
If the server wants to accept fewer sessions than those in the full set in the request, does it accept the request and only configure some, or does it reject the request outright and submit a smaller request to the client?

One way to get around the "submitting separate requests" would be as follows:

Accept all and more case:

Client requests sessions as follows: {session1, session2, session3, allow_change=true}

Server decides that it also wants to configure session4 Server replies back with {ACCEPT: session1, session2, session3, session4} (I thought about adding an additional flag to say "modified", but decided that was unnecessary)

At this point, the server has to commit to configure session1, session2, session3, session4.

The client has to configure session1, session2, and session3. The client can now decide whether or not to configure session4.
If it does, it comes up. If it doesn't, session4 gets garbage-collected.

Accept only some case

Client requests sessions as follows: {session1, session2, session3, session4 allow_change=true}

Server decides that it does not want to configure session2 Server replies back with {ACCEPT: session1, session3, session4}

At this point, the server has to commit to configure session1, session3, session4.

The client has to configure session1, session3, and session4. If the client wants to read back the reply from the server (I think it should, but that's my opinion), the client does not need to configure session2. If the client wants to take the simplest implementation, it can configure session2 as in its original request and leave it to be cleaned up later.

A requirement in both of these cases is that both sides have good router hygiene and garbage collect their down sessions. I think this is a good idea in general, but I don't know if it is something we can fix as a standard.

I switched from "allow_additional" to "allow_change" so we would have fewer flags. "reject_on_mismatch" as proposed by Ben would work as well.

grizz commented 5 months ago

The current way to do it is, if the server wants to request additional sessions, it files a separate request to the client. If the server wants to accept fewer sessions than those in the full set in the request, does it accept the request and only configure some, or does it reject the request outright and submit a smaller request to the client?

That is not my understanding of it -- the server replies with a different set of sessions, it does not file a separate request to the client.

If the client is happy with the session changes, it would then send a request to the server for that set of sessions, if the server agrees, it would reply with the same set, thus agreeing to everything.

Am I misunderstanding that?

Err, just finished reading your use cases, I believe that's how we've always discussed doing this. Either way, it sounds like we are in agreement with how it should work, which does function without having an allow_change flag.

Adding a flag for that allows a short circuit, which is probably worth adding flags for? Since we don't have other flags, I think I could see it either way. If we did have other flags, I would be all for adding that.