Change in osmo-pcu[master]: pdch: rcv_resource_request(): Clarify tbf_free only needed if MS used...

2020-05-14 Thread pespin 
pespin has submitted this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/18223 )

Change subject: pdch: rcv_resource_request(): Clarify tbf_free only needed if 
MS used to exist beforehand
..

pdch: rcv_resource_request(): Clarify tbf_free only needed if MS used to exist 
beforehand

Variable found is used to always call Guard() on MS to avoid possible
unexpected freeing regressions.

Change-Id: I62f24fe04ca10fca19bedda288fe3ed3ce75413f
---
M src/pdch.cpp
1 file changed, 25 insertions(+), 23 deletions(-)

Approvals:
  laforge: Looks good to me, but someone else must approve
  neels: Looks good to me, approved
  Jenkins Builder: Verified



diff --git a/src/pdch.cpp b/src/pdch.cpp
index 149ca1f..048577d 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -548,38 +548,41 @@
struct gprs_rlcmac_sba *sba;

if (request->ID.UnionType) {
-   struct gprs_rlcmac_ul_tbf *ul_tbf = NULL;
-   struct gprs_rlcmac_dl_tbf *dl_tbf = NULL;
+   struct gprs_rlcmac_ul_tbf *ul_tbf;
+   struct gprs_rlcmac_dl_tbf *dl_tbf;
uint32_t tlli = request->ID.u.TLLI;
uint8_t ta = GSM48_TA_INVALID;
+   bool found = true;

GprsMs *ms = bts()->ms_by_tlli(tlli);
-   if (!ms)
+   if (!ms) {
+   found = false;
ms = bts()->ms_alloc(0, 0); /* ms class updated later */
+   }

/* Keep the ms, even if it gets idle temporarily */
GprsMs::Guard guard(ms);
-   ul_tbf = ms->ul_tbf();
-   dl_tbf = ms->dl_tbf();
-   ta = ms->ta();

-   /* We got a RACH so the MS was in packet idle mode and thus
-* didn't have any active TBFs */
-   if (ul_tbf) {
-   LOGPTBFUL(ul_tbf, LOGL_NOTICE,
- "Got RACH from TLLI=0x%08x while TBF still 
exists. Killing pending UL TBF\n",
- tlli);
-   tbf_free(ul_tbf);
-   ul_tbf = NULL;
+   if (found) {
+   ul_tbf = ms->ul_tbf();
+   dl_tbf = ms->dl_tbf();
+   ta = ms->ta();
+   /* We got a RACH so the MS was in packet idle mode and 
thus
+* didn't have any active TBFs */
+   if (ul_tbf) {
+   LOGPTBFUL(ul_tbf, LOGL_NOTICE,
+ "Got RACH from TLLI=0x%08x while TBF 
still exists. Killing pending UL TBF\n",
+ tlli);
+   tbf_free(ul_tbf);
+   }
+   if (dl_tbf) {
+   LOGPTBFUL(dl_tbf, LOGL_NOTICE,
+ "Got RACH from TLLI=0x%08x while TBF 
still exists. Release pending DL TBF\n",
+ tlli);
+   tbf_free(dl_tbf);
+   }
}

-   if (dl_tbf) {
-   LOGPTBFUL(dl_tbf, LOGL_NOTICE,
- "Got RACH from TLLI=0x%08x while TBF still 
exists. Release pending DL TBF\n",
- tlli);
-   tbf_free(dl_tbf);
-   dl_tbf = NULL;
-   }
LOGP(DRLCMAC, LOGL_DEBUG, "MS requests UL TBF "
"in packet resource request of single "
"block, so we provide one:\n");
@@ -606,12 +609,11 @@
if (!ms->ms_class())
LOGP(DRLCMAC, LOGL_NOTICE, "MS does not give us a 
class.\n");
if (ms->egprs_ms_class())
-   LOGP(DRLCMAC, LOGL_NOTICE,
+   LOGP(DRLCMAC, LOGL_INFO,
"MS supports EGPRS multislot class %d.\n",
ms->egprs_ms_class());

ul_tbf = tbf_alloc_ul(bts_data(), ms, trx_no(), tlli, ta);
-
if (!ul_tbf) {
handle_tbf_reject(bts_data(), ms, tlli,
trx_no(), ts_no);

--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/18223
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I62f24fe04ca10fca19bedda288fe3ed3ce75413f
Gerrit-Change-Number: 18223
Gerrit-PatchSet: 2
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Reviewer: pespin 
Gerrit-MessageType: merged

Change in osmo-pcu[master]: pdch: rcv_resource_request(): Clarify tbf_free only needed if MS used...

2020-05-13 Thread neels 
neels has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/18223 )

Change subject: pdch: rcv_resource_request(): Clarify tbf_free only needed if 
MS used to exist beforehand
..


Patch Set 1: Code-Review+2


--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/18223
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I62f24fe04ca10fca19bedda288fe3ed3ce75413f
Gerrit-Change-Number: 18223
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Reviewer: neels 
Gerrit-Comment-Date: Wed, 13 May 2020 12:35:00 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in osmo-pcu[master]: pdch: rcv_resource_request(): Clarify tbf_free only needed if MS used...

2020-05-12 Thread laforge 
laforge has posted comments on this change. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/18223 )

Change subject: pdch: rcv_resource_request(): Clarify tbf_free only needed if 
MS used to exist beforehand
..


Patch Set 1: Code-Review+1


--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/18223
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I62f24fe04ca10fca19bedda288fe3ed3ce75413f
Gerrit-Change-Number: 18223
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge 
Gerrit-Comment-Date: Tue, 12 May 2020 17:59:24 +
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment

Change in osmo-pcu[master]: pdch: rcv_resource_request(): Clarify tbf_free only needed if MS used...

2020-05-12 Thread pespin 
pespin has uploaded this change for review. ( 
https://gerrit.osmocom.org/c/osmo-pcu/+/18223 )


Change subject: pdch: rcv_resource_request(): Clarify tbf_free only needed if 
MS used to exist beforehand
..

pdch: rcv_resource_request(): Clarify tbf_free only needed if MS used to exist 
beforehand

Variable found is used to always call Guard() on MS to avoid possible
unexpected freeing regressions.

Change-Id: I62f24fe04ca10fca19bedda288fe3ed3ce75413f
---
M src/pdch.cpp
1 file changed, 25 insertions(+), 23 deletions(-)



  git pull ssh://gerrit.osmocom.org:29418/osmo-pcu refs/changes/23/18223/1

diff --git a/src/pdch.cpp b/src/pdch.cpp
index 149ca1f..048577d 100644
--- a/src/pdch.cpp
+++ b/src/pdch.cpp
@@ -548,38 +548,41 @@
struct gprs_rlcmac_sba *sba;

if (request->ID.UnionType) {
-   struct gprs_rlcmac_ul_tbf *ul_tbf = NULL;
-   struct gprs_rlcmac_dl_tbf *dl_tbf = NULL;
+   struct gprs_rlcmac_ul_tbf *ul_tbf;
+   struct gprs_rlcmac_dl_tbf *dl_tbf;
uint32_t tlli = request->ID.u.TLLI;
uint8_t ta = GSM48_TA_INVALID;
+   bool found = true;

GprsMs *ms = bts()->ms_by_tlli(tlli);
-   if (!ms)
+   if (!ms) {
+   found = false;
ms = bts()->ms_alloc(0, 0); /* ms class updated later */
+   }

/* Keep the ms, even if it gets idle temporarily */
GprsMs::Guard guard(ms);
-   ul_tbf = ms->ul_tbf();
-   dl_tbf = ms->dl_tbf();
-   ta = ms->ta();

-   /* We got a RACH so the MS was in packet idle mode and thus
-* didn't have any active TBFs */
-   if (ul_tbf) {
-   LOGPTBFUL(ul_tbf, LOGL_NOTICE,
- "Got RACH from TLLI=0x%08x while TBF still 
exists. Killing pending UL TBF\n",
- tlli);
-   tbf_free(ul_tbf);
-   ul_tbf = NULL;
+   if (found) {
+   ul_tbf = ms->ul_tbf();
+   dl_tbf = ms->dl_tbf();
+   ta = ms->ta();
+   /* We got a RACH so the MS was in packet idle mode and 
thus
+* didn't have any active TBFs */
+   if (ul_tbf) {
+   LOGPTBFUL(ul_tbf, LOGL_NOTICE,
+ "Got RACH from TLLI=0x%08x while TBF 
still exists. Killing pending UL TBF\n",
+ tlli);
+   tbf_free(ul_tbf);
+   }
+   if (dl_tbf) {
+   LOGPTBFUL(dl_tbf, LOGL_NOTICE,
+ "Got RACH from TLLI=0x%08x while TBF 
still exists. Release pending DL TBF\n",
+ tlli);
+   tbf_free(dl_tbf);
+   }
}

-   if (dl_tbf) {
-   LOGPTBFUL(dl_tbf, LOGL_NOTICE,
- "Got RACH from TLLI=0x%08x while TBF still 
exists. Release pending DL TBF\n",
- tlli);
-   tbf_free(dl_tbf);
-   dl_tbf = NULL;
-   }
LOGP(DRLCMAC, LOGL_DEBUG, "MS requests UL TBF "
"in packet resource request of single "
"block, so we provide one:\n");
@@ -606,12 +609,11 @@
if (!ms->ms_class())
LOGP(DRLCMAC, LOGL_NOTICE, "MS does not give us a 
class.\n");
if (ms->egprs_ms_class())
-   LOGP(DRLCMAC, LOGL_NOTICE,
+   LOGP(DRLCMAC, LOGL_INFO,
"MS supports EGPRS multislot class %d.\n",
ms->egprs_ms_class());

ul_tbf = tbf_alloc_ul(bts_data(), ms, trx_no(), tlli, ta);
-
if (!ul_tbf) {
handle_tbf_reject(bts_data(), ms, tlli,
trx_no(), ts_no);

--
To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/18223
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings

Gerrit-Project: osmo-pcu
Gerrit-Branch: master
Gerrit-Change-Id: I62f24fe04ca10fca19bedda288fe3ed3ce75413f
Gerrit-Change-Number: 18223
Gerrit-PatchSet: 1
Gerrit-Owner: pespin 
Gerrit-MessageType: newchange