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

Change in osmo-hnbgw[master]: hnbgw: Fix recent regression not closing conn upon rx of SCTP_SHUTDOW...

by pespin

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29451 to look at the new patch set (#3). Change subject: hnbgw: Fix recent regression not closing conn upon rx of SCTP_SHUTDOWN_EVENT ...................................................................... hnbgw: Fix recent regression not closing conn upon rx of SCTP_SHUTDOWN_EVENT Before handling of OSMO_STREAM_SCTP_MSG_FLAGS_NOTIFICATION in recent commit (see Fixes: below), osmo_stream_srv_recv() and internal _sctp_recvmsg_wrapper() in libosmo-netif would return either -EAGAIN or 0 when an sctp notification was received from the kernel. After adding handling of OSMO_STREAM_SCTP_MSG_FLAGS_NOTIFICATION, the code paths for "rc == -EAGAIN" and "rc == 0" would not be executed anymore since the first branch takes preference in the if-else tree. For "rc == -EAGAIN" it's fine because the new branch superseeds what's done on the "rc == -EAGAIN" branch. However, for the "rc == 0", we forgot to actually destroy the connection. The "rc == 0" branch was basically reached when SCTP_SHUTDOWN_EVENT was received because osmo_stream_srv_recv() tried to resemble the interface of regular recv(); let's hence check for that explicitly and destroy the conn object (and the related hnb context in the process) when we receive that event. Fixes: 1de2091515530e531196ebca27b1445cecd1b88d Related: SYS#6113 Change-Id: I11b6af51a58dc250975a696b98d0c0c9ff3df9e0 --- M src/osmo-hnbgw/hnbgw.c 1 file changed, 11 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/51/29451/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29451 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I11b6af51a58dc250975a696b98d0c0c9ff3df9e0 Gerrit-Change-Number: 29451 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

2 years, 9 months

1
0
0 0

Change in osmo-hnbgw[master]: hnbgw: Fix recent regression not closing conn upon rx of SCTP_SHUTDOW...

by pespin

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29451 to look at the new patch set (#2). Change subject: hnbgw: Fix recent regression not closing conn upon rx of SCTP_SHUTDOWN_EVENT ...................................................................... hnbgw: Fix recent regression not closing conn upon rx of SCTP_SHUTDOWN_EVENT Before handling of OSMO_STREAM_SCTP_MSG_FLAGS_NOTIFICATION in recent commit (see Fixes: below), osmo_stream_srv_recv() and internal _sctp_recvmsg_wrapper() in libosmo-netif would return either -EAGAIN or 0 when an sctp notification was received from the kernel. After adding handling of OSMO_STREAM_SCTP_MSG_FLAGS_NOTIFICATION, the code paths for "rc == -EAGAIN" and "rc == 0" would not be executed anymore since the first branch takes preference in the if-else tree. For "rc == -EAGAIN" it's fine because the new branch superseeds what's done on the "rc == -EAGAIN" branch. However, for the "rc == 0", we forgot to actually destroy the connection. The "rc == 0" branch was basically reached when SCTP_SHUTDOWN_EVENT was received because osmo_stream_srv_recv() tried to resemble the interface of regular recv(); let's hence check for that explicitly and destroy the conn object (and the related hnb context in the process) when we receive that event. Fixes: 1de2091515530e531196ebca27b1445cecd1b88d Related: SYS#6113 Change-Id: I11b6af51a58dc250975a696b98d0c0c9ff3df9e0 --- M src/osmo-hnbgw/hnbgw.c 1 file changed, 11 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/51/29451/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29451 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I11b6af51a58dc250975a696b98d0c0c9ff3df9e0 Gerrit-Change-Number: 29451 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-MessageType: newpatchset

2 years, 9 months

1
0
0 0

Change in osmo-hnbgw[master]: hnbgw: Fix recent regression not closing conn upon rx of SCTP_SHUTDOW...

by pespin

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29451 ) Change subject: hnbgw: Fix recent regression not closing conn upon rx of SCTP_SHUTDOWN_EVENT ...................................................................... hnbgw: Fix recent regression not closing conn upon rx of SCTP_SHUTDOWN_EVENT Before handling of OSMO_STREAM_SCTP_MSG_FLAGS_NOTIFICATION in recent commit (see Fixes: below), osmo_stream_srv_recv() and internal _sctp_recvmsg_wrapper() in libosmo-netif would return either -EAGAIN or 0 when an sctp notification was received from the kernel. After adding handling of OSMO_STREAM_SCTP_MSG_FLAGS_NOTIFICATION, the code paths for "rc == -EAGAIN" and "rc == 0" would not be executed anymore since the first branch takes preference in the if-else tree. For "rc == -EAGAIN" it's fine because the new branch superseeds what's done on the "rc == -EAGAIN" branch. However, for the "rc == 0", we forgot to actually destroy the connection. The "rc == 0" branch was basically reached when SCTP_SHUTDOWN_EVENT was received because osmo_stream_srv_recv() tried to resemble the interface of regular recv(); let's hence check for that explicitly and destroy the conn object (and the related hnb context in the process) when we receive that event. Fixes: 1de2091515530e531196ebca27b1445cecd1b88d Related: SYS#6113 Change-Id: I11b6af51a58dc250975a696b98d0c0c9ff3df9e0 --- M src/osmo-hnbgw/hnbgw.c 1 file changed, 11 insertions(+), 4 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hnbgw refs/changes/51/29451/1 diff --git a/src/osmo-hnbgw/hnbgw.c b/src/osmo-hnbgw/hnbgw.c index 124a647..6d75525 100644 --- a/src/osmo-hnbgw/hnbgw.c +++ b/src/osmo-hnbgw/hnbgw.c @@ -254,6 +254,7 @@ /* Notification received */ if (msgb_sctp_msg_flags(msg) & OSMO_STREAM_SCTP_MSG_FLAGS_NOTIFICATION) { union sctp_notification *notif = (union sctp_notification *)msgb_data(msg); + rc = 0; switch (notif->sn_header.sn_type) { case SCTP_ASSOC_CHANGE: switch (notif->sn_assoc_change.sac_state) { @@ -261,17 +262,23 @@ LOGHNB(hnb, DMAIN, LOGL_NOTICE, "HNB SCTP conn RESTARTed, marking as HNBAP-unregistered\n"); hnb->hnb_registered = false; break; + case SCTP_SHUTDOWN_EVENT: + LOGHNB(hnb, DMAIN, LOGL_NOTICE, + "sctp_recvmsg(%s) = SCTP_SHUTDOWN_EVENT, closing conn\n", + osmo_sock_get_name2(osmo_stream_srv_get_ofd(conn)->fd)); + osmo_stream_srv_destroy(conn); + rc = -1; + break; } break; } - msgb_free(msg); - return 0; + goto out; } else if (rc == -EAGAIN) { /* Older versions of osmo_stream_srv_recv() not supporting * msgb_sctp_msg_flags() may still return -EAGAIN when an sctp * notification is received. */ - msgb_free(msg); - return 0; + rc = 0; + goto out; } else if (rc < 0) { LOGHNB(hnb, DMAIN, LOGL_ERROR, "Error during sctp_recvmsg(%s)\n", osmo_sock_get_name2(osmo_stream_srv_get_ofd(conn)->fd)); -- To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29451 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-hnbgw Gerrit-Branch: master Gerrit-Change-Id: I11b6af51a58dc250975a696b98d0c0c9ff3df9e0 Gerrit-Change-Number: 29451 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

2 years, 9 months

1
0
0 0

Change in osmo-ci[master]: lint: ignore STATIC_CONST_CHAR_ARRAY

by pespin

Attention is currently required from: osmith. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ci/+/29448 ) Change subject: lint: ignore STATIC_CONST_CHAR_ARRAY ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: > The reason would be what Vadim wrote in the referenced patch, […] I don't see why we should not care about it. Marking content as const should allow these to be placed in read-only data which is a good thing, and also allow more compiler optimizations. -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/29448 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: I09eecd9382842293ab231eb9cc92756fbe7723c1 Gerrit-Change-Number: 29448 Gerrit-PatchSet: 1 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, 22 Sep 2022 13:36:34 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: osmith <osmith(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 9 months

1
0
0 0

Change in osmo-ci[master]: lint: accept BE spelling of 'acknowledgement'

by osmith

osmith has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ci/+/29449 ) Change subject: lint: accept BE spelling of 'acknowledgement' ...................................................................... lint: accept BE spelling of 'acknowledgement' Related: https://gerrit.osmocom.org/c/libosmo-gprs/+/29402 Change-Id: Ia1a08295a7c7ed6a77f0055d66a161423d8f17f0 --- M lint/checkpatch/spelling.txt 1 file changed, 0 insertions(+), 1 deletion(-) Approvals: osmith: Verified pespin: Looks good to me, but someone else must approve fixeria: Looks good to me, approved diff --git a/lint/checkpatch/spelling.txt b/lint/checkpatch/spelling.txt index 7b6a012..7f0745c 100644 --- a/lint/checkpatch/spelling.txt +++ b/lint/checkpatch/spelling.txt @@ -50,7 +50,6 @@ acitions||actions acitve||active acknowldegement||acknowledgment -acknowledgement||acknowledgment ackowledge||acknowledge ackowledged||acknowledged acording||according 1 is the latest approved patch-set. No files were changed between the latest approved patch-set and the submitted one. -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/29449 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: Ia1a08295a7c7ed6a77f0055d66a161423d8f17f0 Gerrit-Change-Number: 29449 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 9 months

