March 2022 - gerrit-log - lists.osmocom.org

Change in osmo-ttcn3-hacks[master]: BTS_Tests: remove misleading comments in f_sacch_{present,missing}()

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/27487 ) Change subject: BTS_Tests: remove misleading comments in f_sacch_{present,missing}() ...................................................................... BTS_Tests: remove misleading comments in f_sacch_{present,missing}() Change-Id: Ib6264da51d17d8d5d932109afad79d6a9916911b Related: SYS#5838, OS#4008, OS#4009 --- M bts/BTS_Tests.ttcn 1 file changed, 0 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/87/27487/1 diff --git a/bts/BTS_Tests.ttcn b/bts/BTS_Tests.ttcn index 27b004a..b311da1 100644 --- a/bts/BTS_Tests.ttcn +++ b/bts/BTS_Tests.ttcn @@ -1076,7 +1076,6 @@ /* verify that given SACCH payload is present */ private function f_sacch_present(template octetstring l3_exp) runs on ConnHdlr { var L1ctlDlMessage dl; - /* check that the specified SI5 value is actually sent */ timer T_sacch := 3.0; L1CTL.clear; T_sacch.start; @@ -1099,7 +1098,6 @@ /* verify that given SACCH payload is not present */ private function f_sacch_missing(template octetstring l3_exp) runs on ConnHdlr { var L1ctlDlMessage dl; - /* check that the specified SI5 value is actually sent */ timer T_sacch := 3.0; L1CTL.clear; T_sacch.start; -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/27487 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Ib6264da51d17d8d5d932109afad79d6a9916911b Gerrit-Change-Number: 27487 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

3 years, 3 months

1
0
0 0

Change in osmo-bsc-nat[master]: Support forwarding messages with multiple BSCs

