- gerrit-log - lists.osmocom.org

[M] Change in osmo-bsc[master]: Drop use of libosmo-abis osmocom/abis/ipaccess.h
by pespin 29 Nov '24

29 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/38991?usp=email ) Change subject: Drop use of libosmo-abis osmocom/abis/ipaccess.h ...................................................................... Drop use of libosmo-abis osmocom/abis/ipaccess.h That header is only really used to provide an old hack for ipaccess-proxy tool in openbsc.h/osmo-bsc.h, and will be deprecated soon. src/ipaccess/ipaccess-config.c already has an explicit extern declaration for ipacces_fd_cb() anyway, so drop the header in advance. Take the chance to organize a bit better the includes based on dependency chain. Change-Id: I5f9daaf7789a56a08d2ec24b5e25e2e3b65b8b5b --- M src/ipaccess/ipaccess-config.c M src/ipaccess/ipaccess-proxy.c M src/osmo-bsc/bts_ipaccess_nanobts.c M src/osmo-bsc/bts_osmobts.c M src/osmo-bsc/e1_config.c 5 files changed, 30 insertions(+), 34 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/91/38991/1 diff --git a/src/ipaccess/ipaccess-config.c b/src/ipaccess/ipaccess-config.c index 720837e..15dcc54 100644 --- a/src/ipaccess/ipaccess-config.c +++ b/src/ipaccess/ipaccess-config.c @@ -40,21 +40,21 @@ #include <osmocom/core/application.h> #include <osmocom/core/select.h> #include <osmocom/core/timer.h> -#include <osmocom/bsc/ipaccess.h> +#include <osmocom/core/talloc.h> +#include <osmocom/gsm/protocol/gsm_12_21.h> +#include <osmocom/gsm/protocol/ipaccess.h> +#include <osmocom/abis/abis.h> #include <osmocom/abis/e1_input.h> +#include <osmocom/bsc/ipaccess.h> #include <osmocom/bsc/abis_nm.h> #include <osmocom/bsc/signal.h> #include <osmocom/bsc/debug.h> #include <osmocom/bsc/network_listen.h> -#include <osmocom/abis/ipaccess.h> #include <osmocom/bsc/gsm_data.h> #include <osmocom/bsc/abis_nm.h> #include <osmocom/bsc/signal.h> #include <osmocom/bsc/debug.h> #include <osmocom/bsc/network_listen.h> -#include <osmocom/core/talloc.h> -#include <osmocom/abis/abis.h> -#include <osmocom/gsm/protocol/gsm_12_21.h> #include <osmocom/bsc/bss.h> #include <osmocom/bsc/bts.h> diff --git a/src/ipaccess/ipaccess-proxy.c b/src/ipaccess/ipaccess-proxy.c index 62e79cd..e659708 100644 --- a/src/ipaccess/ipaccess-proxy.c +++ b/src/ipaccess/ipaccess-proxy.c @@ -37,18 +37,18 @@ #define _GNU_SOURCE #include <getopt.h> -#include <osmocom/bsc/gsm_data.h> #include <osmocom/core/application.h> #include <osmocom/core/select.h> -#include <osmocom/gsm/tlv.h> #include <osmocom/core/msgb.h> -#include <osmocom/gsm/ipa.h> -#include <osmocom/abis/ipa.h> -#include <osmocom/abis/ipaccess.h> -#include <osmocom/bsc/debug.h> -#include <osmocom/bsc/ipaccess.h> #include <osmocom/core/socket.h> #include <osmocom/core/talloc.h> +#include <osmocom/gsm/tlv.h> +#include <osmocom/gsm/protocol/ipaccess.h> +#include <osmocom/gsm/ipa.h> +#include <osmocom/abis/ipa.h> +#include <osmocom/bsc/gsm_data.h> +#include <osmocom/bsc/debug.h> +#include <osmocom/bsc/ipaccess.h> /* one instance of an ip.access protocol proxy */ struct ipa_proxy { diff --git a/src/osmo-bsc/bts_ipaccess_nanobts.c b/src/osmo-bsc/bts_ipaccess_nanobts.c index 9382fa4..9f6d048 100644 --- a/src/osmo-bsc/bts_ipaccess_nanobts.c +++ b/src/osmo-bsc/bts_ipaccess_nanobts.c @@ -25,25 +25,23 @@ #include <osmocom/gsm/tlv.h> -#include <osmocom/bsc/gsm_data.h> -#include <osmocom/bsc/signal.h> -#include <osmocom/bsc/abis_nm.h> -#include <osmocom/abis/e1_input.h> -#include <osmocom/gsm/tlv.h> #include <osmocom/core/msgb.h> #include <osmocom/core/socket.h> #include <osmocom/core/stat_item.h> #include <osmocom/core/talloc.h> #include <osmocom/core/utils.h> +#include <osmocom/core/logging.h> +#include <osmocom/gsm/tlv.h> +#include <osmocom/gsm/protocol/ipaccess.h> +#include <osmocom/gsm/ipa.h> +#include <osmocom/abis/e1_input.h> +#include <osmocom/abis/subchan_demux.h> #include <osmocom/bsc/gsm_data.h> +#include <osmocom/bsc/signal.h> #include <osmocom/bsc/abis_nm.h> #include <osmocom/bsc/abis_rsl.h> #include <osmocom/bsc/abis_osmo.h> #include <osmocom/bsc/debug.h> -#include <osmocom/abis/subchan_demux.h> -#include <osmocom/gsm/ipa.h> -#include <osmocom/abis/ipaccess.h> -#include <osmocom/core/logging.h> #include <osmocom/bsc/ipaccess.h> #include <osmocom/bsc/bts_ipaccess_nanobts_omlattr.h> #include <osmocom/bsc/paging.h> diff --git a/src/osmo-bsc/bts_osmobts.c b/src/osmo-bsc/bts_osmobts.c index 9312a2a..1df9497 100644 --- a/src/osmo-bsc/bts_osmobts.c +++ b/src/osmo-bsc/bts_osmobts.c @@ -24,21 +24,19 @@ #include <osmocom/gsm/tlv.h> +#include <osmocom/core/msgb.h> +#include <osmocom/core/talloc.h> +#include <osmocom/core/logging.h> +#include <osmocom/gsm/tlv.h> +#include <osmocom/gsm/protocol/ipaccess.h> +#include <osmocom/abis/e1_input.h> +#include <osmocom/abis/subchan_demux.h> #include <osmocom/bsc/gsm_data.h> #include <osmocom/bsc/signal.h> #include <osmocom/bsc/abis_nm.h> #include <osmocom/bsc/bts.h> -#include <osmocom/abis/e1_input.h> -#include <osmocom/gsm/tlv.h> -#include <osmocom/core/msgb.h> -#include <osmocom/core/talloc.h> -#include <osmocom/bsc/gsm_data.h> -#include <osmocom/bsc/abis_nm.h> #include <osmocom/bsc/abis_rsl.h> #include <osmocom/bsc/debug.h> -#include <osmocom/abis/subchan_demux.h> -#include <osmocom/abis/ipaccess.h> -#include <osmocom/core/logging.h> extern struct gsm_bts_model bts_model_nanobts; diff --git a/src/osmo-bsc/e1_config.c b/src/osmo-bsc/e1_config.c index dbea3e9..aaf8b70 100644 --- a/src/osmo-bsc/e1_config.c +++ b/src/osmo-bsc/e1_config.c @@ -23,11 +23,11 @@ #include <time.h> #include <netinet/in.h> -#include <osmocom/bsc/gsm_data.h> -#include <osmocom/abis/e1_input.h> -#include <osmocom/bsc/misdn.h> -#include <osmocom/abis/ipaccess.h> #include <osmocom/core/talloc.h> +#include <osmocom/gsm/protocol/ipaccess.h> +#include <osmocom/abis/e1_input.h> +#include <osmocom/bsc/gsm_data.h> +#include <osmocom/bsc/misdn.h> #include <osmocom/bsc/debug.h> #include <osmocom/bsc/abis_rsl.h> #include <osmocom/bsc/bts.h> -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/38991?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I5f9daaf7789a56a08d2ec24b5e25e2e3b65b8b5b Gerrit-Change-Number: 38991 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[XS] Change in osmo-bts[master]: Drop use of libosmo-abis osmocom/abis/ipaccess.h
by pespin 29 Nov '24

29 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bts/+/38990?usp=email ) Change subject: Drop use of libosmo-abis osmocom/abis/ipaccess.h ...................................................................... Drop use of libosmo-abis osmocom/abis/ipaccess.h That header is only really used to provide an old hack for ipaccess-proxy tool in openbsc.h/osmo-bsc.h, and will be deprecated soon. Take the chance to organize a bit better the includes based on dependency chain. Change-Id: Ie4540eb6f535375a1e4ddf3849602aecb6260914 --- M src/common/abis.c M src/common/oml.c 2 files changed, 3 insertions(+), 3 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bts refs/changes/90/38990/1 diff --git a/src/common/abis.c b/src/common/abis.c index e619ec8..d9c56e9 100644 --- a/src/common/abis.c +++ b/src/common/abis.c @@ -39,10 +39,10 @@ #include <osmocom/core/signal.h> #include <osmocom/core/macaddr.h> #include <osmocom/core/fsm.h> +#include <osmocom/gsm/protocol/ipaccess.h> +#include <osmocom/gsm/ipa.h> #include <osmocom/abis/abis.h> #include <osmocom/abis/e1_input.h> -#include <osmocom/abis/ipaccess.h> -#include <osmocom/gsm/ipa.h> #include <osmo-bts/abis.h> #include <osmo-bts/logging.h> diff --git a/src/common/oml.c b/src/common/oml.c index a9e13b5..35b8664 100644 --- a/src/common/oml.c +++ b/src/common/oml.c @@ -36,10 +36,10 @@ #include <osmocom/core/msgb.h> #include <osmocom/core/utils.h> #include <osmocom/gsm/protocol/gsm_12_21.h> +#include <osmocom/gsm/protocol/ipaccess.h> #include <osmocom/gsm/abis_nm.h> #include <osmocom/gsm/tlv.h> #include <osmocom/abis/e1_input.h> -#include <osmocom/abis/ipaccess.h> #include <osmo-bts/logging.h> #include <osmo-bts/gsm_data.h> -- To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/38990?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-bts Gerrit-Branch: master Gerrit-Change-Id: Ie4540eb6f535375a1e4ddf3849602aecb6260914 Gerrit-Change-Number: 38990 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-hlr[master]: Drop use of libosmo-abis osmocom/abis/ipaccess.h
by pespin 29 Nov '24

