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

[S] Change in osmo-ttcn3-hacks[master]: library/s1ap: comment out optional IE in tr_S1AP_InitialCtxSetupResp

by fixeria

Attention is currently required from: laforge, pespin. fixeria has posted comments on this change by fixeria. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38277?usp=email ) Change subject: library/s1ap: comment out optional IE in tr_S1AP_InitialCtxSetupResp ...................................................................... Patch Set 1: (2 comments) File library/s1ap/S1AP_Templates.ttcn: https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38277/comment/cfb40d6a_e52d… : PS1, Line 734: /*, { > what's wrong with sending this? You are not explaining why. Why was it already commented out here? I think because it's an **optional** IE that requires at least one E-RAB item (i.e. cannot be empty). There needs to be a logic (likely a function) that would be adding this IE conditionally based on the template parameter `rab_failed_items`. For instance, we want to exclude this `protocolIEs` item completely if `istemplatekind(rab_failed_items, "omit")`. > Furthermore, I'd expect we want to test this case in ttcn3. Yes, I would like to add such testcase (if the time permits). But for a normal scenario when eNB reports all E-RABs as set up, this IE shall not be present in the list. This is why I am commenting it out below in `tr_S1AP_InitialCtxSetupResp`. For that special scenario with not all E-RABs being set up we can: * go for adding the additional `f_gen_protocolIEs()` logic, * easy to implement for `?` or `omit` * gets really complex for `*` * add another pair of templates extending `{ts,tr}_S1AP_InitialCtxSetupResp` * not if that would work for a list of `protocolIEs` though * add optional IEs conditionally in the testcase itself https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38277/comment/cdd7908c_be3f… : PS1, Line 755: protocolIEs := { > then please use superset() here. `superset()` removes the need to put the `*` at the end (which was there before my patches). It does not solve the problem of having optional IEs in a template though. I am fine with migrating to `superset()` in a separate patch. -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38277?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: I4765036be69ff10adb8c510d4092834c4e923229 Gerrit-Change-Number: 38277 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 26 Sep 2024 11:55:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>

9 months

1
0
0 0

[S] Change in pysim[master]: cosmetic: use **kwargs instead of **_kwargs

by dexter

dexter has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/38300?usp=email ) Change subject: cosmetic: use **kwargs instead of **_kwargs ...................................................................... Patch Set 2: (1 comment) Patchset: PS2: I think this is more a question of coding convention. In all those files **kwargs is not used. I guess the underscore is inherited from erlang where unused parameters must be prefixed with an underscore, otherwise the compiler would reject the code. In python there is no such limitation and a quick search for **_kwargs also didn't reveal any info if this is a common convention or not. I would suggest to remove the underscore prefix. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/38300?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: I98857cc774185e55a604eb4fbfbf62ed4bd6ded7 Gerrit-Change-Number: 38300 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-CC: Jenkins Builder Gerrit-Comment-Date: Thu, 26 Sep 2024 09:43:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

9 months

1
0
0 0

[S] Change in pysim[master]: cosmetic: use **kwargs instead of **_kwargs

by dexter

dexter has uploaded a new patch set (#2). ( https://gerrit.osmocom.org/c/pysim/+/38300?usp=email ) Change subject: cosmetic: use **kwargs instead of **_kwargs ...................................................................... cosmetic: use **kwargs instead of **_kwargs Some methods sometimes have a **_kwargs parameter, let's be consistent and use **kwargs only. Related: OS#5714 Change-Id: I98857cc774185e55a604eb4fbfbf62ed4bd6ded7 --- M pySim/ts_102_221.py M pySim/ts_31_102.py 2 files changed, 7 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/00/38300/2 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/38300?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: I98857cc774185e55a604eb4fbfbf62ed4bd6ded7 Gerrit-Change-Number: 38300 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-CC: Jenkins Builder

9 months

1
0
0 0

[M] Change in pysim[master]: filesystem: pass total_len to construct of when encoding file contents

by dexter

Attention is currently required from: laforge. Hello Jenkins Builder, laforge, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/pysim/+/38195?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: filesystem: pass total_len to construct of when encoding file contents ...................................................................... filesystem: pass total_len to construct of when encoding file contents In our construct models we frequently use a context parameter "total_len", we also pass this parameter to construct when we decode files, but we do not pass it when we generate files. This is a problem, because when total_len is used in the construct model, this parameter must be known also when decoding the file. Let's make sure that the total_len is properly determined and and passed to construct (via pyosmocom) Related: OS#5714 Change-Id: I1b7a51594fbc5d9fe01132c39354a2fa88d53f9b --- M pySim/filesystem.py M pySim/gsm_r.py M pySim/runtime.py M pySim/sysmocom_sja2.py M pySim/ts_31_102.py M pySim/ts_31_102_telecom.py M pySim/ts_51_011.py M tests/pySim-shell_test/file_content/test_record_uicc.ok 8 files changed, 100 insertions(+), 41 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/95/38195/6 -- To view, visit https://gerrit.osmocom.org/c/pysim/+/38195?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: I1b7a51594fbc5d9fe01132c39354a2fa88d53f9b Gerrit-Change-Number: 38195 Gerrit-PatchSet: 6 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org>

9 months

1
0
0 0

[M] Change in pysim[master]: filesystem: pass total_len to construct of when encoding file contents

by dexter

Attention is currently required from: laforge. dexter has posted comments on this change by dexter. ( https://gerrit.osmocom.org/c/pysim/+/38195?usp=email ) Change subject: filesystem: pass total_len to construct of when encoding file contents ...................................................................... Patch Set 6: (4 comments) File pySim/filesystem.py: https://gerrit.osmocom.org/c/pysim/+/38195/comment/50437558_f503ed26?usp=em… : PS2, Line 1338: def _decode_bin(self, raw_bin_data: bytearray): : chunks = [raw_bin_data[i:i+self.rec_len] : for i in range(0, len(raw_bin_data), self.rec_len)] : return [self.decode_record_bin(x) for x in chunks] : : def _encode_bin(self, abstract_data) -> bytes: : chunks = [self.encode_record_bin(x) for x in abstract_data] : # FIXME: pad to file size : return b''.join(chunks) > There's nothing wrong with anticipating future extensions and passing through `**kwargs`. […] Acknowledged File pySim/filesystem.py: https://gerrit.osmocom.org/c/pysim/+/38195/comment/9c11083f_e8b068e2?usp=em… : PS5, Line 746: __get_size > so this function now actually doesn't return the size, but it returns a dict with a single element […] Yes, that's really odd. I thought it would simplify things a bit when we were using it only with build_construct(), but now we use it in other places too. I have changed it now. https://gerrit.osmocom.org/c/pysim/+/38195/comment/5cec70e2_a0facbdf?usp=em… : PS5, Line 1343: def _encode_bin(self, abstract_data, total_len: int = None, **kwargs) -> bytes: > this also doesn't really match the code above. […] Done File pySim/gsm_r.py: https://gerrit.osmocom.org/c/pysim/+/38195/comment/92766fc6_cf8b1b28?usp=em… : PS5, Line 293: def _encode_record_bin(self, abstract_data : dict, record_nr : int, **kwargs) -> bytearray: > _encode_record_bin now just has **kwargs, while the _encode_bin has a dedicated total_len argument. […] I think we should use **kwargs to transfer total_len. In most cases total_len is not needed at all, so I think **kwargs is the better option here. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/38195?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: I1b7a51594fbc5d9fe01132c39354a2fa88d53f9b Gerrit-Change-Number: 38195 Gerrit-PatchSet: 6 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 26 Sep 2024 09:37:13 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>

9 months

1
0
0 0

[S] Change in pysim[master]: cosmetic: use **kwargs instead of **_kwargs

by dexter

dexter has uploaded this change for review. ( https://gerrit.osmocom.org/c/pysim/+/38300?usp=email ) Change subject: cosmetic: use **kwargs instead of **_kwargs ...................................................................... cosmetic: use **kwargs instead of **_kwargs Some methods sometimes have a **_kwargs parameter, let's be consistent and use **kwargs only. Related OS#5714 Change-Id: I8b193d36a5be2f29a830853e193157e66d64d8d5 Change-Id: I98857cc774185e55a604eb4fbfbf62ed4bd6ded7 --- M pySim/ts_102_221.py M pySim/ts_31_102.py 2 files changed, 7 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/pysim refs/changes/00/38300/1 diff --git a/pySim/ts_102_221.py b/pySim/ts_102_221.py index f09b1e3..fa1bd39 100644 --- a/pySim/ts_102_221.py +++ b/pySim/ts_102_221.py @@ -733,7 +733,7 @@ raise ValueError return by_mode - def _decode_record_bin(self, raw_bin_data, **_kwargs): + def _decode_record_bin(self, raw_bin_data, **kwargs): # we can only guess if we should decode for EF or DF here :( arr_seq = DataObjectSequence('arr', sequence=[AM_DO_EF, SC_DO]) dec = arr_seq.decode_multi(raw_bin_data) @@ -741,7 +741,7 @@ # 'un-flattening' decoder, and hence would be unable to encode :( return dec[0] - def _encode_record_bin(self, in_json, **_kwargs): + def _encode_record_bin(self, in_json, **kwargs): # we can only guess if we should decode for EF or DF here :( arr_seq = DataObjectSequence('arr', sequence=[AM_DO_EF, SC_DO]) return arr_seq.encode_multi(in_json) diff --git a/pySim/ts_31_102.py b/pySim/ts_31_102.py index ab5ee54..b82c411 100644 --- a/pySim/ts_31_102.py +++ b/pySim/ts_31_102.py @@ -405,14 +405,14 @@ desc='Language Indication'): super().__init__(fid, sfid=sfid, name=name, desc=desc, size=size, rec_len=rec_len) - def _decode_record_bin(self, in_bin, **_kwargs): + def _decode_record_bin(self, in_bin, **kwargs): if in_bin == b'\xff\xff': return None else: # officially this is 7-bit GSM alphabet with one padding bit in each byte return in_bin.decode('ascii') - def _encode_record_bin(self, in_json, **_kwargs): + def _encode_record_bin(self, in_json, **kwargs): if in_json is None: return b'\xff\xff' else: @@ -505,7 +505,7 @@ desc='Emergency Call Codes'): super().__init__(fid, sfid=sfid, name=name, desc=desc, rec_len=(4, 20)) - def _decode_record_bin(self, in_bin, **_kwargs): + def _decode_record_bin(self, in_bin, **kwargs): # mandatory parts code = in_bin[:3] if code == b'\xff\xff\xff': @@ -519,7 +519,7 @@ ret['alpha_id'] = parse_construct(EF_ECC.alpha_construct, alpha_id) return ret - def _encode_record_bin(self, in_json, **_kwargs): + def _encode_record_bin(self, in_json, **kwargs): if in_json is None: return b'\xff\xff\xff\xff' code = EF_ECC.cc_construct.build(in_json['call_code']) @@ -682,7 +682,7 @@ def __init__(self, fid='6f65', sfid=None, name='EF.RPLMNAcTD', size=(2, 4), rec_len=2, desc='RPLMN Last used Access Technology', **kwargs): super().__init__(fid, sfid=sfid, name=name, desc=desc, size=size, rec_len=rec_len, **kwargs) - def _decode_record_hex(self, in_hex, **_kwargs): + def _decode_record_hex(self, in_hex, **kwargs): return dec_act(in_hex) # TODO: Encode -- To view, visit https://gerrit.osmocom.org/c/pysim/+/38300?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I98857cc774185e55a604eb4fbfbf62ed4bd6ded7 Gerrit-Change-Number: 38300 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de>

9 months

1
0
0 0

[XS] Change in osmo-ttcn3-hacks[master]: testenv: podman: disable send_redirects

by osmith

osmith has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38299?usp=email ) Change subject: testenv: podman: disable send_redirects ...................................................................... testenv: podman: disable send_redirects When starting podman, set the following sysctls to avoid ICMP redirects. ICMP redirects lead to test failures (TC_pdp4_clients_interact in the GGSN testsuite), and should not be sent in the test environment in general. net.ipv4.conf.all.send_redirects=0 net.ipv4.conf.default.send_redirects=0 It is really needed to set both "all" and "default", or otherwise ICMP redirects still show up. I've seen setting both in this patch: https://patchwork.kernel.org/project/linux-kselftest/patch/1570719055-25110… Fixes: OS#6575 Change-Id: Ie27668f38b80c52ffef4e17b3fe64f0c9109bdea --- M _testenv/testenv/podman.py 1 file changed, 4 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/99/38299/1 diff --git a/_testenv/testenv/podman.py b/_testenv/testenv/podman.py index 0ecf12d..60aecca 100644 --- a/_testenv/testenv/podman.py +++ b/_testenv/testenv/podman.py @@ -209,6 +209,10 @@ f"{apt_dir_var_cache}:/var/cache/apt", "--volume", f"{apt_dir_var_lib}:/var/lib/apt", + "--sysctl", + "net.ipv4.conf.all.send_redirects=0", # OS#6575 + "--sysctl", + "net.ipv4.conf.default.send_redirects=0", # OS#6575 ] if not testenv.args.binary_repo: -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38299?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: Ie27668f38b80c52ffef4e17b3fe64f0c9109bdea Gerrit-Change-Number: 38299 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de>

9 months

1
0
0 0

[XS] Change in ...pyosmocom[master]: tag 0.0.5 for osmocom.construct.Asn1DerInteger

by osmith

Attention is currently required from: dexter, fixeria, laforge. osmith has posted comments on this change by laforge. ( https://gerrit.osmocom.org/c/python/pyosmocom/+/38273?usp=email ) Change subject: tag 0.0.5 for osmocom.construct.Asn1DerInteger ...................................................................... Patch Set 8: (1 comment) File debian/changelog: https://gerrit.osmocom.org/c/python/pyosmocom/+/38273/comment/e476eb6e_97ae… : PS7, Line 3: * osmocom.construct.Asn1DerInteger > Not sure if you understood my comment. […] "gbp dch" can be used for that -- To view, visit https://gerrit.osmocom.org/c/python/pyosmocom/+/38273?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: python/pyosmocom Gerrit-Branch: master Gerrit-Change-Id: I77914901d7f154d3a6e97c39575292e03453ddf7 Gerrit-Change-Number: 38273 Gerrit-PatchSet: 8 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 26 Sep 2024 08:13:31 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>

9 months

1
0
0 0

[XS] Change in ...pyosmocom[master]: tag 0.0.5 for osmocom.construct.Asn1DerInteger

by fixeria

Attention is currently required from: dexter, laforge, osmith. fixeria has posted comments on this change by laforge. ( https://gerrit.osmocom.org/c/python/pyosmocom/+/38273?usp=email ) Change subject: tag 0.0.5 for osmocom.construct.Asn1DerInteger ...................................................................... Patch Set 8: Code-Review-1 (1 comment) File debian/changelog: https://gerrit.osmocom.org/c/python/pyosmocom/+/38273/comment/f105598a_ba83… : PS7, Line 3: * osmocom.construct.Asn1DerInteger > Done Not sure if you understood my comment. This patchset definitely brings more patches than the two you listed here (plus potentially already merged patches). A patch release should include them all if you're tagging the current `HEAD` and not doing a patch-release in a separate branch. There's a tool to generate the changelog file automatically, @osmith@sysmocom.de may help here. -- To view, visit https://gerrit.osmocom.org/c/python/pyosmocom/+/38273?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: python/pyosmocom Gerrit-Branch: master Gerrit-Change-Id: I77914901d7f154d3a6e97c39575292e03453ddf7 Gerrit-Change-Number: 38273 Gerrit-PatchSet: 8 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Thu, 26 Sep 2024 08:07:14 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de>

9 months

1
0
0 0

[L] Change in ...osmo-s1gw[master]: Introduce initial metrics support

by fixeria

Attention is currently required from: pespin. fixeria has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37955?usp=email ) Change subject: Introduce initial metrics support ...................................................................... Patch Set 5: (1 comment) This change is ready for review. Commit Message: https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37955/comment/3d7bd787_34ea… : PS4, Line 2: Vadim Yanitskiy <vyanitskiy(a)sysmocom.de> > Not sure why/how git changed the Author, I'll revert this. Done -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37955?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: I952e198238384dca4be94f91a01d7cfff0a1471f Gerrit-Change-Number: 37955 Gerrit-PatchSet: 5 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 26 Sep 2024 08:01:49 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>

9 months

1
0
0 0

2025

2024

2023

2022

gerrit-log September 2024