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

[L] Change in libosmo-gprs[master]: rlcmac: ul_tbf: Implement support for TBF Starting Time
by pespin 09 Mar '23

09 Mar '23

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/31751 ) Change subject: rlcmac: ul_tbf: Implement support for TBF Starting Time ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31751 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: Id81f16743f2c464e01caf27ba2eb8c0fc715fe8a Gerrit-Change-Number: 31751 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 09 Mar 2023 17:52:00 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in libosmo-gprs[master]: rlcmac: Ignore DATA.ind with len=0
by pespin 09 Mar '23

09 Mar '23

Attention is currently required from: fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-gprs/+/31732 ) Change subject: rlcmac: Ignore DATA.ind with len=0 ...................................................................... Patch Set 2: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-gprs/+/31732 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmo-gprs Gerrit-Branch: master Gerrit-Change-Id: I342c7999261832a5d981e5831e428d2cbd82729e Gerrit-Change-Number: 31732 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(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: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Comment-Date: Thu, 09 Mar 2023 17:51:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: select: Optimize osmo_fd_get_by_fd
by pespin 09 Mar '23

09 Mar '23

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31808 ) Change subject: select: Optimize osmo_fd_get_by_fd ...................................................................... Patch Set 1: (1 comment) File src/core/select.c: https://gerrit.osmocom.org/c/libosmocore/+/31808/comment/8e55334e_1c54d17e PS1, Line 223: if (fd->fd >= 0) > Here I'd like to have an OSMO_ASSERT(fd->fd >=0) instead, but that made one unit test in osmo-mgw hi […] BTW, the fact that we assert there would also probably mean we can optimize osmo_fd_is_registered() to be O(k) instead of O(n), because we can guarantee that the pointer in the table won't leak and will be NULL when the fd is unregistered. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31808 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I8b71547df8bed84192cb160479fa3debf9b7eade Gerrit-Change-Number: 31808 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Thu, 09 Mar 2023 17:51:05 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: select: Optimize osmo_fd_get_by_fd
by pespin 09 Mar '23

09 Mar '23

pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31808 ) Change subject: select: Optimize osmo_fd_get_by_fd ...................................................................... Patch Set 1: (1 comment) File src/core/select.c: https://gerrit.osmocom.org/c/libosmocore/+/31808/comment/cd9f53ae_52bfc27f PS1, Line 223: if (fd->fd >= 0) Here I'd like to have an OSMO_ASSERT(fd->fd >=0) instead, but that made one unit test in osmo-mgw hit the assert: https://gerrit.osmocom.org/c/osmo-mgw/+/31806 Other projects are running fine so far (A previous patch for libosmocore also fixes one related issue heating this assert). Even if that makes older osmo-mgw assert, I think it is still acceptable because it is actually detecting a bug in osmo-mgw. Users of the osmo_fd API shouldn't change the fd value while the osmo_fd is registered, that's calling for a potential mess. This actually even causes bugs inside libosmocore before this patch, see call to osmo_stats_tcp_osmo_fd_unregister(fd) just below, which will probably leak due to not finding the fd because it has been set to -1 prior to unregistering. So, do others agree I should use an OSMO_ASSERT there? this will help in finding bugs during development time by hitting the assert, rather than ending up with fucked up silently at production time. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31808 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I8b71547df8bed84192cb160479fa3debf9b7eade Gerrit-Change-Number: 31808 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Comment-Date: Thu, 09 Mar 2023 17:50:03 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[S] Change in libosmocore[master]: cosmetic: stats_tcp: Fix typo in comment
by pespin 09 Mar '23

