-
Notifications
You must be signed in to change notification settings - Fork 232
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
Add multigram draft 1 #123
Conversation
I'd be grateful for pointers to prior art, and most of all people who review this, point out open questions, etc. :) |
@@ -0,0 +1,96 @@ | |||
# Multigram -- protocol negotiation and multiplexing over datagrams | |||
|
|||
For multiplexing different protocols on the same datagram connection, multigram prepends a 1-byte header to every packet. This header represents an index in a table of protocols shared between both endpoints. This protocol table is negotiated by exchanging the intersection of the endpoint's supported protocols. The protocol table's size of 256 tuples can be increased by nesting multiple multigram headers. |
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.
Why not use a varint instead of a single byte?
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.
Parsing is not so much a problem, but the per-packet overhead should be constant -- for some cases, we'll want to inform the upper protocols about the overhead, so they can avoid fragmentation, or batch multiple small messages into one packet.
We could fit a larger table into to 2 bytes, but I expect that a connection with more than 255 protocols on the same layer will be extremely rare. These two bytes multiply the deeper you nest protocols, in IPFS we currently have 3 layers of multistream I think?
Does this make any sense?
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.
The maximum packet size would be bubbled up from the transports through all the protocol wrappers, e.g. 512 (UDP) - 1 (multigram) - 20 (cryptoauth) - 1 (multigram) - 12 (switch) - 1 (multigram) = 477
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.
hrm... okay. I see the importance of the single byte, but you do mention that you can 'expand' the table by nesting. Nesting seems more complicated than using a varint and informing other layers how much space they have left to consume
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.
Yes, with a good night's sleep, you're absolutely right.
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.
To explain what my concern was: I was thinking the varint width would vary on a packet-per-packet basis, while it'll really only change if you add protocols to the protocol table (i.e. change the multigram connection's state).
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 varint will make packet handling more complex then it should be, you introduce offset handling and unframing that depends on the state of connection.
If the state goes desync for what ever reason getting it back to sync will be extremely hard.
Having constant header file is very nice feature.
Also 255 overlay protocols (that are negotiated on need basis should be more than enough, if more is needed, layering is still an option).
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.
Mh yep we're trading off between a complication of header parsing, and a complication of protocol semantics. We should decide whether we favor performance (int8 field and nested multigrams) over a simpler protocol (varint and no nesting).
We should figure out exactly how complicated nesting is (make experiments), and also how heavy parsing a varint is vs. an int8 (benchmarks). My gut feeling is that we should probably go with an int8, because we'll have to do this for every packet, and multiple times per packet in many use cases.
Not-so-appealing idea: a varint degrades to an int8 nicely, so there might a way to leave it up to implementations.
todo
|
This should be added to the now stubbed out https://github.com/multiformats/multigram, and work should continue there. |
Will open another PR at https://github.com/multiformats/multigram -- plenty of great changes resulted from the discussions had in Lisbon. |
Is multigram still a thing or should it be deprecated? In that case, the https://github.com/multiformats/multigram should be archived and the README of https://github.com/multiformats/multiformats and the website should be updated. The last activity was around 2016/2017. |
@marten-seemann @mxinden thoughts? If we're not using it I agree with @ben221199 that we should make it clear they're not an ongoing concern. |
Thanks for the ping @rvagg. I have not been aware of the multigram project. With QUIC and libp2p/specs#349 I don't see us pushing for a datagram based protocol negotiation protocol any time soon. Thus I am fine with marking this effort deprecated. |
archived it, thanks for raising it @ben221199 |
Maybe I can also make a pull request for https://github.com/multiformats/website. |
Sorry for the late reply. I agree with closing / archiving. Datagram transports are probably better implemented on top of QUIC, which provides native datagrams (encrypted, congestion controlled, forward-secure) at the transport level. |
Braindump related to packet switching (ipfs/notes#143).