October 2024 - gerrit-log - lists.osmocom.org

[S] Change in osmo-msc[master]: libvlr: replace direct call of paging_expired() into a callback
by daniel 10 Oct '24

10 Oct '24

Attention is currently required from: laforge, lynxis lazus. daniel has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-msc/+/38107?usp=email ) Change subject: libvlr: replace direct call of paging_expired() into a callback ...................................................................... Patch Set 3: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/38107?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I7a3d15e8f0fb51c6b32add2de5024fc4d599ecf0 Gerrit-Change-Number: 38107 Gerrit-PatchSet: 3 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Thu, 10 Oct 2024 12:21:16 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[L] Change in osmo-sgsn[master]: Add Routing Areas
by fixeria 10 Oct '24

10 Oct '24

Attention is currently required from: daniel, lynxis lazus. fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email ) Change subject: Add Routing Areas ...................................................................... Patch Set 15: (1 comment) File tests/testsuite.at: https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/21760efe_2f965942?us… : PS15, Line 21: ignore > I tend to use stderr for output meant for humans, which can also contain random data. But in this specific case the stderr contains logging generated by test. It might be a good idea to match at least the `DRA` related logging here. We don't seem to be doing this here in osmo-sgsn, but we do in other projects (e.g. https://cgit.osmocom.org/osmo-bts/tree/tests/power/ms_power_loop_test.err) -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I2474b19a7471a1dea3c863ddf8372b16180211aa Gerrit-Change-Number: 37864 Gerrit-PatchSet: 15 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 09 Oct 2024 22:07:21 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: lynxis lazus <lynxis(a)fe80.eu>

1 0

[L] Change in osmo-sgsn[master]: Add Routing Areas
by lynxis lazus 10 Oct '24

10 Oct '24

Attention is currently required from: daniel, fixeria. lynxis lazus has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email ) Change subject: Add Routing Areas ...................................................................... Patch Set 15: (2 comments) File tests/gprs_routing_area/gprs_routing_area_test.c: https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/f36e3e8c_179c3ccf?us… : PS15, Line 329: static struct log_info_cat gprs_categories[] = { > Do we really need all these logging categories for the test? […] Most likely not, I copied it over from the other tests. File tests/testsuite.at: https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/835d378e_9aa6f937?us… : PS15, Line 21: ignore > Why not matching expected stderr too? I tend to use stderr for output meant for humans, which can also contain random data. -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I2474b19a7471a1dea3c863ddf8372b16180211aa Gerrit-Change-Number: 37864 Gerrit-PatchSet: 15 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Wed, 09 Oct 2024 21:55:48 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[XS] Change in osmo-msc[master]: vlr: use correct types for vlr_timer
by pespin 10 Oct '24

10 Oct '24

Attention is currently required from: fixeria, lynxis lazus. pespin has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-msc/+/38356?usp=email ) Change subject: vlr: use correct types for vlr_timer ...................................................................... Patch Set 1: (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-msc/+/38356/comment/1b402e94_dced8550?usp… : PS1, Line 9: an integer > ... […] an integer is signed (can be negative or positive) by definition unlesss explicitly stated, I see no problem with this. -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/38356?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ic46ddb578962619fc08bd63645c76852e676fcc9 Gerrit-Change-Number: 38356 Gerrit-PatchSet: 1 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 09 Oct 2024 21:38:43 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[XS] Change in osmo-msc[master]: vlr: use correct types for vlr_timer
by fixeria 09 Oct '24

09 Oct '24

Attention is currently required from: lynxis lazus. fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-msc/+/38356?usp=email ) Change subject: vlr: use correct types for vlr_timer ...................................................................... Patch Set 1: Code-Review+1 (1 comment) Commit Message: https://gerrit.osmocom.org/c/osmo-msc/+/38356/comment/dc202da6_462a3577?usp… : PS1, Line 9: an integer ... **signed** integer -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/38356?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: Ic46ddb578962619fc08bd63645c76852e676fcc9 Gerrit-Change-Number: 38356 Gerrit-PatchSet: 1 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 09 Oct 2024 19:50:24 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

1 0

[S] Change in osmo-sgsn[master]: Use %d instead of %i
by fixeria 09 Oct '24

09 Oct '24

Attention is currently required from: dexter, lynxis lazus. fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/38281?usp=email ) Change subject: Use %d instead of %i ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/38281?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: Icd5f5f9b0d2ed8fd82afa406787acb4bd95358bf Gerrit-Change-Number: 38281 Gerrit-PatchSet: 4 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 09 Oct 2024 19:48:43 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in osmo-sgsn[master]: move gsm48_gmm_att_tlvdef into gprs_gmm_util
by fixeria 09 Oct '24

09 Oct '24

