falconia has submitted this change. ( 
https://gerrit.osmocom.org/c/osmo-bts/+/42167?usp=email )

Change subject: RTP: add vty option for ortp vs twrtp selection
......................................................................

RTP: add vty option for ortp vs twrtp selection

With this patch it finally becomes possible for the user to select
which RTP library should be used: ortp or twrtp.  ortp is still
the default for now, in order to not alter behavior for existing
installations until twrtp receives more real world testing by users
beyond the original author.  Future patches may change the default
or even remove ortp support altogether - but twrtp should receive
more testing as a user opt-in feature first.

Related: OS#6474
Change-Id: Iff4e3a399250c16ba8fe4cb12e4e22f4c6b346ec
---
M include/osmo-bts/bts.h
M src/common/bts.c
M src/common/lchan.c
M src/common/vty.c
M tests/osmo-bts.vty
5 files changed, 74 insertions(+), 1 deletion(-)

Approvals:
  pespin: Looks good to me, but someone else must approve
  Jenkins Builder: Verified
  fixeria: Looks good to me, approved




diff --git a/include/osmo-bts/bts.h b/include/osmo-bts/bts.h
index ec13d71..8bd3a83 100644
--- a/include/osmo-bts/bts.h
+++ b/include/osmo-bts/bts.h
@@ -319,6 +319,7 @@
        struct llist_head bsc_oml_hosts;
        unsigned int rtp_jitter_buf_ms;
        bool rtp_jitter_adaptive;
+       bool use_twrtp;
        struct osmo_twjit_config *twjit_cfg;

        uint16_t rtp_port_range_start;
diff --git a/src/common/bts.c b/src/common/bts.c
index 78ea417..11f57a7 100644
--- a/src/common/bts.c
+++ b/src/common/bts.c
@@ -347,6 +347,23 @@
        bts->rtp_priority = -1;
        bts->emit_hr_rfc5993 = true;

+       /* At the present point in OsmoBTS evolution the user can select
+        * which RTP library should be used: Belledonne ortp (legacy) or
+        * Themyscira twrtp (integrated into libosmo-netif).  There is
+        * a desire to eventually deprecate ortp altogether and support
+        * only Osmocom-native twrtp, but because the two implementations
+        * take drastically different approaches to the hard problem of
+        * converting from an incoming RTP stream to fixed timing for
+        * GSM TCH Tx, the transition should involve extensive testing
+        * by users of the software (GSM network operators), as opposed
+        * to being imposed by developers as a flag day change.
+        * The current default is to use ortp, in order to avoid any
+        * surprise changes in behaviour.  It is expected that this
+        * default will change at some point in the future, prior to
+        * full discontinuation of support for ortp.
+        */
+       bts->use_twrtp = false;
+
        /* Default (fall-back) MS/BS Power control parameters */
        power_ctrl_params_def_reset(&bts->bs_dpc_params, true);
        power_ctrl_params_def_reset(&bts->ms_dpc_params, false);
diff --git a/src/common/lchan.c b/src/common/lchan.c
index 1e8b501..1fdb3f1 100644
--- a/src/common/lchan.c
+++ b/src/common/lchan.c
@@ -587,7 +587,7 @@
        //if (!payload_type)
        lchan->tch.last_fn = LCHAN_FN_DUMMY;
        lchan->abis_ip.rtp_socket = rtp_abst_socket_create(lchan->ts->trx,
-                                                          false,
+                                                          bts->use_twrtp,
                                                           bts->twjit_cfg);

        if (!lchan->abis_ip.rtp_socket) {
diff --git a/src/common/vty.c b/src/common/vty.c
index 0411060..af381b5 100644
--- a/src/common/vty.c
+++ b/src/common/vty.c
@@ -424,6 +424,16 @@
                vty_out(vty, " rtp ip-dscp %d%s", bts->rtp_ip_dscp, 
VTY_NEWLINE);
        if (bts->rtp_priority != -1)
                vty_out(vty, " rtp socket-priority %d%s", bts->rtp_priority, 
VTY_NEWLINE);
+       /* We write out "rtp library" setting only when it differs from the
+        * default.  This policy is necessary in order to make the new default
+        * take effect for 'indifferent' users when we change it - and finally,
+        * this vty setting will disappear altogether when we eventually drop
+        * ortp support and make twrtp mandatory.
+        * In the meantime, however, extra attention is required to keep
+        * the following code in sync with changes in the default!
+        */
+       if (bts->use_twrtp)
+               vty_out(vty, " rtp library twrtp%s", VTY_NEWLINE);
        if (bts->rtp_nogaps_mode)
                vty_out(vty, " rtp continuous-streaming%s", VTY_NEWLINE);
        vty_out(vty, " %srtp internal-uplink-ecu%s",
@@ -797,6 +807,19 @@
        return CMD_SUCCESS;
 }

+DEFUN_ATTR(cfg_bts_rtp_library,
+          cfg_bts_rtp_library_cmd,
+          "rtp library (ortp|twrtp)",
+          RTP_STR "RTP library selection\n"
+          "Belledonne ortp\n" "Themyscira twrtp\n",
+          BTS_VTY_ATTR_NEW_LCHAN)
+{
+       struct gsm_bts *bts = vty->index;
+
+       bts->use_twrtp = !strcmp(argv[0], "twrtp");
+       return CMD_SUCCESS;
+}
+
 DEFUN(cfg_bts_rtp_cont_stream,
       cfg_bts_rtp_cont_stream_cmd,
       "rtp continuous-streaming",
@@ -1482,6 +1505,33 @@
        return CMD_SUCCESS;
 }

+/* "show running-config" cannot reliably indicate which RTP library is
+ * selected because we have to omit "rtp library" setting when it matches
+ * the default, and that default is expected to change as we progress
+ * toward eventual removal of ortp.  This additional show command
+ * allows an operator to see unambiguously which RTP library is in use.
+ */
+DEFUN(show_bts_rtp, show_bts_rtp_cmd,
+      "show bts <0-255> rtp",
+      SHOW_STR "Display information about a BTS\n"
+      BTS_NR_STR "RTP library selection\n")
+{
+       const struct gsm_bts *bts;
+
+       bts = gsm_bts_num(g_bts_sm, atoi(argv[0]));
+       if (bts == NULL) {
+               vty_out(vty, "%% can't find BTS '%s'%s",
+                       argv[0], VTY_NEWLINE);
+               return CMD_WARNING;
+       }
+
+       vty_out(vty, "RTP library: %s%s",
+               bts->use_twrtp ? "Themyscira twrtp" : "Belledonne ortp",
+               VTY_NEWLINE);
+
+       return CMD_SUCCESS;
+}
+
 DEFUN(test_send_failure_event_report, test_send_failure_event_report_cmd, 
"test send-failure-event-report <0-255>",
       "Various testing commands\n"
       "Send a test OML failure event report to the BSC\n" BTS_NR_STR)
@@ -2761,6 +2811,7 @@
        install_element_ve(&show_lchan_cmd);
        install_element_ve(&show_lchan_summary_cmd);
        install_element_ve(&show_bts_gprs_cmd);
+       install_element_ve(&show_bts_rtp_cmd);

        install_element_ve(&logging_fltr_l1_sapi_cmd);
        install_element_ve(&no_logging_fltr_l1_sapi_cmd);
@@ -2779,6 +2830,7 @@
        install_element(BTS_NODE, &cfg_bts_rtp_port_range_cmd);
        install_element(BTS_NODE, &cfg_bts_rtp_ip_dscp_cmd);
        install_element(BTS_NODE, &cfg_bts_rtp_priority_cmd);
+       install_element(BTS_NODE, &cfg_bts_rtp_library_cmd);
        install_element(BTS_NODE, &cfg_bts_rtp_cont_stream_cmd);
        install_element(BTS_NODE, &cfg_bts_no_rtp_cont_stream_cmd);
        install_element(BTS_NODE, &cfg_bts_rtp_int_ul_ecu_cmd);
diff --git a/tests/osmo-bts.vty b/tests/osmo-bts.vty
index 94f1fed..f6b1d2e 100644
--- a/tests/osmo-bts.vty
+++ b/tests/osmo-bts.vty
@@ -33,6 +33,7 @@
   <0-255>    BTS Number
 OsmoBTS> show bts 0 ?
   gprs  GPRS/EGPRS configuration
+  rtp   RTP library selection
   <cr>
 OsmoBTS> show trx ?
   [<0-255>]  BTS Number
@@ -134,6 +135,7 @@
   <0-255>    BTS Number
 OsmoBTS# show bts 0 ?
   gprs  GPRS/EGPRS configuration
+  rtp   RTP library selection
   <cr>
 OsmoBTS# show trx ?
   [<0-255>]  BTS Number
@@ -233,6 +235,7 @@
   rtp port-range <1-65534> <1-65534>
   rtp ip-dscp <0-63>
   rtp socket-priority <0-255>
+  rtp library (ortp|twrtp)
   rtp continuous-streaming
   no rtp continuous-streaming
   rtp internal-uplink-ecu

--
To view, visit https://gerrit.osmocom.org/c/osmo-bts/+/42167?usp=email
To unsubscribe, or for help writing mail filters, visit 
https://gerrit.osmocom.org/settings?usp=email

Gerrit-MessageType: merged
Gerrit-Project: osmo-bts
Gerrit-Branch: master
Gerrit-Change-Id: Iff4e3a399250c16ba8fe4cb12e4e22f4c6b346ec
Gerrit-Change-Number: 42167
Gerrit-PatchSet: 6
Gerrit-Owner: falconia <[email protected]>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: falconia <[email protected]>
Gerrit-Reviewer: fixeria <[email protected]>
Gerrit-Reviewer: laforge <[email protected]>
Gerrit-Reviewer: pespin <[email protected]>

Reply via email to