Attention is currently required from: pespin, msuraev.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/32323 )
Change subject: cnpool: allow separate cs7 for IuPS and IuCS
......................................................................
Patch Set 9:
(2 comments)
File include/osmocom/hnbgw/hnbgw.h:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/32323/comment/2e47297f_08df39f5
PS8, Line 87: DECLARE_HASHTABLE(hnbgw_context_map_by_conn_id, 6);
> maybe add a comment what's going to be added in here (struct hnbgw_cnlink?).
it would be pretty much exactly the name of the table:
/* hash table of struct hnbgw_context_map by SCCP conn ID */
but ok if it helps i can do it
https://gerrit.osmocom.org/c/osmo-hnbgw/+/32323/comment/8c1542b3_125575af
PS8, Line 133: if (!cnlink->hnbgw_sccp_user)
> can this really happen? Is it expected to happen? I find it confusing when seeing all these checks w […]
In places where it's established to be safe i can just use cnlink->hnbgw_sccp_usser->ss7->sccp directly.
This function's reason to exist is to avoid NULL dereferences.
I want to call this function in *any* godforsaken corner case from early init to late cleanup, and never run into a NULL deref. I want to get the osmo_sccp_instance, or NULL if there isn't any, for whichever weird reason there might be.
In the branch's final state, this function is called in two places:
cnlink_sccp_addr_to_str(): return a string for logging -- if it is early link startup logging there might be no SCCP instance, or anything leading up to the SCCP instance, assigned yet.
tx_reset_ack(): the SCCP link should be stable, but we're in the middle of a RESET operation, so it could be some state transition, and rarely happens. I decided to just do the extra check.
I thought I'd need it more often, but two places is already enough to not dup all these checks.
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/32323
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Iea1824f1c586723d989c80a909bae16bd2866e08
Gerrit-Change-Number: 32323
Gerrit-PatchSet: 9
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-CC: msuraev <msuraev(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 31 May 2023 23:49:09 +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, msuraev.
Hello Jenkins Builder, laforge, pespin,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-hnbgw/+/32323
to look at the new patch set (#9).
Change subject: cnpool: allow separate cs7 for IuPS and IuCS
......................................................................
cnpool: allow separate cs7 for IuPS and IuCS
Prepare for CN pooling; allow using separate SS7 instances for IuCS and
IuPS.
New struct hnbgw_sccp_user describes a RANAP SCCP user, one per cs7.
Limit struct hnbgw_cnlink to describing a CN peer, using whichever SCCP
instance that is indicated by hnbgw_sccp_user.
Chart sccp.dot shows the changes made.
Related: SYS#6412
Change-Id: Iea1824f1c586723d989c80a909bae16bd2866e08
---
M doc/charts/sccp.dot
M include/osmocom/hnbgw/context_map.h
M include/osmocom/hnbgw/hnbgw.h
M include/osmocom/hnbgw/hnbgw_cn.h
M src/osmo-hnbgw/context_map.c
M src/osmo-hnbgw/context_map_sccp.c
M src/osmo-hnbgw/hnbgw.c
M src/osmo-hnbgw/hnbgw_cn.c
M src/osmo-hnbgw/hnbgw_rua.c
M src/osmo-hnbgw/hnbgw_vty.c
M src/osmo-hnbgw/osmo_hnbgw_main.c
M tests/config/defaults.vty
M tests/config/one_cs7.vty
M tests/config/one_cs7_with_addrs.vty
M tests/config/one_cs7_with_iucs_addr.vty
M tests/config/one_cs7_with_iups_addr.vty
A tests/config/two_cs7.cfg
A tests/config/two_cs7.vty
A tests/config/two_cs7_with_addrs.cfg
A tests/config/two_cs7_with_addrs.vty
20 files changed, 674 insertions(+), 298 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/23/32323/9
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/32323
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Iea1824f1c586723d989c80a909bae16bd2866e08
Gerrit-Change-Number: 32323
Gerrit-PatchSet: 9
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-CC: msuraev <msuraev(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: laforge, pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/32915 )
Change subject: immediately SCCP RLSD on HNB re-register
......................................................................
Patch Set 4:
(1 comment)
File src/osmo-hnbgw/context_map_sccp.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/32915/comment/189e85c3_f0b12310
PS1, Line 362: tx_sccp_df1(fi, ranap_msg);
: tx_sccp_rlsd(fi);
> > ok. i'm still a bit unclear, is this about only tx_sccp_rlsd() or also the other one? […]
I ack your point in things being more complex in various corner cases, but I'm poking the SCCP-SCOC specifically to change from a connected state to a release, particularly to initiate the RLSD from here towards the remote CN, now.
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/32915
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I4e78617ad39bb73fe92097c8a1a8069da6a7f6a1
Gerrit-Change-Number: 32915
Gerrit-PatchSet: 4
Gerrit-Owner: neels <nhofmeyr(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: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 31 May 2023 22:51:50 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
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: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/32989 )
Change subject: add doc/examples/osmo-hnbgw/osmo-hnbgw-cs7.cfg
......................................................................
Patch Set 2:
(2 comments)
File doc/examples/osmo-hnbgw/osmo-hnbgw-cs7.cfg:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/32989/comment/f6b20c28_f446d772
PS2, Line 13: hnbap-allow-tmsi 1
> this line shows as not really interesting in this example? Why not using the default?
i think i just copied some other file.
in general it makes sense to always allow TMSI for HNBAP: we don't even care about HNBAP UE foo at all -- we ACK it but it has zero effect on RANAP where the business happens.
https://gerrit.osmocom.org/c/osmo-hnbgw/+/32989/comment/d4172bef_4ae61150
PS2, Line 19: log stderr
> usually we put log sections at the top.
is there a reason?
i figure it is actually the least important bit of config, so why not put it in the bottom...
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/32989
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I42c3b434a7339cc3efb27b43c893cfb734de9ca4
Gerrit-Change-Number: 32989
Gerrit-PatchSet: 2
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: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 31 May 2023 22:46:05 +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/+/32991 )
Change subject: add startup config tests
......................................................................
Patch Set 5:
(1 comment)
File tests/Makefile.am:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/32991/comment/48534468_69a41e9d
PS5, Line 69: $(srcdir)/config/run_tests.sh "$(top_builddir)/src/osmo-hnbgw/osmo-hnbgw" "$(srcdir)/config/" $U
> I fail to see why cannot you add the for loop directly here, I would expect the entire target is run […]
i guess yes, but multiline shell script in a makefile is always kind of ugly, no?
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/32991
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Ic8bb30e1dd73753c2ff255566382e241918414f7
Gerrit-Change-Number: 32991
Gerrit-PatchSet: 5
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: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 31 May 2023 22:40:13 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/33105 )
Change subject: tbf: Improve TBF name description in logs
......................................................................
tbf: Improve TBF name description in logs
Change format to print the state at the end, to resemble more the same
format used by FSMs.
Furthermore, by moving it at the end, print it only when "enclousure" is
requested, aka when not requested by FSM to update its internal name.
The consequence of this logc is that log lines printed from FSM don't
end up with the same state string printed twice in different places.
While at it, shorten the EGPRS/GPRS indicator to one character, which
should be understandable enough since it matches what's usually seen in
mobile phones to signal one or another.
Change-Id: I86b5f042fae77721b22fc026228677bd56768ba9
---
M src/tbf.cpp
M tests/alloc/AllocTest.err
M tests/app_info/AppInfoTest.err
M tests/ms/MsTest.err
M tests/tbf/TbfTest.err
M tests/types/TypesTest.err
M tests/ulc/PdchUlcTest.err
7 files changed, 446,018 insertions(+), 445,999 deletions(-)
Approvals:
dexter: Looks good to me, but someone else must approve
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/33105
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I86b5f042fae77721b22fc026228677bd56768ba9
Gerrit-Change-Number: 33105
Gerrit-PatchSet: 1
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: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged