June 2023 - gerrit-log - lists.osmocom.org

[L] Change in osmo-hnbgw[master]: cnpool: extract Mobile Identity from RANAP payload
by neels 08 Jun '23

08 Jun '23

Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133 ) Change subject: cnpool: extract Mobile Identity from RANAP payload ...................................................................... Patch Set 5: (1 comment) Patchset: PS5: anyway, submitted the libosmogsm API changes, see 'Depends:', now we need to wait for that to merge. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I373d665c9684b607207f68094188eab63209db51 Gerrit-Change-Number: 33133 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 23:33:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in osmo-hnbgw[master]: cnpool: extract Mobile Identity from RANAP payload
by neels 08 Jun '23

08 Jun '23

Attention is currently required from: laforge, pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133 to look at the new patch set (#5). Change subject: cnpool: extract Mobile Identity from RANAP payload ...................................................................... cnpool: extract Mobile Identity from RANAP payload For InitialUE messages, decode the RANAP and NAS PDU to obtain the Mobile Identity. Store it in map->l3. A following patch will use this mobile identity to decide on which CN links to pick from the CN pools for this subscriber. Add the new information to LOG_MAP() as ubiquitous logging context. Depends: libosmocore Ic5e0406d9e20b0d4e1372fa30ba11a1e69f5cc94 Related: SYS#6412 Change-Id: I373d665c9684b607207f68094188eab63209db51 --- M include/osmocom/hnbgw/context_map.h M include/osmocom/hnbgw/hnbgw.h M src/osmo-hnbgw/Makefile.am A src/osmo-hnbgw/hnbgw_l3.c M src/osmo-hnbgw/hnbgw_rua.c 5 files changed, 364 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/33/33133/5 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I373d665c9684b607207f68094188eab63209db51 Gerrit-Change-Number: 33133 Gerrit-PatchSet: 5 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[L] Change in libosmocore[master]: improve API for osmo_routing_area_id
by neels 08 Jun '23

08 Jun '23

neels has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/33225 ) Change subject: improve API for osmo_routing_area_id ...................................................................... improve API for osmo_routing_area_id Code review for [1] has asked for providing proper API for struct osmo_routing_area_id. For historical reasons, we have struct gprs_ra_id and struct osmo_routing_area_id serving the exact same purpose: represent a decoded 3GPP TS 24.008 § 10.5.5.15 Routing area identification. The "better" one is struct osmo_routing_area_id: it allows using API like osmo_plmn_cmp(), because it is made up of meaningful sub-structs. Implement de/coding using the functions already available for the sub-struct osmo_location_area_id, and simply add the RAC. Add a test in gsm0408_test.c. Note that other utility functions are already available for struct osmo_routing_area_id: osmo_rai_name2(), osmo_rai_cmp(). There is no real need to deprecate struct gprs_ra_id, because there is not really anything wrong with it. It just isn't as well integrated with other utility API as struct osmo_routing_area_id is. Just add comments. [1] osmo-hnbgw.git: cnpool: extract Mobile Identity from RANAP payload https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133 I373d665c9684b607207f68094188eab63209db51 Change-Id: Ic5e0406d9e20b0d4e1372fa30ba11a1e69f5cc94 --- M include/osmocom/gsm/gsm48.h M src/gsm/gsm48.c M src/gsm/libosmogsm.map M tests/gsm0408/gsm0408_test.c M tests/gsm0408/gsm0408_test.ok 5 files changed, 303 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/25/33225/1 diff --git a/include/osmocom/gsm/gsm48.h b/include/osmocom/gsm/gsm48.h index 8323c72..00fb6f4 100644 --- a/include/osmocom/gsm/gsm48.h +++ b/include/osmocom/gsm/gsm48.h @@ -20,7 +20,9 @@ * To mark an invalid / unset MNC, this value shall be used. */ #define GSM_MCC_MNC_INVALID 0xFFFF -/* A parsed GPRS routing area */ +/* A parsed GPRS routing area. + * Preferably use struct osmo_routing_area_id, it is better integrated with API like osmo_plmn_cmp(). + */ struct gprs_ra_id { uint16_t mcc; uint16_t mnc; @@ -104,6 +106,9 @@ int osmo_mobile_identity_encode_msgb(struct msgb *msg, const struct osmo_mobile_identity *mi, bool allow_hex); /* Parse Routeing Area Identifier */ +int osmo_routing_area_id_decode(struct osmo_routing_area_id *dst, const uint8_t *ra_data, size_t ra_data_len); +int osmo_routing_area_id_encode_buf(uint8_t *buf, size_t buflen, const struct osmo_routing_area_id *src); +int osmo_routing_area_id_encode_msgb(struct msgb *msg, const struct osmo_routing_area_id *src); void gsm48_parse_ra(struct gprs_ra_id *raid, const uint8_t *buf); void gsm48_encode_ra(struct gsm48_ra_id *out, const struct gprs_ra_id *raid); int gsm48_construct_ra(uint8_t *buf, const struct gprs_ra_id *raid) OSMO_DEPRECATED("Use gsm48_encode_ra() instead"); diff --git a/src/gsm/gsm48.c b/src/gsm/gsm48.c index df455ac..53e0989 100644 --- a/src/gsm/gsm48.c +++ b/src/gsm/gsm48.c @@ -1334,7 +1334,64 @@ return 1; } -/*! Parse TS 04.08 Routing Area Identifier +/*! Decode to struct osmo_routing_area_id from a 3GPP TS 24.008 § 10.5.5.15 Routing area identification. + * \param[out] dst Store the decoded result here. + * \param[in] ra_data The start of a Routing Area ID in encoded form, to be decoded. + * \param[in] ra_data_len Buffer size available to read from at *ra_data. + * \return the number of decoded bytes on success, or negative on error (if the input buffer is too small). + */ +int osmo_routing_area_id_decode(struct osmo_routing_area_id *dst, const uint8_t *ra_data, size_t ra_data_len) +{ + const struct gsm48_ra_id *ra_id; + if (ra_data_len < sizeof(*ra_id)) + return -ENOSPC; + + gsm48_decode_lai2((void *)ra_data, &dst->lac); + + ra_id = (void *)ra_data; + dst->rac = ra_id->rac; + + return sizeof(*ra_id); +} + +/*! Encode struct osmo_routing_area_id to a 3GPP TS 24.008 § 10.5.5.15 Routing area identification: write to a buffer. + * \param[out] buf Return buffer for encoded Mobile Identity. + * \param[in] buflen sizeof(buf). + * \param[in] src RA to encode. + * \return Amount of bytes written to buf, or negative on error. + */ +int osmo_routing_area_id_encode_buf(uint8_t *buf, size_t buflen, const struct osmo_routing_area_id *src) +{ + struct gsm48_ra_id *ra_id; + if (buflen < sizeof(*ra_id)) + return -ENOSPC; + + gsm48_generate_lai2((void *)buf, &src->lac); + + ra_id = (void *)buf; + ra_id->rac = src->rac; + + return sizeof(*ra_id); +} + +/*! Encode struct osmo_routing_area_id to a 3GPP TS 24.008 § 10.5.5.15 Routing area identification: append to msgb. + * To succeed, the msgb must have tailroom >= sizeof(struct gsm48_ra_id). + * \param[out] msg Append to this msgb. + * \param[in] src Encode this Routing Area ID. + * \return Number of bytes appended to msgb, or negative on error. + */ +int osmo_routing_area_id_encode_msgb(struct msgb *msg, const struct osmo_routing_area_id *src) +{ + int rc = osmo_routing_area_id_encode_buf(msg->tail, msgb_tailroom(msg), src); + if (rc <= 0) + return rc; + msgb_put(msg, rc); + return rc; +} + +/*! Parse TS 04.08 Routing Area Identifier. + * Preferably use osmo_routing_area_id_decode() instead: struct osmo_routing_area_id is better integrated with other API + * like osmo_plmn_cmp(). * \param[out] Caller-provided memory for decoded RA ID * \param[in] buf Input buffer pointing to RAI IE value */ void gsm48_parse_ra(struct gprs_ra_id *raid, const uint8_t *buf) diff --git a/src/gsm/libosmogsm.map b/src/gsm/libosmogsm.map index 0e45dc1..1eea819 100644 --- a/src/gsm/libosmogsm.map +++ b/src/gsm/libosmogsm.map @@ -415,6 +415,9 @@ osmo_mobile_identity_encoded_len; osmo_mobile_identity_encode_buf; osmo_mobile_identity_encode_msgb; +osmo_routing_area_id_decode; +osmo_routing_area_id_encode_buf; +osmo_routing_area_id_encode_msgb; gsm48_mm_att_tlvdef; gsm48_number_of_paging_subchannels; gsm48_parse_ra; diff --git a/tests/gsm0408/gsm0408_test.c b/tests/gsm0408/gsm0408_test.c index 4436d51..f4353e2 100644 --- a/tests/gsm0408/gsm0408_test.c +++ b/tests/gsm0408/gsm0408_test.c @@ -341,6 +341,171 @@ } } +static struct osmo_routing_area_id test_osmo_routing_area_id_items[] = { + { + .lac = { + .plmn = { + .mcc = 77, + .mnc = 121, + }, + .lac = 666, + }, + .rac = 5, + }, + { + .lac = { + .plmn = { + .mcc = 84, + .mnc = 98, + }, + .lac = 11, + }, + .rac = 89, + }, + { + .lac = { + .plmn = { + .mcc = 0, + .mnc = 0, + .mnc_3_digits = false, + /* expecting 000-00, BCD = 00 f0 00 */ + }, + .lac = 0, + }, + .rac = 0, + }, + { + .lac = { + .plmn = { + .mcc = 0, + .mnc = 0, + .mnc_3_digits = true, + /* expecting 000-000, BCD = 00 00 00 */ + }, + .lac = 0, + }, + .rac = 0, + }, + { + .lac = { + .plmn = { + .mcc = 999, + .mnc = 999, + }, + .lac = 65535, + }, + .rac = 255, + }, + { + .lac = { + .plmn = { + .mcc = 1, + .mnc = 2, + .mnc_3_digits = false, + /* expecting 001-02, BCD = 00 f1 20 */ + }, + .lac = 23, + }, + .rac = 42, + }, + { + .lac = { + .plmn = { + .mcc = 1, + .mnc = 2, + .mnc_3_digits = true, + /* expecting 001-002, BCD = 00 21 00 */ + }, + .lac = 23, + }, + .rac = 42, + }, + { + .lac = { + .plmn = { + .mcc = 12, + .mnc = 34, + .mnc_3_digits = false, + /* expecting 012-34, BCD = 10 f2 43 */ + }, + .lac = 56, + }, + .rac = 78, + }, + { + .lac = { + .plmn = { + .mcc = 12, + .mnc = 34, + .mnc_3_digits = true, + /* expecting 012-034, BCD = 10 42 30 */ + }, + .lac = 23, + }, + .rac = 42, + }, + { + .lac = { + .plmn = { + .mcc = 123, + .mnc = 456, + .mnc_3_digits = false, + /* expecting 123-456, BCD = 21 63 54 (false flag has no effect) */ + }, + .lac = 23, + }, + .rac = 42, + }, + { + .lac = { + .plmn = { + .mcc = 123, + .mnc = 456, + .mnc_3_digits = true, + /* expecting 123-456, BCD = 21 63 54 (same) */ + }, + .lac = 23, + }, + .rac = 42, + }, +}; + +static inline void dump_osmo_routing_area_id(const struct osmo_routing_area_id *raid) +{ + printf("%s%s", osmo_rai_name2(raid), raid->lac.plmn.mnc_3_digits ? " (3-digit MNC)" : ""); +} + +static inline void check_osmo_routing_area_id(const struct osmo_routing_area_id *raid) +{ + uint8_t buf[sizeof(struct gsm48_ra_id)] = {}; + struct osmo_routing_area_id raid0 = {}; + int rc; + + printf("RA ID: "); + dump_osmo_routing_area_id(raid); + + rc = osmo_routing_area_id_encode_buf(buf, sizeof(buf), raid); + printf("osmo_routing_area_id_encode_buf(): %src=%d\n", osmo_hexdump(buf, sizeof(buf)), rc); + + rc = osmo_routing_area_id_decode(&raid0, buf, sizeof(buf)); + printf("osmo_routing_area_id_decode(): "); + dump_osmo_routing_area_id(&raid0); + printf(" rc=%d\n", rc); + + if (osmo_rai_cmp(raid, &raid0)) + printf("FAIL\n"); + else + printf("ok\n"); +} + +static void test_osmo_routing_area_id(void) +{ + int i; + printf("==%s()==\n", __func__); + for (i = 0; i < ARRAY_SIZE(test_osmo_routing_area_id_items); i++) + check_osmo_routing_area_id(&test_osmo_routing_area_id_items[i]); +} + static void dump_cm3(struct gsm48_classmark3 *cm3) { printf("mult_band_supp=%02x\n", cm3->mult_band_supp); @@ -1792,6 +1957,7 @@ test_bcd_number_encode_decode(); test_ra_cap(); test_lai_encode_decode(); + test_osmo_routing_area_id(); test_decode_classmark3(); test_si_range_helpers(); diff --git a/tests/gsm0408/gsm0408_test.ok b/tests/gsm0408/gsm0408_test.ok index dc48f84..b966865 100644 --- a/tests/gsm0408/gsm0408_test.ok +++ b/tests/gsm0408/gsm0408_test.ok @@ -386,6 +386,40 @@ Encoded 21 63 54 00 17 gsm48_decode_lai2() gives 123-456-23 (3-digit MNC) passed +==test_osmo_routing_area_id()== +RA ID: 077-121-666-5osmo_routing_area_id_encode_buf(): 70 17 21 02 9a 05 rc=6 +osmo_routing_area_id_decode(): 077-121-666-5 (3-digit MNC) rc=6 +ok +RA ID: 084-98-11-89osmo_routing_area_id_encode_buf(): 80 f4 89 00 0b 59 rc=6 +osmo_routing_area_id_decode(): 084-98-11-89 rc=6 +ok +RA ID: 000-00-0-0osmo_routing_area_id_encode_buf(): 00 f0 00 00 00 00 rc=6 +osmo_routing_area_id_decode(): 000-00-0-0 rc=6 +ok +RA ID: 000-000-0-0 (3-digit MNC)osmo_routing_area_id_encode_buf(): 00 00 00 00 00 00 rc=6 +osmo_routing_area_id_decode(): 000-000-0-0 (3-digit MNC) rc=6 +ok +RA ID: 999-999-65535-255osmo_routing_area_id_encode_buf(): 99 99 99 ff ff ff rc=6 +osmo_routing_area_id_decode(): 999-999-65535-255 (3-digit MNC) rc=6 +ok +RA ID: 001-02-23-42osmo_routing_area_id_encode_buf(): 00 f1 20 00 17 2a rc=6 +osmo_routing_area_id_decode(): 001-02-23-42 rc=6 +ok +RA ID: 001-002-23-42 (3-digit MNC)osmo_routing_area_id_encode_buf(): 00 21 00 00 17 2a rc=6 +osmo_routing_area_id_decode(): 001-002-23-42 (3-digit MNC) rc=6 +ok +RA ID: 012-34-56-78osmo_routing_area_id_encode_buf(): 10 f2 43 00 38 4e rc=6 +osmo_routing_area_id_decode(): 012-34-56-78 rc=6 +ok +RA ID: 012-034-23-42 (3-digit MNC)osmo_routing_area_id_encode_buf(): 10 42 30 00 17 2a rc=6 +osmo_routing_area_id_decode(): 012-034-23-42 (3-digit MNC) rc=6 +ok +RA ID: 123-456-23-42osmo_routing_area_id_encode_buf(): 21 63 54 00 17 2a rc=6 +osmo_routing_area_id_decode(): 123-456-23-42 (3-digit MNC) rc=6 +ok +RA ID: 123-456-23-42 (3-digit MNC)osmo_routing_area_id_encode_buf(): 21 63 54 00 17 2a rc=6 +osmo_routing_area_id_decode(): 123-456-23-42 (3-digit MNC) rc=6 +ok =====cm3_1===== mult_band_supp=06 a5_bits=00 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33225 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ic5e0406d9e20b0d4e1372fa30ba11a1e69f5cc94 Gerrit-Change-Number: 33225 Gerrit-PatchSet: 1 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-hnbgw[master]: fix umts_cell_id_name(): show CID
by neels 07 Jun '23

