<p style="white-space: pre-wrap; word-wrap: break-word;">Haven't managed to read everything yet. so far...</p><p>Patch set 29:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p><a href="https://gerrit.osmocom.org/11642">View Change</a></p><p>35 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/include/osmocom/msc/ran_conn.h">File include/osmocom/msc/ran_conn.h:</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/11642/29/include/osmocom/msc/ran_conn.h@216">Patch Set #29, Line 216:</a> <code style="font-family:monospace,monospace">RAN_CONN_USE_SGs_SERVICE</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Missing comma.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">(yes, it is nicer to keep the last comma, so later additions are single-line patches)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/include/osmocom/msc/sgs_iface.h">File include/osmocom/msc/sgs_iface.h:</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/11642/29/include/osmocom/msc/sgs_iface.h@29">Patch Set #29, Line 29:</a> <code style="font-family:monospace,monospace">  char *sockname;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Here, I guess talloc ownership is clear and straightforward, so it would be ok to keep this as-is,<br>but generally I'm in favor of char[123] storage (see osmo_sock_get_name_buf()).<br>(I now see it as a mistake that I implemented osmo_sock_get_name() as a talloc string.)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/include/osmocom/msc/sgs_server.h">File include/osmocom/msc/sgs_server.h:</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/11642/29/include/osmocom/msc/sgs_server.h@1">Patch Set #29, Line 1:</a> <code style="font-family:monospace,monospace">/* (C 2018 by Harald Welte <laforge@gnumonks.org></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">closing brace (C), and I guess it should contain 2019</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/include/osmocom/msc/sgs_server.h@23">Patch Set #29, Line 23:</a> <code style="font-family:monospace,monospace">#define DEFAULT_VLR_NAME "vlr.example.net"</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Maybe this name could be "*_SGS_SERVER_*"?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/include/osmocom/msc/sgs_vty.h">File include/osmocom/msc/sgs_vty.h:</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/11642/29/include/osmocom/msc/sgs_vty.h@1">Patch Set #29, Line 1:</a> <code style="font-family:monospace,monospace">/* (C 2018 by Harald Welte <laforge@gnumonks.org></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think normally we don't have (C) in .h files?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/include/osmocom/msc/vlr.h">File include/osmocom/msc/vlr.h:</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/11642/29/include/osmocom/msc/vlr.h@114">Patch Set #29, Line 114:</a> <code style="font-family:monospace,monospace">struct vlr_subscr;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Why do we need this? I don't see any references to this struct.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">the struct is defined 7 lines further down; indeed drop this</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/gsm_04_08.c">File src/libmsc/gsm_04_08.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/11642/29/src/libmsc/gsm_04_08.c@453">Patch Set #29, Line 453:</a> <code style="font-family:monospace,monospace">struct msgb *gsm48_create_mm_info(struct gsm_network *net)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">In principle, we put the bare msg composition stuff in libosmocore.<br>(It would be some effort to pull this apart from the osmo-msc local code base, though... so I'm not going to block on it)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/gsm_subscriber.c">File src/libmsc/gsm_subscriber.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/11642/29/src/libmsc/gsm_subscriber.c@119">Patch Set #29, Line 119:</a> <code style="font-family:monospace,monospace">Note</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I think this note should precede the function definition.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Agree, return val info should go to the "API doc" string.<br>In general, I'd prefer to not start comments with "Note:".</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/gsm_subscriber.c@130">Patch Set #29, Line 130:</a> <code style="font-family:monospace,monospace"> case OSMO_RAT_EUTRAN_SGS:</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">meta question:</p><p style="white-space: pre-wrap; word-wrap: break-word;">IIUC on the EUTRAN, the subscriber does not actually send a LU request to the 2G MSC.<br>When a call gets established or SMS handling takes place, the SGs sends something, and then the MS is implicitly set to "attached".</p><p style="white-space: pre-wrap; word-wrap: break-word;">So if an MS is attached to some LTE service, and if now the 2G MSC wants to send it an SMS or call it, that can only work if the MS has before used some 2G fallback service?</p><p style="white-space: pre-wrap; word-wrap: break-word;">Shouldn't all paging already be managed by the LTE side, and the 2G MSC isn't even responsible for paging, and only take over from the LTE once instructed over SGs?</p><p style="white-space: pre-wrap; word-wrap: break-word;">So, it doesn't make sense to me that an LTE subscriber is even listed as 'attached' in our VLR beyond an ongoing SGs transaction. Also, we will not get periodic location updates, and the VLR will remove the subscriber as soon as that period has passed; after that, we will again reject all paging.</p><p style="white-space: pre-wrap; word-wrap: break-word;">How does all this work?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/gsm_subscriber.c@151">Patch Set #29, Line 151:</a> <code style="font-family:monospace,monospace"> * \param serv_ind  sgsap service indicator (in case SGs interface is used to page)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">end with dot</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/msc_vty.c">File src/libmsc/msc_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/11642/29/src/libmsc/msc_vty.c@705">Patch Set #29, Line 705:</a> <code style="font-family:monospace,monospace">  /* SGs related */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">maybe</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  if (vsub->sgs_fsm && vsub->sgs_fsm->state != SGS_UE_ST_NULL) {<br>   ...</pre><p style="white-space: pre-wrap; word-wrap: break-word;">?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c">File src/libmsc/sgs_iface.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/11642/29/src/libmsc/sgs_iface.c@3">Patch Set #29, Line 3:</a> <code style="font-family:monospace,monospace">/* (C 2018 by Harald Welte <laforge@gnumonks.org></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">) 19</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@98">Patch Set #29, Line 98:</a> <code style="font-family:monospace,monospace">     return vsub;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is a bad thing to do in general.<br>You should keep the use count until done with the vlr subscr.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@110">Patch Set #29, Line 110:</a> <code style="font-family:monospace,monospace">          LOGSGC(sgc, LOGL_ERROR, "(sub %s) Cannot allocate connection, subscriber not found in HLR!\n", imsi);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">this is not the HLR ... the vlr_subscr_find_by_imsi() does not contact the HLR.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I am wondering, IUUC the SGs implicitly sets a subscriber as attached. But we might want to first verify that our HLR even knows that IMSI, i.e. actually run a Location Updating Request procedure over GSUP? Later code does look like the SGs is doing that, but this here looks like it doesn't.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@121">Patch Set #29, Line 121:</a> <code style="font-family:monospace,monospace"> conn->vsub = vlr_subscr_get(vsub);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">instead of another get() here, keep the get() use count from vlr_subscr_find_by_imsi().<br>In above 'if (!conn)' err handling do a vlr_subscr_put().</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@277">Patch Set #29, Line 277:</a> <code style="font-family:monospace,monospace">   vsub = vlr_subscr_by_imsi(gsm_network->vlr, imsi);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">In another VTY related patch I did argue that what you're doing here is safe.<br>But in that patch I explicitly wanted the subscriber's use count to be printed without the temporary get().</p><p style="white-space: pre-wrap; word-wrap: break-word;">Here please avoid handling a vsub outside of a get()..put() scope.<br>I know it gets a bit more cumbersome, but for code maintainability it's worth it.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@329">Patch Set #29, Line 329:</a> <code style="font-family:monospace,monospace">       if (imsi) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">this is not so nice, since you're obtaining log context info, even though you don't know whether something gets logged yet. We usually omit all string composition when the logging category is switched off, within the LOGFOO() macros.</p><p style="white-space: pre-wrap; word-wrap: break-word;">The pattern I recently use a lot is: at the beginning, when allocating an FSM instance, place all relevant context information into the FSM instance id; then use LOGPFSML everywhere. That saves the effort of obtaining the log context over and over. Is something similar possible here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@348">Patch Set #29, Line 348:</a> <code style="font-family:monospace,monospace">       else</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">you are *always* logging an error now. I guess the initial 'if (sgsap_iei < 0)' should have a { .. } enclosing all the other error logging? Also maybe that {..} should end in 'return -EINVAL'?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@354">Patch Set #29, Line 354:</a> <code style="font-family:monospace,monospace">   return 0;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">[after reading other code] ...the "status" is intended to always indicate an error on the SGs? Then above would even be ok.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@414">Patch Set #29, Line 414:</a> <code style="font-family:monospace,monospace">    msg = gsm29118_create_mm_info_req(vsub->imsi, msg_mm_info->data + 2, msg_mm_info->len - 2);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(when we're sending MM Info data, is that really a "Request"?)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@454">Patch Set #29, Line 454:</a> <code style="font-family:monospace,monospace">   * we are supposed to start a search procedure as defined in 3GPP TS 23.018 */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Of course our VLR does not even have a VLR subscriber stored if it is not attached. As in my other comment, I doubt that this is useful as it is now.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@851">Patch Set #29, Line 851:</a> <code style="font-family:monospace,monospace">               LOGSGC(sgc, LOGL_NOTICE, "SGsAP Message %s parsing error\n", sgsap_msg_type_name(msg_type));</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">LOGL_ERROR? Also below?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@865">Patch Set #29, Line 865:</a> <code style="font-family:monospace,monospace">             msgb_free(msg);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">you are often repeating the pattern "create_status, tx, free". That would qualify for a little static function to avoid code dup. You could even create something like an 'sgs_fail()' function or macro that also takes a log string, hence do all of error-logging and status tx in one function/macro invocation.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@871">Patch Set #29, Line 871:</a> <code style="font-family:monospace,monospace">             /* FIXME: Is there a well defined minimum length (MCC=3 + MNC=2 + MSIN=1 => 6?) */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">GSM23003_IMSI_MIN_DIGITS</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@872">Patch Set #29, Line 872:</a> <code style="font-family:monospace,monospace">             if (imsi[0] == '\0' || strlen(imsi) <= 6) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(strlen() is enough, it implies a "[0] == '\0' check")</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@984">Patch Set #29, Line 984:</a> <code style="font-family:monospace,monospace">            sgs_tx(sgc, resp);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">others free() after sgs_tx(). One or the other is wrong.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@987">Patch Set #29, Line 987:</a> <code style="font-family:monospace,monospace">                OSMO_ASSERT(false);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">...</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_iface.c@1083">Patch Set #29, Line 1083:</a> <code style="font-family:monospace,monospace">                  LOGMME(mme, LOGL_ERROR, "Ts11 expired more than Ns11 times, giving up\n");</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(would be nice to also log the Ns11 value)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_server.c">File src/libmsc/sgs_server.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/11642/29/src/libmsc/sgs_server.c@1">Patch Set #29, Line 1:</a> <code style="font-family:monospace,monospace">/* (C 2018 by Harald Welte <laforge@gnumonks.org></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">) 19</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_vty.c">File src/libmsc/sgs_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/11642/29/src/libmsc/sgs_vty.c@1">Patch Set #29, Line 1:</a> <code style="font-family:monospace,monospace">/* (C 2018 by Harald Welte <laforge@gnumonks.org></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">) 19</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_vty.c@99">Patch Set #29, Line 99:</a> <code style="font-family:monospace,monospace">      "timer (Ts5|TS6-2|Ts7|Ts11|Ts14|Ts15) <1-120>",</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">have you tested that the keywords are allowed to contain capital letters?<br>Normally for VTY all-caps means that it is a free-form arg.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_vty.c@123">Patch Set #29, Line 123:</a> <code style="font-family:monospace,monospace">      "counter (Ns7|Ns11) <0-255>",</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">same capital chars: tested?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libmsc/sgs_vty.c@142">Patch Set #29, Line 142:</a> <code style="font-family:monospace,monospace">      "show sgs-connections", SHOW_STR</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(prefer line break before SHOW_STR)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libvlr/Makefile.am">File src/libvlr/Makefile.am:</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/11642/29/src/libvlr/Makefile.am@11">Patch Set #29, Line 11:</a> <code style="font-family:monospace,monospace">     $(LIBOSMOGSM_CFLAGS) \</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">interesting, why is this needed?</p><p style="white-space: pre-wrap; word-wrap: break-word;">So far the idea of the VLR is to be separate from GSM specifics. It calls vlr->ops.foo callback functions to request actions, and it receives readily parsed function arguments. So far all message encoding and decoding happens in the MSC or the GSUP client, not in libvlr.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11642/29/src/libvlr/vlr.c">File src/libvlr/vlr.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/11642/29/src/libvlr/vlr.c@838">Patch Set #29, Line 838:</a> <code style="font-family:monospace,monospace">         sgs_lu_in_progress = true;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">this again looks like you are indeed invoking a Location Updating towards the HLR? Earlier the patch looked like that was not the case. Which is it / should it be?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/11642">change 11642</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/11642"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-msc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I73359925fc1ca72b33a1466e6ac41307f2f0b11d </div>
<div style="display:none"> Gerrit-Change-Number: 11642 </div>
<div style="display:none"> Gerrit-PatchSet: 29 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: Stefan Sperling <stsp@stsp.name> </div>
<div style="display:none"> Gerrit-CC: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 08 Jan 2019 16:30:43 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>