diff mbox

OsmoSGSN [PATCH], Network Service

Message ID CA+W2XBuGwG3Tg1E4Y9cdGXe2kftyzRJUwrVBiBS0yVEP5PgCOQ@mail.gmail.com
State Superseded
Headers show

Commit Message

Michal Grznár May 23, 2014, 9:44 a.m. UTC
Hi, I am sorry for my previous bad post format. There are the right diff
files.
And the problem was as I said in Imsi attach procedure new TLLI == new
allocated P-tmsi, and there was a problem that the function  gprs_tmsi2tlli()
function there was not called and so I had to mask the upper bits in
function where the p-tmsi is allocated, there is also a pcap trace where
you can see it.

thanks, regards
Michal


2014-05-17 13:50 GMT+02:00 Harald Welte <laforge@gnumonks.org>:

> Hi Michal,
>
> thanks a lot for reporting back on your experience.
>
> Hoewever, it is customary to post changes as unified diff ('diff -u'
> format here on this list for review.  I have a hard time understanding
> the format that you posted.  Can you pleaes re-post your changes?
> Either with diff -u created manually, or with 'git diff' or similar.
>
> On Mon, May 05, 2014 at 11:13:37PM +0200, Michal Grznár wrote:
> > Hi, I am using OsmoSGSN in topology with OpenGGSN and proprietary
> simulator
> > and the problem is that in IP-subnetwork, which I am using there is no
> > use for RESET or UNBLOCK procedure, so I had to do a PATCH in
> > gprs_ns.c, which was needed to complete succesful connection between
> > sim-bss and OsmoSGSN:
>
> If you would like to see such code merged, please extend osmo-sgsn so
> that the type of BSS can be configured via vty,  and make the code
> conditional.  At that point you could simply have a different config
> file for your sim-bss than we have.
>
> > another PATCH I needed to do was to change a little bit procedure for
> > allocation of P-TMSI in procedure uint32_t sgsn_alloc_ptmsi(void) in
> > gprs_sgsn.c
> >
> > uint32_t sgsn_alloc_ptmsi(void)
> > {
> >       struct sgsn_mm_ctx *mm;
> >       uint32_t ptmsi;
> >
> > restart:
> > +++    ptmsi = rand() | 0xc0000000;              /*because of GPRS IMSI
> > ATTACH*/
> >           llist_for_each_entry(mm, &sgsn_mm_ctxts, list) {
> >                  if (mm->p_tmsi == ptmsi)
> >                           goto restart;
> >           }
> >           return ptmsi;
> > }
> >
> > because in GPRS IMSI ATTACH in message ATTACH COMPLETE (3GPP 24.008,
> > 23.003, 48.018) there is new TLLI==new allocated P-TMSI and I need local
> > TLLI, so I had to do it this way
>
> This is most certainly not the right way to fix the problem.  The TLLI
> is generated at a different layer than the P-TMSI.  The P-TMSI should be
> completely random, and the TLLI derived from that should mask the upper
> bits, using the gprs_tmsi2tlli() function.
>
> Can you please explain more what the actual problem was and how it
> manifested iself?  If possible, include pcap traces for further
> illustration.
>
> Thanks,
>         Harald
>
> --
> - Harald Welte <laforge@gnumonks.org>
> http://laforge.gnumonks.org/
>
> ============================================================================
> "Privacy in residential applications is a desirable marketing option."
>                                                   (ETSI EN 300 175-7 Ch.
> A6)
>

--- gprs_sgsn_before_patch.c	2014-05-23 11:14:48.121829000 +0200
+++ gprs_sgsn_patch.c	2014-05-23 11:15:48.229829001 +0200
@@ -361,7 +361,7 @@
 	uint32_t ptmsi;
 
 restart:
-	ptmsi = rand();
+	ptmsi = rand() | 0xc0000000; //because of GPRS IMSI ATTACH
 	llist_for_each_entry(mm, &sgsn_mm_ctxts, list) {
 		if (mm->p_tmsi == ptmsi)
 			goto restart;

Comments

Holger Freyther May 23, 2014, 11:16 a.m. UTC | #1
On Fri, May 23, 2014 at 11:44:40AM +0200, Michal Grznár wrote:

Hi,

> And the problem was as I said in Imsi attach procedure new TLLI == new
> allocated P-tmsi, and there was a problem that the function  gprs_tmsi2tlli()
> function there was not called and so I had to mask the upper bits in
> function where the p-tmsi is allocated, there is also a pcap trace where
> you can see it.

Could you please elaborate of what/were (e.g. packet numbers) we
can see "it" and what it should be instead? And please use "git diff"
or preferable "git commit" and git format-patch. The "diff" you include
is hand-written and sadly not usable because of this.

And as written by Harald before. The place you patch is not correct.
The method you patch should generate a unique P-TMSI. It might should
mask some of the higher bits. But you need to look at the callers of
this function if the tlli is not updated.

e.g. in src/gprs/gprs_gmm.c you will see something like this:

        ctx->p_tmsi = sgsn_alloc_ptmsi();
#endif

        /* Even if there is no P-TMSI allocated, the MS will switch from
         * foreign TLLI to local TLLI */
        ctx->tlli_new = gprs_tmsi2tlli(ctx->p_tmsi, TLLI_LOCAL);

        /* Inform LLC layer about new TLLI but keep old active */
        gprs_llgmm_assign(ctx->llme, ctx->tlli, ctx->tlli_new,
                          GPRS_ALGO_GEA0, NULL);

So this call to gprs_tmsi2tlli will make sure that 0xc0000000 will
be set. In fact I see two calls to sgsn_alloc_ptmsi and both of them
do the above and assign the new tlli to the context. So please could
you try to explain what you are trying to solve?

holger
Holger Freyther Aug. 5, 2014, 1:18 p.m. UTC | #2
On Fri, May 23, 2014 at 11:44:40AM +0200, Michal Grznár wrote:

Hi,


> Hi, I am sorry for my previous bad post format. There are the right diff
> files.
> And the problem was as I said in Imsi attach procedure new TLLI == new
> allocated P-tmsi, and there was a problem that the function  gprs_tmsi2tlli()
> function there was not called and so I had to mask the upper bits in
> function where the p-tmsi is allocated, there is also a pcap trace where
> you can see it.

sorry for the late reply. The issue is that your MS does not convert the
P-TMSI to a Local-TLLI. On the other hand Jacob has pointed me to some
documentation of a measurement equipment  manufacturer that states that the
local bits/highest two bits of the ptmsi should be set.


> > > restart:
> > > +++    ptmsi = rand() | 0xc0000000;              /*because of GPRS IMSI

we will carry a patch like this.
diff mbox

Patch

--- gprs_ns_before_patch.c	2014-05-23 10:36:09.041829000 +0200
+++ gprs_ns_patch.c	2014-05-23 10:34:17.101829001 +0200
@@ -1217,16 +1217,14 @@ 
 
 	switch (nsh->pdu_type) {
 	case NS_PDUT_ALIVE:
-		/* If we're dead and blocked and suddenly receive a
-		 * NS-ALIVE out of the blue, we might have been re-started
-		 * and should send a NS-RESET to make sure everything recovers
-		 * fine. */
-
-LOGP(DNS, LOGL_INFO, "Rx NS ALIVE\n");		
-if ((*nsvc)->state == NSE_S_BLOCKED)
-			rc = gprs_ns_tx_reset((*nsvc), NS_CAUSE_PDU_INCOMP_PSTATE);
-		else
-			rc = gprs_ns_tx_alive_ack(*nsvc);
+		LOGP(DNS, LOGL_INFO, "Rx NS ALIVE\n");
+		rc = gprs_ns_tx_alive_ack(*nsvc);
+		/*mark NS-VC as unblocked and active*/
+      		(*nsvc)->state = NSE_S_ALIVE;
+		(*nsvc)->remote_state = NSE_S_ALIVE;
+		/*Initiate TEST proc.: Send ALIVE_ACK and start timer*/
+		rc = gprs_ns_tx_simple((*nsvc), NS_PDUT_ALIVE_ACK);
+		nsvc_start_timer((*nsvc), NSVC_TIMER_TNS_TEST);
 		break;
 	case NS_PDUT_ALIVE_ACK:
 		/* stop Tns-alive and start Tns-test */