- gerrit-log - lists.osmocom.org

[XS] Change in ...osmo-s1gw[master]: erab_fsm: fix comment inaccuracies and a typo
by fixeria 07 Mar '26

07 Mar '26

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42290?usp=email ) Change subject: erab_fsm: fix comment inaccuracies and a typo ...................................................................... erab_fsm: fix comment inaccuracies and a typo Two comments copy-pasted from session_establish were left saying "ESTABLISH Req" in session_modify and session_delete handlers where "MODIFY Req" and "DELETE Req" are correct respectively. Also fix typo "unieue" -> "unique" in init/1. Change-Id: If84638142988767363aa080012b44082dee39f90 --- M src/erab_fsm.erl 1 file changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/90/42290/1 diff --git a/src/erab_fsm.erl b/src/erab_fsm.erl index f281e4b..1e39cb7 100644 --- a/src/erab_fsm.erl +++ b/src/erab_fsm.erl @@ -233,7 +233,7 @@ init([UID]) -> set_logging_prefix(UID), ?LOG_DEBUG("Creating E-RAB FSM (UID ~p)", [UID]), - %% request a unieue SEID for this E-RAB FSM + %% request a unique SEID for this E-RAB FSM {ok, SEID} = pfcp_peer:seid_alloc(), {ok, erab_wait_setup_req, #erab_state{uid = UID, @@ -360,7 +360,7 @@ {reply, From, {error, {timeout, ?FUNCTION_NAME}}}}; session_modify(info, #pfcp{type = session_modification_response, - seid = SEID_Rsp, %% matches F-SEID we sent in the ESTABLISH Req + seid = SEID_Rsp, %% matches F-SEID we sent in the MODIFY Req ie = #{pfcp_cause := 'Request accepted'}}, #erab_state{mod_kind = ModKind, from = From, @@ -504,7 +504,7 @@ rel_kind = RelKind} = S) -> case PDU of #pfcp{type = session_deletion_response, - seid = SEID_Rsp, %% matches F-SEID we sent in the ESTABLISH Req + seid = SEID_Rsp, %% matches F-SEID we sent in the DELETE Req ie = #{pfcp_cause := 'Request accepted'}} -> ?LOG_DEBUG("PFCP session deleted"), case RelKind of -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42290?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: If84638142988767363aa080012b44082dee39f90 Gerrit-Change-Number: 42290 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[XS] Change in ...osmo-s1gw[master]: pfcp_peer: wrap seq_nr at 24-bit boundary
by fixeria 07 Mar '26

07 Mar '26

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42291?usp=email ) Change subject: pfcp_peer: wrap seq_nr at 24-bit boundary ...................................................................... pfcp_peer: wrap seq_nr at 24-bit boundary PFCP sequence numbers are 24-bit (3GPP TS 29.244 section 7.2.2.2), but the counter was incremented without wrapping, eventually exceeding the type spec after 16,777,215 requests. Introduce PFCP_SEQ_NR_MAX and use it in the type spec and in the increment expression. Change-Id: Ie269894add9a82c36698320379df3aca3f7ffea8 --- M src/pfcp_peer.erl 1 file changed, 4 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/91/42291/1 diff --git a/src/pfcp_peer.erl b/src/pfcp_peer.erl index f6e9c7d..a4cc2d5 100644 --- a/src/pfcp_peer.erl +++ b/src/pfcp_peer.erl @@ -57,13 +57,14 @@ %% 3GPP TS 29.244, section 4.2 "UDP Header and Port Numbers" -define(PFCP_PORT, 8805). +-define(PFCP_SEQ_NR_MAX, 16#ffffff). -define(PFCP_SEID_MAX, 16#ffffffffffffffff). -define(PFCP_HEARTBEAT_REQ_TIMEOUT, 2000). -type pfcp_session_rsp() :: ok | {error, term()}. -type pfcp_msg_type() :: atom(). --type pfcp_seq_nr() :: 0..16#ffffff. +-type pfcp_seq_nr() :: 0..?PFCP_SEQ_NR_MAX. -type pfcp_seid() :: 0..?PFCP_SEID_MAX. -type pfcp_f_seid() :: #f_seid{}. -type pfcp_ies() :: [term()] | map() | binary(). @@ -462,7 +463,8 @@ Data = pfcp_packet:encode(PDU), case send_data(Data, S) of ok -> - {ok, S#peer_state{seq_nr = SeqNr + 1}}; + NextSeqNr = (SeqNr + 1) band ?PFCP_SEQ_NR_MAX, + {ok, S#peer_state{seq_nr = NextSeqNr}}; {error, Error} -> {{error, Error}, S} end. -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42291?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: Ie269894add9a82c36698320379df3aca3f7ffea8 Gerrit-Change-Number: 42291 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[XS] Change in ...osmo-s1gw[master]: pfcp_peer: replace deprecated erlang:timestamp/0 with os:system_time/1
by fixeria 07 Mar '26

07 Mar '26

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42292?usp=email ) Change subject: pfcp_peer: replace deprecated erlang:timestamp/0 with os:system_time/1 ...................................................................... pfcp_peer: replace deprecated erlang:timestamp/0 with os:system_time/1 erlang:timestamp/0 is deprecated since OTP 18. os:system_time(second) is the modern replacement and yields the result directly in seconds, removing the need to reassemble the {MegaSec, Sec, _} tuple. Change-Id: I4ad886a2222f0cee8a668b42efe8bfac800a55ac --- M src/pfcp_peer.erl 1 file changed, 2 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/92/42292/1 diff --git a/src/pfcp_peer.erl b/src/pfcp_peer.erl index a4cc2d5..e1b3aa1 100644 --- a/src/pfcp_peer.erl +++ b/src/pfcp_peer.erl @@ -477,11 +477,10 @@ -spec get_recovery_timestamp() -> pos_integer(). get_recovery_timestamp() -> - {NowMS, NowS, _NowUS} = erlang:timestamp(), - %% erlang:timestamp() returns time relative to the UNIX epoch (1970), + %% os:system_time/1 returns time relative to the UNIX epoch (1970), %% but for PFCP we need time relative to the NTP era 0 (1900). %% 2208988800 is the diff between 1900 and 1970. - NowMS * 1_000_000 + NowS + 2208988800. + os:system_time(second) + 2208988800. -spec get_f_seid(pfcp_seid(), peer_state()) -> pfcp_f_seid(). -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42292?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: I4ad886a2222f0cee8a668b42efe8bfac800a55ac Gerrit-Change-Number: 42292 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[S] Change in ...osmo-s1gw[master]: pfcp_peer: replace watchdog process with a timer
by fixeria 07 Mar '26

07 Mar '26

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42293?usp=email ) Change subject: pfcp_peer: replace watchdog process with a timer ...................................................................... pfcp_peer: replace watchdog process with a timer The watchdog process is spawned bare (no link/monitor), so if it crashes its failure goes unnoticed, and if pfcp_peer restarts the orphaned watchdog may fire a stale cast at the new process. The watchdog process exists solely to send a delayed message to pfcp_peer on timeout, and to be cancelled (by receiving heartbeat_response) when the response arrives. That's exactly what erlang:start_timer/3 does natively - no separate process needed. Change-Id: I8d71ad8300feefb0aecbf690a825a2b4e9f1102c --- M src/pfcp_peer.erl 1 file changed, 10 insertions(+), 17 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/93/42293/1 diff --git a/src/pfcp_peer.erl b/src/pfcp_peer.erl index e1b3aa1..9910f5f 100644 --- a/src/pfcp_peer.erl +++ b/src/pfcp_peer.erl @@ -88,7 +88,7 @@ -record(heartbeat_state, {from :: undefined | gen_statem:from(), seq_nr :: pfcp_seq_nr(), - pid :: pid() + tref :: reference() }). -record(peer_state, {seid :: pfcp_seid(), @@ -323,11 +323,12 @@ %% Heartbeat Req (timeout) handle_event(_State, - cast, heartbeat_request_watchdog, + info, {timeout, TRef, heartbeat_request_watchdog}, #peer_state{heartbeat = HB} = S) -> case HB of #heartbeat_state{from = From, - seq_nr = SeqNr} -> + seq_nr = SeqNr, + tref = TRef} -> ?LOG_NOTICE("Heartbeat Request (SeqNr=~p) timeout", [SeqNr]), s1gw_metrics:ctr_inc(?S1GW_CTR_PFCP_HEARTBEAT_REQ_TIMEOUT), if @@ -337,7 +338,7 @@ ok end, {keep_state, S#peer_state{heartbeat = undefined}}; - undefined -> + _ -> {keep_state, S} end; @@ -550,25 +551,17 @@ case send_pdu({heartbeat_request, ReqIEs}, S0) of {ok, S1} -> s1gw_metrics:ctr_inc(?S1GW_CTR_PFCP_HEARTBEAT_REQ_TX), - Pid = spawn(fun heartbeat_request_watchdog/0), + TRef = erlang:start_timer(?PFCP_HEARTBEAT_REQ_TIMEOUT, self(), + heartbeat_request_watchdog), HB = #heartbeat_state{from = From, seq_nr = SeqNr, - pid = Pid}, + tref = TRef}, {ok, S1#peer_state{heartbeat = HB}}; Result -> Result end. -heartbeat_request_watchdog() -> - receive - heartbeat_response -> - ok - after - ?PFCP_HEARTBEAT_REQ_TIMEOUT -> - gen_statem:cast(?MODULE, ?FUNCTION_NAME) - end. - recv_heartbeat_response(#pfcp{type = heartbeat_response, seq_no = SeqNr, @@ -580,14 +573,14 @@ case HB of #heartbeat_state{from = From, seq_nr = SeqNr, - pid = Pid} -> + tref = TRef} -> + erlang:cancel_timer(TRef), if From =/= undefined -> gen_statem:reply(From, ok); true -> ok end, - Pid ! heartbeat_response, {ok, S#peer_state{heartbeat = undefined}}; _ -> ?LOG_NOTICE("Heartbeat Response (SeqNr=~p) was not expected", [SeqNr]), -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42293?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: I8d71ad8300feefb0aecbf690a825a2b4e9f1102c Gerrit-Change-Number: 42293 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[XS] Change in ...osmo-s1gw[master]: s1ap_proxy: fix discarded result of handle_ies/3 in HO REQ ACK handler
by fixeria 07 Mar '26

07 Mar '26

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42294?usp=email ) Change subject: s1ap_proxy: fix discarded result of handle_ies/3 in HO REQ ACK handler ...................................................................... s1ap_proxy: fix discarded result of handle_ies/3 in HO REQ ACK handler The call processing the optional E-RABFailedToSetupListHOReqAck IE did not pattern-match its return value, causing any errors returned by handle_ies/3 to be silently swallowed. Bind the result to {_, S2} to make the intent explicit. Change-Id: I1c8feeb63fe23876ae443784980e9dc22a450c54 --- M src/s1ap_proxy.erl 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/94/42294/1 diff --git a/src/s1ap_proxy.erl b/src/s1ap_proxy.erl index a29a817..ef7665e 100644 --- a/src/s1ap_proxy.erl +++ b/src/s1ap_proxy.erl @@ -604,8 +604,8 @@ end, %% handle the (optional) list of failed E-RABs %% no #proxy_state modification is expected here - handle_ies(?'id-E-RABFailedToSetupListHOReqAck', - C0#'HandoverRequestAcknowledge'.protocolIEs, S2), + {_, S2} = handle_ies(?'id-E-RABFailedToSetupListHOReqAck', + C0#'HandoverRequestAcknowledge'.protocolIEs, S2), {Result, S2}; -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42294?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: I1c8feeb63fe23876ae443784980e9dc22a450c54 Gerrit-Change-Number: 42294 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[S] Change in ...osmo-s1gw[master]: enb_registry: call enb_metrics_register/1 from a proper place
by fixeria 07 Mar '26

