<p><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/15885">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/15885/2/src/xua_asp_fsm.c">File src/xua_asp_fsm.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/15885/2/src/xua_asp_fsm.c@459">Patch Set #2, Line 459:</a> <code style="font-family:monospace,monospace">                            break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This one should also be a return?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/15885/2/src/xua_asp_fsm.c@468">Patch Set #2, Line 468:</a> <code style="font-family:monospace,monospace">                                     break;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This one should be a return, otherwise you send an error and later you send an ACK. But it's not really related to this commit, the error was already there as far as I can see.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/15885/2/src/xua_asp_fsm.c@473">Patch Set #2, Line 473:</a> <code style="font-family:monospace,monospace">                                  if (!osmo_ss7_tmode_compatible_xua(as->cfg.mode, traf_mode)) {</code></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">I'd rather have an API passing AS as the first parameter. Because from what I understand from RFC4666, if the mode of the AS is not set by local config, then the one requested by ASPAC is used:<br>"""<br>  If the SGP determines that the mode indicated in an ASP<br>   Active message is unsupported or incompatible with the mode currently<br>   configured for the AS, the SGP responds with an Error message<br>   ("Unsupported / Invalid Traffic Handling Mode").  If the traffic<br>   handling mode of the Application Server is not already known via<br>   configuration data, then the traffic handling mode indicated in the<br>   first ASP Active message causing the transition of the Application<br>   Server state to AS-ACTIVE MAY be used to set the mode.<br>"""</pre><p style="white-space: pre-wrap; word-wrap: break-word;">So basically I think there should be a check like I do in other places:<br>osmo_as_tmode_compatible_xua(as, traf_mode) {<br>if (as->cfg.mode_set_by_vty) { ...then check if as->cfg.mode is compatible with tmode... } else { return true }<br>}</p><p style="white-space: pre-wrap; word-wrap: break-word;">Actually, the best would be adding a new flag as->cfg.mode_set_by_req set to true in the next file where as->cfg.mode is set from the request, and in here we should do the following check:<br>if (as->cfg.mode_set_by_vty || as->cfg.mode_set_by_req) { ...then check if as->cfg.mode is compatible with tmode... } else { return true }</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/15885/2/src/xua_rkm.c">File src/xua_rkm.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/15885/2/src/xua_rkm.c@221">Patch Set #2, Line 221:</a> <code style="font-family:monospace,monospace">                  as->cfg.mode = osmo_ss7_tmode_from_xua(_tmode);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">here: as->cfg.mode_set_by_req = true;</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmo-sccp/+/15885">change 15885</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/libosmo-sccp/+/15885"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmo-sccp </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ic73410fbc88d50710202453f759fa132ce14db4c </div>
<div style="display:none"> Gerrit-Change-Number: 15885 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-CC: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 29 Oct 2019 10:38:41 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>