Attention is currently required from: dexter, laforge, neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/34404?usp=email )
Change subject: mgcp_client_fsm: allocate struct mgcp_conn_peer dynamically
......................................................................
Patch Set 16:
(1 comment)
Commit Message:
https://gerrit.osmocom.org/c/osmo-mgw/+/34404/comment/ceb30f2e_838c0de3
PS13, Line 10: an ABI compatibility in case the struct changes.
> (this is dexter's patch being dragged along in my fmtp branch... […]
Still not addressed?
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/34404?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I523d0fcb020f7d46323c497a4be9ee00d5f242ba
Gerrit-Change-Number: 34404
Gerrit-PatchSet: 16
Gerrit-Owner: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 02 Jan 2024 10:05:29 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35302?usp=email )
Change subject: mgw: do not fail MGCP on codec mismatch
......................................................................
Patch Set 5:
(1 comment)
File src/libosmo-mgcp/mgcp_codec.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35302/comment/9af95248_9addc983
PS1, Line 478: LOGP(DLMGCP, LOGL_ERROR, "no matching codec found\n");
> you definetly want to print the endpoint or conns here.
This one has not been addressed.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35302?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I3d1163fe622bdd7dc42a485f796072524ab39db9
Gerrit-Change-Number: 35302
Gerrit-PatchSet: 5
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 02 Jan 2024 10:04:08 +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.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/35434?usp=email )
Change subject: add fmtp.h
......................................................................
Patch Set 4:
(6 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-mgw/+/35434/comment/587ddad9_c963b35b
PS4, Line 9: Upcoming patches will add handling of arbitrary ftmp strings to
fmtp strings
https://gerrit.osmocom.org/c/osmo-mgw/+/35434/comment/b9932791_b59f7fb2
PS4, Line 13: Add generic API for handling ftmp strings. The primary intended user is
fmtp
Patchset:
PS4:
Only minor stuff needs improving/fixing, feel free to merge it once you fix those.
File src/libosmo-mgcp-client/fmtp.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/35434/comment/eb0de39f_5f52c1e6
PS4, Line 29: for (; fmtp && *fmtp && *fmtp != ';'; fmtp++);
fmtp being nonull should be checked only once at start.
https://gerrit.osmocom.org/c/osmo-mgw/+/35434/comment/97ac2864_6c03457c
PS4, Line 41: * if (osmo_sdp_fmtp_get_val(res, sizeof(res), fmtp_vals, "mode-set"))
IIUC res contains "0,2,4,7" here, maybe worth saying that.
https://gerrit.osmocom.org/c/osmo-mgw/+/35434/comment/cad9b599_1fe5110a
PS4, Line 89: int osmo_sdp_fmtp_get_int(const char *fmtp, const char *option_name, int default_value)
did you check in SDP spec if int range is enough?
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/35434?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I3eaea353dbd98c19212199b564342d0ac16cbc07
Gerrit-Change-Number: 35434
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 02 Jan 2024 09:59:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: fixeria.
matanp has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35475?usp=email )
Change subject: abis_rsl: Add rach expiry timeout
......................................................................
Patch Set 4:
(3 comments)
Commit Message:
https://gerrit.osmocom.org/c/osmo-bsc/+/35475/comment/2752ab53_466fba6e
PS3, Line 7: abis_rsl: Separate rach expiry delay from radio link timeout
> Please elaborate more why a separate parameter is needed (here in COMMIT_MSG).
Ack
File src/osmo-bsc/bts_vty.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/35475/comment/6ad723c8_438622ae
PS3, Line 777: expiry timeout
> Let's combine it into a single command item `expiry-timeout`.
Ack
https://gerrit.osmocom.org/c/osmo-bsc/+/35475/comment/c8eba3d6_3ce9690e
PS3, Line 4532: vty_out(vty, " rach max-delay %u%s", bts->rach_max_delay, VTY_NEWLINE);
> You're missing to print `rach expiry-timeout` here.
Ack
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35475?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Iff7266672dd8bc9ce2b34b0478d98fb70691f425
Gerrit-Change-Number: 35475
Gerrit-PatchSet: 4
Gerrit-Owner: matanp <matan1008(a)gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Tue, 02 Jan 2024 07:20:30 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: fixeria, matanp.
Hello Jenkins Builder, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-bsc/+/35475?usp=email
to look at the new patch set (#4).
The following approvals got outdated and were removed:
Code-Review-1 by fixeria, Verified+1 by Jenkins Builder
Change subject: abis_rsl: Add rach expiry timeout
......................................................................
abis_rsl: Add rach expiry timeout
This allows controlling the RACH DoS attack protection without
increasing call drops rate.
Change-Id: Iff7266672dd8bc9ce2b34b0478d98fb70691f425
---
M include/osmocom/bsc/bts.h
M src/osmo-bsc/abis_rsl.c
M src/osmo-bsc/bts.c
M src/osmo-bsc/bts_vty.c
4 files changed, 33 insertions(+), 3 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/75/35475/4
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35475?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Iff7266672dd8bc9ce2b34b0478d98fb70691f425
Gerrit-Change-Number: 35475
Gerrit-PatchSet: 4
Gerrit-Owner: matanp <matan1008(a)gmail.com>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: matanp <matan1008(a)gmail.com>
Gerrit-MessageType: newpatchset