Attention is currently required from: Hoernchen, laforge, fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/32021 )
Change subject: gmm: Initial implementation of GPRS Attach ...................................................................... Patch Set 1: (12 comments) File include/osmocom/gprs/gmm/gmm_pdu.h: https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/3b3744f7_686e7dae PS1, Line 16: tlv_definition > You should include `<osmocom/gsm/tlv.h>` defining this struct. Ack https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/a64c2952_60e95285 PS1, Line 32: uint8_t sres[4], > Why not a const pointer? Ack File libosmo-gprs-gmm.pc.in: https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/36094b3d_ec293ba4 PS1, Line 9: libosmo-gprs-llc > btw, why is this needed? because of header files? Because gmm_prim uses the llc_prim for the LLGMM API, and it uses the struct defintiions and functions to allocate the primitives. That doesn't mean it forcefully needs the entire LLC stack in libosmo-gprs-llc to function though, since the app is the one forwarding the primitives. File src/gmm/gmm_pdu.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/25ad325b_0148c103 PS1, Line 31: struct gprs_gmm_ms_net_cap { > Run `struct_endianess.py`. Ack https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/8477c5c3_f8aaa1e1 PS1, Line 142: 23 > `GSM_MAC_BLOCK_LEN`? yeah, but this is all temporary to have some base upon to work with. https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/e9254ab7_4361010a PS1, Line 147: rc = bitvec_unhex(&bv, "171933432b37159ef98879cba28c6621e72688b198879c00"); > Makes sense to add a TODO here, since you're hard-coding the payload. yeah, but this is all temporary to have some base upon to work with. https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/9b2fa138_09e2e8b8 PS1, Line 172: msgb_put_u8(msg, sizeof(ms_net_cap_def)); > Use `msgb_lv_put()`. I'll check. https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/a1b3e56f_5585003d PS1, Line 274: GSM_MI_TYPE_TMSI > `GSM_MI_TYPE_IMEI`! Ack https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/de7981c6_5d1aa5b7 PS1, Line 312: (void)imeisv_requested; > What is this for? IDK, I yet have to discover, that's why it's TODO :D but it shows up in the specs in the messages. if you mean the (void) thing, it's to avoid having the "empty if clause" or alike warning. This is left here so that in the future we can quickly find out where to use it once we read the related specs. File src/gmm/gmm_prim.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/751b4541_e51ea1da PS1, Line 107: gmm_llc_down_cb_dummy > `__func__` I don't really expect this to change but ok. https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/6b1e5bf5_b97f17bf PS1, Line 467: rc = gprs_gmm_prim_handle_unsupported(gmm_prim); > This case can fall-through to default, but not critical. I did this on purpose since it's really the only primitive we expect to have here, so the final logic is already in place. https://gerrit.osmocom.org/c/libosmo-gprs/+/32021/comment/a0943413_c28c7cac PS1, Line 589: llc_prim > Why would anybody pass you NULL pointer here? Just curious. >Why would anybody pass you NULL pointer here? Just curious. You can apply this question to literally any ASSERT checking a null pointer in the whole history of code. -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/32021 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I212053b3a3f27ef7d63503c3d5ef08453b2d2056 Gerrit-Change-Number: 32021 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <[email protected]> Gerrit-Reviewer: Hoernchen <[email protected]> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <[email protected]> Gerrit-Reviewer: laforge <[email protected]> Gerrit-Attention: Hoernchen <[email protected]> Gerrit-Attention: laforge <[email protected]> Gerrit-Attention: fixeria <[email protected]> Gerrit-Comment-Date: Thu, 23 Mar 2023 10:29:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <[email protected]> Gerrit-MessageType: comment
