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

[S] Change in osmo-ttcn3-hacks[master]: testenv: improve --config argument parsing

by osmith

osmith has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38285?usp=email ) Change subject: testenv: improve --config argument parsing ...................................................................... testenv: improve --config argument parsing * Support using wildcards for the config names via fnmatch as that makes it much easier to run the ggsn tests against all osmo-ggsn config variations, and update the examples in "testenv.py -h" to illustrate this. * Fix that it didn't complain about an invalid --config argument, as long as there was a valid --config argument before it. * Let raise_error_config_arg only output the invalid --config argument instead of all of them. * Complain if "--config all" is used in combination with another --config argument. * Sort testenv*.cfg files found alphabetically, so they are always executed in the same order. Change-Id: I66b976b0332be523c084a6b5d38d0f62134b495d --- M _testenv/testenv/__init__.py M _testenv/testenv/testdir.py M _testenv/testenv/testenv_cfg.py 3 files changed, 31 insertions(+), 13 deletions(-) Approvals: laforge: Looks good to me, but someone else must approve pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified fixeria: Looks good to me, approved diff --git a/_testenv/testenv/__init__.py b/_testenv/testenv/__init__.py index 87485c7..2af83ed 100644 --- a/_testenv/testenv/__init__.py +++ b/_testenv/testenv/__init__.py @@ -43,7 +43,9 @@ " ./testenv.py run mgw --test TC_crcx\n" " ./testenv.py run mgw --podman --binary-repo osmocom:latest\n" " ./testenv.py run mgw --io-uring\n" - " ./testenv.py run bts --config oml\n", + " ./testenv.py run ggsn --config open5gs\n" + " ./testenv.py run ggsn --config 'osmo_ggsn_v4_only'\n" + " ./testenv.py run ggsn --config 'osmo_ggsn*'\n", ) sub = parser.add_subparsers(title="action", dest="action", required=True) @@ -77,7 +79,7 @@ "-c", "--config", action="append", - help="which testenv.cfg to use, in case the testsuite has multiple (e.g. generic|oml|hopping for bts)", + help="which testenv.cfg to use (supports * wildcards via fnmatch)", ) group.add_argument("-i", "--io-uring", action="store_true", help="set LIBOSMO_IO_BACKEND=IO_URING") diff --git a/_testenv/testenv/testdir.py b/_testenv/testenv/testdir.py index ee698fff..aa0ba47 100644 --- a/_testenv/testenv/testdir.py +++ b/_testenv/testenv/testdir.py @@ -28,7 +28,7 @@ prefix = f"testenv-{testenv.args.testsuite}-" if testenv.args.config: - prefix += f"{'-'.join(testenv.args.config)}-" + prefix += f"{'-'.join(testenv.args.config).replace('*','')}-" if testenv.args.binary_repo: prefix += f"{testenv.args.binary_repo.replace(':','-')}-" prefix += datetime.datetime.now().strftime("%Y%m%d-%H%M") diff --git a/_testenv/testenv/testenv_cfg.py b/_testenv/testenv/testenv_cfg.py index 9642ae8..ba830a1 100644 --- a/_testenv/testenv/testenv_cfg.py +++ b/_testenv/testenv/testenv_cfg.py @@ -1,6 +1,7 @@ # Copyright 2024 sysmocom - s.f.m.c. GmbH # SPDX-License-Identifier: GPL-3.0-or-later import configparser +import fnmatch import glob import logging import os.path @@ -134,14 +135,14 @@ get_vty_host_port(cfg, path) -def raise_error_config_arg(glob_result): +def raise_error_config_arg(glob_result, config_arg): valid = [] for path in glob_result: basename = os.path.basename(path) if basename != "testenv.cfg": valid += [basename.split("_", 1)[1].split(".", -1)[0]] - msg = f"Invalid parameter for --config: {testenv.args.config}" + msg = f"Invalid parameter for --config: {config_arg}" if valid: msg += f" (valid: all, {', '.join(valid)})" @@ -154,7 +155,7 @@ def find_configs(): dir_testsuite = os.path.join(testenv.testsuite.ttcn3_hacks_dir_src, testenv.args.testsuite) pattern = os.path.join(dir_testsuite, "testenv*.cfg") - ret = glob.glob(pattern) + ret = sorted(glob.glob(pattern)) if not ret: logging.error(f"Missing testenv.cfg in: {dir_testsuite}") @@ -185,6 +186,8 @@ def init(): global cfgs + cfgs_all = {} + config_paths = find_configs() for path in config_paths: @@ -201,14 +204,27 @@ handle_latest(cfg, path) verify(cfg, path) + # No --config argument given, and there is only one testenv.cfg if not testenv.args.config: cfgs[basename] = cfg - continue + return - for config_arg in testenv.args.config: - if config_arg == "all" or f"testenv_{config_arg}.cfg" == basename: - cfgs[basename] = cfg - break + cfgs_all[basename] = cfg - if not cfgs: - raise_error_config_arg(config_paths) + # Select configs based on --config argument(s) + for config_arg in testenv.args.config: + if config_arg == "all": + if len(testenv.args.config) != 1: + raise testenv.NoTraceException("Can't use multiple --config arguments if one of them is 'all'") + cfgs = cfgs_all + return + + matched = False + for basename in cfgs_all: + pattern = f"testenv_{config_arg}.cfg" + if fnmatch.fnmatch(basename, pattern): + matched = True + cfgs[basename] = cfgs_all[basename] + + if not matched: + raise_error_config_arg(config_paths, config_arg) -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38285?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I66b976b0332be523c084a6b5d38d0f62134b495d Gerrit-Change-Number: 38285 Gerrit-PatchSet: 4 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

9 months

1
0
0 0

[XS] Change in osmo-ttcn3-hacks[master]: ruff.toml: new file

by osmith

osmith has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38283?usp=email ) Change subject: ruff.toml: new file ...................................................................... ruff.toml: new file Add a file in the root dir of the repository to allow running "ruff format" in order to auto-format the code with expected max line length, PEP-8, etc. Replace _testenv/pyproject.toml with .ruff.toml in the root directory of the repository, so we can exclude "compare-results.py" which doesn't follow that code style. Otherwise it would get formatted too when running "ruff format" in the root dir of the repository. Change-Id: I97ed1ececba85008ca754cf91bf46522e168a894 --- A .ruff.toml D _testenv/pyproject.toml 2 files changed, 4 insertions(+), 2 deletions(-) Approvals: Jenkins Builder: Verified pespin: Looks good to me, but someone else must approve fixeria: Looks good to me, approved laforge: Looks good to me, but someone else must approve diff --git a/.ruff.toml b/.ruff.toml new file mode 100644 index 0000000..ad8c0b5 --- /dev/null +++ b/.ruff.toml @@ -0,0 +1,4 @@ +line-length = 120 +exclude = [ + "compare-results.py", +] diff --git a/_testenv/pyproject.toml b/_testenv/pyproject.toml deleted file mode 100644 index 3719b74..0000000 --- a/_testenv/pyproject.toml +++ /dev/null @@ -1,2 +0,0 @@ -[tool.ruff] -line-length=120 -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/38283?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I97ed1ececba85008ca754cf91bf46522e168a894 Gerrit-Change-Number: 38283 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

9 months

1
0
0 0

[S] Change in ...pyosmocom[master]: test_construct: move testvectors inside unittest class

by dexter

dexter has submitted this change. ( https://gerrit.osmocom.org/c/python/pyosmocom/+/38287?usp=email ) Change subject: test_construct: move testvectors inside unittest class ...................................................................... test_construct: move testvectors inside unittest class The unittest class TestGreedyInt has test vectors declared outside that class, but no other class makes use of this test vectors, so let's move them into TestGreedyInt, where they should be. Related: SYS#7094 Change-Id: Ida24295091efd30406e9a354ca5854150127b3d4 --- M tests/test_construct.py 1 file changed, 5 insertions(+), 5 deletions(-) Approvals: laforge: Looks good to me, approved fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/tests/test_construct.py b/tests/test_construct.py index 9110144..c2aeece 100755 --- a/tests/test_construct.py +++ b/tests/test_construct.py @@ -6,21 +6,21 @@ # pylint: disable=no-name-in-module from construct import FlagsEnum -tests = [ +class TestGreedyInt(unittest.TestCase): + tests = [ ( b'\x80', 0x80 ), ( b'\x80\x01', 0x8001 ), ( b'\x80\x00\x01', 0x800001 ), ( b'\x80\x23\x42\x01', 0x80234201 ), - ] + ] -class TestGreedyInt(unittest.TestCase): def test_GreedyInt_decoder(self): gi = GreedyInteger() - for t in tests: + for t in self.tests: self.assertEqual(gi.parse(t[0]), t[1]) def test_GreedyInt_encoder(self): gi = GreedyInteger() - for t in tests: + for t in self.tests: self.assertEqual(t[0], gi.build(t[1])) pass -- To view, visit https://gerrit.osmocom.org/c/python/pyosmocom/+/38287?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: merged Gerrit-Project: python/pyosmocom Gerrit-Branch: master Gerrit-Change-Id: Ida24295091efd30406e9a354ca5854150127b3d4 Gerrit-Change-Number: 38287 Gerrit-PatchSet: 1 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> 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>

9 months

1
0
0 0

[XS] Change in ...osmo-s1gw[master]: s1ap_proxy: clarify a logging statement, reduce verbosity

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38294?usp=email ) Change subject: s1ap_proxy: clarify a logging statement, reduce verbosity ...................................................................... s1ap_proxy: clarify a logging statement, reduce verbosity Logging all S1AP PDUs at INFO is way to verbose, use DEBUG. Clarify that this is the Rx (receive) direction. Change-Id: I325758c7fb632dc67d0d9cf1f2e58ca337e7a744 --- M src/s1ap_proxy.erl 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/94/38294/1 diff --git a/src/s1ap_proxy.erl b/src/s1ap_proxy.erl index 1df1518..c985601 100644 --- a/src/s1ap_proxy.erl +++ b/src/s1ap_proxy.erl @@ -82,7 +82,7 @@ handle_pdu(Data, S) when is_binary(Data) -> case decode_pdu(Data) of {ok, Pdu} -> - ?LOG_INFO("S1AP PDU: ~p", [Pdu]), + ?LOG_DEBUG("Rx S1AP PDU: ~p", [Pdu]), handle_pdu(Data, Pdu, S); {error, {asn1, Error}} -> ?LOG_ERROR("S1AP PDU decoding failed: ~p", [Error]), -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38294?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: I325758c7fb632dc67d0d9cf1f2e58ca337e7a744 Gerrit-Change-Number: 38294 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

9 months

1
0
0 0

