Change in libosmocore[master]: gsmtap: Solve TCH / FACCH confusion once and for all

This is merely a historical archive of years 2008-2021, before the migration to mailman3.

A maintained and still updated list archive can be found at https://lists.osmocom.org/hyperkitty/list/gerrit-log@lists.osmocom.org/.

laforge gerrit-no-reply at lists.osmocom.org
Sun Mar 8 16:33:03 UTC 2020 

laforge has submitted this change. ( https://gerrit.osmocom.org/c/libosmocore/+/17426 )

Change subject: gsmtap: Solve TCH / FACCH confusion once and for all
......................................................................

gsmtap: Solve TCH / FACCH confusion once and for all

* What we used to call TCH/F and TCH/H in gsmtap are actually only
  FACCH/F and FACCH/H, i.e. the signaling part of Bm/Lm channels

* Give them proper names with backwards compatibility #define

* Split VOICE into VOICE_F and VOICE_H.  If we don't differentiate this,
  a receiver is not able to determine the RSL channel ID of a frame
  without looking at external state/context.  That in turn has been a
  design feature of GSMTAP Um format so far, and programs like
  osmo-bts-virtual rely on it.

Change-Id: I952044a17334f35712e087dc41781805000aebc1
Related: OS#2557
---
M include/osmocom/core/gsmtap.h
M include/osmocom/core/gsmtap_util.h
M src/gsmtap_util.c
3 files changed, 40 insertions(+), 13 deletions(-)

Approvals:
  Jenkins Builder: Verified
  laforge: Looks good to me, approved



diff --git a/include/osmocom/core/gsmtap.h b/include/osmocom/core/gsmtap.h
index 87c18e3..82e9525 100644
--- a/include/osmocom/core/gsmtap.h
+++ b/include/osmocom/core/gsmtap.h
@@ -82,8 +82,8 @@
 #define GSMTAP_CHANNEL_SDCCH	0x06
 #define GSMTAP_CHANNEL_SDCCH4	0x07
 #define GSMTAP_CHANNEL_SDCCH8	0x08
-#define GSMTAP_CHANNEL_TCH_F	0x09	/* Actually, it's FACCH/F (signaling) */
-#define GSMTAP_CHANNEL_TCH_H	0x0a	/* Actually, it's FACCH/H (signaling) */
+#define GSMTAP_CHANNEL_FACCH_F	0x09	/* Actually, it's FACCH/F (signaling) */
+#define GSMTAP_CHANNEL_FACCH_H	0x0a	/* Actually, it's FACCH/H (signaling) */
 #define GSMTAP_CHANNEL_PACCH	0x0b
 #define GSMTAP_CHANNEL_CBCH52	0x0c
 #define GSMTAP_CHANNEL_PDTCH	0x0d
@@ -91,7 +91,10 @@
 #define GSMTAP_CHANNEL_PDCH	GSMTAP_CHANNEL_PDTCH
 #define GSMTAP_CHANNEL_PTCCH	0x0e
 #define GSMTAP_CHANNEL_CBCH51	0x0f
-#define GSMTAP_CHANNEL_VOICE	0x10	/* voice codec payload (HR/FR/EFR/AMR) */
+#define GSMTAP_CHANNEL_VOICE_F	0x10	/* voice codec payload (FR/EFR/AMR) */
+#define GSMTAP_CHANNEL_VOICE_H	0x11	/* voice codec payload (HR/AMR) */
+#define GSMTAP_CHANNEL_TCH_F	GSMTAP_CHANNEL_FACCH_F	/* We used the wrong naming in 2008 when we were young */
+#define GSMTAP_CHANNEL_TCH_H	GSMTAP_CHANNEL_FACCH_H	/* We used the wrong naming in 2008 when we were young */
 
 /* GPRS Coding Scheme CS1..4 */
 #define GSMTAP_GPRS_CS_BASE	0x20
diff --git a/include/osmocom/core/gsmtap_util.h b/include/osmocom/core/gsmtap_util.h
index f8a12a6..d583401 100644
--- a/include/osmocom/core/gsmtap_util.h
+++ b/include/osmocom/core/gsmtap_util.h
@@ -8,7 +8,11 @@
  *  @{
  * \file gsmtap_util.h */
 
+uint8_t chantype_rsl2gsmtap2(uint8_t rsl_chantype, uint8_t rsl_link_id, bool user_plane);
+
 uint8_t chantype_rsl2gsmtap(uint8_t rsl_chantype, uint8_t rsl_link_id);
+	OSMO_DEPRECATED("Use chantype_rsl2gsmtap2() instead")
+
 void chantype_gsmtap2rsl(uint8_t gsmtap_chantype, uint8_t *rsl_chantype, uint8_t *link_id);
 
 struct msgb *gsmtap_makemsg_ex(uint8_t type, uint16_t arfcn, uint8_t ts, uint8_t chan_type,
diff --git a/src/gsmtap_util.c b/src/gsmtap_util.c
index 40ec4b8..9a0ac02 100644
--- a/src/gsmtap_util.c
+++ b/src/gsmtap_util.c
@@ -54,18 +54,25 @@
 /*! convert RSL channel number to GSMTAP channel type
  *  \param[in] rsl_chantype RSL channel type
  *  \param[in] link_id RSL link identifier
+ *  \param[in] user_plane Is this voice/csd user plane (1) or signaling (0)
  *  \returns GSMTAP channel type
  */
-uint8_t chantype_rsl2gsmtap(uint8_t rsl_chantype, uint8_t link_id)
+uint8_t chantype_rsl2gsmtap2(uint8_t rsl_chantype, uint8_t link_id, bool user_plane)
 {
 	uint8_t ret = GSMTAP_CHANNEL_UNKNOWN;
 
 	switch (rsl_chantype) {
 	case RSL_CHAN_Bm_ACCHs:
-		ret = GSMTAP_CHANNEL_TCH_F;
+		if (user_plane)
+			ret = GSMTAP_CHANNEL_VOICE_F;
+		else
+			ret = GSMTAP_CHANNEL_FACCH_F;
 		break;
 	case RSL_CHAN_Lm_ACCHs:
-		ret = GSMTAP_CHANNEL_TCH_H;
+		if (user_plane)
+			ret = GSMTAP_CHANNEL_VOICE_H;
+		else
+			ret = GSMTAP_CHANNEL_FACCH_H;
 		break;
 	case RSL_CHAN_SDCCH4_ACCH:
 		ret = GSMTAP_CHANNEL_SDCCH4;
@@ -100,6 +107,16 @@
 	return ret;
 }
 
+/*! convert RSL channel number to GSMTAP channel type
+ *  \param[in] rsl_chantype RSL channel type
+ *  \param[in] link_id RSL link identifier
+ *  \returns GSMTAP channel type
+ */
+uint8_t chantype_rsl2gsmtap(uint8_t rsl_chantype, uint8_t link_id)
+{
+	return chantype_rsl2gsmtap2(rsl_chantype, link_id, false);
+}
+
 /*! convert GSMTAP channel type to RSL channel number + Link ID
  *  \param[in] gsmtap_chantype GSMTAP channel type
  *  \param[out] rsl_chantype RSL channel mumber
@@ -109,10 +126,12 @@
                          uint8_t *link_id)
 {
 	switch (gsmtap_chantype & ~GSMTAP_CHANNEL_ACCH & 0xff) {
-	case GSMTAP_CHANNEL_TCH_F: // TCH/F, FACCH/F
+	case GSMTAP_CHANNEL_FACCH_F:
+	case GSMTAP_CHANNEL_VOICE_F: // TCH/F
 		*rsl_chantype = RSL_CHAN_Bm_ACCHs;
 		break;
-	case GSMTAP_CHANNEL_TCH_H: // TCH/H, FACCH/H
+	case GSMTAP_CHANNEL_FACCH_H:
+	case GSMTAP_CHANNEL_VOICE_H: // TCH/H
 		*rsl_chantype = RSL_CHAN_Lm_ACCHs;
 		break;
 	case GSMTAP_CHANNEL_SDCCH4: // SDCCH/4
@@ -467,8 +486,8 @@
 	{ GSMTAP_CHANNEL_SDCCH,		"SDCCH" },
 	{ GSMTAP_CHANNEL_SDCCH4,	"SDCCH/4" },
 	{ GSMTAP_CHANNEL_SDCCH8,	"SDCCH/8" },
-	{ GSMTAP_CHANNEL_TCH_F,		"TCH/F/FACCH/F" },
-	{ GSMTAP_CHANNEL_TCH_H,		"TCH/H/FACCH/H" },
+	{ GSMTAP_CHANNEL_FACCH_F,	"FACCH/F" },
+	{ GSMTAP_CHANNEL_FACCH_H,	"FACCH/H" },
 	{ GSMTAP_CHANNEL_PACCH,		"PACCH" },
 	{ GSMTAP_CHANNEL_CBCH52,	"CBCH" },
 	{ GSMTAP_CHANNEL_PDCH,		"PDCH" } ,
@@ -477,9 +496,10 @@
 	{ GSMTAP_CHANNEL_ACCH | GSMTAP_CHANNEL_SDCCH, "LSACCH" },
 	{ GSMTAP_CHANNEL_ACCH | GSMTAP_CHANNEL_SDCCH4, "SACCH/4" },
 	{ GSMTAP_CHANNEL_ACCH | GSMTAP_CHANNEL_SDCCH8, "SACCH/8" },
-	{ GSMTAP_CHANNEL_ACCH | GSMTAP_CHANNEL_TCH_F, "SACCH/F" },
-	{ GSMTAP_CHANNEL_ACCH | GSMTAP_CHANNEL_TCH_H, "SACCH/H" },
-	{ GSMTAP_CHANNEL_VOICE,		"VOICE" },
+	{ GSMTAP_CHANNEL_ACCH | GSMTAP_CHANNEL_FACCH_F, "SACCH/F" },
+	{ GSMTAP_CHANNEL_ACCH | GSMTAP_CHANNEL_FACCH_H, "SACCH/H" },
+	{ GSMTAP_CHANNEL_VOICE_F,	"TCH/F" },
+	{ GSMTAP_CHANNEL_VOICE_H,	"TCH/H" },
 	{ 0, NULL }
 };
 

-- 
To view, visit https://gerrit.osmocom.org/c/libosmocore/+/17426
To unsubscribe, or for help writing mail filters, visit https://gerrit.osmocom.org/settings

Gerrit-Project: libosmocore
Gerrit-Branch: master
Gerrit-Change-Id: I952044a17334f35712e087dc41781805000aebc1
Gerrit-Change-Number: 17426
Gerrit-PatchSet: 1
Gerrit-Owner: laforge <laforge at osmocom.org>
Gerrit-Reviewer: Jenkins Builder
Gerrit-Reviewer: laforge <laforge at osmocom.org>
Gerrit-MessageType: merged
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.osmocom.org/pipermail/gerrit-log/attachments/20200308/25ff43dd/attachment.htm>

More information about the gerrit-log mailing list