software development of the OsmocomBB Open Source GSM BaseBand stack

[PATCH] add LTE defines to GSMTAP

---
 include/osmocom/core/gsmtap.h | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/include/osmocom/core/gsmtap.h b/include/osmocom/core/gsmtap.h
index c1c53d0..5d4755b 100644
--- a/include/osmocom/core/gsmtap.h
+++ b/include/osmocom/core/gsmtap.h
@@ -43,7 +43,7 @@
 #define GSMTAP_TYPE_GMR1_UM	0x0a	/* GMR-1 L2 packets */
 #define GSMTAP_TYPE_UMTS_RLC_MAC	0x0b
 #define GSMTAP_TYPE_UMTS_RRC	0x0c
-
+#define GSMTAP_TYPE_LTE_RRC	0x0d

 /* ====== DO NOT MAKE UNAPPROVED MODIFICATIONS HERE ===== */

@@ -131,6 +131,13 @@

 /* ====== DO NOT MAKE UNAPPROVED MODIFICATIONS HERE ===== */

+#define GSMTAP_LTE_CH_BCCH      0x01
+#define GSMTAP_LTE_CH_CCCH      0x02
+#define GSMTAP_LTE_CH_DCCH      0x03
+#define GSMTAP_LTE_CH_MCCH      0x04
+#define GSMTAP_LTE_CH_PCCH      0x05
+#define GSMTAP_LTE_CH_SCH       0x06
+
 #define GSMTAP_UMTS_CH_PCCH	0x01
 #define GSMTAP_UMTS_CH_CCCH	0x02

(Continue reading)

Re: [PATCH] add LTE defines to GSMTAP

This patch proposes extension to GSMTAP to support LTE frames - the same way it
supports UMTS. I've tried to pick defines to be compatible with wireshark.

Suggestions and extensions are highly appreciated.

--

-- 
best regards,
Max, http://fairwaves.co

Re: [PATCH] add LTE defines to GSMTAP

Hi Max,

@ttsou, you're pretty familiar with LTE right ? Can you give it a
quick look just to get a second opinion. (mine is pretty useless as my
lte knowledge is near zero).

> diff --git a/include/osmocom/core/gsmtap.h b/include/osmocom/core/gsmtap.h
> index c1c53d0..5d4755b 100644
> --- a/include/osmocom/core/gsmtap.h
> +++ b/include/osmocom/core/gsmtap.h
> @@ -43,7 +43,7 @@
>  #define GSMTAP_TYPE_GMR1_UM    0x0a    /* GMR-1 L2 packets */
>  #define GSMTAP_TYPE_UMTS_RLC_MAC       0x0b
>  #define GSMTAP_TYPE_UMTS_RRC   0x0c
> -
> +#define GSMTAP_TYPE_LTE_RRC    0x0d

The '-' is an unecessary whitespace change.

>  /* ====== DO NOT MAKE UNAPPROVED MODIFICATIONS HERE ===== */
>
> +#define GSMTAP_LTE_CH_BCCH      0x01
> +#define GSMTAP_LTE_CH_CCCH      0x02
> +#define GSMTAP_LTE_CH_DCCH      0x03
> +#define GSMTAP_LTE_CH_MCCH      0x04
> +#define GSMTAP_LTE_CH_PCCH      0x05
> +#define GSMTAP_LTE_CH_SCH       0x06
> +
>  #define GSMTAP_UMTS_CH_PCCH    0x01
>  #define GSMTAP_UMTS_CH_CCCH    0x02

(Continue reading)

Re: [PATCH] add LTE defines to GSMTAP

On Tue, Jun 10, 2014 at 9:10 AM, Max Suraev <Max.Suraev@fairwaves.co> wrote:
> +#define GSMTAP_LTE_CH_BCCH      0x01
> +#define GSMTAP_LTE_CH_CCCH      0x02
> +#define GSMTAP_LTE_CH_DCCH      0x03
> +#define GSMTAP_LTE_CH_MCCH      0x04
> +#define GSMTAP_LTE_CH_PCCH      0x05
> +#define GSMTAP_LTE_CH_SCH       0x06

The SCH is a transport channel upon which the other control channels
are mapped. Downlink or uplink SCH won't interact with RRC directly,
so it seems the logical traffic channels DTCH (Dedicated Traffic
Channel) and MTCH (Multicast Traffic Channel) are more appropriate
here.

  -TT

[PATCH] add LTE defines to GSMTAP v2

---
 include/osmocom/core/gsmtap.h | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/include/osmocom/core/gsmtap.h b/include/osmocom/core/gsmtap.h
index c1c53d0..5fac711 100644
--- a/include/osmocom/core/gsmtap.h
+++ b/include/osmocom/core/gsmtap.h
@@ -43,7 +43,7 @@
 #define GSMTAP_TYPE_GMR1_UM	0x0a	/* GMR-1 L2 packets */
 #define GSMTAP_TYPE_UMTS_RLC_MAC	0x0b
 #define GSMTAP_TYPE_UMTS_RRC	0x0c
-
+#define GSMTAP_TYPE_LTE_RRC	0x0d /* LTE interface */

 /* ====== DO NOT MAKE UNAPPROVED MODIFICATIONS HERE ===== */

@@ -135,6 +135,15 @@
 #define GSMTAP_UMTS_CH_CCCH	0x02
 #define GSMTAP_UMTS_CH_DCCH	0x03

+/* sub-types for TYPE_LTE_RRC */
+#define GSMTAP_LTE_CH_BCCH	0x01
+#define GSMTAP_LTE_CH_CCCH	0x02
+#define GSMTAP_LTE_CH_DCCH	0x03
+#define GSMTAP_LTE_CH_MCCH	0x04
+#define GSMTAP_LTE_CH_PCCH	0x05
+#define GSMTAP_LTE_CH_DTCH	0x06
+#define GSMTAP_LTE_CH_MTCH	0x07
+

(Continue reading)

Re: [PATCH] add LTE defines to GSMTAP v2

Thanks for your comments: incorporated @ttsou suggestion on SCH, fixed tab-vs-space.
As for whitespace change in the beginning - I think it's necessary to match usage of
====== header throughout the rest of the file.

--

-- 
best regards,
Max, http://fairwaves.co

Re: [PATCH] add LTE defines to GSMTAP v2

Signed-off-by: Sylvain Munaut <tnt@246tNt.com>

I'll give it a couple of days to make sure everybody interested has a
chance to view it, but if there are no other comments / suggestions in
the mean time, I'll push that to master.

Re: [PATCH] add LTE defines to GSMTAP v2

Applied.

BTW, you might want to include a signed-off-by line next time.

Cheers,

   Sylvain

Re: [PATCH] add LTE defines to GSMTAP v2

Thanks.
Is there some git option for that or it's just plain text in a long comment?

15.06.2014 18:58, Sylvain Munaut пишет:
> Applied.
> 
> BTW, you might want to include a signed-off-by line next time.
> 
> Cheers,
> 
>    Sylvain
> 

--

-- 
best regards,
Max, http://fairwaves.co

Re: [PATCH] add LTE defines to GSMTAP v2

On Sun, Jun 15, 2014 at 5:35 PM, ☎ <Max.Suraev@fairwaves.co> wrote:
> Is there some git option for that or it's just plain text in a long comment?

git commit --signoff

 -TT

Re: [PATCH] add LTE defines to GSMTAP v2

On Sun, Jun 15, 2014 at 11:35:16PM +0200, ☎ wrote:
> 15.06.2014 18:58, Sylvain Munaut пишет:
> > BTW, you might want to include a signed-off-by line next time.
> 
> Is there some git option for that or it's just plain text in a long 
> comment?

That's an option to 'git commit', it's '-s, --signoff', documented in 
the man page.

Kind regards,
-Alex

Re: [PATCH] add LTE defines to GSMTAP v2

Thanks for clarification guys. One more thing - was I supposed to put me or Sylvain
in there?

--

-- 
best regards,
Max, http://fairwaves.co

Re: [PATCH] add LTE defines to GSMTAP v2

> Thanks for clarification guys. One more thing - was I supposed to put me or Sylvain
> in there?

Yourself.

The Signed-off-by chain is essentially taking responsibility for the patch.

When taking a patch off the list, I'll add my own line to it to
basically say I looked over the patch, tested it and that to the best
of my knowledge the patch complies with the license.

Cheers,

   Sylvain