Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adapt control plane API while implementing #4434 #4552

Open
tzaeschke opened this issue Jun 18, 2024 · 11 comments
Open

Adapt control plane API while implementing #4434 #4552

tzaeschke opened this issue Jun 18, 2024 · 11 comments
Labels
i/proposal A new idea requiring additional input and discussion

Comments

@tzaeschke
Copy link
Contributor

tzaeschke commented Jun 18, 2024

As discussed in #4434, SCION will soon get a new control plane communication API.
We could use this opportunity to look at the current API and propose some changes.
If it is okay (and desirable), I will provide a proper proposal document once we determined what actually needs doing...? Please let me know if I should turn this into a document right away.

Clean up deprecated items

E.g. remove the following from the SegmentResponse definition in seg.proto?

    // Deprecated list of signed revocations. Will be removed with header v1.
    repeated bytes deprecated_signed_revocations = 1000;

Adjust the SegmentResponse definition in seg.proto

message SegmentsResponse {
    message Segments {
        // List of path segments.
        repeated PathSegment segments = 1;
    }

    // Mapping from path segment type to path segments. The key is the integer
    // representation of the SegmentType enum.
    map<int32, Segments> segments = 1;

    ...
}

to

message SegmentsResponse {
    // List of path segments.
    repeated PathSegment segmentsUp = 1;
    repeated PathSegment segmentsCore = 2;
    repeated PathSegment segmentsDown = 3;
    ...
}

Rationale

Advantages

  • Faster connection: currently, for a client to establish a connection requires the local daemon (or other library) to perform three request to the controls server (CS) i.e. three network roundtrips. This may be negligible if the segments need to be fetched from other CSs, but if the segments are cached then avoiding two roundtrips may make a notable difference.
  • Reduce code duplication: the API was implemented with the GO daemon running everywhere. However, with potentially many libraries implementing daemon functionality internally, the request logic needs to be implemented in every daemon.
  • Clearer API: currently, there is a map<int32, Segments> which always has exactly one entry. Moreover, "down" segments are indicated as "up" segments (TBC). This could probably be done in a more intuitive way.

Potential future advantages

  • Potential to reduce network I/O: With larger SCION networks, the number of segments is expected to increase considerably. We may reach a point where we cannot sensibly return all known Segments to every client. It may be much easier to return a useful subset of segments if the CS has knowledge of the complete route.
    This may be facilitated by future extensions such as providing simple route policies (fewest-hops, best-bandwidth, lowest-latency, ...).

Disadvantages

  • Increased compute time: I am not sure why (considerable) additional compute is required to perform all queries (UP, CORE DOWN) in one request. I though it might be related to determining whether destination is a CORE AS, but that can be determined easily (see IsCore in helper.go).
    Also, any additional computational cost is also (marginally?) offset by having to handle only a single RPC request instead of having to handle up to three requests, and potentially having to send fewer network packets in the response.
    Can someone provide a short explanation for the additional cost?

EDIT

Please fell free to suggest other (smallish) changes here! "Smallish" because currently the idea is that the changes can implemented without delaying #4434 too much.

@tzaeschke tzaeschke added the i/proposal A new idea requiring additional input and discussion label Jun 18, 2024
@oncilla
Copy link
Contributor

oncilla commented Jun 18, 2024

This proposal would cause the switch to connect to be a breaking change.

If we are introducing a breaking change, I would rather change the API quite heavily to follow a REST like pattern with pagination, search queries and the whole shabam

@tzaeschke
Copy link
Contributor Author

This proposal would cause the switch to connect to be a breaking change.

I assumed that this would be a breaking change anyway, because we are switching from HTTP/2 to HTTP/3? Iḿ probably missing something?

I forgot to mention (I will add note) that this proposal is meant as a catch-all, so please add other improvements.
However, the idea is that the changes are small enough to fit into the timeline for #4434. A complete redesign would probably take considerably longer?

@tzaeschke
Copy link
Contributor Author

This proposal would cause the switch to connect to be a breaking change.

Ah, I think the network stack can be changed separately from the request handler? That makes sense.

@jiceatscion
Copy link
Contributor

Regarding a breaking change...Is there anything terribly wrong with introducing a new API alongside the old one to enable an incremental transition? That's pretty much standard practice in cases like that.

@oncilla
Copy link
Contributor

