pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/30150 )
Change subject: osmux: Log AMR FT when incorrect AMR payload size detected
......................................................................
osmux: Log AMR FT when incorrect AMR payload size detected
Change-Id: Idd9712644d9a6204f1fe972cbbd393d4b7fd34f6
---
M src/osmux_input.c
1 file changed, 2 insertions(+), 2 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/50/30150/1
diff --git a/src/osmux_input.c b/src/osmux_input.c
index eff2423..c1cd2c6 100644
--- a/src/osmux_input.c
+++ b/src/osmux_input.c
@@ -313,8 +313,8 @@
/* The AMR payload does not fit with what we expect */
if (osmo_amr_bytes(amrh->ft) != amr_payload_len) {
LOGP(DLMUX, LOGL_ERROR,
- "Bad AMR frame, expected %zd bytes, got %d bytes\n",
- osmo_amr_bytes(amrh->ft), amr_len);
+ "Bad AMR frame FT=%u, expected %zd bytes, got %d bytes\n",
+ amrh->ft, osmo_amr_bytes(amrh->ft), amr_len);
return -1;
}
return amr_payload_len;
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/30150
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Idd9712644d9a6204f1fe972cbbd393d4b7fd34f6
Gerrit-Change-Number: 30150
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: jtavares.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-remsim/+/30139 )
Change subject: bankd: Add GSMTAP functionality for SIM traffic
......................................................................
Patch Set 2:
(5 comments)
Patchset:
PS2:
In general I'd like to see this feature included, just some minor stuff, see comments.
File src/bankd/gsmtap.c:
https://gerrit.osmocom.org/c/osmo-remsim/+/30139/comment/5090e9b3_1e1d2f79
PS2, Line 28: osmo_bankd_gsmtap_init
there's a problem with the naming prefix, sorry. In general, we reserve the osmo_ prefix of global/exported symbols to our libraries like libosmo{core,abis,gsm,netif,sccp,sigtran,mgcp,...}. Applications should generally not use this prefix to avoid potential future namespace collissions with library functions introduced at a later point.
See also https://osmocom.org/projects/cellular-infrastructure/wiki/Coding_standards#…https://gerrit.osmocom.org/c/osmo-remsim/+/30139/comment/07aac0ee_c5a3ea69
PS2, Line 35: perror("unable to open GSMTAP");
normally we'd want to use the libosmocore logging framework with LOGL_ERROR here.
https://gerrit.osmocom.org/c/osmo-remsim/+/30139/comment/c387ce7e_dc36944b
PS2, Line 56: uint8_t *buf = malloc(gross_len);
not critical, just a comment: we could do without a dynamic heap allocation + memcpy here if we were to use writev() instad of write(). This way we could have the header on the stack, and an iovec[3] for header, mdm-tpdu and sim-tpdu.
It's not critical as this is not a performance critical code path.
https://gerrit.osmocom.org/c/osmo-remsim/+/30139/comment/e54d931c_c9643199
PS2, Line 83: perr
normally we'd want to use the libosmocore logging framework with LOGL_ERROR here.
--
To view, visit https://gerrit.osmocom.org/c/osmo-remsim/+/30139
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-remsim
Gerrit-Branch: master
Gerrit-Change-Id: I05b599858d8758633aa56c3f12f258c27cf42d08
Gerrit-Change-Number: 30139
Gerrit-PatchSet: 2
Gerrit-Owner: jtavares <jtavares(a)kvh.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: jtavares <jtavares(a)kvh.com>
Gerrit-Comment-Date: Mon, 14 Nov 2022 17:12:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: jtavares.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-remsim/+/30138 )
Change subject: rspro_client: implement re-establish delay
......................................................................
Patch Set 2: Code-Review+1
(1 comment)
File src/rspro_client_fsm.c:
https://gerrit.osmocom.org/c/osmo-remsim/+/30138/comment/4c5ce3c9_5a8e61c9
PS1, Line 227: const
> It might have only come about in C90 or C99-- I honestly don't know the history, but it has been the […]
Certainly no need to change, I really do like it.
--
To view, visit https://gerrit.osmocom.org/c/osmo-remsim/+/30138
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-remsim
Gerrit-Branch: master
Gerrit-Change-Id: I86cdc3ba37482e6577b429194d273a2399f32208
Gerrit-Change-Number: 30138
Gerrit-PatchSet: 2
Gerrit-Owner: jtavares <jtavares(a)kvh.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Attention: jtavares <jtavares(a)kvh.com>
Gerrit-Comment-Date: Mon, 14 Nov 2022 17:04:22 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: jtavares <jtavares(a)kvh.com>
Gerrit-MessageType: comment
Attention is currently required from: laforge.
jtavares has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-remsim/+/30138 )
Change subject: rspro_client: implement re-establish delay
......................................................................
Patch Set 2:
(2 comments)
Patchset:
PS2:
Replying to use of const
File src/rspro_client_fsm.c:
https://gerrit.osmocom.org/c/osmo-remsim/+/30138/comment/cf082a84_6f94675e
PS1, Line 227: const
> I had no idea you can use 'const' this way. I only knew it in the context of 'static const', i.e. […]
It might have only come about in C90 or C99-- I honestly don't know the history, but it has been there for a while. I admit that it is a pattern found more in C++ than C codebases, but I'm personally very much in favor of it as a good practice. Helps express the intent, and prevents accidental changes in a variety of situations.
That said, ultimately this your project and so if you prefer it removed, just let me know and I can nuke it. I'll leave unless I hear otherwise.
--
To view, visit https://gerrit.osmocom.org/c/osmo-remsim/+/30138
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-remsim
Gerrit-Branch: master
Gerrit-Change-Id: I86cdc3ba37482e6577b429194d273a2399f32208
Gerrit-Change-Number: 30138
Gerrit-PatchSet: 2
Gerrit-Owner: jtavares <jtavares(a)kvh.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Mon, 14 Nov 2022 16:18:20 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment
Attention is currently required from: laforge.
Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-remsim/+/30138
to look at the new patch set (#2).
Change subject: rspro_client: implement re-establish delay
......................................................................
rspro_client: implement re-establish delay
- add new SRVC_ST_REESTABLISH_DELAY state with delay stipulated by table
k_reestablish_delay_s[], that implements a simple exponential-like back-off
with an upper bound.
- new function srvc_do_reestablish() is used to initiate a reestablish, and
apply the appropriate delay, if any.
- takes external delays (such as TCP connect() delay) into account, and does
not double-penalize.
- delay is reset to shortest possible if there has been no reestablish
initiated in a long time (2x greater than our longest delay). Allows for
fast reconnects even if a delay was used to connect.
- addresses issues https://osmocom.org/issues/5348 and https://osmocom.org/issues/5610
Change-Id: I86cdc3ba37482e6577b429194d273a2399f32208
---
M src/rspro_client_fsm.c
M src/rspro_client_fsm.h
2 files changed, 118 insertions(+), 12 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-remsim refs/changes/38/30138/2
--
To view, visit https://gerrit.osmocom.org/c/osmo-remsim/+/30138
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-remsim
Gerrit-Branch: master
Gerrit-Change-Id: I86cdc3ba37482e6577b429194d273a2399f32208
Gerrit-Change-Number: 30138
Gerrit-PatchSet: 2
Gerrit-Owner: jtavares <jtavares(a)kvh.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: pespin.
osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-iuh/+/30088 )
Change subject: asn1tostruct: fix defines getting redefined
......................................................................
Patch Set 3:
(1 comment)
Patchset:
PS3:
> ACK. IIUC you didn't detect any code change required after this change in the users of the lib?
yes
> In any case it may make sense to apply the renaming only for fields count > 1 (so that if there's 2 or more fields colliding, the first one is kept the old way).
Then we would actually change what the define does. Let's say you use the handy ENHANCEDRELOCATIONCOMPLETEREQUESTIES_RANAP_EXTENDEDRNC_ID_PRESENT define from the example in the commit message, then before this patch it means "(1 << 1)". After this patch, if we keep the name of the define for the first entry and rename the second one, it will mean "(1 << 0)".
I think it's best to cause an error if this really has been used somewhere and we did not catch it since the behavior may not have been what the user intended in the first place.
--
To view, visit https://gerrit.osmocom.org/c/osmo-iuh/+/30088
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-iuh
Gerrit-Branch: master
Gerrit-Change-Id: I2ecae6789899952d1dc5691ab76907abeaa71c12
Gerrit-Change-Number: 30088
Gerrit-PatchSet: 3
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 14 Nov 2022 15:30:14 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: osmith <osmith(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment