<p style="white-space: pre-wrap; word-wrap: break-word;">thanks</p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm still not convinced about mdcx:unavailable and dlcx:unavailable, as in inline comment. Otherwise letting my bikesheds rest.</p><p>Patch set 5:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #d4ffd4;">Code-Review +1</span></p><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18898">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/osmo-mgw/+/18898/4/src/libosmo-mgcp/mgcp_endp.c">File src/libosmo-mgcp/mgcp_endp.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/osmo-mgw/+/18898/4/src/libosmo-mgcp/mgcp_endp.c@468">Patch Set #4, Line 468:</a> <code style="font-family:monospace,monospace">}</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I thought about doing that while I was doing the implementation, but I prefer to have a single funct […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I still disagree but leaving it up to you</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18898/4/src/libosmo-mgcp/mgcp_endp.c@592">Patch Set #4, Line 592:</a> <code style="font-family:monospace,monospace">                * in a properly configured environment!) */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">If this happens its not a problem at the mgw side. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">ok, so the comment means it is the peer program that is not properly configured?<br>The comment sounds like it would be osmo-mgw that is not properly configured, that's why I'm asking about this.<br>(Any program should always obviously guard against peers sending total nonsense...)</p><p style="white-space: pre-wrap; word-wrap: break-word;">it's just a comment so just letting it be</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18898/5/src/libosmo-mgcp/mgcp_endp.c">File src/libosmo-mgcp/mgcp_endp.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/osmo-mgw/+/18898/5/src/libosmo-mgcp/mgcp_endp.c@600">Patch Set #5, Line 600:</a> <code style="font-family:monospace,monospace">bool mgcp_endp_avail(struct mgcp_endpoint *endp)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">IMHO "available" is the wrong word, but leaving it up to you</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18898/4/src/libosmo-mgcp/mgcp_ratectr.c">File src/libosmo-mgcp/mgcp_ratectr.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/osmo-mgw/+/18898/4/src/libosmo-mgcp/mgcp_ratectr.c@94">Patch Set #4, Line 94:</a> <code style="font-family:monospace,monospace"> [MGCP_MDCX_FAIL_AVAIL] = { "mdcx:availability", "endpoint unavailable." },</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">It does not matter if MDCX, CRCX etc would succeed or not, this is checked much later. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I was more thinking in this line: if we receive an MDCX for an endpoint for which we haven't previously seen a CRCX.<br>If there is an MDCX before a CRCX, that is a protocol error (like as if we sent "GARBAGE") and osmo-mgw doesn't even need to check availability.</p><p style="white-space: pre-wrap; word-wrap: break-word;">When the protocol is followed correctly: if an E1 endpoint is blocked, then the first CRCX on it should hit crcx:unavailable above. After that CRCX was successful, there is no situation where an MDCX on the same would hit mdcx:unavailable.</p><p style="white-space: pre-wrap; word-wrap: break-word;">So I doubt that we would ever see this happening.<br>Do you still think counting MDCX and DLCX on blocked endpoints makes sense?<br>I still have my doubts that the MDCX and DLCX counters are useful.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-mgw/+/18898">change 18898</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-mgw/+/18898"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-mgw </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I18e90b10648a7e504371179ad144645fc82e1c27 </div>
<div style="display:none"> Gerrit-Change-Number: 18898 </div>
<div style="display:none"> Gerrit-PatchSet: 5 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 29 Jun 2020 15:54:06 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Comment-In-Reply-To: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Comment-In-Reply-To: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>