fixeria has submitted this change. ( https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39631?usp=email )
Change subject: sctp_server: invalidate handler's Pid on 'EXIT'
......................................................................
sctp_server: invalidate handler's Pid on 'EXIT'
An 'EXIT' signal indicates that a connection handling process is dead,
so the respective pid shall not be referenced anymore. When this
happens, the current implementation shuts the eNB association down
gracefully and simply erases the respective client state from the
clients dictionary.
The problem with this approach is that we're bypassing the logic
in client_del/2 and, as a result, never decrementing the
?S1GW_GAUGE_S1AP_ENB_NUM_SCTP_CONNECTIONS. When in sctp_recv/2 we
receive a 'shutdown_comp' event and call client_del/2, the client
state is already gone.
Instead of erasing the client state on 'EXIT', invalidate handler's
pid by setting it to undefined and keep the clients dictionary intact.
Leave the task of dictionary manipulation up to client_{add,del} API.
Change-Id: I21fedf8579baa54dc1e7ade348abffd7ee9b04c4
Related: SYS#7288
---
M src/sctp_server.erl
1 file changed, 9 insertions(+), 3 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, approved
diff --git a/src/sctp_server.erl b/src/sctp_server.erl
index 9b10bbe..0a42011 100644
--- a/src/sctp_server.erl
+++ b/src/sctp_server.erl
@@ -69,7 +69,7 @@
}).
-record(client_state, {addr_port :: addr_port(),
- pid :: pid()
+ pid :: pid() | undefined
}).
@@ -153,10 +153,12 @@
#server_state{sock = Sock, clients = Clients} = S0) ->
?LOG_DEBUG("Child process ~p terminated with reason ~p", [Pid, Reason]),
case client_find(Pid, S0) of
- {ok, {Aid, _Client}} ->
+ {ok, {Aid, C0}} ->
%% gracefully close the eNB connection
gen_sctp:eof(Sock, #sctp_assoc_change{assoc_id = Aid}),
- S1 = S0#server_state{clients = dict:erase(Aid, Clients)},
+ %% invalidate pid in the client's state
+ C1 = C0#client_state{pid = undefined},
+ S1 = S0#server_state{clients = dict:store(Aid, C1, Clients)},
{noreply, S1};
error ->
{noreply, S0}
@@ -207,6 +209,9 @@
?LOG_DEBUG("eNB connection (id=~p, ~p:~p) -> MME: ~p", [Aid, FromAddr, FromPort, Data]),
s1gw_metrics:ctr_inc(?S1GW_CTR_S1AP_ENB_ALL_RX),
case dict:find(Aid, Clients) of
+ {ok, #client_state{pid = undefined}} ->
+ ?LOG_NOTICE("eNB connection (id=~p, ~p:~p) -> MME data ignored (no handler)",
+ [Aid, FromAddr, FromPort]);
{ok, #client_state{pid = Pid}} ->
Handler:send_data(Pid, Data);
error ->
@@ -247,6 +252,7 @@
s1gw_metrics:gauge_dec(?S1GW_GAUGE_S1AP_ENB_NUM_SCTP_CONNECTIONS),
S#server_state{clients = dict:erase(Aid, Clients)};
error ->
+ ?LOG_ERROR("eNB connection (id=~p) is not known to us?!?", [Aid]),
S
end.
--
To view, visit https://gerrit.osmocom.org/c/erlang/osmo-s1gw/+/39631?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: erlang/osmo-s1gw
Gerrit-Branch: master
Gerrit-Change-Id: I21fedf8579baa54dc1e7ade348abffd7ee9b04c4
Gerrit-Change-Number: 39631
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Attention is currently required from: fixeria, laforge, osmith, pespin.
Hello Jenkins Builder, fixeria, laforge, osmith,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39651?usp=email
to look at the new patch set (#6).
Change subject: stp: Fix brokeness in STP_Tests_M3UA.TC_tmt_loadshare
......................................................................
stp: Fix brokeness in STP_Tests_M3UA.TC_tmt_loadshare
The test was not even setting the traffic-mode in the IUT.
Furthermore, it was expecting pure round-robin behavior, which was the
older behavior of osmo-stp when loadshare traffic-mode was selected.
Actually split the test into 2, naming them properly (since round robin
is not a AS traffic mode in itself, but a possible implementation of the
loadshare traffic-mode.
The new test validates the usual loadshare traffic-mode based on SLS
distribution.
Related: SYS#7112
Depends: libosmo-sigtran.git Change-Id I61340549c596f1c04bc2269dbc165c327bf72037
Change-Id: I16d81cb2f88bb2927f248182ad4f8f27c8c24859
---
M stp/STP_Tests_M3UA.ttcn
M stp/expected-results.xml
2 files changed, 107 insertions(+), 9 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/51/39651/6
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39651?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newpatchset
Gerrit-Project: osmo-ttcn3-hacks
Gerrit-Branch: master
Gerrit-Change-Id: I16d81cb2f88bb2927f248182ad4f8f27c8c24859
Gerrit-Change-Number: 39651
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: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39619?usp=email )
Change subject: stp: Fix expectancies of TC_clnt_quirk_snm_inactive
......................................................................
stp: Fix expectancies of TC_clnt_quirk_snm_inactive
The test STP_Tests_M3UA.TC_clnt_quirk_snm_inactive validates the
snm_inactive quirk by sending a DAUD before the link being activated,
and expecting a DAVA to make sure osmo-stp did indeed process the SNM
message.
However, osmo-stp used to lack proper route validation based on link
state, which means it would incorrectly assumed the link for the
affected PC (55) in the test was active and hence would answer with a
DAVA. After libosmo-sigtran.git Change-Id
I928fb1ef5db6922f1386a188e3fbf9e70780f25d this wrong behavior is fixed,
and hence osmo-stp starts answering with a DUNA instead of a DAVA, since
AS "as-client" has not yet been activated during the test.
Fix the test expectancies by expecting a DUNA instead of a DAVA.
Change-Id: I907981c1487b299df852c405bae1fefff4bf5191
Depends: libosmo-sigtran.git Change-Id I928fb1ef5db6922f1386a188e3fbf9e70780f25d
Related: SYS#7112
---
M stp/STP_Tests_M3UA.ttcn
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
laforge: Looks good to me, but someone else must approve
Jenkins Builder: Verified
osmith: Looks good to me, approved
fixeria: Looks good to me, but someone else must approve
diff --git a/stp/STP_Tests_M3UA.ttcn b/stp/STP_Tests_M3UA.ttcn
index c3598d3..3fb3fc0 100644
--- a/stp/STP_Tests_M3UA.ttcn
+++ b/stp/STP_Tests_M3UA.ttcn
@@ -1325,7 +1325,7 @@
var template (value) M3UA_Point_Codes aff_pcs := { ts_M3UA_PC(f_m3ua_srv_config(0).point_code) };
f_M3UA_send(M3UA_SRV(0), ts_M3UA_DAUD(aff_pcs));
- f_M3UA_exp(M3UA_SRV(0), tr_M3UA_DAVA(aff_pcs));
+ f_M3UA_exp(M3UA_SRV(0), tr_M3UA_DUNA(aff_pcs));
setverdict(pass);
f_no_quirk("no_notify");
--
To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/39619?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: I907981c1487b299df852c405bae1fefff4bf5191
Gerrit-Change-Number: 39619
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(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-Reviewer: pespin <pespin(a)sysmocom.de>