pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/40885?usp=email )

Change subject: llc_queue: Store MetaInfo in msgb headroom instead of cb[] ......................................................................

llc_queue: Store MetaInfo in msgb headroom instead of cb[]

"sizeof(*meta_storage) <= sizeof(llc_msg->cb)" assert fails on debian 13 armv7l. Since struct MetaInfo cannot always fit inside msgb->cb, move the struct MetaInfo into the headroom of the msgb.

Related: OS#6831 Change-Id: I98d966c257d50d3b2fc326dce85ead5591acf51f --- M src/gprs_ms.c M src/llc.c M src/llc.h M tests/llc/LlcTest.cpp 4 files changed, 35 insertions(+), 42 deletions(-)

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

diff --git a/src/gprs_ms.c b/src/gprs_ms.c index ff7316b..1577a4a 100644 --- a/src/gprs_ms.c +++ b/src/gprs_ms.c @@ -1369,13 +1369,10 @@

LOGPMS(ms, DTBFDL, LOGL_DEBUG, "appending %u bytes to DL LLC queue\n", len);

- struct msgb *llc_msg = msgb_alloc(len, "llc_pdu_queue"); - if (!llc_msg) - return -ENOMEM; - llc_queue_calc_pdu_lifetime(ms->bts, pdu_delay_csec, &expire_time); - memcpy(msgb_put(llc_msg, len), data, len); - llc_queue_enqueue(ms_llc_queue(ms), llc_msg, &expire_time); + rc = llc_queue_enqueue(ms_llc_queue(ms), data, len, &expire_time); + if (rc < 0) + return rc; ms_start_llc_timer(ms);

dl_tbf = ms_dl_tbf(ms); diff --git a/src/llc.c b/src/llc.c index 633ae49..478d23d 100644 --- a/src/llc.c +++ b/src/llc.c @@ -141,24 +141,34 @@ } }

-void llc_queue_enqueue(struct gprs_llc_queue *q, struct msgb *llc_msg, const struct timespec *expire_time) +int llc_queue_enqueue(struct gprs_llc_queue *q, const uint8_t *data, uint16_t len, const struct timespec *expire_time) { + struct msgb *llc_msg; struct MetaInfo *meta_storage; - struct gprs_llc_hdr *llc_hdr = (struct gprs_llc_hdr *)msgb_data(llc_msg); + struct gprs_llc_hdr *llc_hdr; enum gprs_llc_queue_prio prio;

- osmo_static_assert(sizeof(*meta_storage) <= sizeof(llc_msg->cb), info_does_not_fit); + llc_msg = msgb_alloc_headroom(sizeof(*meta_storage) + len, sizeof(*meta_storage), "llc_pdu_queue"); + if (!llc_msg) + return -ENOMEM;

- prio = llc_sapi2prio(llc_hdr->sapi); - - q->queue_size += 1; - q->queue_octets += msgb_length(llc_msg); - - meta_storage = (struct MetaInfo *)&llc_msg->cb[0]; + /* Fist set up MetaInfo in the msgb headroom: */ + meta_storage = (struct MetaInfo *)llc_msg->head; osmo_clock_gettime(CLOCK_MONOTONIC, &meta_storage->recv_time); meta_storage->expire_time = *expire_time;

+ /* Copy PDU to msgb data: */ + llc_hdr = (struct gprs_llc_hdr *)msgb_put(llc_msg, len); + memcpy(llc_hdr, data, len); + + prio = llc_sapi2prio(llc_hdr->sapi); + + /* Append: */ + q->queue_size += 1; + q->queue_octets += msgb_length(llc_msg); msgb_enqueue(&q->pq[prio].queue, llc_msg); + + return 0; }

void llc_queue_clear(struct gprs_llc_queue *q, struct gprs_rlcmac_bts *bts) @@ -205,8 +215,8 @@ msg = msg1; msg1 = NULL; } else { - const struct MetaInfo *mi1 = (struct MetaInfo *)&msg1->cb[0]; - const struct MetaInfo *mi2 = (struct MetaInfo *)&msg2->cb[0]; + const struct MetaInfo *mi1 = (struct MetaInfo *)msg1->head; + const struct MetaInfo *mi2 = (struct MetaInfo *)msg2->head;

if (timespeccmp(&mi2->recv_time, &mi1->recv_time, >)) { msg = msg1; @@ -236,20 +246,21 @@ /* Prepend / Put back a previously dequeued LLC frame (llc_queue_dequeue()) */ void llc_queue_merge_prepend(struct gprs_llc_queue *q, const struct gprs_llc *llc) { + struct msgb *llc_msg; struct MetaInfo *meta_storage; unsigned int len = llc_frame_length(llc); - struct msgb *llc_msg = msgb_alloc(len, "llc_pdu_queue");

+ llc_msg = msgb_alloc_headroom(sizeof(*meta_storage) + len, sizeof(*meta_storage), "llc_pdu_queue"); OSMO_ASSERT(llc_msg); - memcpy(msgb_put(llc_msg, len), llc->frame, len);

- q->queue_size += 1; - q->queue_octets += msgb_length(llc_msg); - - meta_storage = (struct MetaInfo *)&llc_msg->cb[0]; + meta_storage = (struct MetaInfo *)llc_msg->head; memcpy(meta_storage, &llc->meta_info, sizeof(struct MetaInfo));

+ memcpy(msgb_put(llc_msg, len), llc->frame, len); + /* Prepend: */ + q->queue_size += 1; + q->queue_octets += msgb_length(llc_msg); llist_add(&llc_msg->list, &q->pq[llc->prio].queue); }

@@ -272,7 +283,7 @@ if (!msg) return NULL;

- meta_storage = (struct MetaInfo *)&msg->cb[0]; + meta_storage = (struct MetaInfo *)msg->head;

q->queue_size -= 1; q->queue_octets -= msgb_length(msg); @@ -305,7 +316,7 @@ timespecadd(&tv_now, &hyst_delta, &tv_now2);

while ((msg = llc_queue_pick_msg(q, &prio))) { - info = (const struct MetaInfo *)&msg->cb[0]; + info = (const struct MetaInfo *)msg->head; const struct timespec *tv_disc = &info->expire_time; const struct timespec *tv_recv = &info->recv_time;

diff --git a/src/llc.h b/src/llc.h index b9f3180..31e427d 100644 --- a/src/llc.h +++ b/src/llc.h @@ -139,7 +139,7 @@ void llc_queue_set_codel_interval(struct gprs_llc_queue *q, unsigned int interval); void llc_queue_move_and_merge(struct gprs_llc_queue *q, struct gprs_llc_queue *o); void llc_queue_merge_prepend(struct gprs_llc_queue *q, const struct gprs_llc *llc); -void llc_queue_enqueue(struct gprs_llc_queue *q, struct msgb *llc_msg, const struct timespec *expire_time); +int llc_queue_enqueue(struct gprs_llc_queue *q, const uint8_t *data, uint16_t len, const struct timespec *expire_time); struct msgb *llc_queue_dequeue(struct gprs_llc_queue *q, enum gprs_llc_queue_prio *out_prio, struct MetaInfo *out_info);

static inline size_t llc_queue_size(const struct gprs_llc_queue *q) diff --git a/tests/llc/LlcTest.cpp b/tests/llc/LlcTest.cpp index 9ed6f21..c0101fb 100644 --- a/tests/llc/LlcTest.cpp +++ b/tests/llc/LlcTest.cpp @@ -56,21 +56,6 @@ return ms_llc_queue(ms); }

-static void enqueue_data(gprs_llc_queue *queue, const uint8_t *data, size_t len, - const struct timespec *expire_time) -{ - struct timespec *tv; - uint8_t *msg_data; - struct msgb *llc_msg = msgb_alloc(len + sizeof(*tv) * 2, - "llc_pdu_queue"); - - msg_data = (uint8_t *)msgb_put(llc_msg, len); - - memcpy(msg_data, data, len); - - llc_queue_enqueue(queue, llc_msg, expire_time); -} - static void dequeue_and_check(gprs_llc_queue *queue, const uint8_t *exp_data, size_t len, const MetaInfo *exp_info) { @@ -96,7 +81,7 @@ static void enqueue_data(gprs_llc_queue *queue, const char *message, const struct timespec *expire_time) { - enqueue_data(queue, (uint8_t *)(message), strlen(message), expire_time); + llc_queue_enqueue(queue, (uint8_t *)(message), strlen(message), expire_time); }

static void dequeue_and_check(gprs_llc_queue *queue, const char *exp_message,