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

Change in osmo-bsc[master]: abis_rsl: add support for sending IMMEDIATE ASSIGNMENT through PCH

by neels

Attention is currently required from: pespin, fixeria, dexter. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31126 ) Change subject: abis_rsl: add support for sending IMMEDIATE ASSIGNMENT through PCH ...................................................................... Patch Set 11: Code-Review+1 (1 comment) File src/osmo-bsc/pcu_sock.c: https://gerrit.osmocom.org/c/osmo-bsc/+/31126/comment/9bef5182_5156cd1f PS11, Line 412: * the IMSI, which we need to compute the paging group */ (you often use a very short line width in comments ... doesn't your editor auto-line-break them? i find the vim shift-V gw in conjunction with set textwidth=120 very useful to have to manually adjust almost nothing) -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31126 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I4452f4973d1ec69c96aad527b057226e8a6edf99 Gerrit-Change-Number: 31126 Gerrit-PatchSet: 11 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Feb 2023 03:34:44 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 4 months

1
0
0 0

Change in osmo-bsc[master]: pcuif_proto: rename PCU_IF_SAPI_AGCH_DT to PCU_IF_SAPI_PCH_DT

by neels

Attention is currently required from: dexter. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/31157 ) Change subject: pcuif_proto: rename PCU_IF_SAPI_AGCH_DT to PCU_IF_SAPI_PCH_DT ...................................................................... Patch Set 6: Code-Review+1 (1 comment) Patchset: PS6: not sure what pespin was referring to; looks good to me -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/31157 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I78abeb62d0267baa31a4727c4bdd027b7230f137 Gerrit-Change-Number: 31157 Gerrit-PatchSet: 6 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Feb 2023 03:30:04 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 4 months

1
0
0 0

Change in libosmocore[master]: Add osmo_sock_get_name_multiaddr_buf()

by neels

Attention is currently required from: fixeria, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/28997 ) Change subject: Add osmo_sock_get_name_multiaddr_buf() ...................................................................... Patch Set 17: (5 comments) File src/core/socket.c: https://gerrit.osmocom.org/c/libosmocore/+/28997/comment/917c5219_90712540 PS17, Line 244: OSMO_STRBUF_PRINTF(*sb, "%s%s", oss->ip, after); same as in https://gerrit.osmocom.org/c/libosmocore/+/28993/13/src/core/socket.c#244 ...is this code dup? i'm confused https://gerrit.osmocom.org/c/libosmocore/+/28997/comment/71668273_dd1f67a5 PS17, Line 252: return -ENOSPC; do not size check, for reasons see https://gerrit.osmocom.org/c/libosmocore/+/28993 comments https://gerrit.osmocom.org/c/libosmocore/+/28997/comment/8143a086_ef7e7fc3 PS17, Line 902: static int sctp_multiaddr_print_ctx_helper(struct osmo_strbuf *sb, int fd, bool local) please let's not introduce string functions that take osmo_strbuf as arguments. (I reasoned earlier today on another patch of yours) https://gerrit.osmocom.org/c/libosmocore/+/28997/comment/886e3e11_5cdf59e2 PS17, Line 924: rc = multiaddr_strbuf(sb, NULL, 0, &addrs, addr_count); here use OSMO_STRBUF_APPEND(sb, multiaddr_buf, ...args...) (if multiaddr_buf() is the correct name for the (char *buf,size_t len) one, i don't recall now) File tests/socket/socket_sctp_test.c: https://gerrit.osmocom.org/c/libosmocore/+/28997/comment/8d089043_5ef8bac8 PS17, Line 141: printf("Bound on %s [%d]\n", buf, rc); the rc printed is returned by the string composition, the printf looks like it's about a socket bind. maybe OSMO_ASSERT(rc < sizeof(buf))? -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/28997 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: If76595ebd1cf26ba904887a36c4cc14a1b5c4521 Gerrit-Change-Number: 28997 Gerrit-PatchSet: 17 Gerrit-Owner: msuraev <msuraev(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Feb 2023 03:27:51 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 4 months

1
0
0 0

Change in libosmo-gprs[master]: rlcmac: Introduce DL TBF creation through PCH ImmAss

by neels

