Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/29551 )
Change subject: vty: 'hopping arfcn add': succeed if adding arfcn already in set
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
I intentionally added this check in Ie27c859e3f16ada08a5cdc8ab4ac6e20a885a378 to facilitate finding duplicates in the hopping configuration. The point is to prevent osmo-bsc from starting in that case. The 'apply-config-file' use case is very specific, I bet there can be similar errors, e.g. when adding neighbor [E]ARFCNs?
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/29551
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ia4e70d20d48a28c46a21dd10358577e5c798744c
Gerrit-Change-Number: 29551
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 20:58:57 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-mgw/+/29535 )
Change subject: osmux: Keep decoding osmux pkt if a batch contains an unknown CID
......................................................................
osmux: Keep decoding osmux pkt if a batch contains an unknown CID
Receiving an unknown CID (for which we don't have an active conn) is
totally expected at the end of the call, were some messages from the
peer may be send for a while, specially if latency is high.
If this occurs, we simply drop that batch and keep decoding the osmux
message in order to keep delivering content to other circuits.
Related: SYS#5987
Change-Id: I315949853bdcc07bef15d2e8579029cc72c427cf
---
M src/libosmo-mgcp/mgcp_osmux.c
1 file changed, 2 insertions(+), 1 deletion(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, but someone else must approve
fixeria: Looks good to me, approved
diff --git a/src/libosmo-mgcp/mgcp_osmux.c b/src/libosmo-mgcp/mgcp_osmux.c
index c3c8427..a4ce024 100644
--- a/src/libosmo-mgcp/mgcp_osmux.c
+++ b/src/libosmo-mgcp/mgcp_osmux.c
@@ -443,7 +443,8 @@
LOGP(DOSMUX, LOGL_DEBUG,
"Cannot find a src conn for circuit_id=%d\n",
osmuxh->circuit_id);
- goto out;
+ rem = msg->len;
+ continue;
}
endp = conn_src->conn->endp;
mgcp_conn_watchdog_kick(conn_src->conn);
--
To view, visit https://gerrit.osmocom.org/c/osmo-mgw/+/29535
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-mgw
Gerrit-Branch: master
Gerrit-Change-Id: I315949853bdcc07bef15d2e8579029cc72c427cf
Gerrit-Change-Number: 29535
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: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/29538 )
Change subject: power_control: add CTRL command for sending default params
......................................................................
power_control: add CTRL command for sending default params
There exists a VTY command for sending default power control params,
but so far there was no CTRL counterpart for it. This patch adds a
SET command 'send-power-control-defaults':
$ osmo_ctrl.py \
--host 127.0.0.1 -p 4249 \
--set "bts.0.send-power-control-defaults" 1
Similar to 'send-new-system-informations', this command takes an
arbitrary dummy value (required for SET), which is simply ignored.
Change-Id: Ib370bd97ee2d9f72f8bec553550b1792d1345387
Related: SYS#6138
(cherry picked from commit 8631189d9ef38107c8a82a770d9d1a826ef8cf29)
---
M doc/manuals/chapters/control.adoc
M doc/manuals/chapters/power_control.adoc
M src/osmo-bsc/bsc_ctrl_commands.c
3 files changed, 42 insertions(+), 2 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
diff --git a/doc/manuals/chapters/control.adoc b/doc/manuals/chapters/control.adoc
index 09e828e..8009ec7 100644
--- a/doc/manuals/chapters/control.adoc
+++ b/doc/manuals/chapters/control.adoc
@@ -33,6 +33,7 @@
|bts.N.cell-identity|RW|No|"<id>"|Set/Get Cell Identity (value between (0, 65535)).
|bts.N.apply-configuration|WO|No|Ignored|Restart BTS via OML.
|bts.N.send-new-system-informations|WO|No|Ignored|Regenerate and resend System Information messages for given BTS.
+|bts.N.send-power-control-defaults|WO|No|Ignored|Resend default power control parameters for given BTS.
|bts.N.channel-load|RO|No|"<name>,<used>,<total>"|See <<chanlo>> for details.
|bts.N.oml-connection-state|RO|No|"connected", "disconnected", "degraded"|Indicate the status of OML connection of BTS.
|bts.N.oml-uptime|RO|No|<uptime>|Return OML link uptime in seconds.
diff --git a/doc/manuals/chapters/power_control.adoc b/doc/manuals/chapters/power_control.adoc
index f5f94f6..9b48f15 100644
--- a/doc/manuals/chapters/power_control.adoc
+++ b/doc/manuals/chapters/power_control.adoc
@@ -53,13 +53,24 @@
link re-establishment and thus interrupting the service. The following command
triggers resending of both MS/BS power control parameters:
+.Example: Resending default power control parameters via the VTY
----
# Resending from the 'enable' node:
-OsmoBSC# bts 0 resend-power-control-defaults
+OsmoBSC# bts 0 <1> resend-power-control-defaults
# Resending from any configuration node (note prefix 'do'):
-OsmoBSC(config-ms-power-ctrl)# do bts 0 resend-power-control-defaults
+OsmoBSC(config-ms-power-ctrl)# do bts 0 <1> resend-power-control-defaults
----
+<1> BTS number for which to resend default power control parameters.
+
+.Example: Resending default power control parameters via the CTRL
+----
+$ osmo_ctrl.py \
+ --host 127.0.0.1 <1> -p 4249 \
+ --set "bts.0.send-power-control-defaults" 1 <2>
+----
+<1> Remote address of the host running osmo-bsc (localhost in this example).
+<2> An arbitrary dummy value (ignored). Required because SET command is used.
NOTE: The above statement mostly applies to parameters for dynamic power control
mode (see below). Switching between power control modes, as well as changing
diff --git a/src/osmo-bsc/bsc_ctrl_commands.c b/src/osmo-bsc/bsc_ctrl_commands.c
index fb8bd0c..304ccf9 100644
--- a/src/osmo-bsc/bsc_ctrl_commands.c
+++ b/src/osmo-bsc/bsc_ctrl_commands.c
@@ -290,6 +290,33 @@
}
CTRL_CMD_DEFINE_WO_NOVRF(bts_si, "send-new-system-informations");
+static int set_bts_power_ctrl_defs(struct ctrl_cmd *cmd, void *data)
+{
+ const struct gsm_bts *bts = cmd->node;
+ const struct gsm_bts_trx *trx;
+
+ if (bts->ms_power_ctrl.mode != GSM_PWR_CTRL_MODE_DYN_BTS) {
+ cmd->reply = "BTS is not using dyn-bts mode";
+ return CTRL_CMD_ERROR;
+ }
+
+ if (bts->model->power_ctrl_send_def_params == NULL) {
+ cmd->reply = "Not implemented for this BTS model";
+ return CTRL_CMD_ERROR;
+ }
+
+ llist_for_each_entry(trx, &bts->trx_list, list) {
+ if (bts->model->power_ctrl_send_def_params(trx) != 0) {
+ cmd->reply = "power_ctrl_send_def_params() failed";
+ return CTRL_CMD_ERROR;
+ }
+ }
+
+ cmd->reply = "Default power control parameters have been sent";
+ return CTRL_CMD_REPLY;
+}
+CTRL_CMD_DEFINE_WO_NOVRF(bts_power_ctrl_defs, "send-power-control-defaults");
+
static int get_bts_chan_load(struct ctrl_cmd *cmd, void *data)
{
int i;
@@ -799,6 +826,7 @@
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_ci);
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_apply_config);
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_si);
+ rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_power_ctrl_defs);
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_chan_load);
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_oml_conn);
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_oml_up);
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/29538
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: 2022q2
Gerrit-Change-Id: Ib370bd97ee2d9f72f8bec553550b1792d1345387
Gerrit-Change-Number: 29538
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
fixeria has submitted this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/29537 )
Change subject: power_control: add CTRL command for sending default params
......................................................................
power_control: add CTRL command for sending default params
There exists a VTY command for sending default power control params,
but so far there was no CTRL counterpart for it. This patch adds a
SET command 'send-power-control-defaults':
$ osmo_ctrl.py \
--host 127.0.0.1 -p 4249 \
--set "bts.0.send-power-control-defaults" 1
Similar to 'send-new-system-informations', this command takes an
arbitrary dummy value (required for SET), which is simply ignored.
Change-Id: Ib370bd97ee2d9f72f8bec553550b1792d1345387
Related: SYS#6138
---
M doc/manuals/chapters/control.adoc
M doc/manuals/chapters/power_control.adoc
M src/osmo-bsc/bsc_ctrl_commands.c
3 files changed, 42 insertions(+), 2 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
diff --git a/doc/manuals/chapters/control.adoc b/doc/manuals/chapters/control.adoc
index 09e828e..8009ec7 100644
--- a/doc/manuals/chapters/control.adoc
+++ b/doc/manuals/chapters/control.adoc
@@ -33,6 +33,7 @@
|bts.N.cell-identity|RW|No|"<id>"|Set/Get Cell Identity (value between (0, 65535)).
|bts.N.apply-configuration|WO|No|Ignored|Restart BTS via OML.
|bts.N.send-new-system-informations|WO|No|Ignored|Regenerate and resend System Information messages for given BTS.
+|bts.N.send-power-control-defaults|WO|No|Ignored|Resend default power control parameters for given BTS.
|bts.N.channel-load|RO|No|"<name>,<used>,<total>"|See <<chanlo>> for details.
|bts.N.oml-connection-state|RO|No|"connected", "disconnected", "degraded"|Indicate the status of OML connection of BTS.
|bts.N.oml-uptime|RO|No|<uptime>|Return OML link uptime in seconds.
diff --git a/doc/manuals/chapters/power_control.adoc b/doc/manuals/chapters/power_control.adoc
index f5f94f6..9b48f15 100644
--- a/doc/manuals/chapters/power_control.adoc
+++ b/doc/manuals/chapters/power_control.adoc
@@ -53,13 +53,24 @@
link re-establishment and thus interrupting the service. The following command
triggers resending of both MS/BS power control parameters:
+.Example: Resending default power control parameters via the VTY
----
# Resending from the 'enable' node:
-OsmoBSC# bts 0 resend-power-control-defaults
+OsmoBSC# bts 0 <1> resend-power-control-defaults
# Resending from any configuration node (note prefix 'do'):
-OsmoBSC(config-ms-power-ctrl)# do bts 0 resend-power-control-defaults
+OsmoBSC(config-ms-power-ctrl)# do bts 0 <1> resend-power-control-defaults
----
+<1> BTS number for which to resend default power control parameters.
+
+.Example: Resending default power control parameters via the CTRL
+----
+$ osmo_ctrl.py \
+ --host 127.0.0.1 <1> -p 4249 \
+ --set "bts.0.send-power-control-defaults" 1 <2>
+----
+<1> Remote address of the host running osmo-bsc (localhost in this example).
+<2> An arbitrary dummy value (ignored). Required because SET command is used.
NOTE: The above statement mostly applies to parameters for dynamic power control
mode (see below). Switching between power control modes, as well as changing
diff --git a/src/osmo-bsc/bsc_ctrl_commands.c b/src/osmo-bsc/bsc_ctrl_commands.c
index fb8bd0c..304ccf9 100644
--- a/src/osmo-bsc/bsc_ctrl_commands.c
+++ b/src/osmo-bsc/bsc_ctrl_commands.c
@@ -290,6 +290,33 @@
}
CTRL_CMD_DEFINE_WO_NOVRF(bts_si, "send-new-system-informations");
+static int set_bts_power_ctrl_defs(struct ctrl_cmd *cmd, void *data)
+{
+ const struct gsm_bts *bts = cmd->node;
+ const struct gsm_bts_trx *trx;
+
+ if (bts->ms_power_ctrl.mode != GSM_PWR_CTRL_MODE_DYN_BTS) {
+ cmd->reply = "BTS is not using dyn-bts mode";
+ return CTRL_CMD_ERROR;
+ }
+
+ if (bts->model->power_ctrl_send_def_params == NULL) {
+ cmd->reply = "Not implemented for this BTS model";
+ return CTRL_CMD_ERROR;
+ }
+
+ llist_for_each_entry(trx, &bts->trx_list, list) {
+ if (bts->model->power_ctrl_send_def_params(trx) != 0) {
+ cmd->reply = "power_ctrl_send_def_params() failed";
+ return CTRL_CMD_ERROR;
+ }
+ }
+
+ cmd->reply = "Default power control parameters have been sent";
+ return CTRL_CMD_REPLY;
+}
+CTRL_CMD_DEFINE_WO_NOVRF(bts_power_ctrl_defs, "send-power-control-defaults");
+
static int get_bts_chan_load(struct ctrl_cmd *cmd, void *data)
{
int i;
@@ -799,6 +826,7 @@
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_ci);
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_apply_config);
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_si);
+ rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_power_ctrl_defs);
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_chan_load);
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_oml_conn);
rc |= ctrl_cmd_install(CTRL_NODE_BTS, &cmd_bts_oml_up);
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/29537
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ib370bd97ee2d9f72f8bec553550b1792d1345387
Gerrit-Change-Number: 29537
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: pespin.
fixeria has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-bsc/+/29537 )
Change subject: power_control: add CTRL command for sending default params
......................................................................
Patch Set 2:
(1 comment)
File src/osmo-bsc/bsc_ctrl_commands.c:
https://gerrit.osmocom.org/c/osmo-bsc/+/29537/comment/ec02a88d_7ff4bd77
PS2, Line 315: cmd->reply = "Default power control parameters have been sent";
> agreeing with pespin
Well, I looked at the 'send-new-system-informations' while adding this new command. I can submit a patch making successful outcome of both commands less verbose, if you think it makes sense.
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/29537
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ib370bd97ee2d9f72f8bec553550b1792d1345387
Gerrit-Change-Number: 29537
Gerrit-PatchSet: 2
Gerrit-Owner: fixeria <vyanitskiy(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 19:13:42 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: laforge <laforge(a)osmocom.org>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/29552 )
Change subject: vty: 'hopping arfcn add': succeed if adding arfcn already in set
......................................................................
vty: 'hopping arfcn add': succeed if adding arfcn already in set
There's no need to fail, simply make it a noop in that case,
everything's fine and everybody is happy (specially when using CTRL
command "apply-config-file" to load a .cfg file containing
modifications.
Related: SYS#6138
(cherry picked from Change-Id Ia4e70d20d48a28c46a21dd10358577e5c798744c)
Change-Id: I1f90f76653a8c4f76d1200ba370dc8fe2a568949
---
M src/osmo-bsc/bts_trx_vty.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/52/29552/1
diff --git a/src/osmo-bsc/bts_trx_vty.c b/src/osmo-bsc/bts_trx_vty.c
index 9cf128e..920df16 100644
--- a/src/osmo-bsc/bts_trx_vty.c
+++ b/src/osmo-bsc/bts_trx_vty.c
@@ -401,7 +401,7 @@
if (bitvec_get_bit_pos(&ts->hopping.arfcns, arfcn) == ONE) {
vty_out(vty, "%% ARFCN %" PRIu16 " is already set%s", arfcn, VTY_NEWLINE);
- return CMD_WARNING;
+ return CMD_SUCCESS;
}
bitvec_set_bit_pos(&ts->hopping.arfcns, arfcn, 1);
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/29552
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: 2022q2
Gerrit-Change-Id: I1f90f76653a8c4f76d1200ba370dc8fe2a568949
Gerrit-Change-Number: 29552
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
pespin has submitted this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536 )
Change subject: Workaround bug where old hnb_context from same remote addr+port is kept
......................................................................
Workaround bug where old hnb_context from same remote addr+port is kept
Under some circumstancies not yet fully known, which seems to
involve bad link quality and high latencies and some specific hNodeB
which reuse its local IP addr+port, it is seen that a 2nd SCTP
connection is created from the same HNB while locally we still keep the
old SCTP connection and its related hnb_context. Hence, when the hNodeB
tries to register again with this new conn, it is rejected all the time
by the HNBGW.
Related: SYS#6113
Change-Id: I33ae901cc37646eca90bf06953e44fcc25f4d6c6
---
M src/osmo-hnbgw/hnbgw_hnbap.c
1 file changed, 29 insertions(+), 2 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/src/osmo-hnbgw/hnbgw_hnbap.c b/src/osmo-hnbgw/hnbgw_hnbap.c
index 1c51b63..23d1f48 100644
--- a/src/osmo-hnbgw/hnbgw_hnbap.c
+++ b/src/osmo-hnbgw/hnbgw_hnbap.c
@@ -395,7 +395,7 @@
static int hnbgw_rx_hnb_register_req(struct hnb_context *ctx, ANY_t *in)
{
- struct hnb_context *hnb;
+ struct hnb_context *hnb, *tmp;
HNBAP_HNBRegisterRequestIEs_t ies;
int rc;
struct osmo_plmn_id plmn;
@@ -422,8 +422,35 @@
ctx->id.mcc = plmn.mcc;
ctx->id.mnc = plmn.mnc;
- llist_for_each_entry(hnb, &ctx->gw->hnb_list, list) {
+ llist_for_each_entry_safe(hnb, tmp, &ctx->gw->hnb_list, list) {
if (hnb->hnb_registered && ctx != hnb && memcmp(&ctx->id, &hnb->id, sizeof(ctx->id)) == 0) {
+ /* If it's coming from the same remote IP addr+port, then it must be our internal
+ * fault (bug), and we release the old context to keep going... */
+ struct osmo_fd *other_fd = osmo_stream_srv_get_ofd(hnb->conn);
+ struct osmo_sockaddr other_osa = {};
+ struct osmo_sockaddr cur_osa = {};
+ socklen_t len = sizeof(other_osa);
+ if (getpeername(other_fd->fd, &other_osa.u.sa, &len) < 0) {
+ LOGHNB(ctx, DHNBAP, LOGL_ERROR, "BUG! Found old registered HNB with invalid socket, releasing it\n");
+ hnb_context_release(hnb);
+ continue;
+ }
+ len = sizeof(cur_osa);
+ if (getpeername(ofd->fd, &cur_osa.u.sa, &len) < 0) {
+ LOGHNB(ctx, DHNBAP, LOGL_ERROR, "Error getpeername(): %s\n", strerror(errno));
+ if (osmo_sockaddr_cmp(&cur_osa, &other_osa) == 0) {
+ LOGHNB(ctx, DHNBAP, LOGL_ERROR, "BUG! Found old registered HNB with same remote address, releasing it\n");
+ hnb_context_release(hnb);
+ continue;
+ }
+ } else if (osmo_sockaddr_cmp(&cur_osa, &other_osa) == 0) {
+ LOGHNB(ctx, DHNBAP, LOGL_ERROR, "BUG! Found old registered HNB with same remote address, releasing it\n");
+ hnb_context_release(hnb);
+ continue;
+ } /* else: addresses are different, we continue below */
+
+ /* If new conn registering same HNB is from anoter remote addr+port, let's reject it to avoid
+ * misconfigurations or someone trying to impersonate an already working HNB: */
LOGHNB(ctx, DHNBAP, LOGL_ERROR, "rejecting HNB-REGISTER-REQ with duplicate cell identity "
"MCC=%u,MNC=%u,LAC=%u,RAC=%u,SAC=%u,CID=%u from %s\n",
ctx->id.mcc, ctx->id.mnc, ctx->id.lac, ctx->id.rac, ctx->id.sac, ctx->id.cid, name);
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I33ae901cc37646eca90bf06953e44fcc25f4d6c6
Gerrit-Change-Number: 29536
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29547 )
Change subject: Close conn when receiving SCTP_ASSOC_CHANGE notification
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> (looks like this function also fails to put() to msgb if sctp_revmsg() rc > 0 in case of NOTIFICATIO […]
This is done in general recv() code path in the other function. This one is for sctp notifications specifically.
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29547
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: If35efd404405f926a4a6cc45862eeadd1b04e08c
Gerrit-Change-Number: 29547
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 16:02:54 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29545 )
Change subject: stream: Return 0 when receiving sctp notification SCTP_COMM_LOST
......................................................................
stream: Return 0 when receiving sctp notification SCTP_COMM_LOST
It was seen on a real pcap trace (sctp & gsmtap_log) that the Linux
kernel stack may decide to kill the connection (sending an ABORT) if
it fails to transmit some data after a while:
ABORT Cause code: "Protocol violation (0x000d)",
Cause Information: "Association exceeded its max_retrans count".
When this occurs, the kernel sends the
MSG_NOTIFICATION,SCTP_ASSOC_CHANGE,SCTP_COMM_LOST notification when
reading from the socket with sctp_recvmsg(). This basically signals that
the socket conn is dead, and subsequent writes to it will result in
send() failures (and receive SCTP_SEND_FAILED notification upon follow
up reads).
It's important to notice that after those events, there's no other sort
of different event like SHUTDOWN coming in, so that's the time at which
we must tell the user to close the socket.
Hence, let's signal the caller that the socket is dead by returning 0,
to comply with usual recv() API.
Related: SYS#6113
Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3
---
M src/stream.c
1 file changed, 2 insertions(+), 1 deletion(-)
Approvals:
pespin: Looks good to me, approved
Jenkins Builder: Verified
diff --git a/src/stream.c b/src/stream.c
index 5e15142..38d24fe 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -1490,7 +1490,8 @@
break;
case SCTP_COMM_LOST:
LOGPC(DLINP, LOGL_DEBUG, " LOST\n");
- break;
+ /* Handle this like a regular disconnect */
+ return 0;
case SCTP_RESTART:
LOGPC(DLINP, LOGL_DEBUG, " RESTART\n");
break;
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29545
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3
Gerrit-Change-Number: 29545
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29550 )
Change subject: stream: Document osmo_stream_srv_recv() SCTP specialties
......................................................................
stream: Document osmo_stream_srv_recv() SCTP specialties
Change-Id: I47b066f26e63afd4bdb135f822667d1cd9479920
---
M src/stream.c
1 file changed, 10 insertions(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
pespin: Looks good to me, approved
laforge: Looks good to me, but someone else must approve
diff --git a/src/stream.c b/src/stream.c
index 38d24fe..8cb3ad0 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -1524,6 +1524,16 @@
* \param[in] conn Stream Server from which to receive
* \param msg pre-allocate message buffer to which received data is appended
* \returns number of bytes read, negative on error.
+ *
+ * If conn is an SCTP connection, additional specific considerations shall be taken:
+ * - msg->cb is always filled with SCTP ppid, and SCTP stream values, see msgb_sctp_*() APIs.
+ * - If an SCTP notification was received when reading from the SCTP socket,
+ * msgb_sctp_msg_flags(msg) will contain bit flag
+ * OSMO_STREAM_SCTP_MSG_FLAGS_NOTIFICATION set, and the msgb will
+ * contain a "union sctp_notification" instead of user data. In this case the
+ * return code will be either 0 (if conn is considered dead after the
+ * notification) or -EAGAIN (if conn is considered still alive after the
+ * notification) resembling the standard recv() API.
*/
int osmo_stream_srv_recv(struct osmo_stream_srv *conn, struct msgb *msg)
{
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29550
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I47b066f26e63afd4bdb135f822667d1cd9479920
Gerrit-Change-Number: 29550
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: neels, laforge.
Hello Jenkins Builder, neels, laforge,
I'd like you to reexamine a change. Please visit
https://gerrit.osmocom.org/c/libosmo-netif/+/29545
to look at the new patch set (#3).
Change subject: stream: Return 0 when receiving sctp notification SCTP_COMM_LOST
......................................................................
stream: Return 0 when receiving sctp notification SCTP_COMM_LOST
It was seen on a real pcap trace (sctp & gsmtap_log) that the Linux
kernel stack may decide to kill the connection (sending an ABORT) if
it fails to transmit some data after a while:
ABORT Cause code: "Protocol violation (0x000d)",
Cause Information: "Association exceeded its max_retrans count".
When this occurs, the kernel sends the
MSG_NOTIFICATION,SCTP_ASSOC_CHANGE,SCTP_COMM_LOST notification when
reading from the socket with sctp_recvmsg(). This basically signals that
the socket conn is dead, and subsequent writes to it will result in
send() failures (and receive SCTP_SEND_FAILED notification upon follow
up reads).
It's important to notice that after those events, there's no other sort
of different event like SHUTDOWN coming in, so that's the time at which
we must tell the user to close the socket.
Hence, let's signal the caller that the socket is dead by returning 0,
to comply with usual recv() API.
Related: SYS#6113
Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3
---
M src/stream.c
1 file changed, 2 insertions(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/libosmo-netif refs/changes/45/29545/3
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29545
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3
Gerrit-Change-Number: 29545
Gerrit-PatchSet: 3
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: laforge <laforge(a)osmocom.org>
Gerrit-MessageType: newpatchset
Attention is currently required from: neels, pespin.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549 )
Change subject: hnb_read_cb(): -EBADF must be returned if conn is freed to avoid use-after-free
......................................................................
Patch Set 2: Code-Review+1
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29549
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I628c59a88d94d299f432f405b37fbe602381d47e
Gerrit-Change-Number: 29549
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 15:56:58 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: pespin.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29548 )
Change subject: hnb_read_cb: use local var to reduce get_ofd() calls
......................................................................
Patch Set 2: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29548
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: Ic7058b5a05b0d34b80617006d4e929a523212221
Gerrit-Change-Number: 29548
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 15:56:17 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
pespin has uploaded this change for review. ( https://gerrit.osmocom.org/c/osmo-bsc/+/29551 )
Change subject: vty: 'hopping arfcn add': succeed if adding arfcn already in set
......................................................................
vty: 'hopping arfcn add': succeed if adding arfcn already in set
There's no need to fail, simply make it a noop in that case,
everything's fine and everybody is happy (specially when using CTRL
command "apply-config-file" to load a .cfg file containing
modifications.
Related: SYS#6138
Change-Id: Ia4e70d20d48a28c46a21dd10358577e5c798744c
---
M src/osmo-bsc/bts_trx_vty.c
1 file changed, 1 insertion(+), 1 deletion(-)
git pull ssh://gerrit.osmocom.org:29418/osmo-bsc refs/changes/51/29551/1
diff --git a/src/osmo-bsc/bts_trx_vty.c b/src/osmo-bsc/bts_trx_vty.c
index 61c52e7..118feca 100644
--- a/src/osmo-bsc/bts_trx_vty.c
+++ b/src/osmo-bsc/bts_trx_vty.c
@@ -402,7 +402,7 @@
if (bitvec_get_bit_pos(&ts->hopping.arfcns, arfcn) == ONE) {
vty_out(vty, "%% ARFCN %" PRIu16 " is already set%s", arfcn, VTY_NEWLINE);
- return CMD_WARNING;
+ return CMD_SUCCESS;
}
bitvec_set_bit_pos(&ts->hopping.arfcns, arfcn, 1);
--
To view, visit https://gerrit.osmocom.org/c/osmo-bsc/+/29551
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-bsc
Gerrit-Branch: master
Gerrit-Change-Id: Ia4e70d20d48a28c46a21dd10358577e5c798744c
Gerrit-Change-Number: 29551
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: newchange
pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29540 )
Change subject: stream: Set proper msgb length when returning sctp_notification
......................................................................
stream: Set proper msgb length when returning sctp_notification
This allows the user to properly check the size of the content in case
the struct ABI changes.
-EAGAIN is still returned to avoid older users of the API reading SCTP
notifications as user data.
Change-Id: I95e2457498fd8e0d790d221cb97695ace0dd673e
---
M src/stream.c
1 file changed, 1 insertion(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
neels: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
diff --git a/src/stream.c b/src/stream.c
index b45b730..31aecc4 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -1476,6 +1476,7 @@
if (flags & MSG_NOTIFICATION) {
union sctp_notification *notif = (union sctp_notification *)msgb_data(msg);
LOGP(DLINP, LOGL_DEBUG, "NOTIFICATION %u flags=0x%x\n", notif->sn_header.sn_type, notif->sn_header.sn_flags);
+ msgb_put(msg, sizeof(union sctp_notification));
msgb_sctp_msg_flags(msg) = OSMO_STREAM_SCTP_MSG_FLAGS_NOTIFICATION;
switch (notif->sn_header.sn_type) {
case SCTP_ASSOC_CHANGE:
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29540
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I95e2457498fd8e0d790d221cb97695ace0dd673e
Gerrit-Change-Number: 29540
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: merged
pespin has submitted this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29541 )
Change subject: stream: Erase sctp_msg_flags if receiving user data
......................................................................
stream: Erase sctp_msg_flags if receiving user data
It could be that the user reuses the msgb passed to
osmo_stream_srv_recv() all the time, hence we need to reset the flags,
otherwise it may end up being set forever.
Change-Id: Id358140db82b018e3973111e530834589c0b7224
---
M src/stream.c
1 file changed, 1 insertion(+), 0 deletions(-)
Approvals:
Jenkins Builder: Verified
laforge: Looks good to me, approved
diff --git a/src/stream.c b/src/stream.c
index 31aecc4..78238b6 100644
--- a/src/stream.c
+++ b/src/stream.c
@@ -1473,6 +1473,7 @@
ret = sctp_recvmsg(fd, msgb_data(msg), msgb_tailroom(msg),
NULL, NULL, &sinfo, &flags);
+ msgb_sctp_msg_flags(msg) = 0;
if (flags & MSG_NOTIFICATION) {
union sctp_notification *notif = (union sctp_notification *)msgb_data(msg);
LOGP(DLINP, LOGL_DEBUG, "NOTIFICATION %u flags=0x%x\n", notif->sn_header.sn_type, notif->sn_header.sn_flags);
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29541
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Id358140db82b018e3973111e530834589c0b7224
Gerrit-Change-Number: 29541
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: pespin <pespin(a)sysmocom.de>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: merged
Attention is currently required from: neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29545 )
Change subject: stream: Return 0 when receiving sctp notification SCTP_COMM_LOST
......................................................................
Patch Set 2:
(2 comments)
Commit Message:
https://gerrit.osmocom.org/c/libosmo-netif/+/29545/comment/95689439_c01e536a
PS1, Line 9: kernel
> I'm not aware that we officialy support any other kernel/OS so far.
Done
https://gerrit.osmocom.org/c/libosmo-netif/+/29545/comment/5596ded7_b1db2ff2
PS1, Line 23: death
> dead
Done
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29545
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: If94d44f25b76a96a5ea402fec9fc14c4e6296ba3
Gerrit-Change-Number: 29545
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 15:40:18 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels, pespin.
laforge has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29543 )
Change subject: stream: Set sctp_ppid and sctp_stream when sctp notifciation is received
......................................................................
Patch Set 1: Code-Review+2
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29543
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I99f6098d8d9fc1c06bc28373bf7ee76f15d5f525
Gerrit-Change-Number: 29543
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 15:32:22 +0000
Gerrit-HasComments: No
Gerrit-Has-Labels: Yes
Gerrit-MessageType: comment
Attention is currently required from: neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29540 )
Change subject: stream: Set proper msgb length when returning sctp_notification
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> this case of sctp_notification is confusing; if it was clear, the ret from sctp_recvmsg() would retu […]
No, tha wouldn't make sense. return code is >0 marking the amount of "user data" read from the conn. What we talk here (sctp_notification) is out-of-band data. The caller knowing nothing about that, aka using the API as regular recv() should not interact with that that, that's why -EAGAIN is returned when out-of-band data is received.
Willing to change that would break lots of existing code.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29540
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I95e2457498fd8e0d790d221cb97695ace0dd673e
Gerrit-Change-Number: 29540
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 15:28:46 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29540 )
Change subject: stream: Set proper msgb length when returning sctp_notification
......................................................................
Patch Set 1: Code-Review+1
(2 comments)
Patchset:
PS1:
this case of sctp_notification is confusing; if it was clear, the ret from sctp_recvmsg() would return the size of the union sctp_notification, and we would always msgb_put() if rc > 0, and not use sizeof(). Do you think that could work / makes sense / have you tried that?
'man sctp_recvmsg' is a very short joke; would be nice to have the relation of ret, flags and sizeof explained by in-code comments, at least what we assume to be true.
if there's anything you can improve, that would be welcome.
File src/stream.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/29540/comment/9d76d392_ac28d363
PS1, Line 1563: msgb_put(msg, ret);
> This is the generic and regular place where user data content is updated in the msgb, so it's fine h […]
i understand now that this is needed here for the recv() case.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29540
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I95e2457498fd8e0d790d221cb97695ace0dd673e
Gerrit-Change-Number: 29540
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 15:23:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: Yes
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Comment-In-Reply-To: pespin <pespin(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29541 )
Change subject: stream: Erase sctp_msg_flags if receiving user data
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
> ah! now i understand, this is about clearing previous state in a msgb passed in from the caller. […]
Not sure what you mean. The function doesn't touch any of those l1234h header, it just puts data at the end of the msgb. It's freedom of the caller to prepare the msgb as he wants regarding where data read is to be stored.
In any case, that's a separate topic.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29541
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Id358140db82b018e3973111e530834589c0b7224
Gerrit-Change-Number: 29541
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 15:17:39 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29541 )
Change subject: stream: Erase sctp_msg_flags if receiving user data
......................................................................
Patch Set 1:
(1 comment)
Patchset:
PS1:
ah! now i understand, this is about clearing previous state in a msgb passed in from the caller. I completely failed to understand that from the commit log.
In that case, how about the msgb->head, ->tail, ->data state, the l[1234]h, all of that might be completely messed up in the msgb provided by the caller, and by writing into the msgb data buffer none of them are accurate anymore.
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29541
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: Id358140db82b018e3973111e530834589c0b7224
Gerrit-Change-Number: 29541
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 15:01:32 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment
Attention is currently required from: neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/libosmo-netif/+/29550 )
Change subject: stream: Document osmo_stream_srv_recv() SCTP specialties
......................................................................
Patch Set 1:
(2 comments)
File src/stream.c:
https://gerrit.osmocom.org/c/libosmo-netif/+/29550/comment/1aab8d22_52b187a5
PS1, Line 1532: he
> she
Ack
https://gerrit.osmocom.org/c/libosmo-netif/+/29550/comment/9d086671_5f29eda4
PS1, Line 1536: * notification) resembling the standard recv() API.
> "and the msgb contains the SCTP notification data" (if that is correct) ?
I just say that exact same thing 2 lines above in the sentence immediately before?
--
To view, visit https://gerrit.osmocom.org/c/libosmo-netif/+/29550
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: libosmo-netif
Gerrit-Branch: master
Gerrit-Change-Id: I47b066f26e63afd4bdb135f822667d1cd9479920
Gerrit-Change-Number: 29550
Gerrit-PatchSet: 1
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 14:55:34 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: neels.
pespin has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536 )
Change subject: Workaround bug where old hnb_context from same remote addr+port is kept
......................................................................
Patch Set 2:
(3 comments)
File src/osmo-hnbgw/hnbgw_hnbap.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536/comment/c3a06a27_fa7c5819
PS2, Line 426: if (hnb->hnb_registered && ctx != hnb && memcmp(&ctx->id, &hnb->id, sizeof(ctx->id)) == 0) {
> (early-exit coding style would be nicer like […]
that'd be a separate path in any case, I'm not really touching this line in this patch.
https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536/comment/d1907fc0_47ba37d3
PS2, Line 428: bug
> is it really a bug?? […]
Yes, it's a bug, this is not expected to happen. See SYS#6113.
I think I already provided a fix for the bug, but in case we have a similar one, this should at least keep the HNB ongoing in production despite of having such a bug.
https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536/comment/96776812_5b6591ac
PS2, Line 434: LOGHNB(ctx, DHNBAP, LOGL_ERROR, "BUG! Found old registered HNB with invalid socket, releasing it\n");
> IMHO the error logs could more clearly indicate that the same HNB is registering a second time, that […]
You get that info when the loop finished, because you see the message NB-REGISTER-REQ...
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I33ae901cc37646eca90bf06953e44fcc25f4d6c6
Gerrit-Change-Number: 29536
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 14:54:28 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Comment-In-Reply-To: neels <nhofmeyr(a)sysmocom.de>
Gerrit-MessageType: comment
Attention is currently required from: pespin.
neels has posted comments on this change. ( https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536 )
Change subject: Workaround bug where old hnb_context from same remote addr+port is kept
......................................................................
Patch Set 2:
(3 comments)
File src/osmo-hnbgw/hnbgw_hnbap.c:
https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536/comment/dbed6eb0_e9d57d1e
PS2, Line 426: if (hnb->hnb_registered && ctx != hnb && memcmp(&ctx->id, &hnb->id, sizeof(ctx->id)) == 0) {
(early-exit coding style would be nicer like
if (!(...))
continue;
...
then the new code block would need less indenting)
https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536/comment/6184dee5_2e036efb
PS2, Line 428: bug
is it really a bug??
from the commit log it sounds like it is handling a hnb peer that opens a second conn for the same context?
if it's a bug it should be possible to figure out how such bug is possible?
https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536/comment/738ca6fc_1f96b35e
PS2, Line 434: LOGHNB(ctx, DHNBAP, LOGL_ERROR, "BUG! Found old registered HNB with invalid socket, releasing it\n");
IMHO the error logs could more clearly indicate that the same HNB is registering a second time, that we have an old record of the *same* HNB that is requesting to register now.
--
To view, visit https://gerrit.osmocom.org/c/osmo-hnbgw/+/29536
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings
Gerrit-Project: osmo-hnbgw
Gerrit-Branch: master
Gerrit-Change-Id: I33ae901cc37646eca90bf06953e44fcc25f4d6c6
Gerrit-Change-Number: 29536
Gerrit-PatchSet: 2
Gerrit-Owner: pespin <pespin(a)sysmocom.de>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge(a)osmocom.org>
Gerrit-CC: neels <nhofmeyr(a)sysmocom.de>
Gerrit-Attention: pespin <pespin(a)sysmocom.de>
Gerrit-Comment-Date: Fri, 30 Sep 2022 14:48:35 +0000
Gerrit-HasComments: Yes
Gerrit-Has-Labels: No
Gerrit-MessageType: comment