[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
arehbein has submitted this change. (
https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email )
Change subject: write_queue: Enable updating max_length field
..
write_queue: Enable updating max_length field
Dequeue and free any excess messages, in case the new queue length
is shorter than the old.
Related: OS#5774
Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002
---
M include/osmocom/core/write_queue.h
M src/core/libosmocore.map
M src/core/write_queue.c
M tests/write_queue/wqueue_test.c
4 files changed, 87 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
fixeria: Looks good to me, but someone else must approve
arehbein: Looks good to me, approved
pespin: Looks good to me, but someone else must approve
diff --git a/include/osmocom/core/write_queue.h
b/include/osmocom/core/write_queue.h
index 6cb0a6b..fe76282 100644
--- a/include/osmocom/core/write_queue.h
+++ b/include/osmocom/core/write_queue.h
@@ -50,6 +50,7 @@
void osmo_wqueue_clear(struct osmo_wqueue *queue);
int osmo_wqueue_enqueue(struct osmo_wqueue *queue, struct msgb *data);
int osmo_wqueue_enqueue_quiet(struct osmo_wqueue *queue, struct msgb *data);
+size_t osmo_wqueue_set_maxlen(struct osmo_wqueue *queue, unsigned int len);
int osmo_wqueue_bfd_cb(struct osmo_fd *fd, unsigned int what);
/*! @} */
diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map
index c0e164b..30814c3 100644
--- a/src/core/libosmocore.map
+++ b/src/core/libosmocore.map
@@ -565,6 +565,7 @@
osmo_wqueue_clear;
osmo_wqueue_enqueue;
osmo_wqueue_enqueue_quiet;
+osmo_wqueue_set_maxlen;
osmo_wqueue_init;
rate_ctr_add;
rate_ctr_difference;
diff --git a/src/core/write_queue.c b/src/core/write_queue.c
index 884cebd..ffa7704 100644
--- a/src/core/write_queue.c
+++ b/src/core/write_queue.c
@@ -147,4 +147,24 @@
queue->bfd.when &= ~OSMO_FD_WRITE;
}
+/* Update write queue length & drop excess messages.
+ * \param[in] queue linked list header of message queue
+ * \param[in] len new max. wqueue length
+ * \returns Number of messages dropped.
+ *
+ * Messages beyond the new maximum message queue size will be dropped.
+ */
+size_t osmo_wqueue_set_maxlen(struct osmo_wqueue *queue, unsigned int len)
+{
+ size_t dropped_msgs = 0;
+ struct msgb *msg;
+ queue->max_length = len;
+ while (queue->current_length > queue->max_length) {
+ msg = msgb_dequeue_count(>msg_queue,
>current_length);
+ msgb_free(msg);
+ dropped_msgs++;
+ }
+ return dropped_msgs;
+}
+
/*! @} */
diff --git a/tests/write_queue/wqueue_test.c b/tests/write_queue/wqueue_test.c
index 3823ef5..d4476f1 100644
--- a/tests/write_queue/wqueue_test.c
+++ b/tests/write_queue/wqueue_test.c
@@ -1,3 +1,14 @@
+/*
+ * (C) 2023 by sysmocom - s.f.m.c. GmbH.
+ * Authors: Holger Hans Peter Freyther
+ * Alexander Rehbein
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ */
+
#include
#include
#include
@@ -15,6 +26,7 @@
struct msgb *msg;
struct osmo_wqueue wqueue;
int rc;
+ size_t dropped_msgs;
osmo_wqueue_init(, 0);
OSMO_ASSERT(wqueue.max_length == 0);
@@ -63,6 +75,46 @@
OSMO_ASSERT(wqueue.current_length == 2);
msgb_free(msg);
osmo_wqueue_clear();
+
+ /* Update limit */
+ OSMO_ASSERT(osmo_wqueue_set_maxlen(, 5) == 0);
+ OSMO_ASSERT(osmo_wqueue_set_maxlen(, 1) == 0);
+ OSMO_ASSERT(osmo_wqueue_set_maxlen(, 4) == 0);
+
+ /* Add three, update limit to 1 */
+ OSMO_ASSERT(wqueue.max_length == 4);
+ msg = msgb_alloc(4096, "msg6");
+ rc = osmo_wqueue_enqueue(, msg);
+ OSMO_ASSERT(rc == 0);
+ OSMO_ASSERT(wqueue.current_length == 1);
+ msg = msgb_alloc(4096, "msg7");
+ rc = osmo_wqueue_enqueue(, msg);
+ OSMO_ASSERT(rc == 0);
+ OSMO_ASSERT(wqueue.current_length == 2);
+ msg = msgb_alloc(4096, "msg8");
+ rc = osmo_wqueue_enqueue(, msg);
+ OSMO_ASSERT(wqueue.current_length == 3);
+ dropped_msgs = osmo_wqueue_set_maxlen(, 1);
+ OSMO_ASSERT(dropped_msgs == 2);
+ osmo_wqueue_clear();
+
+ /* Add three, reduce limit to 3 from 6 */
+ OSMO_ASSERT(osmo_wqueue_set_maxlen(, 6) == 0);
+ OSMO_ASSERT(wqueue.max_length == 6);
+ msg = msgb_alloc(4096, "msg9");
+ rc = osmo_wqueue_enqueue(, msg);
+ OSMO_ASSERT(rc == 0);
+ OSMO_ASSERT(wqueue.current_length == 1);
+ msg = msgb_alloc(4096, "msg10");
+ rc = osmo_wqueue_enqueue(, msg);
+ OSMO_ASSERT(rc == 0);
+ OSMO_ASSERT(wqueue.current_length == 2);
+ msg = msgb_alloc(4096, "msg11");
+ rc = osmo_wqueue_enqueue(, msg);
+
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: dexter, fixeria, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email ) Change subject: write_queue: Enable updating max_length field .. Patch Set 5: Code-Review+2 (2 comments) File src/core/write_queue.c: https://gerrit.osmocom.org/c/libosmocore/+/3/comment/923878c4_ec8e962b PS4, Line 153: * \returns Number of messages dropped. > I would prefer to leave it like this, if that's alright. […] Done https://gerrit.osmocom.org/c/libosmocore/+/3/comment/da14a911_3835276a PS4, Line 165: dropped_msgs++; > It's not going to work if you do this after the while-loop, because the > `queue->current_length` woul […] Done -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: arehbein Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: fixeria Gerrit-Attention: dexter Gerrit-Comment-Date: Tue, 19 Sep 2023 18:45:31 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: arehbein Comment-In-Reply-To: pespin Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: arehbein, dexter. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email ) Change subject: write_queue: Enable updating max_length field .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: dexter Gerrit-Comment-Date: Mon, 18 Sep 2023 18:45:44 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: arehbein, dexter, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email ) Change subject: write_queue: Enable updating max_length field .. Patch Set 5: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-Comment-Date: Mon, 18 Sep 2023 18:43:57 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: dexter, fixeria, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email ) Change subject: write_queue: Enable updating max_length field .. Patch Set 5: (2 comments) File src/core/write_queue.c: https://gerrit.osmocom.org/c/libosmocore/+/3/comment/fa459290_e53c7636 PS4, Line 153: * \returns Number of messages dropped. > I'd return an int, so we can add error codes later on. […] I would prefer to leave it like this, if that's alright. I don't really see how we would add error codes in the future if none of the functions involved are written to return any errors. https://gerrit.osmocom.org/c/libosmocore/+/3/comment/6a075dc4_a2668f0b PS4, Line 157: size_t osmo_wqueue_update_maxlen(struct osmo_wqueue *queue, size_t len) > Let's call this "osmo_wqueue_set_maxlen()". […] Done -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-Comment-Date: Mon, 18 Sep 2023 18:38:01 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: arehbein, dexter, fixeria, pespin. Hello Jenkins Builder, dexter, fixeria, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email to look at the new patch set (#5). The following approvals got outdated and were removed: Code-Review+1 by fixeria, Verified+1 by Jenkins Builder Change subject: write_queue: Enable updating max_length field .. write_queue: Enable updating max_length field Dequeue and free any excess messages, in case the new queue length is shorter than the old. Related: OS#5774 Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 --- M include/osmocom/core/write_queue.h M src/core/libosmocore.map M src/core/write_queue.c M tests/write_queue/wqueue_test.c 4 files changed, 87 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/44/3/5 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: arehbein, dexter, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email ) Change subject: write_queue: Enable updating max_length field .. Patch Set 4: Code-Review+1 (1 comment) File src/core/write_queue.c: https://gerrit.osmocom.org/c/libosmocore/+/3/comment/8710d00b_82692e84 PS4, Line 165: dropped_msgs++; > No need to increment the var N times, simply: […] It's not going to work if you do this after the while-loop, because the `queue->current_length` would always be less or equal to `len`. I see nothing wrong with incrementing the variable. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-Comment-Date: Mon, 18 Sep 2023 17:21:36 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: dexter, fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email ) Change subject: write_queue: Enable updating max_length field .. Patch Set 4: (3 comments) File src/core/write_queue.c: https://gerrit.osmocom.org/c/libosmocore/+/3/comment/1375ee89_5241c1c9 PS4, Line 153: * \returns Number of messages dropped. I'd return an int, so we can add error codes later on. "ssize_t" maybe, so add possibility of a bigger count. https://gerrit.osmocom.org/c/libosmocore/+/3/comment/d53f9bd1_38d2bfdf PS4, Line 157: size_t osmo_wqueue_update_maxlen(struct osmo_wqueue *queue, size_t len) Let's call this "osmo_wqueue_set_maxlen()". queue->max_length is a unsigned int iirc, so why using size_t len above? https://gerrit.osmocom.org/c/libosmocore/+/3/comment/c1a4393b_6779c349 PS4, Line 165: dropped_msgs++; No need to increment the var N times, simply: if (queue->current_length > len) return queue->current_length - len; else return 0; -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: fixeria Gerrit-Attention: dexter Gerrit-Comment-Date: Mon, 18 Sep 2023 15:56:14 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: dexter, fixeria, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email ) Change subject: write_queue: Enable updating max_length field .. Patch Set 4: (2 comments) File src/core/write_queue.c: https://gerrit.osmocom.org/c/libosmocore/+/3/comment/612963d2_25e0991b PS3, Line 161: queue->current_length > queue->max_length > This condition can never be true because you're updating the > `queue->max_length` after the loop, not […] Done File tests/write_queue/wqueue_test.c: https://gerrit.osmocom.org/c/libosmocore/+/3/comment/3dca4dd2_33cd74ca PS3, Line 98: OSMO_ASSERT > Somehow this `assert()` does not assert at all, no matter how I change the > expected value. […] Ah heck, I fixed it locally for the previous test code I added before uploading and somehow managed to recode this error right again *facepalms*. Thanks -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-Comment-Date: Mon, 18 Sep 2023 15:53:13 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: arehbein, dexter, fixeria, pespin. Hello Jenkins Builder, dexter, fixeria, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Code-Review-1 by fixeria, Verified+1 by Jenkins Builder Change subject: write_queue: Enable updating max_length field .. write_queue: Enable updating max_length field Dequeue and free any excess messages, in case the new queue length is shorter than the old. Related: OS#5774 Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 --- M include/osmocom/core/write_queue.h M src/core/libosmocore.map M src/core/write_queue.c M tests/write_queue/wqueue_test.c 4 files changed, 87 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/44/3/4 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: arehbein, dexter, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email ) Change subject: write_queue: Enable updating max_length field .. Patch Set 3: Code-Review-1 (2 comments) File src/core/write_queue.c: https://gerrit.osmocom.org/c/libosmocore/+/3/comment/e8b97450_647808f4 PS3, Line 161: queue->current_length > queue->max_length This condition can never be true because you're updating the `queue->max_length` after the loop, not before. So now this function would not remove any items at all. Do you have a unit test for the scenario of removing exceeding items? Oh, you do. See my other comment then. File tests/write_queue/wqueue_test.c: https://gerrit.osmocom.org/c/libosmocore/+/3/comment/a0b9fb44_a447ccb2 PS3, Line 98: OSMO_ASSERT Somehow this `assert()` does not assert at all, no matter how I change the expected value. This is weird. If I add a `printf()` here, I clearly see that the amount of dropped messages is 0, as expected with the current implementation (see my other comment). Ah, well... I see now. You're doing `=` instead of `==`! -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 3 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-Comment-Date: Mon, 18 Sep 2023 07:02:48 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: dexter, fixeria, pespin. Hello Jenkins Builder, dexter, fixeria, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email to look at the new patch set (#3). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: write_queue: Enable updating max_length field .. write_queue: Enable updating max_length field Dequeue and free any excess messages, in case the new queue length is shorter than the old. Related: OS#5774 Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 --- M include/osmocom/core/write_queue.h M src/core/libosmocore.map M src/core/write_queue.c M tests/write_queue/wqueue_test.c 4 files changed, 87 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/44/3/3 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 3 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: dexter, fixeria, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email ) Change subject: write_queue: Enable updating max_length field .. Patch Set 2: (1 comment) File src/core/write_queue.c: https://gerrit.osmocom.org/c/libosmocore/+/3/comment/5ba3308e_873a6e9d PS1, Line 163: dropped_msgs < diff > Imagine the following situation: the user wants to reduce the queue limit > (`max_length`) from 128 to […] You're absolutely right, thanks. Got lost while coding... -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-Comment-Date: Sun, 17 Sep 2023 21:49:34 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria Gerrit-MessageType: comment
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: arehbein, dexter, fixeria, pespin. Hello Jenkins Builder, dexter, fixeria, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review-1 by fixeria, Verified+1 by Jenkins Builder Change subject: write_queue: Enable updating max_length field .. write_queue: Enable updating max_length field Dequeue and free any excess messages, in case the new queue length is shorter than the old. Related: OS#5774 Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 --- M include/osmocom/core/write_queue.h M src/core/libosmocore.map M src/core/write_queue.c M tests/write_queue/wqueue_test.c 4 files changed, 86 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/44/3/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-MessageType: newpatchset
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: arehbein, dexter, pespin.
fixeria has posted comments on this change. (
https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email )
Change subject: write_queue: Enable updating max_length field
..
Patch Set 1: Code-Review-1
(1 comment)
File src/core/write_queue.c:
https://gerrit.osmocom.org/c/libosmocore/+/3/comment/da9d2090_f795f7c1
PS1, Line 163: dropped_msgs < diff
Imagine the following situation: the user wants to reduce the queue limit
(`max_length`) from 128 to 64. The queue contains 32 items (`current_length`).
```
int diff = queue->max_length - len; // 128 - 64 == 64
queue->max_length = len; // 64
for (dropped_msgs = 0; dropped_msgs < 64 && !llist_empty(q); dropped_msgs++)
```
so IIUC, this function would dequeue and free these 32 items, despite their
count does not exceed the new limit (64). This looks wrong to me. Another
problem is that you're not updating the `current_length` in this function at
all.
I suggest the following:
```
while (queue->current_length > queue->max_length) {
msg = msgb_dequeue_count(>msg_queue, >current_length);
msgb_free(msg);
dropped_msgs++;
}
```
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002
Gerrit-Change-Number: 3
Gerrit-PatchSet: 1
Gerrit-Owner: arehbein
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: pespin
Gerrit-Attention: arehbein
Gerrit-Attention: pespin
Gerrit-Attention: dexter
Gerrit-Comment-Date: Sun, 17 Sep 2023 14:35:58 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
Attention is currently required from: dexter, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email ) Change subject: write_queue: Enable updating max_length field .. Patch Set 1: (1 comment) Patchset: PS1: For reference, this patch was created so this problem here can be tackled: https://gerrit.osmocom.org/c/osmo-bsc/+/33892/comment/cca3c42d_b9504858/ -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002 Gerrit-Change-Number: 3 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: dexter Gerrit-Comment-Date: Sat, 16 Sep 2023 19:31:02 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in libosmocore[master]: write_queue: Enable updating max_length field
arehbein has uploaded this change for review. (
https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email )
Change subject: write_queue: Enable updating max_length field
..
write_queue: Enable updating max_length field
Dequeue and free any excess messages, in case the new queue length
is shorter than the old.
Related: OS#5774
Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002
---
M include/osmocom/core/write_queue.h
M src/core/libosmocore.map
M src/core/write_queue.c
M tests/write_queue/wqueue_test.c
4 files changed, 56 insertions(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/44/3/1
diff --git a/include/osmocom/core/write_queue.h
b/include/osmocom/core/write_queue.h
index 6cb0a6b..0d658af 100644
--- a/include/osmocom/core/write_queue.h
+++ b/include/osmocom/core/write_queue.h
@@ -50,6 +50,7 @@
void osmo_wqueue_clear(struct osmo_wqueue *queue);
int osmo_wqueue_enqueue(struct osmo_wqueue *queue, struct msgb *data);
int osmo_wqueue_enqueue_quiet(struct osmo_wqueue *queue, struct msgb *data);
+size_t osmo_wqueue_update_maxlen(struct osmo_wqueue *queue, size_t len);
int osmo_wqueue_bfd_cb(struct osmo_fd *fd, unsigned int what);
/*! @} */
diff --git a/src/core/libosmocore.map b/src/core/libosmocore.map
index c0e164b..4a9d7bc 100644
--- a/src/core/libosmocore.map
+++ b/src/core/libosmocore.map
@@ -565,6 +565,7 @@
osmo_wqueue_clear;
osmo_wqueue_enqueue;
osmo_wqueue_enqueue_quiet;
+osmo_wqueue_update_maxlen;
osmo_wqueue_init;
rate_ctr_add;
rate_ctr_difference;
diff --git a/src/core/write_queue.c b/src/core/write_queue.c
index 884cebd..7557c7b 100644
--- a/src/core/write_queue.c
+++ b/src/core/write_queue.c
@@ -147,4 +147,22 @@
queue->bfd.when &= ~OSMO_FD_WRITE;
}
+/* Update write queue length & drop excess messages.
+ * \param[in] queue linked list header of message queue
+ * \param[in] len new max. wqueue length
+ * \returns Number of messages dropped.
+ *
+ * Messages beyond the new maximum message queue size will be dropped.
+ */
+size_t osmo_wqueue_update_maxlen(struct osmo_wqueue *queue, size_t len)
+{
+ size_t dropped_msgs;
+ struct llist_head *q = >msg_queue;
+ int diff = queue->max_length - len;
+ queue->max_length = len;
+ for (dropped_msgs = 0; dropped_msgs < diff && !llist_empty(q);
dropped_msgs++)
+ msgb_free(msgb_dequeue(q));
+ return dropped_msgs;
+}
+
/*! @} */
diff --git a/tests/write_queue/wqueue_test.c b/tests/write_queue/wqueue_test.c
index 3823ef5..f82a34a 100644
--- a/tests/write_queue/wqueue_test.c
+++ b/tests/write_queue/wqueue_test.c
@@ -15,6 +15,7 @@
struct msgb *msg;
struct osmo_wqueue wqueue;
int rc;
+ size_t dropped_msgs;
osmo_wqueue_init(, 0);
OSMO_ASSERT(wqueue.max_length == 0);
@@ -63,6 +64,28 @@
OSMO_ASSERT(wqueue.current_length == 2);
msgb_free(msg);
osmo_wqueue_clear();
+
+ /* Update limit */
+ OSMO_ASSERT(osmo_wqueue_update_maxlen(, 5) == 0);
+ OSMO_ASSERT(osmo_wqueue_update_maxlen(, 1) == 0);
+ OSMO_ASSERT(osmo_wqueue_update_maxlen(, 4) == 0);
+
+ /* Add three, update limit to 1 */
+ OSMO_ASSERT(wqueue.max_length == 4);
+ msg = msgb_alloc(4096, "msg6");
+ rc = osmo_wqueue_enqueue(, msg);
+ OSMO_ASSERT(rc == 0);
+ OSMO_ASSERT(wqueue.current_length == 1);
+ msg = msgb_alloc(4096, "msg7");
+ rc = osmo_wqueue_enqueue(, msg);
+ OSMO_ASSERT(rc == 0);
+ OSMO_ASSERT(wqueue.current_length == 2);
+ msg = msgb_alloc(4096, "msg8");
+ rc = osmo_wqueue_enqueue(, msg);
+ OSMO_ASSERT(wqueue.current_length == 3);
+ dropped_msgs = osmo_wqueue_update_maxlen(, 1);
+ OSMO_ASSERT(dropped_msgs = 2);
+ osmo_wqueue_clear();
}
int main(int argc, char **argv)
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/3?usp=email
To unsubscribe, or for help writing mail filters, visit
https://gerrit.osmocom.org/settings
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: Ibfe51a2faf29f8ae160a9c330c9af0d09b5a9002
Gerrit-Change-Number: 3
Gerrit-PatchSet: 1
Gerrit-Owner: arehbein
Gerrit-MessageType: newchange
