- gerrit-log - lists.osmocom.org

[M] Change in ...osmo-epdg[master]: s2b: Implement GTPv2C DeleteBearerReq
by pespin 25 Oct '23

25 Oct '23

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/34896?usp=email ) Change subject: s2b: Implement GTPv2C DeleteBearerReq ...................................................................... s2b: Implement GTPv2C DeleteBearerReq Sessions are now stored/kept upon CreateSession time until deleted through DeleteBearerReq. Related: OS#6046 Change-Id: I1e5af1ead17385d2e494f4c90ffe6455aee850da --- M src/epdg_gtpc_s2b.erl 1 file changed, 137 insertions(+), 26 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-epdg refs/changes/96/34896/1 diff --git a/src/epdg_gtpc_s2b.erl b/src/epdg_gtpc_s2b.erl index 82985a5..a5658f1 100644 --- a/src/epdg_gtpc_s2b.erl +++ b/src/epdg_gtpc_s2b.erl @@ -70,7 +70,9 @@ rport :: non_neg_integer(), restart_counter :: 0..255, seq_no :: 0..16#ffffffff, - sess_list %% TODO: fill it, list of gtp_session + next_local_control_tei = 1 :: 0..16#ffffffff, + next_local_data_tei = 1 :: 0..16#ffffffff, + sessions = sets:new() }). -record(gtp_bearer, { @@ -134,36 +136,45 @@ gen_server:call(?SERVER, {gtpc_create_session_req, {Imsi}}). -handle_call({gtpc_create_session_req, {Imsi}}, _From, State) -> - Sess = new_gtp_session(Imsi, State), - Req = gen_create_session_request(Sess, State), +handle_call({gtpc_create_session_req, {Imsi}}, _From, State0) -> + {Sess0, State1} = find_or_new_gtp_session(Imsi, State0), + Req = gen_create_session_request(Sess0, State1), %TODO: increment State.seq_no. - tx_gtp(Req, State), + tx_gtp(Req, State1), lager:debug("Waiting for CreateSessionResponse~n", []), receive {udp, _Socket, IP, InPortNo, RxMsg} -> try Resp = gtp_packet:decode(RxMsg), - logger:info("s2b: Rx from IP ~p port ~n ~p~n", [IP, InPortNo, Resp]), - %% TODO: store Sess in State. - {reply, {ok, Resp}, State} + lager:info("s2b: Rx from IP ~p port ~p ~p~n", [IP, InPortNo, Resp]), + Sess1 = update_gtp_session_from_create_session_response(Resp, Sess0), + lager:info("s2b: Updated Session after create_session_response: ~p~n", [Sess1]), + State2 = update_gtp_session(Sess0, Sess1, State1), + {reply, {ok, Resp}, State2} catch Any -> - logger:error("Error sending message to receiver, ERROR: ~p~n", [Any]), - {reply, {error, decode_failure}, State} + lager:error("Error sending message to receiver, ERROR: ~p~n", [Any]), + {reply, {error, decode_failure}, State1} end after 5000 -> - logger:error("Timeout waiting for CreateSessionResponse for ~p~n", [Req]), - {reply, timeout, State} + lager:error("Timeout waiting for CreateSessionResponse for ~p~n", [Req]), + {reply, timeout, State1} end. %% @callback gen_server handle_cast(stop, State) -> {stop, normal, State}; -handle_cast(_Req, State) -> +handle_cast(Req, State) -> + lager:info("S2b handle_cast: ~p ~n", [Req]), {noreply, State}. %% @callback gen_server -handle_info(_Info, State) -> +handle_info({udp, _Socket, IP, InPortNo, RxMsg}, State) -> + lager:info("S2b: Rx from IP ~p port ~p: ~p~n", [IP, InPortNo, RxMsg]), + Req = gtp_packet:decode(RxMsg), + lager:info("S2b: Rx from IP ~p port ~p: ~p~n", [IP, InPortNo, Req]), + rx_gtp(Req, State); +handle_info(Info, State) -> + lager:info("S2b handle_info: ~p ~n", [Info]), {noreply, State}. %% @callback gen_server @@ -185,6 +196,76 @@ %% Internal Function Definitions %% ------------------------------------------------------------------ +new_gtp_session(Imsi, State) -> + % TODO: find non-used local TEI inside State + Bearer = #gtp_bearer{ + ebi = 5, + local_data_tei = State#gtp_state.next_local_data_tei + }, + Sess = #gtp_session{imsi = Imsi, + apn = ?APN, + local_control_tei = State#gtp_state.next_local_control_tei, + bearer = Bearer + }, + NewSt = State#gtp_state{next_local_control_tei = State#gtp_state.next_local_control_tei + 1, + next_local_data_tei = State#gtp_state.next_local_data_tei + 1, + sessions = sets:add_element(Sess, State#gtp_state.sessions)}, + {Sess, NewSt}. + +% returns Sess if found, undefined it not +find_gtp_session_by_imsi(Imsi, State) -> + sets:fold( + fun(SessIt = #gtp_session{imsi = Imsi}, _AccIn) -> SessIt; + (_, AccIn) -> AccIn + end, + undefined, + State#gtp_state.sessions). + +find_or_new_gtp_session(Imsi, State) -> + Sess = find_gtp_session_by_imsi(Imsi, State), + case Sess of + #gtp_session{imsi = Imsi} -> + {Sess, State}; + undefined -> + new_gtp_session(Imsi, State) + end. + +update_gtp_session(OldSess, NewSess, State) -> + SetRemoved = sets:del_element(OldSess, State#gtp_state.sessions), + SetUpdated = sets:add_element(NewSess, SetRemoved), + State#gtp_state{sessions = SetUpdated}. + +delete_gtp_session(Sess, State) -> + SetRemoved = sets:del_element(Sess, State#gtp_state.sessions), + State#gtp_state{sessions = SetRemoved}. + +update_gtp_session_from_create_session_response_ie(none, Sess) -> + Sess; +update_gtp_session_from_create_session_response_ie({_, + #v2_fully_qualified_tunnel_endpoint_identifier{ + interface_type = _Interface, + key = TEI, ipv4 = _IP4, ipv6 = _IP6}, + Next}, Sess) -> + update_gtp_session_from_create_session_response_ie(maps:next(Next), Sess#gtp_session{remote_control_tei = TEI}); +update_gtp_session_from_create_session_response_ie({_, _, Next}, + Sess) -> + update_gtp_session_from_create_session_response_ie(maps:next(Next), Sess). + +update_gtp_session_from_create_session_response_ies(#gtp{ie = IEs}, Sess) -> + update_gtp_session_from_create_session_response_ie(maps:next(maps:iterator(IEs)), Sess). + +update_gtp_session_from_create_session_response(Resp = #gtp{version = v2, type = create_session_response}, Sess) -> + update_gtp_session_from_create_session_response_ies(#gtp{ie = Resp#gtp.ie}, Sess). + +% returns Sess if found, undefined it not +find_gtp_session_by_local_teic(LocalControlTei, State) -> + sets:fold( + fun(SessIt = #gtp_session{local_control_tei = LocalControlTei}, _AccIn) -> SessIt; + (_, AccIn) -> AccIn + end, + undefined, + State#gtp_state.sessions). + %% connect/2 connect(Name, {Socket, RemoteAddr, RemotePort}) -> lager:info("~s connecting to IP ~s port ~p~n", [Name, RemoteAddr, RemotePort]), @@ -193,23 +274,27 @@ connect(Address) -> connect(?SVC_NAME, Address). +rx_gtp(Req = #gtp{version = v2, type = delete_bearer_request}, State) -> + Sess = find_gtp_session_by_local_teic(Req#gtp.tei, State), + case Sess of + undefined -> + lager:error("Rx unknown TEI ~p: ~p~n", [Req#gtp.tei, Req]), + {noreply, State}; + Sess -> + Resp = gen_delete_bearer_response(Req, Sess, request_accepted, State), + tx_gtp(Resp, State), + State1 = delete_gtp_session(Sess, State), + {noreply, State1} + end; +rx_gtp(Req, State) -> + lager:error("S2b: UNIMPLEMENTED Rx: ~p~n", [Req]), + {noreply, State}. + tx_gtp(Req, State) -> lager:info("s2b: Tx ~p~n", [Req]), Msg = gtp_packet:encode(Req), gen_udp:send(State#gtp_state.socket, State#gtp_state.raddr, State#gtp_state.rport, Msg). -new_gtp_session(Imsi, _State) -> - % TODO: find non-used local TEI inside State - Bearer = #gtp_bearer{ - ebi = 5, - local_data_tei = 1 - }, - #gtp_session{imsi = Imsi, - apn = ?APN, - local_control_tei = 0, - bearer = Bearer - }. - %% 7.2.1 Create Session Request gen_create_session_request(#gtp_session{imsi = Imsi, apn = Apn, @@ -249,4 +334,17 @@ ], #gtp{version = v2, type = create_session_request, tei = 0, seq_no = SeqNo, ie = IEs}. +gen_delete_bearer_response(Req = #gtp{version = v2, type = delete_bearer_request}, + Sess = #gtp_session{remote_control_tei = RemoteCtlTEI}, + GtpCause, + #gtp_state{restart_counter = RCnt}) -> + IEs = [#v2_recovery{restart_counter = RCnt}, + #v2_cause{v2_cause = GtpCause} + ], + #gtp{version = v2, + type = delete_bearer_response, + tei = RemoteCtlTEI, + seq_no = Req#gtp.seq_no, + ie = IEs}. + -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/34896?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: erlang/osmo-epdg Gerrit-Branch: master Gerrit-Change-Id: I1e5af1ead17385d2e494f4c90ffe6455aee850da Gerrit-Change-Number: 34896 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[M] Change in osmo-ttcn3-hacks[master]: epdg: Test Handover from Wifi back to LTE
by pespin 25 Oct '23

25 Oct '23

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34895?usp=email ) Change subject: epdg: Test Handover from Wifi back to LTE ...................................................................... epdg: Test Handover from Wifi back to LTE The Handover is informed to the EPDG by the PGW, who sends a DeleteBearerRequest when receiving an attach from the 3GPP network once the phone has jumped there. Related: OS#6046 Change-Id: I299faf28fa51dbc5d2de6c72a39a01eca67a5775 --- M epdg/EPDG_Tests.ttcn M library/GTPv2_Templates.ttcn 2 files changed, 83 insertions(+), 19 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-ttcn3-hacks refs/changes/95/34895/1 diff --git a/epdg/EPDG_Tests.ttcn b/epdg/EPDG_Tests.ttcn index e7c8633..83b42fa 100644 --- a/epdg/EPDG_Tests.ttcn +++ b/epdg/EPDG_Tests.ttcn @@ -112,20 +112,27 @@ }; +type record BearerConfig { + /* EPS Bearer ID */ + uint4_t ebi optional, + /* TEI (Data) local side */ + OCT4 teid_local optional, + /* TEI (Data) remote side */ + OCT4 teid_remote optional +}; + type record EPDG_ConnHdlrPars { hexstring imsi, charstring apn, charstring ue_ip, - /* TEI (Data) local side */ - OCT4 teid, /* TEI (Control) local side */ - OCT4 teic, - /* TEI (Data) remote side */ - OCT4 teid_remote optional, + OCT4 teic_local, /* TEI (Control) remote side */ OCT4 teic_remote optional, + BearerConfig bearer optional, + AuthVector vec optional }; @@ -146,10 +153,13 @@ imsi := f_gen_imsi(imsi_suffix), apn := "internet", ue_ip := "192.168.123.50", - teid := '00000000'O, - teic := '00000000'O, - teid_remote := omit, + teic_local := '00000000'O, teic_remote := omit, + bearer := { + ebi := omit, + teid_local := omit, + teid_remote := omit + }, vec := f_gen_auth_vec_3g() }; return pars; @@ -391,31 +401,34 @@ /* Diameter SWx SAR + SAA. */ private altstep as_GTP2C_CreateSession_success() runs on EPDG_ConnHdlr { var PDU_GTPCv2 rx_msg; + var BearerContextIEs rx_bctx_ies; var template (value) FullyQualifiedTEID fteid_c_ie, fteid_u_ie; var template (value) PDN_AddressAllocation paa; - var uint4_t bid; var template (value) BearerContextIEs bctx_ies; [] GTP2.receive(tr_GTP2C_CreateSessionReq(g_pars.imsi)) -> value rx_msg { - /* TODO: parse TEIC and TEID and store it in g_pars.remote_tei{c,d} */ - bid := rx_msg.gtpcv2_pdu.createSessionRequest.bearerContextGrouped[0].bearerContextIEs.ePS_Bearer_ID.ePS_Bearer_ID_Value; + /* Parse TEIC and Bearer EBI and TEID and store it in g_pars */ + g_pars.teic_remote := rx_msg.gtpcv2_pdu.createSessionRequest.fullyQualifiedTEID[0].tEID_GRE_Key; + rx_bctx_ies := rx_msg.gtpcv2_pdu.createSessionRequest.bearerContextGrouped[0].bearerContextIEs; + g_pars.bearer.ebi := rx_bctx_ies.ePS_Bearer_ID.ePS_Bearer_ID_Value; + g_pars.bearer.teid_remote := rx_bctx_ies.fullyQualifiedTEID[0].tEID_GRE_Key; /* allocate + register TEID-C on local side */ - g_pars.teic := f_gtp2_allocate_teid(); - g_pars.teid := g_pars.teic; + g_pars.teic_local := f_gtp2_allocate_teid(); + g_pars.bearer.teid_local := g_pars.teic_local; /* Upon rx of CreateSession, emulate PGW asking the AAA server. */ f_S6b_AA_success(); - fteid_c_ie := ts_GTP2C_FTEID(FTEID_IF_S2b_ePDG_GTPC, g_pars.teic, 0, + fteid_c_ie := ts_GTP2C_FTEID(FTEID_IF_S2b_ePDG_GTPC, g_pars.teic_local, 0, f_inet_addr(mp_s2b_local_ip), omit); - fteid_u_ie := ts_GTP2C_FTEID(FTEID_IF_S2bU_ePDG_GTPU, g_pars.teid, 2, + fteid_u_ie := ts_GTP2C_FTEID(FTEID_IF_S2bU_ePDG_GTPU, g_pars.bearer.teid_local, 2, f_inet_addr(mp_s2b_local_ip), omit); paa := ts_GTP2C_PdnAddrAlloc_v4(f_inet_addr(g_pars.ue_ip)); - bctx_ies := ts_GTP2C_BcContextIE(bid := bid, + bctx_ies := ts_GTP2C_BcContextIE(ebi := g_pars.bearer.ebi, teid_list := { fteid_u_ie }, qos := ts_GTP2C_BearerQos('09'O, 0,0,0,0), - charging_id := ts_GTP2C_ChargingID(g_pars.teic)); + charging_id := ts_GTP2C_ChargingID(g_pars.teic_local)); GTP2.send(ts_GTP2C_CreateSessionResp({ fteid_c_ie }, paa, { ts_GTP2C_BcGrouped(bctx_ies) } )); setverdict(pass); } @@ -428,6 +441,26 @@ as_GTP2C_CreateSession_success(); } +/* Expect DeleteBearerResponse */ +private altstep as_GTP2C_DeleteBearer_success() runs on EPDG_ConnHdlr { + var PDU_GTPCv2 rx_msg; + var template (value) FullyQualifiedTEID fteid_c_ie, fteid_u_ie; + var template (value) PDN_AddressAllocation paa; + + [] GTP2.receive(tr_GTP2C_DeleteBearerResp(g_pars.teic_local)) -> value rx_msg { + setverdict(pass); + } + [] GTP2.receive(PDU_GTPCv2:?) -> value rx_msg { + Misc_Helpers.f_shutdown(__BFILE__, __LINE__, fail, log2str("Unexpected GTP2C msg rx: ", rx_msg)); + } +} +private function f_GTP2C_DeleteBearer_success() runs on EPDG_ConnHdlr { + var integer proc_trans_id := 3; + /* f_gtp2_register_udmsg('63'O); TODO: is this needed? */ + GTP2.send(ts_GTP2C_DeleteBearerReq(g_pars.teic_remote, proc_trans_id, g_pars.bearer.ebi, Access_changed_from_Non_3GPP_to_3GPP)); + as_GTP2C_DeleteBearer_success(); +} + /* GSUP AuthInfo Req + Resp, triggers SWx MAR + MAA. */ private function f_GSUP_AI_success() runs on EPDG_ConnHdlr { var GSUP_PDU rx_gsup; @@ -495,8 +528,25 @@ setverdict(pass); } +private function f_TC_ho_wifi_to_lte(charstring id) runs on EPDG_ConnHdlr { + f_TC_authinfo_normal(id); + /* Whenever UE goes back to 3GPP, PGW will notify us with a Delete Brearer Request + * cause="Access changed from non-3gpp to 3gpp" */ + f_GTP2C_DeleteBearer_success(); +} + +testcase TC_ho_wifi_to_lte() runs on MTC_CT { + var EPDG_ConnHdlrPars pars := f_init_pars(); + var EPDG_ConnHdlr vc_conn; + f_init(); + vc_conn := f_start_handler(refers(f_TC_ho_wifi_to_lte), pars); + vc_conn.done; + setverdict(pass); +} + control { execute ( TC_authinfo_normal() ); + execute ( TC_ho_wifi_to_lte() ); } } diff --git a/library/GTPv2_Templates.ttcn b/library/GTPv2_Templates.ttcn index e1e71f0..6ea06a2 100644 --- a/library/GTPv2_Templates.ttcn +++ b/library/GTPv2_Templates.ttcn @@ -506,11 +506,11 @@ /* 8.28 */ template (value) BearerContextIEs -ts_GTP2C_BcContextIE(template (value) uint4_t bid, +ts_GTP2C_BcContextIE(template (value) uint4_t ebi, template (omit) FullyQualifiedTEID_List teid_list := omit, template (omit) Bearer_QoS qos := ts_GTP2C_BearerQos('09'O, 0,0,0,0), template (omit) ChargingID charging_id := omit) := { - ePS_Bearer_ID := ts_GTP2C_EpsBearerId(bid), + ePS_Bearer_ID := ts_GTP2C_EpsBearerId(ebi), cause := ts_GTP2C_Cause(Request_accepted, '0'B), ePS_Bearer_TFT := omit, fullyQualifiedTEID := teid_list, -- To view, visit https://gerrit.osmocom.org/c/osmo-ttcn3-hacks/+/34895?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ttcn3-hacks Gerrit-Branch: master Gerrit-Change-Id: I299faf28fa51dbc5d2de6c72a39a01eca67a5775 Gerrit-Change-Number: 34895 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-bts[master]: Revert "trx_if: Allow calling trx_if_flush/close from within TRXC cal...
by pespin 25 Oct '23