oncilla commented Jun 19, 2024

@jiceatscion no there is nothing wrong with that.
But then we need to ask ourselves why do we want to couple moving to connect-rpc with re-architecturing the RPCs.

I don't think we should, because as you say, introducing a new API alongside is not an issue, and designing and implementing a new API is quite a bit of effort. There is no concrete proposal how the new API would even look right now.

@olibeck
Copy link

olibeck commented Jun 20, 2024

Hi, I'm the student currently working on the Scion for embedded systems project. Marc asked me to provide my experience / feedback on this topic.

Adjust the SegmentResponse definition in seg.proto

message SegmentsResponse {
    message Segments {
        // List of path segments.
        repeated PathSegment segments = 1;
    }

    // Mapping from path segment type to path segments. The key is the integer
    // representation of the SegmentType enum.
    map<int32, Segments> segments = 1;

    ...
}

to

message SegmentsResponse {
    // List of path segments.
    repeated PathSegment segmentsUp = 1;
    repeated PathSegment segmentsCore = 2;
    repeated PathSegment segmentsDown = 3;
    ...
}

Personally, I think this change makes a lot of sense. When I was first reading the seg.proto, I thought that the SegmentResponse would provide UP, CORE and DOWN segments. I was quite surprised when I saw that I had to split the requests myself. It would definitely make the API a lot clearer.

An additional advantage of this change is that it eliminates the necessity for the end hosts to know all Core ASes to be able to split the requests. I'm looking at this now specifically from my project's perspective. I don't know how often the Core ASes could change but I imagine it would be quite difficult to provide new configuration files to every embedded system if it happens.

Potential future advantages

  • Potential to reduce network I/O: With larger SCION networks, the number of segments is expected to increase considerably. We may reach a point where we cannot sensibly return all known Segments to every client. It may be much easier to return a useful subset of segments if the CS has knowledge of the complete route.
    This may be facilitated by future extensions such as providing simple route policies (fewest-hops, best-bandwidth, lowest-latency, ...).

I also agree that this would be a very nice feature to have, especially with the memory constraints of embedded systems. I'm currently fearing a bit that I could run out of memory when building the paths in a larger topology. A REST like API as suggested by @oncilla would of course solve this problem as well.

@oncilla
Copy link
Contributor

oncilla commented Jun 20, 2024

Hi @olibeck

Thank you for your insight.

This is a sidenote, but have you considered running a centralized daemon outside of your embedded hosts?
The daemon API is much simpler and takes less computation. It also means you do not have to do any crypto operations other than the TLS handshake on your embedded device.

@olibeck
Copy link

olibeck commented Jun 21, 2024

This is a sidenote, but have you considered running a centralized daemon outside of your embedded hosts?
The daemon API is much simpler and takes less computation. It also means you do not have to do any crypto operations other than the TLS handshake on your embedded device.

Hi @oncilla

Yes, I have thought about some kind of proxy but so far I'm hopeful that I will manage without one. I'm not too worried about the cryptographic operations (especially with the chip that I'm using for the project). It is mostly a concern that the SegmentResponse messages could become too large to handle in larger topologies. Pagination, like you suggested, would already solve this problem.

@oncilla
Copy link
Contributor

oncilla commented Jun 21, 2024

Yes, I have thought about some kind of proxy but so far I'm hopeful that I will manage without one.

I'm not really talking about a proxy. I literally mean binding the daemon service to a public IP rather than localhost and point your application to talk to a remote daemon, rather than localhost. (Note this is currently all possible already and only needs configuration)

@olibeck
Copy link

olibeck commented Jun 21, 2024

I'm not really talking about a proxy.

With proxy I meant a service which offers very similar functionality, a remote daemon comes very close to what I had in mind.

I literally mean binding the daemon service to a public IP rather than localhost and point your application to talk to a remote daemon, rather than localhost. (Note this is currently all possible already and only needs configuration)

I was not aware that it's already possible, that makes it actually a quite interesting approach.

If you are interested in continuing our discussion, maybe we should move it somewhere else, I feel like it would be too much off-topic 😅

@oncilla
Copy link
Contributor

oncilla commented Jun 21, 2024

@olibeck Sure. Feel free to reach out on slack or email.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i/proposal A new idea requiring additional input and discussion
Projects
None yet
Development

No branches or pull requests

4 participants