[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