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

[S] Change in osmo-bsc[master]: gprs: fix has_valid_nsvc(): permit local udp port 0
by pespin 31 Mar '23

31 Mar '23

Attention is currently required from: fixeria, daniel, lynxis lazus. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32176 ) Change subject: gprs: fix has_valid_nsvc(): permit local udp port 0 ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: (Copy-pasting from previous Change-Id:) The question here is more: does it make sense from network architecture point of view that PCU binds to a randomly available port for Gb? AFAIU it makes no sense, since it needs to bind to a specific port also known by the SGSN so that it knows where to send packets to. Or is the SGSN available to learn dynamically of the PCU by listening() on its port? Maybe something @lynxis or @daniel will be able to answer better. If it makes no sense that the PCU can bidn to a random port, then sure, let's disallow passing bind port 0. But let's then explain correctly the rationale behind it, which is wrong in current version of this patch. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32176 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I0afd83e77f3daeeb082e7db911610428b5bc587c Gerrit-Change-Number: 32176 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Fri, 31 Mar 2023 14:28:25 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in osmo-bsc[master]: tests: add more tests for GPRS NSVC parameters
by fixeria 31 Mar '23

31 Mar '23

Attention is currently required from: pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32171 ) Change subject: tests: add more tests for GPRS NSVC parameters ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > I am not really liking this approach of "port 0" meaning the NSVC is disabled. […] Neither I do. But we have what we have. I am not changing the logic. This patch is not introducing this approach, but demonstrating it. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32171 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I13d6ac887ddb1c9bc5d1e4122f20405a28f135d4 Gerrit-Change-Number: 32171 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(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, 31 Mar 2023 14:27:41 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-bsc[master]: tests: add more tests for GPRS NSVC parameters
by pespin 31 Mar '23

31 Mar '23

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32171 ) Change subject: tests: add more tests for GPRS NSVC parameters ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32171 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I13d6ac887ddb1c9bc5d1e4122f20405a28f135d4 Gerrit-Change-Number: 32171 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Fri, 31 Mar 2023 14:27:19 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-bsc[master]: tests: rename and extend gprs_{bvci_default->params}.vty
by pespin 31 Mar '23

31 Mar '23

Attention is currently required from: laforge, pespin, daniel, lynxis lazus. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32169 ) Change subject: tests: rename and extend gprs_{bvci_default->params}.vty ...................................................................... Patch Set 2: Code-Review+1 (1 comment) Patchset: PS2: Ok, this is a totally new commit after latest version submit, and the discussion doesn't apply anymore? Not sure if it was the best idea to reuse the Change-Id here. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32169 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ia02e9a97137eb7724f22526fc3768331b35ac55d Gerrit-Change-Number: 32169 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Fri, 31 Mar 2023 14:26:57 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-bsc[master]: tests: rename and extend gprs_{bvci_default->params}.vty
by fixeria 31 Mar '23

31 Mar '23

Attention is currently required from: laforge, pespin, daniel, lynxis lazus. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32169 ) Change subject: tests: rename and extend gprs_{bvci_default->params}.vty ...................................................................... Patch Set 2: (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-bsc/+/32169/comment/cb9ed249_3822af49 PS1, Line 18: The PCU obviously cannot bind to port 0, and will keep re-starting. > The question here is more: does it make sense from network architecture point of view that PCU binds […] I investigated this a bit more and found out that 0 is a perfectly valid value, which has been supported until the following patch was merged: ``` commit 315af2f9ea1e8b9bf6e58caebd9dd7829edecfed Author: Alexander Couzens <lynxis(a)fe80.eu> Date: Mon Dec 19 21:21:32 2022 +0100 ``` See https://gerrit.osmocom.org/c/osmo-bsc/+/32176. I confirm that with this patch applied osmo-pcu happily connects to osmo-sgsn and GPRS service works as expected. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32169 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ia02e9a97137eb7724f22526fc3768331b35ac55d Gerrit-Change-Number: 32169 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Fri, 31 Mar 2023 14:26:00 +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> Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-bsc[master]: tests: add more tests for GPRS NSVC parameters
by pespin 31 Mar '23

31 Mar '23

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32171 ) Change subject: tests: add more tests for GPRS NSVC parameters ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: I am not really liking this approach of "port 0" meaning the NSVC is disabled. We should have some sort of explicitly enabling and disabling NSVCs. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32171 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I13d6ac887ddb1c9bc5d1e4122f20405a28f135d4 Gerrit-Change-Number: 32171 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Fri, 31 Mar 2023 14:25:50 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in osmo-bsc[master]: gprs: fix has_valid_nsvc(): permit local udp port 0
by fixeria 31 Mar '23

31 Mar '23

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32176 ) Change subject: gprs: fix has_valid_nsvc(): permit local udp port 0 ...................................................................... gprs: fix has_valid_nsvc(): permit local udp port 0 NSVC local port 0 is actually a valid value, which tells the PCU to pick a random port for bind()ing. It was supported before 315af2f9e, but now osmo-bsc would simply ignore NSVCs with 'local udp port 0' and leave the respective MOs unconfigured in the BTS. Change-Id: I0afd83e77f3daeeb082e7db911610428b5bc587c Fixes: 315af2f9e "bts: ipa/osmo-bts/sysmobts: MO: add support for the second NSVC" Related: OS#5979 --- M src/osmo-bsc/nm_gprs_nsvc_fsm.c 1 file changed, 22 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/76/32176/1 diff --git a/src/osmo-bsc/nm_gprs_nsvc_fsm.c b/src/osmo-bsc/nm_gprs_nsvc_fsm.c index 5186082..9eb6216 100644 --- a/src/osmo-bsc/nm_gprs_nsvc_fsm.c +++ b/src/osmo-bsc/nm_gprs_nsvc_fsm.c @@ -92,10 +92,15 @@ static bool has_valid_nsvc(struct gsm_gprs_nsvc *nsvc) { + /* remote address must be valid */ + if (osmo_sockaddr_is_any(&nsvc->remote)) + return false; + /* remote port must be valid */ switch (nsvc->remote.u.sa.sa_family) { case AF_INET: + return nsvc->remote.u.sin.sin_port != 0; case AF_INET6: - return (nsvc->local_port > 0 && !osmo_sockaddr_is_any(&nsvc->remote)); + return nsvc->remote.u.sin6.sin6_port != 0; default: return false; } -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32176 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I0afd83e77f3daeeb082e7db911610428b5bc587c Gerrit-Change-Number: 32176 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[M] Change in osmo-bsc[master]: tests: rename and extend gprs_{bvci_default->params}.vty
by fixeria 31 Mar '23

31 Mar '23

Attention is currently required from: laforge, fixeria, daniel, lynxis lazus. Hello Jenkins Builder, laforge, daniel, lynxis lazus, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/32169 to look at the new patch set (#2). Change subject: tests: rename and extend gprs_{bvci_default->params}.vty ...................................................................... tests: rename and extend gprs_{bvci_default->params}.vty Match default values for all GPRS related params, not only the BVCI. Change-Id: Ia02e9a97137eb7724f22526fc3768331b35ac55d Related: OS#5979 --- D tests/gprs_bvci_default.vty A tests/gprs_params.vty 2 files changed, 60 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/69/32169/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32169 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ia02e9a97137eb7724f22526fc3768331b35ac55d Gerrit-Change-Number: 32169 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-MessageType: newpatchset

1 0

[M] Change in osmo-bsc[master]: tests: add more tests for GPRS NSVC parameters
by fixeria 31 Mar '23

31 Mar '23

Attention is currently required from: fixeria. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/32171 to look at the new patch set (#2). Change subject: tests: add more tests for GPRS NSVC parameters ...................................................................... tests: add more tests for GPRS NSVC parameters Change-Id: I13d6ac887ddb1c9bc5d1e4122f20405a28f135d4 Related: OS#5979 --- M tests/gprs_params.vty 1 file changed, 78 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/71/32171/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32171 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I13d6ac887ddb1c9bc5d1e4122f20405a28f135d4 Gerrit-Change-Number: 32171 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-bsc[master]: abis_nm: Only osmo-bts re-purposes the MANUF_ID for BTS feature flags
by laforge 31 Mar '23

31 Mar '23

Attention is currently required from: pespin, fixeria. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/32070 ) Change subject: abis_nm: Only osmo-bts re-purposes the MANUF_ID for BTS feature flags ...................................................................... Patch Set 3: (1 comment) File src/osmo-bsc/abis_nm.c: https://gerrit.osmocom.org/c/osmo-bsc/+/32070/comment/44fd65b4_576cd8f5 PS2, Line 576: if (bts->type == GSM_BTS_TYPE_OSMOBTS && TLVP_PRES_LEN(tp, NM_ATT_MANUF_ID, 2)) { > Probably a switch (bts->type) may be cleaner here. I decided to do away with any nanoBTS handling at this point, as it simply doesn't apply. The features are static. So we don't need a switch as it's a single if-clause. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/32070 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I76cee190dc1f074464df570cdfc3d38559f04846 Gerrit-Change-Number: 32070 Gerrit-PatchSet: 3 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Fri, 31 Mar 2023 14:13:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0