-
Notifications
You must be signed in to change notification settings - Fork 163
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
doc: add some diagrams #4678
doc: add some diagrams #4678
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM minus some questions.
doc/overview.rst
Outdated
This diagram attempts to illustrate the variety of valid configurations. For example, notice that: | ||
|
||
- There may be multiple core ASes per ISD. | ||
- There may be multiple connections between a non-core AS and its core-ASes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this covers at least two scenarios, may be rephrase this one bullet to:
- There may be
multipleconnections between a non-core AS and multipleitscore-ASes - There may be multiple connections between any two ASes
Maybe add a case to the diagram where there are two connections between a given pair of ASes? They could even go through the same border routers...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, maybe add an explanation for the dashed red lines?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. Updated. As for the dashed red line. I did explain: "The dark red dashed arrows show examples of possible paths through the network. I moved the sentence to the end of the diagram description.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- 'QUIC': I think many applications do not use QUIC, just plain UDP (over SCION)?
Grpc
Most applications probably do not use Grpc. Or if they do (as JPAN does, or the daemon) for communication with the control service, I think they are not (and I think should not be) using Grpc over SCION?- Scion Router: why is the SCION router a different color than everything else? Isn't it in the same category as e.g. the Control Service?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't the control service API used via grpc over SCION? I find that part awfully confusing, I must admit. That many application do not use quic and grpc at all is depicted in the diagram by the fact that grpc and quic are not occupying the entire width belong application or CS. However I agree that it's not obvious. Adding arrows, to see if that helps.
As for the color; I was trying to attract attention to the fact that it is in a slightly unsual position in the stack: it does technically consume and produce packets of the highest possible layer, but does not look at anything but the SCION header. Nothing new for a router, but I find it difficult to place on a network stack diagram. I could put it at the top of the SCION layer too. Suggestions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand it, the control service API is used via grpc over SCION only when one control server talks to a different control server in a different AS. An application or daemon will only talk to the control server API that is in the local AS, hence no SCION underneath.
I would probably move the router to the top of the diagram, I think it is very similar to the control server, except that applications connect to it for dataplane reasons, while they connect to the control server for controlplane reasons.
Maybe I would also move the SCMP around, it is also consumed by applications (SCMP errors) and even produced by applications (SCMP echo/traceroute). The only thing that is a pure router protocol is BFD.
And maybe add a small note to the text that GRPC and QUIC are optional?
Also, I don't know what the current state is, but I think grpc is over UDP, not QUIC? GRPC over QUIC is only planned? I really don't know this, it's just something I seem to remember...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grpc not over quic: Indeed, I was confused. All fixed...I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, actually, applications are also using grpc, e.g. JPAN uses Grpc directly, and PAN/snet applications use it indirectly via the daemon.
Another proposal: I wonder what the relevance of grpc (and QUIC) is in this context. Maybe the easiest is to remove GRPC from the diagram? And maybe remove QUIC as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how much detail we want to show in the diagram but the protocol stacks for the control service are as follows:
- Intra-AS: gRPC/HTTP2/TCP/IP
- Inter-AS: gRPC/HTTP2/QUIC/UDP/SCION
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I can do is show intra-AS communications as a dotted line directly b/w application and control service; if it's not going through SCION, no need to complicate matters. For inter-AS I can summarize grpc http and quic into a single box. I think it is important to mention that there is middleware there, but details are not so relevant indeed. Let me try again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How's that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @jiceatscion and @nicorusti)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 5 of 5 files at r1, 2 of 2 files at r2, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jiceatscion and @nicorusti)
doc/overview.rst
Outdated
This diagram attempts to illustrate the variety of valid configurations. For example, notice that: | ||
|
||
- There may be multiple core ASes per ISD. | ||
- There may be multiple connections between a non-core AS and its core-ASes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I understand it, the control service API is used via grpc over SCION only when one control server talks to a different control server in a different AS. An application or daemon will only talk to the control server API that is in the local AS, hence no SCION underneath.
I would probably move the router to the top of the diagram, I think it is very similar to the control server, except that applications connect to it for dataplane reasons, while they connect to the control server for controlplane reasons.
Maybe I would also move the SCMP around, it is also consumed by applications (SCMP errors) and even produced by applications (SCMP echo/traceroute). The only thing that is a pure router protocol is BFD.
And maybe add a small note to the text that GRPC and QUIC are optional?
Also, I don't know what the current state is, but I think grpc is over UDP, not QUIC? GRPC over QUIC is only planned? I really don't know this, it's just something I seem to remember...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @lukedirtwalker, @nicorusti, and @tzaeschke)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
grpc not over quic: Indeed, I was confused. All fixed...I think.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @jiceatscion, @lukedirtwalker, and @nicorusti)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, actually, applications are also using grpc, e.g. JPAN uses Grpc directly, and PAN/snet applications use it indirectly via the daemon.
Another proposal: I wonder what the relevance of grpc (and QUIC) is in this context. Maybe the easiest is to remove GRPC from the diagram? And maybe remove QUIC as well?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r3, all commit messages.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @jiceatscion and @nicorusti)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nicorusti and @tzaeschke)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure how much detail we want to show in the diagram but the protocol stacks for the control service are as follows:
- Intra-AS: gRPC/HTTP2/TCP/IP
- Inter-AS: gRPC/HTTP2/QUIC/UDP/SCION
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @nicorusti and @tzaeschke)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What I can do is show intra-AS communications as a dotted line directly b/w application and control service; if it's not going through SCION, no need to complicate matters. For inter-AS I can summarize grpc http and quic into a single box. I think it is important to mention that there is middleware there, but details are not so relevant indeed. Let me try again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: 4 of 5 files reviewed, 1 unresolved discussion (waiting on @nicorusti and @tzaeschke)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How's that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed 1 of 1 files at r4, all commit messages.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @nicorusti)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewable status: complete! all files reviewed, all discussions resolved (waiting on @nicorusti)
Added a summary of the stack and a summary of the topology.