July 2023 - gerrit-log - lists.osmocom.org

[S] Change in docker-playground[master]: ttcn3-bts-test/jenkins.sh: set mp_pcuif_version
by dexter 03 Jul '23

03 Jul '23

Attention is currently required from: pespin. dexter has posted comments on this change. ( https://gerrit.osmocom.org/c/docker-playground/+/33521 ) Change subject: ttcn3-bts-test/jenkins.sh: set mp_pcuif_version ...................................................................... Patch Set 2: (1 comment) File ttcn3-bts-test/jenkins.sh: https://gerrit.osmocom.org/c/docker-playground/+/33521/comment/a0e8b557_9e5… PS1, Line 130: image_suffix_is_master > if image_suffix_is_master; then […] Done -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/33521 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: Ia28bc0d6d3cbfe63be19443db86631fb67bb80fb Gerrit-Change-Number: 33521 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Mon, 03 Jul 2023 09:07:56 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in docker-playground[master]: ttcn3-bts-test/jenkins.sh: set mp_pcuif_version
by dexter 03 Jul '23

03 Jul '23

Attention is currently required from: pespin. Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/docker-playground/+/33521 to look at the new patch set (#2). Change subject: ttcn3-bts-test/jenkins.sh: set mp_pcuif_version ...................................................................... ttcn3-bts-test/jenkins.sh: set mp_pcuif_version OsmoBTS currently uses PCUIF v.10 but will move to v.11 soon. (see Depends). Unfortuantely this means that we have to execute the TTCN3 testsuite in master with PCUIF v.11 and in latest with PCUIF v.10. This will be the case until the current master becomes the new latest on the next release. Depends: osmo-bts.git I25816ac12e63cc6b641eb414e6bc7eaa9c85fc25 Depends: osmo-ttcn3-hacks.git I08de02e951e10bc8b4381cc2ad32e63f2747e3c4 Change-Id: Ia28bc0d6d3cbfe63be19443db86631fb67bb80fb Related: OS#5927 --- M ttcn3-bts-test/generic/BTS_Tests.cfg M ttcn3-bts-test/jenkins.sh M ttcn3-bts-test/oml/BTS_Tests.cfg M ttcn3-bts-test/virtphy/BTS_Tests.cfg 4 files changed, 35 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/docker-playground refs/changes/21/33521/2 -- To view, visit https://gerrit.osmocom.org/c/docker-playground/+/33521 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: docker-playground Gerrit-Branch: master Gerrit-Change-Id: Ia28bc0d6d3cbfe63be19443db86631fb67bb80fb Gerrit-Change-Number: 33521 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in osmo-dev[master]: src/grd: Add option for cherry-picking
by osmith 03 Jul '23

03 Jul '23

Attention is currently required from: arehbein, neels, pespin, fixeria. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-dev/+/33381 ) Change subject: src/grd: Add option for cherry-picking ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-dev/+/33381 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-dev Gerrit-Branch: master Gerrit-Change-Id: I85b1a2c4915e3da374e4b1201f2e977708fc7c4c Gerrit-Change-Number: 33381 Gerrit-PatchSet: 2 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Mon, 03 Jul 2023 08:59:49 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream: Notify stream_cli on connect()
by pespin 03 Jul '23

03 Jul '23

Attention is currently required from: daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33542 ) Change subject: stream: Notify stream_cli on connect() ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33542 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I29621ca53cdbdf8b5b2d128307fcb6432db669d3 Gerrit-Change-Number: 33542 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Mon, 03 Jul 2023 08:08:46 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-bts[master]: common: Make socket queue max. length configurable
by pespin 03 Jul '23

03 Jul '23

Attention is currently required from: arehbein, laforge, fixeria, msuraev. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31534 ) Change subject: common: Make socket queue max. length configurable ...................................................................... Patch Set 17: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31534 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Id6ba6e4eadce9ce82ef2407f4e28346e7fe4abfa Gerrit-Change-Number: 31534 Gerrit-PatchSet: 17 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: msuraev <msuraev(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Attention: msuraev <msuraev(a)sysmocom.de> Gerrit-Comment-Date: Mon, 03 Jul 2023 08:08:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue
by pespin 03 Jul '23

03 Jul '23

Attention is currently required from: arehbein. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31533 ) Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... Patch Set 13: (1 comment) Patchset: PS2: > So I don't know if anything should be done/what should be done. As mentioned, osmo-bsc has the exact same problem as osmo-bts, so you need to apply a similar patch. osmo_io is an entire separate topic which at current pace may take some time to hit all code in osmo-bsc, so let's port this patch to osmo-bsc which should be pretty quick. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 13 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Mon, 03 Jul 2023 08:05:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue
by arehbein 03 Jul '23

03 Jul '23