25 Oct '23

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/34894?usp=email ) Change subject: Revert "trx_if: Allow calling trx_if_flush/close from within TRXC callback" ...................................................................... Revert "trx_if: Allow calling trx_if_flush/close from within TRXC callback" This reverts commit 4444262a6ab1e1e231ea81c4ec990f1a1f571a1f. This commit introduced several side effects: - tcm is left out of the l1h->trx_ctrl_list and hence won't be ever retransmitted. - Since tcm is removed before rsp callback, the llist may become empty and if somehwere in the rsp callback a new message is enqueud it will be sent immediatelly, and will be retransmitted again when trx_ctrl_read_cb() calls trx_ctrl_send() at the end. Change-Id: Ideb2d08ac8a2902bceeabfb055c59c9a13dbe3c0 Related: OS#6020 --- M src/osmo-bts-trx/trx_if.c 1 file changed, 25 insertions(+), 9 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, approved diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c index fef8c22..3f9fc04 100644 --- a/src/osmo-bts-trx/trx_if.c +++ b/src/osmo-bts-trx/trx_if.c @@ -721,13 +721,6 @@ rsp.cb = tcm->cb; - /* Remove command from list, save it to last_acked and remove previous - * last_acked. Do it before calling callback to avoid user freeing tcm - * pointer if flushing/closing the iface. */ - llist_del(&tcm->list); - talloc_free(l1h->last_acked); - l1h->last_acked = tcm; - /* check for response code */ rc = trx_ctrl_rx_rsp(l1h, &rsp, tcm); if (rc == -EINVAL) @@ -735,11 +728,15 @@ /* re-schedule last cmd in rc seconds time */ if (rc > 0) { - if (!llist_empty(&l1h->trx_ctrl_list)) - osmo_timer_schedule(&l1h->trx_ctrl_timer, rc, 0); + osmo_timer_schedule(&l1h->trx_ctrl_timer, rc, 0); return 0; } + /* remove command from list, save it to last_acked and removed previous last_acked */ + llist_del(&tcm->list); + talloc_free(l1h->last_acked); + l1h->last_acked = tcm; + trx_ctrl_send(l1h); return 0; -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34894?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ideb2d08ac8a2902bceeabfb055c59c9a13dbe3c0 Gerrit-Change-Number: 34894 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-bts[master]: trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2)
by pespin 25 Oct '23

