- gerrit-log - lists.osmocom.org

[M] Change in osmo-hlr[master]: gsup_client: Add new APIs to avoid users accessing struct fields
by pespin 03 Dec '24

03 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/39009?usp=email ) Change subject: gsup_client: Add new APIs to avoid users accessing struct fields ...................................................................... gsup_client: Add new APIs to avoid users accessing struct fields This is a first step towards changing gsup_client implementation to use an osmo_stream_cli instead of libosmo-abis' ipa_client_conn. The libosmo-abis' ipa_client_conn will eventually be deprecated together with all IPA related code in libosmo-abis. In order to be able to make the implementation change, we first need to make sure no users are using the struct fields of gsup_client (this patch); 2nd step will be making the struct private by moving it to a private header; 3rd step will be changing the implementation to use osmo_stream. Related: OS#5896 Change-Id: I401af83232022f1c141eef1f428cbe206a8aaaa2 --- M TODO-RELEASE M include/osmocom/gsupclient/gsup_client.h M src/gsupclient/gsup_client.c M src/gsupclient/gsup_client_mux.c M src/remote_hlr.c 5 files changed, 59 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/09/39009/1 diff --git a/TODO-RELEASE b/TODO-RELEASE index 0ed7189..f14e8b5 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -7,3 +7,6 @@ # 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 +libosmo-gsup-client new API osmo_gsup_client_{get,set}_data(), + osmo_gsup_client_get_rem_addr(), osmo_gsup_client_get_rem_port(), + osmo_gsup_client_is_connected(), osmo_gsup_client_get_ipaccess_unit() \ No newline at end of file diff --git a/include/osmocom/gsupclient/gsup_client.h b/include/osmocom/gsupclient/gsup_client.h index ea66ca1..61cd45d 100644 --- a/include/osmocom/gsupclient/gsup_client.h +++ b/include/osmocom/gsupclient/gsup_client.h @@ -20,6 +20,9 @@ */ #pragma once +#include <stdint.h> +#include <stdbool.h> + #include <osmocom/core/timer.h> #include <osmocom/gsm/oap_client.h> #include <osmocom/gsm/ipa.h> @@ -40,6 +43,7 @@ typedef bool (*osmo_gsup_client_up_down_cb_t)(struct osmo_gsup_client *gsupc, bool up); +/* NOTE: THIS STRUCT IS CONSIDERED PRIVATE, AVOID ACCESSING ITS FIELDS! */ struct osmo_gsup_client { const char *unit_name; /* same as ipa_dev->unit_name, for backwards compat */ @@ -99,3 +103,12 @@ const struct osmo_gsup_message *gsup_msg); struct msgb *osmo_gsup_client_msgb_alloc(void); +void *osmo_gsup_client_get_data(const struct osmo_gsup_client *gsupc); +void osmo_gsup_client_set_data(struct osmo_gsup_client *gsupc, void *data); + +const char *osmo_gsup_client_get_rem_addr(const struct osmo_gsup_client *gsupc); +uint16_t osmo_gsup_client_get_rem_port(const struct osmo_gsup_client *gsupc); + +bool osmo_gsup_client_is_connected(const struct osmo_gsup_client *gsupc); +const struct ipaccess_unit *osmo_gsup_client_get_ipaccess_unit(const struct osmo_gsup_client *gsupc); + diff --git a/src/gsupclient/gsup_client.c b/src/gsupclient/gsup_client.c index 36407d5..eb36e3f 100644 --- a/src/gsupclient/gsup_client.c +++ b/src/gsupclient/gsup_client.c @@ -448,3 +448,36 @@ { return msgb_alloc_headroom(4000, 64, __func__); } + +void *osmo_gsup_client_get_data(const struct osmo_gsup_client *gsupc) +{ + return gsupc->data; +} + +void osmo_gsup_client_set_data(struct osmo_gsup_client *gsupc, void *data) +{ + gsupc->data = data; +} + +const char *osmo_gsup_client_get_rem_addr(const struct osmo_gsup_client *gsupc) +{ + if (!gsupc->link) + return NULL; + return gsupc->link->addr; +} +uint16_t osmo_gsup_client_get_rem_port(const struct osmo_gsup_client *gsupc) +{ + if (!gsupc->link) + return 0; + return gsupc->link->port; +} + +bool osmo_gsup_client_is_connected(const struct osmo_gsup_client *gsupc) +{ + return gsupc->is_connected; +} + +const struct ipaccess_unit *osmo_gsup_client_get_ipaccess_unit(const struct osmo_gsup_client *gsupc) +{ + return gsupc->ipa_dev; +} diff --git a/src/gsupclient/gsup_client_mux.c b/src/gsupclient/gsup_client_mux.c index 7699ca1..45ebd75 100644 --- a/src/gsupclient/gsup_client_mux.c +++ b/src/gsupclient/gsup_client_mux.c @@ -58,7 +58,7 @@ /* Non-static for unit tests */ int gsup_client_mux_rx(struct osmo_gsup_client *gsup_client, struct msgb *msg) { - struct gsup_client_mux *gcm = gsup_client->data; + struct gsup_client_mux *gcm = osmo_gsup_client_get_data(gsup_client); struct osmo_gsup_message gsup; enum osmo_gsup_message_class message_class; int rc; @@ -115,7 +115,7 @@ &gsup_client_mux_rx, NULL); if (!gcm->gsup_client) return -ENOMEM; - gcm->gsup_client->data = gcm; + osmo_gsup_client_set_data(gcm->gsup_client, gcm); return 0; } @@ -144,14 +144,16 @@ struct osmo_gsup_message *gsup_msg) { const char *local_msc_name; + const struct ipaccess_unit *ipa_dev; if (!gcm) return; if (!gcm->gsup_client) return; - if (!gcm->gsup_client->ipa_dev) + ipa_dev = osmo_gsup_client_get_ipaccess_unit(gcm->gsup_client); + if (!ipa_dev) return; - local_msc_name = gcm->gsup_client->ipa_dev->serno; + local_msc_name = ipa_dev->serno; if (!local_msc_name) return; gsup_msg->source_name = (const uint8_t *) local_msc_name; diff --git a/src/remote_hlr.c b/src/remote_hlr.c index 00bfbb1..93157a3 100644 --- a/src/remote_hlr.c +++ b/src/remote_hlr.c @@ -66,7 +66,7 @@ * The local MSC shall be indicated by gsup.destination_name. */ static int remote_hlr_rx(struct osmo_gsup_client *gsupc, struct msgb *msg) { - struct remote_hlr *rh = gsupc->data; + struct remote_hlr *rh = osmo_gsup_client_get_data(gsupc); struct proxy_subscr proxy_subscr; struct osmo_gsup_message gsup; int rc; @@ -108,7 +108,7 @@ static bool remote_hlr_up_down(struct osmo_gsup_client *gsupc, bool up) { - struct remote_hlr *remote_hlr = gsupc->data; + struct remote_hlr *remote_hlr = osmo_gsup_client_get_data(gsupc); struct remote_hlr_pending_up *p, *n; if (!up) { LOG_REMOTE_HLR(remote_hlr, LOGL_NOTICE, "link to remote HLR is down, removing GSUP client\n"); @@ -127,7 +127,7 @@ bool remote_hlr_is_up(struct remote_hlr *remote_hlr) { - return remote_hlr && remote_hlr->gsupc && remote_hlr->gsupc->is_connected; + return remote_hlr && remote_hlr->gsupc && osmo_gsup_client_is_connected(remote_hlr->gsupc); } struct remote_hlr *remote_hlr_get_or_connect(const struct osmo_sockaddr_str *addr, bool connect, @@ -180,7 +180,7 @@ return NULL; } - rh->gsupc->data = rh; + osmo_gsup_client_set_data(rh->gsupc, rh); llist_add(&rh->entry, &remote_hlrs); add_result_cb: -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/39009?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: I401af83232022f1c141eef1f428cbe206a8aaaa2 Gerrit-Change-Number: 39009 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-msc[master]: vty: Avoid accessing gsupclient object fields directly
by pespin 03 Dec '24

03 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-msc/+/39008?usp=email ) Change subject: vty: Avoid accessing gsupclient object fields directly ...................................................................... vty: Avoid accessing gsupclient object fields directly Use new APIs recently added. Change-Id: I7ead95bb86d1ab510090c29390255007923f895f Depends: osmo-hlr.git Change-Id I401af83232022f1c141eef1f428cbe206a8aaaa2 --- M src/libmsc/e_link.c M src/libmsc/gsup_client_mux.c 2 files changed, 13 insertions(+), 8 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-msc refs/changes/08/39008/1 diff --git a/src/libmsc/e_link.c b/src/libmsc/e_link.c index 5107c22..9a1cde8 100644 --- a/src/libmsc/e_link.c +++ b/src/libmsc/e_link.c @@ -114,10 +114,13 @@ struct vlr_subscr *vsub; const char *local_msc_name = NULL; - if (e->gcm && e->gcm->gsup_client && e->gcm->gsup_client->ipa_dev) { - local_msc_name = e->gcm->gsup_client->ipa_dev->serno; - if (!local_msc_name) - local_msc_name = e->gcm->gsup_client->ipa_dev->unit_name; + if (e->gcm && e->gcm->gsup_client) { + const struct ipaccess_unit *ipa_dev = osmo_gsup_client_get_ipaccess_unit(e->gcm->gsup_client); + if (ipa_dev) { + local_msc_name = ipa_dev->serno; + if (!local_msc_name) + local_msc_name = ipa_dev->unit_name; + } } if (!local_msc_name) { diff --git a/src/libmsc/gsup_client_mux.c b/src/libmsc/gsup_client_mux.c index e27b664..2c4c7fd 100644 --- a/src/libmsc/gsup_client_mux.c +++ b/src/libmsc/gsup_client_mux.c @@ -55,7 +55,7 @@ /* Non-static for unit tests */ int gsup_client_mux_rx(struct osmo_gsup_client *gsup_client, struct msgb *msg) { - struct gsup_client_mux *gcm = gsup_client->data; + struct gsup_client_mux *gcm = osmo_gsup_client_get_data(gsup_client); struct osmo_gsup_message gsup; enum osmo_gsup_message_class message_class; int rc; @@ -112,7 +112,7 @@ &gsup_client_mux_rx, NULL); if (!gcm->gsup_client) return -ENOMEM; - gcm->gsup_client->data = gcm; + osmo_gsup_client_set_data(gcm->gsup_client, gcm); return 0; } @@ -141,14 +141,16 @@ struct osmo_gsup_message *gsup_msg) { const char *local_msc_name; + const struct ipaccess_unit *ipa_dev; if (!gcm) return; if (!gcm->gsup_client) return; - if (!gcm->gsup_client->ipa_dev) + ipa_dev = osmo_gsup_client_get_ipaccess_unit(gcm->gsup_client); + if (!ipa_dev) return; - local_msc_name = gcm->gsup_client->ipa_dev->serno; + local_msc_name = ipa_dev->serno; if (!local_msc_name) return; gsup_msg->source_name = (const uint8_t *) local_msc_name; -- To view, visit https://gerrit.osmocom.org/c/osmo-msc/+/39008?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-msc Gerrit-Branch: master Gerrit-Change-Id: I7ead95bb86d1ab510090c29390255007923f895f Gerrit-Change-Number: 39008 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-sgsn[master]: vty: Avoid accessing gsupclient object fields directly
by pespin 03 Dec '24

03 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-sgsn/+/39007?usp=email ) Change subject: vty: Avoid accessing gsupclient object fields directly ...................................................................... vty: Avoid accessing gsupclient object fields directly Use new APIs recently added. Depends: osmo-hlr.git Change-Id I401af83232022f1c141eef1f428cbe206a8aaaa2 Change-Id: Ie443ef0bd983635a5b29e434d79b93ef3f7ce370 --- M TODO-RELEASE M src/sgsn/sgsn_vty.c 2 files changed, 5 insertions(+), 5 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-sgsn refs/changes/07/39007/1 diff --git a/TODO-RELEASE b/TODO-RELEASE index 261da7b..662cdb5 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -10,3 +10,4 @@ libgtp >1.12.0 new field dir_tun_flags in struct pdp_t libgtp >1.12.0 gtp_set_cb_update_context_ind(), gtp_update_context_resp() libosmocore >1.10.0 enum gsm48_gprs_ie_mm: GSM48_IE_GMM_UE_NET_CAP, GSM48_IE_GMM_VD_PREF_UE_USAGE +libosmo-gsup-client >1.8.0 osmo_gsup_client_is_connected(), osmo_gsup_client_get_rem_addr(), osmo_gsup_client_get_rem_port() diff --git a/src/sgsn/sgsn_vty.c b/src/sgsn/sgsn_vty.c index a7e8045..2cb8819 100644 --- a/src/sgsn/sgsn_vty.c +++ b/src/sgsn/sgsn_vty.c @@ -670,11 +670,10 @@ SHOW_STR "Display information about the SGSN") { if (sgsn->gsup_client) { - struct ipa_client_conn *link = sgsn->gsup_client->link; - vty_out(vty, - " Remote authorization: %sconnected to %s:%d via GSUP%s", - sgsn->gsup_client->is_connected ? "" : "not ", - link->addr, link->port, + vty_out(vty, " Remote authorization: %sconnected to %s:%d via GSUP%s", + osmo_gsup_client_is_connected(sgsn->gsup_client) ? "" : "not ", + osmo_gsup_client_get_rem_addr(sgsn->gsup_client), + osmo_gsup_client_get_rem_port(sgsn->gsup_client), VTY_NEWLINE); } if (sgsn->gsn) -- To view, visit https://gerrit.osmocom.org/c/osmo-sgsn/+/39007?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newchange Gerrit-Project: osmo-sgsn Gerrit-Branch: master Gerrit-Change-Id: Ie443ef0bd983635a5b29e434d79b93ef3f7ce370 Gerrit-Change-Number: 39007 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in osmo-hlr[master]: gsup_client: Avoid double memset 0
by pespin 03 Dec '24

03 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-hlr/+/39006?usp=email ) Change subject: gsup_client: Avoid double memset 0 ...................................................................... gsup_client: Avoid double memset 0 Memory is already zeroed during talloc_zero, no need to reset it to zero. Change-Id: I66515fc893ffc64a36abedc01a75b57aed7e932d --- M src/gsupclient/gsup_client.c 1 file changed, 5 insertions(+), 7 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-hlr refs/changes/06/39006/1 diff --git a/src/gsupclient/gsup_client.c b/src/gsupclient/gsup_client.c index e7d03f2..36407d5 100644 --- a/src/gsupclient/gsup_client.c +++ b/src/gsupclient/gsup_client.c @@ -309,13 +309,11 @@ gsupc = talloc_zero(talloc_ctx, struct osmo_gsup_client); OSMO_ASSERT(gsupc); - *gsupc = (struct osmo_gsup_client){ - .unit_name = (const char *)config->ipa_dev->unit_name, /* API backwards compat */ - .ipa_dev = config->ipa_dev, - .read_cb = config->read_cb, - .up_down_cb = config->up_down_cb, - .data = config->data, - }; + gsupc->unit_name = (const char *)config->ipa_dev->unit_name; /* API backwards compat */ + gsupc->ipa_dev = config->ipa_dev; + gsupc->read_cb = config->read_cb; + gsupc->up_down_cb = config->up_down_cb; + gsupc->data = config->data; /* a NULL oapc_config will mark oap_state disabled. */ rc = osmo_oap_client_init(config->oapc_config, &gsupc->oap_state); -- To view, visit https://gerrit.osmocom.org/c/osmo-hlr/+/39006?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: I66515fc893ffc64a36abedc01a75b57aed7e932d Gerrit-Change-Number: 39006 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in pysim[master]: Modem related fixes.
by laforge 03 Dec '24

03 Dec '24

Attention is currently required from: JPM, dexter. laforge has posted comments on this change by JPM. ( https://gerrit.osmocom.org/c/pysim/+/38993?usp=email ) Change subject: Modem related fixes. ...................................................................... Patch Set 1: -Code-Review -- To view, visit https://gerrit.osmocom.org/c/pysim/+/38993?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I3e45603a13496fff41b9f859de486143252fb28d Gerrit-Change-Number: 38993 Gerrit-PatchSet: 1 Gerrit-Owner: JPM <jean-pierre.marcotte.1(a)ens.etsmtl.ca> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: JPM <jean-pierre.marcotte.1(a)ens.etsmtl.ca> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 03 Dec 2024 12:45:08 +0000 Gerrit-HasComments: No Gerrit-Has-Labels: Yes

1 0

[M] Change in osmo-bsc[master]: ipaccess-config: Remove unused function ipaccess_bts_handle_ccm()
by pespin 03 Dec '24

03 Dec '24

pespin has uploaded a new patch set (#3). ( https://gerrit.osmocom.org/c/osmo-bsc/+/39005?usp=email ) Change subject: ipaccess-config: Remove unused function ipaccess_bts_handle_ccm() ...................................................................... ipaccess-config: Remove unused function ipaccess_bts_handle_ccm() This function actually slippted in unintentionally when copying the ipaccess driver from libosmo-abis.git as a base for the ipaccess-config driver. That function is actually not used either by libosmo-abis driver, but was left there as a backward compatible API. Fixes: 76b863eb47ecaaf22fb4ca6ecd19f94a05b2ed5c Change-Id: I126014eed2196e9fe03c65dff7bb384ac81f39f8 --- M src/ipaccess/ipaccess-config-e1-driver.c 1 file changed, 0 insertions(+), 68 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/05/39005/3 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/39005?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I126014eed2196e9fe03c65dff7bb384ac81f39f8 Gerrit-Change-Number: 39005 Gerrit-PatchSet: 3 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-CC: Jenkins Builder

1 0

[M] Change in osmo-bsc[master]: ipaccess-config: Remove unused function ipaccess_bts_handle_ccm()
by pespin 03 Dec '24

03 Dec '24

pespin has uploaded a new patch set (#2). ( https://gerrit.osmocom.org/c/osmo-bsc/+/39005?usp=email ) Change subject: ipaccess-config: Remove unused function ipaccess_bts_handle_ccm() ...................................................................... ipaccess-config: Remove unused function ipaccess_bts_handle_ccm() This function actually slippted in unintentionally when copying the ipaccess driver from libosmo-abis.git as a base for the ipaccess-config driver. That function is actually not used either by libosmo-abis driver, but was left there as a backward compatible API. Fixes: 76b863eb47ecaaf22fb4ca6ecd19f94a05b2ed5c Change-Id: I126014eed2196e9fe03c65dff7bb384ac81f39f8 --- M src/ipaccess/ipaccess-config-e1-driver.c 1 file changed, 0 insertions(+), 65 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/05/39005/2 -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/39005?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: newpatchset Gerrit-Project: osmo-bsc Gerrit-Branch: master Gerrit-Change-Id: I126014eed2196e9fe03c65dff7bb384ac81f39f8 Gerrit-Change-Number: 39005 Gerrit-PatchSet: 2 Gerrit-Owner: pespin <pespin(a)sysmocom.de> Gerrit-CC: Jenkins Builder

1 0

[M] Change in osmo-bsc[master]: ipaccess-config: Remove unused function ipaccess_bts_handle_ccm()
by pespin 03 Dec '24

03 Dec '24

pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/39005?usp=email ) Change subject: ipaccess-config: Remove unused function ipaccess_bts_handle_ccm() ...................................................................... ipaccess-config: Remove unused function ipaccess_bts_handle_ccm() This function actually slippted in unintentionally when copying the ipaccess driver from libosmo-abis.git as a base for the ipaccess-config driver. That function is actually not used either by libosmo-abis driver, but was left there as a backward compatible API. Change-Id: I126014eed2196e9fe03c65dff7bb384ac81f39f8 --- M src/ipaccess/ipaccess-config-e1-driver.c 1 file changed, 0 insertions(+), 65 deletions(-) git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/05/39005/1 diff --git a/src/ipaccess/ipaccess-config-e1-driver.c b/src/ipaccess/ipaccess-config-e1-driver.c index 1303d43..7147242 100644 --- a/src/ipaccess/ipaccess-config-e1-driver.c +++ b/src/ipaccess/ipaccess-config-e1-driver.c @@ -207,71 +207,6 @@ return rc; } -int ipaccess_bts_handle_ccm(struct ipa_client_conn *link, - struct ipaccess_unit *dev, struct msgb *msg) -{ - struct ipaccess_head *hh = (struct ipaccess_head *) msg->data; - - /* special handling for IPA CCM. */ - if (hh->proto != IPAC_PROTO_IPACCESS) - return 0; - - int ret = 0; - uint8_t *data = msgb_l2(msg); - int len = msgb_l2len(msg); - OSMO_ASSERT(len > 0); - uint8_t msg_type = *data; - /* line might not exist if != bsc||bts */ - - /* ping, pong and acknowledgment cases. */ - ret = ipa_ccm_rcvmsg_bts_base(msg, link->ofd); - if (ret < 0) - goto err; - - /* this is a request for identification from the BSC. */ - if (msg_type == IPAC_MSGT_ID_GET) { - struct msgb *rmsg; - /* The ipaccess_unit dev holds generic identity for the whole - * line, hence no trx_id. Patch ipaccess_unit during call to - * ipa_ccm_make_id_resp_from_req() to identify this TRX: */ - int store_trx_nr = dev->trx_id; - if (link->ofd->priv_nr >= E1INP_SIGN_RSL) - dev->trx_id = link->ofd->priv_nr - E1INP_SIGN_RSL; - else - dev->trx_id = 0; - LOGP(DLINP, LOGL_NOTICE, "received ID_GET for unit ID %u/%u/%u\n", - dev->site_id, dev->bts_id, dev->trx_id); - rmsg = ipa_ccm_make_id_resp_from_req(dev, data + 1, len - 1); - dev->trx_id = store_trx_nr; - if (!rmsg) { - LOGP(DLINP, LOGL_ERROR, "Failed parsing ID_GET message.\n"); - goto err; - } - - ret = ipa_send(link->ofd->fd, rmsg->data, rmsg->len); - if (ret != rmsg->len) { - LOGP(DLINP, LOGL_ERROR, "cannot send ID_RESP message. Reason: %s\n", - strerror(errno)); - msgb_free(rmsg); - goto err; - } - msgb_free(rmsg); - - /* send ID_ACK. */ - ret = ipa_ccm_send_id_ack(link->ofd->fd); - if (ret <= 0) { - LOGP(DLINP, LOGL_ERROR, "cannot send ID_ACK message. Reason: %s\n", - strerror(errno)); - goto err; - } - } - return 1; - -err: - ipa_client_conn_close(link); - return -1; -} - static struct msgb *ipa_bts_id_ack(void) { struct msgb *nmsg2; -- To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/39005?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: I126014eed2196e9fe03c65dff7bb384ac81f39f8 Gerrit-Change-Number: 39005 Gerrit-PatchSet: 1 Gerrit-Owner: pespin <pespin(a)sysmocom.de>

1 0

[S] Change in pysim[master]: Modem related fixes.
by fixeria 03 Dec '24

03 Dec '24

Attention is currently required from: JPM, dexter. fixeria has posted comments on this change by JPM. ( https://gerrit.osmocom.org/c/pysim/+/38993?usp=email ) Change subject: Modem related fixes. ...................................................................... Patch Set 1: (1 comment) File pySim/transport/modem_atcmd.py: https://gerrit.osmocom.org/c/pysim/+/38993/comment/bad1c9c5_66a5ba09?usp=em… : PS1, Line 142: f' Oh, BTW. I am all for using f-strings, but pySim generally aims to support old Python versions. We should probably bump the minimum Python version requirement, but for now I would avoid using f-strings. -- To view, visit https://gerrit.osmocom.org/c/pysim/+/38993?usp=email To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings?usp=email Gerrit-MessageType: comment Gerrit-Project: pysim Gerrit-Branch: master Gerrit-Change-Id: I3e45603a13496fff41b9f859de486143252fb28d Gerrit-Change-Number: 38993 Gerrit-PatchSet: 1 Gerrit-Owner: JPM <jean-pierre.marcotte.1(a)ens.etsmtl.ca> Gerrit-Reviewer: Jenkins Builder Gerrit-Reviewer: dexter <pmaier(a)sysmocom.de> Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de> Gerrit-Reviewer: laforge <laforge(a)osmocom.org> Gerrit-Attention: JPM <jean-pierre.marcotte.1(a)ens.etsmtl.ca> Gerrit-Attention: dexter <pmaier(a)sysmocom.de> Gerrit-Comment-Date: Tue, 03 Dec 2024 12:26:35 +0000 Gerrit-HasComments: Yes Gerrit-Has-Labels: No

1 0

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

03 Dec '24

pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-abis/+/38983?usp=email ) Change subject: ipaccess: Convert BSC OML & RSL link to use stream_srv ...................................................................... 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. Drop ipaccess_fd_cb() since it has no real implementation anymore. That was only used by ipaccess-config in osmo-bsc.git, which got its own ipaccess specific e1_line driver recently. The use of that API was a quick dirty hack at the time which should have never happened. Related: SYS#7063 Related: OS#5755 Related: OS#5756 Change-Id: Idf241c8f2fdb86d090d4132a9b316b7236402232 --- M TODO-RELEASE M include/osmocom/abis/ipaccess.h M src/input/ipaccess.c 3 files changed, 274 insertions(+), 337 deletions(-) Approvals: pespin: Looks good to me, approved Jenkins Builder: Verified fixeria: Looks good to me, but someone else must approve osmith: Looks good to me, but someone else must approve diff --git a/TODO-RELEASE b/TODO-RELEASE index 987d77e..bcd3c56 100644 --- a/TODO-RELEASE +++ b/TODO-RELEASE @@ -11,3 +11,4 @@ libosmo-netif >1.5.0 New dependency libosmo-netif >1.5.0 use osmo_stream_cli_set_priority(), osmo_stream_cli_set_ip_dscp() libosmocore >1.10.0 bigger iofd tx_queue.max_length default value +libosmo-abis API break Remove ipaccess_fd_cb() diff --git a/include/osmocom/abis/ipaccess.h b/include/osmocom/abis/ipaccess.h index b1fe2c9..7ed2219 100644 --- a/include/osmocom/abis/ipaccess.h +++ b/include/osmocom/abis/ipaccess.h @@ -4,7 +4,7 @@ #include <stdint.h> #include <osmocom/gsm/protocol/ipaccess.h> -/* quick solution to get openBSC's ipaccess tools working. */ -extern int ipaccess_fd_cb(struct osmo_fd *bfd, unsigned int what); +/* Kept for backward compatibility */ +#pragma message("include osmocom/abis/ipaccess.h is deprecated, please include osmocom/gsm/protocol/ipaccess.h directly instead") #endif /* _OSMO_ABIS_IPACCESS_H */ diff --git a/src/input/ipaccess.c b/src/input/ipaccess.c index 505ec3f..d1fae94 100644 --- a/src/input/ipaccess.c +++ b/src/input/ipaccess.c @@ -1,6 +1,7 @@ /* OpenBSC Abis input driver for ip.access */ -/* (C) 2009-2021 by Harald Welte <laforge(a)gnumonks.org> +/* (C) 2024 by sysmocom - s.f.m.c. GmbH <info(a)sysmocom.de> + * (C) 2009-2021 by Harald Welte <laforge(a)gnumonks.org> * (C) 2010 by Holger Hans Peter Freyther * (C) 2010 by On-Waves * @@ -60,8 +61,6 @@ static void *tall_ipa_ctx; -#define TS1_ALLOC_SIZE 900 - #define DEFAULT_TCP_KEEPALIVE_IDLE_TIMEOUT 30 #define DEFAULT_TCP_KEEPALIVE_INTERVAL 3 #define DEFAULT_TCP_KEEPALIVE_RETRY_COUNT 10 @@ -90,73 +89,20 @@ } } -static int ipaccess_drop(struct osmo_fd *bfd, struct e1inp_line *line) -{ - int ret = 1; - struct e1inp_ts *e1i_ts = ipaccess_line_ts(bfd, line); - e1inp_line_get2(line, __func__); - - ipaccess_keepalive_fsm_cleanup(e1i_ts); - - /* Error case: we did not see any ID_RESP yet for this socket. */ - if (bfd->fd != -1) { - LOGPITS(e1i_ts, DLINP, LOGL_NOTICE, "Forcing socket shutdown\n"); - osmo_fd_unregister(bfd); - close(bfd->fd); - bfd->fd = -1; - switch (line->ops->cfg.ipa.role) { - case E1INP_LINE_R_BSC: - /* This is BSC code, ipaccess_drop() is only called for - accepted() sockets, hence the bfd holds a reference to - e1inp_line in ->data that needs to be released */ - OSMO_ASSERT(bfd->data == line); - bfd->data = NULL; - e1inp_line_put2(line, "ipa_bfd"); - break; - case E1INP_LINE_R_BTS: - /* BTS code: bfd->data contains pointer to struct - * ipa_client_conn. Leave it alive so it reconnects. - */ - break; - default: - break; - } - ret = -ENOENT; - } else { - LOGPITS(e1i_ts, DLINP, LOGL_ERROR, - "Forcing socket shutdown with no signal link set\n"); - } - - msgb_free(e1i_ts->pending_msg); - e1i_ts->pending_msg = NULL; - - /* e1inp_sign_link_destroy releases the socket descriptors for us. */ - if (line->ops->sign_link_down) - line->ops->sign_link_down(line); - - e1inp_line_put2(line, __func__); - return ret; -} - static void ipa_bsc_keepalive_write_server_cb(struct osmo_fsm_inst *fi, void *conn, struct msgb *msg) { - struct osmo_fd *bfd = (struct osmo_fd *)conn; - write(bfd->fd, msg->data, msg->len); - msgb_free(msg); + struct osmo_stream_srv *srv = (struct osmo_stream_srv *)conn; + osmo_stream_srv_send(srv, msg); } static int ipa_bsc_keepalive_timeout_cb(struct osmo_fsm_inst *fi, void *data) { - struct osmo_fd *bfd = (struct osmo_fd *)data; - - if (bfd->fd == -1) - return 1; - - ipaccess_drop(bfd, (struct e1inp_line *)bfd->data); + struct osmo_stream_srv *srv = (struct osmo_stream_srv *)data; + osmo_stream_srv_destroy(srv); return 1; } -static void ipaccess_bsc_keepalive_fsm_alloc(struct e1inp_ts *e1i_ts, struct osmo_fd *bfd, const char *id) +static void ipaccess_bsc_keepalive_fsm_alloc(struct e1inp_ts *e1i_ts, struct osmo_stream_srv *conn, const char *id) { struct e1inp_line *line = e1i_ts->line; struct osmo_fsm_inst *ka_fsm; @@ -165,7 +111,7 @@ if (!line->ipa_kap) return; - ka_fsm = ipa_generic_conn_alloc_keepalive_fsm(tall_ipa_ctx, bfd, line->ipa_kap, id); + ka_fsm = ipa_generic_conn_alloc_keepalive_fsm(conn, conn, line->ipa_kap, id); e1i_ts->driver.ipaccess.ka_fsm = ka_fsm; if (!ka_fsm) { LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "Failed to allocate IPA keepalive FSM\n"); @@ -250,22 +196,37 @@ return cli; } -/* Returns -1 on error, and 0 or 1 on success. If -1 or 1 is returned, line has - * been released and should not be used anymore by the caller. */ -static int ipaccess_rcvmsg(struct e1inp_line *line, struct msgb *msg, - struct osmo_fd *bfd) +static inline struct osmo_stream_srv *ipaccess_bts_e1i_ts_stream_srv(const struct e1inp_ts *e1i_ts) { + OSMO_ASSERT(e1i_ts); + struct osmo_stream_srv *conn = e1i_ts->driver.ipaccess.fd.data; + /* conn may be NULL here, if we are called by user during sign_link_up() + * and hence before we finished to internally move the ipaccess conn under + * the proper final line/ts object. + */ + return conn; +} + +static int ipaccess_bsc_write_cb(struct e1inp_ts *e1i_ts); + +/* Returns -1 on error, and 0 or 1 on success. If -1 or 1 is returned, line has + * been released and should not be used anymore by the caller. + * msg is always freed upon return. + */ +static int ipaccess_bsc_rcvmsg(struct osmo_stream_srv *conn, struct msgb *msg) +{ + struct e1inp_ts *e1i_ts = osmo_stream_srv_get_data(conn); + struct e1inp_line *line = e1i_ts->line; + struct osmo_fd *bfd = &e1i_ts->driver.ipaccess.fd; struct tlv_parsed tlvp; uint8_t msg_type = *(msg->l2h); struct ipaccess_unit unit_data = {}; struct e1inp_sign_link *sign_link; char *unitid; int len, ret; - struct e1inp_ts *e1i_ts; struct osmo_fsm_inst *ka_fsm; /* peek the pong for our keepalive fsm */ - e1i_ts = ipaccess_line_ts(bfd, line); ka_fsm = e1i_ts->driver.ipaccess.ka_fsm; if (ka_fsm && msg_type == IPAC_MSGT_PONG) ipa_keepalive_fsm_pong_received(ka_fsm); @@ -278,7 +239,7 @@ goto err; case 1: /* this is an IPA control message, skip further processing */ - return 0; + goto ret_skip; case 0: /* this is not an IPA control message, continue */ break; @@ -329,17 +290,16 @@ goto err; } - ipaccess_bsc_keepalive_fsm_alloc(e1i_ts, bfd, "oml_bsc_to_bts"); + ipaccess_bsc_keepalive_fsm_alloc(e1i_ts, conn, "oml_bsc_to_bts"); } else if (bfd->priv_nr == E1INP_SIGN_RSL) { - struct e1inp_ts *ts; + struct e1inp_ts *new_ts; struct osmo_fd *newbfd; struct e1inp_line *new_line; char tcp_stat_name[64]; sign_link = - line->ops->sign_link_up(&unit_data, line, - E1INP_SIGN_RSL); + line->ops->sign_link_up(&unit_data, line, E1INP_SIGN_RSL); if (sign_link == NULL) { LOGPIL(line, DLINP, LOGL_ERROR, "Unable to set signal link, closing socket.\n"); goto err; @@ -352,37 +312,36 @@ if (new_line == line) { LOGPIL(line, DLINP, LOGL_ERROR, "Fix your BSC, you should use the " "E1 line used by the OML link for your RSL link.\n"); - return 0; + goto ret_skip; } + + /* Attach srv_conn to new line+ts: */ e1inp_line_get2(new_line, "ipa_bfd"); - ts = e1inp_line_ipa_rsl_ts(new_line, unit_data.trx_id); - newbfd = &ts->driver.ipaccess.fd; + new_ts = e1inp_line_ipa_rsl_ts(new_line, unit_data.trx_id); + newbfd = &new_ts->driver.ipaccess.fd; OSMO_ASSERT(newbfd != bfd); + osmo_fd_setup(newbfd, bfd->fd, 0, NULL, conn, E1INP_SIGN_RSL + unit_data.trx_id); + osmo_stream_srv_set_data(conn, new_ts); - /* preserve 'newbfd->when' flags potentially set by sign_link_up() */ - osmo_fd_setup(newbfd, bfd->fd, newbfd->when | bfd->when, bfd->cb, - new_line, E1INP_SIGN_RSL + unit_data.trx_id); - - - /* now we can release the dummy RSL line (old temporary bfd). */ - osmo_fd_unregister(bfd); + /* Now we can release the old temporary dummy RSL line+ts: */ + osmo_stats_tcp_osmo_fd_unregister(bfd); bfd->fd = -1; - /* bfd->data holds a reference to line, drop it */ - OSMO_ASSERT(bfd->data == line); + /* conn holds a reference to old line, drop it */ + OSMO_ASSERT(bfd->data == conn); bfd->data = NULL; e1inp_line_put2(line, "ipa_bfd"); - ret = osmo_fd_register(newbfd); - if (ret < 0) { - LOGPITS(ts, DLINP, LOGL_ERROR, "could not register FD\n"); - goto err; - } snprintf(tcp_stat_name, sizeof(tcp_stat_name), "site.%u.bts.%u.ipa-rsl.%u", unit_data.site_id, unit_data.bts_id, unit_data.trx_id); osmo_stats_tcp_osmo_fd_register(newbfd, tcp_stat_name); - e1i_ts = ipaccess_line_ts(newbfd, new_line); - ipaccess_bsc_keepalive_fsm_alloc(e1i_ts, newbfd, "rsl_bsc_to_bts"); + ipaccess_bsc_keepalive_fsm_alloc(new_ts, conn, "rsl_bsc_to_bts"); + + /* Now that we attached the srv_conn to the e1ts, transmit packets which may been enqueued + * by user during sign_link_up() and which were delayed due to no srv_conn found: */ + ipaccess_bsc_write_cb(new_ts); + /* Conn is not freed here, so we need to free msgb. */ + msgb_free(msg); return 1; } break; @@ -390,86 +349,14 @@ LOGPIL(line, DLINP, LOGL_ERROR, "Unknown IPA message type\n"); goto err; } + +ret_skip: + msgb_free(msg); return 0; err: - if (bfd->fd != -1) { - osmo_fd_unregister(bfd); - close(bfd->fd); - bfd->fd = -1; - /* This is a BSC accepted socket, bfd->data holds a reference to line, drop it */ - OSMO_ASSERT(bfd->data == line); - bfd->data = NULL; - e1inp_line_put2(line, "ipa_bfd"); - } - return -1; -} - -/* Returns -EBADF if bfd cannot be used by the caller anymore after return. */ -static int handle_ts1_read(struct osmo_fd *bfd) -{ - struct e1inp_line *line = bfd->data; - unsigned int ts_nr = bfd->priv_nr; - struct e1inp_ts *e1i_ts; - struct e1inp_sign_link *link; - struct ipaccess_head *hh; - struct msgb *msg = NULL; - int ret, rc; - - e1i_ts = ipaccess_line_ts(bfd, line); - ret = ipa_msg_recv_buffered(bfd->fd, &msg, &e1i_ts->pending_msg); - if (ret < 0) { - if (ret == -EAGAIN) - return 0; - LOGPITS(e1i_ts, DLINP, LOGL_NOTICE, "Sign link problems, closing socket. Reason: %s\n", - strerror(-ret)); - goto err; - } else if (ret == 0) { - LOGPITS(e1i_ts, DLINP, LOGL_NOTICE, "Sign link vanished, dead socket\n"); - goto err; - } - LOGPITS(e1i_ts, DLMI, LOGL_DEBUG, "RX %u: %s\n", ts_nr, osmo_hexdump(msgb_l2(msg), msgb_l2len(msg))); - - hh = (struct ipaccess_head *) msg->data; - if (hh->proto == IPAC_PROTO_IPACCESS) { - ret = ipaccess_rcvmsg(line, msg, bfd); - /* BIG FAT WARNING: bfd might no longer exist here (ret != 0), - * since ipaccess_rcvmsg() might have free'd it !!! */ - msgb_free(msg); - return ret != 0 ? -EBADF : 0; - } else if (e1i_ts->type == E1INP_TS_TYPE_NONE) { - /* this sign link is not know yet.. complain. */ - LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "Timeslot is not configured.\n"); - goto err_msg; - } - - link = e1inp_lookup_sign_link(e1i_ts, hh->proto, 0); - if (!link) { - LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "no matching signalling link for hh->proto=0x%02x\n", hh->proto); - goto err_msg; - } - msg->dst = link; - - /* XXX better use e1inp_ts_rx? */ - if (!e1i_ts->line->ops->sign_link) { - LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "Fix your application, no action set for signalling messages.\n"); - goto err_msg; - } - rc = e1i_ts->line->ops->sign_link(msg); - if (rc < 0) { - /* Don't close the signalling link if the upper layers report - * an error, that's too strict. BTW, the signalling layer is - * resposible for releasing the message. - */ - LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "Bad signalling message, sign_link returned error: %s.\n", - strerror(-rc)); - } - - return rc; -err_msg: msgb_free(msg); -err: - ipaccess_drop(bfd, line); - return -EBADF; + osmo_stream_srv_destroy(conn); + return -1; } static void ipaccess_close(struct e1inp_sign_link *sign_link) @@ -479,6 +366,7 @@ struct e1inp_line *line = e1i_ts->line; struct osmo_fsm_inst *ka_fsm = e1i_ts->driver.ipaccess.ka_fsm; struct osmo_stream_cli *cli; + struct osmo_stream_srv *conn; /* depending on caller the fsm might be dead */ if (ka_fsm) @@ -495,19 +383,24 @@ bfd->fd = -1; /* Compatibility with older implementations */ break; case E1INP_LINE_R_BSC: - default: - if (bfd->fd != -1) { - osmo_fd_unregister(bfd); - close(bfd->fd); - bfd->fd = -1; - /* If The bfd holds a reference to e1inp_line in ->data (BSC - * accepted() sockets), then release it */ - if (bfd->data == line) { - bfd->data = NULL; - e1inp_line_put2(line, "ipa_bfd"); - } + /* conn may be NULL here, if we are called by user during sign_link_up() + * callback and hence before we finished to internally move the ipaccess conn + * under the proper final line/ts object: + */ + conn = ipaccess_bts_e1i_ts_stream_srv(e1i_ts); + if (conn) { + osmo_stream_srv_destroy(conn); + } else { + LOGPITS(e1i_ts, DLINP, LOGL_DEBUG, + "ipaccess_close() on ts with no srv_conn, " + "probably called during sign_link_up() or sign_link_down() user cb.\n"); } break; + default: + LOGPITS(e1i_ts, DLINP, LOGL_ERROR, + "ipaccess_close() for line with unknown role %d\n", + line->ops->cfg.ipa.role); + break; } } @@ -564,6 +457,57 @@ return rc; } +static int ipaccess_bsc_send_msg(struct e1inp_ts *e1i_ts, + struct e1inp_sign_link *sign_link, + struct osmo_stream_srv *conn, + struct msgb *msg) +{ + switch (sign_link->type) { + case E1INP_SIGN_OML: + case E1INP_SIGN_RSL: + case E1INP_SIGN_OSMO: + break; + default: + LOGPITS(e1i_ts, DLMI, LOGL_NOTICE, "Drop Tx msg for unknown sign_link type %d: %s\n", + sign_link->type, osmo_hexdump(msg->data, msgb_length(msg))); + msgb_free(msg); + return -EINVAL; + } + + msg->l2h = msg->data; + ipa_prepend_header(msg, sign_link->tei); + + LOGPITS(e1i_ts, DLMI, LOGL_DEBUG, "TX: %s\n", osmo_hexdump(msg->l2h, msgb_l2len(msg))); + osmo_stream_srv_send(conn, msg); + return 0; +} + +/* msg was enqueued in sign_link->tx_list. + * Pop it from that list, submit it to osmo_stream_srv. */ +static int ipaccess_bsc_write_cb(struct e1inp_ts *e1i_ts) +{ + int rc = 0; + struct osmo_stream_srv *srv = ipaccess_bts_e1i_ts_stream_srv(e1i_ts); + + /* conn may be NULL here, if we are called by user during sign_link_up() + * ballback and hence before we finished to internally move the ipaccess conn + * under the proper final line/ts object: + */ + if (!srv) { + LOGPITS(e1i_ts, DLMI, LOGL_DEBUG, "Delaying Tx on ts with no srv_conn\n"); + return -ENODEV; + } + + /* get the next msg for this timeslot */ + while (e1i_ts_has_pending_tx_msgs(e1i_ts)) { + struct e1inp_sign_link *sign_link = NULL; + struct msgb *msg; + msg = e1inp_tx_ts(e1i_ts, &sign_link); + rc |= ipaccess_bsc_send_msg(e1i_ts, sign_link, srv, msg); + } + return rc; +} + static int ts_want_write(struct e1inp_ts *e1i_ts) { enum e1inp_line_role role = E1INP_LINE_R_NONE; @@ -578,118 +522,92 @@ /* msg was enqueued in sign_link->tx_list. * Pop it from that list, submit it to osmo_stream_cli: */ return ipaccess_bts_write_cb(e1i_ts); - case E1INP_LINE_R_NONE: case E1INP_LINE_R_BSC: + /* msg was enqueued in sign_link->tx_list. + * Pop it from that list, submit it to osmo_stream_cli: */ + return ipaccess_bsc_write_cb(e1i_ts); + case E1INP_LINE_R_NONE: default: - osmo_fd_write_enable(&e1i_ts->driver.ipaccess.fd); - /* ipaccess_fd_cb will be called from main loop and tx the msgb. */ + LOGPITS(e1i_ts, DLMI, LOGL_ERROR, "want_write for ts with unknown role %d!\n", role); return 0; } } -static void timeout_ts1_write(void *data) +static int ipaccess_bsc_conn_read_cb(struct osmo_stream_srv *conn, int res, struct msgb *msg) { - struct e1inp_ts *e1i_ts = (struct e1inp_ts *)data; + enum ipaccess_proto ipa_proto = osmo_ipa_msgb_cb_proto(msg); + struct e1inp_ts *e1i_ts = osmo_stream_srv_get_data(conn); + struct e1inp_sign_link *link; + int ret, rc; - /* trigger write of ts1, due to tx delay timer */ - if (e1i_ts_has_pending_tx_msgs(e1i_ts)) - ts_want_write(e1i_ts); -} - -static int __handle_ts1_write(struct osmo_fd *bfd, struct e1inp_line *line) -{ - unsigned int ts_nr = bfd->priv_nr; - struct e1inp_ts *e1i_ts; - struct e1inp_sign_link *sign_link; - struct msgb *msg; - int ret; - - e1i_ts = ipaccess_line_ts(bfd, line); - - /* get the next msg for this timeslot */ - msg = e1inp_tx_ts(e1i_ts, &sign_link); - if (!msg) { - /* no message after tx delay timer */ - osmo_fd_write_disable(bfd); - return 0; - } - - switch (sign_link->type) { - case E1INP_SIGN_OML: - case E1INP_SIGN_RSL: - case E1INP_SIGN_OSMO: - break; - default: - /* leave WRITE flag enabled, come back for more msg */ - ret = -EINVAL; - goto out; - } - - msg->l2h = msg->data; - ipa_prepend_header(msg, sign_link->tei); - - LOGPITS(e1i_ts, DLMI, LOGL_DEBUG, "TX %u: %s\n", ts_nr, - osmo_hexdump(msg->l2h, msgb_l2len(msg))); - - ret = send(bfd->fd, msg->data, msg->len, 0); - if (ret != msg->len) { - LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "failed to send A-bis IPA signalling " - "message. Reason: %s\n", strerror(errno)); + if (res <= 0) { + LOGPITS(e1i_ts, DLINP, LOGL_NOTICE, "failed reading from socket: %d\n", res); goto err; } - /* this is some ancient code that apparently exists to slow down writes towards - * some even more ancient nanoBTS 900 units. See git commit - * d49fc5ae24fc9d44d2b284392ab619cc7a69a876 of openbsc.git (now osmo-bsc.git) */ - if (e1i_ts->sign.delay) { - osmo_fd_write_disable(bfd); - /* set tx delay timer for next event */ - osmo_timer_setup(&e1i_ts->sign.tx_timer, timeout_ts1_write, e1i_ts); - osmo_timer_schedule(&e1i_ts->sign.tx_timer, 0, e1i_ts->sign.delay); - } else { -out: - if (!e1i_ts_has_pending_tx_msgs(e1i_ts)) - osmo_fd_write_disable(bfd); + LOGPITS(e1i_ts, DLMI, LOGL_DEBUG, "RX: %s\n", osmo_hexdump(msgb_l2(msg), msgb_l2len(msg))); + + if (ipa_proto == IPAC_PROTO_IPACCESS) { + ret = ipaccess_bsc_rcvmsg(conn, msg); + /* BIG FAT WARNING: bfd might no longer exist here (ret != 0), + * since ipaccess_rcvmsg() might have free'd it !!! */ + return ret != 0 ? -EBADF : 0; } - msgb_free(msg); - return ret; -err: - ipaccess_drop(bfd, line); - msgb_free(msg); - return ret; -} + if (e1i_ts->type == E1INP_TS_TYPE_NONE) { + /* this sign link is not know yet.. complain. */ + LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "Timeslot is not configured.\n"); + goto err; + } -static int handle_ts1_write(struct osmo_fd *bfd) -{ - struct e1inp_line *line = bfd->data; + link = e1inp_lookup_sign_link(e1i_ts, ipa_proto, 0); + if (!link) { + LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "no matching signalling link for ipa_proto=0x%02x\n", ipa_proto); + goto err; + } + msg->dst = link; - return __handle_ts1_write(bfd, line); -} - - -/* callback from select.c in case one of the fd's can be read/written */ -int ipaccess_fd_cb(struct osmo_fd *bfd, unsigned int what) -{ - int rc = 0; - - if (what & OSMO_FD_READ) - rc = handle_ts1_read(bfd); - if (rc != -EBADF && (what & OSMO_FD_WRITE)) - rc = handle_ts1_write(bfd); + /* XXX better use e1inp_ts_rx? */ + if (!e1i_ts->line->ops->sign_link) { + LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "Fix your application, no action set for signalling messages.\n"); + goto err; + } + rc = e1i_ts->line->ops->sign_link(msg); + if (rc < 0) { + /* Don't close the signalling link if the upper layers report + * an error, that's too strict. BTW, the signalling layer is + * resposible for releasing the message. + */ + LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "Bad signalling message, sign_link returned error: %s.\n", + strerror(-rc)); + } return rc; +err: + msgb_free(msg); + osmo_stream_srv_destroy(conn); + return 0; } -static int ipaccess_line_update(struct e1inp_line *line); +static int ipaccess_bsc_conn_closed_cb(struct osmo_stream_srv *conn) +{ + struct e1inp_ts *e1i_ts = osmo_stream_srv_get_data(conn); + struct e1inp_line *line = e1i_ts->line; + struct osmo_fd *bfd = &e1i_ts->driver.ipaccess.fd; + e1inp_line_get2(line, __func__); + e1inp_line_put2(line, "ipa_bfd"); -struct e1inp_driver ipaccess_driver = { - .name = "ipa", - .want_write = ts_want_write, - .line_update = ipaccess_line_update, - .close = ipaccess_close, - .default_delay = 0, - .has_keepalive = 1, -}; + osmo_stats_tcp_osmo_fd_unregister(bfd); + ipaccess_keepalive_fsm_cleanup(e1i_ts); + + osmo_stream_srv_set_data(conn, NULL); + bfd->fd = -1; + bfd->data = NULL; + + if (line->ops->sign_link_down) + line->ops->sign_link_down(line); + e1inp_line_put2(line, __func__); + return 0; +} static void update_fd_settings(struct e1inp_line *line, int fd) { @@ -743,16 +661,19 @@ } /* callback of the OML listening filedescriptor */ -static int ipaccess_bsc_oml_cb(struct ipa_server_link *link, int fd) +static int ipaccess_bsc_oml_accept_cb(struct osmo_stream_srv_link *link, int fd) { int ret; int i; + struct e1inp_line *srv_link_line = osmo_stream_srv_link_get_data(link); struct e1inp_line *line; + struct osmo_stream_srv *conn; struct e1inp_ts *e1i_ts; struct osmo_fd *bfd; + char conn_name[128]; /* clone virtual E1 line for this new OML link. */ - line = e1inp_line_clone(tall_ipa_ctx, link->line, "ipa_bfd"); + line = e1inp_line_clone(tall_ipa_ctx, srv_link_line, "ipa_bfd"); if (line == NULL) { LOGP(DLINP, LOGL_ERROR, "could not clone E1 line\n"); return -ENOMEM; @@ -767,45 +688,47 @@ e1i_ts = e1inp_line_ipa_oml_ts(line); + snprintf(conn_name, sizeof(conn_name), "ts-%u-%u-oml", line->num, e1i_ts->num); + conn = osmo_stream_srv_create2(link, link, fd, e1i_ts); + OSMO_ASSERT(conn); + osmo_stream_srv_set_name(conn, conn_name); + osmo_stream_srv_set_read_cb(conn, ipaccess_bsc_conn_read_cb); + osmo_stream_srv_set_closed_cb(conn, ipaccess_bsc_conn_closed_cb); + osmo_stream_srv_set_segmentation_cb(conn, osmo_ipa_segmentation_cb); + + /* We use bfd->fd in here for osmo_stats_tcp, and bfd->data to access osmo_stream_srv from e1i_ts. */ bfd = &e1i_ts->driver.ipaccess.fd; - osmo_fd_setup(bfd, fd, OSMO_FD_READ, ipaccess_fd_cb, line, E1INP_SIGN_OML); - ret = osmo_fd_register(bfd); - if (ret < 0) { - LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "could not register FD\n"); - goto err_line; - } + osmo_fd_setup(bfd, fd, 0, NULL, conn, E1INP_SIGN_OML); osmo_stats_tcp_osmo_fd_register(bfd, "ipa-oml"); - update_fd_settings(line, bfd->fd); + update_fd_settings(line, fd); /* Request ID. FIXME: request LOCATION, HW/SW VErsion, Unit Name, Serno */ - ret = ipa_ccm_send_id_req(bfd->fd); + ret = ipa_ccm_send_id_req(fd); if (ret < 0) { LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "could not send ID REQ. Reason: %s\n", strerror(errno)); goto err_socket; } - return ret; + return 0; err_socket: - osmo_fd_unregister(bfd); -err_line: - close(bfd->fd); - bfd->fd = -1; - bfd->data = NULL; - e1inp_line_put2(line, "ipa_bfd"); - return ret; + osmo_stream_srv_destroy(conn); + return 0; } -static int ipaccess_bsc_rsl_cb(struct ipa_server_link *link, int fd) +static int ipaccess_bsc_rsl_accept_cb(struct osmo_stream_srv_link *link, int fd) { + struct e1inp_line *srv_link_line = osmo_stream_srv_link_get_data(link); struct e1inp_line *line; + struct osmo_stream_srv *conn; struct e1inp_ts *e1i_ts; struct osmo_fd *bfd; int i, ret; + char conn_name[128]; /* We don't know yet which OML link to associate it with. Thus, we * allocate a temporary E1 line until we have received ID. */ - line = e1inp_line_clone(tall_ipa_ctx, link->line, "ipa_bfd"); + line = e1inp_line_clone(tall_ipa_ctx, srv_link_line, "ipa_bfd"); if (line == NULL) { LOGP(DLINP, LOGL_ERROR, "could not clone E1 line\n"); return -ENOMEM; @@ -814,38 +737,37 @@ for (i = 0; i < ARRAY_SIZE(line->ts); ++i) line->ts[i].driver.ipaccess.fd.fd = -1; - /* we need this to initialize this in case to avoid crashes in case + /* we need to initialize this in case to avoid crashes in case * that the socket is closed before we've seen an ID_RESP. */ e1inp_ts_config_sign(e1inp_line_ipa_oml_ts(line), line); e1i_ts = e1inp_line_ipa_rsl_ts(line, 0); + snprintf(conn_name, sizeof(conn_name), "ts-%u-%u-rsl", line->num, e1i_ts->num); + conn = osmo_stream_srv_create2(link, link, fd, e1i_ts); + OSMO_ASSERT(conn); + osmo_stream_srv_set_name(conn, conn_name); + osmo_stream_srv_set_read_cb(conn, ipaccess_bsc_conn_read_cb); + osmo_stream_srv_set_closed_cb(conn, ipaccess_bsc_conn_closed_cb); + osmo_stream_srv_set_segmentation_cb(conn, osmo_ipa_segmentation_cb); + + /* We use bfd->fd in here for osmo_stats_tcp, and bfd->data to access osmo_stream_srv from e1i_ts. */ bfd = &e1i_ts->driver.ipaccess.fd; - osmo_fd_setup(bfd, fd, OSMO_FD_READ, ipaccess_fd_cb, line, E1INP_SIGN_RSL); - ret = osmo_fd_register(bfd); - if (ret < 0) { - LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "could not register FD\n"); - goto err_line; - } + osmo_fd_setup(bfd, fd, 0, NULL, conn, E1INP_SIGN_RSL); osmo_stats_tcp_osmo_fd_register(bfd, "ipa-rsl"); /* Request ID. FIXME: request LOCATION, HW/SW VErsion, Unit Name, Serno */ - ret = ipa_ccm_send_id_req(bfd->fd); + ret = ipa_ccm_send_id_req(fd); if (ret < 0) { LOGPITS(e1i_ts, DLINP, LOGL_ERROR, "could not send ID REQ. Reason: %s\n", strerror(errno)); goto err_socket; } - update_fd_settings(line, bfd->fd); - return ret; + update_fd_settings(line, fd); + return 0; err_socket: - osmo_fd_unregister(bfd); -err_line: - close(bfd->fd); - bfd->fd = -1; - bfd->data = NULL; - e1inp_line_put2(line, "ipa_bfd"); - return ret; + osmo_stream_srv_destroy(conn); + return 0; } int ipaccess_bts_handle_ccm(struct ipa_client_conn *link, @@ -1103,38 +1025,43 @@ /* We only initialize this line once. */ if (il->line_already_initialized) return 0; - struct ipa_server_link *oml_link, *rsl_link; + struct osmo_stream_srv_link *oml_link, *rsl_link; const char *ipa = e1inp_ipa_get_bind_addr(); LOGPIL(line, DLINP, LOGL_NOTICE, "enabling ipaccess BSC mode on %s " "with OML %u and RSL %u TCP ports\n", ipa, IPA_TCP_PORT_OML, IPA_TCP_PORT_RSL); - oml_link = ipa_server_link_create(tall_ipa_ctx, line, ipa, - IPA_TCP_PORT_OML, - ipaccess_bsc_oml_cb, NULL); - if (oml_link == NULL) { - LOGPIL(line, DLINP, LOGL_ERROR, "cannot create OML BSC link: %s\n", strerror(errno)); - return -ENOMEM; - } - oml_link->dscp = g_e1inp_ipaccess_pars.oml.dscp; - oml_link->priority = g_e1inp_ipaccess_pars.oml.priority; - if (ipa_server_link_open(oml_link) < 0) { - LOGPIL(line, DLINP, LOGL_ERROR, "cannot open OML BSC link: %s\n", strerror(errno)); - ipa_server_link_destroy(oml_link); + oml_link = osmo_stream_srv_link_create(tall_ipa_ctx); + OSMO_ASSERT(oml_link); + osmo_stream_srv_link_set_proto(oml_link, IPPROTO_TCP); + osmo_stream_srv_link_set_addr(oml_link, ipa); + osmo_stream_srv_link_set_port(oml_link, IPA_TCP_PORT_OML); + osmo_stream_srv_link_set_data(oml_link, line); + osmo_stream_srv_link_set_nodelay(oml_link, true); + osmo_stream_srv_link_set_priority(oml_link, g_e1inp_ipaccess_pars.oml.dscp); + osmo_stream_srv_link_set_ip_dscp(oml_link, g_e1inp_ipaccess_pars.oml.priority); + osmo_stream_srv_link_set_accept_cb(oml_link, ipaccess_bsc_oml_accept_cb); + + if (osmo_stream_srv_link_open(oml_link)) { + LOGPIL(line, DLINP, LOGL_ERROR, "cannot open OML BTS link: %s\n", strerror(errno)); + osmo_stream_srv_link_destroy(oml_link); return -EIO; } - rsl_link = ipa_server_link_create(tall_ipa_ctx, line, ipa, - IPA_TCP_PORT_RSL, - ipaccess_bsc_rsl_cb, NULL); - if (rsl_link == NULL) { - LOGPIL(line, DLINP, LOGL_ERROR, "cannot create RSL BSC link: %s\n", strerror(errno)); - return -ENOMEM; - } - rsl_link->dscp = g_e1inp_ipaccess_pars.rsl.dscp; - rsl_link->priority = g_e1inp_ipaccess_pars.rsl.priority; - if (ipa_server_link_open(rsl_link) < 0) { - LOGPIL(line, DLINP, LOGL_ERROR, "cannot open RSL BSC link: %s\n", strerror(errno)); - ipa_server_link_destroy(rsl_link); + + rsl_link = osmo_stream_srv_link_create(tall_ipa_ctx); + OSMO_ASSERT(rsl_link); + osmo_stream_srv_link_set_proto(rsl_link, IPPROTO_TCP); + osmo_stream_srv_link_set_addr(rsl_link, ipa); + osmo_stream_srv_link_set_port(rsl_link, IPA_TCP_PORT_RSL); + osmo_stream_srv_link_set_data(rsl_link, line); + osmo_stream_srv_link_set_nodelay(rsl_link, true); + osmo_stream_srv_link_set_priority(rsl_link, g_e1inp_ipaccess_pars.rsl.dscp); + osmo_stream_srv_link_set_ip_dscp(rsl_link, g_e1inp_ipaccess_pars.rsl.priority); + osmo_stream_srv_link_set_accept_cb(rsl_link, ipaccess_bsc_rsl_accept_cb); + + if (osmo_stream_srv_link_open(rsl_link)) { + LOGPIL(line, DLINP, LOGL_ERROR, "cannot open RSL BTS link: %s\n", strerror(errno)); + osmo_stream_srv_link_destroy(rsl_link); return -EIO; } ret = 0; @@ -1302,6 +1229,15 @@ return 0; } +static struct e1inp_driver ipaccess_driver = { + .name = "ipa", + .want_write = ts_want_write, + .line_update = ipaccess_line_update, + .close = ipaccess_close, + .default_delay = 0, + .has_keepalive = 1, +}; + void e1inp_ipaccess_init(void) { tall_ipa_ctx = talloc_named_const(libosmo_abis_ctx, 1, "ipa"); -- 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: merged Gerrit-Project: libosmo-abis Gerrit-Branch: master Gerrit-Change-Id: Idf241c8f2fdb86d090d4132a9b316b7236402232 Gerrit-Change-Number: 38983 Gerrit-PatchSet: 5 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: laforge <laforge(a)osmocom.org> Gerrit-Reviewer: osmith <osmith(a)sysmocom.de> Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>

1 0

2026

2025

2024

2023

2022

gerrit-log