wire protocol header adjustments

27 views
Skip to first unread message

Pedro Larroy

unread,
Apr 2, 2014, 8:54:19 PM4/2/14
to clearsk...@googlegroups.com
Hi

I made some changes to the wire protocol header to make the size fields homogenous:

https://github.com/larroy/clearskies/commit/44baecdc9c00e9cb43e82c4a9710017aa7b916ba

I submitted a pull request, will change the C++ client accordingly.

Please review.

Pedro.

Steven Jewel

unread,
Apr 2, 2014, 11:51:59 PM4/2/14
to clearsk...@googlegroups.com
On 04/02/2014 06:54 PM, Pedro Larroy wrote:
> I made some changes to the wire protocol header to make the size fields
> homogenous

As a related issue, it seems like it'd make more sense to always do
[size][blob][size][blob] instead of what we are currently doing. So in
other words:

$[msg size]:[signature size]:[msg body][signature][binary payload]

would become:

$[msg size]:[msg body][signature size]:[signature][binary chunk 1
size]:[binary chunk 1]0:

Additionally, we'd make it be a consistent terminating character (we
could use newlines or colons or whatever character you'd like, it
doesn't matter to me.

I was just thinking this would simplify a few things in the
implementation, since reading a block would require the same code
regardless of whether it was json or a signature or a binary chunk. If
you don't think it will then it's not worth changing.

Steven

Pedro Larroy

unread,
Apr 3, 2014, 2:36:08 AM4/3/14
to clearsk...@googlegroups.com

Makes sense.

I will change accordingly and make the pull request again.

Pedro.

--
You received this message because you are subscribed to the Google Groups "ClearSkies Dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to clearskies-dev+unsubscribe@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Pedro Larroy

unread,
Apr 5, 2014, 4:21:14 AM4/5/14
to clearsk...@googlegroups.com
Please have a look at the discussion in: https://github.com/jewel/clearskies/pull/48

Should we use ascii or binary for lengths? If we move towards a binary protocol, we should consider using a more efficient encoding such as msgpack rather than json.

What do you guys think?

Pedro.

Steven Jewel

unread,
Apr 7, 2014, 12:41:28 PM4/7/14
to clearsk...@googlegroups.com
On 04/05/2014 02:21 AM, Pedro Larroy wrote:
> Please have a look at the discussion
> in: https://github.com/jewel/clearskies/pull/48
>
> Should we use ascii or binary for lengths? If we move towards a binary
> protocol, we should consider using a more efficient encoding such as
> msgpack rather than json.
>
> What do you guys think?

I think it's really up to you. I don't see a compelling argument on
either side unless someone wants to go to the trouble of changing the
code. In the meanwhile I think we'd be better off focusing on getting
something working first, and then we can optimize based on real-world
measurements.

msgpack seems like it'd be fine. I looked at using protobufs
originally, but it turns out that they're not as flexible as we'd want
for our protocol. While message types can be extended with new
attributes, it can only be done via a central authority, whereas I think
that for clearskies I'd be better if we have a wide-open forwards
compatibility.

Since msgpack can take raw bytes, we could simplify things further.
However, we'd probably still want to keep the chunked method of sending
binary data so that it's easy to stream large files.

I suggested this switch because the protocol is text-based for
historical reasons -- when I wrote the protocol originally there was no
implementation, and I didn't know how it'd be encrypted. Now that
things are getting more concrete it doesn't seem like it will matter, so
we might as well go with whatever is easiest for the C++ implementation.

Steven
Reply all
Reply to author
Forward
0 new messages