Attention is currently required from: pespin, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/31286 ) Change subject: rlcmac: Introduce DL TBF creation through PCH ImmAss ...................................................................... Patch Set 5: (3 comments) File src/rlcmac/rlcmac.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/d0151c70_6519b4f4 PS5, Line 278: OSMO_ASSERT(bv); > I think so far common practice was to log memory allocation errors and shutdown gracefully. […] when mem alloc fails, logging cannot possibly work. we do in fact assert on alloc success very often. i agree with this assert. https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/552b7fd8_e520b6c1 PS5, Line 291: LOGRLCMAC(LOGL_NOTICE, "TODO: handle decoded dl ctrl block!\n"); hmm, a TODO in the logs? maybe a #warning pragma instead? File src/rlcmac/tbf_dl.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31286/comment/b45fbc9a_ecc23765 PS5, Line 39: // goto err_tbf_destruct; i can understand that some code will come in only later, but you do add an awful lot of these temporarily commented things. Seems that a lot of them will end up surviving eternally and should rather be dropped from patches being merged; let's not let this become a habit, who will tidy up all of them? -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31286 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I7f98e3456ef35d80becdad3481afeb771457b0ef Gerrit-Change-Number: 31286 Gerrit-PatchSet: 5 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Feb 2023 03:15:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 4 months

1
0
0 0

Change in libosmo-gprs[master]: rlcmac: Implement GMMRR-ASSIGN.req

by neels

Attention is currently required from: pespin, msuraev. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/31285 ) Change subject: rlcmac: Implement GMMRR-ASSIGN.req ...................................................................... Patch Set 3: (1 comment) File include/osmocom/gprs/rlcmac/rlcmac_prim.h: https://gerrit.osmocom.org/c/libosmo-gprs/+/31285/comment/877010d4_517be5df PS3, Line 189: struct osmo_gprs_rlcmac_prim *osmo_gprs_rlcmac_prim_alloc_gmmrr_assign_req( > That should either be a separate patch or correction to earlier patch in this series which added typ […] also weird how it has no .c file equivalent. does the function even have a caller? could it be static to the .c file instead? -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31285 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I3178aa1af4a6c05c84253c6befcd4c786b8dd8e9 Gerrit-Change-Number: 31285 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Feb 2023 03:08:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 4 months

1
0
0 0

Change in libosmo-gprs[master]: rlcmac: Initial ul_tbf data support

by neels

Attention is currently required from: pespin, fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/31170 ) Change subject: rlcmac: Initial ul_tbf data support ...................................................................... Patch Set 10: (9 comments) File include/osmocom/gprs/rlcmac/rlc_window_ul.h: https://gerrit.osmocom.org/c/libosmo-gprs/+/31170/comment/1b59c661_14e06836 PS10, Line 72: uint16_t ssn, uint16_t *lost, uint16_t *received); weird indent, neither pure tabs nor aligns File src/rlcmac/coding_scheme.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31170/comment/9af3bbed_4216a0fd PS10, Line 312: default: *need_stuffing = false; break; odd single-line style for switch cases File src/rlcmac/rlc.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31170/comment/25a4afa8_fd5812b8 PS10, Line 100: (extra blank) https://gerrit.osmocom.org/c/libosmo-gprs/+/31170/comment/595ef7f2_794a485f PS10, Line 273: (trailing blank) File src/rlcmac/rlc_window.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31170/comment/31cad603_4bd0eca2 PS10, Line 77: (trailing blank ... just saying because git marks those red for me, seems to be a thing) File src/rlcmac/rlc_window_ul.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31170/comment/cd3b0837_c833d00a PS10, Line 28: (extra blank) https://gerrit.osmocom.org/c/libosmo-gprs/+/31170/comment/54ddb22e_b769b45a PS10, Line 357: () File src/rlcmac/rlcmac_enc.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31170/comment/deb9580d_df15563a PS10, Line 233: \brief we have AUTOBRIEF, can drop the \brief File src/rlcmac/tbf_ul_fsm.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31170/comment/8433f1d6_5d652a6a PS10, Line 44: [GPRS_RLCMAC_TBF_UL_ST_FINISHED] = { }, (may omit unset ones) -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31170 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I0a01d79d16bbfc63aa88e6bb0f432f3772645730 Gerrit-Change-Number: 31170 Gerrit-PatchSet: 10 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: msuraev <msuraev(a)sysmocom.de> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Feb 2023 03:04:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 4 months

1
0
0 0

Change in libosmo-gprs[master]: rlcmac: Initial implementation of UL TBF assignment and scheduler

by neels

Attention is currently required from: pespin, fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/31099 ) Change subject: rlcmac: Initial implementation of UL TBF assignment and scheduler ...................................................................... Patch Set 16: (8 comments) Commit Message: https://gerrit.osmocom.org/c/libosmo-gprs/+/31099/comment/0b02cb09_fc0fce52 PS16, Line 9: er t ("step") Patchset: PS16: in general i wish you wouldn't have a habit of putting comments in the same line as code... this is a lot and i can't really comment on functional accuracy, but looks sane in general. +1 when the T_def has a desc and a proper X-nr assigned File src/rlcmac/rlcmac.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31099/comment/5f53ac77_55fa7d08 PS16, Line 40: { .T=1, .default_val=1, .unit=OSMO_TDEF_S, .desc="foobar", .val=0 }, is this a regression test? doesn't look like one. so probably: add a desc, give it an unused T number like -32 currently and add X32 to https://osmocom.org/projects/cellular-infrastructure/wiki/List_of_Timer_num… drop .val = 0, it is redundant. (also because tdefs will be initialized to default_val so = 0 may be confusing) File src/rlcmac/tbf_ul_ass_fsm.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31099/comment/9a7e3b97_946dd565 PS16, Line 53: [GPRS_RLCMAC_TBF_UL_ASS_ST_COMPL] = { }, (could just omit unset ones) File src/rlcmac/tbf_ul_fsm.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31099/comment/d87efcda_1bebc6cb PS16, Line 42: [GPRS_RLCMAC_TBF_UL_ST_FLOW] = { }, (same) https://gerrit.osmocom.org/c/libosmo-gprs/+/31099/comment/519c7a11_48025cc4 PS16, Line 93: } drop this func? https://gerrit.osmocom.org/c/libosmo-gprs/+/31099/comment/f6c20afd_adaeff1c PS16, Line 147: (extra blank line) https://gerrit.osmocom.org/c/libosmo-gprs/+/31099/comment/e2fd11a8_57d45d51 PS16, Line 207: (trailing blank line) -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31099 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I420c57a9d0b63f9c2805a7c2ae8ce85532a48eef Gerrit-Change-Number: 31099 Gerrit-PatchSet: 16 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Feb 2023 02:56:52 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

2 years, 4 months

1
0
0 0

Change in libosmo-gprs[master]: rlcmac: Introduce lower primitive layer API

by neels

Attention is currently required from: pespin, fixeria. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/31177 ) Change subject: rlcmac: Introduce lower primitive layer API ...................................................................... Patch Set 9: Code-Review+1 (2 comments) Patchset: PS9: can't say i understand the semantics, but code looks fine to me File tests/rlcmac/rlcmac_prim_test.c: https://gerrit.osmocom.org/c/libosmo-gprs/+/31177/comment/fe1ae3bd_3bd0ea63 PS9, Line 112: (drop blank line? / missing '*') -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31177 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I3d97425ec75059ceae983de869419230e8c4df01 Gerrit-Change-Number: 31177 Gerrit-PatchSet: 9 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Feb 2023 02:43:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 4 months

1
0
0 0

Change in libosmo-abis[master]: Reworked mi_e1_line_update() and some of its sub routines

by neels

Attention is currently required from: jolly. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/31296 ) Change subject: Reworked mi_e1_line_update() and some of its sub routines ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Commit Message: https://gerrit.osmocom.org/c/libosmo-abis/+/31296/comment/1cbf8345_c0a4b6d3 PS1, Line 12: printf and fprintf are replaced by logging functions. (would usually be nice to do those things in a separate patch so they don't mix with other changes) -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/31296 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: Iab0776fce6921661b39e9e53376cf01a80bcd42c Gerrit-Change-Number: 31296 Gerrit-PatchSet: 1 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Comment-Date: Mon, 13 Feb 2023 02:39:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 4 months

1
0
0 0

Change in libosmo-abis[master]: Add L1 signals to misdn driver

by neels

Attention is currently required from: jolly, pespin. neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/31033 ) Change subject: Add L1 signals to misdn driver ...................................................................... Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/31033 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: Id8b6fad9b073cf92eee92a6906bf5b57af37e2f0 Gerrit-Change-Number: 31033 Gerrit-PatchSet: 7 Gerrit-Owner: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 13 Feb 2023 02:36:39 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 4 months

1
0
0 0

2025

2024

2023

2022

gerrit-log February 2023