osmo-bsc-nat/88cc1a2dbad497c84f09ec3b513cdca57b314160

{
  "comments": [
    {
      "unresolved": true,
      "key": {
        "uuid": "ba4f9ca7_879604ff",
        "filename": "src/osmo-bsc-nat/bssap_conn.c",
        "patchSetId": 4
      },
      "lineNbr": 47,
      "author": {
        "id": 1000005
      },
      "writtenOn": "2022-06-08T10:58:03Z",
      "side": 1,
      "message": "above we traverse the TLV structure once to store tlv_parsed entries.\ntlv_parsed entries are good for \"random access\", find a specific IE.\nThen we traverse the TLV again to store the tags in their order,\ncaching in an array. Then we iterate the array again, which is sort\nof identical to traversing the TLV data itself.\n\nit seems to me that it makes more sense to not use tlv_parse() at all,\nbut only do tlv_parse_one() and tlv_encode_one() in this loop.\nThen we don\u0027t need to use a random-access API for sequential access.\n\nHence not needing new libosmocore API.\n\nI guess this doesn\u0027t really affect performance in a noticeable way,\nmaking it a bikeshed of sorts?",
      "revId": "88cc1a2dbad497c84f09ec3b513cdca57b314160",
      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "ba2b9a7d_efa30c14",
        "filename": "src/osmo-bsc-nat/bssap_conn.c",
        "patchSetId": 4
      },
      "lineNbr": 47,
      "author": {
        "id": 1000005
      },
      "writtenOn": "2022-06-08T11:03:30Z",
      "side": 1,
      "message": "About leaving all other known and unknown IEs unchanged: I think there can theoretically also be unknown IEIs in the TLV which tlv_parse() skips? not entirely sure about that though.",
      "parentUuid": "ba4f9ca7_879604ff",
      "revId": "88cc1a2dbad497c84f09ec3b513cdca57b314160",
      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "7ada68e1_4223f2f1",
        "filename": "src/osmo-bsc-nat/bssap_conn.c",
        "patchSetId": 4
      },
      "lineNbr": 47,
      "author": {
        "id": 1000074
      },
      "writtenOn": "2022-06-08T11:12:21Z",
      "side": 1,
      "message": "The best approach is probably locating the important IE start of buffer by using tlv_parse, then simply modify that IE buffer with the new content and resend the buffer as it is, no need to encode it again?",
      "parentUuid": "ba2b9a7d_efa30c14",
      "revId": "88cc1a2dbad497c84f09ec3b513cdca57b314160",
      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "cbae2127_e8bcd74b",
        "filename": "src/osmo-bsc-nat/bssap_conn.c",
        "patchSetId": 4
      },
      "lineNbr": 47,
      "author": {
        "id": 1000147
      },
      "writtenOn": "2022-06-08T11:35:04Z",
      "side": 1,
      "message": "The length of the IE is different depending on IPv4 or IPv6 address being encoded (3GPP TS 48.008 § 3.2.2.102). So I can\u0027t just modify the existing buffer without re-encoding, I\u0027ll implement Neels approach. Thanks for the feedback!",
      "parentUuid": "7ada68e1_4223f2f1",
      "revId": "88cc1a2dbad497c84f09ec3b513cdca57b314160",
      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "72ef77e4_0d814e8c",
        "filename": "src/osmo-bsc-nat/bssap_conn.c",
        "patchSetId": 4
      },
      "lineNbr": 47,
      "author": {
        "id": 1000074
      },
      "writtenOn": "2022-06-08T11:39:43Z",
      "side": 1,
      "message": "Of course you can, it\u0027s just a matter of memmove the buffer a few bytes left/right and changing the len value of the IE in the buffer. And maybe some general \"len\" field at the message level. The rest of the IEs/message in the buffer should be independent of that change.",
      "parentUuid": "cbae2127_e8bcd74b",
      "revId": "88cc1a2dbad497c84f09ec3b513cdca57b314160",
      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "8b62b779_d5b703b7",
        "filename": "src/osmo-bsc-nat/bssap_conn.c",
        "patchSetId": 4
      },
      "lineNbr": 47,
      "author": {
        "id": 1000074
      },
      "writtenOn": "2022-06-08T11:40:34Z",
      "side": 1,
      "message": "That\u0027s probably the best approach to be as transparent as possible leaving the whole message untoched, passing through unknown IEs, etc.",
      "parentUuid": "72ef77e4_0d814e8c",
      "revId": "88cc1a2dbad497c84f09ec3b513cdca57b314160",
      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "f0165b8f_2b43f1ff",
        "filename": "src/osmo-bsc-nat/bssap_conn.c",
        "patchSetId": 4
      },
      "lineNbr": 47,
      "author": {
        "id": 1000074
      },
      "writtenOn": "2022-06-08T11:40:55Z",
      "side": 1,
      "message": "See osmo-hnbgw, dexter did it there for RTP afair.",
      "parentUuid": "8b62b779_d5b703b7",
      "revId": "88cc1a2dbad497c84f09ec3b513cdca57b314160",
      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "07976146_4c87bec5",
        "filename": "src/osmo-bsc-nat/bssap_conn.c",
        "patchSetId": 4
      },
      "lineNbr": 47,
      "author": {
        "id": 1000147
      },
      "writtenOn": "2022-06-08T12:23:44Z",
      "side": 1,
      "message": "I\u0027ve looked at the osmo-hnbgw implementation, it\u0027s for ASN.1.\n\nI\u0027ve updated the patch to decode each IE at a time, if it\u0027s the aoip transp layer addr, encode the new address, otherwise re-encode the original IE.\n\nReplacing the IE inside the buffer seems like a hack to me since to locate it correctly we already need to be able to parse the IEs coming before it with the tlv definition currently built against. And we can\u0027t do that reliably if there are unknown IEs and if they do not have TLV_TYPE_TLV but another format like TLV_TYPE_FIXED, TLV_TYPE_T, TLV_TYPE_TV etc. So we may end up decoding garbage and finding what looks like the aoip transp addr IE by chance.",
      "parentUuid": "f0165b8f_2b43f1ff",
      "revId": "88cc1a2dbad497c84f09ec3b513cdca57b314160",
      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
    },
    {
      "unresolved": true,
      "key": {
        "uuid": "04835439_e1639fc9",
        "filename": "src/osmo-bsc-nat/bssap_conn.c",
        "patchSetId": 4
      },
      "lineNbr": 47,
      "author": {
        "id": 1000005
      },
      "writtenOn": "2022-06-08T17:09:58Z",
      "side": 1,
      "message": "Pau is correct that it would indeed be possible to copy the part leading up to the changed IE and then copy the remaining part after the changed IE (or even memmove in the existing buffer), but i think it would be more code than the current patch set. i like the simplicity of the current patch set.",
      "parentUuid": "07976146_4c87bec5",
      "revId": "88cc1a2dbad497c84f09ec3b513cdca57b314160",
      "serverId": "035e6965-6537-41bd-912c-053f3cf69326"
    }
  ]
}