07 Jun '23

Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169 ) Change subject: fix umts_cell_id_name(): show CID ...................................................................... Patch Set 3: (3 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169/comment/cd049b6e_17c7f84f PS3, Line 12: Use OTC_SELECT > That's usually my opinion too, but since I have the feeling I'm usually the only one against this I […] My opinion is firmly with dynamic allocation. IIRC there is a log line that prints two distinct cell IDs. For that alone, i prefer the dynamic approach. But I'm really surprised to hear any praise for static buffers at all. I thought that the consensus and aim is to get away from static buffers, to get away from their notorious disadvantages: - each static buffer can be used only once per va (LOGP(), printf()), or we get bugs and misleading logs that are hard to spot and identify as such. - there is a distinct static buffer for every API / API-function, so the vast vast majority of static buffers just sit around 99.999% of the time simply taking up space and not being used. - truncation / a static buffer needs to be as large as the largest string that is at all possible. A long while back I proposed a kind of static ring buffer that alleviates the worst disadvantages of static buffers, but the code review answer was the OTC_SELECT, which after all I agree is a very elegant solution. So now we go back from elegant to cumbersome??? If you mean to imply that this is inefficient and expensive related to the rest of osmo-hnbgw, then kindly provide the performance measurements that prove it. <- the standard answer for all premature optimization requests. Patchset: PS3: I would gladly change my mind, but so far am not convinced... File src/osmo-hnbgw/hnbgw.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169/comment/9d8261c8_54408599 PS2, Line 348: return talloc_asprintf(OTC_SELECT, "%u-%u-L%u-R%u-S%u-C%u", ucid->mcc, ucid->mnc, ucid->lac, ucid->rac, > I hope this is not called everytime we log a line :) i don't see a problem if it is. We do these kinds of function calls as well as OTC_SELECT allocations A LOT for very many log statements. Noteworthy: producing log lines and calling this function is avoided entirely when the logging category is silenced. So if a user has performance issues, they can crank up the log levels. If you mean to imply that this is inefficient and expensive related to the rest of osmo-hnbgw, then kindly provide the performance measurements that prove it. <- the standard answer for all premature optimization requests. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: Id903b8579ded8b908e644808e440d373bcca8da4 Gerrit-Change-Number: 33169 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 21:22:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[L] Change in osmo-hnbgw[master]: cnpool: extract Mobile Identity from RANAP payload
by neels 07 Jun '23

07 Jun '23

Attention is currently required from: laforge, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133 ) Change subject: cnpool: extract Mobile Identity from RANAP payload ...................................................................... Patch Set 4: (1 comment) File src/osmo-hnbgw/hnbgw_l3.c: https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133/comment/3006fc9a_f901f9c8 PS2, Line 104: /* Old routing area identification 10.5.5.15. */ > > i guess there could be one static function for decoding old RA id for here and below? cannot find […] ok. do you see this as a blocker for this patch? if not we can merge this as-is and fix the libosmogsm API later. -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I373d665c9684b607207f68094188eab63209db51 Gerrit-Change-Number: 33133 Gerrit-PatchSet: 4 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 20:51:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in osmocom-bb[master]: l1ctl_proto: add 'start_fn' field to UL/DL TBF CFG.req messages
by pespin 07 Jun '23

