https://osmocom.org/https://osmocom.org/favicon.ico?16647414092019-05-08T17:41:35ZOpen Source Mobile CommunicationsOsmoBSC - Bug #3833: Fix storage location of AMR S15-S0 bitshttps://osmocom.org/issues/3833?journal_id=143182019-05-08T17:41:35Zlaforge
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>Feedback</i></li></ul><p>both patches are merged. Does that fix/resolved this issue?</p> OsmoBSC - Bug #3833: Fix storage location of AMR S15-S0 bitshttps://osmocom.org/issues/3833?journal_id=146332019-05-28T11:48:00Zdexter
<ul></ul><p>Its a "cosmetic" problem. Neels has some model in mind where temporary and permanent values should be stored. I am not sure that I understand every aspect of this right. I think we should wait until neels is back and then we can fix this together. There is no pressure on this, technically its just moving a struct member to the correct sub struct to have everything consistent.</p> OsmoBSC - Bug #3833: Fix storage location of AMR S15-S0 bitshttps://osmocom.org/issues/3833?journal_id=154412019-07-29T14:02:00Zneelsnhofmeyr@sysmocom.de
<ul></ul><p>lchan->activate.info = the settings that were requested on lchan_activate(). So lchan->activate.* is preliminary and volatile, used before the lchan actually is activated successfully.</p>
<p>As soon as the lchan activation is successful, all valid information should end up in lchan->foo fields (outside of lchan->activate),<br />in this case probably lchan->s15_s0.</p>
<p>The distinction is quite trivial. lchan->activate contains the settings that are requested before setting up the lchan, and aren't verified to be really active.</p>
<p>At any time, some code (bug?) might try to invoke another activation request and overwrite lchan->activate.* data.<br />So any items that describe the actually active lchan should not be kept in lchan->activate.*</p>
<p>A similar concept exists in various other FSMs, it is an important mechanism that separates possibly invalid manipulation requests from the actually verified active settings.</p> OsmoBSC - Bug #3833: Fix storage location of AMR S15-S0 bitshttps://osmocom.org/issues/3833?journal_id=170442020-01-08T22:28:38Zlaforge
<ul><li><strong>Priority</strong> changed from <i>Normal</i> to <i>Low</i></li></ul> OsmoBSC - Bug #3833: Fix storage location of AMR S15-S0 bitshttps://osmocom.org/issues/3833?journal_id=172572020-01-20T14:16:26Zneelsnhofmeyr@sysmocom.de
<ul><li><strong>Status</strong> changed from <i>Feedback</i> to <i>New</i></li><li><strong>Assignee</strong> changed from <i>dexter</i> to <i>neels</i></li></ul><p>laforge wrote:</p>
<blockquote>
<p>both patches are merged. Does that fix/resolved this issue?</p>
</blockquote>
<p>The linked issue <a class="external" href="https://gerrit.osmocom.org/c/osmo-bsc/+/13039">https://gerrit.osmocom.org/c/osmo-bsc/+/13039</a> was merged still containing the problem.<br />It is however "merely" a cosmetic / code maintenance problem, not distinguishable by the user.<br />I guess I should just clean this up since original authors clearly can't be bothered.</p> OsmoBSC - Bug #3833: Fix storage location of AMR S15-S0 bitshttps://osmocom.org/issues/3833?journal_id=172592020-01-20T14:17:11Zneelsnhofmeyr@sysmocom.de
<ul><li><strong>Related to</strong> <i><a class="issue tracker-1 status-1 priority-1 priority-lowest" href="/issues/3787">Bug #3787</a>: Assignment Request modifies conn->codec_list before the Assignment is successful</i> added</li></ul> OsmoBSC - Bug #3833: Fix storage location of AMR S15-S0 bitshttps://osmocom.org/issues/3833?journal_id=201862020-10-28T21:12:25Zneelsnhofmeyr@sysmocom.de
<ul></ul><p>quoting G#13039 comment:</p>
<blockquote>
<p>The assignment_fsm.c stores the chosen ch_mode_rate in lchan->ch_mode_rate, even though the assignment or activation are not complete yet, which violates above ideas and confuses transitory and accepted config.</p>
<p>So assignment_fsm_start() should not leak its state into lchan->*, but should keep a separate ch_mode_rate somewhere else, I'd say in conn->assignment.new_lchan_ch_mode_rate or so. (If it really needs to store it at all.)</p>
<p>This ch_mode_rate should get passed to lchan activation, where lchan_fsm.c then first stores it in lchan->activate.* as transitory state, and when the activation was ACKed copies it to lchan->ch_mode_rate.</p>
<p>That's how we ensure the lchan->* items reflect what is currently used, and transitory requests make their potential mess elsewhere.</p>
</blockquote> OsmoBSC - Bug #3833: Fix storage location of AMR S15-S0 bitshttps://osmocom.org/issues/3833?journal_id=220272021-04-28T14:49:40Zneelsnhofmeyr@sysmocom.de
<ul><li><strong>Status</strong> changed from <i>New</i> to <i>In Progress</i></li></ul><p>cleaning up (part of?) this in the process of <a class="issue tracker-2 status-3 priority-3 priority-high3 closed" title="Feature: handover: for intra-cell re-assignment, use Assignment procedure, not Handover (Resolved)" href="https://osmocom.org/issues/3277">#3277</a></p>
<p>The reason why this is required is that for a re-assignment, the lchan needs to reflect the currently activated chan_mode_rate.<br />So far the chosen chan_mode_rate is only stored in the <strong>previous</strong> lchan data, which defies all logic.</p>
<p>A set of chan_mode_rates shall be chosen in the Assignment procedure, placed in a struct assignment_request,<br />one of them shall be chosen and placed in struct assignment_fsm_data.<br />Upon channel activation/modify the chosen chan_mode_rate must pass through struct lchan_activate_info or lchan_modify_info,<br />and finally, when the activation/modify is ACKed, the new lchan should reflect the actually used chan_mode_rate.<br />Hence, a failed activation/modify will neither change the previous lchan nor the new lchan,<br />and after a successful activation/modify, subsequent re-assignment/modify procedures know an lchan's current mode and rate accurately.</p>
<p>(struct channel_mode_and_rate includes the S15-S0 bits)</p> OsmoBSC - Bug #3833: Fix storage location of AMR S15-S0 bitshttps://osmocom.org/issues/3833?journal_id=220282021-04-28T14:51:11Zneelsnhofmeyr@sysmocom.de
<ul></ul><p>neels wrote:</p>
<blockquote>
<p>cleaning up (part of?) this in the process of <a class="issue tracker-1 status-3 priority-1 priority-lowest closed" title="Bug: osmo-bsc doesn't perform RSL MODE MODIFY on ASSIGNMENT if channel is compatible. (Resolved)" href="https://osmocom.org/issues/3357">#3357</a></p>
</blockquote>
<p>That should have said <a class="issue tracker-2 status-3 priority-3 priority-high3 closed" title="Feature: handover: for intra-cell re-assignment, use Assignment procedure, not Handover (Resolved)" href="https://osmocom.org/issues/3277">#3277</a></p> OsmoBSC - Bug #3833: Fix storage location of AMR S15-S0 bitshttps://osmocom.org/issues/3833?journal_id=257892022-12-13T14:28:05Zfixeria
<ul><li><strong>Priority</strong> changed from <i>Low</i> to <i>High</i></li></ul><p>A year in progress? This is potentially affecting one of our customers provoking a crash of osmo-bsc. Increasing priority.</p> OsmoBSC - Bug #3833: Fix storage location of AMR S15-S0 bitshttps://osmocom.org/issues/3833?journal_id=258002022-12-14T02:37:02Zneelsnhofmeyr@sysmocom.de
<ul><li><strong>Status</strong> changed from <i>In Progress</i> to <i>Resolved</i></li><li><strong>% Done</strong> changed from <i>0</i> to <i>100</i></li></ul><p>Related: <a class="external" href="https://gerrit.osmocom.org/c/osmo-bsc/+/30583">https://gerrit.osmocom.org/c/osmo-bsc/+/30583</a></p>
<p>Solved by: Ie0da36124d73efc28a8809b63d7c96e2167fc412 <a class="external" href="https://gerrit.osmocom.org/c/osmo-bsc/+/24352">https://gerrit.osmocom.org/c/osmo-bsc/+/24352</a><br />more than a year ago. I forgot to update this issue.</p>