Attention is currently required from: arehbein, laforge, pespin.

View Change

2 comments:

• File src/ipa.c:

  • Patch Set #2, Line 378: MSG_CB_IPA_PROTO_EXT_IS_SET_OFFSET

    Can we simply have a packed struct instead of those? (answer is probably: yes! good idea!)

    We wouldn't need the IS_SET flag anyway, I'd expect proto_ext to only have a meaning if proto is IPAC_PROTO_OSMO.

    I like the idea of a packed struct, that way we only use up one cb for both proto and proto_ext.

    I think it would help to have macros to set/get proto/proto_ext from a msgb and use those.

    ```
    struct osmo_ipa_info {
      uint8_t proto;
      uint8_t proto_ext;
    } __attribute__ ((packed));

    #define OSMO_IPA_SET_INFO(msg, proto, proto_ext) \
    do { \
      struct osmo_ipa_info info = {
        .proto = proto,
        .proto_ext = proto_ext,
      };
      (msg)->cb[0] = info;\
    } while (false)

    ```

  • Patch Set #2, Line 432: /* Below: Helper functions for addition of send_ipa functionality in later commit */

    Specially in a .c file :) Looks like it should be removed.

    Iirc the idea was to introduce ipa_check_pull_headers() AND use it in the segmentation_cb().
    The other functions can go in a later commit where they are needed.

To view, visit change 33652. To unsubscribe, or for help writing mail filters, visit settings.