29 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/38989?usp=email ) Change subject: Drop use of libosmo-abis osmocom/abis/ipaccess.h ...................................................................... Drop use of libosmo-abis osmocom/abis/ipaccess.h That header is only really used to provide an old hack for ipaccess-proxy tool in openbsc.h/osmo-bsc.h, and will be deprecated soon. Take the chance to organize a bit better the includes based on dependency chain. Change-Id: I393cd23a09af6d1eefa68dc955415f4fe334d71b --- M include/osmocom/hlr/gsup_server.h M src/gsup_server.c 2 files changed, 5 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/89/38989/1 diff --git a/include/osmocom/hlr/gsup_server.h b/include/osmocom/hlr/gsup_server.h index 86ae4ec..5767dec 100644 --- a/include/osmocom/hlr/gsup_server.h +++ b/include/osmocom/hlr/gsup_server.h @@ -2,9 +2,9 @@ #include <osmocom/core/linuxlist.h> #include <osmocom/core/msgb.h> -#include <osmocom/abis/ipa.h> -#include <osmocom/abis/ipaccess.h> #include <osmocom/gsm/gsup.h> +#include <osmocom/gsm/protocol/ipaccess.h> +#include <osmocom/abis/ipa.h> #include <osmocom/gsupclient/cni_peer_id.h> #include <osmocom/gsupclient/gsup_req.h> diff --git a/src/gsup_server.c b/src/gsup_server.c index 1510bd0..2df2d3d 100644 --- a/src/gsup_server.c +++ b/src/gsup_server.c @@ -24,12 +24,12 @@ #include <osmocom/core/msgb.h> #include <osmocom/core/logging.h> #include <osmocom/core/linuxlist.h> -#include <osmocom/abis/ipa.h> -#include <osmocom/abis/ipaccess.h> -#include <osmocom/gsm/ipa.h> #include <osmocom/gsm/gsm48_ie.h> #include <osmocom/gsm/apn.h> +#include <osmocom/gsm/protocol/ipaccess.h> +#include <osmocom/gsm/ipa.h> #include <osmocom/gsm/gsm23003.h> +#include <osmocom/abis/ipa.h> #include <osmocom/hlr/gsup_server.h> #include <osmocom/hlr/gsup_router.h> -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/38989?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-hlr Gerrit-Branch: master Gerrit-Change-Id: I393cd23a09af6d1eefa68dc955415f4fe334d71b Gerrit-Change-Number: 38989 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[L] Change in libosmo-abis[master]: WIP: ipaccess: Convert BSC OML & RSL link to use stream_srv
by pespin 29 Nov '24

29 Nov '24

Hello Jenkins Builder, I'd like you to reexamine a change. Please visit https://gerrit.osmocom.org/c/libosmo-abis/+/38983?usp=email to look at the new patch set (#2). The following approvals got outdated and were removed: Verified+1 by Jenkins Builder Change subject: WIP: ipaccess: Convert BSC OML & RSL link to use stream_srv ...................................................................... WIP: ipaccess: Convert BSC OML & RSL link to use stream_srv This in turn allows running BSC Abis interfaces through io-uring backend, which should provide performance improvements when used. TODO: Get rid of ipaccess_fd_cb() in osmo-bsc.git ipaccess-config by implementing its own e1i_line ipaccess driver there (due to many peculiarities: tcp/ipa cli instead of srv, different OML port). Related: SYS#7063 Related: OS#5755 Related: OS#5756 Change-Id: Idf241c8f2fdb86d090d4132a9b316b7236402232 --- M include/osmocom/abis/ipaccess.h M src/input/ipaccess.c 2 files changed, 271 insertions(+), 334 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/83/38983/2 -- To view, visit https://gerrit.osmocom.org/c/libosmo-abis/+/38983?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: Idf241c8f2fdb86d090d4132a9b316b7236402232 Gerrit-Change-Number: 38983 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder

1 0

[S] Change in libosmo-abis[master]: Drop internal use of osmocom/abis/ipaccess.h
by pespin 29 Nov '24

29 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-abis/+/38988?usp=email ) Change subject: Drop internal use of osmocom/abis/ipaccess.h ...................................................................... Drop internal use of osmocom/abis/ipaccess.h That header is only really used to provide an old hack for ipaccess-proxy tool in openbsc.h/osmo-bsc.h, and will be deprecated soon. Take the chance to organize a bit better the includes based on dependency chain. Change-Id: Ifc9759b17a9b966cb713f7533a13f6223161a4cd --- M src/input/ipa.c M src/input/ipaccess.c M src/ipa_proxy.c M tests/e1inp_ipa_bts_test.c 4 files changed, 16 insertions(+), 18 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-abis refs/changes/88/38988/1 diff --git a/src/input/ipa.c b/src/input/ipa.c index f902b95..f49d0dc 100644 --- a/src/input/ipa.c +++ b/src/input/ipa.c @@ -13,15 +13,14 @@ #include <osmocom/core/select.h> #include <osmocom/core/timer.h> -#include <osmocom/gsm/tlv.h> #include <osmocom/core/msgb.h> #include <osmocom/core/logging.h> #include <osmocom/core/talloc.h> -#include <osmocom/abis/e1_input.h> -#include <osmocom/abis/ipaccess.h> #include <osmocom/core/socket.h> #include <osmocom/core/backtrace.h> - +#include <osmocom/gsm/tlv.h> +#include <osmocom/gsm/protocol/ipaccess.h> +#include <osmocom/abis/e1_input.h> #include <osmocom/abis/ipa.h> #define LOGIPA(link, level, fmt, args...) LOGP(DLINP, level, "%s:%u " fmt, link->addr, link->port, ## args) diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c index af6e39d..94f1b2b 100644 --- a/src/input/ipaccess.c +++ b/src/input/ipaccess.c @@ -39,21 +39,21 @@ #include <arpa/inet.h> #include <osmocom/core/select.h> -#include <osmocom/gsm/tlv.h> #include <osmocom/core/msgb.h> #include <osmocom/core/macaddr.h> #include <osmocom/core/logging.h> #include <osmocom/core/talloc.h> -#include <osmocom/abis/e1_input.h> -#include <osmocom/abis/ipaccess.h> #include <osmocom/core/socket.h> -#include <osmocom/abis/ipa.h> #include <osmocom/core/backtrace.h> -#include <osmocom/gsm/ipa.h> #include <osmocom/core/stats_tcp.h> #include <osmocom/core/fsm.h> +#include <osmocom/gsm/tlv.h> +#include <osmocom/gsm/protocol/ipaccess.h> +#include <osmocom/gsm/ipa.h> #include <osmocom/netif/stream.h> #include <osmocom/netif/ipa.h> +#include <osmocom/abis/e1_input.h> +#include <osmocom/abis/ipa.h> /* global parameters of IPA input driver */ struct ipa_pars g_e1inp_ipaccess_pars; diff --git a/src/ipa_proxy.c b/src/ipa_proxy.c index 441dd3f..04d3f9c 100644 --- a/src/ipa_proxy.c +++ b/src/ipa_proxy.c @@ -14,17 +14,16 @@ #include <osmocom/core/select.h> #include <osmocom/core/bitvec.h> -#include <osmocom/gsm/tlv.h> #include <osmocom/core/msgb.h> #include <osmocom/core/logging.h> #include <osmocom/core/talloc.h> -#include <osmocom/abis/e1_input.h> -#include <osmocom/abis/ipaccess.h> #include <osmocom/core/socket.h> - -#include <osmocom/abis/ipa.h> #include <osmocom/vty/vty.h> #include <osmocom/vty/command.h> +#include <osmocom/gsm/tlv.h> +#include <osmocom/gsm/protocol/ipaccess.h> +#include <osmocom/abis/e1_input.h> +#include <osmocom/abis/ipa.h> static void *tall_ipa_proxy_ctx; diff --git a/tests/e1inp_ipa_bts_test.c b/tests/e1inp_ipa_bts_test.c index 68dfefb..4488fe7 100644 --- a/tests/e1inp_ipa_bts_test.c +++ b/tests/e1inp_ipa_bts_test.c @@ -1,15 +1,15 @@ #include <stdio.h> #include <signal.h> -#include <osmocom/core/talloc.h> #include <string.h> #include <unistd.h> -#include <osmocom/abis/abis.h> -#include <osmocom/abis/e1_input.h> +#include <osmocom/core/talloc.h> #include <osmocom/core/logging.h> #include <osmocom/core/application.h> -#include <osmocom/abis/ipaccess.h> #include <osmocom/gsm/protocol/gsm_12_21.h> +#include <osmocom/gsm/protocol/ipaccess.h> #include <osmocom/gsm/ipa.h> +#include <osmocom/abis/abis.h> +#include <osmocom/abis/e1_input.h> static void *tall_test; static struct e1inp_sign_link *oml_sign_link, *rsl_sign_link; -- 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: newchange 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>

1 0

[M] Change in libosmo-netif[master]: stream_cli: Fix discard 1st msg received quick after connect
by laforge 29 Nov '24

29 Nov '24

Attention is currently required from: pespin. laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-netif/+/38987?usp=email ) Change subject: stream_cli: Fix discard 1st msg received quick after connect ...................................................................... Patch Set 1: Code-Review+1 -- 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: comment Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I98cd51d4bb87d3572245446648ced44a23a622ef Gerrit-Change-Number: 38987 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 29 Nov 2024 16:38:44 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[S] Change in libosmo-netif[master]: stream_cli: Explicitly ignore return code of internal functions
by laforge 29 Nov '24

29 Nov '24

Attention is currently required from: pespin. laforge has posted comments on this change by pespin. ( https://gerrit.osmocom.org/c/libosmo-netif/+/38986?usp=email ) Change subject: stream_cli: Explicitly ignore return code of internal functions ...................................................................... Patch Set 1: Code-Review+1 -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/38986?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I61b7001047e8eb1e1347f532d67f3f00dc8f67ec Gerrit-Change-Number: 38986 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: pespin <pespin(a)sysmocom.de> Gerrit-Comment-Date: Fri, 29 Nov 2024 16:37:30 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in libosmo-netif[master]: stream_cli: Fix discard 1st msg received quick after connect
by pespin 29 Nov '24

29 Nov '24

pespin has uploaded this change for review. ( 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, 51 insertions(+), 31 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/87/38987/1 diff --git a/src/stream_cli.c b/src/stream_cli.c index fe62607..131969d 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,15 @@ switch (cli->state) { case STREAM_CLI_STATE_CONNECTING: - msgb_free(msg); - stream_cli_handle_connecting(cli, res); + LOGSCLI(cli, LOGL_NOTICE, "stream_cli_iofd_read_cb(res=%d, msg=%p msg_len=%d)\n", res, msg, msg ? msgb_length(msg) : -1); + 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 +639,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: newchange Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I98cd51d4bb87d3572245446648ced44a23a622ef Gerrit-Change-Number: 38987 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in libosmo-netif[master]: stream_cli: Explicitly ignore return code of internal functions
by pespin 29 Nov '24

29 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmo-netif/+/38986?usp=email ) Change subject: stream_cli: Explicitly ignore return code of internal functions ...................................................................... stream_cli: Explicitly ignore return code of internal functions Make coverity happy. We don't really care about the return code of the stream_cli_reconnect() function in the code path, since the return value is only used internally/privately in the object and there's no further access/use of the object after calling it in osmo_stream_cli_reconnect(). Change-Id: I61b7001047e8eb1e1347f532d67f3f00dc8f67ec --- M src/stream_cli.c 1 file changed, 13 insertions(+), 13 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/86/38986/1 diff --git a/src/stream_cli.c b/src/stream_cli.c index 85ede60..fe62607 100644 --- a/src/stream_cli.c +++ b/src/stream_cli.c @@ -251,7 +251,7 @@ * we close any existing connection (if any) and schedule a re-connect timer */ void osmo_stream_cli_reconnect(struct osmo_stream_cli *cli) { - stream_cli_reconnect(cli); + (void)stream_cli_reconnect(cli); } /*! Check if Osmocom Stream Client is in connected state. @@ -383,7 +383,7 @@ return 0; } msgb_free(msg); - stream_cli_reconnect(cli); + (void)stream_cli_reconnect(cli); return 0; } @@ -419,13 +419,13 @@ if (ret < 0) { LOGSCLI(cli, LOGL_ERROR, "connect failed (%d)\n", res); - stream_cli_reconnect(cli); + (void)stream_cli_reconnect(cli); return; } ret = getsockopt(fd, SOL_SOCKET, SO_ERROR, &error, &len); if (ret >= 0 && error > 0) { LOGSCLI(cli, LOGL_ERROR, "connect so_error (%d)\n", error); - stream_cli_reconnect(cli); + (void)stream_cli_reconnect(cli); return; } @@ -458,7 +458,7 @@ if (cli->connect_cb) cli->connect_cb(cli); cli->in_cb_mask &= ~IN_CB_MASK_CONNECT_CB; - free_delayed_if_needed(cli); + (void)free_delayed_if_needed(cli); } static int osmo_stream_cli_fd_cb(struct osmo_fd *ofd, unsigned int what) @@ -467,7 +467,7 @@ switch (cli->state) { case STREAM_CLI_STATE_CONNECTING: - stream_cli_handle_connecting(cli, 0); + (void)stream_cli_handle_connecting(cli, 0); break; case STREAM_CLI_STATE_CONNECTED: if (what & OSMO_FD_READ) { @@ -558,7 +558,7 @@ cli->iofd_read_cb(cli, res, msg); cli->in_cb_mask &= ~IN_CB_MASK_READ_CB; OSMO_ASSERT(cli->in_cb_mask == 0); - free_delayed_if_needed(cli); + (void)free_delayed_if_needed(cli); break; default: osmo_panic("%s() called with unexpected state %d\n", __func__, cli->state); @@ -576,7 +576,7 @@ case STREAM_CLI_STATE_CONNECTED: if (msg && res <= 0) { LOGSCLI(cli, LOGL_ERROR, "received error %d in response to send\n", res); - stream_cli_reconnect(cli); + (void)stream_cli_reconnect(cli); } break; default: @@ -630,7 +630,7 @@ cli->iofd_read_cb(cli, res, msg); cli->in_cb_mask &= ~IN_CB_MASK_READ_CB; OSMO_ASSERT(cli->in_cb_mask == 0); - free_delayed_if_needed(cli); + (void)free_delayed_if_needed(cli); break; default: osmo_panic("%s() called with unexpected state %d\n", __func__, cli->state); @@ -1026,7 +1026,7 @@ if (ret < 0) { LOGSCLI(cli, LOGL_ERROR, "connect: socket creation error (%d)\n", ret); if (reconnect) - stream_cli_reconnect(cli); + (void)stream_cli_reconnect(cli); return ret; } osmo_fd_setup(&cli->ofd, ret, OSMO_FD_READ | OSMO_FD_WRITE, osmo_stream_cli_fd_cb, cli, 0); @@ -1199,7 +1199,7 @@ if (ret < 0) { LOGSCLI(cli, LOGL_ERROR, "connect: socket creation error (%d)\n", ret); - stream_cli_reconnect(cli); + (void)stream_cli_reconnect(cli); return ret; } @@ -1368,11 +1368,11 @@ LOGSCLI(cli, LOGL_ERROR, "lost connection with srv (%d)\n", errno); else LOGSCLI(cli, LOGL_ERROR, "recv failed (%d)\n", errno); - stream_cli_reconnect(cli); + (void)stream_cli_reconnect(cli); return ret; } else if (ret == 0) { LOGSCLI(cli, LOGL_ERROR, "connection closed with srv\n"); - osmo_stream_cli_reconnect(cli); + (void)stream_cli_reconnect(cli); return ret; } msgb_put(msg, ret); -- To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/38986?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmo-netif Gerrit-Branch: master Gerrit-Change-Id: I61b7001047e8eb1e1347f532d67f3f00dc8f67ec Gerrit-Change-Number: 38986 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in libosmocore[master]: osmo_io: Improve osmo_iofd_notify_connected() documentation
by pespin 29 Nov '24

29 Nov '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/libosmocore/+/38985?usp=email ) Change subject: osmo_io: Improve osmo_iofd_notify_connected() documentation ...................................................................... osmo_io: Improve osmo_iofd_notify_connected() documentation The mentioned scenario happens when using osmo_stream_cli inside libosmo-abis BTS code connecting to an osmo-bsc in the loopback interface. The client sends SYN and the server sends SYN+ACK and then quickly sends an IPA ID GET. Change-Id: Ica20a050b98d117995a5b625b23ab9faa61aabee --- M src/core/osmo_io.c 1 file changed, 12 insertions(+), 2 deletions(-) git pull ssh://gerrit.osmocom.org:29418/libosmocore refs/changes/85/38985/1 diff --git a/src/core/osmo_io.c b/src/core/osmo_io.c index 98d8ef3..9c93e25 100644 --- a/src/core/osmo_io.c +++ b/src/core/osmo_io.c @@ -994,12 +994,22 @@ /*! Request notification of the user if/when a client socket is connected. * Calling this function will request osmo_io to notify the user (via - * write call-back) once a non-blocking outbound connect() of the - * socket completes. + * write call-back with res=0 and msgb=NULL) once a non-blocking outbound + * connect() of the socket completes. * * This only works for connection oriented sockets in either * OSMO_IO_FD_MODE_READ_WRITE or OSMO_IO_FD_MODE_RECVMSG_SENDMSG mode. * + * The fact that the write call-back is called with msgb=NULL can be used to + * distinguish before this "connected" notification and a socket write failure. + * + * If the server transmits data quick enough after accepting the connection, + * it may happen that a read call-back is triggered towards the user before this + * special write-callback, since both events may come together from the kernel. + * Hence under those scenarios where server starts the communication, it is + * important not to assume or require that the write-callback(res=0, msgb=NULL) + * will be the first one triggered. + * * \param[in] iofd the file descriptor */ void osmo_iofd_notify_connected(struct osmo_io_fd *iofd) { -- To view, visit https://gerrit.osmocom.org/c/libosmocore/+/38985?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: libosmocore Gerrit-Branch: master Gerrit-Change-Id: Ica20a050b98d117995a5b625b23ab9faa61aabee Gerrit-Change-Number: 38985 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

2026

2025

2024

2023

2022

gerrit-log