Attention is currently required from: laforge.
Hello Jenkins Builder, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33180
to look at the new patch set (#5).
Change subject: include Global RNC-ID in RESET-ACK
......................................................................
include Global RNC-ID in RESET-ACK
According to 3GPP TS 25.413 8.26.2.1, "The RNC shall include the Global
RNC-ID IE in the RESET ACKNOWLEDGE message."
Related: SYS#6441
Change-Id: I49d351e90dfe4a7c4dfdd26542565f2d9bd3d605
---
M src/osmo-hnbgw/cnlink.c
1 file changed, 42 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/80/33180/5
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33180
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I49d351e90dfe4a7c4dfdd26542565f2d9bd3d605
Gerrit-Change-Number: 33180
Gerrit-PatchSet: 5
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: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33180 )
Change subject: include Global RNC-ID in RESET-ACK
......................................................................
Patch Set 4:
(1 comment)
File src/osmo-hnbgw/cnlink.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33180/comment/a1cf4c29_58d4fd1b
PS4, Line 187: i
> same comment as in previous patch about RESET
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33180
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I49d351e90dfe4a7c4dfdd26542565f2d9bd3d605
Gerrit-Change-Number: 33180
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: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Thu, 08 Jun 2023 01:47:05 +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.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33179 )
Change subject: include Global RNC-ID in RESET
......................................................................
Patch Set 4:
(1 comment)
File src/osmo-hnbgw/cnlink.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33179/comment/7f9d43cd_07ec19d9
PS4, Line 147: /
> why are we handling that case if it is in violation of the 3GPP specs? For backwards compatibility […]
yes, it's the backwards compat idea.
until recently Id3eefdea889a736fd5957b80280fa45b9547b792 , osmo-hnbgw never even emitted a RANAP RESET message in the first place. We only ever responded with RESET ACK. So there isn't really much point in staying backwards compatible to nothing.
But the point is, osmo-hnbgw.cfg without a PLMN used to work before this patch, and we should try not to disrupt a working config.
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33179
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I2cd5d7ea86a6ed0916befe219dbf21373afbd95b
Gerrit-Change-Number: 33179
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Comment-Date: Thu, 08 Jun 2023 01:46:51 +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: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33178 )
Change subject: cfg: add 'hnbgw' / 'plmn MCC MNC'
......................................................................
Patch Set 4:
(1 comment)
Patchset:
PS4:
> Comments not yet addressed
(btw, in case it is interesting to know why this occurs / to explain that this is not meant to ignore comments: this regularly happens from fixing problems in earlier patches, resolving merge conflicts through the branch, and re-submitting for gerrit verification to see that it still builds; I had not gotten as far as addressing comments on patches later in the branch, when already submitting fixes for earlier patches.)
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33178
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: If404c3859acdfba274c719c7cb7d16f97d831a2a
Gerrit-Change-Number: 33178
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 08 Jun 2023 01:10:51 +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: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33178 )
Change subject: cfg: add 'hnbgw' / 'plmn MCC MNC'
......................................................................
Patch Set 4:
(2 comments)
File src/osmo-hnbgw/hnbgw_l3.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33178/comment/30213fcc_03b4753f
PS3, Line 276: /* The user has not configured a PLMN, gues from the InitialUE message's LAI IE's PLMN */
> guess
Done
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33178/comment/070ab3dd_c32d6af2
PS3, Line 281: osmo_plmn_from_bcd(ies->lai.pLMNidentity.buf, &local_plmn);
> maybe adding a log line here to let the user know we are deciding PLMN on our own here.
a log line here would say something like
Using local PLMN as indicated in RANAP InitialUE message: 001-01
and the other if-branch would say
Using local PLMN from cfg file: 001-01
Is that what you have in mind?
In general, it would be good if you could give a very specific example of what you would insert, so we don't need to ping pong on the details.
BTW another idea would be to compare the InitialUE PLMNs (there are something like four of them in there) with the PLMN from the cfg file, but i was too busy for that yet.
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33178
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: If404c3859acdfba274c719c7cb7d16f97d831a2a
Gerrit-Change-Number: 33178
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 08 Jun 2023 01:04:37 +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: laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33173 )
Change subject: cnpool: add context_map_cnlink_lost() handling
......................................................................
Patch Set 4:
(2 comments)
File include/osmocom/hnbgw/context_map.h:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33173/comment/92775ff8_2337b3d1
PS3, Line 70: MAP_SCCP_EV_CN_LINK_LOST,
> I'm also a bit confused. […]
Scenario:
- There are many individual SCCP connections to a given MSC, SCCP Connection-Oriented.
- Then there we receive a single RANAP RESET, SCCP connection-less UDT.
The connection-less RESET pre-empts all the many individual UE conns.
When the MSC sends us a RESET, it tells us in a connection-less way that it has restarted, and we should no longer attempt to use any SCCP connection-oriented conns to that MSC that were active before = any connected SCCP CO to this peer are definitely lost. (There should be the usual SCCP teardown happening, worst case from inactivity timeouts, but in this situation we want to make sure to flush out previous state to start afresh with the newly restarted MSC.)
It would be a misnomer / layer mixup to call this event RX_RESET, because this FSM is about a single UE's SCCP CO. Obviously the SCCP CO does not receive a RESET. Instead, the connection-less unitdata received a RESET, and in consequence, the SCCP CO are all now to be considered lost.
idk, is _EV_RANAP_LOST any better?
The same action would qualify for any other reason why we might want all SCCP CO to get lost with the least possible amount of wire activity; that's why i picked this more general name. _EV_RANAP_RESET is technically too specific.
File src/osmo-hnbgw/context_map_sccp.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33173/comment/63e08382_1ec6a77b
PS3, Line 385: case MAP_SCCP_EV_CN_LINK_LOST:
> MAP_SCCP_EV_CN_LINK_RX_RESET?
(see above)
(let me resolve this one, so there is a single unresolved item for this CR aspect, above)
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33173
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Ic0a6fcfb747dc093528ca2bd12a269ad390d465c
Gerrit-Change-Number: 33173
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 08 Jun 2023 00:49:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin.
neels 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/96cbece1_0acd35c4
PS3, Line 373: result = talloc_asprintf(OTC_SELECT, "%s %s", result, ctx->identity_info);
> Ack
same answer as other patch: IMHO this is the best way.
I do not think changing this would have any noticeable impact on performance of osmo-hnbgw.
If it turns out to be performance critical, most definitely outputting the log line is much more heavy on load, and the only solution with any noticeable effect is anyway to scale down logging by cranking up category levels. You're of course aware that all of this code is skipped when no target will output the line.
Another approach you might suggest would be a 'name' cache added to struct hnb_context, and make sure to keep it updated as hnb_context members change. I would rather avoid that complexity: The current code is guaranteed to never print outdated items.
Have you guys thought this through? Am I missing something?
File src/osmo-hnbgw/hnbgw_cn.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33171/comment/1cf17292_fae68e7d
PS3, Line 515: LOG_CNLINK(cnlink, DCN, LOGL_NOTICE, "using: cs7-%u %s <-> %s %s %s\n",
> Ack
Some context: this is the single "i am ready" notification for one SCCP link to an MSC or SGSN. This is called exactly once per 'msc N' and 'sgsn N' at program startup, at the point where the cn link to the msc or sgsn has resolved the cs7 instance N that it will use, and local and remote point-codes for this CN peer. The function might be called again *only* when the user changes the SCCP configuration via telnet VTY so that SCCP links need to be restarted.
This would be a single 'LOG_CNLINK(...)' somewhere, but some scenarios reach this point in a different code path.
If you still think it is important to change this code:
wdym by 'extra log line'?
Do i understand this right: you are saying, it should be a macro to retain the caller's __FILE__ __LINE__ information?
--
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: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 08 Jun 2023 00:22:25 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels, laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133 )
Change subject: cnpool: extract Mobile Identity from RANAP payload
......................................................................
Patch Set 5:
(1 comment)
File src/osmo-hnbgw/hnbgw_l3.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133/comment/df92cabd_71e8c731
PS5, Line 101: return -ENOSPC;
oh yes, and the new patch set also does proper bounds checking
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/33133
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I373d665c9684b607207f68094188eab63209db51
Gerrit-Change-Number: 33133
Gerrit-PatchSet: 5
Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Jun 2023 23:35:06 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment