<p style="white-space: pre-wrap; word-wrap: break-word;">After assigning #4070 to myself, Harald pointed me at this WIP patch. I've fixed up the cosmetics and added a no-VTY command as suggested in fixeria's review.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Set to -2, because it's segfaulting for me when testing with real hardware. I'll look into it more tomorrow.</p><p>Patch set 8:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -2</span></p><p><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743">View Change</a></p><p>10 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/e1_input_vty.c">File src/e1_input_vty.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-abis/+/14743/7/src/e1_input_vty.c@175">Patch Set #7, Line 175:</a> <code style="font-family:monospace,monospace">Enable</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">If there is a way to enable this feature, shouldn't there be a way to disable it?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Line below says "0 disables keepalive". But this is not consistent with "e1_line <0-255> keepalive <1-300> ..." (zero is not allowed!), which has "no e1_line <0-255> keepalive". (Also there is no !interval code below that actually disables it).</p><p style="white-space: pre-wrap; word-wrap: break-word;">I've changed it to match "e1_line <1-255> keepalive".</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/e1_input_vty.c@193">Patch Set #7, Line 193:</a> <code style="font-family:monospace,monospace">      if (!line->driver->has_keepalive) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This check is redundant, due to the check above only the "ipa" driver is allowed. It would not make sense to disable has_keepalive for ipa. Removed.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/e1_input_vty.c@195">Patch Set #7, Line 195:</a> <code style="font-family:monospace,monospace">line->driver->name, VTY_NEWLINE);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Cosmetic: alignment.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/e1_input_vty.c@282">Patch Set #7, Line 282:</a> <code style="font-family:monospace,monospace">                                        line->ipa_kap->interval, line->ipa_kap->wait_for_resp,</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Cosmetic: alignment.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c">File src/input/ipaccess.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-abis/+/14743/7/src/input/ipaccess.c@65">Patch Set #7, Line 65:</a> <code style="font-family:monospace,monospace">int</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">bool?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Changing to bool makes sense to me. However, this patch only moves the struct around, changing from int -> bool is a separate change. So I've added a follow-up patch.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c@65">Patch Set #7, Line 65:</a> <code style="font-family:monospace,monospace">   </code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Please use tabs.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c@103">Patch Set #7, Line 103:</a> <code style="font-family:monospace,monospace"> {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">coding style</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c@105">Patch Set #7, Line 105:</a> <code style="font-family:monospace,monospace">send</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Cosmetic (not a merge blocker): send() without flags equals to write().</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c@109">Patch Set #7, Line 109:</a> <code style="font-family:monospace,monospace"> {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">coding style</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Done</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743/7/src/input/ipaccess.c@120">Patch Set #7, Line 120:</a> <code style="font-family:monospace,monospace">    ka_fsm_for_ts(il, bfd) = NULL;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">It segfaults here when testing with hardware, I'm looking into it.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmo-abis/+/14743">change 14743</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-abis/+/14743"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmo-abis </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: I30e3bd601e55355aaf738ee2f2c44c1ec2c46c6a </div>
<div style="display:none"> Gerrit-Change-Number: 14743 </div>
<div style="display:none"> Gerrit-PatchSet: 8 </div>
<div style="display:none"> Gerrit-Owner: Hoernchen <ewild@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Hoernchen <ewild@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-CC: laforge <laforge@osmocom.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 21 Jan 2020 14:57:15 +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: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-MessageType: comment </div>