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

[S] Change in osmo-bsc[master]: ctrl: Remove unused vty includes

by matanp

matanp has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/35744?usp=email ) Change subject: ctrl: Remove unused vty includes ...................................................................... ctrl: Remove unused vty includes Change-Id: Ib464d0be9df839740a648ff6747130c18d33455d --- M src/osmo-bsc/bts_ctrl.c M src/osmo-bsc/bts_trx_ctrl.c 2 files changed, 9 insertions(+), 6 deletions(-) Approvals: pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified fixeria: Looks good to me, approved diff --git a/src/osmo-bsc/bts_ctrl.c b/src/osmo-bsc/bts_ctrl.c index 9c14190..eac01bf 100644 --- a/src/osmo-bsc/bts_ctrl.c +++ b/src/osmo-bsc/bts_ctrl.c @@ -24,9 +24,6 @@ #include <osmocom/ctrl/control_cmd.h> -#include <osmocom/vty/command.h> -#include <osmocom/vty/misc.h> - #include <osmocom/bsc/ctrl.h> #include <osmocom/bsc/osmo_bsc_rf.h> #include <osmocom/bsc/bts.h> diff --git a/src/osmo-bsc/bts_trx_ctrl.c b/src/osmo-bsc/bts_trx_ctrl.c index ff58cb7..258203c 100644 --- a/src/osmo-bsc/bts_trx_ctrl.c +++ b/src/osmo-bsc/bts_trx_ctrl.c @@ -24,9 +24,6 @@ #include <osmocom/ctrl/control_cmd.h> -#include <osmocom/vty/command.h> -#include <osmocom/vty/misc.h> - #include <osmocom/bsc/ctrl.h> #include <osmocom/bsc/osmo_bsc_rf.h> #include <osmocom/bsc/bts.h> -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/35744?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: Ib464d0be9df839740a648ff6747130c18d33455d Gerrit-Change-Number: 35744 Gerrit-PatchSet: 2 Gerrit-Owner: matanp <matan1008(a)gmail.com> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: matanp <matan1008(a)gmail.com> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 year, 5 months

1
0
0 0

[S] Change in ...osmo-epdg[master]: epdg_ue_fsm: Introduce public function to get pid from imsi

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35896?usp=email ) Change subject: epdg_ue_fsm: Introduce public function to get pid from imsi ...................................................................... epdg_ue_fsm: Introduce public function to get pid from imsi Change-Id: Ibc4478d8d48171c302daf8aaebd037227d3e3a98 --- M src/epdg_ue_fsm.erl 1 file changed, 20 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-epdg refs/changes/96/35896/1 diff --git a/src/epdg_ue_fsm.erl b/src/epdg_ue_fsm.erl index 94cf9c4..711f695 100644 --- a/src/epdg_ue_fsm.erl +++ b/src/epdg_ue_fsm.erl @@ -39,6 +39,7 @@ -export([start_link/1, stop/1]). -export([init/1,callback_mode/0,terminate/3]). +-export([get_server_name_by_imsi/1, get_pid_by_imsi/1]). -export([auth_request/1, lu_request/1, tunnel_request/1, purge_ms_request/1]). -export([received_swm_auth_response/2, received_swm_auth_compl_response/2, received_swm_session_termination_answer/2]). -export([received_gtpc_create_session_response/2, received_gtpc_delete_session_response/2, received_gtpc_delete_bearer_request/1]). @@ -52,10 +53,18 @@ tear_down_gsup_cause = 0 :: integer() }). -start_link(Imsi) -> +get_server_name_by_imsi(Imsi) -> ServerName = lists:concat([?NAME, "_", binary_to_list(Imsi)]), + list_to_atom(ServerName). + +get_pid_by_imsi(Imsi) -> + ServerName = get_server_name_by_imsi(Imsi), + whereis(ServerName). + +start_link(Imsi) -> + ServerName = get_server_name_by_imsi(Imsi), lager:info("ue_fsm start_link(~p)~n", [ServerName]), - gen_statem:start_link({local, list_to_atom(ServerName)}, ?MODULE, Imsi, [{debug, [trace]}]). + gen_statem:start_link({local, ServerName}, ?MODULE, Imsi, [{debug, [trace]}]). stop(SrvRef) -> gen_statem:stop(SrvRef). -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35896?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: Ibc4478d8d48171c302daf8aaebd037227d3e3a98 Gerrit-Change-Number: 35896 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[S] Change in ...osmo-epdg[master]: gsup_server: Delete session if initial auth failed

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35892?usp=email ) Change subject: gsup_server: Delete session if initial auth failed ...................................................................... gsup_server: Delete session if initial auth failed It may be that due to a bug the previous session was kept alive and the related epdg_ue_fsm has actually already terminated. In any case, there's no use in keeping the session at this initial transaction after having created it, so simply delete it if it fails. Change-Id: Iebbab786e9647a026b2937115004747cd1883da0 --- M src/epdg_ue_fsm.erl M src/gsup_server.erl 2 files changed, 23 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-epdg refs/changes/92/35892/1 diff --git a/src/epdg_ue_fsm.erl b/src/epdg_ue_fsm.erl index 21f59da..94cf9c4 100644 --- a/src/epdg_ue_fsm.erl +++ b/src/epdg_ue_fsm.erl @@ -37,7 +37,7 @@ -include_lib("osmo_gsup/include/gsup_protocol.hrl"). -include_lib("gtplib/include/gtp_packet.hrl"). --export([start_link/1]). +-export([start_link/1, stop/1]). -export([init/1,callback_mode/0,terminate/3]). -export([auth_request/1, lu_request/1, tunnel_request/1, purge_ms_request/1]). -export([received_swm_auth_response/2, received_swm_auth_compl_response/2, received_swm_session_termination_answer/2]). @@ -57,6 +57,9 @@ lager:info("ue_fsm start_link(~p)~n", [ServerName]), gen_statem:start_link({local, list_to_atom(ServerName)}, ?MODULE, Imsi, [{debug, [trace]}]). +stop(SrvRef) -> + gen_statem:stop(SrvRef). + auth_request(Pid) -> lager:info("ue_fsm auth_request~n", []), try diff --git a/src/gsup_server.erl b/src/gsup_server.erl index 0c7a723..fd2a8b5 100644 --- a/src/gsup_server.erl +++ b/src/gsup_server.erl @@ -234,7 +234,7 @@ handle_info({ipa, Socket, ?IPAC_PROTO_EXT_GSUP, _GsupMsgRx = #{message_type := send_auth_info_req, imsi := Imsi}}, State0) -> {UE, State1} = find_or_new_gsups_ue(Imsi, State0), case epdg_ue_fsm:auth_request(UE#gsups_ue.pid) of - ok -> ok; + ok -> State2 = State1; {error, Err} -> lager:error("Auth Req for Imsi ~p failed: ~p~n", [Imsi, Err]), Resp = #{message_type => send_auth_info_err, @@ -242,9 +242,11 @@ message_class => 5, cause => ?GSUP_CAUSE_NET_FAIL }, - tx_gsup(Socket, Resp) + tx_gsup(Socket, Resp), + epdg_ue_fsm:stop(UE#gsups_ue.pid), + State2 = delete_gsups_ue(Imsi, State1) end, - {noreply, State1}; + {noreply, State2}; % location update request / when a UE wants to connect to a specific APN. This will trigger a AAA->HLR Request Server Assignment Request % FIXME: add APN instead of hardcoded internet -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35892?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: Iebbab786e9647a026b2937115004747cd1883da0 Gerrit-Change-Number: 35892 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[M] Change in ...osmo-epdg[master]: AAA-Server: s6b: Fetch UE FSM Pid without asking SWm module

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35893?usp=email ) Change subject: AAA-Server: s6b: Fetch UE FSM Pid without asking SWm module ...................................................................... AAA-Server: s6b: Fetch UE FSM Pid without asking SWm module Instead, keep the implementation in the UE FSM module itself, and take advantage of the fact that the PID is registered and can be derived from the IMSI. Change-Id: I0e62f3715da61d2d8cfabaf89557aeb1cd95f89a --- M src/aaa_diameter_s6b_cb.erl M src/aaa_diameter_swm.erl M src/aaa_ue_fsm.erl 3 files changed, 34 insertions(+), 26 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-epdg refs/changes/93/35893/1 diff --git a/src/aaa_diameter_s6b_cb.erl b/src/aaa_diameter_s6b_cb.erl index 339f735..7e3915d 100644 --- a/src/aaa_diameter_s6b_cb.erl +++ b/src/aaa_diameter_s6b_cb.erl @@ -59,15 +59,15 @@ 'Auth-Request-Type' = AuthReqType, 'User-Name' = [UserName], 'Service-Selection' = [Apn]} = Req, - Result = aaa_diameter_swm:get_ue_fsm_by_imsi(UserName), - case Result of - {ok, Pid} -> - ok = aaa_ue_fsm:ev_rx_s6b_aar(Pid, Apn), + PidRes = aaa_ue_fsm:get_pid_by_imsi(UserName), + case PidRes of + PidRes when is_pid(PidRes) -> + ok = aaa_ue_fsm:ev_rx_s6b_aar(PidRes, Apn), lager:debug("Waiting for S6b AAA~n", []), receive {aaa, ResultCode} -> lager:debug("Rx AAA with ResultCode=~p~n", [ResultCode]) end; - _ -> lager:error("Error looking up FSM for IMSI~n", [UserName]), + undefined -> lager:error("Error looking up FSM for IMSI~n", [UserName]), ResultCode = ?'RULE-FAILURE-CODE_CM_AUTHORIZATION_REJECTED' end, Resp = #'AAA'{'Session-Id'= SessionId, @@ -88,10 +88,10 @@ 'Auth-Application-Id' = _AuthAppId, 'Termination-Cause' = _TermCause, 'User-Name' = [UserName]} = Req, - Result = aaa_diameter_swm:get_ue_fsm_by_imsi(UserName), - case Result of - {ok, Pid} -> - case aaa_ue_fsm:ev_rx_s6b_str(Pid) of + PidRes = aaa_ue_fsm:get_pid_by_imsi(UserName), + case PidRes of + PidRes when is_pid(PidRes) -> + case aaa_ue_fsm:ev_rx_s6b_str(PidRes) of ok -> lager:debug("Waiting for S6b STA~n", []), receive @@ -104,7 +104,7 @@ {error, _} -> ResultCode = ?'RULE-FAILURE-CODE_CM_AUTHORIZATION_REJECTED' end; - _ -> lager:error("Error looking up FSM for IMSI~n", [UserName]), + undefined -> lager:error("Error looking up FSM for IMSI~n", [UserName]), ResultCode = ?'RULE-FAILURE-CODE_CM_AUTHORIZATION_REJECTED' end, % 3GPP TS 29.273 9.2.2.3.2 Session-Termination-Answer (STA) Command: diff --git a/src/aaa_diameter_swm.erl b/src/aaa_diameter_swm.erl index bcedcac..ee3a8cb 100644 --- a/src/aaa_diameter_swm.erl +++ b/src/aaa_diameter_swm.erl @@ -19,7 +19,6 @@ -export([start_link/0]). -export([init/1, handle_call/3, handle_cast/2, handle_info/2]). -export([code_change/3, terminate/2]). --export([get_ue_fsm_by_imsi/1]). -export([auth_request/1, auth_compl_request/2, session_termination_request/1]). -export([auth_response/2, auth_compl_response/2, session_termination_answer/2]). @@ -33,9 +32,6 @@ TableId = ets:new(auth_req, [bag, named_table]), {ok, #swm_state{table_id = TableId}}. -get_ue_fsm_by_imsi(Imsi) -> - _Result = gen_server:call(?SERVER, {get_ue_fsm_by_imsi, Imsi}). - %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% % Tx over emulated SWm wire: %%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%%% @@ -101,16 +97,6 @@ error_logger:error_report(["unknown handle_info", {module, ?MODULE}, {info, Info}, {state, S}]), {noreply, S}. -handle_call({get_ue_fsm_by_imsi, Imsi}, _From, State) -> - Sess = find_swm_session_by_imsi(Imsi, State), - lager:debug("find_swm_session_by_imsi(~p) returned ~p~n", [Imsi, Sess]), - case Sess of - #swm_session{} -> - {reply, {ok ,Sess#swm_session.pid}, State}; - undefined -> - {reply, {error, imsi_unknown}, State} - end; - handle_call({epdg_auth_resp, Imsi, Result}, _From, State) -> epdg_diameter_swm:auth_response(Imsi, Result), {reply, ok, State}; diff --git a/src/aaa_ue_fsm.erl b/src/aaa_ue_fsm.erl index 52918d2..5cdb553 100644 --- a/src/aaa_ue_fsm.erl +++ b/src/aaa_ue_fsm.erl @@ -40,6 +40,7 @@ -export([start_link/1]). -export([init/1,callback_mode/0,terminate/3]). +-export([get_server_name_by_imsi/1, get_pid_by_imsi/1]). -export([ev_swm_auth_req/1, ev_swm_auth_compl/2, ev_rx_swm_str/1, ev_rx_swx_maa/2, ev_rx_swx_saa/2, ev_rx_s6b_aar/2, ev_rx_s6b_str/1]). -export([state_new/3, state_wait_swx_maa/3, state_wait_swx_saa/3, state_authenticated/3, state_authenticated_wait_swx_saa/3]). @@ -52,10 +53,18 @@ s6b_resp_pid :: pid() }). -start_link(Imsi) -> +get_server_name_by_imsi(Imsi) -> ServerName = lists:concat([?NAME, "_", Imsi]), + list_to_atom(ServerName). + +get_pid_by_imsi(Imsi) -> + ServerName = get_server_name_by_imsi(Imsi), + whereis(ServerName). + +start_link(Imsi) -> + ServerName = get_server_name_by_imsi(Imsi), lager:info("ue_fsm start_link(~p)~n", [ServerName]), - gen_statem:start_link({local, list_to_atom(ServerName)}, ?MODULE, Imsi, [{debug, [trace]}]). + gen_statem:start_link({local, ServerName}, ?MODULE, Imsi, [{debug, [trace]}]). ev_swm_auth_req(Pid) -> lager:info("ue_fsm ev_swm_auth_req~n", []), -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35893?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: I0e62f3715da61d2d8cfabaf89557aeb1cd95f89a Gerrit-Change-Number: 35893 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[S] Change in ...osmo-epdg[master]: gsup_server: Log error if epdg_ue_fsm:auth_request() fails

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35891?usp=email ) Change subject: gsup_server: Log error if epdg_ue_fsm:auth_request() fails ...................................................................... gsup_server: Log error if epdg_ue_fsm:auth_request() fails Change-Id: Iea5cc5736d8de9d652c15d9bc12ab16833c1ae7b --- M src/gsup_server.erl 1 file changed, 11 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-epdg refs/changes/91/35891/1 diff --git a/src/gsup_server.erl b/src/gsup_server.erl index 5c536fc..0c7a723 100644 --- a/src/gsup_server.erl +++ b/src/gsup_server.erl @@ -235,7 +235,8 @@ {UE, State1} = find_or_new_gsups_ue(Imsi, State0), case epdg_ue_fsm:auth_request(UE#gsups_ue.pid) of ok -> ok; - {error, _} -> + {error, Err} -> + lager:error("Auth Req for Imsi ~p failed: ~p~n", [Imsi, Err]), Resp = #{message_type => send_auth_info_err, imsi => Imsi, message_class => 5, -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35891?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: Iea5cc5736d8de9d652c15d9bc12ab16833c1ae7b Gerrit-Change-Number: 35891 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[S] Change in ...osmo-epdg[master]: gsup_server: Fix removal of session from set

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35894?usp=email ) Change subject: gsup_server: Fix removal of session from set ...................................................................... gsup_server: Fix removal of session from set Change-Id: Iec9bce7494e3849c9e94158516f471f97182725b --- M src/gsup_server.erl 1 file changed, 22 insertions(+), 6 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-epdg refs/changes/94/35894/1 diff --git a/src/gsup_server.erl b/src/gsup_server.erl index fd2a8b5..8a070bf 100644 --- a/src/gsup_server.erl +++ b/src/gsup_server.erl @@ -198,7 +198,7 @@ } end, tx_gsup(Socket, Resp), - State1 = delete_gsups_ue(Imsi, State0), + State1 = delete_gsups_ue_by_imsi(Imsi, State0), {noreply, State1}; % Our GSUP CEAI implementation for "IKEv2 Information Delete Request" @@ -244,7 +244,7 @@ }, tx_gsup(Socket, Resp), epdg_ue_fsm:stop(UE#gsups_ue.pid), - State2 = delete_gsups_ue(Imsi, State1) + State2 = delete_gsups_ue(UE, State1) end, {noreply, State2}; @@ -331,7 +331,7 @@ lager:info("GSUP: Rx ~p~n", [GsupMsgRx]), UE = find_gsups_ue_by_imsi(Imsi, State0), case UE of - #gsups_ue{imsi = Imsi} -> State1 = delete_gsups_ue(Imsi, State0); + #gsups_ue{imsi = Imsi} -> State1 = delete_gsups_ue(UE, State0); undefined -> State1 = State0 end, {noreply, State1}; @@ -401,6 +401,13 @@ new_gsups_ue(Imsi, State) end. -delete_gsups_ue(Imsi, State) -> - SetRemoved = sets:del_element(Imsi, State#gsups_state.ues), - State#gsups_state{ues = SetRemoved}. \ No newline at end of file +delete_gsups_ue(UE, State) -> + SetRemoved = sets:del_element(UE, State#gsups_state.ues), + lager:debug("Removed UE ~p from ~p~n", [UE, SetRemoved]), + State#gsups_state{ues = SetRemoved}. + +delete_gsups_ue_by_imsi(Imsi, State) -> + case find_gsups_ue_by_imsi(Imsi, State) of + undefined -> State; + UE-> delete_gsups_ue(UE, State) + end. \ No newline at end of file -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35894?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: Iec9bce7494e3849c9e94158516f471f97182725b Gerrit-Change-Number: 35894 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[S] Change in ...osmo-epdg[master]: epdg_diameter_swm: Update PID if session already exists

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35895?usp=email ) Change subject: epdg_diameter_swm: Update PID if session already exists ...................................................................... epdg_diameter_swm: Update PID if session already exists If the FSM was recreated, during the first step it may have a different PID. Update it so that we send the answer back to the proper PID. Change-Id: I5e4f0ff4292c364f47d61ba629df50e8e3d35017 --- M src/epdg_diameter_swm.erl 1 file changed, 22 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/erlang/osmo-epdg refs/changes/95/35895/1 diff --git a/src/epdg_diameter_swm.erl b/src/epdg_diameter_swm.erl index 57c0921..800df44 100644 --- a/src/epdg_diameter_swm.erl +++ b/src/epdg_diameter_swm.erl @@ -181,7 +181,15 @@ Sess = find_swm_session_by_imsi(Imsi, State), case Sess of #swm_session{imsi = Imsi} -> - {Sess, State}; + % Update Pid since it may have changed: + Sess1 = Sess#swm_session{pid = Pid}, + State1 = update_swm_session(Sess, Sess1, State), + {Sess1, State1}; undefined -> new_swm_session(Imsi, Pid, State) - end. \ No newline at end of file + end. + +update_swm_session(OldSess, NewSess, State) -> + SetRemoved = sets:del_element(OldSess, State#swm_state.sessions), + SetUpdated = sets:add_element(NewSess, SetRemoved), + State#swm_state{sessions = SetUpdated}. -- To view, visit https://gerrit.osmocom.org/c/erlang/osmo-epdg/+/35895?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: I5e4f0ff4292c364f47d61ba629df50e8e3d35017 Gerrit-Change-Number: 35895 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 5 months

1
0
0 0

[M] Change in libosmo-gprs[master]: tbf_ul: Clean and assert code path generating new UL data block

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/35882?usp=email ) Change subject: tbf_ul: Clean and assert code path generating new UL data block ...................................................................... tbf_ul: Clean and assert code path generating new UL data block Clarify the several paths when generating a new BSN (UL data block). The function create_new_bsn() is only called if there's some LLC frame ready to be transmitted and put in ul_tbf->llc_tx_msg, so the do while {} condition checking for no llc_tx_msg is guaranteed to only be run in the event some LLC frame chunks have been included in the UL RLC block. This fact allows simplifying the code and at the same time fix a Coverity CID regarding a code path which could not really happen. We now guard this case with an ASSERT. Related: Coverity CID#345063 Change-Id: I6d4ae377a60839f76295c0e7e6aaa1fe53e37315 --- M src/rlcmac/tbf_ul.c 1 file changed, 37 insertions(+), 15 deletions(-) Approvals: jolly: Looks good to me, but someone else must approve fixeria: Looks good to me, but someone else must approve Jenkins Builder: Verified pespin: Looks good to me, approved diff --git a/src/rlcmac/tbf_ul.c b/src/rlcmac/tbf_ul.c index 0ea0a9c..67c9e98 100644 --- a/src/rlcmac/tbf_ul.c +++ b/src/rlcmac/tbf_ul.c @@ -684,6 +684,7 @@ if (!ul_tbf->llc_tx_msg || msgb_length(ul_tbf->llc_tx_msg) == 0) gprs_rlcmac_ul_tbf_schedule_next_llc_frame(ul_tbf); + /* This function shall only be called if there's some LLC payload not yet transmitted: */ OSMO_ASSERT(ul_tbf->llc_tx_msg); OSMO_ASSERT(gprs_rlcmac_mcs_is_valid(cs)); @@ -734,27 +735,29 @@ int payload_written = 0; if (!ul_tbf->llc_tx_msg || msgb_length(ul_tbf->llc_tx_msg) == 0) { + const int space = block_data_len - write_offset; + /* Only get here in case we already encoded the end of + * some LLC payload chunk and there's no more new LLC + * payloads to send */ + OSMO_ASSERT(num_chunks > 0); + /* The data just drained, store the current fn */ if (ul_tbf->last_ul_drained_fn < 0) ul_tbf->last_ul_drained_fn = bi->fn; - int space = block_data_len - write_offset; + /* Nothing to send, and we already put some data in + * rlcmac data block, we are done */ + LOGPTBFUL(ul_tbf, LOGL_DEBUG, + "LLC queue completely drained and there's " + "still %d free bytes in rlcmac data block\n", space); - if (num_chunks != 0) { - /* Nothing to send, and we already put some data in - * rlcmac data block, we are done */ - LOGPTBFUL(ul_tbf, LOGL_DEBUG, - "LLC queue completely drained and there's " - "still %d free bytes in rlcmac data block\n", space); - - if (gprs_rlcmac_mcs_is_edge(cs)) { - /* in EGPRS there's no M bit, so we need - * to flag padding with LI=127 */ - gprs_rlcmac_rlc_data_to_ul_append_egprs_li_padding( - rdbi, &write_offset, &num_chunks, data); - } - break; + if (gprs_rlcmac_mcs_is_edge(cs)) { + /* in EGPRS there's no M bit, so we need to flag + * padding with LI=127 */ + gprs_rlcmac_rlc_data_to_ul_append_egprs_li_padding( + rdbi, &write_offset, &num_chunks, data); } + break; } ar = gprs_rlcmac_enc_append_ul_data(rdbi, cs, ul_tbf->llc_tx_msg, &write_offset, -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/35882?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I6d4ae377a60839f76295c0e7e6aaa1fe53e37315 Gerrit-Change-Number: 35882 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 year, 5 months

1
0
0 0

[M] Change in libosmo-gprs[master]: tbf_ul: Clean and assert code path generating new UL data block

by pespin

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/35882?usp=email ) Change subject: tbf_ul: Clean and assert code path generating new UL data block ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/35882?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I6d4ae377a60839f76295c0e7e6aaa1fe53e37315 Gerrit-Change-Number: 35882 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Feb 2024 12:16:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 5 months

1
0
0 0

[M] Change in libosmo-gprs[master]: tbf_ul: Clean and assert code path generating new UL data block

by jolly

Attention is currently required from: pespin. jolly has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/35882?usp=email ) Change subject: tbf_ul: Clean and assert code path generating new UL data block ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/35882?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I6d4ae377a60839f76295c0e7e6aaa1fe53e37315 Gerrit-Change-Number: 35882 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 07 Feb 2024 09:47:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 5 months

1
0
0 0

2025

2024

2023

2022

gerrit-log February 2024