Hello Jenkins Builder,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-netif/+/29260
to look at the new patch set (#3).
Change subject: osmux: Allow the user to alloc msgbs used to provide generated RTP packets
......................................................................
osmux: Allow the user to alloc msgbs used to provide generated RTP packets
This is useful for users of the API which need to keep forwarding the
msgb to lower layers which may need prepending a new header to the msgb,
like osmo-bts with l1sap.
Related: OS#5987
Change-Id: I632654221826340423e1e97b0f8ed9a2baf6c6c3
---
M include/osmocom/netif/osmux.h
M src/osmux.c
2 files changed, 27 insertions(+), 4 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/60/29260/3
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29260
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I632654221826340423e1e97b0f8ed9a2baf6c6c3
Gerrit-Change-Number: 29260
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: msuraev.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29178 )
Change subject: Properly handle send() return code
......................................................................
Patch Set 1:
(1 comment)
File src/stream.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/29178/comment/672fe614_f22e5884
PS1, Line 1334: LOGP(DLINP, LOGL_ERROR, "error to send: %s\n", (ret == -1) ? strerror(errno) : strerror(-ret));
> I didn't said that I don't care - see the comment above.
why do you say you are "not terribly concerned with" such systems? Do you know the code errors can change even within linux system running on different platforms?
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29178
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: If9b80e855b740254d5414ea50b4ce594855da8e9
Gerrit-Change-Number: 29178
Gerrit-PatchSet: 1
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Sep 2022 11:37:09 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels, msuraev.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/29284 )
Change subject: SIGTRAN: arrange the comments in the encoder to match the spec
......................................................................
Patch Set 1: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/29284
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: Ib986137057856afb8725541d912db210a9a46294
Gerrit-Change-Number: 29284
Gerrit-PatchSet: 1
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Sep 2022 11:35:34 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: osmith, fixeria.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/29279 )
Change subject: llc: gprs_llc_hdr_parse(): make the input data pointer const
......................................................................
Patch Set 1:
(1 comment)
File src/gprs/gprs_llc_parse.c:
https://gerrit.osmocom.org/c/osmo-sgsn/+/29279/comment/0d401120_e0cfa9cc
PS1, Line 138: ghp->data = (uint8_t *)&ctrl[3];
> > You are also telling the user that calling the function won't make any kind of modification at any […]
The way those functions are writen is controversial and basically reflects some limitations of C (no overloading). Ideally there'd be 2 functions, one const and nother non-const.
See for instance https://stackoverflow.com/questions/16860418/why-strrchr-returns-char-inste…
I see your point. But I still think in this caseit only confuses readers by pretending this function will not conclude in that buffer being modified at some point after being passed to it.
So I really see no advantage in using const there, and a lot of possible drawbacks.
--
To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/29279
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-sgsn
Gerrit-Branch: master
Gerrit-Change-Id: I9757d2be5af589ccbe4d3d406637a33690284754
Gerrit-Change-Number: 29279
Gerrit-PatchSet: 1
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Sep 2022 11:32:26 +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>
Gerrit-MessageType: comment
msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29178 )
Change subject: Properly handle send() return code
......................................................................
Patch Set 1:
(1 comment)
File src/stream.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/29178/comment/f23f0cf3_1a9106f9
PS1, Line 1334: LOGP(DLINP, LOGL_ERROR, "error to send: %s\n", (ret == -1) ? strerror(errno) : strerror(-ret));
> why do you say you don't care about such systems? Do you know the code errors can change even within […]
I didn't said that I don't care - see the comment above.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29178
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: If9b80e855b740254d5414ea50b4ce594855da8e9
Gerrit-Change-Number: 29178
Gerrit-PatchSet: 1
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Sep 2022 11:00:15 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: fixeria <vyanitskiy(a)sysmocom.de>
Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de>
Gerrit-MessageType: comment
msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/29284 )
Change subject: SIGTRAN: arrange the comments in the encoder to match the spec
......................................................................
Set Ready For Review
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/29284
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: Ib986137057856afb8725541d912db210a9a46294
Gerrit-Change-Number: 29284
Gerrit-PatchSet: 1
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-CC: Jenkins Builder
Gerrit-Comment-Date: Wed, 07 Sep 2022 10:39:17 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: neels, laforge, pespin, fixeria.
Hello Jenkins Builder, neels, laforge, fixeria,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084
to look at the new patch set (#17).
Change subject: SIGTRAN: cache Optional Data for SCCP CR/CC/RLSD
......................................................................
SIGTRAN: cache Optional Data for SCCP CR/CC/RLSD
The length limit of optional Data parameter is 130 bytes according to ITU-T Rec Q.713 §4.2..§4.5. If we receive CR, CC or
RLSD message with bigger data - cache it if necessary and send via separate DT1 message after connection becomes active.
Fixes: OS#5579
Change-Id: I0033faf9da393418930252233ce74d62cd1cef8a
---
M src/sccp_scoc.c
1 file changed, 130 insertions(+), 13 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-sccp refs/changes/84/29084/17
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/29084
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: I0033faf9da393418930252233ce74d62cd1cef8a
Gerrit-Change-Number: 29084
Gerrit-PatchSet: 17
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-MessageType: newpatchset
Attention is currently required from: neels, laforge, pespin, fixeria.
msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-sccp/+/29084 )
Change subject: SIGTRAN: cache Optional Data for SCCP CR/CC/RLSD
......................................................................
Patch Set 16:
(14 comments)
File src/sccp_scoc.c:
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/e310bcbb_d100f350
PS14, Line 1110: xua_opt_data_send_cache(conn, SUA_CO_CORE, xua->hdr.msg_class);
> maybe i was misreading this code. […]
Ack
File src/sccp_scoc.c:
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/fe2b440c_1c650313
PS15, Line 642: /* optional: importance */
> like i said before, a patch should ideally do one thing. […]
Done
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/260bad71_8dd729c7
PS15, Line 600: if (
> Simply drop it then.
Done
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/050a586c_5d9ec7ac
PS15, Line 606: } else
> not critical, but IMHO there should be curly braces. […]
Ack
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/73da6543_4b9fb0b5
PS15, Line 606: } else
> not critical, but IMHO there should be curly braces. […]
Ack
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/3a90071f_211ad50f
PS15, Line 635: LOGP(DLSCCP, LOGL_ERROR, "replacing unsent %u bytes of optional data cache with %s optional data\n",
> I understand, this case is where we want to cache optional data, but there already is other data in […]
Ack
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/87ac74a3_b8be3def
PS15, Line 635: LOGP(DLSCCP, LOGL_ERROR, "replacing unsent %u bytes of optional data cache with %s optional data\n",
> I understand, this case is where we want to cache optional data, but there already is other data in […]
Ack
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/7138c7ce_a9fe6137
PS15, Line 784: xua_msg_add_sccp_addr(xua, SUA_IEI_DEST_ADDR, &conn->calling_addr);
> Then write a new comit "comments are placed to match the fields order in the spec. […]
Done
File src/sccp_scoc.c:
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/b8ad7bb9_9260bd9a
PS16, Line 592: nt exp_type
> could you explain a situation where there might be a mismatch from the expected type? Isn't it alway […]
Both expected types refer to the message which was source of the optional data. This check is simply an additional safeguard to ensure we hadn't screwed up while adjusting FSM. The caching happens in one place, sending in another but we always know what kind of Optional Data we're about to send (i. e. from which message it was cached). So here we compare the type of the message as recorded while caching with the type of the message FSM thinks it's sending.
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/441d202f_57a781cb
PS16, Line 638: msgb_trim(conn->opt_data_cache, 0);
> (i'd prefer sanitation: msgb_free() here, and always alloc a new one. […]
I don't: if msgb_trim() is implemented properly there should be no difference in our case (fixed buffer size) besides wasted CPU cycle on unnecessary function calls.
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/d2098370_4a00e0bd
PS16, Line 667: see Figure C.3 / Q.714 (sheet 2 of 6) */
> (osmocom asks to put '*' on every line of comment)
Done
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/994a5e32_94ddf4ec
PS16, Line 673: if (xua_drop_data_check_drop(prim, SCCP_MAX_DATA, "cache overrun"))
> the optional data is larger than the upper limit of an SCCP DATA section -- this is not related to c […]
Done
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/5099038f_2e5fe8c1
PS16, Line 675: /* There's no need to cache the optional data since the connection is still active at this point */
> /* Send the Optional Data in a DT1 ahead of the RLSD, because it is too large to be sent in one mess […]
Done
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/b2b8d5d3_9fe7a406
PS16, Line 712: ount
> (again modifying unrelated comments)
Done
--
To view, visit https://gerrit.osmocom.org/c/libosmo-sccp/+/29084
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-sccp
Gerrit-Branch: master
Gerrit-Change-Id: I0033faf9da393418930252233ce74d62cd1cef8a
Gerrit-Change-Number: 29084
Gerrit-PatchSet: 16
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-CC: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Sep 2022 10:37:23 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels, dexter.
msuraev has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-msc/+/28846 )
Change subject: Make esme struct shared
......................................................................
Patch Set 19:
(1 comment)
File src/utils/Makefile.am:
https://gerrit.osmocom.org/c/osmo-msc/+/28846/comment/567a72e9_ac590b4c
PS19, Line 45: $(top_builddir)/src/libvlr/libvlr.a \
> smpp_mirror certainly does not need libvlr.a. […]
I completely agree - this problems dates back to BSC/MSC split and the shared gsm_network struct which includes everything and a kitchen sink and as a result pulls those dependencies. Resolving that is outside of the scope for those patches though.
--
To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/28846
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-msc
Gerrit-Branch: master
Gerrit-Change-Id: I8f7ac2c00d16660925dd0b03aa1a0973edf9eb70
Gerrit-Change-Number: 28846
Gerrit-PatchSet: 19
Gerrit-Owner: msuraev <msuraev(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: dexter <pmaier(a)sysmocom.de>
Gerrit-Comment-Date: Wed, 07 Sep 2022 09:52:05 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment