<p>laforge <strong>submitted</strong> this change.</p><p><a href="https://gerrit.osmocom.org/c/osmo-pcu/+/22827">View Change</a></p><div style="white-space:pre-wrap">Approvals:
Jenkins Builder: Verified
daniel: Looks good to me, but someone else must approve
laforge: Looks good to me, approved
</div><pre style="font-family: monospace,monospace; white-space: pre-wrap;">nacc_fsm: Support receiving Pkt Cell Change Notify in state WAIT_RESOLVE_RAC_CI<br><br>If the message is a duplicate (same tgt cell), simply ignore it.<br>If the message contains a different tgt cell, restart the resolution:<br>* Avoid re-creating the socket in that case<br>* Avoid potentially picking a CTRL response for an older request<br><br>Related: SYS#4909<br>Change-Id: Ia2ed2580bbbdd6d3464833257b0dcb8ec6f8d699<br>---<br>M src/nacc_fsm.c<br>M src/neigh_cache.c<br>M src/neigh_cache.h<br>3 files changed, 62 insertions(+), 21 deletions(-)<br><br></pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><span>diff --git a/src/nacc_fsm.c b/src/nacc_fsm.c</span><br><span>index 99c8164..8d5f23d 100644</span><br><span>--- a/src/nacc_fsm.c</span><br><span>+++ b/src/nacc_fsm.c</span><br><span>@@ -40,6 +40,9 @@</span><br><span> </span><br><span> #define X(s) (1 << (s))</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+/* Infer CTRL id (seqnum) for a given tgt arfcn+bsic (bsic range: 0-63) */</span><br><span style="color: hsl(120, 100%, 40%);">+#define arfcn_bsic_2_ctrl_id(arfcn, bsic) ((arfcn) * 100 + (bsic))</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> static const struct osmo_tdef_state_timeout nacc_fsm_timeouts[32] = {</span><br><span> [NACC_ST_INITIAL] = {},</span><br><span> [NACC_ST_WAIT_RESOLVE_RAC_CI] = { .T = PCU_TDEF_NEIGH_RESOLVE_TO },</span><br><span>@@ -354,15 +357,20 @@</span><br><span> LOGPFSML(fi, LOGL_DEBUG, "No CGI-PS found in cache, resolving " NEIGH_CACHE_ENTRY_KEY_FMT "...\n",</span><br><span> NEIGH_CACHE_ENTRY_KEY_ARGS(&ctx->neigh_key));</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- rc = osmo_sock_init2_ofd(&ctx->neigh_ctrl_conn->write_queue.bfd,</span><br><span style="color: hsl(0, 100%, 40%);">- AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP,</span><br><span style="color: hsl(0, 100%, 40%);">- NULL, 0, pcu->vty.neigh_ctrl_addr, pcu->vty.neigh_ctrl_port,</span><br><span style="color: hsl(0, 100%, 40%);">- OSMO_SOCK_F_CONNECT);</span><br><span style="color: hsl(0, 100%, 40%);">- if (rc < 0) {</span><br><span style="color: hsl(0, 100%, 40%);">- LOGPFSML(fi, LOGL_ERROR,</span><br><span style="color: hsl(0, 100%, 40%);">- "Failed to establish CTRL (neighbor resolution) connection to BSC r=%s:%u\n\n",</span><br><span style="color: hsl(0, 100%, 40%);">- pcu->vty.neigh_ctrl_addr, pcu->vty.neigh_ctrl_port);</span><br><span style="color: hsl(0, 100%, 40%);">- goto err_term;</span><br><span style="color: hsl(120, 100%, 40%);">+ /* We may have changed to this state previously (eg: we are handling</span><br><span style="color: hsl(120, 100%, 40%);">+ * another Pkt cell Change Notify with different target). Avoid</span><br><span style="color: hsl(120, 100%, 40%);">+ * re-creating the socket in that case. */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (ctx->neigh_ctrl_conn->write_queue.bfd.fd == -1) {</span><br><span style="color: hsl(120, 100%, 40%);">+ rc = osmo_sock_init2_ofd(&ctx->neigh_ctrl_conn->write_queue.bfd,</span><br><span style="color: hsl(120, 100%, 40%);">+ AF_UNSPEC, SOCK_STREAM, IPPROTO_TCP,</span><br><span style="color: hsl(120, 100%, 40%);">+ NULL, 0, pcu->vty.neigh_ctrl_addr, pcu->vty.neigh_ctrl_port,</span><br><span style="color: hsl(120, 100%, 40%);">+ OSMO_SOCK_F_CONNECT);</span><br><span style="color: hsl(120, 100%, 40%);">+ if (rc < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPFSML(fi, LOGL_ERROR,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Failed to establish CTRL (neighbor resolution) connection to BSC r=%s:%u\n\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ pcu->vty.neigh_ctrl_addr, pcu->vty.neigh_ctrl_port);</span><br><span style="color: hsl(120, 100%, 40%);">+ goto err_term;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> }</span><br><span> </span><br><span> cmd = ctrl_cmd_create(ctx, CTRL_TYPE_GET);</span><br><span>@@ -371,7 +379,8 @@</span><br><span> goto err_term;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- cmd->id = talloc_asprintf(cmd, "1");</span><br><span style="color: hsl(120, 100%, 40%);">+ cmd->id = talloc_asprintf(cmd, "%u", arfcn_bsic_2_ctrl_id(ctx->neigh_key.tgt_arfcn,</span><br><span style="color: hsl(120, 100%, 40%);">+ ctx->neigh_key.tgt_bsic));</span><br><span> cmd->variable = talloc_asprintf(cmd, "neighbor_resolve_cgi_ps_from_lac_ci.%d.%d.%d.%d",</span><br><span> ctx->neigh_key.local_lac, ctx->neigh_key.local_ci,</span><br><span> ctx->neigh_key.tgt_arfcn, ctx->neigh_key.tgt_bsic);</span><br><span>@@ -392,8 +401,25 @@</span><br><span> </span><br><span> static void st_wait_resolve_rac_ci(struct osmo_fsm_inst *fi, uint32_t event, void *data)</span><br><span> {</span><br><span style="color: hsl(120, 100%, 40%);">+ struct nacc_fsm_ctx *ctx = (struct nacc_fsm_ctx *)fi->priv;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct gprs_rlcmac_bts *bts = ctx->ms->bts;</span><br><span style="color: hsl(120, 100%, 40%);">+ Packet_Cell_Change_Notification_t *notif;</span><br><span style="color: hsl(120, 100%, 40%);">+ struct neigh_cache_entry_key neigh_key;</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> switch (event) {</span><br><span> case NACC_EV_RX_CELL_CHG_NOTIFICATION:</span><br><span style="color: hsl(120, 100%, 40%);">+ notif = (Packet_Cell_Change_Notification_t *)data;</span><br><span style="color: hsl(120, 100%, 40%);">+ if (fill_neigh_key_from_bts_pkt_cell_chg_not(&neigh_key, bts, notif) < 0) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPFSML(fi, LOGL_NOTICE, "TargetCell type=0x%x not supported\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ notif->Target_Cell.UnionType);</span><br><span style="color: hsl(120, 100%, 40%);">+ nacc_fsm_state_chg(fi, NACC_ST_TX_CELL_CHG_CONTINUE);</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+ /* If tgt cell changed, restart resolving it */</span><br><span style="color: hsl(120, 100%, 40%);">+ if (!neigh_cache_entry_key_eq(&ctx->neigh_key, &neigh_key)) {</span><br><span style="color: hsl(120, 100%, 40%);">+ ctx->neigh_key = neigh_key;</span><br><span style="color: hsl(120, 100%, 40%);">+ nacc_fsm_state_chg(fi, NACC_ST_WAIT_RESOLVE_RAC_CI);</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span> break;</span><br><span> case NACC_EV_RX_RAC_CI:</span><br><span> /* Assumption: ctx->cgi_ps has been filled by caller of the event */</span><br><span>@@ -588,8 +614,10 @@</span><br><span> },</span><br><span> [NACC_ST_WAIT_RESOLVE_RAC_CI] = {</span><br><span> .in_event_mask =</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NACC_EV_RX_CELL_CHG_NOTIFICATION) |</span><br><span> X(NACC_EV_RX_RAC_CI),</span><br><span> .out_state_mask =</span><br><span style="color: hsl(120, 100%, 40%);">+ X(NACC_ST_WAIT_RESOLVE_RAC_CI) |</span><br><span> X(NACC_ST_WAIT_REQUEST_SI) |</span><br><span> X(NACC_ST_TX_CELL_CHG_CONTINUE),</span><br><span> .name = "WAIT_RESOLVE_RAC_CI",</span><br><span>@@ -662,15 +690,28 @@</span><br><span> {</span><br><span> struct nacc_fsm_ctx *ctx = (struct nacc_fsm_ctx *)data;</span><br><span> char *tmp = NULL, *tok, *saveptr;</span><br><span style="color: hsl(120, 100%, 40%);">+ unsigned int exp_id;</span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">- LOGPFSML(ctx->fi, LOGL_NOTICE, "Received CTRL message: type=%d %s: %s\n",</span><br><span style="color: hsl(0, 100%, 40%);">- cmd->type, cmd->variable, osmo_escape_str(cmd->reply, -1));</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPFSML(ctx->fi, LOGL_NOTICE, "Received CTRL message: type=%d %s %s: %s\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ cmd->type, cmd->variable, cmd->id, osmo_escape_str(cmd->reply, -1));</span><br><span> </span><br><span> if (cmd->type != CTRL_TYPE_GET_REPLY || !cmd->reply) {</span><br><span> nacc_fsm_state_chg(ctx->fi, NACC_ST_TX_CELL_CHG_CONTINUE);</span><br><span> return;</span><br><span> }</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+ /* Validate it's the seqnum from our last GET cmd, and not from older</span><br><span style="color: hsl(120, 100%, 40%);">+ * one we may have requested in case MS decided to resend Pkt Cell</span><br><span style="color: hsl(120, 100%, 40%);">+ * Change Notify with a different tgt cell:</span><br><span style="color: hsl(120, 100%, 40%);">+ */</span><br><span style="color: hsl(120, 100%, 40%);">+ exp_id = arfcn_bsic_2_ctrl_id(ctx->neigh_key.tgt_arfcn, ctx->neigh_key.tgt_bsic);</span><br><span style="color: hsl(120, 100%, 40%);">+ if ((unsigned int)atoi(cmd->id) != exp_id) {</span><br><span style="color: hsl(120, 100%, 40%);">+ LOGPFSML(ctx->fi, LOGL_INFO,</span><br><span style="color: hsl(120, 100%, 40%);">+ "Received CTRL message with id=%s doesn't match our expected last id=%d, ignoring\n",</span><br><span style="color: hsl(120, 100%, 40%);">+ cmd->id, exp_id);</span><br><span style="color: hsl(120, 100%, 40%);">+ return;</span><br><span style="color: hsl(120, 100%, 40%);">+ }</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> /* TODO: Potentially validate cmd->variable contains same params as we</span><br><span> sent, and that cmd->id matches the original set. We may want to keep</span><br><span> the original cmd around by setting cmd->defer=1 when sending it. */</span><br><span>diff --git a/src/neigh_cache.c b/src/neigh_cache.c</span><br><span>index ae619d3..3217f40 100644</span><br><span>--- a/src/neigh_cache.c</span><br><span>+++ b/src/neigh_cache.c</span><br><span>@@ -27,15 +27,6 @@</span><br><span> #include <neigh_cache.h></span><br><span> #include <gprs_debug.h></span><br><span> </span><br><span style="color: hsl(0, 100%, 40%);">-static inline bool neigh_cache_entry_key_eq(const struct neigh_cache_entry_key *a,</span><br><span style="color: hsl(0, 100%, 40%);">- const struct neigh_cache_entry_key *b)</span><br><span style="color: hsl(0, 100%, 40%);">-{</span><br><span style="color: hsl(0, 100%, 40%);">- return a->local_lac == b->local_lac &&</span><br><span style="color: hsl(0, 100%, 40%);">- a->local_ci == b->local_ci &&</span><br><span style="color: hsl(0, 100%, 40%);">- a->tgt_arfcn == b->tgt_arfcn &&</span><br><span style="color: hsl(0, 100%, 40%);">- a->tgt_bsic == b->tgt_bsic;</span><br><span style="color: hsl(0, 100%, 40%);">-}</span><br><span style="color: hsl(0, 100%, 40%);">-</span><br><span> static void neigh_cache_schedule_cleanup(struct neigh_cache *cache);</span><br><span> static void neigh_cache_cleanup_cb(void *data)</span><br><span> {</span><br><span>diff --git a/src/neigh_cache.h b/src/neigh_cache.h</span><br><span>index 4fed0fa..90260cd 100644</span><br><span>--- a/src/neigh_cache.h</span><br><span>+++ b/src/neigh_cache.h</span><br><span>@@ -48,6 +48,15 @@</span><br><span> #define NEIGH_CACHE_ENTRY_KEY_FMT "%" PRIu16 "-%" PRIu16 "-%" PRIu16 "-%" PRIu8</span><br><span> #define NEIGH_CACHE_ENTRY_KEY_ARGS(key) (key)->local_lac, (key)->local_ci, (key)->tgt_arfcn, (key)->tgt_bsic</span><br><span> </span><br><span style="color: hsl(120, 100%, 40%);">+static inline bool neigh_cache_entry_key_eq(const struct neigh_cache_entry_key *a,</span><br><span style="color: hsl(120, 100%, 40%);">+ const struct neigh_cache_entry_key *b)</span><br><span style="color: hsl(120, 100%, 40%);">+{</span><br><span style="color: hsl(120, 100%, 40%);">+ return a->local_lac == b->local_lac &&</span><br><span style="color: hsl(120, 100%, 40%);">+ a->local_ci == b->local_ci &&</span><br><span style="color: hsl(120, 100%, 40%);">+ a->tgt_arfcn == b->tgt_arfcn &&</span><br><span style="color: hsl(120, 100%, 40%);">+ a->tgt_bsic == b->tgt_bsic;</span><br><span style="color: hsl(120, 100%, 40%);">+}</span><br><span style="color: hsl(120, 100%, 40%);">+</span><br><span> struct neigh_cache_entry {</span><br><span> struct llist_head list; /* to be included in neigh_cache->list */</span><br><span> struct timespec update_ts;</span><br><span></span><br></pre><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-pcu/+/22827">change 22827</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.osmocom.org/c/osmo-pcu/+/22827"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-pcu </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ia2ed2580bbbdd6d3464833257b0dcb8ec6f8d699 </div>
<div style="display:none"> Gerrit-Change-Number: 22827 </div>
<div style="display:none"> Gerrit-PatchSet: 3 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: daniel <dwillmann@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-MessageType: merged </div>