osmith has uploaded this change for review. ( https://gerrit.osmocom.org/c/libgtpnl/+/29262 )
Change subject: README: fix link to homepage
......................................................................
README: fix link to homepage
Old link is 404, so adjust it.
Change-Id: I9ae4e9cb53618f4119170eb00e771c9033c52229
---
M README.md
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libgtpnl refs/changes/62/29262/1
diff --git a/README.md b/README.md
index 23c1d25..5d3b5bf 100644
--- a/README.md
+++ b/README.md
@@ -13,7 +13,7 @@
## Homepage
The official homepage of the project is
-<https://osmocom.org/projects/libgtpnl/wiki/>
+<https://osmocom.org/projects/linux-kernel-gtp-u/wiki/Libgtpnl>
GIT Repository
--------------
--
To view, visit https://gerrit.osmocom.org/c/libgtpnl/+/29262
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libgtpnl
Gerrit-Branch: master
Gerrit-Change-Id: I9ae4e9cb53618f4119170eb00e771c9033c52229
Gerrit-Change-Number: 29262
Gerrit-PatchSet: 1
Gerrit-Owner: osmith <osmith(a)sysmocom.de>
Gerrit-MessageType: newchange
Attention is currently required from: neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29256 )
Change subject: osmux: Drop long time deprecated APIs
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://gerrit.osmocom.org/c/libosmo-netif/+/29256/comment/1acc0f5e_25bb4b63
PS2, Line 10: been
> "not been"? […]
We'll require a major bump anyway since the whole API will be improved/extended/changed (see for instance the new _alloc() function in follow up commit).
The idea of deprecating APIs is not to keep them forever, but to signal users that the API will be removed at some point, and give some rational time to them to do move to the new APIs.
4 years of time given the amount of users of this API looks more than reasonable to me.
File src/osmux.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/29256/comment/d9df38cb_756f393a
PS2, Line 49: #if 0
> this is "deprecated API" ?
Yes, it's implementation stuff from APIs being removed. After those are gone, this is not needed and hence it is removed as part of the API implementation.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29256
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Icbdd364a8161a8113dbf1406716946f684d0a853
Gerrit-Change-Number: 29256
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 01 Sep 2022 11:52:37 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29257 )
Change subject: osmux: Move osmux_xfrm_output_set_tx_cb() further down to the xfrm_output section
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29257
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I46628d1f712e9c5c56c306e27c34ff45731c0172
Gerrit-Change-Number: 29257
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 01 Sep 2022 11:46:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29256 )
Change subject: osmux: Drop long time deprecated APIs
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://gerrit.osmocom.org/c/libosmo-netif/+/29256/comment/feab05bb_7d3307bd
PS2, Line 10: been
"not been"?
what makes you so sure, the idea behind keeping deprecated API is to remain backwards compatible *forever*. dropping would require a major version bump.
if they make things hard to understand, maybe just move to the bottom / a new file
File src/osmux.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/29256/comment/e8ca9dcc_88b9b269
PS2, Line 49: #if 0
this is "deprecated API" ?
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29256
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Icbdd364a8161a8113dbf1406716946f684d0a853
Gerrit-Change-Number: 29256
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 01 Sep 2022 11:45:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29258 )
Change subject: osmux: Allocate struct osmux_out_handle through API
......................................................................
Patch Set 4: Code-Review+1
(1 comment)
File src/osmux.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/29258/comment/e8054752_9c585571
PS4, Line 873: /* Returned pointer can be freed with regular talloc_free, queue will be flushed
should be doxygen comment?
/*!
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29258
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Ie8df581f375c9a183a7af60b431561bda82f6e34
Gerrit-Change-Number: 29258
Gerrit-PatchSet: 4
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 01 Sep 2022 11:38:51 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: laforge, pespin, fixeria, msuraev.
neels 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:
(9 comments)
File src/sccp_scoc.c:
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/cef92837_7aa7730a
PS14, Line 1110: xua_opt_data_send_cache(conn, SUA_CO_CORE, xua->hdr.msg_class);
> I think I'm missing a point in here. […]
maybe i was misreading this code. It looks to me like below scu_gen_encode_and_send(N_CONNECT, CONFIRM) *sends* a Conn Conf; that made me assume this here is the side that received the Conn Req and is responding with a CC.
Then again above case "CC_IND" looks like this *receives* a Conn Conf, didn't see that before.. now i'm confused between the prims, which side is which.
File src/sccp_scoc.c:
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/b76d4a42_2892dd9f
PS15, Line 642: /* optional: importance */
> The RLC does not have optional importance field.
like i said before, a patch should ideally do one thing.
To fix other bits along with it, just put it in a separate commit.
it is an act of courtesy to code reviewers; it is harder to read a complex patch when unrelated fixes go with it.
it also makes it possible to work with patches as "atoms", though we hardly ever need that part of it.
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/e9d63318_a58c7883
PS15, Line 635: LOGP(DLSCCP, LOGL_ERROR, "replacing unsent %u bytes of optional data cache with %s optional data\n",
> AFAIU that's something coming from outside, from a peer, so not really an error of the program itsel […]
I understand, this case is where we want to cache optional data, but there already is other data in the cache. Is this even possible to happen? ... maybe if we send a CR to the peer, but receive no response, and try again with the same CR. Then we already have data in the cache that could not be sent -- so seeing this error means that it is a software bug, did not clean up unsent cached data after a failure??
IMHO a comment and the log message could clarify this, like "ERROR: caching optional data for N-CONNECT, but there already is optional data occupying the cache. Dropping previous data."
ERROR category is good.
File src/sccp_scoc.c:
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/264d5726_895dcaeb
PS16, Line 592: nt exp_type
could you explain a situation where there might be a mismatch from the expected type? Isn't it always DT1 to be sent at the right time?
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/24694b57_c30425dc
PS16, Line 638: msgb_trim(conn->opt_data_cache, 0);
(i'd prefer sanitation: msgb_free() here, and always alloc a new one.)
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/d76a2fe0_c812277d
PS16, Line 667: see Figure C.3 / Q.714 (sheet 2 of 6) */
(osmocom asks to put '*' on every line of comment)
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/d0fb57e1_5cc3e68f
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 cache at all, the conn is still open and no cache is ever involved. it is a protocol error caused by the caller. Is it even possible / likely?
Also not an overrun. There is plenty of space in a msgb, there is no previous data in the cache...
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/2949a077_4679ee42
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 message. */
https://gerrit.osmocom.org/c/libosmo-sccp/+/29084/comment/c84d1859_9f367b02
PS16, Line 712: ount
(again modifying unrelated comments)
--
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: laforge <laforge(a)osmocom.org>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: msuraev <msuraev(a)sysmocom.de>
Gerrit-Comment-Date: Thu, 01 Sep 2022 11:27:45 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Comment-In-Reply-To: msuraev <msuraev(a)sysmocom.de>
Gerrit-MessageType: comment