by osmith

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476 ) Change subject: Support forwarding messages with multiple BSCs ...................................................................... Patch Set 2: (6 comments) File doc/examples/osmo-bsc-nat/osmo-bsc-nat.cfg: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/305d5e25_b04733ed PS1, Line 16: sccp-address bsc > why is this dropped in this patch? It's not useful to hardcode one BSC's address in the config anymore, with this patch only the dynamically stored BSCs (as soon as they send their RESET) get used. This part of the config was read by sccp_sap_get_peer_addr_out in bsc_nat_fsm.c, which gets dropped in this patch. File include/osmocom/bsc_nat/bsc_nat.h: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/c86a1bdb_7ea8eb31 PS1, Line 49: struct conn { > subscr_conn? conn seems to generic, too many layers, protocols, levels. […] Done https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/03636b9d_e4ad1c0f PS1, Line 87: void bsc_nat_conn_del(struct conn *conn); > subscr_conn_free() Done File src/osmo-bsc-nat/bsc_nat.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/980afcf0_64679fd7 PS1, Line 121: llist_for_each_entry(conn, &bsc_nat->conns, list) { > for_each_entry_safe Done https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/8c2a4248_25bb9de9 PS1, Line 131: llist_for_each_entry(conn, &bsc_nat->conns, list) { > for_each_entry_safe Done https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476/comment/b6250258_c66b3920 PS1, Line 241: llist_for_each_entry(msc, &bsc_nat->mscs, list) { > for_each_entry_safe (previous commit) Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27476 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: I1556aa665fbb0a97507f98794e74820731fa6935 Gerrit-Change-Number: 27476 Gerrit-PatchSet: 2 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: Fri, 11 Mar 2022 12:35:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

3 years, 3 months

1
0
0 0

Change in osmo-bsc-nat[master]: sccp_sap_up_ran: ignore RAN until MSC is connected

by osmith

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27475 ) Change subject: sccp_sap_up_ran: ignore RAN until MSC is connected ...................................................................... Patch Set 2: (2 comments) File include/osmocom/bsc_nat/msc_fsm.h: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27475/comment/0de6590c_e89b7a90 PS1, Line 24: bool msc_fsm_is_connected(struct msc *msc); > I see lots of functions msc_fsm_* being used in lots of places, even outside "msc" object. […] Done File src/osmo-bsc-nat/bsc_nat_fsm.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27475/comment/320d7a9c_81a53805 PS1, Line 218: if (!msc_fsm_is_connected(msc)) { > msc_is_connected. I don't care about fsm internals here. Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27475 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: Idf9501412484fa92e8836952609fba7c5443d6c9 Gerrit-Change-Number: 27475 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 11 Mar 2022 12:35:36 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

3 years, 3 months

1
0
0 0

Change in osmo-bsc-nat[master]: Send RESET to MSC on start up

by osmith

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27474 ) Change subject: Send RESET to MSC on start up ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc-nat/msc_fsm.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27474/comment/46a73fe1_cf97f292 PS1, Line 33: }; > You should add the entire FSM file in this commit, not the previous one. Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27474 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: Icfec3ec0168c7040e88a536fa48da339349fb6cf Gerrit-Change-Number: 27474 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 11 Mar 2022 12:35:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

3 years, 3 months

1
0
0 0

Change in osmo-bsc-nat[master]: Store MSC

by osmith

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27473 ) Change subject: Store MSC ...................................................................... Patch Set 2: (6 comments) File include/osmocom/bsc_nat/bsc_nat.h: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27473/comment/14c166f9_e4d8483c PS1, Line 57: struct msc *bsc_nat_msc_add_from_addr_book(struct bsc_nat *bsc_nat); > I wonder what is this function doing. See the other comment. https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27473/comment/790a2f45_dc6d0678 PS1, Line 58: struct msc *bsc_nat_msc_get(struct bsc_nat *bsc_nat); > you get it identified by nothing? Yes, since there is currently only one MSC. I'd adjust the function when support for multiple MSCs gets added. As I understand it, MSC pooling is outside of the scope of SYS#5560, OS#2545. Related: https://osmocom.org/projects/osmo-bscnat/wiki/AoIP_OsmoBSCNAT#MSC-pooling File src/osmo-bsc-nat/bsc_nat.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27473/comment/5cf854e3_92289cf0 PS1, Line 70: struct msc *bsc_nat_msc_add_from_addr_book(struct bsc_nat *bsc_nat) > Not sure we really need this function, just use the content directly, since this will go away at som […] This function adds the msc from the following block in the config: cs7 instance 0 sccp-address msc routing-indicator PC point-code 0.23.1 subsystem-number 254 When OsmoBSCNAT supports multiple MSCs, it does not go away, but rather we need to read multiple msc entries from the config (msc0, msc1, ... probably). This is necessary, because the BSCNAT needs to send the RESET towards the MSC, so it needs to know its address. (We don't need to know the address of the BSCs, since they send the RESET to BSCNAT and then we know the address.) Therefore I'd keep a function for adding the MSC from the address book. But the return value isn't really used and doesn't make sense once this parses multiple MSCs, I've changed it to int, return 0 on success and -ENOENT on error. https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27473/comment/8e96e1d2_760f0485 PS1, Line 93: void bsc_nat_msc_del(struct bsc_nat *bsc_nat, struct msc *msc) > IMHO this should be an API of the MSC object (see the bsc_nat is actually not used at all). […] Done File src/osmo-bsc-nat/main.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27473/comment/160b1db0_f353ef48 PS1, Line 202: if (!bsc_nat_msc_add_from_addr_book(g_bsc_nat)) > You can probably call the content of the function here directly. […] IMHO it makes main() a bit more readable if this remains inside a function. File src/osmo-bsc-nat/msc_fsm.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27473/comment/e93f3000_f480aeb7 PS1, Line 44: { 0, NULL } > this half done FSM shouldn't be submitted this way. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27473 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: I711df0c649728f1007857fbfda500ed5ef69287b Gerrit-Change-Number: 27473 Gerrit-PatchSet: 2 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: Fri, 11 Mar 2022 12:35:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

3 years, 3 months

1
0
0 0

Change in osmo-bsc-nat[master]: Store BSCs

