Attention is currently required from: pespin.
osmith has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-abis/+/38988?usp=email )
Change subject: Drop internal use of osmocom/abis/ipaccess.h
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38988?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: Ifc9759b17a9b966cb713f7533a13f6223161a4cd
Gerrit-Change-Number: 38988
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: osmith <osmith(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Mon, 02 Dec 2024 12:39:27 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-abis/+/38997?usp=email )
Change subject: ipaccess: Assert fd is positive
......................................................................
ipaccess: Assert fd is positive
Make coverity happy. We are in a rx path from socket, so socket should
be fine at that point.
Closes: Coverity CID#435266
Change-Id: I09004040107fb72b6170618a6253a6225f4351c2
---
M src/input/ipaccess.c
1 file changed, 1 insertion(+), 0 deletions(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/97/38997/1
diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c
index 5a24686..505ec3f 100644
--- a/src/input/ipaccess.c
+++ b/src/input/ipaccess.c
@@ -958,6 +958,7 @@
/* ping, pong and acknowledgment cases. */
struct osmo_fd tmp_ofd = { .fd = osmo_stream_cli_get_fd(cli) };
+ OSMO_ASSERT(tmp_ofd.fd >= 0);
ret = ipa_ccm_rcvmsg_bts_base(msg, &tmp_ofd);
if (ret < 0)
goto err;
--
To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38997?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: newchange
Gerrit-Project: libosmo-abis
Gerrit-Branch: master
Gerrit-Change-Id: I09004040107fb72b6170618a6253a6225f4351c2
Gerrit-Change-Number: 38997
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Attention is currently required from: lynxis lazus.
pespin has posted comments on this change by lynxis lazus. ( https://gerrit.osmocom.org/c/libosmocore/+/38996?usp=email )
Change subject: gsm48: add additional GSM 24.008 IE for GMM
......................................................................
Patch Set 1: Code-Review+1
(1 comment)
Patchset:
PS1:
Please remember to add it to the tlv defintion in osmo-sgsn and libosmo-gprs.
--
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/38996?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: comment
Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I4824a319948f1088137c13d1cf610a1b1c2529f2
Gerrit-Change-Number: 38996
Gerrit-PatchSet: 1
Gerrit-Owner: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: lynxis lazus <lynxis(a)fe80.eu>
Gerrit-Comment-Date: Mon, 02 Dec 2024 10:20:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38981?usp=email )
Change subject: ipaccess-proxy: Fix wrong cb function passed
......................................................................
ipaccess-proxy: Fix wrong cb function passed
It makes no sense that ipaccess-proxy is using the ipaccess_fd_cb()
function from libosmo-abis, since that function expects an e1_input
ipaccess driver to be set up by the app, which ipaccess-proxy never sets
up. Furthermore, even the data pointer being passed doesn't match the
one expected by the callback.
This seems to be a bug introduced 10 years ago in osmo-bsc.git
eb623019382fdbff36fb8a72052732e225aef67e, where the name of the private
function callback used by ipaccess-proxy was changed to
proxy_ipaccess_fd_cb() to avoid colliding with the ipaccess_fd_cb() from
libosmo-abis, but only 1 of the 2 references was changed at the time,
leaving the other one pointing to the wrong function.
Fixes: eb623019382fdbff36fb8a72052732e225aef67e
Change-Id: I1702d9913a462a36e4b78b503a8338ae2ad140b1
---
M src/ipaccess/ipaccess-proxy.c
1 file changed, 1 insertion(+), 1 deletion(-)
Approvals:
osmith: Looks good to me, but someone else must approve
Jenkins Builder: Verified
fixeria: Looks good to me, approved
diff --git a/src/ipaccess/ipaccess-proxy.c b/src/ipaccess/ipaccess-proxy.c
index 7ede283..62e79cd 100644
--- a/src/ipaccess/ipaccess-proxy.c
+++ b/src/ipaccess/ipaccess-proxy.c
@@ -1021,7 +1021,7 @@
}
bfd = &ipc->fd;
- osmo_fd_setup(bfd, ret, OSMO_FD_READ | OSMO_FD_WRITE, ipaccess_fd_cb, ipc, priv_nr);
+ osmo_fd_setup(bfd, ret, OSMO_FD_READ | OSMO_FD_WRITE, proxy_ipaccess_fd_cb, ipc, priv_nr);
ret = setsockopt(bfd->fd, SOL_SOCKET, SO_REUSEADDR, &on, sizeof(on));
if (ret < 0) {
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38981?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: I1702d9913a462a36e4b78b503a8338ae2ad140b1
Gerrit-Change-Number: 38981
Gerrit-PatchSet: 1
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: osmith <osmith(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/38987?usp=email )
Change subject: stream_cli: Fix discard 1st msg received quick after connect
......................................................................
stream_cli: Fix discard 1st msg received quick after connect
Even if setting osmo_iofd_notify_connected(), it may happen that a read
call-back is triggered towards the user before this
special write-callback is triggered.
stream_cli was already accounting for that in stream_cli_iofd_read_cb()
state STREAM_CLI_STATE_CONNECTING, but was discarding the msgb instead
of pushing it upwards towards the user through read_cb after
transitioning to STREAM_CLI_STATE_CONNECTED.
As a result, eg when an ipaccess client using stream_cli (BTS, liboamo-abis
e1_line ipaccess driver) connected to an ipaccess server (BSC) and the
server quickly transmitted an IPA ID GET, it would get lost.
Related: libosmocore.git Change-Id Ica20a050b98d117995a5b625b23ab9faa61aabee
Change-Id: I98cd51d4bb87d3572245446648ced44a23a622ef
---
M src/stream_cli.c
1 file changed, 50 insertions(+), 31 deletions(-)
Approvals:
pespin: Looks good to me, approved
Jenkins Builder: Verified
fixeria: Looks good to me, but someone else must approve
diff --git a/src/stream_cli.c b/src/stream_cli.c
index fe62607..48a8858 100644
--- a/src/stream_cli.c
+++ b/src/stream_cli.c
@@ -409,7 +409,8 @@
#endif
}
-static void stream_cli_handle_connecting(struct osmo_stream_cli *cli, int res)
+/* returns true if cli is freed */
+static bool stream_cli_handle_connecting(struct osmo_stream_cli *cli, int res)
{
int error, ret = res;
socklen_t len = sizeof(error);
@@ -419,14 +420,12 @@
if (ret < 0) {
LOGSCLI(cli, LOGL_ERROR, "connect failed (%d)\n", res);
- (void)stream_cli_reconnect(cli);
- return;
+ return stream_cli_reconnect(cli);
}
ret = getsockopt(fd, SOL_SOCKET, SO_ERROR, &error, &len);
if (ret >= 0 && error > 0) {
LOGSCLI(cli, LOGL_ERROR, "connect so_error (%d)\n", error);
- (void)stream_cli_reconnect(cli);
- return;
+ return stream_cli_reconnect(cli);
}
/* If messages got enqueued while 'connecting', keep WRITE flag
@@ -458,7 +457,7 @@
if (cli->connect_cb)
cli->connect_cb(cli);
cli->in_cb_mask &= ~IN_CB_MASK_CONNECT_CB;
- (void)free_delayed_if_needed(cli);
+ return free_delayed_if_needed(cli);
}
static int osmo_stream_cli_fd_cb(struct osmo_fd *ofd, unsigned int what)
@@ -528,8 +527,14 @@
switch (cli->state) {
case STREAM_CLI_STATE_CONNECTING:
- msgb_free(msg);
- stream_cli_handle_connecting(cli, res);
+ freed = stream_cli_handle_connecting(cli, res);
+ if (freed)
+ return; /* msg was also freed as part of the talloc tree. */
+ if (cli->state != STREAM_CLI_STATE_CONNECTED) {
+ msgb_free(msg);
+ return;
+ }
+ /* Follow below common path submitting read_cb(msg) to user. */
break;
case STREAM_CLI_STATE_CONNECTED:
switch (res) {
@@ -549,35 +554,41 @@
}
if (freed)
return; /* msg was also freed as part of the talloc tree. */
- /* Notify user of new data or error: */
- if (!cli->iofd_read_cb) {
- msgb_free(msg);
- return;
- }
- cli->in_cb_mask |= IN_CB_MASK_READ_CB;
- cli->iofd_read_cb(cli, res, msg);
- cli->in_cb_mask &= ~IN_CB_MASK_READ_CB;
- OSMO_ASSERT(cli->in_cb_mask == 0);
- (void)free_delayed_if_needed(cli);
+ /* Follow below common path submitting read_cb(msg) to user. */
break;
default:
osmo_panic("%s() called with unexpected state %d\n", __func__, cli->state);
}
+
+ /* Notify user of new data or error: */
+ if (!cli->iofd_read_cb) {
+ msgb_free(msg);
+ return;
+ }
+ cli->in_cb_mask |= IN_CB_MASK_READ_CB;
+ cli->iofd_read_cb(cli, res, msg);
+ cli->in_cb_mask &= ~IN_CB_MASK_READ_CB;
+ OSMO_ASSERT(cli->in_cb_mask == 0);
+ (void)free_delayed_if_needed(cli);
}
static void stream_cli_iofd_write_cb(struct osmo_io_fd *iofd, int res, struct msgb *msg)
{
struct osmo_stream_cli *cli = osmo_iofd_get_data(iofd);
+ /* msgb is not owned by us here, no need to free it. */
switch (cli->state) {
case STREAM_CLI_STATE_CONNECTING:
- stream_cli_handle_connecting(cli, res);
+ (void)stream_cli_handle_connecting(cli, res);
break;
case STREAM_CLI_STATE_CONNECTED:
if (msg && res <= 0) {
LOGSCLI(cli, LOGL_ERROR, "received error %d in response to send\n", res);
(void)stream_cli_reconnect(cli);
}
+ /* res=0 && msgb=NULL: "connected notify", but we already received before a read_cb
+ * which moved us to CONNECTED state. Do nothing.
+ */
break;
default:
osmo_panic("%s() called with unexpected state %d\n", __func__, cli->state);
@@ -601,8 +612,14 @@
switch (cli->state) {
case STREAM_CLI_STATE_CONNECTING:
- msgb_free(msg);
- stream_cli_handle_connecting(cli, res);
+ freed = stream_cli_handle_connecting(cli, res);
+ if (freed)
+ return; /* msg was also freed as part of the talloc tree. */
+ if (cli->state != STREAM_CLI_STATE_CONNECTED) {
+ msgb_free(msg);
+ return;
+ }
+ /* Follow below common path submitting read_cb(msg) to user. */
break;
case STREAM_CLI_STATE_CONNECTED:
switch (res) {
@@ -621,20 +638,22 @@
}
if (freed)
return; /* msg was also freed as part of the talloc tree. */
- /* Notify user of new data or error: */
- if (!cli->iofd_read_cb) {
- msgb_free(msg);
- return;
- }
- cli->in_cb_mask |= IN_CB_MASK_READ_CB;
- cli->iofd_read_cb(cli, res, msg);
- cli->in_cb_mask &= ~IN_CB_MASK_READ_CB;
- OSMO_ASSERT(cli->in_cb_mask == 0);
- (void)free_delayed_if_needed(cli);
+ /* Follow below common path submitting read_cb(msg) to user. */
break;
default:
osmo_panic("%s() called with unexpected state %d\n", __func__, cli->state);
}
+
+ /* Notify user of new data or error: */
+ if (!cli->iofd_read_cb) {
+ msgb_free(msg);
+ return;
+ }
+ cli->in_cb_mask |= IN_CB_MASK_READ_CB;
+ cli->iofd_read_cb(cli, res, msg);
+ cli->in_cb_mask &= ~IN_CB_MASK_READ_CB;
+ OSMO_ASSERT(cli->in_cb_mask == 0);
+ (void)free_delayed_if_needed(cli);
}
static const struct osmo_io_ops osmo_stream_cli_ioops_sctp = {
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/38987?usp=email
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email
Gerrit-MessageType: merged
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I98cd51d4bb87d3572245446648ced44a23a622ef
Gerrit-Change-Number: 38987
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: daniel <dwillmann(a)sysmocom.de>
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: jolly <andreas(a)eversberg.eu>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>