arehbein has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31533 ) Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... Patch Set 13: (1 comment) Patchset: PS2: > BTW, once merged, remember to submit a similar fix in osmo-bsc.git (it also has a pcu_sock). […] I reran the setup two times, respectively pausing either osmo-bsc or osmo-pcu, and didn't encounter any increase in memory usage (only had to adapt the osmo-bsc config to add `role sg` as well as `sctp-role client` in order to get it to start, and osmo-pcu was complaining about an outdated PCU IF, but I suppose that didn't break the PCU IF communication). I also vaguely remember mentioning making a similar change to the other components using the PCU IF, with the response being that we would be using osmo io soon anyways, thus eliminating the need for osmo_wqueues doing the processing. So I don't know if anything should be done/what should be done. -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 13 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Sun, 02 Jul 2023 21:45:02 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[S] Change in libosmo-netif[master]: stream: Notify stream_cli on connect()
by laforge 02 Jul '23

02 Jul '23

Attention is currently required from: daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33542 ) Change subject: stream: Notify stream_cli on connect() ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33542 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I29621ca53cdbdf8b5b2d128307fcb6432db669d3 Gerrit-Change-Number: 33542 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Sun, 02 Jul 2023 17:54:31 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: osmo_io: Add osmo_iofd_notify_connected()
by laforge 02 Jul '23

02 Jul '23

Attention is currently required from: daniel. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/33497 ) Change subject: osmo_io: Add osmo_iofd_notify_connected() ...................................................................... Patch Set 4: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/33497 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ieed10bc94c8aad821c0a8f7764db0e05c054c1e3 Gerrit-Change-Number: 33497 Gerrit-PatchSet: 4 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Sun, 02 Jul 2023 17:53:21 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in osmo-bts[master]: common: Have PCU socket connection use osmo_wqueue
by arehbein 02 Jul '23

02 Jul '23

arehbein has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/31533 ) Change subject: common: Have PCU socket connection use osmo_wqueue ...................................................................... common: Have PCU socket connection use osmo_wqueue Fixes memleak in case of connected PCU process being suspended without proper close on socket Related: OS#5774 Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 --- M include/osmo-bts/bts.h M src/common/pcu_sock.c 2 files changed, 50 insertions(+), 53 deletions(-) Approvals: laforge: Looks good to me, approved fixeria: Looks good to me, but someone else must approve pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified diff --git a/include/osmo-bts/bts.h b/include/osmo-bts/bts.h index 62017d7..dcd8459 100644 --- a/include/osmo-bts/bts.h +++ b/include/osmo-bts/bts.h @@ -137,6 +137,8 @@ char *addr; }; +#define BTS_PCU_SOCK_WQUEUE_LEN_DEFAULT 10 + /* One BTS */ struct gsm_bts { /* list header in g_bts_sm->bts_list */ diff --git a/src/common/pcu_sock.c b/src/common/pcu_sock.c index 16048e2..33b6740 100644 --- a/src/common/pcu_sock.c +++ b/src/common/pcu_sock.c @@ -31,6 +31,7 @@ #include <osmocom/core/utils.h> #include <osmocom/core/select.h> #include <osmocom/core/socket.h> +#include <osmocom/core/write_queue.h> #include <osmocom/gsm/gsm23003.h> #include <osmocom/gsm/abis_nm.h> #include <osmo-bts/logging.h> @@ -962,15 +963,17 @@ struct pcu_sock_state { struct osmo_fd listen_bfd; /* fd for listen socket */ - struct osmo_fd conn_bfd; /* fd for connection to lcr */ - struct llist_head upqueue; /* queue for sending messages */ + struct osmo_wqueue upqueue; /* For sending messages; has fd for conn. to PCU */ }; +static void pcu_sock_close(struct pcu_sock_state *state); + int pcu_sock_send(struct msgb *msg) { struct pcu_sock_state *state = g_bts_sm->gprs.pcu_state; struct osmo_fd *conn_bfd; struct gsm_pcu_if *pcu_prim = (struct gsm_pcu_if *) msg->data; + int rc; if (!state) { if (pcu_prim->msg_type != PCU_IF_MSG_TIME_IND && @@ -980,7 +983,7 @@ msgb_free(msg); return -EINVAL; } - conn_bfd = &state->conn_bfd; + conn_bfd = &state->upqueue.bfd; if (conn_bfd->fd <= 0) { if (pcu_prim->msg_type != PCU_IF_MSG_TIME_IND && pcu_prim->msg_type != PCU_IF_MSG_INTERF_IND) @@ -989,15 +992,22 @@ msgb_free(msg); return -EIO; } - msgb_enqueue(&state->upqueue, msg); - osmo_fd_write_enable(conn_bfd); + rc = osmo_wqueue_enqueue(&state->upqueue, msg); + if (rc < 0) { + if (rc == -ENOSPC) + LOGP(DPCU, LOGL_NOTICE, "PCU not reacting (more than %u messages waiting). Closing connection\n", + state->upqueue.max_length); + pcu_sock_close(state); + msgb_free(msg); + return rc; + } return 0; } static void pcu_sock_close(struct pcu_sock_state *state) { - struct osmo_fd *bfd = &state->conn_bfd; + struct osmo_fd *bfd = &state->upqueue.bfd; struct gsm_bts *bts; struct gsm_bts_trx *trx; unsigned int tn; @@ -1043,11 +1053,7 @@ } } - /* flush the queue */ - while (!llist_empty(&state->upqueue)) { - struct msgb *msg = msgb_dequeue(&state->upqueue); - msgb_free(msg); - } + osmo_wqueue_clear(&state->upqueue); } static int pcu_sock_read(struct osmo_fd *bfd) @@ -1096,59 +1102,34 @@ return -1; } -static int pcu_sock_write(struct osmo_fd *bfd) +static int pcu_sock_write(struct osmo_fd *bfd, struct msgb *msg) { struct pcu_sock_state *state = bfd->data; - struct msgb *msg; int rc; - while ((msg = msgb_dequeue(&state->upqueue))) { - /* bug hunter 8-): maybe someone forgot msgb_put(...) ? */ - OSMO_ASSERT(msgb_length(msg) > 0); - - /* try to send it over the socket */ - rc = write(bfd->fd, msgb_data(msg), msgb_length(msg)); - if (OSMO_UNLIKELY(rc == 0)) - goto close; - if (OSMO_UNLIKELY(rc < 0)) { - if (errno == EAGAIN) { - /* Re-insert at the start of the queue, skip disabling fd WRITE */ - llist_add(&msg->list, &state->upqueue); - return 0; - } - goto close; - } - msgb_free(msg); + /* bug hunter 8-): maybe someone forgot msgb_put(...) ? */ + OSMO_ASSERT(msgb_length(msg) > 0); + /* try to send it over the socket */ + rc = write(bfd->fd, msgb_data(msg), msgb_length(msg)); + if (OSMO_UNLIKELY(rc == 0)) + goto close; + if (OSMO_UNLIKELY(rc < 0)) { + if (errno == EAGAIN) + return -EAGAIN; + return -1; } - osmo_fd_write_disable(bfd); return 0; close: - msgb_free(msg); pcu_sock_close(state); return -1; } -static int pcu_sock_cb(struct osmo_fd *bfd, unsigned int flags) -{ - int rc = 0; - - if (flags & OSMO_FD_READ) - rc = pcu_sock_read(bfd); - if (rc < 0) - return rc; - - if (flags & OSMO_FD_WRITE) - rc = pcu_sock_write(bfd); - - return rc; -} - /* accept connection coming from PCU */ static int pcu_sock_accept(struct osmo_fd *bfd, unsigned int flags) { struct pcu_sock_state *state = (struct pcu_sock_state *)bfd->data; - struct osmo_fd *conn_bfd = &state->conn_bfd; + struct osmo_fd *conn_bfd = &state->upqueue.bfd; struct sockaddr_un un_addr; socklen_t len; int fd; @@ -1168,7 +1149,7 @@ return 0; } - osmo_fd_setup(conn_bfd, fd, OSMO_FD_READ, pcu_sock_cb, state, 0); + osmo_fd_setup(conn_bfd, fd, OSMO_FD_READ, osmo_wqueue_bfd_cb, state, 0); if (osmo_fd_register(conn_bfd) != 0) { LOGP(DPCU, LOGL_ERROR, "Failed to register new connection fd\n"); @@ -1195,8 +1176,10 @@ if (!state) return -ENOMEM; - INIT_LLIST_HEAD(&state->upqueue); - state->conn_bfd.fd = -1; + osmo_wqueue_init(&state->upqueue, BTS_PCU_SOCK_WQUEUE_LEN_DEFAULT); + state->upqueue.read_cb = pcu_sock_read; + state->upqueue.write_cb = pcu_sock_write; + state->upqueue.bfd.fd = -1; bfd = &state->listen_bfd; @@ -1237,7 +1220,7 @@ return; osmo_signal_unregister_handler(SS_GLOBAL, pcu_if_signal_cb, NULL); - conn_bfd = &state->conn_bfd; + conn_bfd = &state->upqueue.bfd; if (conn_bfd->fd > 0) pcu_sock_close(state); bfd = &state->listen_bfd; @@ -1252,7 +1235,7 @@ if (!state) return false; - if (state->conn_bfd.fd <= 0) + if (state->upqueue.bfd.fd <= 0) return false; return true; } -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/31533 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ia6e61dda4b3cd4bba76e6acb7771d70335062fe1 Gerrit-Change-Number: 31533 Gerrit-PatchSet: 13 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0