[L] Change in osmo-sgsn[master]: Add Gn support to allow MME->SGSN, SGSN->MME cell reselection

Attention is currently required from: lynxis lazus. laforge has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/39563?usp=email ) Change subject: Add Gn support to allow MME->SGSN, SGSN->MME cell reselection ...................................................................... Patch Set 6: (12 comments) Patchset: PS6: overall my gut feeling is that there's way too much open-coded pointer arithmetic and manual buffer/length/index handling for my taste. If we cannot use msgb for some reason (it has stuff like msgb_may_pull, msgb_tailroom, ...), then maybe there's some other/better helpers we can develop for this use case? File src/sgsn/gprs_gmm.c: https://gerrit.osmocom.org/c/osmo-sgsn/+/39563/comment/b83b3465_d5fbbdee?us… : PS6, Line 1488: static int get_guti(struct msgb *msg, struct gprs_gmm_ra_upd_req *req, uint32_t ptmsi, struct osmo_guti *guti, uint16_t *nas_token) I would usually assume that the first argument is the non-const output, followed by const arguments for input-only data. Here, I guess 'req' can be const. But then it seems nas_token is also an output argument.. so the function not only gets guti but also nas_token? And msg is not used at all? I guess this needs some thought about naming, unused arguments and const input data. https://gerrit.osmocom.org/c/osmo-sgsn/+/39563/comment/c1721823_4e2bd641?us… : PS6, Line 1628: LOGP(DGPRS, LOGL_ERROR, "Can't decode guti\n"); LOGIUP at least, so we get some context? Possibly even the entire input data so that we have a chane of figuring out why this happens if we ever see the error? https://gerrit.osmocom.org/c/osmo-sgsn/+/39563/comment/36d37e70_19e60e6c?us… : PS6, Line 1635: LOGP(DGPRS, LOGL_ERROR, "No MME found for Gummei %s", osmo_gummei_name(&guti.gummei)); Existing code above uses LOGIUP, so for consstency also add that? https://gerrit.osmocom.org/c/osmo-sgsn/+/39563/comment/36d1fbf9_323ca7f7?us… : PS6, Line 2625: /* FIXME: reject with impl. */ maybe at least log an error if this ever happens and we know we should be doing something? File src/sgsn/gprs_rau_fsm.c: https://gerrit.osmocom.org/c/osmo-sgsn/+/39563/comment/e459dd21_61b24d7d?us… : PS6, Line 211: /* FIXME */ this is the fourth fixme in less than 20 lines of code... are we sure this is ready yet? File src/sgsn/sgsn_libgtp.c: https://gerrit.osmocom.org/c/osmo-sgsn/+/39563/comment/c381fd58_13bcd666?us… : PS6, Line 923: // CKSN let's stick to C style comments, laso below. https://gerrit.osmocom.org/c/osmo-sgsn/+/39563/comment/4009c195_63479ac3?us… : PS6, Line 1002: MEMCPY_CHK(ptr, &mmctx->drx_parms, sizeof(mmctx->drx_parms)); all of this custom working with a pointer and bounds checking makes me wonder why the code is not using a msgb here? doesn't have to be the actual overall signaling message msgb, but just a local buffer? https://gerrit.osmocom.org/c/osmo-sgsn/+/39563/comment/31ff63cc_ff275571?us… : PS6, Line 1051: //GTP_LOGPKG(LOGL_ERROR, peer, pack, len, looks like left-over / cleanup needed? https://gerrit.osmocom.org/c/osmo-sgsn/+/39563/comment/dc3030a8_5b7d50e7?us… : PS6, Line 1119: // resp_it++; likewise, leftover/cleanup/bug? https://gerrit.osmocom.org/c/osmo-sgsn/+/39563/comment/0e47ab01_41393e1f?us… : PS6, Line 1151: static int validate_quintlets(uint8_t *buf, unsigned int buf_len) are there really quintlets in 3GPP somewhere? I'm familiar with quintets or quintuplets so far. https://gerrit.osmocom.org/c/osmo-sgsn/+/39563/comment/d7c70b80_64fb3cc6?us… : PS6, Line 1328: quintlets quintuplets? -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/39563?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: Iffe83b31db2b6e6869fedf2351375184096cff6f Gerrit-Change-Number: 39563 Gerrit-PatchSet: 6 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Mon, 10 Mar 2025 19:12:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No