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

[M] Change in libosmocore[master]: osmo_io: Remove union in struct osmo_io_ops
by Jenkins Builder 29 Nov '23

29 Nov '23

Attention is currently required from: arehbein, daniel. Jenkins Builder has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35079?usp=email ) Change subject: osmo_io: Remove union in struct osmo_io_ops ...................................................................... Patch Set 3: (1 comment) File src/core/osmo_io.c: Robot Comment from checkpatch (run ID jenkins-gerrit-lint-12714): https://gerrit.osmocom.org/c/libosmocore/+/35079/comment/ebf6c5e6_a0b1fc97 PS3, Line 388: static bool iofd_read_callback_set(struct osmo_io_fd* iofd) "foo* bar" should be "foo *bar" -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35079?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: I138d57843edc29000530bb7896bcb239002ecbec Gerrit-Change-Number: 35079 Gerrit-PatchSet: 3 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Comment-Date: Wed, 29 Nov 2023 17:59:08 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: osmo_io: Remove union in struct osmo_io_ops
by daniel 29 Nov '23

29 Nov '23

Attention is currently required from: arehbein, daniel. Hello Jenkins Builder, arehbein, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/35079?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: osmo_io: Remove union in struct osmo_io_ops ...................................................................... osmo_io: Remove union in struct osmo_io_ops This allows us to check that the correct callbacks are set. The checks are already in e.g. osmo_iofd_write_msgb(), but the union prevented us from distinguishing between write_cb() and sendto_cb() etc. Change-Id: I138d57843edc29000530bb7896bcb239002ecbec Related: #OS6263 --- M TODO-RELEASE M include/osmocom/core/osmo_io.h M src/core/osmo_io.c 3 files changed, 49 insertions(+), 32 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/79/35079/3 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35079?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: I138d57843edc29000530bb7896bcb239002ecbec Gerrit-Change-Number: 35079 Gerrit-PatchSet: 3 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmocore[master]: utils: Link with libosmoisdn to avoid undefined references
by pespin 29 Nov '23

29 Nov '23

Attention is currently required from: daniel, fixeria, jolly. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35163?usp=email ) Change subject: utils: Link with libosmoisdn to avoid undefined references ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35163?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: I9e99231899b269a4ec0faf720aff4dc8d2b74323 Gerrit-Change-Number: 35163 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: fixeria <axilirator(a)gmail.com> Gerrit-Reviewer: jolly <andreas(a)eversberg.eu> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Attention: jolly <andreas(a)eversberg.eu> Gerrit-Attention: daniel <dwillmann(a)sysmocom.de> Gerrit-Attention: fixeria <axilirator(a)gmail.com> Gerrit-Comment-Date: Wed, 29 Nov 2023 17:31:22 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[S] Change in osmo-ggsn[master]: libgtp: Remove defines for reserved causes in gtp.h
by daniel 29 Nov '23

29 Nov '23

daniel has submitted this change. ( https://gerrit.osmocom.org/c/osmo-ggsn/+/35162?usp=email ) Change subject: libgtp: Remove defines for reserved causes in gtp.h ...................................................................... libgtp: Remove defines for reserved causes in gtp.h As discussed in Gerrit change I9c3bf64537ef2223e29f8082861fa32fde26bf68 remove defines that don't serve any purpose. These are defines for reserved values and changing them later if a newer spec defined them would break API. Keep the comments to explain the missing values. Change-Id: I8db0aa0ade59785443a407b51dea326144406dcf --- M TODO-RELEASE M gtp/gtp.h 2 files changed, 26 insertions(+), 9 deletions(-) Approvals: daniel: Looks good to me, approved pespin: Looks good to me, but someone else must approve Jenkins Builder: Verified osmith: Looks good to me, but someone else must approve diff --git a/TODO-RELEASE b/TODO-RELEASE index d0852fc..d8cc3af 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -7,3 +7,4 @@ # If any interfaces have been added since the last public release: c:r:a + 1. # If any interfaces have been removed or changed since the last public release: c:r:0. #library what description / commit summary line +libgtp REMOVE remove GTP cause defines of reserved values diff --git a/gtp/gtp.h b/gtp/gtp.h index c066fa6..7465cf1 100644 --- a/gtp/gtp.h +++ b/gtp/gtp.h @@ -53,7 +53,7 @@ #define GTP_UPDATE_PDP_RSP 19 /* Update PDP Context Response */ #define GTP_DELETE_PDP_REQ 20 /* Delete PDP Context Request */ #define GTP_DELETE_PDP_RSP 21 /* Delete PDP Context Response */ - /* 22-25 For future use. *//* In version GTP 1 anonomous PDP context */ +/* 22-25 For future use. *//* In version GTP 1 anonomous PDP context */ #define GTP_ERROR 26 /* Error Indication */ #define GTP_PDU_NOT_REQ 27 /* PDU Notification Request */ #define GTP_PDU_NOT_RSP 28 /* PDU Notification Response */ @@ -99,21 +99,21 @@ #define GTPCAUSE_NO_ID_NEEDED 3 /* No identity needed */ #define GTPCAUSE_MS_REFUSES_X 4 /* MS refuses */ #define GTPCAUSE_MS_NOT_RESP_X 5 /* MS is not GPRS responding */ -#define GTPCAUSE_006 6 /* For future use 6-48 */ -#define GTPCAUSE_049 49 /* Cause values reserved for GPRS charging protocol use (See GTP' in GSM 12.15) 49-63 */ -#define GTPCAUSE_064 64 /* For future use 64-127 */ +/* 6-48 For future use */ +/* 49-63 Cause values reserved for GPRS charging protocol use (See GTP' in GSM 12.15) */ +/* 64-127 For future use */ #define GTPCAUSE_ACC_REQ 128 /* Request accepted */ #define GTPCAUSE_NEW_PDP_NET_PREF 129 /* New PDP type due to network preference */ #define GTPCAUSE_NEW_PDP_ADDR_BEAR 130 /* New PDP type due to single address bearer only */ -#define GTPCAUSE_131 131 /* For future use 131-176 */ -#define GTPCAUSE_177 177 /* Cause values reserved for GPRS charging protocol use (See GTP' In GSM 12.15) 177-191 */ +/* 131-176 For future use */ +/* 177-191 Cause values reserved for GPRS charging protocol use (See GTP' In GSM 12.15) */ #define GTPCAUSE_NON_EXIST 192 /* Non-existent */ #define GTPCAUSE_INVALID_MESSAGE 193 /* Invalid message format */ #define GTPCAUSE_IMSI_NOT_KNOWN 194 /* IMSI not known */ #define GTPCAUSE_MS_DETACHED 195 /* MS is GPRS detached */ #define GTPCAUSE_MS_NOT_RESP 196 /* MS is not GPRS responding */ #define GTPCAUSE_MS_REFUSES 197 /* MS refuses */ -#define GTPCAUSE_198 198 /* For future use */ +/* 198 For future use */ #define GTPCAUSE_NO_RESOURCES 199 /* No resources available */ #define GTPCAUSE_NOT_SUPPORTED 200 /* Service not supported */ #define GTPCAUSE_MAN_IE_INCORRECT 201 /* Mandatory IE incorrect */ @@ -136,8 +136,8 @@ #define GTPCAUSE_SYN_ERR_FILTER 218 /* Syntactic errors in packet filter(s) */ #define GTPCAUSE_MISSING_APN 219 /* Missing or unknown APN */ #define GTPCAUSE_UNKNOWN_PDP 220 /* Unknown PDP address or PDP type */ -#define GTPCAUSE_221 221 /* For Future Use 221-240 */ -#define GTPCAUSE_241 241 /* Cause Values Reserved For Gprs Charging Protocol Use (See Gtp' In Gsm 12.15) 241-255 */ +/* 221-240 For future use */ +/* 241-255 Cause Values Reserved For Gprs Charging Protocol Use (See Gtp' In Gsm 12.15) */ static inline bool gtp_cause_successful(uint8_t cause) { -- To view, visit https://gerrit.osmocom.org/c/osmo-ggsn/+/35162?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ggsn Gerrit-Branch: master Gerrit-Change-Id: I8db0aa0ade59785443a407b51dea326144406dcf Gerrit-Change-Number: 35162 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-MessageType: merged

1 0

[S] Change in osmo-ggsn[master]: libgtp: Remove defines for reserved causes in gtp.h
by daniel 29 Nov '23

29 Nov '23

daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-ggsn/+/35162?usp=email ) Change subject: libgtp: Remove defines for reserved causes in gtp.h ...................................................................... Patch Set 1: Code-Review+2 -- To view, visit https://gerrit.osmocom.org/c/osmo-ggsn/+/35162?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-ggsn Gerrit-Branch: master Gerrit-Change-Id: I8db0aa0ade59785443a407b51dea326144406dcf Gerrit-Change-Number: 35162 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Wed, 29 Nov 2023 17:18:01 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: osmo_io: Remove union in struct osmo_io_ops
by daniel 29 Nov '23

