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
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
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
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
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
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
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
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
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