1
0
0 0

Change in osmo-ci[master]: lint: accept BE spelling of 'acknowledgement'

by osmith

osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ci/+/29449 ) Change subject: lint: accept BE spelling of 'acknowledgement' ...................................................................... Patch Set 2: Verified+1 -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/29449 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: Ia1a08295a7c7ed6a77f0055d66a161423d8f17f0 Gerrit-Change-Number: 29449 Gerrit-PatchSet: 2 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 22 Sep 2022 13:15:06 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 9 months

1
0
0 0

Change in osmo-ci[master]: lint: ignore STATIC_CONST_CHAR_ARRAY

by osmith

Attention is currently required from: pespin. osmith has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ci/+/29448 ) Change subject: lint: ignore STATIC_CONST_CHAR_ARRAY ...................................................................... Patch Set 1: (1 comment) Patchset: PS1: The reason would be what Vadim wrote in the referenced patch, > const * const - "should" does not mean must. It's a unit test, and I don't really care whether the test vectors end up in RODATA. It would actually be the first time for me using 'const * const', I don't remember anybody strictly requiring this during code review. So it seems like we don't care about it. Or do we? In general I try to adjust the linter so that when it fails then most likely something is wrong that really needs to be fixed, rather than failing more often with things that might be wrong but oftentimes we don't care about it (like max line length). -- To view, visit https://gerrit.osmocom.org/c/osmo-ci/+/29448 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ci Gerrit-Branch: master Gerrit-Change-Id: I09eecd9382842293ab231eb9cc92756fbe7723c1 Gerrit-Change-Number: 29448 Gerrit-PatchSet: 1 Gerrit-Owner: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Thu, 22 Sep 2022 13:14:55 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

2 years, 9 months

1
0
0 0