[S] Change in ...osmo-s1gw[master]: s1ap_proxy: add types for S1AP PDU and IE tuples

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38295?usp=email ) Change subject: s1ap_proxy: add types for S1AP PDU and IE tuples ...................................................................... s1ap_proxy: add types for S1AP PDU and IE tuples Change-Id: I39e0a12aade5e31c414cb639dbf333af2a5b7be4 --- M src/s1ap_proxy.erl 1 file changed, 13 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/95/38295/1 diff --git a/src/s1ap_proxy.erl b/src/s1ap_proxy.erl index c985601..c386f29 100644 --- a/src/s1ap_proxy.erl +++ b/src/s1ap_proxy.erl @@ -50,6 +50,12 @@ -include("S1AP-IEs.hrl"). +-type s1ap_pdu() :: {initiatingMessage, #'InitiatingMessage'{}} | + {successfulOutcome, #'SuccessfulOutcome'{}} | + {unsuccessfulOutcome, #'UnsuccessfulOutcome'{}}. +-type s1ap_ie() :: #'ProtocolIE-Field'{}. +-type s1ap_ie_val() :: tuple(). + -type mme_ue_id() :: 0..16#ffffffff. -type enb_ue_id() :: 0..16#ffffff. -type erab_id() :: 0..16#ff. @@ -102,14 +108,14 @@ %% ------------------------------------------------------------------ %% Encode an S1AP PDU --spec encode_pdu(tuple()) -> {ok, binary()} | - {error, {asn1, tuple()}}. +-spec encode_pdu(s1ap_pdu()) -> {ok, binary()} | + {error, {asn1, tuple()}}. encode_pdu(Pdu) -> 'S1AP-PDU-Descriptions':encode('S1AP-PDU', Pdu). %% Decode an S1AP PDU --spec decode_pdu(binary()) -> {ok, tuple()} | +-spec decode_pdu(binary()) -> {ok, s1ap_pdu()} | {error, {asn1, tuple()}}. decode_pdu(Data) -> 'S1AP-PDU-Descriptions':decode('S1AP-PDU', Data). @@ -118,6 +124,7 @@ %% Helper function for handle_pdu/3. %% Attempt to encode a new (modified) S1AP PDU, %% return a new binary() on success or Data on error. +-spec handle_pdu_new(binary(), s1ap_pdu(), proxy_state()) -> {binary(), proxy_state()}. handle_pdu_new(Data, NewPdu, S) -> case encode_pdu(NewPdu) of {ok, NewData} -> @@ -128,6 +135,8 @@ end. +-spec handle_pdu(binary(), s1ap_pdu(), proxy_state()) -> {binary(), proxy_state()}. + %% 9.1.3.1 E-RAB SETUP REQUEST handle_pdu(Data, {Outcome = initiatingMessage, #'InitiatingMessage'{procedureCode = ?'id-E-RABSetup', @@ -216,7 +225,7 @@ %% Handle a single IE (Information Element) --spec handle_ie(tuple(), proxy_state()) -> {tuple(), proxy_state()}. +-spec handle_ie(s1ap_ie(), proxy_state()) -> {s1ap_ie_val(), proxy_state()}. %% E-RAB SETUP REQUEST related IEs handle_ie(#'ProtocolIE-Field'{id = ?'id-E-RABToBeSetupListBearerSUReq', -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38295?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: I39e0a12aade5e31c414cb639dbf333af2a5b7be4 Gerrit-Change-Number: 38295 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

9 months

1
0
0 0

[XS] Change in ...osmo-s1gw[master]: sctp_proxy: fix calling gauge_dec() on a counter

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38296?usp=email ) Change subject: sctp_proxy: fix calling gauge_dec() on a counter ...................................................................... sctp_proxy: fix calling gauge_dec() on a counter A follow-up commit will add type hints to prevent such errors. Change-Id: Ic1e29061838e9c96cc4687308c6c56db7011ae9f --- M src/sctp_proxy.erl 1 file changed, 1 insertion(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/96/38296/1 diff --git a/src/sctp_proxy.erl b/src/sctp_proxy.erl index c085703..9f9b746 100644 --- a/src/sctp_proxy.erl +++ b/src/sctp_proxy.erl @@ -258,7 +258,7 @@ sctp_send_pending([Data | Pending], S0) -> S1 = sctp_send(Data, S0), - s1gw_metrics:gauge_dec(?S1GW_CTR_S1AP_PROXY_UPLINK_PACKETS_QUEUED), + s1gw_metrics:gauge_dec(?S1GW_GAUGE_S1AP_PROXY_UPLINK_PACKETS_QUEUED), sctp_send_pending(Pending, S1); sctp_send_pending([], S) -> -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38296?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: Ic1e29061838e9c96cc4687308c6c56db7011ae9f Gerrit-Change-Number: 38296 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

9 months

1
0
0 0

[S] Change in ...osmo-s1gw[master]: s1gw_metrics: more precise type hints

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38297?usp=email ) Change subject: s1gw_metrics: more precise type hints ...................................................................... s1gw_metrics: more precise type hints This enables dialyzer to detect errors when: * ctr_* API is used on an ?S1GW_GAUGE_* definition, and * gauge_* API is used on an ?S1GW_CTR_* definition. One of such errors has been fixed in a previous commit. Change-Id: I36eaa5e04037e165e43eb960c541f967c394a2ea --- M src/s1gw_metrics.erl 1 file changed, 16 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/97/38297/1 diff --git a/src/s1gw_metrics.erl b/src/s1gw_metrics.erl index 6b078e5..ee14d08 100644 --- a/src/s1gw_metrics.erl +++ b/src/s1gw_metrics.erl @@ -82,31 +82,34 @@ ?S1GW_GAUGE_S1AP_PROXY_UPLINK_PACKETS_QUEUED ]). --spec new_ctr(list()) -> ok. +-type counter() :: [ctr | _]. +-type gauge() :: [gauge | _]. + +-spec new_ctr(counter()) -> ok. new_ctr(Name) -> %%?LOG_INFO("New counter ~p", [Name]), ok = exometer:new(Name, counter). --spec new_ctrs(list()) -> ok. +-spec new_ctrs(list(counter())) -> ok. new_ctrs([]) -> ok; new_ctrs([Name | MoreNames]) -> new_ctr(Name), new_ctrs(MoreNames). --spec new_gauge(list()) -> ok. +-spec new_gauge(gauge()) -> ok. new_gauge(Name) -> %%?LOG_INFO("New gauge ~p", [Name]), ok = exometer:new(Name, gauge). --spec new_gauges(list()) -> ok. +-spec new_gauges(list(gauge())) -> ok. new_gauges([]) -> ok; new_gauges([Name | MoreNames]) -> new_gauge(Name), new_gauges(MoreNames). --spec get_current_value(list()) -> integer(). +-spec get_current_value(counter() | gauge()) -> integer(). get_current_value(Name) -> Result = exometer:get_value(Name, value), {ok, [{value, PrevVal}]} = Result, @@ -124,43 +127,43 @@ %%%%%%%%%%%%% %% CTR APIs %%%%%%%%%%%%% --spec ctr_reset(list()) -> ok | {error, any()}. +-spec ctr_reset(counter()) -> ok | {error, any()}. ctr_reset(Name) -> ?LOG_DEBUG("ctr_reset(~p)", [Name]), exometer:reset(Name). --spec ctr_inc(list(), integer()) -> ok | {error, any()}. +-spec ctr_inc(counter(), integer()) -> ok | {error, any()}. ctr_inc(Name, Value) -> ?LOG_DEBUG("ctr_inc(~p, ~p)", [Name, Value]), exometer:update(Name, Value). --spec ctr_inc(list()) -> ok | {error, any()}. +-spec ctr_inc(counter()) -> ok | {error, any()}. ctr_inc(Name) -> ctr_inc(Name, 1). %%%%%%%%%%%%% %% GAUGE APIs %%%%%%%%%%%%% --spec gauge_reset(list()) -> ok | {error, any()}. +-spec gauge_reset(gauge()) -> ok | {error, any()}. gauge_reset(Name) -> ?LOG_DEBUG("gauge_reset(~p)", [Name]), exometer:reset(Name). --spec gauge_set(list(), integer()) -> ok | {error, any()}. +-spec gauge_set(gauge(), integer()) -> ok | {error, any()}. gauge_set(Name, Value) -> exometer:update(Name, Value). --spec gauge_inc(list(), integer()) -> ok | {error, any()}. +-spec gauge_inc(gauge(), integer()) -> ok | {error, any()}. gauge_inc(Name, Value) -> PrevVal = get_current_value(Name), ?LOG_DEBUG("gauge_inc(~p, ~p): pre_val=~p", [Name, Value, PrevVal]), exometer:update(Name, Value + PrevVal). --spec gauge_inc(list()) -> ok | {error, any()}. +-spec gauge_inc(gauge()) -> ok | {error, any()}. gauge_inc(Name) -> gauge_inc(Name, 1). --spec gauge_dec(list()) -> ok | {error, any()}. +-spec gauge_dec(gauge()) -> ok | {error, any()}. gauge_dec(Name) -> gauge_inc(Name, -1). -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38297?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: I36eaa5e04037e165e43eb960c541f967c394a2ea Gerrit-Change-Number: 38297 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