Attention is currently required from: daniel, lynxis lazus. fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/37868?usp=email ) Change subject: move gsm48_gmm_att_tlvdef into gprs_gmm_util ...................................................................... Patch Set 15: (4 comments) File include/osmocom/sgsn/gprs_gmm_util.h: https://gerrit.osmocom.org/c/osmo-sgsn/+/37868/comment/693a3aa5_ffce1f5e?us… : PS15, Line 1: #ifndef _GPRS_GMM_UTIL_H `#pragma once` https://gerrit.osmocom.org/c/osmo-sgsn/+/37868/comment/8a8c60c2_d2db8885?us… : PS15, Line 6: struct msgb; Why adding it here? File src/sgsn/gprs_gmm_util.c: https://gerrit.osmocom.org/c/osmo-sgsn/+/37868/comment/e1617968_d4877d83?us… : PS15, Line 9: Author: Alexander Couzens <lynxis(a)fe80.eu> `git-blame` tells me that Harald and Pau are the authors of code you're moving here. https://gerrit.osmocom.org/c/osmo-sgsn/+/37868/comment/b97122e7_4c6d38e1?us… : PS15, Line 25: #include <osmocom/gsm/gsm48.h> Unneeded import? -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/37868?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I67dcdb986fd01dc093501d324b5c376246a5d30d Gerrit-Change-Number: 37868 Gerrit-PatchSet: 15 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 09 Oct 2024 19:44:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[S] Change in osmo-sgsn[master]: Refactor diffing same GMM messages
by fixeria 09 Oct '24

09 Oct '24

Attention is currently required from: daniel, lynxis lazus. fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/37867?usp=email ) Change subject: Refactor diffing same GMM messages ...................................................................... Patch Set 15: (3 comments) Commit Message: https://gerrit.osmocom.org/c/osmo-sgsn/+/37867/comment/be19f081_f61eaf54?us… : PS15, Line 9: isn'nt typo: isn't File src/sgsn/gprs_gmm.c: https://gerrit.osmocom.org/c/osmo-sgsn/+/37867/comment/885cad83_f2335cd8?us… : PS15, Line 1432: Checks if two attach request contain the IEs and IE values This needs to be updated. https://gerrit.osmocom.org/c/osmo-sgsn/+/37867/comment/2fa1e281_be90b6eb?us… : PS15, Line 1435: return 1/-1 following memcmp() This is not true. From `man memcmp`: ``` The memcmp() function returns an integer less than, equal to, or greater than zero ... ``` Now it's a positive or negative integer on mismatch. -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/37867?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: Ie698d3a6894a5796663c22c8bfd12b47acda57e6 Gerrit-Change-Number: 37867 Gerrit-PatchSet: 15 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 09 Oct 2024 19:39:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[M] Change in osmo-sgsn[master]: Implement correct Routing Area based paging
by fixeria 09 Oct '24

09 Oct '24

Attention is currently required from: daniel, lynxis lazus. fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/37866?usp=email ) Change subject: Implement correct Routing Area based paging ...................................................................... Patch Set 14: (1 comment) File src/sgsn/sgsn_libgtp.c: https://gerrit.osmocom.org/c/osmo-sgsn/+/37866/comment/be19b07c_18eb12f1?us… : PS14, Line 869: rate_ctr_inc(rate_ctr_group_get_ctr(mm->ctrg, GMM_CTR_PAGING_PS)); : Why not doing this inside of `sgsn_ra_geran_page_ra()`? Does it make sense to increment the counter if `rc != 0`? -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/37866?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I181da9f656e394ccfcb8999021a5b7e13ca0419f Gerrit-Change-Number: 37866 Gerrit-PatchSet: 14 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 09 Oct 2024 19:32:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

[L] Change in osmo-sgsn[master]: Add Routing Areas
by fixeria 09 Oct '24

09 Oct '24

Attention is currently required from: daniel, lynxis lazus. fixeria has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email ) Change subject: Add Routing Areas ...................................................................... Patch Set 15: (3 comments) File src/sgsn/gprs_routing_area.c: https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/f6d1087d_006a8b62?us… : PS15, Line 1: /* SGSN Routing Area for 2G */ missing a copyright header File tests/gprs_routing_area/gprs_routing_area_test.c: https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/41c9264b_24312cf9?us… : PS15, Line 329: static struct log_info_cat gprs_categories[] = { Do we really need all these logging categories for the test? Why you are not adding DRA here? File tests/testsuite.at: https://gerrit.osmocom.org/c/osmo-sgsn/+/37864/comment/cbd78f7c_56d38119?us… : PS15, Line 21: ignore Why not matching expected stderr too? -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/37864?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: I2474b19a7471a1dea3c863ddf8372b16180211aa Gerrit-Change-Number: 37864 Gerrit-PatchSet: 15 Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 09 Oct 2024 19:22:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0