fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmocom-bb/+/32628 )
Change subject: trxcon/l1sched: allocate primitives of fixed size (64 + 64) ...................................................................... trxcon/l1sched: allocate primitives of fixed size (64 + 64) When running trxcon with GSMTAP Um logging enabled (-g cmd line arg), in handle_prim_rach_cnf() we msgb_put() one or two bytes to the given msgb. This causes a segfault, because the L1SCHED_PRIM_T_RACH prims have 0 tailroom bytes available. While we could allocate L1SCHED_PRIM_T_RACH with a few extra bytes, a more fundamental approach is to allocate all l1sched primitives with a fixed tailroom. Change-Id: Ica87b147e11744a69dcd7c056376dcf6b98f9ca6 Fixes: ff9db9de "trxcon/l1sched: rework the primitive API" Related: OS#5500 --- M src/host/trxcon/include/osmocom/bb/l1sched/prim.h M src/host/trxcon/src/sched_lchan_sch.c M src/host/trxcon/src/sched_prim.c M src/host/trxcon/src/sched_trx.c M src/host/trxcon/src/trxcon_fsm.c 5 files changed, 33 insertions(+), 14 deletions(-) Approvals: laforge: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/src/host/trxcon/include/osmocom/bb/l1sched/prim.h b/src/host/trxcon/include/osmocom/bb/l1sched/prim.h index 0981dc9..0575ad8 100644 --- a/src/host/trxcon/include/osmocom/bb/l1sched/prim.h +++ b/src/host/trxcon/include/osmocom/bb/l1sched/prim.h @@ -114,8 +114,7 @@ enum osmo_prim_operation op); struct msgb *l1sched_prim_alloc(enum l1sched_prim_type type, - enum osmo_prim_operation op, - size_t extra_size); + enum osmo_prim_operation op); struct msgb *l1sched_lchan_prim_dequeue(struct l1sched_lchan_state *lchan, uint32_t fn); void l1sched_lchan_prim_assign_dummy(struct l1sched_lchan_state *lchan); diff --git a/src/host/trxcon/src/sched_lchan_sch.c b/src/host/trxcon/src/sched_lchan_sch.c index f433de6..e242005 100644 --- a/src/host/trxcon/src/sched_lchan_sch.c +++ b/src/host/trxcon/src/sched_lchan_sch.c @@ -66,7 +66,7 @@ struct l1sched_prim *prim; struct msgb *msg; - msg = l1sched_prim_alloc(L1SCHED_PRIM_T_SCH, PRIM_OP_INDICATION, 0); + msg = l1sched_prim_alloc(L1SCHED_PRIM_T_SCH, PRIM_OP_INDICATION); OSMO_ASSERT(msg != NULL); prim = l1sched_prim_from_msgb(msg); diff --git a/src/host/trxcon/src/sched_prim.c b/src/host/trxcon/src/sched_prim.c index 5f0ee29..380d5a8 100644 --- a/src/host/trxcon/src/sched_prim.c +++ b/src/host/trxcon/src/sched_prim.c @@ -35,7 +35,8 @@ #include <osmocom/bb/l1sched/l1sched.h> #include <osmocom/bb/l1sched/logging.h> -#define L1SCHED_PRIM_HEADROOM 128 +#define L1SCHED_PRIM_HEADROOM 64 +#define L1SCHED_PRIM_TAILROOM 64 osmo_static_assert(sizeof(struct l1sched_prim) <= L1SCHED_PRIM_HEADROOM, l1sched_prim_size); @@ -61,12 +62,11 @@ } struct msgb *l1sched_prim_alloc(enum l1sched_prim_type type, - enum osmo_prim_operation op, - size_t extra_size) + enum osmo_prim_operation op) { struct msgb *msg; - msg = msgb_alloc_headroom(L1SCHED_PRIM_HEADROOM + extra_size, + msg = msgb_alloc_headroom(L1SCHED_PRIM_HEADROOM + L1SCHED_PRIM_TAILROOM, L1SCHED_PRIM_HEADROOM, "l1sched_prim"); if (msg == NULL) return NULL; @@ -89,7 +89,7 @@ bool cached; /* Allocate a new primitive */ - msg = l1sched_prim_alloc(L1SCHED_PRIM_T_DATA, PRIM_OP_REQUEST, GSM_MACBLOCK_LEN); + msg = l1sched_prim_alloc(L1SCHED_PRIM_T_DATA, PRIM_OP_REQUEST); OSMO_ASSERT(msg != NULL); prim = l1sched_prim_from_msgb(msg); @@ -469,7 +469,7 @@ if (!prim_len) return; - msg = l1sched_prim_alloc(L1SCHED_PRIM_T_DATA, PRIM_OP_REQUEST, prim_len); + msg = l1sched_prim_alloc(L1SCHED_PRIM_T_DATA, PRIM_OP_REQUEST); OSMO_ASSERT(msg != NULL); prim = l1sched_prim_from_msgb(msg); @@ -498,7 +498,7 @@ lchan_desc = &l1sched_lchan_desc[lchan->type]; - msg = l1sched_prim_alloc(L1SCHED_PRIM_T_DATA, PRIM_OP_INDICATION, data_len); + msg = l1sched_prim_alloc(L1SCHED_PRIM_T_DATA, PRIM_OP_INDICATION); OSMO_ASSERT(msg != NULL); prim = l1sched_prim_from_msgb(msg); diff --git a/src/host/trxcon/src/sched_trx.c b/src/host/trxcon/src/sched_trx.c index bcd24f0..ccac32c 100644 --- a/src/host/trxcon/src/sched_trx.c +++ b/src/host/trxcon/src/sched_trx.c @@ -74,7 +74,7 @@ struct l1sched_prim *prim; struct msgb *msg; - msg = l1sched_prim_alloc(L1SCHED_PRIM_T_PCHAN_COMB, PRIM_OP_INDICATION, 0); + msg = l1sched_prim_alloc(L1SCHED_PRIM_T_PCHAN_COMB, PRIM_OP_INDICATION); OSMO_ASSERT(msg != NULL); prim = l1sched_prim_from_msgb(msg); diff --git a/src/host/trxcon/src/trxcon_fsm.c b/src/host/trxcon/src/trxcon_fsm.c index 598f2bd..eabab3d 100644 --- a/src/host/trxcon/src/trxcon_fsm.c +++ b/src/host/trxcon/src/trxcon_fsm.c @@ -286,7 +286,7 @@ struct l1sched_prim *prim; struct msgb *msg; - msg = l1sched_prim_alloc(L1SCHED_PRIM_T_RACH, PRIM_OP_REQUEST, 0); + msg = l1sched_prim_alloc(L1SCHED_PRIM_T_RACH, PRIM_OP_REQUEST); OSMO_ASSERT(msg != NULL); prim = l1sched_prim_from_msgb(msg); @@ -492,7 +492,7 @@ struct l1sched_prim *prim; struct msgb *msg; - msg = l1sched_prim_alloc(L1SCHED_PRIM_T_DATA, PRIM_OP_REQUEST, req->data_len); + msg = l1sched_prim_alloc(L1SCHED_PRIM_T_DATA, PRIM_OP_REQUEST); OSMO_ASSERT(msg != NULL); prim = l1sched_prim_from_msgb(msg); @@ -563,7 +563,7 @@ if (l1gprs_handle_ul_block_req(trxcon->gprs, &block_req, msg) != 0) return; - msg = l1sched_prim_alloc(L1SCHED_PRIM_T_DATA, PRIM_OP_REQUEST, block_req.data_len); + msg = l1sched_prim_alloc(L1SCHED_PRIM_T_DATA, PRIM_OP_REQUEST); OSMO_ASSERT(msg != NULL); prim = l1sched_prim_from_msgb(msg); -- To view, visit https://gerrit.osmocom.org/c/osmocom-bb/+/32628 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmocom-bb Gerrit-Branch: master Gerrit-Change-Id: Ica87b147e11744a69dcd7c056376dcf6b98f9ca6 Gerrit-Change-Number: 32628 Gerrit-PatchSet: 2 Gerrit-Owner: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanits...@sysmocom.de> Gerrit-Reviewer: laforge <lafo...@osmocom.org> Gerrit-Reviewer: osmith <osm...@sysmocom.de> Gerrit-Reviewer: pespin <pes...@sysmocom.de> Gerrit-CC: msuraev <msur...@sysmocom.de> Gerrit-MessageType: merged