07 Mar '26

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42295?usp=email ) Change subject: enb_registry: call enb_metrics_register/1 from a proper place ...................................................................... enb_registry: call enb_metrics_register/1 from a proper place Change-Id: I38237463aa9c968f89bf4f195407a18cba7e73c9 --- M src/enb_registry.erl 1 file changed, 6 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/95/42295/1 diff --git a/src/enb_registry.erl b/src/enb_registry.erl index e40d9f5..54eca5d 100644 --- a/src/enb_registry.erl +++ b/src/enb_registry.erl @@ -235,7 +235,6 @@ {ok, EnbInfo0} -> ?LOG_INFO("eNB (handle=~p) event: ~p", [Handle, Event]), EnbInfo1 = enb_handle_event(EnbInfo0, Event), - enb_metrics_register(EnbInfo1), {noreply, S#state{enbs = maps:update(Handle, EnbInfo1, ENBs)}}; error -> ?LOG_ERROR("eNB (handle=~p) is *not* registered", [Handle]), @@ -283,11 +282,9 @@ %% private API %% ------------------------------------------------------------------ --spec enb_metrics_register(enb_info()) -> term(). -enb_metrics_register(#{genb_id_str := GlobalENBId}) -> - catch exometer:new(?S1GW_CTR_ENB_UPTIME(GlobalENBId), counter); - -enb_metrics_register(_) -> ok. +-spec enb_metrics_register(string()) -> term(). +enb_metrics_register(GlobalENBId) -> + catch exometer:new(?S1GW_CTR_ENB_UPTIME(GlobalENBId), counter). -spec enb_metrics_reset(enb_info()) -> term(). @@ -307,9 +304,11 @@ mme_conn_info => ConnInfo}; enb_handle_event(EnbInfo, {s1setup, GENBId}) -> + GlobalENBId = s1ap_utils:genb_id_str(GENBId), + enb_metrics_register(GlobalENBId), EnbInfo#{state => s1setup, genb_id => GENBId, - genb_id_str => s1ap_utils:genb_id_str(GENBId)}; + genb_id_str => GlobalENBId}; enb_handle_event(EnbInfo, Event) -> ?LOG_ERROR("Unhandled event: ~p", [Event]), -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42295?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: I38237463aa9c968f89bf4f195407a18cba7e73c9 Gerrit-Change-Number: 42295 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[XS] Change in ...osmo-s1gw[master]: enb_registry: fix pattern match in handle_info 'DOWN' handler
by fixeria 07 Mar '26

07 Mar '26

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42296?usp=email ) Change subject: enb_registry: fix pattern match in handle_info 'DOWN' handler ...................................................................... enb_registry: fix pattern match in handle_info 'DOWN' handler PIDs maps pid() => enb_handle() (an integer), so maps:find/2 returns {ok, SomeHandle}. The old pattern {ok, Pid} tried to match an integer against the already-bound pid() variable, which never succeeds. This handler is only called on abnormal termination (process crash), so the broken match caused the registry entry to never be cleaned up in that case. Fix by using a fresh {ok, Handle} binding and removing the now-redundant follow-up maps:get/2 call. Change-Id: I4cb044e8071c4ae2fc48c507f8733af6c617ec46 --- M src/enb_registry.erl 1 file changed, 1 insertion(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/96/42296/1 diff --git a/src/enb_registry.erl b/src/enb_registry.erl index 54eca5d..d2487a0 100644 --- a/src/enb_registry.erl +++ b/src/enb_registry.erl @@ -257,8 +257,7 @@ pids = PIDs} = S) -> ?LOG_INFO("eNB process (pid=~p) terminated with reason ~p", [Pid, Reason]), case maps:find(Pid, PIDs) of - {ok, Pid} -> - Handle = maps:get(Pid, PIDs), + {ok, Handle} -> enb_metrics_reset(maps:get(Handle, ENBs)), ?LOG_INFO("eNB (handle=~p, pid=~p) has been unregistered", [Handle, Pid]), {noreply, S#state{enbs = maps:remove(Handle, ENBs), -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42296?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: I4cb044e8071c4ae2fc48c507f8733af6c617ec46 Gerrit-Change-Number: 42296 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[XS] Change in ...osmo-s1gw[master]: enb_registry: fix duplicate registration check in enb_register/0
by fixeria 07 Mar '26

07 Mar '26

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42297?usp=email ) Change subject: enb_registry: fix duplicate registration check in enb_register/0 ...................................................................... enb_registry: fix duplicate registration check in enb_register/0 next_handle was bound in the function head and reused in the case pattern {ok, Handle}, turning what looks like a binding into an equality test against next_handle. The duplicate check therefore only fired if the previously assigned handle happened to equal the current next_handle counter, which is essentially never true. Fix by removing next_handle from the function head pattern and reading it with S#state.next_handle inside the error branch, so that Handle in {ok, Handle} is always a fresh binding that matches any existing handle. Change-Id: Ia64f3e2e79bea055e5c37df9bce91a4bcbf7184c --- M src/enb_registry.erl 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/97/42297/1 diff --git a/src/enb_registry.erl b/src/enb_registry.erl index d2487a0..29a2575 100644 --- a/src/enb_registry.erl +++ b/src/enb_registry.erl @@ -155,13 +155,13 @@ handle_call(enb_register, {Pid, _Tag}, #state{enbs = ENBs, - pids = PIDs, - next_handle = Handle} = S) -> + pids = PIDs} = S) -> case maps:find(Pid, PIDs) of {ok, Handle} -> ?LOG_ERROR("eNB (handle=~p, pid=~p) is *already* registered", [Handle, Pid]), {reply, {error, already_registered}, S}; error -> + Handle = S#state.next_handle, %% keep an eye on the process being registered MonRef = erlang:monitor(process, Pid), %% create and store an initial eNB state -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42297?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: Ia64f3e2e79bea055e5c37df9bce91a4bcbf7184c Gerrit-Change-Number: 42297 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[XS] Change in ...osmo-s1gw[master]: enb_registry: fix addr/port filter logic (use andalso)
by fixeria 07 Mar '26

07 Mar '26

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42298?usp=email ) Change subject: enb_registry: fix addr/port filter logic (use andalso) ...................................................................... enb_registry: fix addr/port filter logic (use andalso) In Erlang a function body returns only its last expression. The comma between the two enb_filter_by_sub_field/2 calls caused the result of the addr check to be silently discarded, so the filter only tested the port. Replace the comma with andalso to require both to match. Change-Id: I8061636cd1077a4f3a9e9d37a31224f5e373becb --- M src/enb_registry.erl 1 file changed, 2 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/98/42298/1 diff --git a/src/enb_registry.erl b/src/enb_registry.erl index 29a2575..3c9e8ef 100644 --- a/src/enb_registry.erl +++ b/src/enb_registry.erl @@ -330,11 +330,11 @@ fun(_, Item) -> enb_filter_by_sub_field({mme_conn_info, mme_aid, Aid}, Item) end; enb_filter({enb_addr_port, {Addr, Port}}) -> - fun(_, Item) -> enb_filter_by_sub_field({enb_conn_info, addr, Addr}, Item), + fun(_, Item) -> enb_filter_by_sub_field({enb_conn_info, addr, Addr}, Item) andalso enb_filter_by_sub_field({enb_conn_info, port, Port}, Item) end; enb_filter({mme_addr_port, {Addr, Port}}) -> - fun(_, Item) -> enb_filter_by_sub_field({mme_conn_info, mme_addr, Addr}, Item), + fun(_, Item) -> enb_filter_by_sub_field({mme_conn_info, mme_addr, Addr}, Item) andalso enb_filter_by_sub_field({mme_conn_info, mme_port, Port}, Item) end; enb_filter(Filter) -> -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42298?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: I8061636cd1077a4f3a9e9d37a31224f5e373becb Gerrit-Change-Number: 42298 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