07 Jun '23

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33222 ) Change subject: l1ctl_proto: add 'start_fn' field to UL/DL TBF CFG.req messages ...................................................................... Patch Set 1: (1 comment) File src/host/layer23/src/common/l1ctl.c: https://gerrit.osmocom.org/c/osmocom-bb/+/33222/comment/4a6dc7af_6ba318ac PS1, Line 1050: .start_fn = htonl(start_fn), why are you passing it as a big endian? Shouldn't we be doing the same with slotmask then? -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33222 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ibb6a05165fe1c81268fb0e3674adae4065e78171 Gerrit-Change-Number: 33222 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 19:08:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[L] Change in libosmo-netif[master]: Add osmo_io support to osmo_stream_cli and osmo_stream_srv
by pespin 07 Jun '23

07 Jun '23

Attention is currently required from: arehbein, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33193 ) Change subject: Add osmo_io support to osmo_stream_cli and osmo_stream_srv ...................................................................... Patch Set 3: (5 comments) File include/osmocom/netif/stream.h: https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/8b8466fb_ad4fb8e2 PS3, Line 48: struct osmo_stream_srv *osmo_stream_srv_create_iofd(void *ctx, const char *name, struct osmo_stream_srv_link *link, int fd, int (*read_cb)(struct osmo_stream_srv *conn, struct msgb *msg), int (*closed_cb)(struct osmo_stream_srv *conn), void *data); Does that mean use needs to use one or another depending on whether iofd is used? Probably that's not the case, so this one should be named osmo_stream_srv_create2. Maybe add extra APIs to set each callback instead. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/8b731669_a00dfe5c PS3, Line 83: void osmo_stream_cli_set_iofd_read_cb(struct osmo_stream_cli *cli, int (*read_cb)(struct osmo_stream_cli *cli, struct msgb *msg)); So for client you add a new setter API, but for srv you update an existing one? This looks strange. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/41d831e8_bf7a46b1 PS3, Line 88: struct osmo_stream_cli *osmo_stream_cli_create_iofd(void *ctx, const char *name); osmo_stream_cli_create2 (no iofd specific) File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/8c683eb8_5dbf7ef4 PS3, Line 1360: #define OSMO_STREAM_SRV_F_FLUSH_DESTROY (1 << 0) Why is this code block moved from below to to here? You are mixing srv_link and srv code then. https://gerrit.osmocom.org/c/libosmo-netif/+/33193/comment/96c519ea_a21ecbcd PS3, Line 1610: rc = conn->read_cb(conn); Mayeb the read_cb rename can be done in a separate prior patch to simplify this one. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33193 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I2f52c7107c392b6f4b0bf2a84f8c873c084a200c Gerrit-Change-Number: 33193 Gerrit-PatchSet: 3 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 19:05:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream: Update log messages
by pespin 07 Jun '23