09 Mar '23

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/31807 ) Change subject: cosmetic: stats_tcp: Fix typo in comment ...................................................................... cosmetic: stats_tcp: Fix typo in comment Take the chance to shorten a bit the lengthy lines Change-Id: I3215dea7e3198b728745a779ad1b6ebfa3795cb5 --- M src/core/stats_tcp.c 1 file changed, 15 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/07/31807/1 diff --git a/src/core/stats_tcp.c b/src/core/stats_tcp.c index ebb380e..c6459fe 100644 --- a/src/core/stats_tcp.c +++ b/src/core/stats_tcp.c @@ -247,8 +247,10 @@ pthread_mutex_lock(&stats_tcp_lock); llist_for_each_entry(stats_tcp_entry, &stats_tcp, entry) { if (fd->fd == stats_tcp_entry->fd->fd) { - /* In case we want to remove exactly that item which is also selected as the current itemy, we - * must designate either a different item or invalidate the current item. */ + /* In case we want to remove exactly that item which is also + * selected as the current item, we must designate either a + * different item or invalidate the current item. + */ if (stats_tcp_entry == stats_tcp_entry_cur) { if (llist_count(&stats_tcp) > 2) next_stats_tcp_entry(); -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31807 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I3215dea7e3198b728745a779ad1b6ebfa3795cb5 Gerrit-Change-Number: 31807 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[M] Change in libosmocore[master]: select: Optimize osmo_fd_get_by_fd
by pespin 09 Mar '23

09 Mar '23

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/31808 ) Change subject: select: Optimize osmo_fd_get_by_fd ...................................................................... select: Optimize osmo_fd_get_by_fd Optimize osmo_fd_get_by_fd() from O(n) to O(k) by means of allocating dynamically growing array. Make use from the fact that the kernel always tries to use the smallest possible unused value when allocating a new fd. Change-Id: I8b71547df8bed84192cb160479fa3debf9b7eade --- M src/core/select.c 1 file changed, 62 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/08/31808/1 diff --git a/src/core/select.c b/src/core/select.c index 72f794f..6ff3243 100644 --- a/src/core/select.c +++ b/src/core/select.c @@ -54,6 +54,39 @@ static __thread struct llist_head osmo_fds; /* TLS cannot use LLIST_HEAD() */ static __thread int unregistered_count; +/* Array of struct osmo_fd * (size "max_fd") ordered by "ofd->fd" */ +static __thread struct { + struct osmo_fd **table; + unsigned int size; +} osmo_fd_lookup; + +static void osmo_fd_lookup_table_extend(unsigned int new_max_fd) +{ + unsigned int min_new_size; + unsigned int pw2; + unsigned int new_size; + + /* First calculate the minimally required new size of the array in bytes: */ + min_new_size = (new_max_fd + 1) * sizeof(struct osmo_fd *); + /* Now find the lower power of two of min_new_size (in bytes): */ + pw2 = 32 - __builtin_clz(min_new_size); + /* get next (upper side) power of 2: */ + pw2++; + OSMO_ASSERT(pw2 <= 31); /* Avoid shifting more than 31 bits */ + new_size = 1 << (pw2 + 1); + + /* Let's make it at least 1024B, to avoid reallocating quickly at startup */ + if (new_size < 1024) + new_size = 1024; + if (new_size > osmo_fd_lookup.size) { + uint8_t *ptr = talloc_realloc_size(OTC_GLOBAL, osmo_fd_lookup.table, new_size); + OSMO_ASSERT(ptr); + memset(ptr + osmo_fd_lookup.size, 0, new_size - osmo_fd_lookup.size); + osmo_fd_lookup.table = (struct osmo_fd **)ptr; + osmo_fd_lookup.size = new_size; + } +} + #ifndef FORCE_IO_SELECT struct poll_state { /* array of pollfd */ @@ -142,8 +175,10 @@ return flags; /* Register FD */ - if (fd->fd > maxfd) + if (fd->fd > maxfd) { maxfd = fd->fd; + osmo_fd_lookup_table_extend(maxfd); + } #ifdef OSMO_FD_CHECK if (osmo_fd_is_registered(fd)) { @@ -166,6 +201,7 @@ #endif /* FORCE_IO_SELECT */ llist_add_tail(&fd->list, &osmo_fds); + osmo_fd_lookup.table[fd->fd] = fd; return 0; } @@ -180,6 +216,12 @@ * osmo_fd_is_registered() */ unregistered_count++; llist_del(&fd->list); + OSMO_ASSERT(fd->fd <= maxfd); + /* Some old programs incorrectly changed fd value to -1 before + * unregistering, so we avoid asserting here, in order to keep backward + * compatibility... */ + if (fd->fd >= 0) + osmo_fd_lookup.table[fd->fd] = NULL; #ifndef FORCE_IO_SELECT g_poll.num_registered--; #endif /* FORCE_IO_SELECT */ @@ -464,19 +506,16 @@ * \returns \ref osmo_fd for \ref fd; NULL in case it doesn't exist */ struct osmo_fd *osmo_fd_get_by_fd(int fd) { - struct osmo_fd *ofd; - - llist_for_each_entry(ofd, &osmo_fds, list) { - if (ofd->fd == fd) - return ofd; - } - return NULL; + if (fd > maxfd) + return NULL; + return osmo_fd_lookup.table[fd]; } /*! initialize the osmocom select abstraction for the current thread */ void osmo_select_init(void) { INIT_LLIST_HEAD(&osmo_fds); + osmo_fd_lookup_table_extend(0); } /* ensure main thread always has pre-initialized osmo_fds */ -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31808 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I8b71547df8bed84192cb160479fa3debf9b7eade Gerrit-Change-Number: 31808 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in libosmocore[master]: gsmtap.h: Define a packet type for encapsulation of GSM RLP frames
by pespin 09 Mar '23

09 Mar '23

Attention is currently required from: laforge. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/31805 ) Change subject: gsmtap.h: Define a packet type for encapsulation of GSM RLP frames ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31805 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibb7f0731c31e5a5cd2911f733da5510ce0f3a1d6 Gerrit-Change-Number: 31805 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Comment-Date: Thu, 09 Mar 2023 17:35:52 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-mgw[master]: mgcp_network: Unregister osmo_fd before closing fd
by pespin 09 Mar '23

09 Mar '23

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-mgw/+/31806 ) Change subject: mgcp_network: Unregister osmo_fd before closing fd ...................................................................... mgcp_network: Unregister osmo_fd before closing fd Change-Id: Ib937e56be62327f52d9cf03a07d7620080356ee8 --- M src/libosmo-mgcp/mgcp_network.c 1 file changed, 11 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-mgw refs/changes/06/31806/1 diff --git a/src/libosmo-mgcp/mgcp_network.c b/src/libosmo-mgcp/mgcp_network.c index d6028a2..af9ae99 100644 --- a/src/libosmo-mgcp/mgcp_network.c +++ b/src/libosmo-mgcp/mgcp_network.c @@ -1676,14 +1676,14 @@ void mgcp_free_rtp_port(struct mgcp_rtp_end *end) { if (end->rtp.fd != -1) { + osmo_fd_unregister(&end->rtp); close(end->rtp.fd); end->rtp.fd = -1; - osmo_fd_unregister(&end->rtp); } if (end->rtcp.fd != -1) { + osmo_fd_unregister(&end->rtcp); close(end->rtcp.fd); end->rtcp.fd = -1; - osmo_fd_unregister(&end->rtcp); } } -- To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/31806 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-mgw Gerrit-Branch: master Gerrit-Change-Id: Ib937e56be62327f52d9cf03a07d7620080356ee8 Gerrit-Change-Number: 31806 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in libosmocore[master]: gsmtap.h: Define a packet type for encapsulation of GSM RLP frames
by laforge 09 Mar '23