by osmith

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27472 ) Change subject: Store BSCs ...................................................................... Patch Set 2: (3 comments) File include/osmocom/bsc_nat/bsc_nat.h: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27472/comment/b372e9af_fb935cf0 PS1, Line 43: struct llist_head bscs; /* list of struct bsc */ > I think we usually would use bsc_list here, but not critical. If it's not that important, I'd prefer to keep it as-is. File src/osmo-bsc-nat/bsc_nat.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27472/comment/51116ae5_17d578d7 PS1, Line 94: llist_for_each_entry(bsc, &bsc_nat->bscs, list) { > This needs to be for_each_entry_safe AFAICT, otherwise you get a use-after-free. ACK, thanks! File src/osmo-bsc-nat/bssap.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27472/comment/9392a6ad_b766b8f7 PS1, Line 44: bsc = bsc_nat_bsc_add(g_bsc_nat, addr); > could this return null? No, it does talloc_zero and OSMO_ASSERT on it. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27472 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: Icd7316c49ef26fb45ad45a2ccc1a7916bfb0a387 Gerrit-Change-Number: 27472 Gerrit-PatchSet: 2 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: Fri, 11 Mar 2022 12:35:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

3 years, 3 months

1
0
0 0

Change in osmo-bsc-nat[master]: Reply to BSC's RESET with RESET ACK directly

by osmith

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27471 ) Change subject: Reply to BSC's RESET with RESET ACK directly ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc-nat/bssap.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27471/comment/779cde45_3d2688d4 PS1, Line 70: return bssap_ran_rcvmsg_udt(addr, msg, length - sizeof(struct bssmap_header)); > shouldn't the length field check in the "if" clause above be checked against sizeof(struct bssmap_he […] No, because sizeof(struct bssmap_header) is already substracted from length in line 95. I've based this on how it's done in OsmoBSC: https://git.osmocom.org/osmo-bsc/tree/src/osmo-bsc/osmo_bsc_bssap.c?id=663d… -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27471 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: I3223409e25c93b625d67634caf68efe9772e2558 Gerrit-Change-Number: 27471 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 11 Mar 2022 12:35:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

3 years, 3 months

1
0
0 0

Change in osmo-bsc-nat[master]: bsc_nat_sccp_inst: local_sccp_addr -> addr

by osmith

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27470 ) Change subject: bsc_nat_sccp_inst: local_sccp_addr -> addr ...................................................................... Patch Set 2: (1 comment) File include/osmocom/bsc_nat/bsc_nat.h: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27470/comment/bbee2312_1d1a38a2 PS1, Line 28: struct osmo_sccp_addr addr; /* OsmoBSCNAT's local address */ > Maybe loc_addr or local_addr would be more intuitive when reading code. I've used addr in other places too in later patches in this patchset, IMHO it is nicely consistent and short: sccp_inst->addr subscr_conn->bsc->addr subscr_conn->msc->addr Since there is no other address for the sccp_inst it should be clear that it's the local address. Also there's the comment in case it's not clear. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27470 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: I3083374d589487ed960507e7a431c45914afc5dd Gerrit-Change-Number: 27470 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 11 Mar 2022 12:35:11 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

3 years, 3 months

1
0
0 0

Change in osmo-bsc-nat[master]: bsc_nat_fsm: drop LOG_SCCP macro

by osmith

Attention is currently required from: fixeria. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27468 ) Change subject: bsc_nat_fsm: drop LOG_SCCP macro ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc-nat/bsc_nat_fsm.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27468/comment/a76ee290_cb5fdf96 PS1, Line 75: " is neither called address %s nor calling address %s!\n", > Alignment is broken here. Not critical, but makes the code look inconsistent. Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27468 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: I49187d4e3d804c4786ee52eaebdd911e6c6cfa6c Gerrit-Change-Number: 27468 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Fri, 11 Mar 2022 12:35:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

3 years, 3 months

1
0
0 0

Change in osmo-bsc-nat[master]: bsc_nat_fsm: tweak forwarding log messages

by osmith

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27467 ) Change subject: bsc_nat_fsm: tweak forwarding log messages ...................................................................... Patch Set 2: (1 comment) File src/osmo-bsc-nat/bsc_nat.c: https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27467/comment/42d9af46_816e7570 PS1, Line 60: snprintf(buf, sizeof(buf), "PC=%s in %s", osmo_ss7_pointcode_print(NULL, addr->pc), > Sound like we can improve/shorten this further: CN-PC=%s and RAN-PC=%s That's shorter yes, but I'd argue "PC=%s in RAN" and "PC=%s in CN" is more readable and fits better with "BSC(PC=%s)" and "MSC(PC=%s)" in later patches. You gave +1 so it seems it's not that important to you and I'll leave it as is. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc-nat/+/27467 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc-nat Gerrit-Branch: master Gerrit-Change-Id: I85f65ad3c15a10958fbfe97aef00fc386e85ef63 Gerrit-Change-Number: 27467 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 11 Mar 2022 12:35:00 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

3 years, 3 months

1
0
0 0

2025

2024

2023

2022

gerrit-log March 2022