Attention is currently required from: pespin.

View Change

3 comments:

• File include/osmocom/tlv/tlv.h:

  • Patch Set #1, Line 102: unsigned int tag;

    Isn't this duplicated in load_tl callback?

    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:

  • Patch Set #1, Line 266: static int t16l16v_store_tl(uint8_t *dst_data, size_t dst_data_avail, unsigned int tag, size_t len,

    From my experience after looking at open5gs TLV parser, these callbacks can avoided. […]

    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)

  • Patch Set #1, Line 275: osmo_store16be(tag, dst_data);

    You may want to also support setting instance in the API, to avoid API breakage in the future when w […]

    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 change 27217. To unsubscribe, or for help writing mail filters, visit settings.