09 Mar '23

laforge has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/31805 ) Change subject: gsmtap.h: Define a packet type for encapsulation of GSM RLP frames ...................................................................... gsmtap.h: Define a packet type for encapsulation of GSM RLP frames This allows us to feed RLP frames (occurring in GSM CSD) into wireshark for dissection. Change-Id: Ibb7f0731c31e5a5cd2911f733da5510ce0f3a1d6 --- M include/osmocom/core/gsmtap.h 1 file changed, 13 insertions(+), 0 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/05/31805/1 diff --git a/include/osmocom/core/gsmtap.h b/include/osmocom/core/gsmtap.h index b0985f0..2387b20 100644 --- a/include/osmocom/core/gsmtap.h +++ b/include/osmocom/core/gsmtap.h @@ -49,6 +49,7 @@ #define GSMTAP_TYPE_QC_DIAG 0x11 /* Qualcomm DIAG frame */ #define GSMTAP_TYPE_LTE_NAS 0x12 /* LTE Non-Access Stratum */ #define GSMTAP_TYPE_E1T1 0x13 /* E1/T1 Lines */ +#define GSMTAP_TYPE_GSM_RLP 0x14 /* GSM RLP frames as per 3GPP TS 24.022 */ /* ====== DO NOT MAKE UNAPPROVED MODIFICATIONS HERE ===== */ -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31805 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ibb7f0731c31e5a5cd2911f733da5510ce0f3a1d6 Gerrit-Change-Number: 31805 Gerrit-PatchSet: 1 Gerrit-Owner: laforge <laforge(a)osmocom.org> Gerrit-MessageType: newchange

1 0

[S] Change in libosmocore[master]: logging: Unregister osmo_fd before closing fd
by pespin 09 Mar '23

09 Mar '23

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/31804 ) Change subject: logging: Unregister osmo_fd before closing fd ...................................................................... logging: Unregister osmo_fd before closing fd Change-Id: I0754ac3110b63d2a380068010830af6c85b0a653 --- M src/core/logging.c 1 file changed, 10 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/04/31804/1 diff --git a/src/core/logging.c b/src/core/logging.c index 938c65f..bcfe318 100644 --- a/src/core/logging.c +++ b/src/core/logging.c @@ -1327,12 +1327,12 @@ } wq = target->tgt_file.wqueue; if (wq) { + osmo_fd_unregister(&wq->bfd); if (wq->bfd.fd >= 0) { if (target->type == LOG_TGT_TYPE_FILE) close(wq->bfd.fd); wq->bfd.fd = -1; } - osmo_fd_unregister(&wq->bfd); osmo_wqueue_clear(wq); talloc_free(wq); target->tgt_file.wqueue = NULL; -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/31804 To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: I0754ac3110b63d2a380068010830af6c85b0a653 Gerrit-Change-Number: 31804 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: newchange

1 0