Attention is currently required from: laforge.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/33318 )
Change subject: [untested] layer23: migrate away from gsm48_generate_mid_from_*
......................................................................
Patch Set 1:
(1 comment)
File src/host/layer23/src/mobile/gsm48_mm.c:
https://gerrit.osmocom.org/c/osmocom-bb/+/33318/comment/a0821863_78963b18
PS1, Line 310: int rc = osmo_mobile_identity_encode_buf(buf, 11, &mi, false);
I think this one is missing the encoding of the GSM48_IE_MOBILE_ID part. See libosmo-gprs.git/src/gmm/gmm_pdu.c:238
--
To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/33318
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmocom-bb
Gerrit-Branch: master
Gerrit-Change-Id: Ib0d7f76cd635e8d1092ffc1d07ecb29ec0435dda
Gerrit-Change-Number: 33318
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Wed, 14 Jun 2023 14:40:56 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin, dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310 )
Change subject: mgwpool: Document keepalive feature
......................................................................
Patch Set 3:
(1 comment)
File common/chapters/mgwpool.adoc:
https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310/comment/1a823131_289b…
PS2, Line 203: considering
> https://dictionary.cambridge.org/dictionary/english/consider […]
Not willing to block here, but it reads as: "the timeout considering the MGW to be non-reachable". I suggest: "A `keepalive timeout`, which on expiry will declare the MGW as non-reachable ...".
--
To view, visit https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-gsm-manuals
Gerrit-Branch: master
Gerrit-Change-Id: I2cb4e2098b71b386278eb6026271a6d786a34c2a
Gerrit-Change-Number: 33310
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 14 Jun 2023 14:22:31 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin, dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/33312 )
Change subject: mgw: Allow auditing speciall 'null' endpoint
......................................................................
Patch Set 2:
(1 comment)
File src/libosmo-mgcp/mgcp_endp.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/33312/comment/0cf3afbf_c8ebdf24
PS2, Line 237: strstr
From the man page:
```
DESCRIPTION
The strstr() function finds the first occurrence of the substring needle in the string haystack.
RETURN VALUE
These functions return a pointer to the beginning of the located substring, or NULL if the substring is not found.
```
This means that basically any endpoint name containing `null` would be considered a null-endpoint (e.g. epname="notanull")? Maybe better use `strncmp(epname, "null", 4)` or even `strncmp(epname, "null@", 5)` here instead?
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/33312
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: Ia409b16e9211e6261e2e0f21288544289d6f3733
Gerrit-Change-Number: 33312
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 14 Jun 2023 14:16:10 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: fixeria, dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310 )
Change subject: mgwpool: Document keepalive feature
......................................................................
Patch Set 2:
(2 comments)
File common/chapters/mgwpool.adoc:
https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310/comment/1494197b_76cd…
PS2, Line 203: considering
> Not sure if "considering" is the right word here, maybe declaring/making?
https://dictionary.cambridge.org/dictionary/english/consider
"to believe someone or something to be, or think of him, her, or it as something"
So yes, it's fine imho.
https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310/comment/ad4a9cbb_62ba…
PS2, Line 219: 10
> It's 20 in the config snippet, but here you say 10?
Ack
--
To view, visit https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-gsm-manuals
Gerrit-Branch: master
Gerrit-Change-Id: I2cb4e2098b71b386278eb6026271a6d786a34c2a
Gerrit-Change-Number: 33310
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 14 Jun 2023 14:14:21 +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: pespin, dexter.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310 )
Change subject: mgwpool: Document keepalive feature
......................................................................
Patch Set 2:
(3 comments)
File common/chapters/mgwpool.adoc:
https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310/comment/5f2a32a9_ffb3…
PS2, Line 184: on the MGW against the MGW
did you mean "on the MGCP against MGW"?
https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310/comment/afde05f7_a7c5…
PS2, Line 203: considering
Not sure if "considering" is the right word here, maybe declaring/making?
https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310/comment/1e67f434_ac1c…
PS2, Line 219: 10
It's 20 in the config snippet, but here you say 10?
--
To view, visit https://gerrit.osmocom.org/c/osmo-gsm-manuals/+/33310
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-gsm-manuals
Gerrit-Branch: master
Gerrit-Change-Id: I2cb4e2098b71b386278eb6026271a6d786a34c2a
Gerrit-Change-Number: 33310
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 14 Jun 2023 14:07:02 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: laforge, dexter.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/33308 )
Change subject: mgcp-client: Add keepalive feature
......................................................................
Patch Set 4:
(2 comments)
File include/osmocom/mgcp_client/mgcp_client_internal.h:
https://gerrit.osmocom.org/c/osmo-mgw/+/33308/comment/9ec15b9c_c5525e19
PS4, Line 22: conn_up
> one mihgt have differentiated betwween UP, DOWN and UNKNOWN. […]
well, we really never know if they are UP, we just guess they are UP since they answered some time ago. The only thing that really changes is how far ago we ended up knowing that it was alive.
So I'm not really sure we benefit from adding the UNKNOWN state, imho it makes things more confusing, but I can add it if you think it makes more sense.
File src/libosmo-mgcp-client/mgcp_client_vty.c:
https://gerrit.osmocom.org/c/osmo-mgw/+/33308/comment/c9124941_a8682974
PS4, Line 676: pool_member->clien
> one could also keep the bool conn_up and add a check here if keepalives are actually enabled. […]
not sure if it really benefits from operational point of view, since the MGW in UNKNOWN state should be picked at some point (so same as UP), and then at that point it may already become UP.
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/33308
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I3dc74c78548d017f272da863d5282dc5e0020ca3
Gerrit-Change-Number: 33308
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 14 Jun 2023 14:01:41 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: comment