Change in osmo-upf[master]: libosmo-tlv: add versatile TLV de- and encoder

Attention is currently required from: pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-upf/+/27217 ) Change subject: libosmo-tlv: add versatile TLV de- and encoder ...................................................................... Patch Set 1: (3 comments) File include/osmocom/tlv/tlv.h: https://gerrit.osmocom.org/c/osmo-upf/+/27217/comment/d4989161_ba027bf6 PS1, Line 102: unsigned int tag; I added the struct osmo_tlv_load parameter later, to add flexibility. At first the intention was to limit the scope of this function to straight direct pointers. Let me see whether we should neutralize that argument... yes, just passing a tlv pointer works, thanks File src/libosmo-tlv/tlv.c: https://gerrit.osmocom.org/c/osmo-upf/+/27217/comment/20b76ce0_6f579fe4 PS1, Line 266: static int t16l16v_store_tl(uint8_t *dst_data, size_t dst_data_avail, unsigned int tag, size_t len, the point of the callbacks is that i do not want to load this API with a representation of what kinds of headers are possible. This function is merely a simple example for the caller. If more complex TL are needed, the caller plugs a corresponding implementation. In particular, if a protocol is mixed TLV with TV, we need some lookup for which T are TLV and which are TV. If a protocol has a variable L or T depending on T or even the value of L ... seen all of this, and it is cumbersome to invent data structures that abstractly represent all of these cases. IIUC, the Open5gs TLV parser is unable to deal with older GSM TLV because of this, where half the staff tended to nip off single bytes of protocol wherever they could, including T or L bytes. (while the other half of staff spammed the protocols with weird duplications, I guess) Using these callbacks is a very simple way to be completely flexible in what the TL part looks like, because the caller defines that. In fact, that makes this an arbitrary XYZV parser, because the headers need not be TL at all. The way how i could agree to avoid callbacks would be if we generate all of the TLV parsing code and put the header parsing implementations as static function in the generated code -- but they should still be provided by the caller. But why avoid these callbacks? Have you profiled that this causes substantial load? Have you seen that -O3 is not able to neutralize these far jmps? I am infinitely more concerned about top-down list iterations. Which we do, *a lot*. Seems to work out fine though so far. (This is the point i always make, remember) https://gerrit.osmocom.org/c/osmo-upf/+/27217/comment/c4cf1cf0_c1aa3c1c PS1, Line 275: osmo_store16be(tag, dst_data); not sure what you mean by "instance". Maybe the answer is, this function is just an example for one specific TL encoder? -- To view, visit https://gerrit.osmocom.org/c/osmo-upf/+/27217 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-upf Gerrit-Branch: master Gerrit-Change-Id: Ib0fd00d9f288ffe13b7e67701f3e47073587404a Gerrit-Change-Number: 27217 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 18 Feb 2022 13:05:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment