[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
daniel has submitted this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email )
Change subject: meas_feed: Use osmo_io instead of write queue
..
meas_feed: Use osmo_io instead of write queue
Related: OS#6170
Change-Id: Ib0570a3242e2846062e24c93cd31137acdee
---
M include/osmocom/bsc/meas_feed.h
M src/osmo-bsc/bsc_vty.c
M src/osmo-bsc/meas_feed.c
M src/osmo-bsc/osmo_bsc_main.c
4 files changed, 54 insertions(+), 54 deletions(-)
Approvals:
fixeria: Looks good to me, but someone else must approve
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
pespin: Looks good to me, approved
diff --git a/include/osmocom/bsc/meas_feed.h b/include/osmocom/bsc/meas_feed.h
index f2bd4ba..447eab8 100644
--- a/include/osmocom/bsc/meas_feed.h
+++ b/include/osmocom/bsc/meas_feed.h
@@ -35,12 +35,12 @@
};
#define MEAS_FEED_VERSION 1
-#define MEAS_FEED_WQUEUE_MAX_LEN_DEFAULT 100
+#define MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT 100
int meas_feed_cfg_set(const char *dst_host, uint16_t dst_port);
void meas_feed_scenario_set(const char *name);
-void meas_feed_wqueue_max_length_set(unsigned int max_length);
+void meas_feed_txqueue_max_length_set(unsigned int max_length);
void meas_feed_cfg_get(char **host, uint16_t *port);
const char *meas_feed_scenario_get(void);
-unsigned int meas_feed_wqueue_max_length_get(void);
+unsigned int meas_feed_txqueue_max_length_get(void);
diff --git a/src/osmo-bsc/bsc_vty.c b/src/osmo-bsc/bsc_vty.c
index d758832..bdc18b6 100644
--- a/src/osmo-bsc/bsc_vty.c
+++ b/src/osmo-bsc/bsc_vty.c
@@ -385,7 +385,7 @@
uint16_t meas_port;
char *meas_host;
const char *meas_scenario;
- unsigned int max_len = meas_feed_wqueue_max_length_get();
+ unsigned int max_len = meas_feed_txqueue_max_length_get();
meas_feed_cfg_get(_host, _port);
meas_scenario = meas_feed_scenario_get();
@@ -396,7 +396,7 @@
if (strlen(meas_scenario) > 0)
vty_out(vty, " meas-feed scenario %s%s",
meas_scenario, VTY_NEWLINE);
- if (max_len != MEAS_FEED_WQUEUE_MAX_LEN_DEFAULT)
+ if (max_len != MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT)
vty_out(vty, " meas-feed write-queue-max-length %u%s",
max_len, VTY_NEWLINE);
}
@@ -2424,7 +2424,7 @@
"Maximum number of messages to be queued waiting for transmission\n",
CMD_ATTR_IMMEDIATE)
{
- meas_feed_wqueue_max_length_set(atoi(argv[0]));
+ meas_feed_txqueue_max_length_set(atoi(argv[0]));
return CMD_SUCCESS;
}
diff --git a/src/osmo-bsc/meas_feed.c b/src/osmo-bsc/meas_feed.c
index 23b7d04..b18478f 100644
--- a/src/osmo-bsc/meas_feed.c
+++ b/src/osmo-bsc/meas_feed.c
@@ -6,7 +6,7 @@
#include
#include
-#include
+#include
#include
#include
@@ -23,17 +23,14 @@
#include
struct meas_feed_state {
- struct osmo_wqueue wqueue;
- unsigned int wqueue_max_len;
+ struct osmo_io_fd *io_fd;
char scenario[31+1];
char *dst_host;
uint16_t dst_port;
+ size_t txqueue_max;
};
-static struct meas_feed_state g_mfs = {
- .wqueue.bfd.fd = -1,
- .wqueue_max_len = MEAS_FEED_WQUEUE_MAX_LEN_DEFAULT,
-};
+static struct meas_feed_state g_mfs = { .txqueue_max =
MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT };
static int process_meas_rep(struct gsm_meas_rep *mr)
{
@@ -41,7 +38,7 @@
struct meas_feed_meas *mfm;
struct bsc_subscr *bsub;
- OSMO_ASSERT(g_mfs.wqueue.bfd.fd != -1);
+ OSMO_ASSERT(g_mfs.io_fd != NULL);
/* ignore measurements as long as we don't know who it is */
if (!mr->lchan) {
@@ -90,7 +87,7 @@
mfm->ss_nr = mr->lchan->nr;
/* and send it to the socket */
- if (osmo_wqueue_enqueue(_mfs.wqueue, msg) != 0) {
+ if (osmo_iofd_write_msgb(g_mfs.io_fd, msg)) {
LOGP(DMEAS, LOGL_ERROR, "meas_feed %s: sending measurement
report failed\n",
gsm_lchan_name(mr->lchan));
msgb_free(msg);
@@ -115,63 +112,54 @@
return 0;
}
-static int feed_write_cb(struct osmo_fd *ofd, struct msgb *msg)
-{
- return write(ofd->fd, msgb_data(msg), msgb_length(msg));
-}
-
-static int feed_read_cb(struct osmo_fd *ofd)
-{
- int rc;
- char buf[256];
-
- rc = read(ofd->fd, buf, sizeof(buf));
- osmo_fd_read_disable(ofd);
-
- return rc;
-}
-
static void meas_feed_close(void)
{
- if (g_mfs.wqueue.bfd.fd == -1)
+ if (g_mfs.io_fd == NULL)
return;
osmo_signal_unregister_handler(SS_LCHAN, meas_feed_sig_cb, NULL);
- osmo_wqueue_clear(_mfs.wqueue);
- osmo_fd_unregister(_mfs.wqueue.bfd);
- close(g_mfs.wqueue.bfd.fd);
- g_mfs.wqueue.bfd.fd = -1;
+
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 7 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: laforge Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 03 Oct 2023 12:43:43 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 7: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 7 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 03 Oct 2023 10:40:20 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, daniel, pespin. fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 7: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 7 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 03 Oct 2023 05:52:36 + Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: daniel, fixeria, pespin.
arehbein has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email )
Change subject: meas_feed: Use osmo_io instead of write queue
..
Patch Set 7:
(3 comments)
File src/osmo-bsc/meas_feed.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/24c2c084_401fadb8
PS5, Line 148: &(struct osmo_io_ops) {
: .read_cb = meas_feed_noop_cb,
: .write_cb = meas_feed_noop_cb,
: .segmentation_cb = NULL,
:}, NULL);
> @vyanitskiy@sysmocom. […]
Done
File src/osmo-bsc/meas_feed.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/374cc3cf_123b50dd
PS6, Line 33: static struct meas_feed_state g_mfs = {.txqueue_max =
MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT};
> I gree with Daniel, we usually use the way he proposed. […]
I hope it's okay with `g_mfs` as a oneliner, I have added the space before and
after.
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/aeea4ec6_0e28a606
PS6, Line 133: struct osmo_io_ops meas_feed_oio = {.read_cb = NULL, .write_cb
= meas_feed_noop_cb,
> Ack
Done
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee
Gerrit-Change-Number: 34526
Gerrit-PatchSet: 7
Gerrit-Owner: arehbein
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: pespin
Gerrit-Attention: pespin
Gerrit-Attention: fixeria
Gerrit-Attention: daniel
Gerrit-Comment-Date: Mon, 02 Oct 2023 19:12:19 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein
Comment-In-Reply-To: fixeria
Comment-In-Reply-To: pespin
Comment-In-Reply-To: daniel
Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, daniel, fixeria. Hello Jenkins Builder, daniel, fixeria, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email to look at the new patch set (#7). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: meas_feed: Use osmo_io instead of write queue .. meas_feed: Use osmo_io instead of write queue Related: OS#6170 Change-Id: Ib0570a3242e2846062e24c93cd31137acdee --- M include/osmocom/bsc/meas_feed.h M src/osmo-bsc/bsc_vty.c M src/osmo-bsc/meas_feed.c M src/osmo-bsc/osmo_bsc_main.c 4 files changed, 54 insertions(+), 54 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/26/34526/7 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 7 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: fixeria Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, daniel, fixeria.
pespin has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email )
Change subject: meas_feed: Use osmo_io instead of write queue
..
Patch Set 6:
(2 comments)
File src/osmo-bsc/meas_feed.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/90a2121e_c07296f8
PS6, Line 33: static struct meas_feed_state g_mfs = {.txqueue_max =
MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT};
> I kind of like using a different style for structs as opposed to the style
> used for function definit […]
I gree with Daniel, we usually use the way he proposed.
For sure at least you should leave some space before/after "{ }"
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/5b57b162_4bfcb0ba
PS6, Line 133: struct osmo_io_ops meas_feed_oio = {.read_cb = NULL, .write_cb
= meas_feed_noop_cb,
> Here as well
Ack
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee
Gerrit-Change-Number: 34526
Gerrit-PatchSet: 6
Gerrit-Owner: arehbein
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: pespin
Gerrit-Attention: arehbein
Gerrit-Attention: fixeria
Gerrit-Attention: daniel
Gerrit-Comment-Date: Mon, 02 Oct 2023 16:50:28 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: arehbein
Comment-In-Reply-To: daniel
Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: daniel, fixeria, pespin.
arehbein has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email )
Change subject: meas_feed: Use osmo_io instead of write queue
..
Patch Set 6:
(3 comments)
File src/osmo-bsc/meas_feed.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/0d553836_798d4cea
PS5, Line 148: &(struct osmo_io_ops) {
: .read_cb = meas_feed_noop_cb,
: .write_cb = meas_feed_noop_cb,
: .segmentation_cb = NULL,
:}, NULL);
> The read callback (if set) also needs to free the msgb, otherwise we leak
> memory. […]
@vyanits...@sysmocom.de I have adapted the code, although I do find it helpful
to see what is effectually being passed when using an unnamed struct.
@dwillm...@sysmocom.de Shouldn't we declare that part about freeing the msg
buff in the comment for the `read_cb` member in
`libosmocore.git:include/osmocom/core/osmo_io.h`?
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/4865ff1a_05d03cae
PS5, Line 156:
meas_feed_txqueue_max_length_set(MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT
> This looks suspicious. I see a few potential problems: […]
Done
File src/osmo-bsc/meas_feed.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/7c481d4b_6a9e9533
PS6, Line 33: static struct meas_feed_state g_mfs = {.txqueue_max =
MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT};
> Not sure if we have preferred way in osmocom, but I'd prefer each initializer
> element on its own lin […]
I kind of like using a different style for structs as opposed to the style used
for function definitions and elements of control flow. If it's not a must, I'd
like to keep it this way
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee
Gerrit-Change-Number: 34526
Gerrit-PatchSet: 6
Gerrit-Owner: arehbein
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: pespin
Gerrit-Attention: fixeria
Gerrit-Attention: pespin
Gerrit-Attention: daniel
Gerrit-Comment-Date: Mon, 02 Oct 2023 16:44:36 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria
Comment-In-Reply-To: daniel
Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, fixeria, pespin.
daniel has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email )
Change subject: meas_feed: Use osmo_io instead of write queue
..
Patch Set 6:
(2 comments)
File src/osmo-bsc/meas_feed.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/8d79aec3_162f3443
PS6, Line 33: static struct meas_feed_state g_mfs = {.txqueue_max =
MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT};
Not sure if we have preferred way in osmocom, but I'd prefer each initializer
element on its own line.
```
static struct meas_feed_state g_mfs = {
.txqueue_max = MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT,
};
```
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/f74ff94f_607a94b9
PS6, Line 133: struct osmo_io_ops meas_feed_oio = {.read_cb = NULL, .write_cb
= meas_feed_noop_cb,
Here as well
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee
Gerrit-Change-Number: 34526
Gerrit-PatchSet: 6
Gerrit-Owner: arehbein
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: pespin
Gerrit-Attention: arehbein
Gerrit-Attention: fixeria
Gerrit-Attention: pespin
Gerrit-Comment-Date: Mon, 02 Oct 2023 16:12:48 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, fixeria, pespin. Hello Jenkins Builder, daniel, fixeria, pespin, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email to look at the new patch set (#6). The following approvals got outdated and were removed: Code-Review-1 by fixeria, Verified+1 by Jenkins Builder Change subject: meas_feed: Use osmo_io instead of write queue .. meas_feed: Use osmo_io instead of write queue Related: OS#6170 Change-Id: Ib0570a3242e2846062e24c93cd31137acdee --- M include/osmocom/bsc/meas_feed.h M src/osmo-bsc/bsc_vty.c M src/osmo-bsc/meas_feed.c M src/osmo-bsc/osmo_bsc_main.c 4 files changed, 51 insertions(+), 54 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/26/34526/6 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 6 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: fixeria Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: fixeria Gerrit-Attention: pespin Gerrit-MessageType: newpatchset
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, fixeria, pespin.
daniel has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email )
Change subject: meas_feed: Use osmo_io instead of write queue
..
Patch Set 5:
(1 comment)
File src/osmo-bsc/meas_feed.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/62ac249e_866f77d2
PS5, Line 148: &(struct osmo_io_ops) {
: .read_cb = meas_feed_noop_cb,
: .write_cb = meas_feed_noop_cb,
: .segmentation_cb = NULL,
:}, NULL);
> Can we have this struct defined outside of the function call? […]
The read callback (if set) also needs to free the msgb, otherwise we leak
memory.
Since I11ce072510b591f7881d09888524426579bd0169 is merged you could also leave
the read_cb NULL.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee
Gerrit-Change-Number: 34526
Gerrit-PatchSet: 5
Gerrit-Owner: arehbein
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: pespin
Gerrit-Attention: arehbein
Gerrit-Attention: fixeria
Gerrit-Attention: pespin
Gerrit-Comment-Date: Mon, 02 Oct 2023 12:37:11 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: fixeria
Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, daniel, pespin.
fixeria has posted comments on this change. (
https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email )
Change subject: meas_feed: Use osmo_io instead of write queue
..
Patch Set 5: Code-Review-1
(2 comments)
File src/osmo-bsc/meas_feed.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/c0b96c80_692b4958
PS5, Line 148: &(struct osmo_io_ops) {
: .read_cb = meas_feed_noop_cb,
: .write_cb = meas_feed_noop_cb,
: .segmentation_cb = NULL,
:}, NULL);
Can we have this struct defined outside of the function call?
IMO, passing a struct pointer like this is not so readable.
https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/b998a2e0_83fb04f5
PS5, Line 156:
meas_feed_txqueue_max_length_set(MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT
This looks suspicious. I see a few potential problems:
* Scenario a): in the config file `meas-feed write-queue-max-length <1-65535>`
goes before `meas-feed destination ADDR <0-65535>`: in this case you're
overwriting the user's setting with `MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT`.
* Scenario b): a user willing to change the destination addr/port entering the
VTY and issuing the `meas-feed destination ADDR <0-65535>` command: in this
case you're again overwriting the user's setting with
`MEAS_FEED_TXQUEUE_MAX_LEN_DEFAULT`.
I think the correct way would be to call:
```
osmo_iofd_set_txqueue_max_length(g_mfs.io_fd, g_mfs.max_length);
```
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee
Gerrit-Change-Number: 34526
Gerrit-PatchSet: 5
Gerrit-Owner: arehbein
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel
Gerrit-Reviewer: fixeria
Gerrit-Reviewer: pespin
Gerrit-Attention: arehbein
Gerrit-Attention: pespin
Gerrit-Attention: daniel
Gerrit-Comment-Date: Mon, 02 Oct 2023 08:29:10 +
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: daniel, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 5: (3 comments) This change is ready for review. Patchset: PS1: > Build tests are passing now; there are some TODO-marked comments that may > need clarification/I still […] Done Patchset: PS4: > I agree that renaming the VTY command in particular is unnecessary and will > cause more fallout than […] Done File src/osmo-bsc/meas_feed.c: https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/8f9d33e8_f83bf043 PS4, Line 117: if (g_mfs.io_fd == NULL) /* TODO: Check if this is the right condition to check if io_fd is initialized */ > ah yeah. Pushing an update to the patch unset the WIP flag that I originally > pushed the patch with. […] Done -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 5 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Sun, 01 Oct 2023 22:36:30 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: pespin Comment-In-Reply-To: daniel Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: daniel, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 4: (1 comment) Patchset: PS4: > I changed it because otherwise the code would be misleading, since we're not > using write queues anym […] *renamed it -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 26 Sep 2023 09:37:19 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: daniel, pespin. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 4: (2 comments) This change is ready for review. Patchset: PS4: > I think you are totally mixing 2 things in this patch: […] I changed it because otherwise the code would be misleading, since we're not using write queues anymore. Hence it's part of the switch from using a write queue to using osmo_io File src/osmo-bsc/meas_feed.c: https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/51bc3cd8_a4f0730e PS4, Line 117: if (g_mfs.io_fd == NULL) /* TODO: Check if this is the right condition to check if io_fd is initialized */ > TODOs ah yeah. Pushing an update to the patch unset the WIP flag that I originally pushed the patch with. It wasn't meant to be active in its current state -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-Attention: pespin Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 26 Sep 2023 09:36:55 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 4: (1 comment) File src/osmo-bsc/meas_feed.c: https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/8f2b49ea_c7b3809a PS4, Line 117: if (g_mfs.io_fd == NULL) /* TODO: Check if this is the right condition to check if io_fd is initialized */ > all these TOOs ofc need to be fixed before this patch can be merged. TODOs -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 26 Sep 2023 09:29:21 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 4: (1 comment) File src/osmo-bsc/meas_feed.c: https://gerrit.osmocom.org/c/osmo-bsc/+/34526/comment/e6e80dce_9cc2373c PS4, Line 117: if (g_mfs.io_fd == NULL) /* TODO: Check if this is the right condition to check if io_fd is initialized */ all these TOOs ofc need to be fixed before this patch can be merged. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 26 Sep 2023 09:25:41 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 4: Code-Review-1 (1 comment) Patchset: PS4: I think you are totally mixing 2 things in this patch: - Renaming wqueue to txqueue - Doing whatever the commit description says. For the first point, I'd really drop it, I see no good point in changing the name there. -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Reviewer: pespin Gerrit-Attention: arehbein Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 26 Sep 2023 09:24:29 + Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: daniel. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 4: (1 comment) Patchset: PS1: > okay, tests show something is probably not yet right with the sockets Build tests are passing now; there are some TODO-marked comments that may need clarification/I still have to run a setup that is configured to send meas reports -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Attention: daniel Gerrit-Comment-Date: Tue, 26 Sep 2023 09:16:16 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, daniel. Hello Jenkins Builder, daniel, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email to look at the new patch set (#4). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: meas_feed: Use osmo_io instead of write queue .. meas_feed: Use osmo_io instead of write queue Related: OS#6170 Change-Id: Ib0570a3242e2846062e24c93cd31137acdee --- M include/osmocom/bsc/meas_feed.h M src/osmo-bsc/bsc_vty.c M src/osmo-bsc/meas_feed.c M src/osmo-bsc/osmo_bsc_main.c 4 files changed, 68 insertions(+), 58 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/26/34526/4 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 4 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Attention: arehbein Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: arehbein, daniel. Hello Jenkins Builder, daniel, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: meas_feed: Use osmo_io instead of write queue .. meas_feed: Use osmo_io instead of write queue Related: OS#6170 Change-Id: Ib0570a3242e2846062e24c93cd31137acdee --- M include/osmocom/bsc/meas_feed.h M src/osmo-bsc/bsc_vty.c M src/osmo-bsc/meas_feed.c M src/osmo-bsc/osmo_bsc_main.c 4 files changed, 69 insertions(+), 58 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/26/34526/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 3 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Attention: arehbein Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: daniel. Hello Jenkins Builder, daniel, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified-1 by Jenkins Builder Change subject: meas_feed: Use osmo_io instead of write queue .. meas_feed: Use osmo_io instead of write queue Related: OS#6170 Change-Id: Ib0570a3242e2846062e24c93cd31137acdee --- M include/osmocom/bsc/meas_feed.h M src/osmo-bsc/bsc_vty.c M src/osmo-bsc/meas_feed.c M src/osmo-bsc/osmo_bsc_main.c 4 files changed, 66 insertions(+), 58 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/26/34526/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Attention: daniel Gerrit-MessageType: newpatchset
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: daniel. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 1: (1 comment) Patchset: PS1: > TODOs: […] okay, tests show something is probably not yet right with the sockets -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel Gerrit-Attention: daniel Gerrit-Comment-Date: Mon, 25 Sep 2023 22:49:09 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein Gerrit-MessageType: comment
[M] Change in osmo-bsc[master]: meas_feed: Use osmo_io instead of write queue
Attention is currently required from: daniel. arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/34526?usp=email ) Change subject: meas_feed: Use osmo_io instead of write queue .. Patch Set 1: (1 comment) This change is ready for review. Patchset: PS1: TODOs: - tests may have to be fixed - one or two open questions (see TODO-comments) - not yet tested -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/34526?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: Ib0570a3242e2846062e24c93cd31137acdee Gerrit-Change-Number: 34526 Gerrit-PatchSet: 1 Gerrit-Owner: arehbein Gerrit-Reviewer: daniel Gerrit-CC: Jenkins Builder Gerrit-Attention: daniel Gerrit-Comment-Date: Mon, 25 Sep 2023 22:45:53 + Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment
