Attention is currently required from: neels, pespin.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33171 )
Change subject: tweak lots of logging
......................................................................
Patch Set 4:
(2 comments)
File src/osmo-hnbgw/hnbgw.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33171/comment/f30bd2a4_d2e26ac6
PS3, Line 373: result = talloc_asprintf(OTC_SELECT, "%s %s", result, ctx->identity_info);
> Again, I hope this is not called every time we log a line.
Ack
File src/osmo-hnbgw/hnbgw_cn.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33171/comment/0b2619f6_e248dec4
PS3, Line 515: LOG_CNLINK(cnlink, DCN, LOGL_NOTICE, "using: cs7-%u %s <-> %s %s %s\n",
> This would rpboably better with a couple defines _FMT and _ARGS, so that it ends up in same log line […]
Ack
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33171
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I41275d8c3e272177976a9302795884666c35cd06
Gerrit-Change-Number: 33171
Gerrit-PatchSet: 4
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: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Jun 2023 09:50:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels.
laforge 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:
(1 comment)
Commit Message:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33169/comment/a0475b94_2edca0f5
PS3, Line 12: Use OTC_SELECT
I'm also a bit worried about this change, depending on how often we end up logging the cell-ID. We really don't want to put the extra load on talloc if there's no reason for it. Extending the static buffer size is much cheaper.
--
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: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Jun 2023 09:48:52 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32734 )
Change subject: paging: do not confirm PAGING COMMAND messages
......................................................................
Patch Set 12: Code-Review-1
(4 comments)
Patchset:
PS12:
CR-1 due to wrong pointer arithmetic.
File src/common/pcu_sock.c:
https://gerrit.osmocom.org/c/osmo-bts/+/32734/comment/66cbc845_6389d526
PS12, Line 670: imsi
This array can be moved to the respective scope of use now.
https://gerrit.osmocom.org/c/osmo-bts/+/32734/comment/98ce7c0f_e7b55591
PS12, Line 680: struct gsm48_imm_ass *gsm48_imm_ass
`const` please
https://gerrit.osmocom.org/c/osmo-bts/+/32734/comment/3f476a31_70c3394f
PS12, Line 683: (struct gsm48_imm_ass *)data_req->data + 3
This looks wrong to me. Without braces the `+ 3` is not "skip three bytes", but actually "skip three sizeof(struct gsm48_imm_ass)". gdb confirms this:
```
(gdb) p (struct gsm48_imm_ass *)0x00
$6 = (struct gsm48_imm_ass *) 0x0
(gdb) p (struct gsm48_imm_ass *)0x00 + 3
$7 = (struct gsm48_imm_ass *) 0x24
(gdb) p/x sizeof(struct gsm48_imm_ass) * 3
$8 = 0x24
```
It should be:
```
gsm48_imm_ass = (struct gsm48_imm_ass *)&data_req->data[3];
```
or
```
gsm48_imm_ass = (struct gsm48_imm_ass *)(data_req->data + 3);
```
--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32734
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: I8b8264d28b1b1deb08774cdba58dd4c6dafe115d
Gerrit-Change-Number: 32734
Gerrit-PatchSet: 12
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Jun 2023 09:48:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: arehbein.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33201 )
Change subject: stream: Add IPA send function/IPA-mode read to srv
......................................................................
Patch Set 1:
(3 comments)
File src/stream.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/8dffe91a_2056f1e2
PS1, Line 632: SRV
why SRV? isn't it CLI_OR_INP or something like that?
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/e7542ad7_700a835c
PS1, Line 1952: from the msgb structure's l1 and possibly l2 headers
why from the l1/l2 headers? Wouldn't it be more in-line with existign code to use something like the msgb->cb for this?
In general, if there is additional information (like SCTP PPID) that the transmitting function needs to know, we pass it via msgb->cb. The actual data part should only contain things that are part of the message
https://gerrit.osmocom.org/c/libosmo-netif/+/33201/comment/a5d3578e_9e9a5754
PS1, Line 1959: OSMO_ASSERT
below non-ipa code has OSMO_ASSERT(conn), too. Yours not. Any specific rationale?
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33201
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I61e1fe59166c46595efe8c1f32b8f2607cb6c529
Gerrit-Change-Number: 33201
Gerrit-PatchSet: 1
Gerrit-Owner: arehbein <arehbein(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: arehbein <arehbein(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Jun 2023 09:44:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment