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

[S] Change in libosmo-netif[master]: stream_srv: Fix connection error handling

by pespin

Attention is currently required from: daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/34338?usp=email ) Change subject: stream_srv: Fix connection error handling ...................................................................... Patch Set 1: Code-Review-1 (2 comments) File src/stream_srv.c: https://gerrit.osmocom.org/c/libosmo-netif/+/34338/comment/b464b426_172c8954 PS1, Line 489: if (res <= 0) { OSMO_UNLIKELY() https://gerrit.osmocom.org/c/libosmo-netif/+/34338/comment/74deb02f_3759ae00 PS1, Line 493: if (conn->flags & OSMO_STREAM_SRV_F_FLUSH_DESTROY) { so if iofd_read_cb is not sent you won't destroy the conn? this looks wrong? -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/34338?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I84eea2717f3762830f3f5b115e6fc8545eaa4fd5 Gerrit-Change-Number: 34338 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 07 Sep 2023 15:04:18 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[S] Change in libosmocore[master]: osmo_io: Init iofd_msghdr to zero

by pespin

Attention is currently required from: daniel. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/34337?usp=email ) Change subject: osmo_io: Init iofd_msghdr to zero ...................................................................... Patch Set 1: Code-Review-1 (1 comment) File src/core/osmo_io_poll.c: https://gerrit.osmocom.org/c/libosmocore/+/34337/comment/07b40d03_735f3885 PS1, Line 58: hdr.msg = msg; Better do hdr = {}; here while adding the other fields, otherwise you are memzeroing twice. It may sound like not a lot, but this is called in a hot path everytime a recv or send is done... -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34337?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: I21114ad57784126cfdeb4a932ed44dbf23946fbe Gerrit-Change-Number: 34337 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Thu, 07 Sep 2023 15:01:23 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[L] Change in libosmo-netif[master]: stream: Add client-side (segmentation) support for IPA

by pespin

Attention is currently required from: arehbein, fixeria, osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33198?usp=email ) Change subject: stream: Add client-side (segmentation) support for IPA ...................................................................... Patch Set 21: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33198?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I822abf52c6ae396c90b5c50228a0a39c848d3de6 Gerrit-Change-Number: 33198 Gerrit-PatchSet: 21 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 07 Sep 2023 14:57:33 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[S] Change in libosmo-netif[master]: stream: Fix endless loop on server on client disconnect

by daniel

Attention is currently required from: arehbein, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33196?usp=email ) Change subject: stream: Fix endless loop on server on client disconnect ...................................................................... Patch Set 3: Code-Review-1 (1 comment) File src/stream.c: https://gerrit.osmocom.org/c/libosmo-netif/+/33196/comment/21c2fe32_c17d85bf PS3, Line 1349: osmo_stream_srv_set_flush_and_destroy(conn); > why aren't you simply disabling READ poll flag here if res == 0? […] I don't think this is the correct fix and (partially) agree with Pau. On error (or EOF) we should simply destroy the connection. The library code should not use osmo_stream_srv_set_flush_and_destroy(). This function is for the user to indicate that we should send out all pending messages and then close the connection. I uploaded https://gerrit.osmocom.org/c/libosmo-netif/+/34338 which should fix this properly. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33196?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I012ddf14ae17642a52d34026d85ab6958cf488a1 Gerrit-Change-Number: 33196 Gerrit-PatchSet: 3 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 07 Sep 2023 14:51:07 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: Yes Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[S] Change in libosmo-netif[master]: stream_srv: Fix connection error handling

by daniel

daniel has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/34338?usp=email ) Change subject: stream_srv: Fix connection error handling ...................................................................... stream_srv: Fix connection error handling If read returned an error or the stream got closed then simply destroy the connection. If the user code called osmo_stream_srv_set_flush_and_destroy() then ignore any incoming messages and destroy the connection once the tx queue is empty. Change-Id: I84eea2717f3762830f3f5b115e6fc8545eaa4fd5 --- M src/stream_srv.c 1 file changed, 25 insertions(+), 10 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/38/34338/1 diff --git a/src/stream_srv.c b/src/stream_srv.c index f46175b..1f78707 100644 --- a/src/stream_srv.c +++ b/src/stream_srv.c @@ -486,17 +486,17 @@ struct osmo_stream_srv *conn = osmo_iofd_get_data(iofd); LOGSSRV(conn, LOGL_DEBUG, "message received (res=%d)\n", res); - if (conn->flags & OSMO_STREAM_SRV_F_FLUSH_DESTROY) { - LOGSSRV(conn, LOGL_INFO, "Connection is being flushed and closed; ignoring received message\n"); - msgb_free(msg); - return; - } - if (res <= 0) { - osmo_stream_srv_set_flush_and_destroy(conn); - if (osmo_iofd_txqueue_len(iofd) == 0) - osmo_stream_srv_destroy(conn); + /* This connection is dead, destroy it. */ + osmo_stream_srv_destroy(conn); } else if (conn->iofd_read_cb) { + if (conn->flags & OSMO_STREAM_SRV_F_FLUSH_DESTROY) { + LOGSSRV(conn, LOGL_INFO, "Connection is being flushed and closed; ignoring received message\n"); + msgb_free(msg); + if (osmo_iofd_txqueue_len(iofd) == 0) + osmo_stream_srv_destroy(conn); + return; + } conn->iofd_read_cb(conn, msg); } } @@ -506,7 +506,7 @@ struct osmo_stream_srv *conn = osmo_iofd_get_data(iofd); LOGSSRV(conn, LOGL_DEBUG, "connected write\n"); - if (res == -1) + if (res < 0) LOGSSRV(conn, LOGL_ERROR, "error to send: %s\n", strerror(errno)); if (osmo_iofd_txqueue_len(iofd) == 0) -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/34338?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I84eea2717f3762830f3f5b115e6fc8545eaa4fd5 Gerrit-Change-Number: 34338 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 9 months

1
0
0 0

[S] Change in libosmocore[master]: osmo_io: Init iofd_msghdr to zero

by daniel

daniel has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/34337?usp=email ) Change subject: osmo_io: Init iofd_msghdr to zero ...................................................................... osmo_io: Init iofd_msghdr to zero Avoid uninitialized read, found with valgrind Change-Id: I21114ad57784126cfdeb4a932ed44dbf23946fbe --- M src/core/osmo_io_poll.c 1 file changed, 12 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/37/34337/1 diff --git a/src/core/osmo_io_poll.c b/src/core/osmo_io_poll.c index c230188..5e3be30 100644 --- a/src/core/osmo_io_poll.c +++ b/src/core/osmo_io_poll.c @@ -48,7 +48,7 @@ int rc, flags = 0; if (what & OSMO_FD_READ) { - struct iofd_msghdr hdr; + struct iofd_msghdr hdr = {0}; msg = iofd_msgb_pending_or_alloc(iofd); if (!msg) { LOGPIO(iofd, LOGL_ERROR, "Could not allocate msgb for reading\n"); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/34337?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: I21114ad57784126cfdeb4a932ed44dbf23946fbe Gerrit-Change-Number: 34337 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: newchange

1 year, 9 months

1
0
0 0

[L] Change in libosmo-netif[master]: stream: Add client-side (segmentation) support for IPA

by daniel

Attention is currently required from: arehbein, fixeria, osmith, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/33198?usp=email ) Change subject: stream: Add client-side (segmentation) support for IPA ...................................................................... Patch Set 21: (1 comment) Patchset: PS21: > I guess printing the timestamp infront of the log should just be removed? from a quick glance this s […] I just retriggered jenkins and it succeeds again... The tests uses osmo's gettimeofday_override functionality so it shouldn't cause these weird changes due to (wall clock) time delays. It's counting main loop iterations and the only way a mismatch could happen here (that I can think of) is if either the sending fd wasn't writable after enqueuing the msg or the receiving fd didn't become readable directly after the write. So it's definitely worrying that we don't know the cause for this issue, but since nobody can reproduce it right now I'd say we continue with this patch (with timestamps) and see if it resurfaces. -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/33198?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I822abf52c6ae396c90b5c50228a0a39c848d3de6 Gerrit-Change-Number: 33198 Gerrit-PatchSet: 21 Gerrit-Owner: arehbein <arehbein(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-CC: daniel <dwillmann(a)sysmocom.de> Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-CC: osmith <osmith(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 07 Sep 2023 14:05:45 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Comment-In-Reply-To: arehbein <arehbein(a)sysmocom.de> Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[M] Change in osmo-ci[master]: jobs/osmocom-obs-sync: new job

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ci/+/34334?usp=email ) Change subject: jobs/osmocom-obs-sync: new job ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/34334?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: Ibcf02eb5feee2b314ff28861835040c21aea4294 Gerrit-Change-Number: 34334 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Thu, 07 Sep 2023 13:15:47 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[L] Change in osmo-ci[master]: OBS: add sync_obs_projects.py

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ci/+/34333?usp=email ) Change subject: OBS: add sync_obs_projects.py ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/34333?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: I00122f7819ba682cb791c370d781377cfb302629 Gerrit-Change-Number: 34333 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Thu, 07 Sep 2023 13:15:38 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

[S] Change in osmo-ci[master]: OBS: lib/osc: add private proj variable

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ci/+/34332?usp=email ) Change subject: OBS: lib/osc: add private proj variable ...................................................................... Patch Set 2: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/34332?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: Iabb871bcb432f2fbdaec9cbcab3d663ccf969901 Gerrit-Change-Number: 34332 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: osmith <osmith(a)sysmocom.de> Gerrit-Comment-Date: Thu, 07 Sep 2023 13:14:41 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 year, 9 months

1
0
0 0

2025

2024

2023

2022

gerrit-log September 2023