- gerrit-log - lists.osmocom.org

[M] Change in osmo-ttcn3-hacks[master]: buildsystem: move related scripts to subdir

by pespin

Attention is currently required from: dexter, fixeria, jolly, osmith. pespin has posted comments on this change by osmith. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40026?usp=email ) Change subject: buildsystem: move related scripts to subdir ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/40026?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Ieeefdfd74b2698794fb12fec4ed32565dc9f36a6 Gerrit-Change-Number: 40026 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 22 Apr 2025 09:08:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

2 months, 3 weeks

1
0
0 0

[M] Change in pysim[master]: [1/6] personalization: refactor: drop ClassVarMeta use

by laforge

Attention is currently required from: neels. laforge has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39741?usp=email ) Change subject: [1/6] personalization: refactor: drop ClassVarMeta use ...................................................................... Patch Set 9: Code-Review-1 (2 comments) Patchset: PS9: I retain my position that I don't understand why this change is made in the first place, and where it improves upon the existing code. "Not using metaclasses" is not something I understand as an improvement, and clearly the readability has also not improved. I'm also a bit dissatisfied by the fact that this is the first change you introduce as part of your work on this task. It's not a functional change or anything that adds a feature, but it's a change that is mostly cosmetic and alters the existing code to be more to your personal preference without rendering a [AFAICT, and apparently fixeria agrees] clear advantage. File pySim/esim/saip/personalization.py: https://gerrit.osmocom.org/c/pysim/+/39741/comment/6f7d2351_753b426c?usp=em… : PS9, Line 37: class ConfigurableParameter: why is the new ConfugirableParameter not an ABC anymore? Can it be used directly, without further inheriting from it? -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39741?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I60ea8fd11fb438ec90ddb08b17b658cbb789c051 Gerrit-Change-Number: 39741 Gerrit-PatchSet: 9 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 22 Apr 2025 08:57:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

2 months, 3 weeks

1
0
0 0

[M] Change in pysim[master]: [2/6] personalization: refactor ConfigurableParameter, Iccid, Imsi

by laforge

Attention is currently required from: dexter, fixeria, neels. laforge has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39742?usp=email ) Change subject: [2/6] personalization: refactor ConfigurableParameter, Iccid, Imsi ...................................................................... Patch Set 9: Code-Review+1 (1 comment) File pySim/esim/saip/personalization.py: https://gerrit.osmocom.org/c/pysim/+/39742/comment/be49d769_ab10cb9e?usp=em… : PS7, Line 100: allow_types = (str, int, ) > no, the trailing comma does not add a third item. […] Thanks for your explanation. If I was in your shoes, I'd be a bit more hesitant in intoducing personal coding style preferences into a pre-existing large code base that AFAICT doesn't use this anywhere so far. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39742?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I6522be4c463e34897ca9bff2309b3706a88b3ce8 Gerrit-Change-Number: 39742 Gerrit-PatchSet: 9 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 22 Apr 2025 08:53:29 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de> Comment-In-Reply-To: laforge <laforge(a)osmocom.org>

2 months, 3 weeks

1
0
0 0

[L] Change in pysim[master]: personalization: add param_source.py, implement batch personalization

by laforge

Attention is currently required from: neels. laforge has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/40096?usp=email ) Change subject: personalization: add param_source.py, implement batch personalization ...................................................................... Patch Set 3: (3 comments) File pySim/esim/saip/param_source.py: https://gerrit.osmocom.org/c/pysim/+/40096/comment/53b4167f_f48d274a?usp=em… : PS3, Line 33: abstract if it's abstract, should it be an abc.ABC ? https://gerrit.osmocom.org/c/pysim/+/40096/comment/9aa7501a_9cbcced8?usp=em… : PS3, Line 132: RandomDigitSource IMHO it's a bit unintuitive that the IncDigit inherits from RandomDigit. Maybe add a DigitSource from which both IncDigit + RandomDigit inherits? https://gerrit.osmocom.org/c/pysim/+/40096/comment/ddc198b0_c5058a11?usp=em… : PS3, Line 133: 'incrementing sequence of digits' cosmetic: the docstring here uses single-quotes, below for the method it uses double-quotes. This looks a bit inconsistent. In the existing code base, I think we always use multi-line """ style comments for this, irrespective if the actual comment is just a single line. That makes it easier to expand it later on without having to change quoting style. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/40096?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I497c60c101ea0eea980e8b1a4b1f36c0eda39002 Gerrit-Change-Number: 40096 Gerrit-PatchSet: 3 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 22 Apr 2025 08:48:33 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

2 months, 3 weeks

1
0
0 0

[S] Change in pysim[master]: personalization: discover all useful ConfigurableParameter subclasses

by laforge

Attention is currently required from: neels. laforge has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/40095?usp=email ) Change subject: personalization: discover all useful ConfigurableParameter subclasses ...................................................................... Patch Set 2: Code-Review+1 (1 comment) File pySim/esim/saip/personalization.py: https://gerrit.osmocom.org/c/pysim/+/40095/comment/5ae2b002_e9c557b9?usp=em… : PS2, Line 497: is_abstract = False I guess a minor syntactic improvement would be implementation via a decorator instead of a class variable? Not critical at all, just my two cents. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/40095?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I4970657294130b6b65d50ff19ffbb9ebab3be609 Gerrit-Change-Number: 40095 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 22 Apr 2025 08:43:28 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

2 months, 3 weeks

1
0
0 0

[S] Change in pysim[master]: personalization: set default values

by laforge

Attention is currently required from: neels. laforge has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/40094?usp=email ) Change subject: personalization: set default values ...................................................................... Patch Set 2: Code-Review-1 (7 comments) Commit Message: https://gerrit.osmocom.org/c/pysim/+/40094/comment/02406a05_f38afaab?usp=em… : PS2, Line 11: This is useful for user interaction, to prefill an input field that : indicates a valid input to modify to taste. I beg to differ. Filling default values is dangerous as it makes the form validate without the user having provided reasonable input to all of them. In other words, having default values is likely to make it easy for users to personalize profiles with something they don't want? It might depend on the actual parameter, but for sure something like IMSI or K/OPc should never have a default, as there is no reasonable default that is true for most use cases. Those are always individual... File pySim/esim/saip/personalization.py: https://gerrit.osmocom.org/c/pysim/+/40094/comment/dc3446be_48aa8fe5?usp=em… : PS2, Line 278: default_value there's no point of an all-zero default ICCID. https://gerrit.osmocom.org/c/pysim/+/40094/comment/36215e1a_5cff6661?usp=em… : PS2, Line 299: I also think there's no point in having a default for the IMSI https://gerrit.osmocom.org/c/pysim/+/40094/comment/decfdbb0_d7ea037e?usp=em… : PS2, Line 467: default_value = '0' * allow_len I'm not sure why any security key / pin should have any default value at all. This seems more like its introducing a security issue as anyone not explicitly setting a PUK would get 00000000 instead of a warning/error/exception? https://gerrit.osmocom.org/c/pysim/+/40094/comment/d1fe8be5_64b46f16?usp=em… : PS2, Line 494: default_value = '0' * max_len same here https://gerrit.osmocom.org/c/pysim/+/40094/comment/1382f00c_548564dc?usp=em… : PS2, Line 565: default_value = 1 # Milenage here I agree that a default actually does make sense. https://gerrit.osmocom.org/c/pysim/+/40094/comment/16919990_827bb8c0?usp=em… : PS2, Line 582: default_value = '00' * allow_len no cryptographic key should have any default value. It's just creating security nightmares. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/40094?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I2672fedcbc32cb7a6cb0c233a4a22112bd9aae03 Gerrit-Change-Number: 40094 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 22 Apr 2025 08:41:26 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes

2 months, 3 weeks

1
0
0 0

[S] Change in pysim[master]: personalization: set some typical parameter names

by laforge

Attention is currently required from: neels. laforge has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/40093?usp=email ) Change subject: personalization: set some typical parameter names ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/40093?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I31f390d634e58c384589c50a33ca45d6f86d4e10 Gerrit-Change-Number: 40093 Gerrit-PatchSet: 2 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Comment-Date: Tue, 22 Apr 2025 08:22:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

2 months, 3 weeks

1
0
0 0

[M] Change in osmocom-bb[master]: trx_toolkit/udp_link: Optimize UDPLink.send

by kirr

