ynl-gen-cpp: add support for generating rt-addr #7
Conversation
zb-gong
changed the title
zb-gong
force-pushed
the
cpp-feature
branch
9 times, most recently
from
518e170 to
73012a4
Compare
|
Could you rebase and reorder these two patches? Add the cleanup first? There are some merge marker removals there which TBH I don't fully understand (as in I don't see where they were commited but now they are being removed). The commits should follow the Linux kernel format: |
|
Thanks for the review! I reordered the patches and updated the commits. I did not see any merge markers removed in either patch, please let me know if I miss anything. |
|
Please remove the binary file. The values reported by the sample look quite different to what the C sample from the kernel prints :( |
zb-gong
force-pushed
the
cpp-feature
branch
4 times, most recently
from
7c33824 to
0b0eec0
Compare
|
Removed the binary. |
ynl-gen-cpp.py
Outdated
| f'struct {c_lower(self.get("struct"))}*data = (struct {c_lower(self.get("struct"))}*)ynl_attr_data(attr);', | ||
| f"count = (len % struct_sz > 0) ? len / struct_sz + 1 : len / struct_sz;", | ||
| f"{var}->{self.c_name}.resize(count);", | ||
| f"memcpy({var}->{self.c_name}.data(), data, len);", |
There was a problem hiding this comment.
The length stuff doesn't look right here.. Do you see kernel responding with more than one cache info?
The reason for the length related dance in the C code is that the struct may grow or shrink (new members added at the end), and the library must be compatible with that. So we carefully allocate "at least" the number of bytes that the struct has in the uAPI headers that the build is against. And we set the len member to how much of the struct kernel actually sent us. The user of the library can then do something like
if (rsp->_len.field < offsetofend(struct x, member))
return x; // member of interest is not reported by the kernel
Having said that, obviously for C it's a pointer. So if the kernel returned an array of structs and user knows that they can simply index the pointer like an array...
There was a problem hiding this comment.
I see. Thanks for the context. Fixed it!
There was a problem hiding this comment.
The fix memcpy the partial data if the struct grows (len > struct_sz) and copies len bytes to cacheinfo if the struct shrinks (len < struct_sz).
Please let me know if the current fix makes sense. This is not exactly the same as C implementation. If it has to follow the exact behavior of C implementation, I think it has to play with pointer like C does and adds an extra length field.
There was a problem hiding this comment.
Any reason for sticking to the vector? The vector would only ever have zero or one entry, right?
cc: @fomichev
There was a problem hiding this comment.
Haven't looked for too long, but yes, we've been using std::optional for these types of things. Should be similar here: emplace(empty_value) and memcpy the parts you want.
There was a problem hiding this comment.
Yeah I was thinking about it, but I found there were some dependencies (e.g. .size()). So there will be more changes due to that.
Yeah I will make it optional and deal with the dependencies. Probably will split the patch.
zb-gong
force-pushed
the
cpp-feature
branch
2 times, most recently
from
28c481f to
0e12297
Compare
This patch introduces the `TypeBinaryStruct` class to handle attributes that are of binary type and structured as a `struct` in the `rt-addr`. Unlike the standard `TypeBinary`, `TypeBinaryStruct` has a defined structure, so the `struct_member` should be `optional<T>` and the `presence_type` is `flag`. Signed-off-by: Zibo Gong <[email protected]>
|
Split the patch: the first introduced the |
This patch add basic support for netlink-raw protocol. The support can correctly generate rt-addr. To test that, a rt-addr test added in samples/. Signed-off-by: Zibo Gong <[email protected]>
3 participants
Add support for generating rt-addr:
ynl_cpp::ynl_socket& ys..instead of->attr_listwith a fixed header. In that case, the struct should be generated as well.The other changes are auto-formatted by vscode.