25 Oct '23

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email ) Change subject: trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2) ...................................................................... trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2) - If the llist is flushed during rx rsp callback, when the flow is returned to trx_ctrl_read_cb() it would access tcm which was in the llist and end up in use-after-free. - We need to store state on whether code path is inside the read_cb in order to: -- Delay transmission of new message if callback calls trx_if_flush() followed by trx_ctrl_send(), since the trx_ctrl_send() at the end of trx_ctrl_read_cb would retransmit it again immediatelly. -- Avoid accessing tcm pointer if the callback called trx_if_flush(), since it has been freed. Related: OS#6020 Change-Id: Ibdffa4644aa3a7d219452644d3e74b411734f1df --- M src/osmo-bts-trx/l1_if.h M src/osmo-bts-trx/trx_if.c 2 files changed, 52 insertions(+), 7 deletions(-) Approvals: daniel: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/src/osmo-bts-trx/l1_if.h b/src/osmo-bts-trx/l1_if.h index 18d84c2..84fd4b5 100644 --- a/src/osmo-bts-trx/l1_if.h +++ b/src/osmo-bts-trx/l1_if.h @@ -120,6 +120,10 @@ struct llist_head trx_ctrl_list; /* Latest RSPed cmd, used to catch duplicate RSPs from sent retransmissions */ struct trx_ctrl_msg *last_acked; + /* Whether the code path is in the middle of handling a received message. */ + bool in_trx_ctrl_read_cb; + /* Whether the l1h->trx_ctrl_list was flushed by the callback handling a received message */ + bool flushed_while_in_trx_ctrl_read_cb; //struct gsm_bts_trx *trx; struct phy_instance *phy_inst; diff --git a/src/osmo-bts-trx/trx_if.c b/src/osmo-bts-trx/trx_if.c index 3f9fc04..89078a3 100644 --- a/src/osmo-bts-trx/trx_if.c +++ b/src/osmo-bts-trx/trx_if.c @@ -269,8 +269,10 @@ tcm->cmd, tcm->params_len ? " " : "", tcm->params); llist_add_tail(&tcm->list, &l1h->trx_ctrl_list); - /* send message, if we didn't already have pending messages */ - if (prev == NULL) + /* send message, if we didn't already have pending messages. + * If we are in the rx_rsp callback code path, skip sending, the + * callback will do so when returning to it. */ + if (prev == NULL && !l1h->in_trx_ctrl_read_cb) trx_ctrl_send(l1h); return 0; @@ -673,6 +675,7 @@ struct trx_ctrl_rsp rsp; int len, rc; struct trx_ctrl_msg *tcm; + bool flushed; len = recv(ofd->fd, buf, sizeof(buf) - 1, 0); if (len <= 0) @@ -722,21 +725,34 @@ rsp.cb = tcm->cb; /* check for response code */ + l1h->in_trx_ctrl_read_cb = true; rc = trx_ctrl_rx_rsp(l1h, &rsp, tcm); + /* Reset state: */ + flushed = l1h->flushed_while_in_trx_ctrl_read_cb; + l1h->flushed_while_in_trx_ctrl_read_cb = false; + l1h->in_trx_ctrl_read_cb = false; + if (rc == -EINVAL) goto rsp_error; /* re-schedule last cmd in rc seconds time */ if (rc > 0) { - osmo_timer_schedule(&l1h->trx_ctrl_timer, rc, 0); + /* The queue may have been flushed in the trx_ctrl_rx_rsp(): */ + if (!llist_empty(&l1h->trx_ctrl_list)) + osmo_timer_schedule(&l1h->trx_ctrl_timer, rc, 0); return 0; } - /* remove command from list, save it to last_acked and removed previous last_acked */ - llist_del(&tcm->list); - talloc_free(l1h->last_acked); - l1h->last_acked = tcm; + if (!flushed) { + /* Remove command from list, save it to last_acked and removed + * previous last_acked */ + llist_del(&tcm->list); + talloc_free(l1h->last_acked); + l1h->last_acked = tcm; + } /* else: tcm was freed by trx_if_flush(), do not access it. */ + + /* Send next message waiting in the list: */ trx_ctrl_send(l1h); return 0; @@ -1224,6 +1240,10 @@ /* Tx queue is now empty, so there's no point in keeping the retrans timer armed: */ osmo_timer_del(&l1h->trx_ctrl_timer); + + /* If we are in read_cb, signal to the returning code path that we freed the list. */ + if (l1h->in_trx_ctrl_read_cb) + l1h->flushed_while_in_trx_ctrl_read_cb = true; } /*! close the TRX for given handle (data + control socket) */ -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ibdffa4644aa3a7d219452644d3e74b411734f1df Gerrit-Change-Number: 32552 Gerrit-PatchSet: 7 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <axilirator(a)gmail.com> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[M] Change in osmo-bts[master]: trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2)
by pespin 25 Oct '23

25 Oct '23

Attention is currently required from: fixeria, fixeria, laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email ) Change subject: trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2) ...................................................................... Patch Set 6: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ibdffa4644aa3a7d219452644d3e74b411734f1df Gerrit-Change-Number: 32552 Gerrit-PatchSet: 6 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <axilirator(a)gmail.com> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: fixeria <axilirator(a)gmail.com> Gerrit-Comment-Date: Wed, 25 Oct 2023 16:00:57 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-bts[master]: Revert "trx_if: Allow calling trx_if_flush/close from within TRXC cal...
by pespin 25 Oct '23

25 Oct '23

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/34894?usp=email ) Change subject: Revert "trx_if: Allow calling trx_if_flush/close from within TRXC callback" ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/34894?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ideb2d08ac8a2902bceeabfb055c59c9a13dbe3c0 Gerrit-Change-Number: 34894 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 25 Oct 2023 16:00:56 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-bts[master]: trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2)
by daniel 25 Oct '23

25 Oct '23

Attention is currently required from: fixeria, fixeria, laforge, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email ) Change subject: trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2) ...................................................................... Patch Set 6: Code-Review+1 (2 comments) File src/osmo-bts-trx/l1_if.h: https://gerrit.osmocom.org/c/osmo-bts/+/32552/comment/2565e7db_0cd7c392 PS5, Line 125: lis > list Done https://gerrit.osmocom.org/c/osmo-bts/+/32552/comment/54d8fa80_a1011157 PS5, Line 126: wile > while Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ibdffa4644aa3a7d219452644d3e74b411734f1df Gerrit-Change-Number: 32552 Gerrit-PatchSet: 6 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: fixeria <axilirator(a)gmail.com> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <axilirator(a)gmail.com> Gerrit-Comment-Date: Wed, 25 Oct 2023 15:51:15 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-bts[master]: trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2)
by pespin 25 Oct '23

25 Oct '23

Attention is currently required from: fixeria, fixeria, laforge, pespin. pespin has uploaded a new patch set (#6) to the change originally created by fixeria. ( https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email ) The following approvals got outdated and were removed: Code-Review+1 by fixeria, Verified+1 by Jenkins Builder Change subject: trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2) ...................................................................... trx_if: Allow calling trx_if_flush/close from within TRXC callback (v2) - If the llist is flushed during rx rsp callback, when the flow is returned to trx_ctrl_read_cb() it would access tcm which was in the llist and end up in use-after-free. - We need to store state on whether code path is inside the read_cb in order to: -- Delay transmission of new message if callback calls trx_if_flush() followed by trx_ctrl_send(), since the trx_ctrl_send() at the end of trx_ctrl_read_cb would retransmit it again immediatelly. -- Avoid accessing tcm pointer if the callback called trx_if_flush(), since it has been freed. Related: OS#6020 Change-Id: Ibdffa4644aa3a7d219452644d3e74b411734f1df --- M src/osmo-bts-trx/l1_if.h M src/osmo-bts-trx/trx_if.c 2 files changed, 52 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/52/32552/6 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/32552?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ibdffa4644aa3a7d219452644d3e74b411734f1df Gerrit-Change-Number: 32552 Gerrit-PatchSet: 6 Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <axilirator(a)gmail.com> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <axilirator(a)gmail.com> Gerrit-MessageType: newpatchset

1 0

[S] Change in pysim[master]: pySim-shell: don't get trapped in applications without file system
by dexter 25 Oct '23

25 Oct '23

Attention is currently required from: laforge, lynxis lazus. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/34884?usp=email ) Change subject: pySim-shell: don't get trapped in applications without file system ...................................................................... Patch Set 2: (1 comment) File pySim-shell.py: https://gerrit.osmocom.org/c/pysim/+/34884/comment/e371d6fe_e4a0b750 PS2, Line 620: "a0000000871002", "a0000000871004"]: > I like the approach in general, but I think we shouldn't open-code those AIDs explicitly here. […] I have now changed the code so that it does no longer use the hardcoded identifiers. Also not "ADF.USIM" or "ADF.ISIM". (I still need to add ADF.HPSIM) -- To view, visit https://gerrit.osmocom.org/c/pysim/+/34884?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Ia2fdd65f430c07acb1afdaf265d24c6928b654e0 Gerrit-Change-Number: 34884 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 25 Oct 2023 15:08:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Gerrit-MessageType: comment

1 0

[S] Change in pysim[master]: pySim-shell: don't get trapped in applications without file system
by dexter 25 Oct '23

25 Oct '23

Attention is currently required from: laforge, lynxis lazus. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/pysim/+/34884?usp=email ) Change subject: pySim-shell: don't get trapped in applications without file system ...................................................................... Patch Set 3: (1 comment) File pySim-shell.py: https://gerrit.osmocom.org/c/pysim/+/34884/comment/7362d555_a4f43514 PS2, Line 618: # > you're mixing python-style space indent with tabs here. Please avoid tabs in python source. Done -- To view, visit https://gerrit.osmocom.org/c/pysim/+/34884?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: Ia2fdd65f430c07acb1afdaf265d24c6928b654e0 Gerrit-Change-Number: 34884 Gerrit-PatchSet: 3 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 25 Oct 2023 15:04:58 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Gerrit-MessageType: comment

1 0

2026

2025

2024

2023

2022

gerrit-log