topic descriptor documentation

[sjames]

Hi,

I now have a better handle of Rust procedural macros and I am reviewing my Rust binding. I am aiming for a seamless use of DDS from Rust and want to skip idlc. I plan to write an attribute or derive macro that will generate the code to bind to cyclonedds directly without invoking the codegenerator. The key to achieving this is to understand the operators used in the _ops structure.

static const uint32_t HelloWorldData_Msg_ops [] = { DDS_OP_ADR | DDS_OP_TYPE_4BY | DDS_OP_FLAG_SGN | DDS_OP_FLAG_KEY, offsetof (HelloWorldData_Msg, userID), DDS_OP_ADR | DDS_OP_TYPE_STR, offsetof (HelloWorldData_Msg, message), DDS_OP_RTS };

I have a basic understanding of this as I attempted to implement a rust idlc (https://github.com/sjames/cyclonedds-idlc). I suspended this when I learned @k0ekk0ek was implementing a C++ version. My sole motivation was to avoid using Java. My motivations have changed now.

Before I dive into this Yak shaving activity, I was wondering if the _ops structure is documented somewhere. I can imagine this getting complicated for complex types.

Regards, Sojan

[k0ekk0ek]

Hi @sjames! The best way to understand the opcodes is to have a look at dds_opcodes.h. There's some comments in there that should make life a little easier. However, you may want to consider postponing that as there's a new serializer in the works to fully support extensible types in Cyclone DDS. I believe the existing opcodes will keep being supported, but ymmv. @dennis-adlink is working on the aforementioned and he might be able to give you some pointers. That being said, the opcode interface won't change often but is best not considered a stable programming interface. The route that I'd take (feel free to deviate) is to implement a serializer for Rust. The C++ and Python bindings do so too. The new idlc supports plugins. The idea behind this is to have one frontend for parsing IDL (yes there's the Bison dependency, but at least it's not a runtime dependency and a pretty minor one to have at build time) and have a generator for each specific binding. Each binding then implements a means to serialize the language constructs to CDR via the sertype/serdata interface because that's the most optimal way (only one copy involved). You'll gain performance if you don't go through the vm as well.

Based on the route you want to take. If the intent is to shave the Bison dependency, we'd be open to using a handwritten parser instead if one would suddenly be available :slightly_smiling_face: If the intent is to write the best possible Rust binding, I'd write a generator for idlc and provide my own Rust serializer (we'd be willing to help out there too, for completeness there this too, and this might be something to have a look at). If you prefer to write you're own Rust based IDL parser+generator (which can be a fun excercise), the best route is probably to generate the types and write a serializer.

For writing a serializer, I bet there's something available within the Rust community. The best way forward is then to have a look at the Cyclone DDS C++ binding (datatopic.hpp) which details how to output/inject CDR blobs directly.

My answer is perhaps a bit much, but I wanted to give you a complete overview. I'm happy to provide more information based on the direction you choose.

[sjames]

Wow, thank you for the detailed reply. I was not aware of the sertype/serdata interface. Can you give me some pointers on this?

My intention is to skip the IDL. I want to create structs and enums in Rust, annotate them with a macro that generates the bindings. I thought that the only way was to create the op-codes, but now I should definitely look at sertype/serdata.

Whether I do this using the idlc or whether I write my own - I'll keep that decision pending.

[eboasson]

Hi @sjames, the documentation on that is really very limited and what little there is probably only makes sense once you figured it out anyway. So the best pointers are really the various places where it gets used:

• https://github.com/eclipse-cyclonedds/cyclonedds/blob/master/src/core/ddsi/src/ddsi_serdata_default.c and https://github.com/eclipse-cyclonedds/cyclonedds/blob/master/src/core/ddsi/src/ddsi_sertype_default.c (the "original" implementation of the interface)
• https://github.com/eclipse-cyclonedds/cyclonedds/blob/master/src/core/ddsc/tests/cdr.c (for testing purposes, also a little bit in the hope that it would give a more accessible implementation)
• https://github.com/eclipse-cyclonedds/cyclonedds-cxx/blob/templated-streaming/src/ddscxx/include/org/eclipse/cyclonedds/topic/datatopic.hpp (templated implementation, the C++ backend of the IDL compiler uses this)
• https://github.com/eclipse-cyclonedds/cyclonedds-python/blob/master/src/cyclonedds/clayer/src/pysertype.c
• https://github.com/ros2/rmw_cyclonedds/blob/master/rmw_cyclonedds_cpp/src/serdata.cpp (ROS 2 doesn't do key fields, so this implementation is missing a few things).

I do have a feeling I should give you a bit of a backgrounder as well, but now is not a good time to write that. What I can say is that the key idea is that the Cyclone core code deals with struct ddsi_serdata pointers with (effectively) a vtable pointing to the operations that can be performed on it.

The key operations are constructing a ddsi_serdata from an application sample and from serialized representation, and vice-versa, but there is a fair bit of extraneous stuff as well. For example, an untyped/type-erased version also exists, which is used for maintaining the key-to-instance-handle mapping (because a mapping entry can be used for multiple types (of the same implementation) and can outlast the lifetime of the one based on which it was created). One thing to note is that once you write your own implementation and use that exclusively, then you know the internals of the representation (thanks to https://github.com/eclipse-cyclonedds/cyclonedds/blob/a840e0a1c34f6e667f6f4bb67425e1c746b52d5d/src/core/ddsi/src/ddsi_serdata.c#L40) and can use that. The C++ and ROS 2 bindings definitely use of that.

Also note that this interface grew out of a code base in which the separation between the notion of a sample and its implementation didn't exist, and it shows in the interface in the form of some unpleasant implementation details. The intent is to fix that after the 0.8.0 release. The version marker you must place at the beginning of the struct ddsi_serdata_ops and struct ddsi_sertype_ops exists to allow updating this interface while maintaining backwards-compatibility, but you would probably want to update rather than stick with the old one.

It is almost a given that you will run into some gotchas when you start on implementing this interface: everybody, including myself did. So please don't hesitate and ask for more information when you have questions or see strange behaviours!

[sjames]

Thank you @eboasson and @k0ekk0ek. This makes sense. I like this so much more than the opcodes and this is actually going to make things easier. I think I should be able to use a serde cdr serializer/deserializer. Which version of CDR does cyclone use?

I'm sure I will have questions when I try this out.

[sjames]

I've started to look at the C++ and Python bindings. I think I get the idea, but I seem to be stuck on one point.

Referring this piece of code in https://github.com/eclipse-cyclonedds/cyclonedds-cxx/blob/ecec923fd56780f7f6300ee8b7be9e522dffe710/src/ddscxx/include/org/eclipse/cyclonedds/topic/datatopic.hpp#L206 (the python binding has the same implementation)

the NN_RMSG_PAYLOADOFF and NN_RDTA_PAYLOAD_OFF macro treat the fragchain->rmsg as a byte buffer, but struct nn_rmsg defined in https://github.com/eclipse-cyclonedds/cyclonedds/blob/f879dc0ef56eb00857c0cbb66ee87c577ff527e8/src/core/ddsi/include/dds/ddsi/q_radmin.h#L67

Is not a byte buffer.

memcpy is used to copy data after the offsets are computed. What am I missing?

Both the python and the C++ implementation seem to be copying the content into a buffer.
https://github.com/eclipse-cyclonedds/cyclonedds-python/blob/f42d6b6679f7681437f499ce8aa4200219381db9/src/cyclonedds/clayer/src/pysertype.c#L250

I was wondering if there was a way to avoid this copy and directly deserialize into the sample.

[eboasson]

Hi @sjames!

It's the ugliest thing, that nn_rmsg ...

RTPS messages that arrive over the network are received in large-ish buffers that can typically hold multiple messages simultaneously and which are managed by a custom allocator (*). The nn_rmsg struct is allocated from there and is immediately followed by the raw RTPS message. (Once upon a time there was an array of unsigned chars at the end — see 3afce30c370532d27ffe1838b6618f0649bfbda5 — but that turned out to be relying on a gcc/clang extensions.)

The simplified code that constructs these nn_rmsgs does:

• nn_rmsg *rmsg = nn_rmsg_new(...) to get a pointer to a sufficiently large block of memory with an initialized nn_rmsg
• size = recvfrom(socket, rmsg+1, 64*1024 /*max UDP*/, 0, NULL, NULL) to receive the UDP message in memory immediately following it
• nn_rmsg_setsize(rmsg,size) to "shrink wrap" the rmsg because most messages are far smaller than 64kB
• then it processes the message while recording any associated decoder state (DDSI receiver states, additional data for recovering order, an extended subset of DDS sample info, &c.) immediately following the message in the same block.

so while that byte buffer isn't visible in the type definitions, it really is there.

The data is copied because the original implementation stored the serialized data internally and only deserialized it during read/take calls (and to a lesser extent, because it is slightly easier to deserialize a single, contiguous message than a fragmented one), but there is no obligation to do that. Indeed, it turns out that the original considerations, leading to the decision to store the samples in serialized form internally, no longer match the currently important use cases and actually harm performance, but it hasn't been changed out of laziness. You're free to deserialize immediately, and it'll probably make your Rust binding the fastest 😁.

One thing that changed recently and is that the DDSI protocol no longer allows fragment sizes that are not a multiple of 8 bytes. That means you can now rely on the primitive types never being split across multiple fragments, making deserializing directly from fragments a lot more attractive.

(*) This is from either before Windows got scatter/gather interfaces for the network stack, or before I managed to find them. In any case, it was the days that Windows XP still was a thing.

[sjames]

Ok, that explains the macros and the offsets. :-) thanks.

If I manage to deserialize the sample without the memcpy, where do I store the sample?

I could hang on to the buffers (without the copy) and return it to the pool after the de-serialization, but I don't know if the interface allows it. I could deserialize and store the sample in the serdata structure and make it available when a sample is requested.

Any other ideas?

On a side note, I'm going to be working on this in August, so please prepare for a lot more questions. :-)

[k0ekk0ek]

Hi @sjames! It just so happens that I started looking into a Rust binding myself (I forked your cyclonedds-rs a week or so ago :slightly_smiling_face:), by looking at the language mapping itself. The rtps-gen by Frank Rehberger has a mapping that looks complete enough for DDS, except for how modules work. IDL modules are like C++ namespaces and can be reopened, which is not the case for Rust modules. It may not be all that relevant for you yet, but I can describe the way I think it could work, maybe you have some input too as I'm not very familiar with the Rust language.

The problem: Rust modules cannot be reopened (IDL can reopen from different files even) and the IDL compiler cannot update .rs files that are already there, so we/I need a way to write updates to modules and have the same result whether someone processes a file once or multiple times and add new types, constants etc if introduced from different IDL files.

My current approach would be to create a directory for each module. e.g. foo for a module named foo and create a mod.rs file in that directory that includes everything from foo/.mod. .mod was chosen because it's not an identifier so it doesn't clash with other modules, which would be stored under foo directly. Every definition in foo could then be stored in foo/.mod/<identifier>. I've created a proof-of-concept to see whether it'd work.

Disclaimer: It's terrible code (not very familiar with Rust yet), but it does work. Btw, it's inspired by the automod crate. module.zip

[sjames]

@k0ekk0ek I used code from rtps-gen for my experimental cyclonedds-idlc (https://github.com/sjames/cyclonedds-idlc.git). You can enable this generator in https://github.com/sjames/cycloneddscodegen.git by enabling the "rust_codegen" feature.

About multiple files: Would this happen during separate runs of the IDL compiler, or during a single run when one IDL may include another? What if you keep the module structure in-memory and update this as you parse new files. You can generate code after the in-memory structure is finalized.

I would prefer to avoid creating the generated files in the source tree. Instead, I'd generate the file at runtime via build.rs and inject it into the build from lib.rs like this. https://github.com/sjames/cyclonedds-sys/blob/8503a4008809c9831541627a398825b82d2f1e51/src/lib.rs#L25 . The generated files will be in the build folder and gets regenerated on any change of the IDL, thanks to Cargo. https://github.com/sjames/cyclonedds-sys/blob/8503a4008809c9831541627a398825b82d2f1e51/build.rs#L515 for the line in build.rs where the OUT folder is used for generated files.

What if you create a single file with all the modules within it? (You don't need to have a file per module). This makes it easier to parse the file (using the syn crate). The quote crate may then help you modify it when generating the updated types. The modifications you intend to do in the filesystem could be done in-memory.

I'll take a look at module.zip.

It's been a while since I've worked on this codebase. I will be on a sabbatical from Aug and looking forward to some quality time doing some development.

Cheers, Sojan

[eboasson]

If I manage to deserialize the sample without the memcpy, where do I store the sample?

That's up to you. As long as you can work with pointers that can be interpreted as pointers to struct ddsi_serdata (i.e., feel free to have other data at fixed offsets from it, like C++ inheritance does) any scheme you can come up with goes as far as Cyclone is concerned. You control allocation and deallocation.

The C++ binding specializes it to a serdata<T> containing a T (if I'm not mistaken), but you could also specialize it to contain an Arc<T> (or would that be an Arc<Cell<T>>? 🤔 Clearly my rust needs a bit of polishing — with apologies for the meagre attempt at avoiding only the most obvious pun).

What really matters is how you want to return it to the application. Since you control the API, you have to option to default to returning an immutable, refcounted sample. That would be the nicest in my view, because that'll allow you to avoid copies.

I could hang on to the buffers (without the copy) and return it to the pool after the de-serialization, but I don't know if the interface allows it.

It is technically allowed, and I am quite sure it'll work fine, but I am not sure what the consequences would be when the samples are small. For huge ones — like the 10MB point clouds of today — it's probably fine. My reasoning here simply concerns the lifetime and possible fragmentation of those big buffers: if you hold on to them for a long time, I expect you can get ballooning memory use, but for huge samples, that seems less likely because a single sample then spans multiple buffers anyway.

One day, I'll switch to another scatter-gather implementation, one that really works with the operating system interfaces. Then, holding on to them should be fine in all cases.

I could deserialize and store the sample in the serdata structure and make it available when a sample is requested.

So yes, that, or a reference counted pointer that you can hand out multiple times. That is especially attractive when there are multiple readers for the same topic.

On a side note, I'm going to be working on this in August, so please prepare for a lot more questions. :-)

Yay! 😀

[sjames]

Some more reading and it all makes sense now! I think I'll deserialize the data and hang on to the deserialized type in the serdata struct. The samples will be an Arc.

The next bit of the puzzle is the key and the key-hash. Would you be able to give me a background on this please?

On a different note, the deserializer I'm writing seems to work only on network data. Is the shared-memory implementation of iceoryx hooked in transparently or would I need to write a different deserializer for it?

[k0ekk0ek]

At this point it's not transparent, but I wouldn't worry about shared memory right from the start as it's easier to get things going first. Also, data is not serialized to shared memory, the sample is the in language presentation.

[eboasson]

The next bit of the puzzle is the key and the key-hash. Would you be able to give me a background on this please?

The first bit is that a sample (ddsi_serdata) is either a full sample (SDK_DATA) or a "key only" one (SDK_KEY; a.k.a. an "invalid sample") for communicating disposes and unregisters. Given the API, &c., it is pretty obvious that you both cases need to be converted to/from CDR and sample representation.

The second is that I decided to maintain a global mapping of key values to instance handles, rather than a local one per data reader/writer (but what is the fun in that? it is what the spec says, and thus you always see them used in the context of a specific reader/writer in the API) — that's ddsi_tkmap/ddsi_tkmap_instance, implemented using a concurrent hash table. The argument here is that by some judicious choices in the hash functions and equality tests, I can make different topics with the same key value map to the same instance handle, which then makes it relatively easy to build "multi topics" at either application level or inside a reader history cache. (TMI? Who knows!)

That table requires two things:

• the entries you store must be just the key value, because otherwise you could hold on to a ton of memory for no benefit if the samples are large (e.g., if there is, say, an integer key and a octet sequence in a topic, and the first encounter of the key value is a sample a 100MB sequence, you might have to hold onto those 100MB because the instance, and therefore the instance handle, continues to exist even when the application has already taken it);
• the entries better not refer to a DDS topic or a ddsi_sertype because that desire to use the same mapping for the same key value across topics means you have to be able to interpret it any way you like (and besides there is the bit that topic definitions should disappear when no longer needed).

Hence the "untyped" variant (I admit that some of the details are a bit of quick hack, but the type erasure does make sense here). When a sample comes in, it is turned into a serdata, that one is then looked up by key value in the hash table. If its key isn't found, Cyclone invokes the to_untyped operation to create an untyped serdata (with only the key value in there) and stores that in the hash table.

The reader history cache doesn't actually store invalid samples for "not alive" samples (i.e., unregistered or disposed), instead encoding the state using a few bits in the instance. When a read or take operation needs to return an invalid sample to signal a state change not associated with any data, it invokes the untyped_to_sample operation. "Obviously", the regular to_sample for a serdata of type T is "typically" identical to untyped_to_sample for a untyped serdata invoked by a reader history cache of type T.

That leaves the hashes. There are two variants: the first is the struct ddsi_serdata::hash field which is supposed to be set to the hash of the key value of that serdata's sample. That is, it is what the aforementioned hash tables uses to lookup the key value. You're supposed to fill it — and it'd better be using a decent hash function or else. struct ddsi_sertype::serdata_basehash is supposed to be mixed into this to reduce the likelihood of collisions on completely different sertype implementations. (If the implementation is the same, presumably it knows how to test key values for equality, if the implementations are different, say for C and C++, then one can't expect the equality test to handle that combination. But if the equality necessarily returns false, it is attractive to try to avoid a hash collision.)

(See also https://github.com/eclipse-cyclonedds/cyclonedds/blob/f879dc0ef56eb00857c0cbb66ee87c577ff527e8/src/core/ddsi/src/ddsi_serdata_default.c#L113 and https://github.com/eclipse-cyclonedds/cyclonedds/blob/f879dc0ef56eb00857c0cbb66ee87c577ff527e8/src/core/ddsi/src/ddsi_serdata_default.c#L122)

Finally, there is the "(DDSI) keyhash", which is distinct from the hash of the key mentioned above. That thing (*) is defined in the DDSI spec (near the end of the document, I don't know the section number of the top of my head) but it basically comes down to: serialize the key value as big-endian CDR with all padding set to 0. Then, if the maximum serialized length of the key type is at most 16 bytes, that's your keyhash after padding it out to 16 bytes with 0s; else you compute the MD5 hash of it and use that.

Note that it concerns the maximum serialized length of the key's type, not its value. E.g., if the key type is an unbounded string the maximum serialized length is slightly over 4GB and it should you need to compute the MD5 hash, even if the sample has an empty string as its key, of which the serialized representation requires only 5 bytes.

The keyhash is only ever used when a DDS security is enabled with protection of the key value (clearly, it is infeasible to figure out the likely key value for a given MD5, even if you happen to guess the structure of the key and it only has a few valid values, like track identifiers in air traffic management systems ...), if there is a remote reader that requests it (which is, I think, a Cyclone extension), or if you force its inclusion in data messages using the configuration. It is not used for anything internal.

(*) I wanted to write "monstrosity" but decided that it does potentially serve one useful purpose in bridging networks, so calling it that may be over the top.

[sjames]

@eboasson : very clear explanation, thank you. I think this is more than enough to set me on my way.

I think it is possible to mark multiple fields as a key, if that is the case, should the hashes be computed over the contents of all the fields that are marked as keys?

[eboasson]

Yes — keeping in mind that for the Cyclone-internal hash you're free to implement it however you want, but for the DDSI keyhash you have to respect CDR alignment rules. Of course you're free to effectively compute the DDSI keyhash and derive the internal hash from it, like in "ddsi_serdata_default.c".

If you do that, then it is tempting to also use those 16 bytes as a proxy for the actual key in comparing two samples' key values, like happens in "ddsi_serdata_default.c ", but that is actually not a good idea. If you define a topic

struct {
  octet k[64]; //@key
};

then publish two samples with different contents that just accidentally happen to produce an MD5 collision (trivially generated these days if you have 64 bytes), things break in a rather amusing way. Just for laughs you could add a dispose, too, of course 🥲

In short, I definitely need to fix "ddsi_serdata_default.c" to not ever rely on DDSI keyhash to determine equality ... it is a bit of inherited code and in practice doesn't seem to break things and that makes it have a lowish priority. Please don't repeat that ancient mistake in the Rust code!

[sjames]

Understood.

When kind == SDK_KEY, what is in the CDR stream? Is it the serialized stream of just the keyhash?

https://github.com/eclipse-cyclonedds/cyclonedds-cxx/blob/ecec923fd56780f7f6300ee8b7be9e522dffe710/src/ddscxx/include/org/eclipse/cyclonedds/topic/datatopic.hpp#L126

[eboasson]

It is the actual key value. So if you have a topic

@topic
struct S {
  octet a; //@key
  double b;
  string c; //@key
  octet d;
};

and a value of { 0x20, 8.8e26, "space", 236 }, then, with thanks to Perl 🙂, in little-endian for SDK_DATA you should get:

[-4] 0x00 0x01 0x00 0x01
[ 0] 0x20    p    p    p
[ 4]    p    p    p    p
[ 8] 0x87 0x03 0xb7 0xfc
[12] 0x59 0xbf 0x86 0x45
[16] 0x06    0    0    0
[20]  's'  'p'  'a'  'c' 
[24]  'e' '\0' 0xec    x

where 'p' is ordinary internal padding and 'x' is padding at the end. Both may have any value (so you can also explicitly clear it, which is what I do for the C code to avoid possibly leaking some data and also to keep on the good side of valgrind when passing the buffer to sendmsg).

The padding at the end is because the RTPS specification has all elements aligned to a four byte boundary. It is currently actually copied out by Cyclone, so it should be present even if it is highly likely that no harm is done if it reads those few bytes beyond the data.

It used to be that there was no information in the message on how much padding had been added at the end to reach this boundary, which made it, e.g., possible for the writer to publish one octet and for a receiver (with a mismatching type definition) to deserialize two octets. So now the number of padding bytes added at the end to reach a 4-byte boundary is encoded in the least significant 2 bits of the 2nd byte of the CDR "encoding options".

That also hopefully explains why I have the "[-4]" offset: that's the CDR encoding and options. The offsets here are the offsets to be used for alignment calculations (so the first byte of the payload is considered to be at offset 0 for alignment calculations, and the encoding information is therefore effectively at offset -4).

To keep things confusing, the message processing of Cyclone then treats all this as a blob, considering the encoding header to start at offset 0 and the size (therefore) to be 4 bytes larger than the size of the actual data. (A nasty quirk that has bitten everyone who played with this. That's including me 😖🤓.)

For SDK_KEY for the same sample, you'd get:

[-4] 0x00 0x01 0x00 0x02
[ 0] 0x20    p    p    p
[ 4] 0x06    0    0    0
[ 8]  's'  'p'  'a'  'c' 
[12]  'e' '\0'    x    x

For the DDSI keyhash, as the serialized form is not guaranteed to fit in 16 bytes for this type, it has to be the MD5 of the big-endian serialization with padding cleared:

MD5(
  [ 0] 0x20    0    0    0
  [ 4]    0    0    0 0x06
  [ 8]  's'  'p'  'a'  'c'
  [12]  'e' '\0')

so that's:

• no encoding header
• big-endian CDR
• padding cleared
• no padding out to a multiple of 4 bytes

all very convenient and no risk whatsoever of confusion. If instead the type had been:

@topic
struct S {
  octet a; //@key
  double b;
  string<5> c; //@key
  octet d;
};

then the maximum serialized length of the key would have been (1 [field a] + 3 [pad] + 4 [act. length of c] + 5 [contents of c] + 1 [string terminator]) = 14 ≤ 16 bytes and therefore the DDSI keyhash would simply be:

[ 0] 0x20    0    0    0
[ 4]    0    0    0 0x06
[ 8]  's'  'p'  'a'  'c'
[12]  'e' '\0'    0    0

(padded out with 0s to 16 bytes).

I guess that should answer the question in sufficient detail. (@reicheratwork @thijsmie I am sorry I didn't make this effort before, I'm sure it would've been helpful to you, too ...)

--

cobbled together perl script for the above:

$t = "C x![d] d x![L] L/Z* C"; # for data
@d = (0x20,8.8e26,"space",236);
#$t = "C x![L] L/Z*"; # for key
#@d = (0x20,"space");
($u = lc $t) =~ y/fd/lq/; $u =~ s;l/z\*;l a* c;g; # for formatting
for (@d) { # make input for formatting template; strings better not look like numbers
  push @e, -1;
  push @e, ("x"x(length $_), 1) if ($_ eq '' || ($_ ^ $_)); # presumably a string
}
@x=unpack("C*",pack($t,@d)); @y=unpack("C*",pack($u,@e));
printf "[-4] 0x%02x 0x%02x 0x%02x 0x%02x\n", 0, 1, 0, (((@x%4) == 0) ? 0 : (4-(@x%4)));
for(my $i = 0; $i < @x; $i += 4) {
  printf "[%2d]", $i;
  for(my $j = 0; $j < 4; $j++) {
    do { print "    x"; next } if $i+$j >= @x;
    $x = $x[$i+$j]; $y = $y[$i+$j];
    if ($y == 0) { print "    p"; }
    elsif ($y == 1) { print " '\\0'"; }
    elsif ($y < 255) { printf "  '%c'", $x; }
    elsif ($x == 0) { print "    0"; }
    else { printf " 0x%02x", $x; }
  }
  print "\n";
}

[sjames]

@eboasson

I've made some progress, but there are two methods in the interface I could not figure out

1. serdata_from_keyhash : Is this to create an untyped serdata from the keyhash?
2. untyped_to_sample : The only way we can create a sample from untyped serdata is to create a sample with default values. Is this the intention?

If interested, you can check out progress in cyclonedds-rs:

1. Rust deserializer: https://github.com/sjames/cyclonedds-rs/blob/master/src/serdes.rs
2. Test to create topic using the new deserializer : https://github.com/sjames/cyclonedds-rs/blob/33ff7c176cacc8ac59c10d20616d9b660596d50c/src/dds_topic.rs#L93
3. Derive macro : https://github.com/sjames/cyclonedds-rs/blob/c3a29804e3cf84538b24f7aad34960a76cb1911b/dds_derive/src/lib.rs#L26

I just did the refactoring and will probably start testing later this week.

Regards, Sojan

[eboasson]

Hi @sjames

I've made some progress, but there are two methods in the interface I could not figure out

1. serdata_from_keyhash : Is this to create an untyped serdata from the keyhash?

Oops. I'd suppressed all memories of it because it really shouldn't exist. It is only there because the primary author of the DDSI spec refuses to abide by it (unless something changed recently), sending disposes and unregister messages with nothing but a DDSI keyhash rather than the serialized key value that is required.

This function provides an alternate route to create an SDK_KEY serdata, with the expectation that it fails (i.e., returns a null pointer) in case of a type where the keyhash is the MD5 hash of the actual key value.

untyped_to_sample : The only way we can create a sample from untyped serdata is to create a sample with default values. Is this the intention?

It is always called with the type needed as an argument, so you can still fill in the key values. This is something that Cyclone does despite the spec saying the data is completely invalid if valid_data in the sample info is false:

[2.1.2.5.1.4] To ensure corerctness and portability, the valid_data flag must be examined by the application prior to accessing the Data associated with the DataSample and if the flag is set to FALSE, the application should not access the Data associated with the DataSample, that is, the application should access only the SampleInfo.

The problem with the spec is that when you get an invalid sample, the instance handle need no longer be valid either. The typical case for that is when the writer does a "dispose" followed by an "unregister" and the application takes the sample after the "unregister". Then, because there are no live writers, the middleware may (and I'd argue must eventually, and so might as well immediately) reclaim any resources for the instance. That includes the instance handle — even if, in Cyclone at least, the instance handle may survive because of other readers, other topics, &c.

That in turn means that an application that asynchronously takes everything it receives and that needs to know the key value of disposed instances — a perfectly reasonable application, even if I personally would recommend limiting the takes to garbage collection — must either go through the effort of maintaining its own instance handle-to-key value map, or must pick an implementation that cares enough to provide the actual key value also in invalid samples.

For example, in:

w = create_writer(T) // T just a single integer field k that is key
r = create_reader(T)
write(w, {k=1})
(info0, data0) = take(r)
dispose(w, {k=1})
unregister(w, {k=1})
(info1, data1) = take(r)
lookup_instance(r, info1.instance_handle)

info1.valid_data will be false and lookup_instance will fail because the instance handle is no longer valid.

[sjames]

Thanks @eboasson . I'm not sure I understood the previous email as this was deep into the specification and I know DDS superficially.

Good news is that I got the roundtrip example working with the Rust serializer! The roundtrip Pong is implemented in Rust and I use the cyclonedds RoundtripPing example. (I have not tested anything else yet)

Here is the macro invocation to create the topic: https://github.com/sjames/roundtrip-example/blob/a1f75bcf7a2515c3685fc46985bc87422b755e6e/src/main.rs#L18

`mod RoundTripModule { use cyclonedds_rs::{*};

[derive(Default, Deserialize, Serialize, Topic)]

     pub struct DataType
     {
         pub payload : Vec<u8>,
     }

}`

Just derive the "Topic' and you are done. You can use the module structure to build up a sensible path for the type. No dependency on the code generator. :-). The API mirrors the cyclone-dds API, but with this infrastructure in place, I have some ideas to make the API more ergonomic.

The Rust roundtrip-pong is at https://github.com/sjames/roundtrip-example.git . It works fine for small messages, but I see some strange errors when I bump up the message size. See below the errors when I used this command line for the ping. (./RoundtripPing 100000 0 0)

' Roundtrip Pong! Typename:"RoundTripModule::DataType" Listener hooked for data available 1627983029.992517 [0] recvUC: ddsi_udp_conn_write to udp/172.23.110.10:34867 failed with retcode -58 1627983029.996726 [0] recvUC: ddsi_udp_conn_write to udp/172.23.110.10:34867 failed with retcode -58 1627983030.000075 [0] recvUC: ddsi_udp_conn_write to udp/172.23.110.10:34867 failed with retcode -58 1627983030.004412 [0] recvUC: ddsi_udp_conn_write to udp/172.23.110.10:34867 failed with retcode -58 1627983030.471612 [0] tev: ddsi_udp_conn_write to udp/172.23.110.10:34867 failed with retcode -58 1627983030.575318 [0] tev: ddsi_udp_conn_write to udp/172.23.110.10:34867 failed with retcode -58 1627983030.676001 [0] tev: ddsi_udp_conn_write to udp/172.23.110.10:34867 failed with retcode -58 1627983030.687795 [0] tev: ddsi_udp_conn_write to udp/172.23.110.10:34867 failed with retcode -58 .....continues '

[eboasson]

Thanks @eboasson . I'm not sure I understood the previous email as this was deep into the specification and I know DDS superficially.

No worries — besides, the superficiality will undoubtedly disappear as you continue with your project!

Good news is that I got the roundtrip example working with the Rust serializer! The roundtrip Pong is implemented in Rust and I use the cyclonedds RoundtripPing example. (I have not tested anything else yet)

Congratulations! The biggest hurdle has been taken, the rest is mere details.

Just derive the "Topic' and you are done. You can use the module structure to build up a sensible path for the type. No dependency on the code generator. :-). The API mirrors the cyclone-dds API, but with this infrastructure in place, I have some ideas to make the API more ergonomic.

So now we have Rust and Python as proper languages where you can define topics the way it should be done 😁. (Now all I need to do is add Haskell and we're all set!) Regarding the ergonomics, just do something that feels right for Rust. Apart from a few terrible mistakes, the C API of Cyclone is simply the way it is because it fits with how one (I, anyway) writes C, and I'm sure that idiomatic Rust would lead to something else.

1627983029.992517 [0] recvUC: ddsi_udp_conn_write to udp/172.23.110.10:34867 failed with retcode -58

That's interesting: -58 maps to https://github.com/eclipse-cyclonedds/cyclonedds/blob/f879dc0ef56eb00857c0cbb66ee87c577ff527e8/src/ddsrt/include/dds/ddsrt/retcode.h#L72 and tracing it back, it must have come from https://github.com/eclipse-cyclonedds/cyclonedds/blob/f879dc0ef56eb00857c0cbb66ee87c577ff527e8/src/ddsrt/src/sockets/posix/socket.c#L492

The Linux man page says:

EMSGSIZE: The socket type requires that message be sent atomically, and the size of the message to be sent made this impossible.

but it should never produce an RTPS message larger than max(General/MaxMessageSize,General/FragmentSize+a bit of overhead) and the defaults for those are just under 15kB and 1344B. That should always work on UDP, no?

If you could reproduce that test with Cyclone's tracing enabled using

<Tracing>
  <Category>trace</Category>
  <OutputFile>cdds.log.${CYCLONEDDS_PID}</OutputFile>
</Tracing>

you should be able to find that error message. I'd be interested what it says just before that, where it constructs the message. There should be enough information in there to ascertain what sizes it tried to send.

[sjames]

cdds.log.20086.zip

The file is attached. I used the same command line for ping.

./RoundtripPing 100000 0 0

[eboasson]

It is really trying to send a 100092 byte packet, and that indeed is not going to work:

1627985383.014308 [0]        tev: xpack_addmsg 0x7fa6a4000b60 0x7fa69401a390 0(rexmit(87a91001:c311f934:47695bac:103:#1/1)): niov 0 sz 0 => now niov 3 sz 100092
1627985383.014327 [0]        tev: nn_xpack_send 100092: 0x7fa6a4000b6c:20 0x7fa69401a478:64 0x7fa694001ce0:100008 [ udp/172.23.110.10:50818@4ddsi_udp_conn_write to udp/172.23.110.10:50818 failed with retcode -58
1627985383.014359 [0]        tev:  ]
1627985383.014362 [0]        tev: traffic-xmit (1) 100092

but why, given:

1627985366.183432 [0] roundtrip-: config: Domain/General/MaxMessageSize/#text: 65500 B {1}
1627985366.183434 [0] roundtrip-: config: Domain/General/MaxRexmitMessageSize/#text: 1456 B {}
1627985366.183436 [0] roundtrip-: config: Domain/General/FragmentSize/#text: 4000 B {1}

That's a completely new misbehaviour ...

💡

Got it! (I think anyway)

https://github.com/sjames/cyclonedds-rs/blob/de3a48073d56d4f014ef797f87dfbe8e3eaadd74/src/serdes.rs#L480

looks like the culprit: it seems to return a reference to the full sample, rather than to just the bytes requested by the offset and size parameters. (0 ≤ offset < ddsi_serdata_size() and offset + size ≤ ddsi_serdata_size() so it should be pretty straightforward to return a slice rather than the full buffer.)

[sjames]

🤦 That was the problem. Fixed now!

[sjames]

@eboasson @k0ekk0ek : I guess you could close this ticket now. There is a lot of good documentation here for when you write the Haskell binding. 😄

[eboasson]

@sjames

There is a lot of good documentation here for when you write the Haskell binding. 😄

I'm not sure the documentation was needed so much given that I am the primary culprit anyway, but this may have just given me the final push to give it a go, wasting some (🌧💨🥶) summer hours. (It seems this year all the warmth in Europe decided to party in the Mediterranean, instead of favouring a more equitable distribution across the continent, which is a bummer here and a problem down south ...). Anyway, for your amusement: https://github.com/eboasson/cyclonedds-haskell. It is far from complete, far from good, far from everything but a proof-of-concept, really! 😀

[sjames]

@sjames

There is a lot of good documentation here for when you write the Haskell binding. smile

I'm not sure the documentation was needed so much given that I am the primary culprit anyway, but this may have just given me the final push to give it a go, wasting some () summer hours. (It seems this year all the warmth in Europe decided to party in the Mediterranean, instead of favouring a more equitable distribution across the continent, which is a bummer here and a problem down south ...). Anyway, for your amusement: https://github.com/eboasson/cyclonedds-haskell. It is far from complete, far from good, far from everything but a proof-of-concept, really!

:-) . TBH, I just cannot get my head around Haskell. I tried to do something useful with it but hit a wall quickly... Rust was easy in comparison. I guess it is too computer sciency for me.

Talking about being bummed, I was supposed to be on a sailboat in the Canary islands by now... one of the main reasons for my sabbatical. :-(.