07 Jun '23

Attention is currently required from: daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33221 ) Change subject: stream: Update log messages ...................................................................... Patch Set 2: (3 comments) File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33221/comment/d564d226_5d0bf948 PS2, Line 564: LOGSCLI(cli, LOGL_DEBUG, "connection done.\n"); while at it, please remove the final dots. I just submitted a patch doing the same (I didn't see yours): https://gerrit.osmocom.org/c/libosmo-netif/+/33224 I'm happy to drop mine if you include it into yours. https://gerrit.osmocom.org/c/libosmo-netif/+/33221/comment/bf28c770_3b550f39 PS2, Line 1605: LOGP(DLINP, LOGL_NOTICE, "Connection is being flushed and closed; ignoring received message\n"); INFO. NOTICE is too much, you are printing this every time you receive a message. https://gerrit.osmocom.org/c/libosmo-netif/+/33221/comment/01a1a4f7_28e5828e PS2, Line 1710: if (conn == NULL) or simply OSMO_ASSERT(conn) -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33221 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: Ife20b9d18e6ca86a06991d68165694e31052c58a Gerrit-Change-Number: 33221 Gerrit-PatchSet: 2 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Jun 2023 18:50:37 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream: Drop dot at the end of log line
by pespin 07 Jun '23

07 Jun '23

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-netif/+/33224 to look at the new patch set (#2). Change subject: stream: Drop dot at the end of log line ...................................................................... stream: Drop dot at the end of log line Change-Id: I59b0e56088c60e3de5f3a2f38f158e16074a65c9 --- M src/stream.c 1 file changed, 12 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/24/33224/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33224 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I59b0e56088c60e3de5f3a2f38f158e16074a65c9 Gerrit-Change-Number: 33224 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmo-netif[master]: stream: Drop dot at the end of log line
by pespin 07 Jun '23

07 Jun '23

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33224 ) Change subject: stream: Drop dot at the end of log line ...................................................................... stream: Drop dot at the end of log line Change-Id: I59b0e56088c60e3de5f3a2f38f158e16074a65c9 --- M src/stream.c 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/24/33224/1 diff --git a/src/stream.c b/src/stream.c index 6afb7ca..aad237f 100644 --- a/src/stream.c +++ b/src/stream.c @@ -439,7 +439,7 @@ if (llist_empty(&cli->tx_queue)) osmo_fd_write_disable(&cli->ofd); - LOGSCLI(cli, LOGL_DEBUG, "connection done.\n"); + LOGSCLI(cli, LOGL_DEBUG, "connection done\n"); cli->state = STREAM_CLI_STATE_CONNECTED; switch (cli->sk_domain) { case AF_UNIX: -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33224 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I59b0e56088c60e3de5f3a2f38f158e16074a65c9 Gerrit-Change-Number: 33224 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0