29 Nov '23

Attention is currently required from: arehbein. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35079?usp=email ) Change subject: osmo_io: Remove union in struct osmo_io_ops ...................................................................... Set Ready For Review -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35079?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: I138d57843edc29000530bb7896bcb239002ecbec Gerrit-Change-Number: 35079 Gerrit-PatchSet: 2 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: arehbein <arehbein(a)sysmocom.de> Gerrit-Attention: arehbein <arehbein(a)sysmocom.de> Gerrit-Comment-Date: Wed, 29 Nov 2023 17:16:23 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: No Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: osmo_io: Factor out and use common send function from backend
by daniel 29 Nov '23

29 Nov '23

Attention is currently required from: laforge, lynxis lazus, pespin. daniel has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmocore/+/35078?usp=email ) Change subject: osmo_io: Factor out and use common send function from backend ...................................................................... Patch Set 2: (5 comments) File src/core/osmo_io.c: https://gerrit.osmocom.org/c/libosmocore/+/35078/comment/c63c3397_1e3aa3a8 PS1, Line 343: void iofd_handle_send(struct osmo_io_fd *iofd, int rc, struct iofd_msghdr *msghdr) > this function (like unfortuntaely many functions in the code base) would benefit from a comment desc […] Done https://gerrit.osmocom.org/c/libosmocore/+/35078/comment/dde3791b_d6c6a0ea PS1, Line 343: void iofd_handle_send(struct osmo_io_fd *iofd, int rc, struct iofd_msghdr *msghdr) > this function (like unfortuntaely many functions in the code base) would benefit from a comment desc […] Done https://gerrit.osmocom.org/c/libosmocore/+/35078/comment/8d2930d4_ca0fc16d PS1, Line 348: if (rc > 0 && rc < msgb_length(msg)) { Regarding your comment about rc == 0, maybe one missing failure case is rc == 0 && rc < msgb_length(msg)? Not sure though. https://gerrit.osmocom.org/c/libosmocore/+/35078/comment/57942bf2_352fb2a8 PS1, Line 360: > rc == 0 case seems to be missing here? IMO rc == 0 in send means we have successfully "written" 0 bytes to the socket (which also means the socket is writable). This is (IMO correctly) handled in the success case below. The failure cases should either be rc < msgb_length() or rc == -EAGAIN. File src/core/osmo_io_uring.c: https://gerrit.osmocom.org/c/libosmocore/+/35078/comment/93398c92_a3e6f35c PS1, Line 188: iofd_msghdr_free(msghdr); > Yes, previous you didn't checked for -EAGAIN, now you're checking it and doing things based on it. You marked this as resolved, but just to be clear: I think this behavior is okay. It's debatable whether we'd ever encounter -EAGAIN for a send in io_uring, but if we do I'd say reenqueueing is the correct behavior. -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35078?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: I6da2653d32aedd0e7872be0cf90a841b56462e59 Gerrit-Change-Number: 35078 Gerrit-PatchSet: 2 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-Comment-Date: Wed, 29 Nov 2023 17:04:12 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: laforge <laforge(a)osmocom.org> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: daniel <dwillmann(a)sysmocom.de> Comment-In-Reply-To: lynxis lazus <lynxis(a)fe80.eu> Gerrit-MessageType: comment

1 0

[M] Change in libosmocore[master]: osmo_io: Factor out and use common send function from backend
by daniel 29 Nov '23

29 Nov '23

Attention is currently required from: laforge, lynxis lazus, pespin. Hello Jenkins Builder, lynxis lazus, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmocore/+/35078?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Code-Review+1 by lynxis lazus, Verified-1 by Jenkins Builder Change subject: osmo_io: Factor out and use common send function from backend ...................................................................... osmo_io: Factor out and use common send function from backend This handles reenqueuing a message on EAGAIN and incomplete write Change-Id: I6da2653d32aedd0e7872be0cf90a841b56462e59 --- M src/core/osmo_io.c M src/core/osmo_io_internal.h M src/core/osmo_io_poll.c M src/core/osmo_io_uring.c 4 files changed, 57 insertions(+), 57 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/78/35078/2 -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35078?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: I6da2653d32aedd0e7872be0cf90a841b56462e59 Gerrit-Change-Number: 35078 Gerrit-PatchSet: 2 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: lynxis lazus <lynxis(a)fe80.eu> Gerrit-CC: laforge <laforge(a)osmocom.org> Gerrit-CC: pespin <pespin(a)sysmocom.de> Gerrit-Attention: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu> Gerrit-MessageType: newpatchset