9 months

1
0
0 0

[S] Change in ...osmo-s1gw[master]: s1ap_proxy: move sctp_proxy:handle_pdu() to proc_safe()

by fixeria

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38298?usp=email ) Change subject: s1ap_proxy: move sctp_proxy:handle_pdu() to proc_safe() ...................................................................... s1ap_proxy: move sctp_proxy:handle_pdu() to proc_safe() This way we can have more precise counting of PDUs forwarded as-is due to exceptions, for which this patch also adds a counter. Change-Id: I16d4cf279a930d35ca179bd9b49234d10180e5c5 --- M include/s1gw_metrics.hrl M src/s1ap_proxy.erl M src/s1gw_metrics.erl M src/sctp_proxy.erl 4 files changed, 21 insertions(+), 15 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/98/38298/1 diff --git a/include/s1gw_metrics.hrl b/include/s1gw_metrics.hrl index 551c136..bb7043f 100644 --- a/include/s1gw_metrics.hrl +++ b/include/s1gw_metrics.hrl @@ -6,6 +6,7 @@ -define(S1GW_CTR_S1AP_ENB_ALL_RX, [ctr, s1ap, enb, all, rx]). -define(S1GW_CTR_S1AP_ENB_ALL_RX_UNKNOWN_ENB, [ctr, s1ap, enb, all, rx_unknown_enb]). -define(S1GW_CTR_S1AP_PROXY_UPLINK_PACKETS_QUEUED, [ctr, s1ap, proxy, uplink_packets_queued]). +-define(S1GW_CTR_S1AP_PROXY_EXCEPTION, [ctr, s1ap, proxy, exception]). -define(S1GW_CTR_S1AP_PROXY_IN_PKT_ALL, [ctr, s1ap, proxy, in_pkt, all]). -define(S1GW_CTR_S1AP_PROXY_IN_PKT_DECODE_ERROR, [ctr, s1ap, proxy, in_pkt, decode_error]). -define(S1GW_CTR_S1AP_PROXY_IN_PKT_PROC_ERROR, [ctr, s1ap, proxy, in_pkt, proc_error]). diff --git a/src/s1ap_proxy.erl b/src/s1ap_proxy.erl index baf3216..27bbde5 100644 --- a/src/s1ap_proxy.erl +++ b/src/s1ap_proxy.erl @@ -37,6 +37,7 @@ -export([init/0, deinit/1, proc/2, + proc_safe/2, handle_exit/2, encode_pdu/1, decode_pdu/1]). @@ -90,7 +91,8 @@ %% Process an S1AP PDU --spec proc(binary(), proxy_state()) -> {{proxy_action(), binary()}, proxy_state()}. +-type proc_result() :: {{proxy_action(), binary()}, proxy_state()}. +-spec proc(binary(), proxy_state()) -> proc_result(). proc(OrigData, S0) -> s1gw_metrics:ctr_inc(?S1GW_CTR_S1AP_PROXY_IN_PKT_ALL), case decode_pdu(OrigData) of @@ -122,6 +124,20 @@ end. +%% A safe wrapper for proc/2 +-spec proc_safe(binary(), proxy_state()) -> proc_result(). +proc_safe(OrigData, S) -> + try proc(OrigData, S) of + Result -> Result + catch + Exception:Reason:StackTrace -> + ?LOG_ERROR("An exception occurred: ~p, ~p, ~p", [Exception, Reason, StackTrace]), + s1gw_metrics:ctr_inc(?S1GW_CTR_S1AP_PROXY_EXCEPTION), + s1gw_metrics:ctr_inc(?S1GW_CTR_S1AP_PROXY_OUT_PKT_FWD_UNMODIFIED), + {{forward, OrigData}, S} %% XXX: proxy as-is or drop? + end. + + %% Handle an exit event -spec handle_exit(pid(), proxy_state()) -> proxy_state(). handle_exit(Pid, #proxy_state{erabs = ERABs} = S) -> diff --git a/src/s1gw_metrics.erl b/src/s1gw_metrics.erl index ee14d08..e065a4a 100644 --- a/src/s1gw_metrics.erl +++ b/src/s1gw_metrics.erl @@ -56,6 +56,7 @@ ?S1GW_CTR_S1AP_ENB_ALL_RX, ?S1GW_CTR_S1AP_ENB_ALL_RX_UNKNOWN_ENB, ?S1GW_CTR_S1AP_PROXY_UPLINK_PACKETS_QUEUED, + ?S1GW_CTR_S1AP_PROXY_EXCEPTION, %% exception(s) occurred %% s1ap_proxy: INcoming PDU counters ?S1GW_CTR_S1AP_PROXY_IN_PKT_ALL, %% received total ?S1GW_CTR_S1AP_PROXY_IN_PKT_DECODE_ERROR, %% failed to decode diff --git a/src/sctp_proxy.erl b/src/sctp_proxy.erl index 9f9b746..f771788 100644 --- a/src/sctp_proxy.erl +++ b/src/sctp_proxy.erl @@ -171,7 +171,7 @@ priv := Priv} = S) -> ?LOG_DEBUG("MME connection (id=~p, ~p:~p) -> eNB: ~p", [Aid, MmeAddr, MmePort, Data]), - {Action, NewPriv} = handle_pdu(Data, Priv), + {Action, NewPriv} = s1ap_proxy:proc_safe(Data, Priv), case Action of {forward, FwdData} -> sctp_server:send_data(EnbAid, FwdData); @@ -224,25 +224,13 @@ %% private API %% ------------------------------------------------------------------ -%% A safe wrapper for s1ap_proxy:proc/2 --spec handle_pdu(binary(), term()) -> {{s1ap_proxy:proxy_action(), binary()}, term()}. -handle_pdu(OrigData, Priv) when is_binary(OrigData) -> - try s1ap_proxy:proc(OrigData, Priv) of - Result -> Result %% {Action, NewPriv} - catch - Exception:Reason:StackTrace -> - ?LOG_ERROR("An exception occurred: ~p, ~p, ~p", [Exception, Reason, StackTrace]), - {{forward, OrigData}, Priv} %% XXX: proxy as-is or drop? - end. - - %% Send a single message to the MME sctp_send(Data, #{sock := Sock, enb_aid := EnbAid, mme_aid := Aid, priv := Priv} = S) -> - {Action, NewPriv} = handle_pdu(Data, Priv), + {Action, NewPriv} = s1ap_proxy:proc_safe(Data, Priv), case Action of {forward, FwdData} -> ok = sctp_client:send_data({Sock, Aid}, FwdData); -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38298?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: I16d4cf279a930d35ca179bd9b49234d10180e5c5 Gerrit-Change-Number: 38298 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

9 months

1
0
0 0

[M] Change in ...osmo-s1gw[master]: s1ap_proxy: handle E-RAB RELEASE INDICATION

by fixeria

Attention is currently required from: fixeria, pespin. Hello Jenkins Builder, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38241?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Code-Review+1 by pespin, Verified+1 by Jenkins Builder Change subject: s1ap_proxy: handle E-RAB RELEASE INDICATION ...................................................................... s1ap_proxy: handle E-RAB RELEASE INDICATION Change-Id: I13584620e28edf529325dc661c4b40d09acacc3f --- M src/s1ap_proxy.erl M test/s1ap_proxy_test.erl 2 files changed, 54 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/41/38241/4 -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/38241?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: I13584620e28edf529325dc661c4b40d09acacc3f Gerrit-Change-Number: 38241 Gerrit-PatchSet: 4 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-Attention: fixeria <vyanitskiy(a)sysmocom.de>

9 months

1
0
0 0

[L] Change in ...osmo-s1gw[master]: s1ap_proxy: Support replying errors

by fixeria

Attention is currently required from: fixeria, laforge, lynxis lazus. fixeria has uploaded a new patch set (#6) to the change originally created by pespin. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37924?usp=email ) The following approvals got outdated and were removed: Code-Review+1 by laforge, Code-Review-1 by fixeria, Verified+1 by Jenkins Builder Change subject: s1ap_proxy: Support replying errors ...................................................................... s1ap_proxy: Support replying errors Sometimes it is needed not only to forward, but to transmit messages initiated at the S1GW. This is the case, for instance, if the MME wants to initiate an E-RAB establishment, but the co-located UPF is not available at the time or responds with an error. This patch adds the generic infrastructure to be able to reply (errors) and implements the above mentioned specific case for the S1AP E-RAB SETUP REQUEST procedure. Change-Id: I242e84fb09b00f4794b6e1aa770f348a0e60aea4 --- M src/s1ap_proxy.erl M src/sctp_proxy.erl M test/s1ap_proxy_test.erl 3 files changed, 281 insertions(+), 147 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/24/37924/6 -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/37924?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: erlang/osmo-s1gw Gerrit-Branch: master Gerrit-Change-Id: I242e84fb09b00f4794b6e1aa770f348a0e60aea4 Gerrit-Change-Number: 37924 Gerrit-PatchSet: 6 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-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>

9 months

1
0
0 0

2025

2024

2023

2022

gerrit-log September 2024