Change in osmo-bts[master]: vty: Fix SPEECH_MODE printed with hex prefix but dec value

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/29450 ) Change subject: vty: Fix SPEECH_MODE printed with hex prefix but dec value ...................................................................... vty: Fix SPEECH_MODE printed with hex prefix but dec value Change-Id: I597ff582f47b00d895611eae8a810fe3ebfe8339 --- M src/common/vty.c 1 file changed, 1 insertion(+), 1 deletion(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, approved diff --git a/src/common/vty.c b/src/common/vty.c index ffe4baf..3cd76fb 100644 --- a/src/common/vty.c +++ b/src/common/vty.c @@ -1946,7 +1946,7 @@ } if (lchan->abis_ip.connect_ip) { ia.s_addr = htonl(lchan->abis_ip.connect_ip); - vty_out(vty, " Conn. IP: %s Port %u RTP_TYPE=%u SPEECH_MODE=0x%02u%s", + vty_out(vty, " Conn. IP: %s Port %u RTP_TYPE=%u SPEECH_MODE=0x%02x%s", inet_ntoa(ia), lchan->abis_ip.connect_port, lchan->abis_ip.rtp_payload, lchan->abis_ip.speech_mode, VTY_NEWLINE); -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/29450 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: I597ff582f47b00d895611eae8a810fe3ebfe8339 Gerrit-Change-Number: 29450 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 9 months

1
0
0 0

Change in osmo-bts[master]: vty: Print Osmux CID on lchans using Osmux

by pespin

pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bts/+/29438 ) Change subject: vty: Print Osmux CID on lchans using Osmux ...................................................................... vty: Print Osmux CID on lchans using Osmux Related: SYS#5987 Change-Id: Ide6edefda828e9ce04fbb60cf547857f322d5f40 --- M src/common/vty.c 1 file changed, 12 insertions(+), 6 deletions(-) Approvals: Jenkins Builder: Verified fixeria: Looks good to me, approved diff --git a/src/common/vty.c b/src/common/vty.c index 3cd76fb..38448b1 100644 --- a/src/common/vty.c +++ b/src/common/vty.c @@ -1939,17 +1939,23 @@ if (lchan->abis_ip.bound_ip) { ia.s_addr = htonl(lchan->abis_ip.bound_ip); - vty_out(vty, " Bound IP: %s Port %u RTP_TYPE2=%u CONN_ID=%u%s", + vty_out(vty, " Bound IP: %s Port %u CONN_ID=%u", inet_ntoa(ia), lchan->abis_ip.bound_port, - lchan->abis_ip.rtp_payload2, lchan->abis_ip.conn_id, - VTY_NEWLINE); + lchan->abis_ip.conn_id); + if (lchan->abis_ip.osmux.use) + vty_out(vty, " Osmux_CID=%u%s", lchan->abis_ip.osmux.local_cid, VTY_NEWLINE); + else + vty_out(vty, " RTP_TYPE2=%u%s", lchan->abis_ip.rtp_payload2, VTY_NEWLINE); } if (lchan->abis_ip.connect_ip) { ia.s_addr = htonl(lchan->abis_ip.connect_ip); - vty_out(vty, " Conn. IP: %s Port %u RTP_TYPE=%u SPEECH_MODE=0x%02x%s", + vty_out(vty, " Conn. IP: %s Port %u SPEECH_MODE=0x%02x", inet_ntoa(ia), lchan->abis_ip.connect_port, - lchan->abis_ip.rtp_payload, lchan->abis_ip.speech_mode, - VTY_NEWLINE); + lchan->abis_ip.speech_mode); + if (lchan->abis_ip.osmux.use) + vty_out(vty, " Osmux_CID=%u%s", lchan->abis_ip.osmux.remote_cid, VTY_NEWLINE); + else + vty_out(vty, " RTP_TYPE=%u%s", lchan->abis_ip.rtp_payload, VTY_NEWLINE); } #define LAPDM_ESTABLISHED(link, sapi_idx) \ (link).datalink[sapi_idx].dl.state == LAPD_STATE_MF_EST -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/29438 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ide6edefda828e9ce04fbb60cf547857f322d5f40 Gerrit-Change-Number: 29438 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

2 years, 9 months

1
0
0 0

Change in osmo-tetra[master]: More robust bounds checking on llc pdu parsing

by laforge

Attention is currently required from: wbokslag. laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-tetra/+/29447 ) Change subject: More robust bounds checking on llc pdu parsing ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-tetra/+/29447 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-tetra Gerrit-Branch: master Gerrit-Change-Id: I118ba2227a22afd295fffaa51aab3e45e85ff3d7 Gerrit-Change-Number: 29447 Gerrit-PatchSet: 2 Gerrit-Owner: wbokslag <w.bokslag(a)midnightblue.nl> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: wbokslag <w.bokslag(a)midnightblue.nl> Gerrit-Comment-Date: Thu, 22 Sep 2022 12:20:14 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

2 years, 9 months

1
0
0 0

2025

2024

2023

2022

gerrit-log September 2022