1 0

[S] Change in libosmocore[master]: utils: Link with libosmoisdn to avoid undefined references
by daniel 29 Nov '23

29 Nov '23

daniel has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/35163?usp=email ) Change subject: utils: Link with libosmoisdn to avoid undefined references ...................................................................... utils: Link with libosmoisdn to avoid undefined references make[3]: Entering directory '/home/daniel/scm/osmo/libosmocore/utils' CCLD osmo-arfcn /usr/bin/ld: ../src/gsm/.libs/libosmogsm.so: undefined reference to `lapd_dl_set_flags(a)LIBOSMOISDN_1.0' /usr/bin/ld: ../src/gsm/.libs/libosmogsm.so: undefined reference to `lapd_t200_timeout(a)LIBOSMOISDN_1.0' /usr/bin/ld: ../src/gsm/.libs/libosmogsm.so: undefined reference to `lapd_ph_rts_ind(a)LIBOSMOISDN_1.0' collect2: error: ld returned 1 exit status Change-Id: I9e99231899b269a4ec0faf720aff4dc8d2b74323 --- M utils/Makefile.am 1 file changed, 17 insertions(+), 1 deletion(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/63/35163/1 diff --git a/utils/Makefile.am b/utils/Makefile.am index 3ec71ea..0e11601 100644 --- a/utils/Makefile.am +++ b/utils/Makefile.am @@ -3,7 +3,7 @@ noinst_PROGRAMS = AM_CPPFLAGS = -I$(top_srcdir)/include -I$(top_builddir)/include AM_CFLAGS = -Wall $(TALLOC_CFLAGS) $(PTHREAD_CFLAGS) -LDADD = $(top_builddir)/src/core/libosmocore.la $(top_builddir)/src/gsm/libosmogsm.la $(PTHREAD_LIBS) +LDADD = $(top_builddir)/src/core/libosmocore.la $(top_builddir)/src/isdn/libosmoisdn.la $(top_builddir)/src/gsm/libosmogsm.la $(PTHREAD_LIBS) if ENABLE_UTILITIES EXTRA_DIST = conv_gen.py conv_codes_gsm.py -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/35163?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: I9e99231899b269a4ec0faf720aff4dc8d2b74323 Gerrit-Change-Number: 35163 Gerrit-PatchSet: 1 Gerrit-Owner: daniel <dwillmann(a)sysmocom.de> Gerrit-MessageType: newchange

1 0

[S] Change in osmo-pcu[master]: gprs_rlcmac_sched: rewrite logic around idle block skip
by pespin 29 Nov '23

29 Nov '23

Attention is currently required from: dexter, fixeria. pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-pcu/+/35151?usp=email ) Change subject: gprs_rlcmac_sched: rewrite logic around idle block skip ...................................................................... Patch Set 2: (1 comment) File src/gprs_rlcmac_sched.cpp: https://gerrit.osmocom.org/c/osmo-pcu/+/35151/comment/47256c5d_e54ac3a3 PS1, Line 486: * way we help BTS energy saving (on TRX!=C0) by sending nothing > Was this ever tested? Not myself, but I'd expect them to transmit nothing or whatever? maybe @laforge@osmocom.org can say from the top of his head since he knows better that l1 part. It makes sense that it sends nothing, and hence why we have to force it to transmit a dummy rlcmac block on trx0. -- To view, visit https://gerrit.osmocom.org/c/osmo-pcu/+/35151?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings Gerrit-Project: osmo-pcu Gerrit-Branch: master Gerrit-Change-Id: Iadb62748b18605bf158169b317f880352bc0a5a6 Gerrit-Change-Number: 35151 Gerrit-PatchSet: 2 Gerrit-Owner: dexter <pmaier(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-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Wed, 29 Nov 2023 16:37:20 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de> Comment-In-Reply-To: pespin <pespin(a)sysmocom.de> Comment-In-Reply-To: dexter <pmaier(a)sysmocom.de> Gerrit-MessageType: comment

1 0