1 0

[M] Change in ...osmo-s1gw[master]: enb_registry: rework handling of eNB property updates
by fixeria 07 Mar '26

07 Mar '26

fixeria has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42299?usp=email ) Change subject: enb_registry: rework handling of eNB property updates ...................................................................... enb_registry: rework handling of eNB property updates The idea of an enb_event/0 containing the current MME connection state and the associated information (Global-eNB-ID, eNB/MME conn info) looked promising initially, however turned out to be impractical in light of ongoing MME pooling related changes. * remove type enb_state/0 * rename type enb_event/0 -> enb_prop/0 * rename function enb_event/2 -> enb_update/2 (now private) * enb_prop/0: separate the state from other properties * enb_update/2: accept a list of enb_prop/0 - enb_proplist/0 * add typed notify_*() helpers that group related property updates, ensuring consistency and serving as the sole public call sites * notify_mme_connecting(): explicitly clears mme_conn_info * openapi: EnbItem.state reflects the actual enb_proxy FSM state Change-Id: Ib5c5dedce729151cd7350390987fdd008b254ef4 Related: SYS#7052 --- M contrib/openapi.yaml M doc/osmo-s1gw-cli.md M priv/openapi.json M src/enb_proxy.erl M src/enb_registry.erl 5 files changed, 103 insertions(+), 63 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-s1gw refs/changes/99/42299/1 diff --git a/contrib/openapi.yaml b/contrib/openapi.yaml index 307ae1c..b28b4f8 100644 --- a/contrib/openapi.yaml +++ b/contrib/openapi.yaml @@ -292,8 +292,8 @@ $ref: '#/components/schemas/Pid' state: type: string - enum: [connecting, connected, s1setup] - description: Connection state + enum: [wait_s1setup_req, connecting, wait_s1setup_rsp, connected] + description: eNB proxy state uptime: type: integer description: Uptime (in seconds) diff --git a/doc/osmo-s1gw-cli.md b/doc/osmo-s1gw-cli.md index 4a0bd22..be5ffc3 100644 --- a/doc/osmo-s1gw-cli.md +++ b/doc/osmo-s1gw-cli.md @@ -174,18 +174,18 @@ ``` OsmoS1GW# enb_list -| eNB handle | PID | Global-eNB-ID | State | eNB addr:port (aid) | MME addr:port (aid) | Uptime (s) | # E-RABs | -|--------------|-----------|-----------------|---------|-------------------------|-------------------------|--------------|------------| -| 0 | <0.699.0> | 001-01-0 | s1setup | 127.0.1.10:56767 (5706) | 127.0.2.10:36412 (5707) | 418 | 0 | -| 1 | <0.701.0> | 001-01-1 | s1setup | 127.0.1.10:54140 (5710) | 127.0.2.10:36412 (5711) | 33 | 3 | -| 2 | <0.703.0> | 001-01-2 | s1setup | 127.0.1.10:34076 (5714) | 127.0.2.10:36412 (5715) | 3600 | 20 | -| 3 | <0.705.0> | 001-01-3 | s1setup | 127.0.1.10:46501 (5718) | 127.0.2.10:36412 (5719) | 869 | 13 | -| 4 | <0.707.0> | 001-01-4 | s1setup | 127.0.1.10:35610 (5722) | 127.0.2.10:36412 (5723) | 18 | 0 | -| 5 | <0.709.0> | 001-01-5 | s1setup | 127.0.1.10:37610 (5726) | 127.0.2.10:36412 (5727) | 933 | 129 | -| 6 | <0.711.0> | 001-01-6 | s1setup | 127.0.1.10:58447 (5730) | 127.0.2.10:36412 (5741) | 535 | 6 | -| 7 | <0.713.0> | 001-01-7 | s1setup | 127.0.1.10:35842 (5732) | 127.0.2.10:36412 (5743) | 736 | 8 | -| 8 | <0.715.0> | 001-01-8 | s1setup | 127.0.1.10:57362 (5734) | 127.0.2.10:36412 (5745) | 521 | 99 | -| 9 | <0.717.0> | 001-01-9 | s1setup | 127.0.1.10:50955 (5736) | 127.0.2.10:36412 (5747) | 33 | 1 | +| eNB handle | PID | Global-eNB-ID | State | eNB addr:port (aid) | MME addr:port (aid) | Uptime (s) | # E-RABs | +|--------------|-----------|-----------------|-----------|-------------------------|-------------------------|--------------|------------| +| 0 | <0.699.0> | 001-01-0 | connected | 127.0.1.10:56767 (5706) | 127.0.2.10:36412 (5707) | 418 | 0 | +| 1 | <0.701.0> | 001-01-1 | connected | 127.0.1.10:54140 (5710) | 127.0.2.10:36412 (5711) | 33 | 3 | +| 2 | <0.703.0> | 001-01-2 | connected | 127.0.1.10:34076 (5714) | 127.0.2.10:36412 (5715) | 3600 | 20 | +| 3 | <0.705.0> | 001-01-3 | connected | 127.0.1.10:46501 (5718) | 127.0.2.10:36412 (5719) | 869 | 13 | +| 4 | <0.707.0> | 001-01-4 | connected | 127.0.1.10:35610 (5722) | 127.0.2.10:36412 (5723) | 18 | 0 | +| 5 | <0.709.0> | 001-01-5 | connected | 127.0.1.10:37610 (5726) | 127.0.2.10:36412 (5727) | 933 | 129 | +| 6 | <0.711.0> | 001-01-6 | connected | 127.0.1.10:58447 (5730) | 127.0.2.10:36412 (5741) | 535 | 6 | +| 7 | <0.713.0> | 001-01-7 | connected | 127.0.1.10:35842 (5732) | 127.0.2.10:36412 (5743) | 736 | 8 | +| 8 | <0.715.0> | 001-01-8 | connected | 127.0.1.10:57362 (5734) | 127.0.2.10:36412 (5745) | 521 | 99 | +| 9 | <0.717.0> | 001-01-9 | connected | 127.0.1.10:50955 (5736) | 127.0.2.10:36412 (5747) | 33 | 1 | ``` ### `enb_info` @@ -219,7 +219,7 @@ | eNB handle | 8 | | PID | <0.715.0> | | Global-eNB-ID | 001-01-8 | -| State | s1setup | +| State | connected | | eNB addr:port (aid) | 127.0.1.10:57362 (5734) | | MME addr:port (aid) | 127.0.2.10:36412 (5745) | | Uptime (s) | 521 | @@ -235,7 +235,7 @@ | eNB handle | 8 | | PID | <0.715.0> | | Global-eNB-ID | 001-01-8 | -| State | s1setup | +| State | connected | | eNB addr:port (aid) | 127.0.1.10:57362 (5734) | | MME addr:port (aid) | 127.0.2.10:36412 (5745) | | Uptime (s) | 521 | diff --git a/priv/openapi.json b/priv/openapi.json index c13d20c..d117fe9 100644 --- a/priv/openapi.json +++ b/priv/openapi.json @@ -432,11 +432,12 @@ "state": { "type": "string", "enum": [ + "wait_s1setup_req", "connecting", - "connected", - "s1setup" + "wait_s1setup_rsp", + "connected" ], - "description": "Connection state" + "description": "eNB proxy state" }, "uptime": { "type": "integer", diff --git a/src/enb_proxy.erl b/src/enb_proxy.erl index 4495e31..0aa00db 100644 --- a/src/enb_proxy.erl +++ b/src/enb_proxy.erl @@ -58,8 +58,8 @@ -include("S1AP-Constants.hrl"). --type conn_info() :: #{state := atom(), - handler := pid(), +%% XXX: replace/merge with mme_registry:mme_info()? +-type conn_info() :: #{handler := pid(), mme_addr := inet:ip_address(), mme_port := inet:port_number(), enb_aid := gen_sctp:assoc_id(), @@ -115,7 +115,6 @@ init([EnbConnInfo, MmeConnCfg]) -> {ok, EnbHandle} = enb_registry:enb_register(), - enb_registry:enb_event(EnbHandle, {connecting, EnbConnInfo}), {ok, Pid} = s1ap_proxy:start_link(self()), {ok, wait_s1setup_req, #state{enb_aid = maps:get(aid, EnbConnInfo), @@ -132,6 +131,8 @@ %% WAIT_S1SETUP_REQ state wait_s1setup_req(enter, _OldState, S) -> ?LOG_INFO("State enter: ~p", [?FUNCTION_NAME]), + enb_registry:notify_enb_connected(S#state.enb_handle, + S#state.enb_conn_info), {next_state, ?FUNCTION_NAME, S, [{state_timeout, 5_000, s1setup_req_timeout}]}; @@ -158,7 +159,7 @@ S#state{genb_id_str = GlobalENBId}), %% signal the Global-eNB-ID to other modules s1ap_proxy:set_genb_id(S#state.handler, GlobalENBId), - enb_registry:enb_event(S#state.enb_handle, {s1setup, GENBId}), + enb_registry:notify_genb_id(S#state.enb_handle, GENBId), gtpu_kpi_enb_register(S#state{genb_id_str = GlobalENBId}), {next_state, connecting, S#state{s1setup_req = Data, @@ -180,6 +181,7 @@ connecting(enter, OldState, #state{mme_conn_cfg = MmeConnCfg} = S) -> ?LOG_INFO("State change: ~p -> ~p", [OldState, ?FUNCTION_NAME]), + enb_registry:notify_mme_connecting(S#state.enb_handle), %% Initiate connection establishment with the MME {ok, Sock} = sctp_client:connect(MmeConnCfg), %% loop transition to enable state_timeout @@ -222,6 +224,7 @@ %% WAIT_S1SETUP_RSP state wait_s1setup_rsp(enter, OldState, S) -> ?LOG_INFO("State change: ~p -> ~p", [OldState, ?FUNCTION_NAME]), + enb_registry:notify_mme_wait_rsp(S#state.enb_handle), {next_state, ?FUNCTION_NAME, S, [{state_timeout, 5_000, s1setup_rsp_timeout}]}; @@ -288,11 +291,9 @@ %% CONNECTED state -connected(enter, OldState, - #state{enb_handle = Handle} = S) -> +connected(enter, OldState, S) -> ?LOG_INFO("State change: ~p -> ~p", [OldState, ?FUNCTION_NAME]), - MmeConnInfo = conn_info(?FUNCTION_NAME, S), - ok = enb_registry:enb_event(Handle, {?FUNCTION_NAME, MmeConnInfo}), + enb_registry:notify_mme_connected(S#state.enb_handle, conn_info(S)), {keep_state, S}; %% Handle an eNB -> MME data forwarding request (forward) @@ -338,9 +339,8 @@ %% Event handler for all states -handle_event(State, {call, From}, fetch_info, S) -> - Info = conn_info(State, S), - {keep_state_and_data, {reply, From, Info}}; +handle_event(_State, {call, From}, fetch_info, S) -> + {keep_state_and_data, {reply, From, conn_info(S)}}; handle_event(State, {call, From}, EventData, _S) -> ?LOG_ERROR("Unexpected call in state ~p: ~p", [State, EventData]), @@ -425,10 +425,9 @@ end. --spec conn_info(atom(), state()) -> conn_info(). -conn_info(State, #state{mme_conn_cfg = MmeConnCfg} = S) -> - Info = #{state => State, - handler => S#state.handler, +-spec conn_info(state()) -> conn_info(). +conn_info(#state{mme_conn_cfg = MmeConnCfg} = S) -> + Info = #{handler => S#state.handler, mme_addr => maps:get(raddr, MmeConnCfg), mme_port => maps:get(rport, MmeConnCfg), enb_aid => S#state.enb_aid, diff --git a/src/enb_registry.erl b/src/enb_registry.erl index 3c9e8ef..61f0c59 100644 --- a/src/enb_registry.erl +++ b/src/enb_registry.erl @@ -43,7 +43,11 @@ -export([start_link/0, enb_register/0, enb_unregister/1, - enb_event/2, + notify_enb_connected/2, + notify_genb_id/2, + notify_mme_connecting/1, + notify_mme_wait_rsp/1, + notify_mme_connected/2, fetch_enb_info/1, fetch_enb_list/0, fetch_enb_list/1, @@ -59,13 +63,11 @@ -type enb_handle() :: non_neg_integer(). --type enb_state() :: connecting %% S1GW -> MME connection in progress - | connected %% S1GW -> MME connection established - | s1setup. %% S1 SETUP procedure completed - --type enb_event() :: {connecting, sctp_server:conn_info()} - | {connected, enb_proxy:conn_info()} - | {s1setup, s1ap_utils:genb_id()}. +-type enb_prop() :: {state, atom()} + | {enb_conn_info, sctp_server:conn_info()} + | {mme_conn_info, undefined | enb_proxy:conn_info()} + | {genb_id, s1ap_utils:genb_id()}. +-type enb_proplist() :: [enb_prop()]. -type enb_filter() :: {genb_id_str, string()} | {enb_sctp_aid, gen_sctp:assoc_id()} @@ -76,7 +78,7 @@ -type enb_info() :: #{handle := enb_handle(), %% unique identifier of this eNB pid := pid(), %% pid() of the registrant mon_ref := reference(), %% monitor() reference - state := enb_state(), %% connection state + state := atom(), %% enb_proxy FSM state reg_time := integer(), %% registration time (monotonic) uptime := non_neg_integer(), %% seconds since reg_time genb_id => s1ap_utils:genb_id(), %% Global-eNB-ID @@ -91,7 +93,6 @@ }). -export_type([enb_handle/0, - enb_state/0, enb_info/0]). @@ -114,9 +115,34 @@ gen_server:call(?MODULE, {?FUNCTION_NAME, Handle}). --spec enb_event(enb_handle(), enb_event()) -> ok. -enb_event(Handle, Event) -> - gen_server:cast(?MODULE, {?FUNCTION_NAME, Handle, Event}). +%% eNB SCTP connection established; waiting for S1 SETUP REQUEST +-spec notify_enb_connected(enb_handle(), sctp_server:conn_info()) -> ok. +notify_enb_connected(Handle, ConnInfo) -> + enb_update(Handle, [{state, wait_s1setup_req}, {enb_conn_info, ConnInfo}]). + + +%% Global-eNB-ID is now known (from S1 SETUP REQUEST) +-spec notify_genb_id(enb_handle(), s1ap_utils:genb_id()) -> ok. +notify_genb_id(Handle, GENBId) -> + enb_update(Handle, [{genb_id, GENBId}]). + + +%% MME connection attempt in progress; clears any stale mme_conn_info +-spec notify_mme_connecting(enb_handle()) -> ok. +notify_mme_connecting(Handle) -> + enb_update(Handle, [{state, connecting}, {mme_conn_info, undefined}]). + + +%% MME SCTP link established; S1 SETUP REQUEST forwarded, awaiting response +-spec notify_mme_wait_rsp(enb_handle()) -> ok. +notify_mme_wait_rsp(Handle) -> + enb_update(Handle, [{state, wait_s1setup_rsp}]). + + +%% S1 SETUP complete; eNB is fully operational +-spec notify_mme_connected(enb_handle(), enb_proxy:conn_info()) -> ok. +notify_mme_connected(Handle, ConnInfo) -> + enb_update(Handle, [{state, connected}, {mme_conn_info, ConnInfo}]). -spec fetch_enb_info(enb_handle() | pid()) -> {ok, enb_info()} | error. @@ -168,7 +194,7 @@ EnbInfo = #{handle => Handle, pid => Pid, mon_ref => MonRef, - state => connecting, + state => unknown, reg_time => erlang:monotonic_time(), uptime => 0}, ?LOG_INFO("eNB (handle=~p, pid ~p) registered", [Handle, Pid]), @@ -229,12 +255,12 @@ {reply, {error, not_implemented}, S}. -handle_cast({enb_event, Handle, Event}, +handle_cast({enb_update, Handle, PropList}, #state{enbs = ENBs} = S) -> case maps:find(Handle, ENBs) of {ok, EnbInfo0} -> - ?LOG_INFO("eNB (handle=~p) event: ~p", [Handle, Event]), - EnbInfo1 = enb_handle_event(EnbInfo0, Event), + ?LOG_DEBUG("eNB (handle=~p) update: ~p", [Handle, PropList]), + EnbInfo1 = enb_update_proplist(EnbInfo0, PropList), {noreply, S#state{enbs = maps:update(Handle, EnbInfo1, ENBs)}}; error -> ?LOG_ERROR("eNB (handle=~p) is *not* registered", [Handle]), @@ -281,6 +307,11 @@ %% private API %% ------------------------------------------------------------------ +-spec enb_update(enb_handle(), enb_proplist()) -> ok. +enb_update(Handle, PropList) -> + gen_server:cast(?MODULE, {?FUNCTION_NAME, Handle, PropList}). + + -spec enb_metrics_register(string()) -> term(). enb_metrics_register(GlobalENBId) -> catch exometer:new(?S1GW_CTR_ENB_UPTIME(GlobalENBId), counter). @@ -293,24 +324,33 @@ enb_metrics_reset(_) -> ok. --spec enb_handle_event(enb_info(), enb_event()) -> enb_info(). -enb_handle_event(EnbInfo, {connecting, ConnInfo}) -> - EnbInfo#{state => connecting, - enb_conn_info => ConnInfo}; +-spec enb_update_proplist(enb_info(), enb_proplist()) -> enb_info(). +enb_update_proplist(EnbInfo, PropList) -> + lists:foldl(fun enb_update_prop/2, EnbInfo, PropList). -enb_handle_event(EnbInfo, {connected, ConnInfo}) -> - EnbInfo#{state => connected, - mme_conn_info => ConnInfo}; -enb_handle_event(EnbInfo, {s1setup, GENBId}) -> +-spec enb_update_prop(enb_info(), enb_prop()) -> enb_info(). +enb_update_prop({state, State}, EnbInfo) -> + EnbInfo#{state => State}; + +enb_update_prop({enb_conn_info, ConnInfo}, EnbInfo) -> + EnbInfo#{enb_conn_info => ConnInfo}; + +enb_update_prop({mme_conn_info, undefined}, EnbInfo) -> + maps:remove(mme_conn_info, EnbInfo); + +enb_update_prop({mme_conn_info, ConnInfo}, EnbInfo) -> + EnbInfo#{mme_conn_info => ConnInfo}; + +enb_update_prop({genb_id, GENBId}, EnbInfo) -> GlobalENBId = s1ap_utils:genb_id_str(GENBId), enb_metrics_register(GlobalENBId), - EnbInfo#{state => s1setup, - genb_id => GENBId, + EnbInfo#{genb_id => GENBId, genb_id_str => GlobalENBId}; -enb_handle_event(EnbInfo, Event) -> - ?LOG_ERROR("Unhandled event: ~p", [Event]), +enb_update_prop(Prop, EnbInfo) -> + ?LOG_ERROR("eNB (handle=~p) update: unknown property ~p", + [maps:get(handle, EnbInfo), Prop]), EnbInfo. -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/42299?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: Ib5c5dedce729151cd7350390987fdd008b254ef4 Gerrit-Change-Number: 42299 Gerrit-PatchSet: 1 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>

1 0

2026

2025

2024

2023

2022

gerrit-log