Attention is currently required from: laforge. kirr has posted comments on this change by kirr. ( https://gerrit.osmocom.org/c/osmocom-bb/+/40055?usp=email ) Change subject: trx_toolkit/udp_link: Optimize UDPLink.send ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: > just a random comment: As we do a lot of io_uring in libosmocore and programs using it in recent tim […] Harald, thanks for feedback. Current epoll + send/recv seems to be enough for BTS + 1-2-3 Ms, but if we are to go with more Ms it might indeed be better to optimize more. In my experience most of the overhead was in python-related wrapping, but once that is removed it indeed the IO syscalls that will become the bottleneck. We do not forward a lot of data, as GSM bursts are small, but we do forward a lot of packets, as each GSM burst is being sent via 1 UDP message and to keep the forarding latency minimal we cannot coalesce those messages even for different slots in the same GSM frame. So using io_uring might indeed make sense at some point. When/if we are to do that I would suggest to cosider using libosmocore directly instead of going through additional third-party library. It should hopefully easy to do that from Cython. Kirill -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/40055?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: I83204545066a925dadbcd0b72cbbc2e3407129fe Gerrit-Change-Number: 40055 Gerrit-PatchSet: 2 Gerrit-Owner: kirr <kirr(a)nexedi.com> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Sun, 20 Apr 2025 13:16:32 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org>

2 months, 3 weeks

1
0
0 0

[M] Change in pysim[master]: [1/6] personalization: refactor: drop ClassVarMeta use

by fixeria

Attention is currently required from: laforge, neels. fixeria has posted comments on this change by neels. ( https://gerrit.osmocom.org/c/pysim/+/39741?usp=email ) Change subject: [1/6] personalization: refactor: drop ClassVarMeta use ...................................................................... Patch Set 9: (1 comment) Patchset: PS6: I find it funny that you're saying "normal python" / "plain standard python", as if this was some kind of dialect or a language on top of Python. But the metaclasses are part of the language. PEP 3115 (https://peps.python.org/pep-3115/) dates back to 07-Mar-2007. As I commented earlier: > IMO, the old approach is much more compact and thus more readable. so I would vote against this kind of refactoring, sorry. > i don't understand what purpose a meta class serves. In this particular case it allows assigning class attributes in a compact way. > * fixeria's example shows that the shortest way to achieve class attributes is You say it's shorter, but looking at this patch I see the modified classes taking nearly twice more lines than the old code. My example simply shows a metaclass `MetaFoo` implementation, that allows to assign class attributes in both ways. But you don't seem to like the idea of mixing up different styles. > * upcoming patches add a lot of class attributes, and i would like to consistently use one way. I chose the "normal python" way, so as a first step, this patch drops the meta class. Consistency does sound like a solid argument, ok. And indeed, the approach of using metaclasses becomes less readable when you have a lot of attributes. But this can be avoided by creating intermediate classes (like you already do). And by having less attributes, e.g. `len = (min, max)` instead of `min_len` + `max_len`. Not willing to block you, just sharing my view. Leaving the final decision up to @laforge@osmocom.org. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39741?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I60ea8fd11fb438ec90ddb08b17b658cbb789c051 Gerrit-Change-Number: 39741 Gerrit-PatchSet: 9 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Sun, 20 Apr 2025 08:06:01 +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: fixeria <vyanitskiy(a)sysmocom.de>

2 months, 3 weeks

1
0
0 0

[M] Change in pysim[master]: [2/6] personalization: refactor ConfigurableParameter, Iccid, Imsi

by neels

Attention is currently required from: dexter, fixeria, laforge. Hello Jenkins Builder, dexter, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/pysim/+/39742?usp=email to look at the new patch set (#9). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: [2/6] personalization: refactor ConfigurableParameter, Iccid, Imsi ...................................................................... [2/6] personalization: refactor ConfigurableParameter, Iccid, Imsi Main points/rationales of the refactoring, details below: 1) common validation implementation 2) offer classmethods The new features are optional, and will be heavily used by batch personalization patches coming soon. Implement Iccid and Imsi to use the new way, with a common abstract DecimalParam implementation. So far leave the other parameter classes working as they always did, to follow suit in subsequent commits. Details: 1) common validation implementation: There are very common validation steps in the various parameter implementations. It is more convenient and much more readable to implement those once and set simple validation parameters per subclass. So there now is a validate_val() classmethod, which subclasses can use as-is to apply the validation parameters -- or subclasses can override their cls.validate_val() for specialized validation. (Those subclasses that this patch doesn't touch still override the self.validate() instance method. Hence they still work as before this patch, but don't use the new common features yet.) 2) offer stateless classmethods: It is useful for... - batch processing of multiple profiles (in upcoming patches) and - user input validation to be able to have classmethods that do what self.validate() and self.apply() do, but do not modify any self.* members. So far the paradigm was to create a class instance to keep state about the value. This remains available, but in addition we make available the paradigm of a singleton that is stateless (the classmethods). Using self.validate() and self.apply() still work the same as before this patch, i.e. via self.input_value and self.value -- but in addition, there are now classmethods that don't touch self.* members. Related: SYS#6768 Change-Id: I6522be4c463e34897ca9bff2309b3706a88b3ce8 --- M pySim/esim/saip/personalization.py 1 file changed, 179 insertions(+), 34 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/42/39742/9 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/39742?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I6522be4c463e34897ca9bff2309b3706a88b3ce8 Gerrit-Change-Number: 39742 Gerrit-PatchSet: 9 Gerrit-Owner: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de>

2 months, 3 weeks

1
0
0 0

2025

2024

2023

2022

gerrit-log